proposed improvement to Descriptor.configure (WiP) :
https://github.com/ndeloof/jenkins/tree/JENKINS-48018
*tl;dr: *
provide default implementation to Descriptor#configure using databinding.
reset descriptor to default values based on properties types default *or*
convention to declare a public {{property}}_default constant. Also updated
jelly taglibs to use this same constant in form inputs as default value
(until explicitly set)
>>> this configuration mechanism isn't atomic, and
>>> there's some possible race condition for related attributes to have
>>> inconsistent values seen from another thread while the configuration is
>>> being changed.
>>
>> This isn't a new problem, right?
>
> It _would_ be a new problem if you refactored an existing
> configuration form to use the recommended idioms and thereby
> introduced the race condition.
No, this race condition already exists while one submit configuration form,
whenever we use @databound or not, other threads _already_ can see
descriptor in a weird intermediate state. Hopefully one does not update
global configuration every millisecond.
2017-11-15 15:44 GMT+01:00 Jesse Glick <[email protected]>:
> On Tue, Nov 14, 2017 at 10:19 PM, Kohsuke Kawaguchi <[email protected]>
> wrote:
> > I suppose we could create a variant of req.bindJSON(this,json) that does
> set
> > null on proeprties that are not specified in JSON.
>
> Yes, that would be a helpful convenience API, and it would make sense
> to call it in a newly introduced default implementation for
> `Descriptor.configure`. For compatibility reasons we could probably
> not change the implementation of `GlobalConfiguration.configure`,
> unfortunately.
>
> >> this configuration mechanism isn't atomic, and
> >> there's some possible race condition for related attributes to have
> >> inconsistent values seen from another thread while the configuration is
> >> being changed.
> >
> > This isn't a new problem, right?
>
> It _would_ be a new problem if you refactored an existing
> configuration form to use the recommended idioms and thereby
> introduced the race condition.
>
> > When this matters some use of
> > 'synchronized' statement is in order anyway.
>
> Would not help. You can synchronize `configure(StaplerRequest,
> JSONObject)` so that the nulling out of all properties followed by the
> setting of most properties is atomic. But that will not protect
> individual `@DataBoundSetter` calls from the proposed C-a-C plugin. We
> would need to introduce a scripting-friendly API for configuring an
> arbitrary object (create new `Describable` or modify existing
> `Descriptor` or `ReconfigurableDescribable`), but without any tie to
> Stapler or web forms, along the lines of
>
> http://javadoc.jenkins.io/workflow-step-api/org/
> jenkinsci/plugins/workflow/steps/StepDescriptor.html#
> newInstance-java.util.Map-
>
> This could live in `structs` though we would then have a problem with
> any affected settings defined in `jenkins-core`.
>
> The more straightforward workaround for such cases is the use of an
> explicit object representing a zero choice, as I mentioned previously.
> This does have an effect on the UI, at least using the currently
> available controls (which would only support a dropdown or a radio
> button list, not a checkbox).
>
> --
> You received this message because you are subscribed to the Google Groups
> "Jenkins Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/
> msgid/jenkinsci-dev/CANfRfr0fLxzZ_4JtTsedFY_-22wdLCJGBuR1e%3DOcKCdD9Zrz%
> 3DA%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
--
You received this message because you are subscribed to the Google Groups
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jenkinsci-dev/CANMVJzn0g2StRCR%2B5Q5Vb_ikORphSmM8Xzu%2B-%3DCf6L-5oXxohA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
