Skip to content

Commit

Permalink
Merge pull request #41 from stephenc/jenkins-44891
Browse files Browse the repository at this point in the history 
[FIXED JENKINS-44891] Migrate SCMSource's id field to a @DataBoundSetter
  • Loading branch information
@stephenc
stephenc committed Jun 14, 2017
2 parents d13c2f2 + db4c727 commit 4cf51ff
Show file tree
Hide file tree
Showing 11 changed files with 151 additions and 46 deletions.
57 changes: 56 additions & 1 deletion src/main/java/jenkins/scm/api/SCMSource.java
View file
Expand Up @@ -53,6 +53,8 @@
import java.util.logging.Logger;
import jenkins.model.TransientActionFactory;
import net.jcip.annotations.GuardedBy;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;

/**
* A {@link SCMSource} is responsible for fetching {@link SCMHead} and corresponding {@link SCMRevision} instances from
Expand Down Expand Up @@ -131,14 +133,67 @@ public boolean equals(Object obj) {
*
* @param id the id or {@code null}.
*/
@Deprecated
protected SCMSource(@CheckForNull String id) {
this.id = id == null ? UUID.randomUUID().toString() : id;
}

/**
* The ID of this source. The ID is not related to anything at all.
* Constructor.
*/
protected SCMSource() {
}

/**
* Sets the ID that is used to ensure this {@link SCMSource} can be retrieved from its {@link SCMSourceOwner}
* provided that this {@link SCMSource} does not already {@link #hasId()}.
* <p>
* Note this is a <strong>one-shot</strong> setter. If {@link #getId()} is called first, then its value will stick,
* otherwise the first call to {@link #setId(String)} will stick.
*
* @param id the ID, this is an opaque token expected to be unique within any one {@link SCMSourceOwner}.
* @see #hasId()
* @see #getId()
* @since 2.2.0
*/
@DataBoundSetter
public final synchronized void setId(@CheckForNull String id) {
if (this.id == null) {
this.id = id;
} else if (!this.id.equals(id)) {
throw new IllegalStateException("The ID cannot be changed after it has been set.");
}
}

/**
* Variant of {@link #setId(String)} that can be useful for method chaining.
* @param id the ID
* @return {@code this} for method chaining
*/
public final SCMSource withId(@CheckForNull String id) {
setId(id);
return this;
}

