+ Since 1.598 overrides of Descriptor.getId were not correctly handled by form binding, breaking at least the CloudBees Templates plugin.
+ (issue 26781)
Archiving of large artifacts. Tar implementation cannot handle files having a size >8GB.
(issue 10629)
diff --git a/core/src/main/java/hudson/DescriptorExtensionList.java b/core/src/main/java/hudson/DescriptorExtensionList.java
index a3917b2e84b02cd1f9c997f966b9d485977040bc..e4b9a3e9fc5336a71cc4b07292870cbcc9f2db70 100644
--- a/core/src/main/java/hudson/DescriptorExtensionList.java
+++ b/core/src/main/java/hudson/DescriptorExtensionList.java
@@ -109,6 +109,7 @@ public class DescriptorExtensionList, D extends Descrip
*
* @param fqcn
* Fully qualified name of the descriptor, not the describable.
+ * @deprecated {@link Descriptor#getId} is supposed to be used for new code, not the descriptor class name.
*/
public D find(String fqcn) {
return Descriptor.find(this,fqcn);
diff --git a/core/src/main/java/hudson/model/ComputerSet.java b/core/src/main/java/hudson/model/ComputerSet.java
index 0502c64329059df41e606c956aa8227b3b564e6c..9bec52ce5a18bf33a2b97696e8f614dfdf92233e 100644
--- a/core/src/main/java/hudson/model/ComputerSet.java
+++ b/core/src/main/java/hudson/model/ComputerSet.java
@@ -290,6 +290,7 @@ public final class ComputerSet extends AbstractModelObject implements Describabl
JSONObject formData = req.getSubmittedForm();
formData.put("name", fixedName);
+ // TODO type is probably NodeDescriptor.id but confirm
Node result = NodeDescriptor.all().find(type).newInstance(req, formData);
app.addNode(result);
diff --git a/core/src/main/java/hudson/model/Descriptor.java b/core/src/main/java/hudson/model/Descriptor.java
index 2b5bcef0cf608b16b8759371d0929c48406e61be..940b8beab947af9646a0fec21d5a1b04243c2edf 100644
--- a/core/src/main/java/hudson/model/Descriptor.java
+++ b/core/src/main/java/hudson/model/Descriptor.java
@@ -909,14 +909,23 @@ public abstract class Descriptor> implements Saveable {
if (formData!=null) {
for (Object o : JSONArray.fromObject(formData)) {
JSONObject jo = (JSONObject)o;
- String kind = jo.optString("$class", null);
- if (kind == null) {
- // Legacy: Remove once plugins have been staged onto $class
- kind = jo.getString("kind");
+ Descriptor d = null;
+ // 'kind' and '$class' are mutually exclusive (see class-entry.jelly), but to be more lenient on the reader side,
+ // we check them both anyway. 'kind' (which maps to ID) is more unique than '$class', which can have multiple matching
+ // Descriptors, so we prefer 'kind' if it's present.
+ String kind = jo.optString("kind", null);
+ if (kind != null) {
+ d = findById(descriptors, kind);
+ }
+ if (d == null) {
+ kind = jo.getString("$class");
+ d = findByDescribableClassName(descriptors, kind);
+ if (d == null) d = findByClassName(descriptors, kind);
}
- Descriptor d = find(descriptors, kind);
if (d != null) {
items.add(d.newInstance(req, jo));
+ } else {
+ LOGGER.warning("Received unexpected formData for descriptor " + kind);
}
}
}
@@ -925,22 +934,58 @@ public abstract class Descriptor> implements Saveable {
}
/**
- * Finds a descriptor from a collection by its class name.
+ * Finds a descriptor from a collection by its ID.
+ * @param id should match {@link #getId}
+ * @since 1.610
*/
- public static @CheckForNull T find(Collection extends T> list, String className) {
+ public static @CheckForNull T findById(Collection extends T> list, String id) {
+ for (T d : list) {
+ if(d.getId().equals(id))
+ return d;
+ }
+ return null;
+ }
+
+ /**
+ * Finds a descriptor from a collection by the class name of the {@link Descriptor}.
+ * This is useless as of the introduction of {@link #getId} and so only very old compatibility code needs it.
+ */
+ private static @CheckForNull T findByClassName(Collection extends T> list, String className) {
for (T d : list) {
if(d.getClass().getName().equals(className))
return d;
}
- // Since we introduced Descriptor.getId(), it is a preferred method of identifying descriptor by a string.
- // To make that migration easier without breaking compatibility, let's also match up with the id.
+ return null;
+ }
+
+ /**
+ * Finds a descriptor from a collection by the class name of the {@link Describable} it describes.
+ * @param className should match {@link Class#getName} of a {@link #clazz}
+ * @since 1.610
+ */
+ public static @CheckForNull T findByDescribableClassName(Collection extends T> list, String className) {
for (T d : list) {
- if(d.getId().equals(className))
+ if(d.clazz.getName().equals(className))
return d;
}
return null;
}
+ /**
+ * Finds a descriptor from a collection by its class name or ID.
+ * @deprecated choose between {@link #findById} or {@link #findByDescribableClassName}
+ */
+ public static @CheckForNull T find(Collection extends T> list, String string) {
+ T d = findByClassName(list, string);
+ if (d != null) {
+ return d;
+ }
+ return findById(list, string);
+ }
+
+ /**
+ * @deprecated choose between {@link #findById} or {@link #findByDescribableClassName}
+ */
public static @CheckForNull Descriptor find(String className) {
return find(ExtensionList.lookup(Descriptor.class),className);
}
diff --git a/core/src/main/java/hudson/model/Items.java b/core/src/main/java/hudson/model/Items.java
index 7630b05348949d7228c6ecb0a64cb5a37b43eeef..03432c4d271ac57d6f25459e990bcdc3219715e5 100644
--- a/core/src/main/java/hudson/model/Items.java
+++ b/core/src/main/java/hudson/model/Items.java
@@ -148,6 +148,9 @@ public class Items {
return result;
}
+ /**
+ * @deprecated Underspecified what the parameter is. {@link Descriptor#getId}? A {@link Describable} class name?
+ */
public static TopLevelItemDescriptor getDescriptor(String fqcn) {
return Descriptor.find(all(), fqcn);
}
diff --git a/core/src/main/java/hudson/tools/DownloadFromUrlInstaller.java b/core/src/main/java/hudson/tools/DownloadFromUrlInstaller.java
index 938325f6681de75c893be2ceb6169f5596773a44..c8397599f0ec5a7155e900de8e3227ae5c562bd7 100644
--- a/core/src/main/java/hudson/tools/DownloadFromUrlInstaller.java
+++ b/core/src/main/java/hudson/tools/DownloadFromUrlInstaller.java
@@ -122,15 +122,16 @@ public abstract class DownloadFromUrlInstaller extends ToolInstaller {
}
protected Downloadable createDownloadable() {
- return new Downloadable(getId());
+ return new Downloadable(getDownloadableId());
}
/**
* This ID needs to be unique, and needs to match the ID token in the JSON update file.
*
* By default we use the fully-qualified class name of the {@link DownloadFromUrlInstaller} subtype.
+ * @since 1.610
*/
- public String getId() {
+ public String getDownloadableId() {
return clazz.getName().replace('$','.');
}
@@ -144,7 +145,7 @@ public abstract class DownloadFromUrlInstaller extends ToolInstaller {
* @return never null.
*/
public List extends Installable> getInstallables() throws IOException {
- JSONObject d = Downloadable.get(getId()).getData();
+ JSONObject d = Downloadable.get(getDownloadableId()).getData();
if(d==null) return Collections.emptyList();
return Arrays.asList(((InstallableList)JSONObject.toBean(d,InstallableList.class)).list);
}
diff --git a/core/src/main/java/hudson/tools/ToolLocationNodeProperty.java b/core/src/main/java/hudson/tools/ToolLocationNodeProperty.java
index 55526dfb19c01b0667759326513fdf693abc372c..881dc5e1a127c1efabd6cc3aa5f69a0a71418452 100644
--- a/core/src/main/java/hudson/tools/ToolLocationNodeProperty.java
+++ b/core/src/main/java/hudson/tools/ToolLocationNodeProperty.java
@@ -167,6 +167,7 @@ public class ToolLocationNodeProperty extends NodeProperty {
return home;
}
+ @SuppressWarnings("deprecation") // TODO this was mistakenly made to be the ToolDescriptor class name, rather than .id as you would expect; now baked into serial form
public ToolDescriptor getType() {
if (descriptor == null) {
descriptor = (ToolDescriptor) Descriptor.find(type);
diff --git a/core/src/main/java/hudson/util/DescriptorList.java b/core/src/main/java/hudson/util/DescriptorList.java
index 3f90c68c777495df7dd1a6a13024f14ac85a477d..88003c687fb435a0efa316b7efe03e15e65791d9 100644
--- a/core/src/main/java/hudson/util/DescriptorList.java
+++ b/core/src/main/java/hudson/util/DescriptorList.java
@@ -196,6 +196,7 @@ public final class DescriptorList> extends AbstractList
/**
* Finds the descriptor that has the matching fully-qualified class name.
+ * @deprecated Underspecified what the parameter is. {@link Descriptor#getId}? A {@link Describable} class name?
*/
public Descriptor find(String fqcn) {
return Descriptor.find(this,fqcn);
diff --git a/core/src/main/resources/lib/form/class-entry.jelly b/core/src/main/resources/lib/form/class-entry.jelly
index e03af3a754661f219fbe7e7c1695192eb1a601a3..c5e1575e860bb19bc6a39c006af1926a9b5ce4e8 100644
--- a/core/src/main/resources/lib/form/class-entry.jelly
+++ b/core/src/main/resources/lib/form/class-entry.jelly
@@ -26,6 +26,16 @@ THE SOFTWARE.
xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
Invisible <f:entry> type for embedding a descriptor's $class field.
+
+ Most of the time a Descriptor has an unique class name that we can use to instantiate the right Describable
+ class, so we use the '$class' to represent that to clarify the intent.
+
+ In some other times, such as templates, there are multiple Descriptors with the same Descriptor.clazz
+ but different IDs, and in that case we put 'kind' to indicate that. In this case, to avoid confusing
+ readers we do not put non-unique '$class'.
+
+ See Descriptor.newInstancesFromHeteroList for how the reader side is handled.
+
The describable class that we are instantiating via structured form submission.
@@ -36,12 +46,14 @@ THE SOFTWARE.
-
-
-
-
-
-
-
+
+
+
+
+
+
+
+
+
-
\ No newline at end of file
+
diff --git a/test/src/test/java/hudson/model/DescriptorTest.java b/test/src/test/java/hudson/model/DescriptorTest.java
index 1c2ef01f8190815dcab15b51012ed8a6aced03d5..06885a568f91a77e751e2717c9db0f0b2b9a6280 100644
--- a/test/src/test/java/hudson/model/DescriptorTest.java
+++ b/test/src/test/java/hudson/model/DescriptorTest.java
@@ -24,14 +24,24 @@
package hudson.model;
+import hudson.Launcher;
import hudson.model.Descriptor.PropertyType;
+import hudson.tasks.BuildStepDescriptor;
+import hudson.tasks.Builder;
import hudson.tasks.Shell;
+import java.io.IOException;
+import java.util.List;
+import jenkins.model.Jenkins;
+import net.sf.json.JSONObject;
import static org.junit.Assert.*;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.TestExtension;
+import org.kohsuke.stapler.StaplerRequest;
+@SuppressWarnings({"unchecked", "rawtypes"})
public class DescriptorTest {
public @Rule JenkinsRule rule = new JenkinsRule();
@@ -51,4 +61,57 @@ public class DescriptorTest {
}
}
+ @Issue("JENKINS-26781")
+ @Test public void overriddenId() throws Exception {
+ FreeStyleProject p = rule.createFreeStyleProject();
+ p.getBuildersList().add(new BuilderImpl("builder-a"));
+ rule.configRoundtrip(p);
+ List builders = p.getBuildersList();
+ assertEquals(1, builders.size());
+ assertEquals(BuilderImpl.class, builders.get(0).getClass());
+ assertEquals("builder-a", ((BuilderImpl) builders.get(0)).id);
+ rule.assertLogContains("running builder-a", rule.buildAndAssertSuccess(p));
+ p.getBuildersList().replace(new BuilderImpl("builder-b"));
+ rule.configRoundtrip(p);
+ builders = p.getBuildersList();
+ assertEquals(1, builders.size());
+ assertEquals(BuilderImpl.class, builders.get(0).getClass());
+ assertEquals("builder-b", ((BuilderImpl) builders.get(0)).id);
+ rule.assertLogContains("running builder-b", rule.buildAndAssertSuccess(p));
+ }
+ private static final class BuilderImpl extends Builder {
+ private final String id;
+ BuilderImpl(String id) {
+ this.id = id;
+ }
+ @Override public boolean perform(AbstractBuild,?> build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException {
+ listener.getLogger().println("running " + getDescriptor().getId());
+ return true;
+ }
+ @Override public Descriptor getDescriptor() {
+ return (Descriptor) Jenkins.getInstance().getDescriptorByName(id);
+ }
+ }
+ private static final class DescriptorImpl extends BuildStepDescriptor {
+ private final String id;
+ DescriptorImpl(String id) {
+ super(BuilderImpl.class);
+ this.id = id;
+ }
+ @Override public String getId() {
+ return id;
+ }
+ @Override public Builder newInstance(StaplerRequest req, JSONObject formData) throws Descriptor.FormException {
+ return new BuilderImpl(id);
+ }
+ @Override public String getDisplayName() {
+ return id;
+ }
+ @Override public boolean isApplicable(Class extends AbstractProject> jobType) {
+ return true;
+ }
+ }
+ @TestExtension("overriddenId") public static final BuildStepDescriptor builderA = new DescriptorImpl("builder-a");
+ @TestExtension("overriddenId") public static final BuildStepDescriptor builderB = new DescriptorImpl("builder-b");
+
}