[wildfly-dev] Fixing handling of 'required' attributes with 'alternatives'

Wed Nov 23 03:52:49 EST 2016

Thanks for the info, Brian. I'll check the console with the new server-side
impl (should affect the transaction attributes only, but will also do some
more testing)

On Wed, Nov 23, 2016 at 12:50 AM, Brian Stansberry <
brian.stansberry at redhat.com> wrote:

> Hi Harald,
>
> This PR is merged so this will be there once this week’s core release is
> merged into full (probably by Monday).
>
> > On Sep 7, 2016, at 3:59 PM, Brian Stansberry <
> brian.stansberry at redhat.com> wrote:
> >
> > PR for this: https://github.com/wildfly/wildfly-core/pull/1781
> >
> >> On Sep 6, 2016, at 1:56 PM, Brian Stansberry <
> brian.stansberry at redhat.com> wrote:
> >>
> >>>
> >>> On Sep 6, 2016, at 11:58 AM, Harald Pehl <hpehl at redhat.com> wrote:
> >>>
> >>> Thanks Brian for bringing this up. Everything which makes the metadata
> more consistent helps management client such as HAL. And as more and more
> forms are based on MBUI (generated based on the r-r-d metadata) this is
> even more important.
> >>>
> >>> Right now we already have some logic behind the 'nillable' and
> 'alternatives' attributes to decide whether an attribute is required or not:
> >>>
> >>> boolean required = attributeDescription.hasDefined("nillable") &&
> !attributeDescription.get("nillable").asBoolean();
> >>> if (attributeDescription.hasDefined("alternatives") &&
> !attributeDescription.get("alternatives").asList().isEmpty()) {
> >>>   required = false;
> >>> }
> >>>
> >>
> >> Ah, good. So my proposed change to how nillable is calculated shouldn’t
> change the final result you get above for your required variable. :) So
> once I do what I propose on the server side you can adapt to it when
> convenient.
> >>
> >>> In HAL we use attribute descriptions to add new resources and to
> modify existing resources. For the former we rely on the
> 'request-properties' node of the add operation description. The latter uses
> the 'attributes' node of the r-r-d op. If we want to make changes, it's
> important to be consistent and apply them to both nodes. Right now these
> two nodes already have slightly different attributes: The
> 'request-properties' already contain a 'required' attribute whereas the
> 'attributes' don't.
> >>
> >> Yes, this inconsistency is part of the overall task of WFCORE-1556. The
> actual metadata we generate doesn’t comply with the spec in [a] and [b] and
> then the spec itself is inconsistent between those two sections in how it
> deals with “required” vs “nillable”. And there’s no reason for that.
> >>
> >> [a] https://docs.jboss.org/author/display/WFLY/Admin+Guide#AdminGuide-
> DescriptionofanAttribute
> >> [b] https://docs.jboss.org/author/display/WFLY/Admin+Guide#AdminGuide-
> DescriptionofanOperationParameterorReturnValue
> >>
> >>>
> >>> The proposal makes sense to me and the impact on HAL should be
> minimal. Some questions I have:
> >>>
> >>> 1. Will the metadata contain both 'nillable' and 'required'? With
> 'required' being the leading attribute and 'nillable' being deprecated but
> still there for backwards compability?
> >>>
> >>
> >> It will contain both. I wouldn’t characterize either as leading or
> deprecated. Rather they have different functions:
> >>
> >> required — indicates the attribute/parameter represents something that
> must be configured in some way. But configuring one of the ‘alternatives’
> suffices.
> >> nillable — indicates that attribute/parameter may have an undefined
> value for some reason
> >>
> >> So nillable is useful to a client simply wanting to know whether it
> needs to deal with ‘undefined’. A more sophisticated client would use
> ‘required’ plus ‘alternatives’.
> >>
> >>> 2. Will your proposal also cover the 'request-properties' for the add
> operation?
> >>>
> >>
> >> Yes, they will be made consistent, with all attribute and parameter
> descriptions exposing the same metadata fields with the same conceptual
> meaning.
> >>
> >> In terms of the server side implementation, for almost all add
> operations, the same AttributeDefinition instance is used for generating
> both the attribute description and the add parameter description. And for
> all parameter descriptions we use the same AttributeDefinition classes that
> we use for resource attributes, so the behavior we put in the AD class will
> apply to both.
> >>
> >>>
> >>> On Tue, Sep 6, 2016 at 3:10 PM, Brian Stansberry <
> brian.stansberry at redhat.com> wrote:
> >>> tl;dr;
> >>>
> >>> It’s not an uncommon thing to have a management attribute A that is
> required; i.e. must be set, but also has an alternative attribute B, where
> setting B satisfies the requirement for A, making an undefined value for A
> legal. We’re not handling that correctly and I want to fix it.[1] But
> fixing it will require some coordination with the HAL console.
> >>>
> >>> Right now the way AttributeDefinition building works it’s not
> practical to declare that an attribute is required but only if no
> alternative is set. So instead attributes are declared as if they aren’t
> really required. This leads to less than helpful input validation, e.g. [2]
> and [3], since the attribute definition is imprecise.
> >>>
> >>> The change I’d like to make will alter the read-resource-description
> output for 4 attributes, changing the value of the ‘nillable’ description
> from ‘false’ to ‘true’ so I want to coordinate that with the console team.
> >>>
> >>> Long version
> >>>
> >>> Harald and Claudio you guys are the main audience here. :)
> >>>
> >>> For even longer version see description and comments on [1].
> >>>
> >>> In a nutshell, if devs set the ‘allowNull’ property on an
> AttributeDefinition to ‘true’, the r-r-d output for the attribute has
> “nillable” => true. But there is no way to say “allowNull but only if an
> alternative is set.” So people are setting “allowNull” to true even if the
> attribute should be set in the absence of alternatives. And HAL has no
> metadata available to it to tell users they *must* set one of the
> alternatives. So I want to:
> >>>
> >>> 1) Add a setRequired(boolean) method to the AD builders where the fact
> that it means “must be defined if no alternative is defined” is explicitly
> declared
> >>> 2) @Deprecate setAllowNull and point to setRequired
> >>> 3) Clarify the meaning of setAllowNull(true) as being the same as
> setRequired(false)
> >>> 4) Change the builder ‘allowNull’ constructor param name to “optional”
> and document its meaning as “allowing undefined values even in the absence
> of defined alternatives”.  I could call the param ‘notRequired’ which is
> clearer in meaning but odd.
> >>>
> >>> Then I will add a new ‘required’ metadata field to the r-r-d output
> and change the impl of the existing ’nillable’ metadata to a logical
> “!required || (alternatives != null && alternatives.length > 0)”
> >>>
> >>> This will result in a change in the ‘nillable’ value for 4 attributes
> in the WildFly model from ‘false’ to ’true':
> >>>
> >>> /subsystem=transactions PROCESS_ID_SOCKET_BINDING and PROCESS_ID_UUID
> >>> /core-service=management/security-realm=*/authenticaton=ldap
> USERNAME_FILTER and ADVANCED_FILTER
> >>>
> >>> The latter two are not exposed in the console so the issue really is
> the two transaction attributes.
> >>>
> >>> I haven’t checked other subsystems in things like Teiid or JDG, but if
> there are only 4 in all of WildFly I doubt there are many.
> >>>
> >>> Also, if people start using the new behavior to correct problems like
> [2] and [3] people may expect the console to understand and reflect the
> concept of “required but only if there is no alternative’.
> >>>
> >>> [1] https://issues.jboss.org/browse/WFCORE-1556
> >>> [2] https://issues.jboss.org/browse/WFLY-6608
> >>> [3] https://issues.jboss.org/browse/WFLY-6607
> >>>
> >>> --
> >>> Brian Stansberry
> >>> Manager, Senior Principal Software Engineer
> >>> JBoss by Red Hat
> >>>
> >>>
> >>>
> >>>
> >>> _______________________________________________
> >>> wildfly-dev mailing list
> >>> wildfly-dev at lists.jboss.org
> >>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
> >>>
> >>>
> >>>
> >>> --
> >>> ---
> >>> Harald Pehl
> >>> JBoss by Red Hat
> >>> http://hpehl.info
> >>
> >> --
> >> Brian Stansberry
> >> Manager, Senior Principal Software Engineer
> >> JBoss by Red Hat
> >>
> >>
> >>
> >>
> >> _______________________________________________
> >> wildfly-dev mailing list
> >> wildfly-dev at lists.jboss.org
> >> https://lists.jboss.org/mailman/listinfo/wildfly-dev
> >
> > --
> > Brian Stansberry
> > Manager, Senior Principal Software Engineer
> > JBoss by Red Hat
> >
> >
> >
> >
> > _______________________________________________
> > wildfly-dev mailing list
> > wildfly-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
> --
> Brian Stansberry
> Manager, Senior Principal Software Engineer
> JBoss by Red Hat
>
>
>
>

-- 
Harald Pehl
JBoss by Red Hat
http://hpehl.info
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/wildfly-dev/attachments/20161123/7702b9f0/attachment-0001.html 