Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-13324

Perforce mail address resolver should fall back to other resolvers if mail address is invalid

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Major
    • Resolution: Fixed
    • Component/s: perforce-plugin
    • Labels:
      None
    • Environment:
      Perforce 2011.1
      perforce-plugin 1.3.12
    • Similar Issues:

      Description

      Perforce mail address resolver is always returning a string, no matter what the actual email address is. I don't know what is the order how Jenkins loops through the various MailAddressResolver instances but if some instance returns an actual string (i.e. non-null), it will stick with that. I don't think it's feasible to really check whether the email address provided by Perforce is valid, but PerforceMailResolver could check for some value (e.g. "n/a") and return null if it matches. At least with Perforce 2011.1 you can't have an empty value for an email address.

      There could be an UI element for configuring which email address would be thought as invalid, but I'm fine with hardcoding "n/a" there and documenting it somewhere. Would this be ok?

        Attachments

          Activity

          Hide
          rpetti Rob Petti added a comment -

          PerforceMailResolver will return null if no email address is found. The problem as you said is that perforce will always have an email set for users, and will invent an email address if none is supplied (usually user@host or user@client).

          I don't really have a problem with hard coding some value such as 'n/a', assuming that perforce will accept it as a valid email address. Though I question the need for it, since perforce admins will need to forcefully reset all the email addresses to that value anyways. If someone needs to go through all that trouble to set email addresses to 'n/a', why not just set the email addresses correctly instead?

          Show
          rpetti Rob Petti added a comment - PerforceMailResolver will return null if no email address is found. The problem as you said is that perforce will always have an email set for users, and will invent an email address if none is supplied (usually user@host or user@client). I don't really have a problem with hard coding some value such as 'n/a', assuming that perforce will accept it as a valid email address. Though I question the need for it, since perforce admins will need to forcefully reset all the email addresses to that value anyways. If someone needs to go through all that trouble to set email addresses to 'n/a', why not just set the email addresses correctly instead?
          Hide
          miktap Mikko Tapaninen added a comment -

          PerforceMailResolver will return null if no email address is found. The problem as you said is that perforce will always have an email set for users, and will invent an email address if none is supplied (usually user@host or user@client).

          True. But if it finds a Perforce user then it will return some string. And that's a problem for us because we can't have all the email addresses in Perforce. We should look them from LDAP instead. For this reason some predefined string could be interpreted as null.

          Show
          miktap Mikko Tapaninen added a comment - PerforceMailResolver will return null if no email address is found. The problem as you said is that perforce will always have an email set for users, and will invent an email address if none is supplied (usually user@host or user@client). True. But if it finds a Perforce user then it will return some string. And that's a problem for us because we can't have all the email addresses in Perforce. We should look them from LDAP instead. For this reason some predefined string could be interpreted as null.
          Hide
          rpetti Rob Petti added a comment -

          Rather than using a predefined string and having to set every single perforce account email to it, why not just add a global option to disable the PerforceMailResolver?

          Show
          rpetti Rob Petti added a comment - Rather than using a predefined string and having to set every single perforce account email to it, why not just add a global option to disable the PerforceMailResolver?
          Hide
          miktap Mikko Tapaninen added a comment -

          That's an alternative. But we also have Perforce specific accounts (buildbots or other robot accounts) and for those we could store email to Perforce. I guess it's not vital to resolve email addresses for these.

          It's easier for me to use predefined stting but a global option would be simpler for an end user.

          Show
          miktap Mikko Tapaninen added a comment - That's an alternative. But we also have Perforce specific accounts (buildbots or other robot accounts) and for those we could store email to Perforce. I guess it's not vital to resolve email addresses for these. It's easier for me to use predefined stting but a global option would be simpler for an end user.
          Hide
          rpetti Rob Petti added a comment -

          Alright, let's go for a string for now then. Is 'n/a' actually accepted as a valid email by perforce?

          Show
          rpetti Rob Petti added a comment - Alright, let's go for a string for now then. Is 'n/a' actually accepted as a valid email by perforce?
          Hide
          miktap Mikko Tapaninen added a comment -

          Yep, 'n/a' is accepted. At least with 2011.1.

          Show
          miktap Mikko Tapaninen added a comment - Yep, 'n/a' is accepted. At least with 2011.1.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Rob Petti
          Path:
          src/main/java/hudson/plugins/perforce/PerforceMailResolver.java
          http://jenkins-ci.org/commit/perforce-plugin/1986bb15c18ccbae3f5c2e597db1a549691f4504
          Log:
          [FIXED JENKINS-13324] don't return invalid email addresses in mailresolver

          it will now check that the email address matches '.@.' before returning it to jenkins

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Rob Petti Path: src/main/java/hudson/plugins/perforce/PerforceMailResolver.java http://jenkins-ci.org/commit/perforce-plugin/1986bb15c18ccbae3f5c2e597db1a549691f4504 Log: [FIXED JENKINS-13324] don't return invalid email addresses in mailresolver it will now check that the email address matches '. @. ' before returning it to jenkins
          Hide
          dogfood dogfood added a comment -

          Integrated in plugins_perforce #219
          [FIXED JENKINS-13324] don't return invalid email addresses in mailresolver (Revision 1986bb15c18ccbae3f5c2e597db1a549691f4504)

          Result = SUCCESS
          Rob Petti :
          Files :

          • src/main/java/hudson/plugins/perforce/PerforceMailResolver.java
          Show
          dogfood dogfood added a comment - Integrated in plugins_perforce #219 [FIXED JENKINS-13324] don't return invalid email addresses in mailresolver (Revision 1986bb15c18ccbae3f5c2e597db1a549691f4504) Result = SUCCESS Rob Petti : Files : src/main/java/hudson/plugins/perforce/PerforceMailResolver.java

            People

            • Assignee:
              Unassigned
              Reporter:
              miktap Mikko Tapaninen
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: