Skip to content

Commit

Permalink
Merge pull request #2114 from jglick/CauseAction-JENKINS-33467
Browse files Browse the repository at this point in the history 
[JENKINS-33467] Do not store duplicate Cause’s
  • Loading branch information
@daniel-beck
daniel-beck committed Mar 11, 2016
2 parents c5a3b2f + 6ae54ad commit ed564e6
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 49 deletions.
31 changes: 10 additions & 21 deletions core/src/main/java/hudson/model/Cause.java
View file
Expand Up @@ -37,6 +37,7 @@
import hudson.Util;
import java.io.IOException;
import java.util.HashSet;
import java.util.Objects;
import java.util.Set;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
Expand Down Expand Up @@ -212,25 +213,18 @@ public boolean equals(Object rhs) {

final UpstreamCause o = (UpstreamCause) rhs;

if (upstreamBuild != o.upstreamBuild) return false;
if (!upstreamCauses.equals(o.upstreamCauses)) return false;
if (upstreamUrl == null ? o.upstreamUrl != null : !upstreamUrl.equals(o.upstreamUrl)) return false;
if (upstreamProject == null ? o.upstreamProject != null : !upstreamProject.equals(o.upstreamProject)) return false;

return true;
return Objects.equals(upstreamBuild, o.upstreamBuild) &&
Objects.equals(upstreamCauses, o.upstreamCauses) &&
Objects.equals(upstreamUrl, o.upstreamUrl) &&
Objects.equals(upstreamProject, o.upstreamProject);
}

/**
* @since 1.515
*/
@Override
public int hashCode() {

int hashCode = 17;
hashCode = hashCode * 31 + upstreamCauses.hashCode();
hashCode = hashCode * 31 + upstreamBuild;
hashCode = hashCode * 31 + (upstreamUrl == null ? 0 : upstreamUrl.hashCode ());
return hashCode * 31 + (upstreamProject == null ? 0 : upstreamProject.hashCode ());
return Objects.hash(upstreamCauses, upstreamBuild, upstreamUrl, upstreamProject);
}