/**
* Returns {@code true} if and only if this {@link SCMSource} has been assigned an ID. Once an ID has been assigned
* it should be preserved.
*
* @return {@code true} if and only if this {@link SCMSource} has been assigned an ID.
* @see #setId(String)
* @see #getId()
* @since 2.2.0
*/
public final synchronized boolean hasId() {
return this.id != null;
}
/**
* The ID of this source. The ID is not related to anything at all. If this {@link SCMSource} does not have an ID
* then one will be generated.
*
* @return the ID of this source.
* @see #setId(String)
* @see #hasId()
*/
@NonNull
public final synchronized String getId() {
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/jenkins/scm/impl/NullSCMSource.java
View file
Expand Up @@ -53,7 +53,8 @@ public class NullSCMSource extends SCMSource {
* Constructor.
*/
public NullSCMSource() {
super(ID);
super();
setId(ID);
}

/**
Expand Down
49 changes: 39 additions & 10 deletions src/main/java/jenkins/scm/impl/SingleSCMSource.java
View file
Expand Up @@ -26,33 +26,32 @@
import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Extension;
import hudson.RestrictedSince;
import hudson.model.Describable;
import hudson.model.Descriptor;
import hudson.model.TaskListener;
import hudson.model.TopLevelItemDescriptor;
import hudson.scm.NullSCM;
import hudson.scm.SCM;
import hudson.scm.SCMDescriptor;
import jenkins.scm.api.SCMFile;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import jenkins.scm.api.SCMHead;
import jenkins.scm.api.SCMHeadEvent;
import jenkins.scm.api.SCMHeadObserver;
import jenkins.scm.api.SCMProbe;
import jenkins.scm.api.SCMProbeStat;
import jenkins.scm.api.SCMRevision;
import jenkins.scm.api.SCMSource;
import jenkins.scm.api.SCMSourceCriteria;
import jenkins.scm.api.SCMSourceDescriptor;
import jenkins.scm.api.SCMSourceOwner;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;

/**
* A single fixed branch using a legacy SCM implementation.
*
Expand All @@ -79,14 +78,30 @@ public class SingleSCMSource extends SCMSource {
/**
* Our constructor.
*
* @param id source id.
* @param name the name of the branch.
* @param scm the configuration.
* @since 2.2.0
*/
@SuppressWarnings("unused") // stapler
@DataBoundConstructor
public SingleSCMSource(String name, SCM scm) {
this.name = name;
this.scm = scm;
}

/**
* Legacy constructor.
*
* @param id source id.
* @param name the name of the branch.
* @param scm the configuration.
* @deprecated use {@link #SingleSCMSource(String, SCM)} and {@link #setId(String)}.
*/
@Restricted(NoExternalUse.class)
@RestrictedSince("2.2.0")
@Deprecated
public SingleSCMSource(String id, String name, SCM scm) {
super(id);
setId(id);
this.name = name;
this.scm = scm;
}
Expand Down Expand Up @@ -123,6 +138,20 @@ public SCM build(@NonNull SCMHead head, @CheckForNull SCMRevision revision) {
return new NullSCM();
}

/**
* {@inheritDoc}
*/
@Override
public String toString() {
return "SingleSCMSource{" +
"super=" + super.toString() +
", name='" + name + '\'' +
", scm=" + scm +
", head=" + head +
", revisionHash=" + revisionHash +
'}';
}

/**
* Our revision class.
*/
Expand Down
20 changes: 13 additions & 7 deletions src/test/java/jenkins/scm/api/SCMRevisionActionTest.java
View file
Expand Up @@ -16,7 +16,7 @@ public class SCMRevisionActionTest {
public void given__legacyData__when__gettingRevision__then__legacyReturned() throws Exception {
MockSCMController c = MockSCMController.create();
try {
MockSCMSource source = new MockSCMSource("foo", c, "test", new MockSCMDiscoverBranches());
MockSCMSource source = new MockSCMSource(c, "test", new MockSCMDiscoverBranches());
SCMRevision revision = new MockSCMRevision(new MockSCMHead("head"), "hash");
Actionable a = new ActionableImpl();
a.addAction(new SCMRevisionAction(revision, null));
Expand All @@ -30,9 +30,12 @@ public void given__legacyData__when__gettingRevision__then__legacyReturned() thr
public void given__mixedData__when__gettingRevision__then__legacyReturnedForUnmatched() throws Exception {
MockSCMController c = MockSCMController.create();
try {
MockSCMSource source1 = new MockSCMSource("foo", c, "test", new MockSCMDiscoverBranches());
MockSCMSource source2 = new MockSCMSource("bar", c, "test", new MockSCMDiscoverBranches());
MockSCMSource source3 = new MockSCMSource("manchu", c, "test", new MockSCMDiscoverBranches());
MockSCMSource source1 = new MockSCMSource(c, "test", new MockSCMDiscoverBranches());
source1.setId("foo");
MockSCMSource source2 = new MockSCMSource(c, "test", new MockSCMDiscoverBranches());
source2.setId("bar");
MockSCMSource source3 = new MockSCMSource( c, "test", new MockSCMDiscoverBranches());
source3.setId("manchu");
SCMRevision revision1 = new MockSCMRevision(new MockSCMHead("head1"), "hash1");
SCMRevision revision2 = new MockSCMRevision(new MockSCMHead("head2"), "hash2");
SCMRevision revision3 = new MockSCMRevision(new MockSCMHead("head3"), "hash3");
Expand All @@ -52,9 +55,12 @@ public void given__mixedData__when__gettingRevision__then__legacyReturnedForUnma
public void given__mixedData__when__gettingRevision__then__firstlegacyReturnedForUnmatched() throws Exception {
MockSCMController c = MockSCMController.create();
try {
MockSCMSource source1 = new MockSCMSource("foo", c, "test", new MockSCMDiscoverBranches());
MockSCMSource source2 = new MockSCMSource("bar", c, "test", new MockSCMDiscoverBranches());
MockSCMSource source3 = new MockSCMSource("manchu", c, "test", new MockSCMDiscoverBranches());
MockSCMSource source1 = new MockSCMSource(c, "test", new MockSCMDiscoverBranches());
source1.setId("foo");
MockSCMSource source2 = new MockSCMSource(c, "test", new MockSCMDiscoverBranches());
source2.setId("bar");
MockSCMSource source3 = new MockSCMSource(c, "test", new MockSCMDiscoverBranches());
source3.setId("manchu");
SCMRevision revision1 = new MockSCMRevision(new MockSCMHead("head1"), "hash1");
SCMRevision revision2 = new MockSCMRevision(new MockSCMHead("head2"), "hash2");
SCMRevision revision3 = new MockSCMRevision(new MockSCMHead("head3"), "hash3");
Expand Down
11 changes: 7 additions & 4 deletions src/test/java/jenkins/scm/impl/SingleSCMSourceTest.java
View file
Expand Up @@ -74,7 +74,6 @@ public void configRoundtrip() throws Exception {
try {
c.createRepository("foo");
SingleSCMSource source = new SingleSCMSource(
"the-id",
"the-name",
new MockSCM(
c,
Expand All @@ -83,17 +82,21 @@ public void configRoundtrip() throws Exception {
null
)
);
source.setId("the-id");
SCMSourceBuilder builder = new SCMSourceBuilder(source);
r.assertEqualDataBoundBeans(new SCMSourceBuilder(new SingleSCMSource(
"the-id",
SingleSCMSource expected = new SingleSCMSource(
"the-name",
new MockSCM(
c,
"foo",
new MockSCMHead("master"),
null
)
)), r.configRoundtrip(builder));
);
expected.setId("the-id");
SCMSource actual = r.configRoundtrip(builder).scm;
System.out.printf("expected=%s%nactual=%s%n", expected, actual);
r.assertEqualDataBoundBeans(expected, actual);
} finally {
c.close();
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/jenkins/scm/impl/SymbolAnnotationsTest.java
View file
Expand Up @@ -86,7 +86,7 @@ public class SymbolAnnotationsTest {
MockSCMController c = MockSCMController.create();
try {
c.createRepository("test");
MockSCMSource instance = new MockSCMSource(null, c, "test", new MockSCMDiscoverBranches(),
MockSCMSource instance = new MockSCMSource(c, "test", new MockSCMDiscoverBranches(),
new MockSCMDiscoverChangeRequests(), new MockSCMDiscoverTags(),
new WildcardSCMHeadFilterTrait("*", "ignore"), new RegexSCMHeadFilterTrait("i.*"));
assertThat(DescribableModel.uninstantiate2_(instance).toString(), allOf(
Expand Down Expand Up @@ -156,7 +156,7 @@ public class SymbolAnnotationsTest {
try {
c.createRepository("test");
SingleSCMNavigator instance = new SingleSCMNavigator("foo",
Collections.<SCMSource>singletonList(new MockSCMSource(null, c, "test"))
Collections.<SCMSource>singletonList(new MockSCMSource(c, "test"))
);
assertThat(DescribableModel.uninstantiate2_(instance).toString(), allOf(
startsWith("@"),
Expand Down
3 changes: 2 additions & 1 deletion src/test/java/jenkins/scm/impl/mock/MockSCMNavigator.java
View file
Expand Up @@ -114,7 +114,8 @@ public void visitSources(@NonNull SCMSourceObserver observer) throws IOException
@NonNull
@Override
public SCMSource create(@NonNull String name) {
return new MockSCMSourceBuilder(getId() + "::" + name, controller, name)
return new MockSCMSourceBuilder(controller, name)
.withId(getId() + "::" + name)
.withRequest(request)
.build();
}
Expand Down
17 changes: 9 additions & 8 deletions src/test/java/jenkins/scm/impl/mock/MockSCMSource.java
View file
Expand Up @@ -29,6 +29,7 @@
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.Nullable;
import hudson.Extension;
import hudson.RestrictedSince;
import hudson.model.Action;
import hudson.model.TaskListener;
import hudson.scm.SCM;
Expand Down Expand Up @@ -63,6 +64,8 @@
import jenkins.scm.impl.UncategorizedSCMHeadCategory;
import org.apache.commons.io.IOUtils;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;

Expand All @@ -73,27 +76,25 @@ public class MockSCMSource extends SCMSource {
private transient MockSCMController controller;

@DataBoundConstructor
public MockSCMSource(@CheckForNull String id, String controllerId, String repository, List<SCMSourceTrait> traits) {
super(id);
public MockSCMSource(String controllerId, String repository, List<SCMSourceTrait> traits) {
this.controllerId = controllerId;
this.repository = repository;
this.traits = new ArrayList<SCMSourceTrait>(traits);
}

public MockSCMSource(@CheckForNull String id, String controllerId, String repository, SCMSourceTrait... traits) {
this(id, controllerId, repository, Arrays.asList(traits));
public MockSCMSource(String controllerId, String repository, SCMSourceTrait... traits) {
this(controllerId, repository, Arrays.asList(traits));
}

public MockSCMSource(String id, MockSCMController controller, String repository, List<SCMSourceTrait> traits) {
super(id);
public MockSCMSource(MockSCMController controller, String repository, List<SCMSourceTrait> traits) {
this.controllerId = controller.getId();
this.controller = controller;
this.repository = repository;
this.traits = new ArrayList<SCMSourceTrait>(traits);
}

public MockSCMSource(String id, MockSCMController controller, String repository, SCMSourceTrait... traits) {
this(id, controller, repository, Arrays.asList(traits));
public MockSCMSource(MockSCMController controller, String repository, SCMSourceTrait... traits) {
this(controller, repository, Arrays.asList(traits));
}

public String getControllerId() {
Expand Down
25 changes: 17 additions & 8 deletions src/test/java/jenkins/scm/impl/mock/MockSCMSourceBuilder.java
View file
Expand Up @@ -28,32 +28,41 @@
import jenkins.scm.api.trait.SCMSourceBuilder;

public class MockSCMSourceBuilder extends SCMSourceBuilder<MockSCMSourceBuilder, MockSCMSource> {
private final String id;
private String id;
private final String controllerId;
private final MockSCMController controller;
private final String repository;

public MockSCMSourceBuilder(String id, MockSCMController controller, String repository) {
public MockSCMSourceBuilder(MockSCMController controller, String repository) {
super(MockSCMSource.class, repository);
this.id = id;
this.controllerId = controller.getId();
this.controller = controller;
this.repository = repository;
}

public MockSCMSourceBuilder(String id, String controllerId, String repository) {
public MockSCMSourceBuilder(String controllerId, String repository) {
super(MockSCMSource.class, repository);
this.id = id;
this.controllerId = controllerId;
this.controller = null;
this.repository = repository;
}

public final MockSCMSourceBuilder withId(String id) {
this.id = id;
return this;
}

public final String id() {
return id;
}

@NonNull
@Override
public MockSCMSource build() {
return controller == null
? new MockSCMSource(id, controllerId, repository, traits())
: new MockSCMSource(id, controller, repository, traits());
MockSCMSource source = controller == null
? new MockSCMSource(controllerId, repository, traits())
: new MockSCMSource(controller, repository, traits());
source.setId(id());
return source;
}
}
2 changes: 1 addition & 1 deletion src/test/java/jenkins/scm/impl/trait/RegexSCMHeadFilterTraitTest.java
View file
Expand Up @@ -51,7 +51,7 @@ public void given_sourceWithRegexRule_when_scanning_then_ruleApplied() throws Ex
c.createRepository("foo");
c.createBranch("foo", "fork");
c.createBranch("foo", "alt");
MockSCMSource src = new MockSCMSource("foo", c, "foo", new MockSCMDiscoverBranches(), new RegexSCMHeadFilterTrait("[fm].*"));
MockSCMSource src = new MockSCMSource(c, "foo", new MockSCMDiscoverBranches(), new RegexSCMHeadFilterTrait("[fm].*"));
Map<SCMHead, SCMRevision> result = src.fetch(null, SCMHeadObserver.collect(), null, null).result();
Set<String> names = new TreeSet<String>();
for (SCMHead h: result.keySet()) {
Expand Down

0 comments on commit 4cf51ff

Please sign in to comment.