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

Optimizations to REST API #1323

Merged
merged 22 commits into from
Aug 17, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions blueocean-jwt/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,9 @@
<artifactId>mailer</artifactId>
</dependency>

<dependency>
<groupId>joda-time</groupId>
<artifactId>joda-time</artifactId>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<JwtRsaDigitalSignatureKey> 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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,22 @@ public int compare(BluePipeline _pipeline1, BluePipeline _pipeline2) {
BranchImpl pipeline1 = (BranchImpl)_pipeline1;
BranchImpl pipeline2 = (BranchImpl)_pipeline2;

BranchImpl.Branch branch1 = pipeline1.getBranch();
if (branch1 == null) {
return -1;
}

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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@
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;

import javax.annotation.CheckForNull;
import java.util.Collection;
import java.util.concurrent.ExecutionException;

Expand All @@ -38,8 +37,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;

Expand All @@ -49,22 +46,16 @@ public BranchImpl(BlueOrganization org, Job job, Link parent) {
this.parent = parent;
}

@Exported(name = ISSUES, skipNull = true, inline = true)
public Collection<BlueIssue> getIssues() {
return BlueIssueFactory.resolve(job);
}

@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() {
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
Expand Down Expand Up @@ -108,13 +99,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<BlueIssue> issues;

public Branch(String url, boolean primary) {
public Branch(String url, boolean primary, Collection<BlueIssue> issues) {
this.url = url;
this.primary = primary;
this.issues = issues;
}

@Exported(name = BRANCH_URL)
Expand All @@ -127,6 +121,11 @@ public boolean isPrimary() {
return primary;
}

@Exported(name = ISSUES, skipNull = true, inline = true)
public Collection<BlueIssue> getIssues() {
return issues;
}

public static Branch getBranch(final Job job) {
try {
return Caches.BRANCH_METADATA.get(job.getFullName()).orNull();
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -8,6 +9,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;
Expand Down Expand Up @@ -35,13 +37,13 @@ class Caches {
static final LoadingCache<String, Optional<PullRequest>> 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<String, Optional<Branch>> 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 {
Expand Down Expand Up @@ -80,12 +82,13 @@ public void onDeleted(Item item) {
static class BranchCacheLoader extends CacheLoader<String, Optional<Branch>> {
private Jenkins jenkins;

BranchCacheLoader(Jenkins jenkins) {
BranchCacheLoader(@Nullable Jenkins jenkins) {
this.jenkins = jenkins;
}

@Override
public Optional<Branch> 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();
Expand All @@ -96,19 +99,20 @@ public Optional<Branch> 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)));
}
}

static class PullRequestCacheLoader extends CacheLoader<String, Optional<PullRequest>> {
private Jenkins jenkins;

PullRequestCacheLoader(Jenkins jenkins) {
PullRequestCacheLoader(@Nullable Jenkins jenkins) {
this.jenkins = jenkins;
}

@Override
public Optional<PullRequest> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -96,6 +95,21 @@ public Date getEndTime() {
return null;
}

@Override
public String getStartTimeString() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe declare these as abstract? default implementation returning null can be misleading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

WDYM?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ignore that comment, for some reason I mistook it as one of the model class. All good here:)

return null;
}

@Override
public String getEnQueueTimeString() {
return null;
}

@Override
public String getEndTimeString() {
return null;
}

@Override
public Long getDurationInMillis() {
if(folderComputation.isBuilding()){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Edge> getEdges() {
return edges;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -103,6 +104,11 @@ public Long getDurationInMillis() {
return node.getTiming().getTotalDurationMillis();
}

@Override
public String getStartTimeString(){
return AbstractRunImpl.DATE_FORMAT.print(getStartTime().getTime());
}

@Override
public Object getLog() {
LogAction logAction = node.getNode().getAction(LogAction.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public class BranchContainerImplTest extends PipelineBaseTest {
public void setup() throws Exception{
super.setup();
setupScm();
Caches.BRANCH_METADATA.invalidateAll();
}

@Test
Expand Down Expand Up @@ -69,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());
Expand Down
Original file line number Diff line number Diff line change
@@ -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());
}
}
Loading