From e4a7925cc635150acdb6eee145b13e0af5225ae7 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Mon, 14 Aug 2017 15:12:16 +1000 Subject: [PATCH 01/22] Add a cache for expensive BlueTestSummary calculation --- .../embedded/rest/AbstractRunImpl.java | 7 +++- .../service/embedded/rest/Caches.java | 37 +++++++++++++++++++ .../blueocean/rest/model/BlueTestSummary.java | 4 ++ 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/Caches.java diff --git a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java index 4cfcda6901e..b8deee037e6 100644 --- a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java +++ b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java @@ -29,6 +29,7 @@ import javax.annotation.Nullable; import java.util.Collection; import java.util.Date; +import java.util.concurrent.ExecutionException; /** * Basic {@link BlueRun} implementation. @@ -186,7 +187,11 @@ public BlueTestResultContainer getTests() { @Override public BlueTestSummary getTestSummary() { - return BlueTestResultFactory.resolve(run, this).summary; + if (getStateObj() == BlueRunState.FINISHED) { + return Caches.loadTestSummary(run, this).orNull(); + } else { + return BlueTestResultFactory.resolve(run, this).summary; + } } public Collection getActions() { diff --git a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/Caches.java b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/Caches.java new file mode 100644 index 00000000000..c85822e7fdd --- /dev/null +++ b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/Caches.java @@ -0,0 +1,37 @@ +package io.jenkins.blueocean.service.embedded.rest; + +import com.google.common.base.Optional; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import hudson.model.Run; +import io.jenkins.blueocean.rest.Reachable; +import io.jenkins.blueocean.rest.factory.BlueTestResultFactory; +import io.jenkins.blueocean.rest.model.BlueTestSummary; + +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; + +final class Caches { + + private static final long TEST_SUMMARY_CACHE_MAX_SIZE = Long.getLong("TEST_SUMMARY_CACHE_MAX_SIZE", 10000); + + private static final Cache> TEST_SUMMARY = CacheBuilder.newBuilder() + .maximumSize(TEST_SUMMARY_CACHE_MAX_SIZE) + .expireAfterAccess(1, TimeUnit.DAYS) + .build(); + + static Optional loadTestSummary(final Run run, final Reachable parent) { + try { + return TEST_SUMMARY.get(run.getExternalizableId(), new Callable>() { + @Override + public Optional call() throws Exception { + BlueTestSummary summary = BlueTestResultFactory.resolve(run, parent).summary; + return summary == null ? Optional.absent() : Optional.of(summary); + } + }); + } catch (ExecutionException e) { + return Optional.absent(); + } + } +} diff --git a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueTestSummary.java b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueTestSummary.java index 36311f6d659..9dc72f9ac89 100644 --- a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueTestSummary.java +++ b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueTestSummary.java @@ -78,4 +78,8 @@ public BlueTestSummary tally(BlueTestSummary summary) { this.total + summary.total ); } + + public static BlueTestSummary empty() { + return new BlueTestSummary(0, 0, 0, 0, 0, 0, 0); + } } From 3d0125d94816ae9bd69ee21fc63c8a05b1fb990c Mon Sep 17 00:00:00 2001 From: James Dumay Date: Mon, 14 Aug 2017 15:32:28 +1000 Subject: [PATCH 02/22] NotExportableException exception thrown in Model::ctor adding many milliseconds to export --- .../commons/stapler/export/ModelBuilder.java | 18 +- .../commons/stapler/export/Property.java | 210 +++++++++--------- 2 files changed, 125 insertions(+), 103 deletions(-) diff --git a/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java b/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java index 9d5de5c53eb..f38b17b22d6 100644 --- a/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java +++ b/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java @@ -44,12 +44,26 @@ public class ModelBuilder { /*package*/ final Map models = new ConcurrentHashMap(); public Model get(Class type) throws NotExportableException { + if (type.getAnnotation(ExportedBean.class) == null) { + throw new NotExportableException(type); + } + return get(type, null, null); + } + + public Model getOrNull(Class type) throws NotExportableException { return get(type, null, null); } - public Model get(Class type, @CheckForNull Class propertyOwner, @Nullable String property) throws NotExportableException { + public Model get(Class type, @CheckForNull Class propertyOwner, @Nullable String property) { + if (type.getAnnotation(ExportedBean.class) == null) { + throw new NotExportableException(type); + } + return getOrNull(type, propertyOwner, property); + } + + public Model getOrNull(Class type, @CheckForNull Class propertyOwner, @Nullable String property) throws NotExportableException { Model m = models.get(type); - if(m==null) { + if(m==null && type.getAnnotation(ExportedBean.class) != null) { m = new Model(this, type, propertyOwner, property); } return m; diff --git a/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/Property.java b/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/Property.java index ec54b0e8a4e..68be75dd46a 100644 --- a/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/Property.java +++ b/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/Property.java @@ -141,15 +141,14 @@ public void writeTo(Object object, TreePruner pruner, DataWriter writer) throws if (merge) { // merged property will get all its properties written here if (d != null) { - Model model; - try { - model = owner.get(d.getClass(), parent.type, name); - } catch (NotExportableException e) { + if (d.getClass().getAnnotation(ExportedBean.class) == null) { if(writer.getExportConfig().isSkipIfFail()){ return; + } else { + throw new NotExportableException(d.getClass()); } - throw e; } + Model model = owner.getOrNull(d.getClass(), parent.type, name); model.writeNestedObjectTo(d, new FilteringTreePruner(parent.HAS_PROPERTY_NAME_IN_ANCESTORY,child), writer); } } else { @@ -201,118 +200,127 @@ private void writeValue(Type expected, Object value, TreePruner pruner, DataWrit Class c = value.getClass(); - Model model; - try { - model = owner.get(c, parent.type, name); - } catch (NotExportableException ex) { - if(STRING_TYPES.contains(c)) { - writer.value(value.toString()); - return; - } - if(PRIMITIVE_TYPES.contains(c)) { - writer.valuePrimitive(value); - return; - } - Class act = c.getComponentType(); - if (act !=null) { // array - Range r = pruner.getRange(); - writer.startArray(); - if (value instanceof Object[]) { - // typical case - for (Object item : r.apply((Object[]) value)) { - writeBuffered(act, item, pruner, writer); - } - } else { - // more generic case - int len = Math.min(r.max, Array.getLength(value)); - for (int i=r.min; i) value).entrySet()) { - BufferedDataWriter buffer = new BufferedDataWriter(writer.getExportConfig()); - try { - writeStartObjectNullType(buffer); - buffer.name(verboseMap[0]); - writeValue(null, e.getKey(), pruner, buffer); - buffer.name(verboseMap[1]); - writeValue(null, e.getValue(), pruner, buffer); - buffer.endObject(); - buffer.finished(); - } catch (IOException x) { - if (x.getCause() instanceof InvocationTargetException) { - LOGGER.log(Level.WARNING, "skipping export of " + e, x); - } + writer.endArray(); + return; + } + if(value instanceof Iterable) { + writer.startArray(); + Type expectedItemType = Types.getTypeArgument(expected, 0, null); + for (Object item : pruner.getRange().apply((Iterable) value)) { + writeBuffered(expectedItemType, item, pruner, writer); + } + writer.endArray(); + return; + } + if(value instanceof Map) { + if (verboseMap!=null) {// verbose form + writer.startArray(); + for (Map.Entry e : ((Map) value).entrySet()) { + BufferedDataWriter buffer = new BufferedDataWriter(writer.getExportConfig()); + try { + writeStartObjectNullType(buffer); + buffer.name(verboseMap[0]); + writeValue(null, e.getKey(), pruner, buffer); + buffer.name(verboseMap[1]); + writeValue(null, e.getValue(), pruner, buffer); + buffer.endObject(); + buffer.finished(); + } catch (IOException x) { + if (x.getCause() instanceof InvocationTargetException) { + LOGGER.log(Level.WARNING, "skipping export of " + e, x); } - buffer.commit(writer); } - writer.endArray(); - } else {// compact form - writeStartObjectNullType(writer); - for (Map.Entry e : ((Map) value).entrySet()) { - BufferedDataWriter buffer = new BufferedDataWriter(writer.getExportConfig()); - try { - buffer.name(e.getKey().toString()); - writeValue(null, e.getValue(), pruner, buffer); - buffer.finished(); - } catch (IOException x) { - if (x.getCause() instanceof InvocationTargetException) { - LOGGER.log(Level.WARNING, "skipping export of " + e, x); - } + buffer.commit(writer); + } + writer.endArray(); + } else {// compact form + writeStartObjectNullType(writer); + for (Map.Entry e : ((Map) value).entrySet()) { + BufferedDataWriter buffer = new BufferedDataWriter(writer.getExportConfig()); + try { + buffer.name(e.getKey().toString()); + writeValue(null, e.getValue(), pruner, buffer); + buffer.finished(); + } catch (IOException x) { + if (x.getCause() instanceof InvocationTargetException) { + LOGGER.log(Level.WARNING, "skipping export of " + e, x); } - buffer.commit(writer); } - writer.endObject(); + buffer.commit(writer); } - return; - } - if(value instanceof Date) { - writer.valuePrimitive(((Date) value).getTime()); - return; - } - if(value instanceof Calendar) { - writer.valuePrimitive(((Calendar) value).getTimeInMillis()); - return; - } - if(value instanceof Enum) { - writer.value(value.toString()); - return; - } - - if (skipIfFail) { - writer.startObject(); writer.endObject(); - return; } - - throw ex; + return; } - - try { - writer.type(expected, value.getClass()); - } catch (AbstractMethodError _) { - // legacy impl that doesn't understand it + if(value instanceof Date) { + writer.valuePrimitive(((Date) value).getTime()); + return; + } + if(value instanceof Calendar) { + writer.valuePrimitive(((Calendar) value).getTimeInMillis()); + return; + } + if(value instanceof Enum) { + writer.value(value.toString()); + return; } + if (skipIfFail) { + writer.startObject(); + writer.endObject(); + return; + } - writer.startObject(); - model.writeNestedObjectTo(value, pruner, writer); - writer.endObject(); + throw new NotExportableException(c); } private static class BufferedDataWriter implements DataWriter { From 38032244d3b0e101665703f8d3458b5c5df31c68 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Mon, 14 Aug 2017 15:36:38 +1000 Subject: [PATCH 03/22] Branch metadata did not hit cache --- .../io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java index 85f0fe71e91..94880898d96 100644 --- a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java +++ b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java @@ -61,10 +61,7 @@ public PullRequest getPullRequest() { @Exported(name = Branch.BRANCH, inline = true) public Branch getBranch() { - ObjectMetadataAction om = job.getAction(ObjectMetadataAction.class); - PrimaryInstanceMetadataAction pima = job.getAction(PrimaryInstanceMetadataAction.class); - String url = om != null && om.getObjectUrl() != null ? om.getObjectUrl() : null; - return new Branch(url, pima != null); + return Branch.getBranch(job); } @Override From 2ab30e576758cbbb4723094b84b5aa607c561eb5 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Mon, 14 Aug 2017 15:45:38 +1000 Subject: [PATCH 04/22] Move branch issues onto the Branch model where it belongs It will be cached and the action lookup here wont happen --- .../rest/impl/pipeline/BranchImpl.java | 19 +++++++++---------- .../blueocean/rest/impl/pipeline/Caches.java | 4 ++-- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java index 94880898d96..6322fbb050f 100644 --- a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java +++ b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java @@ -19,8 +19,6 @@ import io.jenkins.blueocean.rest.model.BluePipelineScm; import io.jenkins.blueocean.rest.model.Resource; import jenkins.branch.MultiBranchProject; -import jenkins.scm.api.metadata.ObjectMetadataAction; -import jenkins.scm.api.metadata.PrimaryInstanceMetadataAction; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.kohsuke.stapler.export.Exported; import org.kohsuke.stapler.export.ExportedBean; @@ -38,8 +36,6 @@ @Capability({BLUE_BRANCH, JENKINS_WORKFLOW_JOB, PULL_REQUEST}) public class BranchImpl extends PipelineImpl { - public static final String ISSUES = "issues"; - private final Link parent; protected final Job job; @@ -49,11 +45,6 @@ public BranchImpl(BlueOrganization org, Job job, Link parent) { this.parent = parent; } - @Exported(name = ISSUES, skipNull = true, inline = true) - public Collection getIssues() { - return BlueIssueFactory.resolve(job); - } - @Exported(name = PullRequest.PULL_REQUEST, inline = true, skipNull = true) public PullRequest getPullRequest() { return PullRequest.get(job); @@ -105,13 +96,16 @@ public static class Branch { public static final String BRANCH = "branch"; public static final String BRANCH_URL = "url"; public static final String BRANCH_PRIMARY = "isPrimary"; + public static final String ISSUES = "issues"; private final String url; private final boolean primary; + private final Collection issues; - public Branch(String url, boolean primary) { + public Branch(String url, boolean primary, Collection issues) { this.url = url; this.primary = primary; + this.issues = issues; } @Exported(name = BRANCH_URL) @@ -124,6 +118,11 @@ public boolean isPrimary() { return primary; } + @Exported(name = ISSUES, skipNull = true, inline = true) + public Collection getIssues() { + return issues; + } + public static Branch getBranch(final Job job) { try { return Caches.BRANCH_METADATA.get(job.getFullName()).orNull(); diff --git a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/Caches.java b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/Caches.java index f44d25b87ec..49b07accdd2 100644 --- a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/Caches.java +++ b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/Caches.java @@ -8,6 +8,7 @@ import hudson.model.Item; import hudson.model.Job; import hudson.model.listeners.ItemListener; +import io.jenkins.blueocean.rest.factory.BlueIssueFactory; import io.jenkins.blueocean.rest.impl.pipeline.BranchImpl.Branch; import io.jenkins.blueocean.rest.impl.pipeline.BranchImpl.PullRequest; import jenkins.model.Jenkins; @@ -17,7 +18,6 @@ import jenkins.scm.api.metadata.PrimaryInstanceMetadataAction; import jenkins.scm.api.mixin.ChangeRequestSCMHead; -import javax.annotation.Nullable; import java.util.concurrent.TimeUnit; class Caches { @@ -96,7 +96,7 @@ public Optional load(String key) throws Exception { return Optional.absent(); } String url = om != null && om.getObjectUrl() != null ? om.getObjectUrl() : null; - return Optional.of(new Branch(url, pima != null)); + return Optional.of(new Branch(url, pima != null, BlueIssueFactory.resolve(job))); } } From f17af17e448565dead6312d05e0177d0084da355 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Mon, 14 Aug 2017 15:48:14 +1000 Subject: [PATCH 05/22] Remove up to 5ms per call off of constructing date SimpleDateFormat objs --- .../io/jenkins/blueocean/rest/model/BluePipelineStep.java | 2 +- .../main/java/io/jenkins/blueocean/rest/model/BlueRun.java | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BluePipelineStep.java b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BluePipelineStep.java index 239d1b685ee..c93860edd81 100644 --- a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BluePipelineStep.java +++ b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BluePipelineStep.java @@ -53,7 +53,7 @@ public final String getStartTimeString(){ if(getStartTime() == null) { return null; } - return new SimpleDateFormat(BlueRun.DATE_FORMAT_STRING).format(getStartTime()); + return BlueRun.DATE_FORMAT.format(getStartTime()); } @Exported(name= DURATION_IN_MILLIS) diff --git a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java index f28043aa6c6..bb88a99138b 100644 --- a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java +++ b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java @@ -53,6 +53,7 @@ public abstract class BlueRun extends Resource { /** Date String format */ public static final String DATE_FORMAT_STRING = "yyyy-MM-dd'T'HH:mm:ss.SSSZ"; + public static final SimpleDateFormat DATE_FORMAT = new SimpleDateFormat(DATE_FORMAT_STRING); /** @@ -96,7 +97,7 @@ public abstract class BlueRun extends Resource { */ @Exported(name=START_TIME) public final String getStartTimeString(){ - return new SimpleDateFormat(DATE_FORMAT_STRING).format(getStartTime()); + return DATE_FORMAT.format(getStartTime()); } /** @@ -106,7 +107,7 @@ public final String getStartTimeString(){ @Exported(name=ENQUEUE_TIME) public final String getEnQueueTimeString() { - return new SimpleDateFormat(DATE_FORMAT_STRING).format(getEnQueueTime()); + return DATE_FORMAT.format(getEnQueueTime()); } /** @@ -120,7 +121,7 @@ public final String getEndTimeString(){ if(endTime == null) { return null; } else { - return new SimpleDateFormat(DATE_FORMAT_STRING).format(endTime); + return DATE_FORMAT.format(endTime); } } From cccef359a50447f4ea4ad21e33681ae0437407a7 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Mon, 14 Aug 2017 16:10:30 +1000 Subject: [PATCH 06/22] Use jodatime for a threadsafe datetimeformatter --- blueocean-rest/pom.xml | 4 ++++ .../blueocean/rest/model/BluePipelineStep.java | 2 +- .../io/jenkins/blueocean/rest/model/BlueRun.java | 12 +++++++----- pom.xml | 6 ++++++ 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/blueocean-rest/pom.xml b/blueocean-rest/pom.xml index 4b848c878b6..09d3a622dac 100644 --- a/blueocean-rest/pom.xml +++ b/blueocean-rest/pom.xml @@ -23,6 +23,10 @@ io.jenkins.blueocean.rest.annotation capability-annotation + + joda-time + joda-time + diff --git a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BluePipelineStep.java b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BluePipelineStep.java index c93860edd81..4062ed8b6bb 100644 --- a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BluePipelineStep.java +++ b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BluePipelineStep.java @@ -53,7 +53,7 @@ public final String getStartTimeString(){ if(getStartTime() == null) { return null; } - return BlueRun.DATE_FORMAT.format(getStartTime()); + return BlueRun.DATE_FORMAT.print(getStartTime().getTime()); } @Exported(name= DURATION_IN_MILLIS) diff --git a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java index bb88a99138b..1cfa2f89522 100644 --- a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java +++ b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java @@ -3,6 +3,9 @@ import io.jenkins.blueocean.commons.stapler.TreeResponse; import io.jenkins.blueocean.rest.Navigable; import io.jenkins.blueocean.rest.annotation.Capability; +import org.joda.time.format.DateTimeFormat; +import org.joda.time.format.DateTimeFormatter; +import org.joda.time.format.DateTimeFormatterBuilder; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.WebMethod; import org.kohsuke.stapler.export.Exported; @@ -53,8 +56,7 @@ public abstract class BlueRun extends Resource { /** Date String format */ public static final String DATE_FORMAT_STRING = "yyyy-MM-dd'T'HH:mm:ss.SSSZ"; - public static final SimpleDateFormat DATE_FORMAT = new SimpleDateFormat(DATE_FORMAT_STRING); - + public static final DateTimeFormatter DATE_FORMAT = DateTimeFormat.forPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ"); /** * @return name of the organization @@ -97,7 +99,7 @@ public abstract class BlueRun extends Resource { */ @Exported(name=START_TIME) public final String getStartTimeString(){ - return DATE_FORMAT.format(getStartTime()); + return DATE_FORMAT.print(getStartTime().getTime()); } /** @@ -107,7 +109,7 @@ public final String getStartTimeString(){ @Exported(name=ENQUEUE_TIME) public final String getEnQueueTimeString() { - return DATE_FORMAT.format(getEnQueueTime()); + return DATE_FORMAT.print(getEnQueueTime().getTime()); } /** @@ -121,7 +123,7 @@ public final String getEndTimeString(){ if(endTime == null) { return null; } else { - return DATE_FORMAT.format(endTime); + return DATE_FORMAT.print(endTime.getTime()); } } diff --git a/pom.xml b/pom.xml index 4f1a41780bf..eb1e5af88b3 100644 --- a/pom.xml +++ b/pom.xml @@ -592,6 +592,12 @@ 1.3.0 + + joda-time + joda-time + 2.9.9 + + org.jenkins-ci.plugins From bef47fbd729a286fab324aa141dfcfe4ca361d72 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Mon, 14 Aug 2017 16:13:56 +1000 Subject: [PATCH 07/22] Found another 7ms to DateTimeFormatter construction --- .../blueocean/service/embedded/rest/ChangeSetResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/ChangeSetResource.java b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/ChangeSetResource.java index 8da765d6d32..1cfbe82a197 100644 --- a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/ChangeSetResource.java +++ b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/ChangeSetResource.java @@ -52,7 +52,7 @@ public BlueUser getAuthor() { @Override public String getTimestamp(){ if(changeSet.getTimestamp() > 0) { - return new SimpleDateFormat(BlueRun.DATE_FORMAT_STRING).format(changeSet.getTimestamp()); + return BlueRun.DATE_FORMAT.print(changeSet.getTimestamp()); }else{ return null; } From f6191020c3b6678fdc37ae616a35d727ac25334b Mon Sep 17 00:00:00 2001 From: James Dumay Date: Mon, 14 Aug 2017 16:17:26 +1000 Subject: [PATCH 08/22] Here's a few more SimpleDateFormat usages in hot paths --- blueocean-jwt/pom.xml | 4 ++++ .../blueocean/auth/jwt/impl/SigningKeyProviderImpl.java | 5 ++++- .../java/io/jenkins/blueocean/rest/model/BlueQueueItem.java | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/blueocean-jwt/pom.xml b/blueocean-jwt/pom.xml index 108a1b47af0..3291001ff1b 100644 --- a/blueocean-jwt/pom.xml +++ b/blueocean-jwt/pom.xml @@ -29,5 +29,9 @@ mailer + + joda-time + joda-time + diff --git a/blueocean-jwt/src/main/java/io/jenkins/blueocean/auth/jwt/impl/SigningKeyProviderImpl.java b/blueocean-jwt/src/main/java/io/jenkins/blueocean/auth/jwt/impl/SigningKeyProviderImpl.java index c2169eb0de7..1c31d531f3a 100644 --- a/blueocean-jwt/src/main/java/io/jenkins/blueocean/auth/jwt/impl/SigningKeyProviderImpl.java +++ b/blueocean-jwt/src/main/java/io/jenkins/blueocean/auth/jwt/impl/SigningKeyProviderImpl.java @@ -6,6 +6,8 @@ import io.jenkins.blueocean.auth.jwt.SigningKey; import io.jenkins.blueocean.auth.jwt.SigningPublicKey; import io.jenkins.blueocean.commons.ServiceException; +import org.joda.time.format.DateTimeFormat; +import org.joda.time.format.DateTimeFormatter; import java.io.IOException; import java.security.interfaces.RSAPublicKey; @@ -27,12 +29,13 @@ public class SigningKeyProviderImpl extends JwtSigningKeyProvider { private static final Logger LOGGER = Logger.getLogger(SigningKeyProviderImpl.class.getName()); private static final Pattern YYYYMM = Pattern.compile("[0-9]{6}"); + private static final DateTimeFormatter DATE_FORMAT = DateTimeFormat.forPattern("yyyyMM"); private final AtomicReference key = new AtomicReference<>(); @Override public SigningKey select(JwtToken token) { - String id = new SimpleDateFormat("yyyyMM").format(new Date()); + String id = DATE_FORMAT.print(new Date().getTime()); JwtRsaDigitalSignatureKey k = key.get(); if (k==null || !k.getId().equals(id)) key.set(k=new JwtRsaDigitalSignatureKey(id)); diff --git a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueQueueItem.java b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueQueueItem.java index bec071c6ca8..54a7f01fbd8 100644 --- a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueQueueItem.java +++ b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueQueueItem.java @@ -54,7 +54,7 @@ public abstract class BlueQueueItem extends Resource { @Exported(name=QUEUED_TIME) public final String getQueuedTimeString(){ - return new SimpleDateFormat(DATE_FORMAT_STRING).format(getQueuedTime()); + return BlueRun.DATE_FORMAT.print(getQueuedTime().getTime()); } /** * From 38a1b293a08ff3e7ae4662ebd5650da64eeda00a Mon Sep 17 00:00:00 2001 From: James Dumay Date: Mon, 14 Aug 2017 16:39:06 +1000 Subject: [PATCH 09/22] Remove empty() --- .../java/io/jenkins/blueocean/rest/model/BlueTestSummary.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueTestSummary.java b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueTestSummary.java index 9dc72f9ac89..36311f6d659 100644 --- a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueTestSummary.java +++ b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueTestSummary.java @@ -78,8 +78,4 @@ public BlueTestSummary tally(BlueTestSummary summary) { this.total + summary.total ); } - - public static BlueTestSummary empty() { - return new BlueTestSummary(0, 0, 0, 0, 0, 0, 0); - } } From 51708cde20f9162453d2807e25f749722495475d Mon Sep 17 00:00:00 2001 From: James Dumay Date: Wed, 16 Aug 2017 10:50:12 +1000 Subject: [PATCH 10/22] Move jodatime usage to impl --- .../pipeline/OrganizationFolderRunImpl.java | 16 +++++++++++- .../rest/impl/pipeline/PipelineNodeImpl.java | 8 ++++++ .../rest/impl/pipeline/PipelineStepImpl.java | 9 +++++++ blueocean-rest-impl/pom.xml | 5 ++++ .../embedded/rest/AbstractRunImpl.java | 26 +++++++++++++++++++ .../embedded/rest/ChangeSetResource.java | 2 +- .../service/embedded/rest/QueueItemImpl.java | 6 +++++ .../service/embedded/rest/QueuedBlueRun.java | 15 +++++++++++ blueocean-rest/pom.xml | 4 --- .../rest/model/BluePipelineStep.java | 7 +---- .../blueocean/rest/model/BlueQueueItem.java | 5 ++-- .../jenkins/blueocean/rest/model/BlueRun.java | 24 +++-------------- 12 files changed, 92 insertions(+), 35 deletions(-) diff --git a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/OrganizationFolderRunImpl.java b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/OrganizationFolderRunImpl.java index 4a98064dbe1..eea5d53b774 100644 --- a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/OrganizationFolderRunImpl.java +++ b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/OrganizationFolderRunImpl.java @@ -8,7 +8,6 @@ import io.jenkins.blueocean.rest.model.BlueActionProxy; import io.jenkins.blueocean.rest.model.BlueArtifactContainer; import io.jenkins.blueocean.rest.model.BlueChangeSetEntry; -import io.jenkins.blueocean.rest.model.BlueOrganization; import io.jenkins.blueocean.rest.model.BluePipelineNodeContainer; import io.jenkins.blueocean.rest.model.BluePipelineStepContainer; import io.jenkins.blueocean.rest.model.BlueRun; @@ -96,6 +95,21 @@ public Date getEndTime() { return null; } + @Override + public String getStartTimeString() { + return null; + } + + @Override + public String getEnQueueTimeString() { + return null; + } + + @Override + public String getEndTimeString() { + return null; + } + @Override public Long getDurationInMillis() { if(folderComputation.isBuilding()){ diff --git a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineNodeImpl.java b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineNodeImpl.java index 95f3ae6287d..5110549940f 100644 --- a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineNodeImpl.java +++ b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineNodeImpl.java @@ -9,6 +9,7 @@ import io.jenkins.blueocean.rest.model.BluePipelineStep; import io.jenkins.blueocean.rest.model.BluePipelineStepContainer; import io.jenkins.blueocean.rest.model.BlueRun; +import io.jenkins.blueocean.service.embedded.rest.AbstractRunImpl; import io.jenkins.blueocean.service.embedded.rest.ActionProxiesImpl; import org.jenkinsci.plugins.workflow.actions.LogAction; import org.jenkinsci.plugins.workflow.graph.FlowNode; @@ -79,6 +80,13 @@ public Date getStartTime() { return new Date(nodeTime); } + public String getStartTimeString(){ + if(getStartTime() == null) { + return null; + } + return AbstractRunImpl.DATE_FORMAT.print(getStartTime().getTime()); + } + @Override public List getEdges() { return edges; diff --git a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineStepImpl.java b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineStepImpl.java index c703c294619..5f5bd501111 100644 --- a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineStepImpl.java +++ b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineStepImpl.java @@ -15,6 +15,7 @@ import io.jenkins.blueocean.rest.model.BlueInputStep; import io.jenkins.blueocean.rest.model.BluePipelineStep; import io.jenkins.blueocean.rest.model.BlueRun; +import io.jenkins.blueocean.service.embedded.rest.AbstractRunImpl; import io.jenkins.blueocean.service.embedded.rest.ActionProxiesImpl; import io.jenkins.blueocean.service.embedded.rest.LogAppender; import io.jenkins.blueocean.service.embedded.rest.LogResource; @@ -103,6 +104,14 @@ public Long getDurationInMillis() { return node.getTiming().getTotalDurationMillis(); } + @Override + public String getStartTimeString(){ + if(getStartTime() == null) { + return null; + } + return AbstractRunImpl.DATE_FORMAT.print(getStartTime().getTime()); + } + @Override public Object getLog() { LogAction logAction = node.getNode().getAction(LogAction.class); diff --git a/blueocean-rest-impl/pom.xml b/blueocean-rest-impl/pom.xml index 2f1a88c2424..76bad63ec95 100644 --- a/blueocean-rest-impl/pom.xml +++ b/blueocean-rest-impl/pom.xml @@ -121,6 +121,11 @@ ua-parser + + joda-time + joda-time + + org.jenkins-ci.main maven-plugin diff --git a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java index b8deee037e6..ee86952d5bc 100644 --- a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java +++ b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java @@ -23,7 +23,10 @@ import io.jenkins.blueocean.rest.model.Container; import io.jenkins.blueocean.rest.model.Containers; import io.jenkins.blueocean.rest.model.GenericResource; +import org.joda.time.format.DateTimeFormat; +import org.joda.time.format.DateTimeFormatter; import org.kohsuke.stapler.QueryParameter; +import org.kohsuke.stapler.export.Exported; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -37,6 +40,9 @@ * @author Vivek Pandey */ public abstract class AbstractRunImpl extends BlueRun { + + public static final DateTimeFormatter DATE_FORMAT = DateTimeFormat.forPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ"); + protected final T run; protected final BlueOrganization organization; @@ -89,6 +95,26 @@ public Date getEnQueueTime() { return new Date(run.getTimeInMillis()); } + @Override + public String getEnQueueTimeString() { + return DATE_FORMAT.print(getEnQueueTime().getTime()); + } + + @Override + public String getStartTimeString(){ + return DATE_FORMAT.print(getStartTime().getTime()); + } + + @Override + public String getEndTimeString(){ + Date endTime = getEndTime(); + if(endTime == null) { + return null; + } else { + return DATE_FORMAT.print(endTime.getTime()); + } + } + @Override public BlueRunState getStateObj() { if(!run.hasntStartedYet() && run.isLogUpdated()) { diff --git a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/ChangeSetResource.java b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/ChangeSetResource.java index 1cfbe82a197..ec000defee2 100644 --- a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/ChangeSetResource.java +++ b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/ChangeSetResource.java @@ -52,7 +52,7 @@ public BlueUser getAuthor() { @Override public String getTimestamp(){ if(changeSet.getTimestamp() > 0) { - return BlueRun.DATE_FORMAT.print(changeSet.getTimestamp()); + return AbstractRunImpl.DATE_FORMAT.print(changeSet.getTimestamp()); }else{ return null; } diff --git a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/QueueItemImpl.java b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/QueueItemImpl.java index 63ff44795ba..1bc1aa3e9a4 100644 --- a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/QueueItemImpl.java +++ b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/QueueItemImpl.java @@ -15,6 +15,7 @@ import io.jenkins.blueocean.rest.model.BlueRun.BlueRunState; import io.jenkins.blueocean.service.embedded.rest.AbstractRunImpl.BlueCauseImpl; import jenkins.model.Jenkins; +import org.kohsuke.stapler.export.Exported; import java.util.Collection; import java.util.Date; @@ -67,6 +68,11 @@ public Date getQueuedTime() { return new Date(item.getInQueueSince()); } + @Exported(name=QUEUED_TIME) + public String getQueuedTimeString(){ + return AbstractRunImpl.DATE_FORMAT.print(getQueuedTime().getTime()); + } + @Override public int getExpectedBuildNumber() { return expectedBuildNumber; diff --git a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/QueuedBlueRun.java b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/QueuedBlueRun.java index 9cce3cb45ae..9a0399b2432 100644 --- a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/QueuedBlueRun.java +++ b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/QueuedBlueRun.java @@ -122,6 +122,21 @@ public BlueRun stop(Boolean blocking, Integer timeOutInSecs) { return new QueuedBlueRun(BlueRunState.FINISHED, BlueRunResult.ABORTED, item, parent); } + @Override + public String getStartTimeString() { + return null; + } + + @Override + public String getEnQueueTimeString() { + return null; + } + + @Override + public String getEndTimeString() { + return null; + } + @Override public String getArtifactsZipFile() { return null; diff --git a/blueocean-rest/pom.xml b/blueocean-rest/pom.xml index 09d3a622dac..4b848c878b6 100644 --- a/blueocean-rest/pom.xml +++ b/blueocean-rest/pom.xml @@ -23,10 +23,6 @@ io.jenkins.blueocean.rest.annotation capability-annotation - - joda-time - joda-time - diff --git a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BluePipelineStep.java b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BluePipelineStep.java index 4062ed8b6bb..fbe39028f4a 100644 --- a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BluePipelineStep.java +++ b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BluePipelineStep.java @@ -49,12 +49,7 @@ public abstract class BluePipelineStep extends Resource{ public abstract Date getStartTime(); @Exported(name = START_TIME) - public final String getStartTimeString(){ - if(getStartTime() == null) { - return null; - } - return BlueRun.DATE_FORMAT.print(getStartTime().getTime()); - } + public abstract String getStartTimeString(); @Exported(name= DURATION_IN_MILLIS) public abstract Long getDurationInMillis(); diff --git a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueQueueItem.java b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueQueueItem.java index 54a7f01fbd8..b38b0861817 100644 --- a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueQueueItem.java +++ b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueQueueItem.java @@ -53,9 +53,8 @@ public abstract class BlueQueueItem extends Resource { public abstract Date getQueuedTime(); @Exported(name=QUEUED_TIME) - public final String getQueuedTimeString(){ - return BlueRun.DATE_FORMAT.print(getQueuedTime().getTime()); - } + public abstract String getQueuedTimeString(); + /** * * @return The expected build number of the build. This may change. diff --git a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java index 1cfa2f89522..5caa40cd99a 100644 --- a/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java +++ b/blueocean-rest/src/main/java/io/jenkins/blueocean/rest/model/BlueRun.java @@ -3,9 +3,6 @@ import io.jenkins.blueocean.commons.stapler.TreeResponse; import io.jenkins.blueocean.rest.Navigable; import io.jenkins.blueocean.rest.annotation.Capability; -import org.joda.time.format.DateTimeFormat; -import org.joda.time.format.DateTimeFormatter; -import org.joda.time.format.DateTimeFormatterBuilder; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.WebMethod; import org.kohsuke.stapler.export.Exported; @@ -14,7 +11,6 @@ import org.kohsuke.stapler.verb.PUT; import javax.annotation.Nonnull; -import java.text.SimpleDateFormat; import java.util.Collection; import java.util.Date; @@ -56,7 +52,6 @@ public abstract class BlueRun extends Resource { /** Date String format */ public static final String DATE_FORMAT_STRING = "yyyy-MM-dd'T'HH:mm:ss.SSSZ"; - public static final DateTimeFormatter DATE_FORMAT = DateTimeFormat.forPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ"); /** * @return name of the organization @@ -98,19 +93,15 @@ public abstract class BlueRun extends Resource { * @return run start time */ @Exported(name=START_TIME) - public final String getStartTimeString(){ - return DATE_FORMAT.print(getStartTime().getTime()); - } + public abstract String getStartTimeString(); /** * @return Time when build is scheduled and is in queue waiting for executor */ public abstract Date getEnQueueTime(); - @Exported(name=ENQUEUE_TIME) - public final String getEnQueueTimeString() { - return DATE_FORMAT.print(getEnQueueTime().getTime()); - } + @Exported(name=ENQUEUE_TIME) + public abstract String getEnQueueTimeString(); /** * @return Build end time @@ -118,14 +109,7 @@ public final String getEnQueueTimeString() { public abstract Date getEndTime(); @Exported(name=END_TIME) - public final String getEndTimeString(){ - Date endTime = getEndTime(); - if(endTime == null) { - return null; - } else { - return DATE_FORMAT.print(endTime.getTime()); - } - } + public abstract String getEndTimeString(); /** * @return Build duration in milli seconds From f878a36aa24fe48b2b3f5a1cd68146b730d68ce9 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Wed, 16 Aug 2017 13:09:01 +1000 Subject: [PATCH 11/22] Move BlueTestSummary cache into RunImpl --- .../embedded/rest/AbstractRunImpl.java | 23 +++++++++++- .../service/embedded/rest/Caches.java | 37 ------------------- 2 files changed, 22 insertions(+), 38 deletions(-) delete mode 100644 blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/Caches.java diff --git a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java index ee86952d5bc..b09d84c620e 100644 --- a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java +++ b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java @@ -1,6 +1,9 @@ package io.jenkins.blueocean.service.embedded.rest; import com.google.common.base.Function; +import com.google.common.base.Optional; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import com.google.common.collect.Collections2; import hudson.model.Action; import hudson.model.CauseAction; @@ -32,7 +35,9 @@ import javax.annotation.Nullable; import java.util.Collection; import java.util.Date; +import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; /** * Basic {@link BlueRun} implementation. @@ -43,6 +48,12 @@ public abstract class AbstractRunImpl extends BlueRun { public static final DateTimeFormatter DATE_FORMAT = DateTimeFormat.forPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ"); + private static final long TEST_SUMMARY_CACHE_MAX_SIZE = Long.getLong("TEST_SUMMARY_CACHE_MAX_SIZE", 10000); + private static final Cache> TEST_SUMMARY = CacheBuilder.newBuilder() + .maximumSize(TEST_SUMMARY_CACHE_MAX_SIZE) + .expireAfterAccess(1, TimeUnit.DAYS) + .build(); + protected final T run; protected final BlueOrganization organization; @@ -214,7 +225,17 @@ public BlueTestResultContainer getTests() { @Override public BlueTestSummary getTestSummary() { if (getStateObj() == BlueRunState.FINISHED) { - return Caches.loadTestSummary(run, this).orNull(); + try { + return TEST_SUMMARY.get(run.getExternalizableId(), new Callable>() { + @Override + public Optional call() throws Exception { + BlueTestSummary summary = BlueTestResultFactory.resolve(run, parent).summary; + return summary == null ? Optional.absent() : Optional.of(summary); + } + }).orNull(); + } catch (ExecutionException e) { + return null; + } } else { return BlueTestResultFactory.resolve(run, this).summary; } diff --git a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/Caches.java b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/Caches.java deleted file mode 100644 index c85822e7fdd..00000000000 --- a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/Caches.java +++ /dev/null @@ -1,37 +0,0 @@ -package io.jenkins.blueocean.service.embedded.rest; - -import com.google.common.base.Optional; -import com.google.common.cache.Cache; -import com.google.common.cache.CacheBuilder; -import hudson.model.Run; -import io.jenkins.blueocean.rest.Reachable; -import io.jenkins.blueocean.rest.factory.BlueTestResultFactory; -import io.jenkins.blueocean.rest.model.BlueTestSummary; - -import java.util.concurrent.Callable; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; - -final class Caches { - - private static final long TEST_SUMMARY_CACHE_MAX_SIZE = Long.getLong("TEST_SUMMARY_CACHE_MAX_SIZE", 10000); - - private static final Cache> TEST_SUMMARY = CacheBuilder.newBuilder() - .maximumSize(TEST_SUMMARY_CACHE_MAX_SIZE) - .expireAfterAccess(1, TimeUnit.DAYS) - .build(); - - static Optional loadTestSummary(final Run run, final Reachable parent) { - try { - return TEST_SUMMARY.get(run.getExternalizableId(), new Callable>() { - @Override - public Optional call() throws Exception { - BlueTestSummary summary = BlueTestResultFactory.resolve(run, parent).summary; - return summary == null ? Optional.absent() : Optional.of(summary); - } - }); - } catch (ExecutionException e) { - return Optional.absent(); - } - } -} From 717cf53b1b73a22e1870f8efe2fc2ae70f6bfb06 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Wed, 16 Aug 2017 14:59:16 +1000 Subject: [PATCH 12/22] publishHTML --- Jenkinsfile | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Jenkinsfile b/Jenkinsfile index 92a0a068cf8..8b3d3a06c9f 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -71,6 +71,13 @@ node() { if (err.toString().contains('exit code 143')) { currentBuild.result = "ABORTED" } + + publishHTML target: [ + allowMissing: false, + reportDir: 'target/code-coverage/report/', + reportFiles: '*/**' + reportName: 'coverage' + ] } finally { stage('Cleanup') { sh "${env.WORKSPACE}/acceptance-tests/runner/scripts/stop-selenium.sh" From e8eab7363e89eb1b1eceb7f503b70a980afe6a0b Mon Sep 17 00:00:00 2001 From: James Dumay Date: Wed, 16 Aug 2017 15:32:24 +1000 Subject: [PATCH 13/22] LOL this fixes jacoco --- blueocean-rest-impl/pom.xml | 15 ++++++ .../rest/BlueJUnitTestResultTest.java | 46 +++++++++++++++++++ .../embedded/rest/DefaultRunImplTest.java | 36 +++++++++++++++ .../rest/BlueJUnitTestResultTest.jenkinsfile | 11 +++++ 4 files changed, 108 insertions(+) create mode 100644 blueocean-rest-impl/src/test/java/io/jenkins/blueocean/service/embedded/rest/BlueJUnitTestResultTest.java create mode 100644 blueocean-rest-impl/src/test/java/io/jenkins/blueocean/service/embedded/rest/DefaultRunImplTest.java create mode 100644 blueocean-rest-impl/src/test/resources/io/jenkins/blueocean/service/embedded/rest/BlueJUnitTestResultTest.jenkinsfile diff --git a/blueocean-rest-impl/pom.xml b/blueocean-rest-impl/pom.xml index 76bad63ec95..8c3afe76433 100644 --- a/blueocean-rest-impl/pom.xml +++ b/blueocean-rest-impl/pom.xml @@ -146,6 +146,21 @@ 1.7 test + + org.jenkins-ci.plugins.workflow + workflow-cps + test + + + org.jenkins-ci.plugins.workflow + workflow-durable-task-step + test + + + org.jenkins-ci.plugins.workflow + workflow-basic-steps + test + diff --git a/blueocean-rest-impl/src/test/java/io/jenkins/blueocean/service/embedded/rest/BlueJUnitTestResultTest.java b/blueocean-rest-impl/src/test/java/io/jenkins/blueocean/service/embedded/rest/BlueJUnitTestResultTest.java new file mode 100644 index 00000000000..944905dc3a3 --- /dev/null +++ b/blueocean-rest-impl/src/test/java/io/jenkins/blueocean/service/embedded/rest/BlueJUnitTestResultTest.java @@ -0,0 +1,46 @@ +package io.jenkins.blueocean.service.embedded.rest; + +import com.google.common.base.Charsets; +import com.google.common.collect.Iterators; +import com.google.common.io.Resources; +import hudson.model.Run; +import io.jenkins.blueocean.rest.Reachable; +import io.jenkins.blueocean.rest.factory.BlueRunFactory; +import io.jenkins.blueocean.rest.hal.Link; +import io.jenkins.blueocean.rest.model.BlueRun; +import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; +import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + +import java.net.URL; + +public class BlueJUnitTestResultTest { + + @Rule + public JenkinsRule j = new JenkinsRule(); + + @Test + public void createsTestResult() throws Exception { + URL resource = Resources.getResource(getClass(), "BlueJUnitTestResultTest.jenkinsfile"); + String jenkinsFile = Resources.toString(resource, Charsets.UTF_8); + + WorkflowJob p = j.createProject(WorkflowJob.class, "project"); + p.setDefinition(new CpsFlowDefinition(jenkinsFile, false)); + p.save(); + + Run r = p.scheduleBuild2(0).waitForStart(); + j.waitUntilNoActivity(); + + BlueRun test = BlueRunFactory.getRun(r, new Reachable() { + @Override + public Link getLink() { + return new Link("test"); + } + }); + + Assert.assertEquals(3, Iterators.size(test.getTests().iterator())); + } +} diff --git a/blueocean-rest-impl/src/test/java/io/jenkins/blueocean/service/embedded/rest/DefaultRunImplTest.java b/blueocean-rest-impl/src/test/java/io/jenkins/blueocean/service/embedded/rest/DefaultRunImplTest.java new file mode 100644 index 00000000000..080080ca360 --- /dev/null +++ b/blueocean-rest-impl/src/test/java/io/jenkins/blueocean/service/embedded/rest/DefaultRunImplTest.java @@ -0,0 +1,36 @@ +package io.jenkins.blueocean.service.embedded.rest; + +import hudson.model.Run; +import io.jenkins.blueocean.rest.Reachable; +import io.jenkins.blueocean.rest.factory.BlueRunFactory; +import io.jenkins.blueocean.rest.hal.Link; +import io.jenkins.blueocean.rest.model.BlueRun; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class DefaultRunImplTest { + @Rule + public JenkinsRule j = new JenkinsRule(); + + @Test + public void unknownRunTypeResolvesToDefaultRunImpl() throws Exception { + Reachable parent = new Reachable() { + @Override + public Link getLink() { + return new Link("foo"); + } + }; + + Run run = mock(Run.class); + when(run.getParent()).thenReturn(j.createFreeStyleProject()); + + BlueRun blueRun = BlueRunFactory.getRun(run, parent); + Assert.assertNotNull(blueRun); + Assert.assertTrue(blueRun instanceof DefaultRunImpl); + } +} diff --git a/blueocean-rest-impl/src/test/resources/io/jenkins/blueocean/service/embedded/rest/BlueJUnitTestResultTest.jenkinsfile b/blueocean-rest-impl/src/test/resources/io/jenkins/blueocean/service/embedded/rest/BlueJUnitTestResultTest.jenkinsfile new file mode 100644 index 00000000000..6caaf137ef2 --- /dev/null +++ b/blueocean-rest-impl/src/test/resources/io/jenkins/blueocean/service/embedded/rest/BlueJUnitTestResultTest.jenkinsfile @@ -0,0 +1,11 @@ +node { + test = "\n" + + " \n" + + " \n" + + " \n" + + " details about failure \n" + + " \n" + + ""; + writeFile file:'result.xml', text: test + junit 'result.xml' +} From fa90145c9acbdd6dc660712c7c3483a55b705aa1 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Wed, 16 Aug 2017 15:33:52 +1000 Subject: [PATCH 14/22] Make same as stapler PR --- .../blueocean/commons/stapler/export/ModelBuilder.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java b/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java index f38b17b22d6..1f530b3bec1 100644 --- a/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java +++ b/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java @@ -50,10 +50,6 @@ public Model get(Class type) throws NotExportableException { return get(type, null, null); } - public Model getOrNull(Class type) throws NotExportableException { - return get(type, null, null); - } - public Model get(Class type, @CheckForNull Class propertyOwner, @Nullable String property) { if (type.getAnnotation(ExportedBean.class) == null) { throw new NotExportableException(type); From 9e806285e78971bb7cf4217d62ead2e449363e2a Mon Sep 17 00:00:00 2001 From: James Dumay Date: Wed, 16 Aug 2017 15:38:43 +1000 Subject: [PATCH 15/22] Revert Jenkinsfile change --- Jenkinsfile | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 8b3d3a06c9f..92a0a068cf8 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -71,13 +71,6 @@ node() { if (err.toString().contains('exit code 143')) { currentBuild.result = "ABORTED" } - - publishHTML target: [ - allowMissing: false, - reportDir: 'target/code-coverage/report/', - reportFiles: '*/**' - reportName: 'coverage' - ] } finally { stage('Cleanup') { sh "${env.WORKSPACE}/acceptance-tests/runner/scripts/stop-selenium.sh" From 5aa7adf06f35fe532cee50a84aff1e346e3cf3a5 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Wed, 16 Aug 2017 18:30:40 +1000 Subject: [PATCH 16/22] Fix not null build failure --- .../jenkins/blueocean/rest/impl/pipeline/PipelineStepImpl.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineStepImpl.java b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineStepImpl.java index 5f5bd501111..3c0b0bbd1df 100644 --- a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineStepImpl.java +++ b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/PipelineStepImpl.java @@ -106,9 +106,6 @@ public Long getDurationInMillis() { @Override public String getStartTimeString(){ - if(getStartTime() == null) { - return null; - } return AbstractRunImpl.DATE_FORMAT.print(getStartTime().getTime()); } From ae740f509d06f2cd35c2b6eb6090b44003fefd7f Mon Sep 17 00:00:00 2001 From: James Dumay Date: Thu, 17 Aug 2017 09:17:29 +1000 Subject: [PATCH 17/22] Remove stapler parts of micro opt --- .../commons/stapler/export/ModelBuilder.java | 14 +- .../commons/stapler/export/Property.java | 210 +++++++++--------- 2 files changed, 103 insertions(+), 121 deletions(-) diff --git a/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java b/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java index 1f530b3bec1..9d5de5c53eb 100644 --- a/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java +++ b/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/ModelBuilder.java @@ -44,22 +44,12 @@ public class ModelBuilder { /*package*/ final Map models = new ConcurrentHashMap(); public Model get(Class type) throws NotExportableException { - if (type.getAnnotation(ExportedBean.class) == null) { - throw new NotExportableException(type); - } return get(type, null, null); } - public Model get(Class type, @CheckForNull Class propertyOwner, @Nullable String property) { - if (type.getAnnotation(ExportedBean.class) == null) { - throw new NotExportableException(type); - } - return getOrNull(type, propertyOwner, property); - } - - public Model getOrNull(Class type, @CheckForNull Class propertyOwner, @Nullable String property) throws NotExportableException { + public Model get(Class type, @CheckForNull Class propertyOwner, @Nullable String property) throws NotExportableException { Model m = models.get(type); - if(m==null && type.getAnnotation(ExportedBean.class) != null) { + if(m==null) { m = new Model(this, type, propertyOwner, property); } return m; diff --git a/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/Property.java b/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/Property.java index 68be75dd46a..ec54b0e8a4e 100644 --- a/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/Property.java +++ b/blueocean-commons/src/main/java/io/jenkins/blueocean/commons/stapler/export/Property.java @@ -141,14 +141,15 @@ public void writeTo(Object object, TreePruner pruner, DataWriter writer) throws if (merge) { // merged property will get all its properties written here if (d != null) { - if (d.getClass().getAnnotation(ExportedBean.class) == null) { + Model model; + try { + model = owner.get(d.getClass(), parent.type, name); + } catch (NotExportableException e) { if(writer.getExportConfig().isSkipIfFail()){ return; - } else { - throw new NotExportableException(d.getClass()); } + throw e; } - Model model = owner.getOrNull(d.getClass(), parent.type, name); model.writeNestedObjectTo(d, new FilteringTreePruner(parent.HAS_PROPERTY_NAME_IN_ANCESTORY,child), writer); } } else { @@ -200,127 +201,118 @@ private void writeValue(Type expected, Object value, TreePruner pruner, DataWrit Class c = value.getClass(); - if (c.getAnnotation(ExportedBean.class) == null) { - handleNotExportable(expected, value, pruner, writer, skipIfFail, c); - } else { - try { - Model model = owner.getOrNull(c, parent.type, name); - if (model == null) { - throw new NotExportableException(c); - } - writer.startObject(); - model.writeNestedObjectTo(value, pruner, writer); - writer.endObject(); - } catch (NotExportableException ex) { - handleNotExportable(expected, value, pruner, writer, skipIfFail, c); - return; - } - } - + Model model; try { - writer.type(expected, value.getClass()); - } catch (AbstractMethodError _) { - // legacy impl that doesn't understand it - } - } - - private void handleNotExportable(Type expected, Object value, TreePruner pruner, DataWriter writer, boolean skipIfFail, Class c) throws IOException { - if(STRING_TYPES.contains(c)) { - writer.value(value.toString()); - return; - } - if(PRIMITIVE_TYPES.contains(c)) { - writer.valuePrimitive(value); - return; - } - Class act = c.getComponentType(); - if (act !=null) { // array - Range r = pruner.getRange(); - writer.startArray(); - if (value instanceof Object[]) { - // typical case - for (Object item : r.apply((Object[]) value)) { - writeBuffered(act, item, pruner, writer); - } - } else { - // more generic case - int len = Math.min(r.max, Array.getLength(value)); - for (int i=r.min; i) value).entrySet()) { - BufferedDataWriter buffer = new BufferedDataWriter(writer.getExportConfig()); - try { - writeStartObjectNullType(buffer); - buffer.name(verboseMap[0]); - writeValue(null, e.getKey(), pruner, buffer); - buffer.name(verboseMap[1]); - writeValue(null, e.getValue(), pruner, buffer); - buffer.endObject(); - buffer.finished(); - } catch (IOException x) { - if (x.getCause() instanceof InvocationTargetException) { - LOGGER.log(Level.WARNING, "skipping export of " + e, x); - } + if (value instanceof Object[]) { + // typical case + for (Object item : r.apply((Object[]) value)) { + writeBuffered(act, item, pruner, writer); + } + } else { + // more generic case + int len = Math.min(r.max, Array.getLength(value)); + for (int i=r.min; i) value).entrySet()) { - BufferedDataWriter buffer = new BufferedDataWriter(writer.getExportConfig()); - try { - buffer.name(e.getKey().toString()); - writeValue(null, e.getValue(), pruner, buffer); - buffer.finished(); - } catch (IOException x) { - if (x.getCause() instanceof InvocationTargetException) { - LOGGER.log(Level.WARNING, "skipping export of " + e, x); + return; + } + if(value instanceof Iterable) { + writer.startArray(); + Type expectedItemType = Types.getTypeArgument(expected, 0, null); + for (Object item : pruner.getRange().apply((Iterable) value)) { + writeBuffered(expectedItemType, item, pruner, writer); + } + writer.endArray(); + return; + } + if(value instanceof Map) { + if (verboseMap!=null) {// verbose form + writer.startArray(); + for (Map.Entry e : ((Map) value).entrySet()) { + BufferedDataWriter buffer = new BufferedDataWriter(writer.getExportConfig()); + try { + writeStartObjectNullType(buffer); + buffer.name(verboseMap[0]); + writeValue(null, e.getKey(), pruner, buffer); + buffer.name(verboseMap[1]); + writeValue(null, e.getValue(), pruner, buffer); + buffer.endObject(); + buffer.finished(); + } catch (IOException x) { + if (x.getCause() instanceof InvocationTargetException) { + LOGGER.log(Level.WARNING, "skipping export of " + e, x); + } } + buffer.commit(writer); } - buffer.commit(writer); + writer.endArray(); + } else {// compact form + writeStartObjectNullType(writer); + for (Map.Entry e : ((Map) value).entrySet()) { + BufferedDataWriter buffer = new BufferedDataWriter(writer.getExportConfig()); + try { + buffer.name(e.getKey().toString()); + writeValue(null, e.getValue(), pruner, buffer); + buffer.finished(); + } catch (IOException x) { + if (x.getCause() instanceof InvocationTargetException) { + LOGGER.log(Level.WARNING, "skipping export of " + e, x); + } + } + buffer.commit(writer); + } + writer.endObject(); } + return; + } + if(value instanceof Date) { + writer.valuePrimitive(((Date) value).getTime()); + return; + } + if(value instanceof Calendar) { + writer.valuePrimitive(((Calendar) value).getTimeInMillis()); + return; + } + if(value instanceof Enum) { + writer.value(value.toString()); + return; + } + + if (skipIfFail) { + writer.startObject(); writer.endObject(); + return; } - return; - } - if(value instanceof Date) { - writer.valuePrimitive(((Date) value).getTime()); - return; - } - if(value instanceof Calendar) { - writer.valuePrimitive(((Calendar) value).getTimeInMillis()); - return; - } - if(value instanceof Enum) { - writer.value(value.toString()); - return; + + throw ex; } - if (skipIfFail) { - writer.startObject(); - writer.endObject(); - return; + try { + writer.type(expected, value.getClass()); + } catch (AbstractMethodError _) { + // legacy impl that doesn't understand it } - throw new NotExportableException(c); + + writer.startObject(); + model.writeNestedObjectTo(value, pruner, writer); + writer.endObject(); } private static class BufferedDataWriter implements DataWriter { From 2e717f75ecd71851121e879072cca5804e039502 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Thu, 17 Aug 2017 10:13:16 +1000 Subject: [PATCH 18/22] Move branch metadata tests out to own class, fix tests --- .../impl/pipeline/BranchContainerImpl.java | 8 ++ .../blueocean/rest/impl/pipeline/Caches.java | 12 ++- .../impl/pipeline/BranchMetadataTest.java | 89 +++++++++++++++++++ .../rest/impl/pipeline/MultiBranchTest.java | 29 ------ 4 files changed, 105 insertions(+), 33 deletions(-) create mode 100644 blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/BranchMetadataTest.java diff --git a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImpl.java b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImpl.java index 9000aa849b5..f7411937658 100644 --- a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImpl.java +++ b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImpl.java @@ -37,6 +37,14 @@ public int compare(BluePipeline _pipeline1, BluePipeline _pipeline2) { BranchImpl pipeline1 = (BranchImpl)_pipeline1; BranchImpl pipeline2 = (BranchImpl)_pipeline2; + if (pipeline1.getBranch() == null) { + return -1; + } + + if (pipeline2.getBranch() == null) { + return 1; + } + // If one pipeline isnt the primary there is no need to go further if(pipeline1.getBranch().isPrimary() && !pipeline2.getBranch().isPrimary()) { return -1; diff --git a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/Caches.java b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/Caches.java index 49b07accdd2..a56d6606d8f 100644 --- a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/Caches.java +++ b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/Caches.java @@ -1,5 +1,6 @@ package io.jenkins.blueocean.rest.impl.pipeline; +import com.google.common.base.Objects; import com.google.common.base.Optional; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; @@ -18,6 +19,7 @@ import jenkins.scm.api.metadata.PrimaryInstanceMetadataAction; import jenkins.scm.api.mixin.ChangeRequestSCMHead; +import javax.annotation.Nullable; import java.util.concurrent.TimeUnit; class Caches { @@ -35,13 +37,13 @@ class Caches { static final LoadingCache> PULL_REQUEST_METADATA = CacheBuilder.newBuilder() .maximumSize(PR_METADATA_CACHE_MAX_SIZE) .expireAfterAccess(1, TimeUnit.DAYS) - .build(new PullRequestCacheLoader(Jenkins.getInstance().getItemGroup())); + .build(new PullRequestCacheLoader(null)); static final LoadingCache> BRANCH_METADATA = CacheBuilder.newBuilder() .maximumSize(BRANCH_METADATA_CACHE_MAX_SIZE) .expireAfterAccess(1, TimeUnit.DAYS) - .build(new BranchCacheLoader(Jenkins.getInstance().getItemGroup())); + .build(new BranchCacheLoader(null)); @Extension public static class ListenerImpl extends ItemListener { @@ -80,12 +82,13 @@ public void onDeleted(Item item) { static class BranchCacheLoader extends CacheLoader> { private Jenkins jenkins; - BranchCacheLoader(Jenkins jenkins) { + BranchCacheLoader(@Nullable Jenkins jenkins) { this.jenkins = jenkins; } @Override public Optional load(String key) throws Exception { + Jenkins jenkins = Objects.firstNonNull(this.jenkins, Jenkins.getInstance()); Job job = jenkins.getItemByFullName(key, Job.class); if (job == null) { return Optional.absent(); @@ -103,12 +106,13 @@ public Optional load(String key) throws Exception { static class PullRequestCacheLoader extends CacheLoader> { private Jenkins jenkins; - PullRequestCacheLoader(Jenkins jenkins) { + PullRequestCacheLoader(@Nullable Jenkins jenkins) { this.jenkins = jenkins; } @Override public Optional load(String key) throws Exception { + Jenkins jenkins = Objects.firstNonNull(this.jenkins, Jenkins.getInstance()); Job job = jenkins.getItemByFullName(key, Job.class); if (job == null) { return Optional.absent(); diff --git a/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/BranchMetadataTest.java b/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/BranchMetadataTest.java new file mode 100644 index 00000000000..afaafe1d0cd --- /dev/null +++ b/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/BranchMetadataTest.java @@ -0,0 +1,89 @@ +package io.jenkins.blueocean.rest.impl.pipeline; + +import hudson.model.Job; +import io.jenkins.blueocean.rest.hal.Link; +import io.jenkins.blueocean.rest.model.BlueOrganization; +import jenkins.model.Jenkins; +import jenkins.scm.api.metadata.ObjectMetadataAction; +import jenkins.scm.api.metadata.PrimaryInstanceMetadataAction; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@RunWith(PowerMockRunner.class) +@PrepareForTest(Jenkins.class) +public class BranchMetadataTest { + + Jenkins jenkins; + BlueOrganization org; + Job job; + BranchImpl branch; + + @Before + public void setup() { + Caches.BRANCH_METADATA.invalidateAll(); + + jenkins = mock(Jenkins.class); + + PowerMockito.mockStatic(Jenkins.class); + when(Jenkins.getInstance()).thenReturn(jenkins); + + when(jenkins.getFullName()).thenReturn(""); + + job = mock(Job.class); + when(job.getParent()).thenReturn(jenkins); + when(job.getFullName()).thenReturn("BobsPipeline"); + when(jenkins.getItemByFullName("BobsPipeline", Job.class)).thenReturn(job); + + org = mock(BlueOrganization.class); + branch = new BranchImpl(org, job, new Link("foo")); + } + + @Test + public void testGetURL() { + assertNull(branch.getBranch()); + + ObjectMetadataAction oma = new ObjectMetadataAction( + "My Branch", + "A feature branch", + "https://path/to/branch" + ); + when(job.getAction(ObjectMetadataAction.class)).thenReturn(oma); + + Caches.BRANCH_METADATA.invalidateAll(); + + assertNotNull(branch.getBranch()); + assertFalse(branch.getBranch().isPrimary()); + assertEquals("https://path/to/branch", branch.getBranch().getUrl()); + } + + @Test + public void testBranchInfo() { + assertNull(branch.getBranch()); + + ObjectMetadataAction oma = new ObjectMetadataAction( + "My Branch", + "A feature branch", + "https://path/to/branch" + ); + when(job.getAction(ObjectMetadataAction.class)).thenReturn(oma); + when(job.getAction(PrimaryInstanceMetadataAction.class)).thenReturn(new PrimaryInstanceMetadataAction()); + + Caches.BRANCH_METADATA.invalidateAll(); + + assertEquals("https://path/to/branch", branch.getBranch().getUrl()); + assertTrue(branch.getBranch().isPrimary()); + } +} diff --git a/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/MultiBranchTest.java b/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/MultiBranchTest.java index 29fb16e9caa..daeabb1a377 100644 --- a/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/MultiBranchTest.java +++ b/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/MultiBranchTest.java @@ -96,35 +96,6 @@ private boolean runAllTests() { return System.getenv("RUN_MULTIBRANCH_TESTS") != null; } - @Test - public void testGetURL() { - BlueOrganization org = mock(BlueOrganization.class); - Job job = mock(Job.class); - BranchImpl branch = new BranchImpl(org, job, new Link("foo")); - assertNotNull(branch.getBranch()); - assertNull(branch.getBranch().getUrl()); - assertFalse(branch.getBranch().isPrimary()); - ObjectMetadataAction oma = new ObjectMetadataAction("My Branch", "A feature branch", "https://path/to/branch"); - when(job.getAction(ObjectMetadataAction.class)).thenReturn(oma); - assertEquals("https://path/to/branch", branch.getBranch().getUrl()); - } - - @Test - public void testBranchInfo() { - BlueOrganization org = mock(BlueOrganization.class); - Job job = mock(Job.class); - BranchImpl branch = new BranchImpl(org, job, new Link("foo")); - assertNotNull(branch.getBranch()); - assertNull(branch.getBranch().getUrl()); - assertFalse(branch.getBranch().isPrimary()); - ObjectMetadataAction oma = new ObjectMetadataAction("My Branch", "A feature branch", "https://path/to/branch"); - when(job.getAction(ObjectMetadataAction.class)).thenReturn(oma); - assertEquals("https://path/to/branch", branch.getBranch().getUrl()); - assertFalse(branch.getBranch().isPrimary()); - when(job.getAction(PrimaryInstanceMetadataAction.class)).thenReturn(new PrimaryInstanceMetadataAction()); - assertTrue(branch.getBranch().isPrimary()); - } - @Test public void resolveMbpLink() throws Exception { WorkflowMultiBranchProject mp = j.jenkins.createProject(WorkflowMultiBranchProject.class, "p"); From a02d0ef511c6e40c140894a684dade269636efa1 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Thu, 17 Aug 2017 10:22:15 +1000 Subject: [PATCH 19/22] Ensure users checkForNull on pull request and branch fetch --- .../io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java | 3 +++ .../blueocean/service/embedded/rest/AbstractRunImpl.java | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java index 6322fbb050f..e8c4505e680 100644 --- a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java +++ b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchImpl.java @@ -23,6 +23,7 @@ import org.kohsuke.stapler.export.Exported; import org.kohsuke.stapler.export.ExportedBean; +import javax.annotation.CheckForNull; import java.util.Collection; import java.util.concurrent.ExecutionException; @@ -46,11 +47,13 @@ public BranchImpl(BlueOrganization org, Job job, Link parent) { } @Exported(name = PullRequest.PULL_REQUEST, inline = true, skipNull = true) + @CheckForNull public PullRequest getPullRequest() { return PullRequest.get(job); } @Exported(name = Branch.BRANCH, inline = true) + @CheckForNull public Branch getBranch() { return Branch.getBranch(job); } diff --git a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java index b09d84c620e..3c13558d723 100644 --- a/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java +++ b/blueocean-rest-impl/src/main/java/io/jenkins/blueocean/service/embedded/rest/AbstractRunImpl.java @@ -38,6 +38,8 @@ import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.logging.Level; +import java.util.logging.Logger; /** * Basic {@link BlueRun} implementation. @@ -47,6 +49,7 @@ public abstract class AbstractRunImpl extends BlueRun { public static final DateTimeFormatter DATE_FORMAT = DateTimeFormat.forPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ"); + private static final Logger LOGGER = Logger.getLogger(AbstractRunImpl.class.getName()); private static final long TEST_SUMMARY_CACHE_MAX_SIZE = Long.getLong("TEST_SUMMARY_CACHE_MAX_SIZE", 10000); private static final Cache> TEST_SUMMARY = CacheBuilder.newBuilder() @@ -234,6 +237,7 @@ public Optional call() throws Exception { } }).orNull(); } catch (ExecutionException e) { + LOGGER.log(Level.SEVERE, "Could not load summary from cache", e); return null; } } else { From c34f393d99e589ba87a1c419ae2a61da0efe5c51 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Thu, 17 Aug 2017 10:28:52 +1000 Subject: [PATCH 20/22] Invalidate caches --- .../blueocean/rest/impl/pipeline/BranchContainerImplTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImplTest.java b/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImplTest.java index ffe365f4e35..f98f8ea0189 100644 --- a/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImplTest.java +++ b/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImplTest.java @@ -31,6 +31,7 @@ public class BranchContainerImplTest extends PipelineBaseTest { public void setup() throws Exception{ super.setup(); setupScm(); + Caches.BRANCH_METADATA.invalidateAll(); } @Test From 5fb42e0acfcdf1f67176070d114b71a671a11d9b Mon Sep 17 00:00:00 2001 From: James Dumay Date: Thu, 17 Aug 2017 10:50:05 +1000 Subject: [PATCH 21/22] Fix findbugs... good thing i added @CheckForNull here --- .../rest/impl/pipeline/BranchContainerImpl.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImpl.java b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImpl.java index f7411937658..3a80dd7a6bf 100644 --- a/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImpl.java +++ b/blueocean-pipeline-api-impl/src/main/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImpl.java @@ -37,20 +37,22 @@ public int compare(BluePipeline _pipeline1, BluePipeline _pipeline2) { BranchImpl pipeline1 = (BranchImpl)_pipeline1; BranchImpl pipeline2 = (BranchImpl)_pipeline2; - if (pipeline1.getBranch() == null) { + BranchImpl.Branch branch1 = pipeline1.getBranch(); + if (branch1 == null) { return -1; } - if (pipeline2.getBranch() == null) { + BranchImpl.Branch branch2 = pipeline2.getBranch(); + if (branch2 == null) { return 1; } // If one pipeline isnt the primary there is no need to go further - if(pipeline1.getBranch().isPrimary() && !pipeline2.getBranch().isPrimary()) { + if(branch1.isPrimary() && !branch2.isPrimary()) { return -1; } - if(!pipeline1.getBranch().isPrimary() && pipeline2.getBranch().isPrimary()) { + if(!branch1.isPrimary() && branch2.isPrimary()) { return 1; } From 96011756eb0517a852c45cb096863c1ebc49a197 Mon Sep 17 00:00:00 2001 From: James Dumay Date: Thu, 17 Aug 2017 11:26:41 +1000 Subject: [PATCH 22/22] Remove non deterministic assert --- .../blueocean/rest/impl/pipeline/BranchContainerImplTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImplTest.java b/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImplTest.java index f98f8ea0189..383a25ee693 100644 --- a/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImplTest.java +++ b/blueocean-pipeline-api-impl/src/test/java/io/jenkins/blueocean/rest/impl/pipeline/BranchContainerImplTest.java @@ -70,7 +70,6 @@ public void testBranchOrdering() throws Exception { Assert.assertEquals(4,l.size()); Map o = (Map)l.get(1); Map o2 = (Map)l.get(0); - Assert.assertTrue(o.get("name").equals("feature2") || o.get("name").equals("feature4")); WorkflowJob j1 = findBranchProject(mp, (String)o.get("name")); j.waitForCompletion(j1.getLastBuild());