Skip to content

Commit

Permalink
Fix JENKINS-29970
Browse files Browse the repository at this point in the history
Add check for non-AbstractBuild stuff
  • Loading branch information
slide committed Sep 7, 2015
1 parent f57b2be commit 4659ead
Show file tree
Hide file tree
Showing 10 changed files with 148 additions and 111 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -4,7 +4,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>1.580.1</version>
<version>1.609.2</version>

This comment has been minimized.

Copy link
@jglick

jglick Sep 8, 2015

Member

Out of curiosity, was this core update needed for some new API?

This comment has been minimized.

Copy link
@slide

slide Sep 8, 2015

Author Member

No, it slipped through in the commits actually, but I do tend to try and track the LTS releases.

</parent>

<licenses>
Expand Down
18 changes: 11 additions & 7 deletions src/main/java/hudson/plugins/emailext/EmailRecipientUtils.java
@@ -1,6 +1,7 @@
package hudson.plugins.emailext;

import java.io.UnsupportedEncodingException;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.Set;
import java.util.logging.Logger;
Expand Down Expand Up @@ -139,19 +140,22 @@ private static String fixupSpaces(String input) {

public static boolean isExcludedRecipient(String userName, TaskListener listener) {
ExtendedEmailPublisherDescriptor descriptor = Jenkins.getInstance().getDescriptorByType(ExtendedEmailPublisherDescriptor.class);
StringTokenizer tokens = new StringTokenizer(descriptor.getExcludedCommitters(), ",");
while (tokens.hasMoreTokens()) {
String check = tokens.nextToken().trim();
descriptor.debug(listener.getLogger(), "Checking '%s' against '%s' to see if they are excluded", userName, check);
if (check.equalsIgnoreCase(userName)) {
return true;
if(descriptor.getExcludedCommitters() != null) {
StringTokenizer tokens = new StringTokenizer(descriptor.getExcludedCommitters(), ", ");
while (tokens.hasMoreTokens()) {
String check = tokens.nextToken().trim();
descriptor.debug(listener.getLogger(), "Checking '%s' against '%s' to see if they are excluded", userName, check);
if (check.equalsIgnoreCase(userName)) {
return true;
}
}
}
return false;
}

public static boolean isExcludedRecipient(User user, TaskListener listener) {
String[] testValues = new String[] { user.getFullName(), user.getId(), user.getDisplayName() };
Mailer.UserProperty prop = user.getProperty(Mailer.UserProperty.class);
String[] testValues = new String[] { user.getFullName(), user.getId(), user.getDisplayName(), prop != null ? prop.getAddress() : null };
for(String testValue : testValues) {
if(testValue != null && isExcludedRecipient(testValue, listener)) {
return true;
Expand Down
Expand Up @@ -64,8 +64,6 @@ public class ExtendedEmailPublisher extends Notifier implements MatrixAggregatab

private static final String CONTENT_TRANSFER_ENCODING = System.getProperty(ExtendedEmailPublisher.class.getName() + ".Content-Transfer-Encoding");

public static final String DEFAULT_RECIPIENTS_TEXT = "";

public static final String DEFAULT_SUBJECT_TEXT = "$PROJECT_NAME - Build # $BUILD_NUMBER - $BUILD_STATUS!";

public static final String DEFAULT_BODY_TEXT = "$PROJECT_NAME - Build # $BUILD_NUMBER - $BUILD_STATUS:\n\n"
Expand Down
Expand Up @@ -33,7 +33,7 @@ public CulpritsRecipientProvider() {

@Override
public void addRecipients(ExtendedEmailPublisherContext context, EnvVars env, Set<InternetAddress> to, Set<InternetAddress> cc, Set<InternetAddress> bcc) {
ExtendedEmailPublisherDescriptor descriptor = Jenkins.getInstance().getDescriptorByType(ExtendedEmailPublisherDescriptor.class);
ExtendedEmailPublisherDescriptor descriptor = Jenkins.getInstance().getDescriptorByType(ExtendedEmailPublisherDescriptor.class);
Set<User> users = context.getBuild().getCulprits();

for (User user : users) {
Expand Down
Expand Up @@ -84,25 +84,20 @@ public void send(final String format, final Object... args) {
} else {
users = new HashSet<User>();
debug.send("Collecting builds where a test started failing...");
final HashSet<AbstractBuild<?, ?>> buildsWhereATestStartedFailing = new HashSet<AbstractBuild<?, ?>>();
final HashSet<Run<?, ?>> buildsWhereATestStartedFailing = new HashSet<Run<?, ?>>();
for (final TestResult caseResult : testResultAction.getFailedTests()) {
final Run<?, ?> runWhereTestStartedFailing = caseResult.getFailedSinceRun();
if (runWhereTestStartedFailing instanceof AbstractBuild) {
final AbstractBuild<?, ?> buildWhereTestStartedFailing = (AbstractBuild<?, ?>) runWhereTestStartedFailing;
debug.send(" buildWhereTestStartedFailing: %d", buildWhereTestStartedFailing.getNumber());
buildsWhereATestStartedFailing.add(buildWhereTestStartedFailing);
} else {
debug.send(" runWhereTestStartedFailing was not an instance of AbstractBuild");
}
debug.send(" runWhereTestStartedFailing: %d", runWhereTestStartedFailing.getNumber());
buildsWhereATestStartedFailing.add(runWhereTestStartedFailing);
}
// For each build where a test started failing, walk backward looking for build results worse than
// UNSTABLE. All of those builds will be used to find suspects.
debug.send("Collecting builds with suspects...");
final HashSet<AbstractBuild<?, ?>> buildsWithSuspects = new HashSet<AbstractBuild<?, ?>>();
for (final AbstractBuild<?, ?> buildWhereATestStartedFailing : buildsWhereATestStartedFailing) {
final HashSet<Run<?, ?>> buildsWithSuspects = new HashSet<Run<?, ?>>();
for (final Run<?, ?> buildWhereATestStartedFailing : buildsWhereATestStartedFailing) {
debug.send(" buildWhereATestStartedFailing: %d", buildWhereATestStartedFailing.getNumber());
buildsWithSuspects.add(buildWhereATestStartedFailing);
AbstractBuild<?, ?> previousBuildToCheck = buildWhereATestStartedFailing.getPreviousCompletedBuild();
Run<?, ?> previousBuildToCheck = buildWhereATestStartedFailing.getPreviousCompletedBuild();
if (previousBuildToCheck != null) {
debug.send(" previousBuildToCheck: %d", previousBuildToCheck.getNumber());
}
Expand Down
Expand Up @@ -80,15 +80,15 @@ public void send(final String format, final Object... args) {
} else {
users = new HashSet<User>();
debug.send("Collecting builds with suspects...");
final HashSet<AbstractBuild<?, ?>> buildsWithSuspects = new HashSet<AbstractBuild<?, ?>>();
final HashSet<Run<?, ?>> buildsWithSuspects = new HashSet<Run<?, ?>>();
Run<?, ?> firstFailedBuild = currentBuild;
Run<?, ?> candidate = currentBuild;
while (candidate != null && candidate.getResult().isWorseOrEqualTo(Result.FAILURE)) {
firstFailedBuild = candidate;
candidate = candidate.getPreviousCompletedBuild();
}
if (firstFailedBuild instanceof AbstractBuild) {

This comment has been minimized.

Copy link
@vtintillier

vtintillier Jun 1, 2018

Member

Hello @slide

shouldn't this if (firstFailedBuild instanceof AbstractBuild) { check be removed now that buildsWithSuspects is a collection of Runs ?

I was investigating wwhy I did not get emails for build failures when using brokenBuildSuspects() in a pipeline, and I saw that debug message: firstFailedBuild was not an instance of AbstractBuild

Should I open an issue and a PR?

Thanks

This comment has been minimized.

Copy link
@slide

slide Jun 1, 2018

Author Member

I don't maintain this plugin anymore

This comment has been minimized.

Copy link
@davidvanlaatum

davidvanlaatum Jun 2, 2018

Contributor

yes if you can prove that it works

buildsWithSuspects.add((AbstractBuild<?, ?>) firstFailedBuild);
buildsWithSuspects.add(firstFailedBuild);
} else {
debug.send(" firstFailedBuild was not an instance of AbstractBuild");
}
Expand Down
Expand Up @@ -23,20 +23,21 @@
*/
package hudson.plugins.emailext.plugins.recipients;

import java.lang.reflect.Field;
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;
import java.util.logging.Logger;

import javax.mail.internet.InternetAddress;

import hudson.EnvVars;
import hudson.model.AbstractBuild;
import hudson.model.TaskListener;
import hudson.model.User;
import hudson.model.*;
import hudson.plugins.emailext.EmailRecipientUtils;
import hudson.scm.ChangeLogSet;

public final class RecipientProviderUtilities {
private static final Logger LOGGER = Logger.getLogger(RecipientProviderUtilities.class.getName());

private RecipientProviderUtilities() {
}
Expand All @@ -45,34 +46,36 @@ public interface IDebug {
void send(final String format, final Object... args);
}

public static Set<User> getChangeSetAuthors(final Collection<AbstractBuild<?, ?>> builds, final IDebug debug) {
public static Set<User> getChangeSetAuthors(final Collection<Run<?, ?>> runs, final IDebug debug) {
debug.send(" Collecting change authors...");
final Set<User> users = new HashSet<User>();
for (final AbstractBuild<?, ?> build : builds) {
debug.send(" build: %d", build.getNumber());
final ChangeLogSet<?> changeLogSet = build.getChangeSet();
if (changeLogSet == null) {
debug.send(" changeLogSet was null");
} else {
final Set<User> changeAuthors = new HashSet<User>();
for (final ChangeLogSet.Entry change : changeLogSet) {
final User changeAuthor = change.getAuthor();
if (changeAuthors.add(changeAuthor)) {
debug.send(" adding author: %s", changeAuthor.getFullName());
for (final Run<?, ?> run : runs) {
debug.send(" build: %d", run.getNumber());
if(run instanceof AbstractBuild<?,?>) {
final ChangeLogSet<?> changeLogSet = ((AbstractBuild<?,?>)run).getChangeSet();
if (changeLogSet == null) {
debug.send(" changeLogSet was null");
} else {
final Set<User> changeAuthors = new HashSet<User>();
for (final ChangeLogSet.Entry change : changeLogSet) {
final User changeAuthor = change.getAuthor();
if (changeAuthors.add(changeAuthor)) {
debug.send(" adding author: %s", changeAuthor.getFullName());
}
}
users.addAll(changeAuthors);
}
users.addAll(changeAuthors);
}
}
return users;
}

public static Set<User> getUsersTriggeringTheBuilds(final Collection<AbstractBuild<?, ?>> builds, final IDebug debug) {
public static Set<User> getUsersTriggeringTheBuilds(final Collection<Run<?, ?>> runs, final IDebug debug) {
debug.send(" Collecting build requestors...");
final Set<User> users = new HashSet<User>();
for (final AbstractBuild<?, ?> build : builds) {
debug.send(" build: %d", build.getNumber());
final User buildRequestor = RequesterRecipientProvider.getUserTriggeringTheBuild(build);
for (final Run<?, ?> run : runs) {
debug.send(" build: %d", run.getNumber());
final User buildRequestor = getUserTriggeringTheBuild(run);
if (buildRequestor != null) {
debug.send(" adding requestor: %s", buildRequestor.getFullName());
users.add(buildRequestor);
Expand All @@ -83,6 +86,46 @@ public static Set<User> getUsersTriggeringTheBuilds(final Collection<AbstractBui
return users;
}

private static User getByUserIdCause(Run<?, ?> run) {
try {
Cause.UserIdCause cause = run.getCause(Cause.UserIdCause.class);
if (cause != null) {
String id = cause.getUserId();
return User.get(id, false, null);
}

} catch (Exception e) {
LOGGER.info(e.getMessage());
}
return null;
}

@SuppressWarnings("deprecated")
private static User getByLegacyUserCause(Run<?, ?> run) {
try {
Cause.UserCause userCause = run.getCause(Cause.UserCause.class);
// userCause.getUserName() returns displayName which may be different from authentication name
// Therefore use reflection to access the real authenticationName
if (userCause != null) {
Field authenticationName = Cause.UserCause.class.getDeclaredField("authenticationName");
authenticationName.setAccessible(true);
String name = (String) authenticationName.get(userCause);
return User.get(name, false, null);
}
} catch (Exception e) {
LOGGER.info(e.getMessage());
}
return null;
}

public static User getUserTriggeringTheBuild(final Run<?, ?> run) {
User user = getByUserIdCause(run);
if (user == null) {
user = getByLegacyUserCause(run);
}
return user;
}

public static void addUsers(final Set<User> users, final TaskListener listener, final EnvVars env,
final Set<InternetAddress> to, final Set<InternetAddress> cc, final Set<InternetAddress> bcc, final IDebug debug) {
for (final User user : users) {
Expand All @@ -101,5 +144,4 @@ public static void addUsers(final Set<User> users, final TaskListener listener,
}
}
}

}
@@ -1,15 +1,11 @@
package hudson.plugins.emailext.plugins.recipients;

import hudson.model.*;
import hudson.plugins.emailext.EmailRecipientUtils;
import hudson.plugins.emailext.plugins.RecipientProviderDescriptor;
import hudson.plugins.emailext.plugins.RecipientProvider;
import hudson.EnvVars;
import hudson.Extension;
import hudson.model.AbstractBuild;
import hudson.model.AbstractProject;
import hudson.model.Cause;
import hudson.model.TaskListener;
import hudson.model.User;
import hudson.plugins.emailext.ExtendedEmailPublisherContext;
import hudson.tasks.Mailer;
import java.lang.reflect.Field;
Expand All @@ -34,12 +30,13 @@ public RequesterRecipientProvider() {
@Override
public void addRecipients(ExtendedEmailPublisherContext context, EnvVars env, Set<InternetAddress> to, Set<InternetAddress> cc, Set<InternetAddress> bcc) {
// looking for Upstream build.
AbstractBuild<?, ?> cur = context.getBuild();
Run<?, ?> cur = context.getBuild();
Cause.UpstreamCause upc = context.getBuild().getCause(Cause.UpstreamCause.class);
while (upc != null) {
// UpstreamCause.getUpStreamProject() returns the full name, so use getItemByFullName
AbstractProject<?, ?> p = (AbstractProject<?, ?>) Jenkins.getInstance().getItemByFullName(upc.getUpstreamProject());
Job<?, ?> p = (Job<?, ?>) Jenkins.getInstance().getItemByFullName(upc.getUpstreamProject());
if (p == null) {
context.getListener().getLogger().print("There is a break in the project linkage, could not retrieve upstream project information");
break;
}
cur = p.getBuildByNumber(upc.getUpstreamBuild());
Expand All @@ -48,18 +45,10 @@ public void addRecipients(ExtendedEmailPublisherContext context, EnvVars env, Se
addUserTriggeringTheBuild(cur, to, cc, bcc, env, context.getListener());
}

public static User getUserTriggeringTheBuild(final AbstractBuild<?, ?> build) {
User user = getByUserIdCause(build);
if (user == null) {
user = getByLegacyUserCause(build);
}
return user;
}

private static void addUserTriggeringTheBuild(AbstractBuild<?, ?> build, Set<InternetAddress> to,
private static void addUserTriggeringTheBuild(Run<?, ?> run, Set<InternetAddress> to,
Set<InternetAddress> cc, Set<InternetAddress> bcc, EnvVars env, TaskListener listener) {

final User user = getUserTriggeringTheBuild(build);
final User user = RecipientProviderUtilities.getUserTriggeringTheBuild(run);
if (user != null) {
String adrs = user.getProperty(Mailer.UserProperty.class).getAddress();
if (adrs != null) {
Expand All @@ -72,37 +61,7 @@ private static void addUserTriggeringTheBuild(AbstractBuild<?, ?> build, Set<Int
}

@SuppressWarnings("unchecked")
private static User getByUserIdCause(AbstractBuild<?, ?> build) {
try {
Cause.UserIdCause cause = build.getCause(Cause.UserIdCause.class);
if (cause != null) {
String id = cause.getUserId();
return User.get(id, false, null);
}

} catch (Exception e) {
LOGGER.info(e.getMessage());
}
return null;
}

@SuppressWarnings("deprecated")
private static User getByLegacyUserCause(AbstractBuild<?, ?> build) {
try {
Cause.UserCause userCause = build.getCause(Cause.UserCause.class);
// userCause.getUserName() returns displayName which may be different from authentication name
// Therefore use reflection to access the real authenticationName
if (userCause != null) {
Field authenticationName = Cause.UserCause.class.getDeclaredField("authenticationName");
authenticationName.setAccessible(true);
String name = (String) authenticationName.get(userCause);
return User.get(name, false, null);
}
} catch (Exception e) {
LOGGER.info(e.getMessage());
}
return null;
}

@Extension
public static final class DescriptorImpl extends RecipientProviderDescriptor {
Expand Down

0 comments on commit 4659ead

Please sign in to comment.