private @Nonnull Cause trim(@Nonnull Cause c, int depth, Set<String> traversed) {
Expand Down Expand Up @@ -430,13 +424,12 @@ public void print(TaskListener listener) {

@Override
public boolean equals(Object o) {
return o instanceof UserIdCause && Arrays.equals(new Object[]{userId},
new Object[]{((UserIdCause) o).userId});
return o instanceof UserIdCause && Objects.equals(userId, ((UserIdCause) o).userId);
}

@Override
public int hashCode() {
return 295 + (this.userId != null ? this.userId.hashCode() : 0);
return Objects.hash(userId);
}
}

Expand Down Expand Up @@ -473,16 +466,12 @@ public String getNote() {

@Override
public boolean equals(Object o) {
return o instanceof RemoteCause && Arrays.equals(new Object[] {addr, note},
new Object[] {((RemoteCause)o).addr, ((RemoteCause)o).note});
return o instanceof RemoteCause && Objects.equals(addr, ((RemoteCause) o).addr) && Objects.equals(note, ((RemoteCause) o).note);
}

@Override
public int hashCode() {
int hash = 5;
hash = 83 * hash + (this.addr != null ? this.addr.hashCode() : 0);
hash = 83 * hash + (this.note != null ? this.note.hashCode() : 0);
return hash;
return Objects.hash(addr, note);
}
}
}
70 changes: 48 additions & 22 deletions core/src/main/java/hudson/model/CauseAction.java
View file
Expand Up @@ -34,6 +34,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -47,35 +48,55 @@ public class CauseAction implements FoldableAction, RunAction2 {
@Deprecated
// there can be multiple causes, so this is deprecated
private transient Cause cause;

private List<Cause> causes = new ArrayList<Cause>();

/** @deprecated JENKINS-33467 inefficient */
@Deprecated
private transient List<Cause> causes;

private Map<Cause,Integer> causeBag = new LinkedHashMap<>();

public CauseAction(Cause c) {
this.causes.add(c);
this.causeBag.put(c, 1);
}

private void addCause(Cause c) {
synchronized (causeBag) {
Integer cnt = causeBag.get(c);
causeBag.put(c, cnt == null ? 1 : cnt + 1);
}
}
private void addCauses(Collection<? extends Cause> causes) {
for (Cause cause : causes) {
addCause(cause);
}
}

public CauseAction(Cause... c) {
this(Arrays.asList(c));
}

public CauseAction(Collection<? extends Cause> causes) {
this.causes.addAll(causes);
addCauses(causes);
}

public CauseAction(CauseAction ca) {
this.causes.addAll(ca.causes);
addCauses(ca.getCauses());
}

@Exported(visibility=2)
public List<Cause> getCauses() {
return causes;
List<Cause> r = new ArrayList<>();
for (Map.Entry<Cause,Integer> entry : causeBag.entrySet()) {
r.addAll(Collections.nCopies(entry.getValue(), entry.getKey()));
}
return r;
}

/**
* Finds the cause of the specific type.
*/
public <T extends Cause> T findCause(Class<T> type) {
for (Cause c : causes)
for (Cause c : causeBag.keySet())
if (type.isInstance(c))
return type.cast(c);
return null;
Expand All @@ -99,14 +120,7 @@ public String getUrlName() {
* @return Map of Cause to number of occurrences of that Cause
*/
public Map<Cause,Integer> getCauseCounts() {
Map<Cause,Integer> result = new LinkedHashMap<Cause,Integer>();
for (Cause c : causes) {
if (c != null) {
Integer i = result.get(c);
result.put(c, i == null ? 1 : i.intValue() + 1);
}
}
return result;
return Collections.unmodifiableMap(causeBag);
}

/**
Expand All @@ -115,12 +129,14 @@ public Map<Cause,Integer> getCauseCounts() {
*/
@Deprecated
public String getShortDescription() {
if(causes.isEmpty()) return "N/A";
return causes.get(0).getShortDescription();
if (causeBag.isEmpty()) {
return "N/A";
}
return causeBag.keySet().iterator().next().getShortDescription();
}

@Override public void onLoad(Run<?,?> owner) {
for (Cause c : causes) {
for (Cause c : causeBag.keySet()) {
if (c != null) {
c.onLoad(owner);
}
Expand All @@ -131,7 +147,7 @@ public String getShortDescription() {
* When hooked up to build, notify {@link Cause}s.
*/
@Override public void onAttached(Run<?,?> owner) {
for (Cause c : causes) {
for (Cause c : causeBag.keySet()) {
if (c != null) {
c.onAddedTo(owner);
}
Expand All @@ -141,7 +157,7 @@ public String getShortDescription() {
public void foldIntoExisting(hudson.model.Queue.Item item, Task owner, List<Action> otherActions) {
CauseAction existing = item.getAction(CauseAction.class);
if (existing!=null) {
existing.causes.addAll(this.causes);
existing.addCauses(getCauses());
return;
}
// no CauseAction found, so add a copy of this one
Expand All @@ -153,9 +169,19 @@ public static class ConverterImpl extends XStream2.PassthruConverter<CauseAction
@Override protected void callback(CauseAction ca, UnmarshallingContext context) {
// if we are being read in from an older version
if (ca.cause != null) {
if (ca.causes == null) ca.causes = new ArrayList<Cause>();
ca.causes.add(ca.cause);
if (ca.causeBag == null) {
ca.causeBag = new LinkedHashMap<>();
}
ca.addCause(ca.cause);
OldDataMonitor.report(context, "1.288");
ca.cause = null;
} else if (ca.causes != null) {
if (ca.causeBag == null) {
ca.causeBag = new LinkedHashMap<>();
}
ca.addCauses(ca.causes);
OldDataMonitor.report(context, "1.653");
ca.causes = null;
}
}
}
Expand Down
13 changes: 7 additions & 6 deletions test/src/test/java/hudson/model/QueueTest.java
View file
Expand Up @@ -305,6 +305,7 @@ public static final class FileItemPersistenceTestServlet extends HttpServlet {
}
}

@Issue("JENKINS-33467")
@Test public void foldableCauseAction() throws Exception {
final OneShotEvent buildStarted = new OneShotEvent();
final OneShotEvent buildShouldComplete = new OneShotEvent();
Expand Down Expand Up @@ -348,14 +349,13 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
StringBuilder causes = new StringBuilder();
for (Cause c : ca.getCauses()) causes.append(c.getShortDescription() + "\n");
assertEquals("Build causes should have all items, even duplicates",
"Started by user SYSTEM\nStarted by an SCM change\n"
+ "Started by user SYSTEM\nStarted by timer\n"
"Started by user SYSTEM\nStarted by user SYSTEM\n"
+ "Started by an SCM change\nStarted by an SCM change\nStarted by an SCM change\n"
+ "Started by timer\nStarted by timer\n"
+ "Started by remote host 1.2.3.4 with note: test\n"
+ "Started by remote host 4.3.2.1 with note: test\n"
+ "Started by an SCM change\n"
+ "Started by remote host 1.2.3.4 with note: test\n"
+ "Started by remote host 1.2.3.4 with note: foo\n"
+ "Started by an SCM change\nStarted by timer\n",
+ "Started by remote host 4.3.2.1 with note: test\n"
+ "Started by remote host 1.2.3.4 with note: foo\n",
causes.toString());

// View for build should group duplicates
Expand All @@ -369,6 +369,7 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
+ "Started by remote host 1.2.3.4 with note: test (2 times) "
+ "Started by remote host 4.3.2.1 with note: test "
+ "Started by remote host 1.2.3.4 with note: foo"));
System.out.println(new XmlFile(new File(build.getRootDir(), "build.xml")).asString());
}

@Issue("JENKINS-8790")
Expand Down

0 comments on commit ed564e6

Please sign in to comment.