Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to include causes in the JCasC export, and read causes from it on import. #139

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import javax.annotation.Nonnull;
import java.io.File;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.logging.Level;
Expand Down Expand Up @@ -155,6 +156,7 @@ public class PluginImpl extends GlobalConfiguration {
private String fallbackCategoriesAsString;
private transient List<String> fallbackCategories;

private boolean exportCausesToJCasCEnabled;
private transient CopyOnWriteList<FailureCause> causes;

private KnowledgeBase knowledgeBase;
Expand Down Expand Up @@ -629,6 +631,64 @@ public void setSlackFailureCategories(String slackFailureCategories) {
this.slackFailureCategories = slackFailureCategories;
}

/**
* Get if causes from the current knowledge base should be included in the JCasC export is enabled.
* @return exportCausesToJCasCEnabled - on or off. null == off.
*/
public boolean isExportCausesToJCasCEnabled() {
return exportCausesToJCasCEnabled;
}

/**
* Set if this feature is enabled or not. When on, causes will in the JCasC export.
*
* @param exportCausesToJCasCEnabled on or off. null == off.
*/
@DataBoundSetter
public void setExportCausesToJCasCEnabled(boolean exportCausesToJCasCEnabled) {
this.exportCausesToJCasCEnabled = exportCausesToJCasCEnabled;
}

/**
* Save causes to the db. Assumes that the ids are kept from when the causes were fetched.
* Failing to do so may result in duplicate causes.
*
* @param causes value
*/
@DataBoundSetter
public void setCauses(List<FailureCause> causes) {
if (knowledgeBase != null) {
for (FailureCause cause:causes) {
try {
knowledgeBase.saveCause(cause);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am quite certain this will create duplicates for anything else than the file based knowledge base. Unless the id field is set. At least the javadoc should reflect that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing a unit test for that should be relatively easy. There are a couple of integration tests using an embedded mongodb that you can take inspiration from.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a note about duplicates to the javadoc, there isn't much I can do about missing ids short of skipping entries that don't have one, thoughts on the preferred behavior?

} catch (Exception e) {
logger.log(Level.WARNING, "Failed to save cause to the knowledge base", e);
}
}
}
}

/**
* Get the causes, used by JCasC to include it in the configuration export.
*
* @return the causes
*/
public Collection<FailureCause> getCauses() {
// The Causes export can be very large so we only include it if enabled.
if (!isExportCausesToJCasCEnabled()) {
return null;
}
if (knowledgeBase != null) {
try {
return knowledgeBase.getCauses();
} catch (Exception e) {
logger.log(Level.WARNING, "Failed to get causes from the knowledge base", e);
return null;
allenbenz marked this conversation as resolved.
Show resolved Hide resolved
}
}
return null;
}

/**
* The number of threads to have in the pool for each build. Used by the {@link BuildFailureScanner}.
* Will return nothing less than {@link #MINIMUM_NR_OF_SCAN_THREADS}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.Stapler;
import org.kohsuke.stapler.StaplerRequest;
Expand Down Expand Up @@ -86,13 +87,26 @@ public class FailureCause implements Serializable, Action, Describable<FailureCa
* @param id the id.
* @param name the name of this FailureCause.
* @param description the description of this FailureCause.
* @param indications the list of indications
*/
@DataBoundConstructor
public FailureCause(String id, String name, String description, List<Indication> indications) {
this(id, name, description, null, null, (List<String>)null,
indications, null);
}

/**
* Standard constructor.
*
* @param id the id.
* @param name the name of this FailureCause.
* @param description the description of this FailureCause.
* @param comment the comment of this FailureCause.
* @param lastOccurred the time at which this FailureCause last occurred.
* @param categories the categories of this FailureCause.
* @param indications the list of indications
* @param modifications the modification history of this FailureCause.
*/
@DataBoundConstructor
public FailureCause(String id, String name, String description, String comment,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public FailureCause(String id, String name, String description, String comment,
@Deprecated
public FailureCause(String id, String name, String description, String comment,

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this is really Deprecated, the fewer parameter DataBoundConstructor and the added DataBoundSetters allow JCasC to handle optional fields, but this constructor is a lot nicer to use in code.

Though if you still want it Deprecated that's fine too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deprecation in this scenario would only be a flag to anyone coding against it that it is not the one actually used in runtime. It can still be used in tests, just annotated with something if sportbugs or checkstyle complains.

Date lastOccurred, String categories, List<Indication> indications,
List<FailureCauseModification> modifications) {
Expand Down Expand Up @@ -401,6 +415,7 @@ public Date getAndInitiateLastOccurred() {
*
* @param lastOccurred the occurrence to set.
*/
@DataBoundSetter
public void setLastOccurred(Date lastOccurred) {
if (lastOccurred == null) {
this.lastOccurred = null;
Expand Down Expand Up @@ -441,6 +456,26 @@ public List<String> getCategories() {
return categories;
}

/**
* Setter for the comment.
*
* @param comment the comment
*/
@DataBoundSetter
public void setComment(String comment) {
this.comment = comment;
}

/**
* Setter for the FailureCauseModifications done to this FailureCause.
*
* @param modifications the modifications
*/
@DataBoundSetter
public void setModifications(List<FailureCauseModification> modifications) {
this.modifications = modifications;
}

/**
* Returns the categories as a String, used for the view.
*
Expand Down Expand Up @@ -541,6 +576,7 @@ private void loadLastOccurred() {
*
* @param categories the categories.
*/
@DataBoundSetter
public void setCategories(List<String> categories) {
this.categories = categories;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import org.kohsuke.stapler.DataBoundConstructor;

import java.io.Serializable;
import java.util.Date;
Expand Down Expand Up @@ -54,6 +55,22 @@ public FailureCauseModification(@JsonProperty("user") String user, @JsonProperty
}
}

/**
* Constructor for FailureCauseModification.
*
* @param user The user who made the modification.
* @param time The time at which the modification was done.
*/
@DataBoundConstructor
public FailureCauseModification(String user, String time) {
this.user = user;
if (time == null) {
this.time = null;
} else {
this.time = new Date(time);
}
}

/**
* Getter for the time.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import hudson.model.Run;
import hudson.util.FormValidation;
import jenkins.model.Jenkins;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.interceptor.RequirePOST;
Expand All @@ -59,6 +60,7 @@
*/
@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, property = "@class", visible = true)
@JsonIgnoreProperties(ignoreUnknown = true)
@Symbol("buildLog")
public class BuildLogIndication extends Indication {

private static final long serialVersionUID = -2889792693081908532L;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.sonyericsson.jenkins.plugins.bfa.model.MultilineBuildLogFailureReader;
import hudson.Extension;
import hudson.model.Hudson;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.DataBoundConstructor;

import java.util.regex.Pattern;
Expand All @@ -42,6 +43,7 @@
*/
@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, property = "@class", visible = true)
@JsonIgnoreProperties(ignoreUnknown = true)
@Symbol("multilineBuildLog")
public class MultilineBuildLogIndication extends BuildLogIndication {

private static final long serialVersionUID = 8436383594898812087L;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,8 @@
<f:entry title="${%Max size of log file}" description="${%maxLogSize}">
<f:textbox field="maxLogSize" />
</f:entry>
<f:entry title="${%Include causes to the JCasC export}" description="${%exportCausesToJCasCEnabled}">
<f:checkbox field="exportCausesToJCasCEnabled" />
</f:entry>
</f:advanced>
</j:jelly>
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
enableGerritTriggerDescription=This option allows BFA to forward the description of the found causes to the Gerrit-Trigger-plugin, ultimately allowing users to see their build issues directly inside Gerrit.
exportCausesToJCasCEnabled=If the knowledge base is large this will make the JCasC export large.
nrOfScanThreadsDescription=Number of threads per build to use when scanning the failed builds.
testResultParsingEnabledDescription=Treat failed test cases (as indicated by JUnit/xUnit/... publishers) as failure causes.
testResultCategoriesDescription=A space-separated list of categories to use for failure causes representing failed test cases.
Expand Down
Loading