Add support for the upcoming '$class' annotation change...

[mattmoor]

... while maintaining support for older cores.

This is anticipation of this PR: https://github.com/stapler/stapler/pull/39 getting merged into the Jenkins core.

[mattmoor]

Failure seems erroneous.

[ikedam]

Note: Changing "stapler-class" to "$stapler" will be introduced in stapler-1.232 (not yet released).

[mattmoor]

'$class' not '$stapler' and yes, I know, but I wanted to get a couple PRs like this going so that plugin authors could merge them.

My changes for '$class' to plugins should be backwards compatible, and the core will temporarily continue to emit 'stapler-class', until I can get the various PRs merged into the couple plugins like this that look for stapler-class directly.

[jglick]

IMO this whole class should be deleted. What is it for?

[mattmoor]

It looks like it is used to implement custom binding for build steps it is wrapping: https://github.com/jenkinsci/build-timeout-plugin/blob/12cb64b88b9f5b173ab94694761def4183487461/src/main/java/hudson/plugins/build_timeout/operations/BuildStepOperation.java#L241

We have a few places that we do something similar, and I do something similar in the DSL.

[mattmoor]

FYI, this is needed to wrap steps without @DataBoundConstructor annotations.

[mattmoor]

More precisely, it is needed to wrap steps with "newInstance" methods.

I know we had a problem with Gradle, for instance.

[ikedam]

I do this trick also for flexible-publish and authorize-project. It might better for me to find another way to call newInstance of proper classes.

I'll re-trigger the test by closing and reopening this ticket. (it's the only way I know to retrigger).

[mattmoor]

With so many places doing this it may be worth factoring a utility plugin for this; we have multiple places that do this as well :)

Here is how I factored it in yaml-project, which works around Stapler's need for binding to occur within a request context, and supports jobs and views as well: https://github.com/jenkinsci/yaml-project-plugin/blob/master/src/main/java/com/google/jenkins/plugins/dsl/util/Binder.java

@jglick it occurred to me that (depending on how you guys are doing the snippet stuff in Workflow) that this could be a problem for you as well?

@ikedam I can submit a PR for those plugins as well, they didn't come up in my original search, so thanks for the pointers.

LMK if this sounds like a useful refactoring, and I will try and get to it soon.

[jglick]

this is needed to wrap steps without @DataBoundConstructor annotations

There are some? Can we just file bugs against them?

[jglick]

The existence of Binder suggests that Stapler needs to be refactored a bit to make its binding code available in a broader context. (CC @kohsuke)

[ikedam]

authorize-project overrides newInstance like this: https://github.com/jenkinsci/authorize-project-plugin/blob/authorize-project-1.0.3/src/main/java/org/jenkinsci/plugins/authorizeproject/strategy/SpecificUsersAuthorizationStrategy.java#L313

[mattmoor]

https://github.com/jenkinsci/authorize-project-plugin/pull/7 https://github.com/jenkinsci/flexible-publish-plugin/pull/5

@jglick Yes, I agree. However, I think that Jenkins could generally use a disentangling from the notion that everything happens from within the context of a web request, which is a huge project of which this is one small piece. :)

[ikedam]

I thought Descriptor#newInstancesFromHeteroList may help us, but it doesn't. A hetelo-list uses its own property "kind" instead of "stapler-class"

[mattmoor]

Yep, the plan is to unify around "$class". -M

On Sun, Oct 26, 2014 at 4:58 PM, ikedam notifications@github.com wrote:

I thought Descriptor#newInstancesFromHeteroList http://javadoc.jenkins-ci.org/hudson/model/Descriptor.html#newInstancesFromHeteroList%28org.kohsuke.stapler.StaplerRequest,%20net.sf.json.JSONObject,%20java.lang.String,%20java.util.Collection%29 may help us, but it doesn't. A hetelo-list uses its own property "kind" instead of "stapler-class"

— Reply to this email directly or view it on GitHub https://github.com/jenkinsci/build-timeout-plugin/pull/35#issuecomment-60538235 .

Matthew Moore DI/Docker (aka Convoy) Developer Infrastructure @ Google

[ikedam]

I conclude this is the only way for dropdownDescriptor to work both with stapler < 1.232 and stapler >= 1.232. I'll merge this change as soon as Jenkins core with $stapler gets available and I test this fix with that.

Anyway, as there are other plugins using stapler-class, I want an announcement in jenkinsci-dev. (I don't think it's reasonable for @mattmoor to make pull requests for all those plugins)

[mattmoor]

Sure, I think a mail to jenkins-dev sounds like a good idea.

FWIW, I think that we should continue having Jenkins forms submit both $class and stapler-class (for some time, to facilitate the migration), which is what my core changes for this do.

Do you have any other suggestions? I apologize if you think I am being unreasonable...

thanks for the feedback and suggestion, -M

[ikedam]

I think I should apologize if I made you uncomfortable...

Your pull requests look perfect to me. Anyway I know accessing stapler-class is a bad workaround and can cause incompatibility. The problem is that we don't know until when we should fix plugins... how Jenkins core developers plan about that?

Submitting both stapler-class and $class may not help so much as most plugins uses stapler-class in jelly files rather than java files. (That is, they expect Jenkins recieve that rather than submit that.)

We'd better prepare an instructions for plugin developers.

• For plugins using stapler-class in jelly files: they should use proper tags provided by Jenkins core, such as descriptorRadioList and dropdownDescriptorSelector.
• For plugins using stapler-class in Java codes: I think your solution is the only way for now.
  • Jenkins core should provide an alternate way in a future release so that plugins no longer need to access stapler-class and $class (maybe the way @jglick mentioned).

[ikedam]

Sorry for long absence. I tested this with jenkins-1.588-JENKINS-25403 and saw it works correct. I also fixed the error message with 33b21d98ae412d5ff2657f3d442173d06b0bee9c.