Details
-
Type:
Bug
-
Status: Reopened (View Workflow)
-
Priority:
Major
-
Resolution: Unresolved
-
Component/s: conditional-buildstep-plugin
-
Labels:None
-
Similar Issues:
Description
Some exotic build steps (cloudbees templates ones, that are dynamically generated) aren't created using @DataBoundConstructor databinding, but by a custom newInstance implementation in constructor.
DefaultBuilderDescriptorLister is checking the buildStep for a @DataBoundConstructor, for some unknown reason (I've searched the git history, but this whole code was added as a single commit), but shouldn't imho
Attachments
Issue Links
- depends on
-
-
- Resolved
-
- is blocking
-
-
- Resolved
-
- is related to
-
JENKINS-6797 Add @DataBoundConstructor
-
- Resolved
-
- links to
Activity
Code changed in jenkins
User: Nicolas De Loof
Path:
pom.xml
src/main/java/org/jenkinsci/plugins/conditionalbuildstep/lister/DefaultBuilderDescriptorLister.java
http://jenkins-ci.org/commit/conditional-buildstep-plugin/132178a305c0d8ece4bd58622811497745816667
Log:
[FIXED JENKINS-15445] rely on 1.536 so JENKINS-18629 doesn’t prevent BuildStep without a @DataBoundConstructor to be used.
@ndeloof Looking at JENKINS-18629 I think we ca implement this now with a check for the version of the core. So if core > 1.536 then we ignore the databoundconsrructor limitation.
Nicolas, do you have any code/plugin where this would work with the 'newInstance' method?
In my experience this does not (yet) work - it seams that Stapler is not falling back in every case to the 'newInstance' method if the DataBoundConstructor is not present.
Code changed in jenkins
User: imod
Path:
src/main/java/org/jenkinsci/plugins/conditionalbuildstep/lister/DefaultBuilderDescriptorLister.java
http://jenkins-ci.org/commit/conditional-buildstep-plugin/022995b2e0e9744a382b553609f446c9625885bb
Log:
revert change for JENKINS-15445 until the issues with DataBoundConstructor and newInstance are sorted out
The history is not in this plugin because the code was copied from the Flexible Publish plugin.
The original check is both necessary - and not aggressive enough. The descriptors should be filtered for those with newInstance methods too. Some plugins appear to have both DBC and newInstance!
The reason they need to be filtered is that the Flexible publish creates instances using a data bound constructor. Any instances of build step that are contained within will fail to be configured corectly if it uses configuration from the form and also uses newInstance.
- 100% of buildsteps I tested which used newInstance failed to be correctly configured when the form was submitted.*
The descriptor listers are used to enable publishers as build steps and build steps as publishers (using the Any Build Step plugin). For this reason, build steps returned need to be configurable using both configuration mechanisms.
If you believe that the newInstance will be correctly configured when contained within a DBC configured container, please add a unit test to the Any Build Step plugin showing successful configuration submission of a newInstance configured buildstep (builder) inside the Flexible Publish publisher.
Maybe I've missed something, and you can help me undestand, but like I said, I did not find a single build step that configured correctly when using newInstance.
bap
added a comment - The history is not in this plugin because the code was copied from the Flexible Publish plugin.
The original check is both necessary - and not aggressive enough. The descriptors should be filtered for those with newInstance methods too. Some plugins appear to have both DBC and newInstance!
The reason they need to be filtered is that the Flexible publish creates instances using a data bound constructor. Any instances of build step that are contained within will fail to be configured corectly if it uses configuration from the form and also uses newInstance.
100% of buildsteps I tested which used newInstance failed to be correctly configured when the form was submitted.*
The descriptor listers are used to enable publishers as build steps and build steps as publishers (using the Any Build Step plugin). For this reason, build steps returned need to be configurable using both configuration mechanisms.
If you believe that the newInstance will be correctly configured when contained within a DBC configured container, please add a unit test to the Any Build Step plugin showing successful configuration submission of a newInstance configured buildstep (builder) inside the Flexible Publish publisher.
Maybe I've missed something, and you can help me undestand, but like I said, I did not find a single build step that configured correctly when using newInstance. Code changed in jenkins
User: Nicolas De Loof
Path:
src/main/java/org/jenkinsci/plugins/conditionalbuildstep/lister/DefaultBuilderDescriptorLister.java
http://jenkins-ci.org/commit/conditional-buildstep-plugin/fcfc219e627afaa8b19395674a756fe4dda43e7d
Log:
[FIXED JENKINS-15445] use Functions#getBuilderDescriptors to select the matching descriptors
don't check for @DataBoundConstructor. Descriptor may not use it to create instances
Nicolas De Loof This PR does not seem to cause issues with the latest version of Jenkins anymore. Can we get it re-attempted or am I just missing something?