Browse files

JENKINS-12881: Refactor the message retrieval

Refactor at which point the message is retrieved, and
which class is responsible for doing so. Should be more in
line with the original intent of the classes, and not
introduce a high degree of coupling.
  • Loading branch information...
1 parent f79e10b commit 7e4c31972ba20d5b556a8d7f470f6e050998c1b4 @jhansche committed Feb 24, 2012
View
55 ...va/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ToGerritRunListener.java
@@ -30,7 +30,10 @@
import com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.model.BuildsStartedStats;
import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.GerritCause;
import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.GerritTrigger;
+
+import hudson.EnvVars;
import hudson.Extension;
+import hudson.FilePath;
import hudson.model.AbstractBuild;
import hudson.model.AbstractProject;
import hudson.model.Cause;
@@ -41,6 +44,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.io.IOException;
import java.util.List;
/**
@@ -93,7 +97,21 @@ public synchronized void onCompleted(AbstractBuild r, TaskListener listener) {
}
event.fireBuildCompleted(r);
if (!cause.isSilentMode()) {
- PatchSetKey key = memory.completed(event, r, listener);
+ PatchSetKey key = memory.completed(event, r);
+
+ if (r.getResult() != Result.SUCCESS)
+ {
+ try {
+ // Attempt to record the failure message, if applicable
+ String failureMessage = this.obtainFailureMessage(event, r, listener);
+ memory.setEntryFailureMessage(key, r, failureMessage);
+ } catch (IOException e) {
+ e.printStackTrace(listener.getLogger());
+ } catch (InterruptedException e) {
+ e.printStackTrace(listener.getLogger());
+ }
+ }
+
updateTriggerContexts(r, key);
if (memory.isAllBuildsCompleted(key)) {
try {
@@ -111,6 +129,41 @@ public synchronized void onCompleted(AbstractBuild r, TaskListener listener) {
}
}
+ /**
+ * Attempt to obtain the failure message for a build.
+ *
+ * @param event
+ * @param build
+ * @param listener
+ * @return
+ * @throws IOException
+ * @throws InterruptedException
+ */
+ private String obtainFailureMessage(PatchsetCreated event, AbstractBuild build, TaskListener listener)
+ throws IOException, InterruptedException {
+ AbstractProject project = build.getProject();
+ String content = null;
+
+ // This is an unsuccessful build
+ GerritTrigger trigger = GerritTrigger.getTrigger(project);
+
+ String filename = trigger.getBuildUnsuccessfulFilepath();
+
+ if (filename != null && !filename.isEmpty()) {
+ FilePath path = build.getWorkspace().child(filename);
+
+ if (path.exists()) {
+ content = path.readToString();
+ }
+
+ EnvVars envVars = (listener == null) ? build.getEnvironment()
+ : build.getEnvironment(listener);
+ content = envVars.expand(content);
+ }
+
+ return content;
+ }
+
@Override
public synchronized void onStarted(AbstractBuild r, TaskListener listener) {
GerritCause cause = getCause(r);
View
91 ...java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/model/BuildMemory.java
@@ -25,6 +25,7 @@
import com.sonyericsson.hudson.plugins.gerrit.gerritevents.dto.events.PatchsetCreated;
import com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.model.BuildMemory.MemoryImprint.Entry;
+import com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.model.BuildMemory.PatchSetKey;
import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.GerritCause;
import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.GerritTrigger;
import com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.data.TriggerContext;
@@ -156,28 +157,14 @@ public synchronized boolean isAllBuildsStarted(PatchSetKey key) {
}
}
-
/**
* Sets the memory that a build is completed for an event.
*
* @param event the event
* @param build the build.
* @return the key to the memory.
- * @deprecated Use {@link #completed(PatchsetCreated, AbstractBuild, TaskListener)}
*/
- @Deprecated
public synchronized PatchSetKey completed(PatchsetCreated event, AbstractBuild build) {
- return this.completed(event, build, null);
- }
- /**
- * Sets the memory that a build is completed for an event.
- *
- * @param event the event
- * @param build the build.
- * @param listener the listener.
- * @return the key to the memory.
- */
- public synchronized PatchSetKey completed(PatchsetCreated event, AbstractBuild build, TaskListener listener) {
PatchSetKey key = createKey(event);
MemoryImprint pb = memory.get(key);
if (pb == null) {
@@ -186,7 +173,6 @@ public synchronized PatchSetKey completed(PatchsetCreated event, AbstractBuild b
memory.put(key, pb);
}
pb.set(build.getProject(), build, true);
- pb.updateFailureReason(build.getProject(), build, listener);
return key;
}
@@ -509,60 +495,6 @@ private synchronized void set(AbstractProject project, AbstractBuild build, bool
}
}
- public void updateFailureReason(AbstractProject project, AbstractBuild build,
- TaskListener listener) {
-
- if (build.getResult() != Result.SUCCESS) {
- // This is an unsuccessful build
- GerritTrigger trigger = GerritTrigger.getTrigger(project);
-
- Entry entry = getEntry(project);
- // This was already checked in set()
- assert entry != null;
-
- try {
- entry.setUnsuccessfulMessage(this.getUnsuccessfulMessage(trigger, build,
- listener));
- } catch (IOException e) {
- e.printStackTrace();
- } catch (InterruptedException e) {
- e.printStackTrace();
- }
- }
- }
-
- /**
- * Returns the message read from the configured file.
- *
- * @param trigger
- * @param listener
- * @param listener
- * @return The Environment-expanded string, or null
- * @throws IOException
- * @throws InterruptedException
- */
- public String getUnsuccessfulMessage(GerritTrigger trigger, AbstractBuild build,
- TaskListener listener) throws IOException, InterruptedException {
- String filename = trigger.getBuildUnsuccessfulFilepath();
- String content = null;
-
- if (filename == null || filename.isEmpty()) {
- return null;
- }
-
- FilePath path = build.getWorkspace().child(filename);
-
- if (path.exists()) {
- content = path.readToString();
- }
-
- EnvVars envVars = (listener == null) ? build.getEnvironment() : build
- .getEnvironment(listener);
- content = envVars.expand(content);
-
- return content;
- }
-
/**
* Tells if all builds have a value (not null).
*
@@ -764,7 +696,7 @@ private void setBuild(AbstractBuild build) {
*
* @param unsuccessfulMessage
*/
- public void setUnsuccessfulMessage(String unsuccessfulMessage) {
+ private void setUnsuccessfulMessage(String unsuccessfulMessage) {
this.unsuccessfulMessage = unsuccessfulMessage;
}
@@ -917,4 +849,23 @@ public int compareTo(PatchSetKey o) {
}
}
}
+
+ /**
+ * Records the failure message for the given build.
+ *
+ * @param key
+ * @param r
+ * @param failureMessage
+ */
+ public void setEntryFailureMessage(PatchSetKey key, AbstractBuild r, String failureMessage) {
+ MemoryImprint pb = getMemoryImprint(key);
+
+ if (pb != null) {
+ Entry entry = pb.getEntry(r.getProject());
+
+ if (entry != null) {
+ entry.setUnsuccessfulMessage(failureMessage);
+ }
+ }
+ }
}

0 comments on commit 7e4c319

Please sign in to comment.