Skip to content

Commit

Permalink
Add Perforce User's email to Jenkins User property.
Browse files Browse the repository at this point in the history 
JENKINS-28421
  • Loading branch information
@p4paul
p4paul committed Aug 14, 2015
1 parent 360b789 commit 5f936ad
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
6 changes: 6 additions & 0 deletions src/main/java/org/jenkinsci/plugins/p4/changes/P4ChangeEntry.java
View file
Expand Up @@ -2,6 +2,7 @@

import hudson.model.User;
import hudson.scm.ChangeLogSet;
import hudson.tasks.Mailer.UserProperty;

import java.text.ParseException;
import java.text.SimpleDateFormat;
Expand Down Expand Up @@ -60,6 +61,11 @@ public void setChange(ConnectionHelper p4, int changeId) throws Exception {
String user = changelist.getUsername();
author = User.get(user);

// set email property on user
String email = p4.getEmail(user);
UserProperty userProp = new UserProperty(email);
author.addProperty(userProp);

This comment has been minimized.

Sign in to view

Copy link
@daniel-beck

daniel-beck Aug 15, 2015

Member

This probably shouldn't be done unconditionally. Users may have configured other addresses already. An implementation of MailAddressResolver is probably the better approach.

This comment has been minimized.

Sign in to view

Copy link
@p4paul

p4paul Aug 17, 2015

Author Contributor

I tried to use MailAddressResolver however the SCM is only in scope within the context of a Job/Build. Jenkins deployments may have more than one SCM system, so it is not obvious which one to contact to get the user's email.
Perhaps I could extend UserProperty to save a 'Perforce email' during the P4ChangeEntry, then retrieve this with MailAddressResolver.

This comment has been minimized.

Sign in to view

Copy link
@daniel-beck

daniel-beck Aug 19, 2015

Member

@p4paul Maybe use this to add data to a Jenkins global user/email address map which is the data source for a MailAddressResolver rather than setting the property?

Mailer.UserProperty has associated UI, so you don't want multiple instances of that.

Alternatively,y define your own (invisible) UserProperty that records the perforce email address for a user, and use that as data source for your MailAddressResolver. It would still edit user records during changelog computation, which is weird, but at least wouldn't reuse the regular Mailer.UserProperty in this way.

This comment has been minimized.

Sign in to view

Copy link
@p4paul

p4paul Aug 19, 2015

Author Contributor

@daniel-beck Hi Daniel, I must admit I thought I was adding to Jenkins global user/email address map; it seemed that way when I tested it.

The Perforce User object is only visible during a build or processing a change. Perhaps moving this into the Build step makes more sense. As browsing to 'asynchPeople/' is now very slow as it scans the changes.

This comment has been minimized.

Sign in to view

Copy link
@daniel-beck

daniel-beck Aug 19, 2015

Member

I must admit I thought I was adding to Jenkins global user/email address map; it seemed that way when I tested it.

You were basically editing the user record. My suggestion is to keep the data separate from that, in memory only (not persisted) and not edit the user record. Then implement MailAddressResolver that uses this separate data structure as data source. If a user is in there, return the recorded address.

This comment has been minimized.

Sign in to view

Copy link
@daniel-beck

daniel-beck Aug 19, 2015

Member

I think just editing user records during changelog computation would be unexpected behavior, while just gathering data for dynamic lookup would not be.

// set date of change
date = changelist.getDate();

Expand Down
9 changes: 8 additions & 1 deletion src/main/java/org/jenkinsci/plugins/p4/client/ConnectionHelper.java
View file
Expand Up @@ -25,6 +25,7 @@
import com.perforce.p4java.client.IClient;
import com.perforce.p4java.core.IChangelist;
import com.perforce.p4java.core.ILabel;
import com.perforce.p4java.core.IUser;
import com.perforce.p4java.core.file.FileSpecBuilder;
import com.perforce.p4java.core.file.FileSpecOpStatus;
import com.perforce.p4java.core.file.IFileSpec;
Expand All @@ -44,7 +45,7 @@ public class ConnectionHelper {
.getName());

private boolean abort = false;

protected final ConnectionConfig connectionConfig;
protected final AuthorisationConfig authorisationConfig;
protected IOptionsServer connection;
Expand Down Expand Up @@ -290,6 +291,12 @@ public boolean isClient(String name) throws Exception {
}
}

public String getEmail(String userName) throws Exception {
IUser user = connection.getUser(userName);
String email = user.getEmail();
return email;
}

/**
* Get Perforce Label
*
Expand Down

0 comments on commit 5f936ad

Please sign in to comment.