[wildfly-dev] Re: Optional expressions RFE, looking for feedback

I don't think we've reached a consensus yet. We're adding a reserved word "$default", if an expression resolves to exactly this string then AttributeDefinition will try to supply the default value. In case a default value doesn't exist then "${foo}" and "${foo:$default}" are functionally the same expression. Optionally I want to add the "?" shorthand, if an expression string ends with "?}" then that will be replaced with ":$default}" before the expression gets resolved. Any comments? As for testing, should I write a new test case? I don't see a test for the "resolve_" methods of AttributeDefinition. On Thu, Oct 21, 2021 at 2:29 AM Brian Stansberry < brian.stansberry(a)redhat.com&gt; wrote: > On Wed, Oct 20, 2021 at 10:37 AM Michal Petrov <mpetrov(a)redhat.com&gt; > wrote: > >> Sorry for the late reply. >> > > No problem; my average response time on this thread is really long! > >> >> I was going along with David's suggestion allowing the magic value in >> nested expressions. If that's not the plan then there are no issues since >> that was the original intent. >> > > I may have gotten lost in the back and forth, or missed something, or > just misunderstood, and maybe still am. :) > > Anyway, what I outlined in my last post is how I think this needs to > work. > > >> My remaining question is if "?" is a viable shorthand for ":$default". >> It could be expanded inside AttributeDefinition.resolveValue so there would >> be no need to modify ExpressionResolver. >> > > I don't think the AD should be trying to interpret the input text; that's > the work of the ExpressionResolver. > > >> On Thu, Sep 30, 2021 at 9:05 PM Brian Stansberry < >> brian.stansberry(a)redhat.com&gt; wrote: >> >>> >>> >>> On Thu, Sep 30, 2021 at 1:17 PM Michal Petrov <mpetrov(a)redhat.com&gt; >>> wrote: >>> >>>> On Thu, Sep 30, 2021 at 5:41 PM Brian Stansberry < >>>> brian.stansberry(a)redhat.com&gt; wrote: >>>> >>>>> >>>>> >>>>> On Thu, Sep 30, 2021 at 10:05 AM Michal Petrov <mpetrov(a)redhat.com&gt; >>>>> wrote: >>>>> >>>>>> Well the default would still be replaced with something different >>>>>> depending on the attribute, so to me the contract stops being simple. >>>>>> >>>>>> That said I'm ok to go with "$default", the expression resolver >>>>>> would treat it as any other string so there's no issue there. >>>>>> Then I still have some questions: >>>>>> - what to do if the attribute has no default value, we can treat it >>>>>> as "undefined" but as Brian mentioned this should then fail in a nested >>>>>> expression; or we could treat it as empty string but that doesn't seem ok >>>>>> either >>>>>> >>>>> >>>>> I don't think empty string is an option. >>>>> >>>>> Either it fails or the parsing doesn't treat the $default as a magic >>>>> value and hands back the resulting string to the caller. So >>>>> "${foo.bar:$default}1" results in "$default1" being passed back. The >>>>> contract between the caller and the parser doesn't see that as a magic >>>>> value so the caller handles it like any other string. >>>>> >>>>> Thinking about it more that sounds better, and I think aligns with >>>>> what David was saying in his response to me when I posted about failing and >>>>> nulls and etc. >>>>> >>>> >>>> That's what I had in mind, we wouldn't need to change the parser and >>>> $default would be processed as any other string. Then the attribute >>>> definition would simply replace any occurence of $default with the default >>>> value. But what do we do if there's no default value? If it means >>>> $default=null do we throw an error with "$default1"? >>>> >>> >>> I'm going to speak in concrete code terms here to help me be clear to >>> myself in what I'm thinking. >>> >>> AttributeDefinition.resolveValue is what does this. And it calls >>> ExpressionResolver.resolveExpressions which is what we're talking >>> about when we talk about 'parsing'. >>> >>> If that call returns a ModelNode whose value is "$default1" then the >>> code just carries on. That node gets passed to validator.validateParameter >>> and it either validates or an OFE is thrown. >>> >>> If ExpressionResolver.resolveExpressions returns ModeType.STRING node >>> whose value is "$default" *and* the initial value it was asked to resolve >>> was not "$default" then we know there was some resolution that happened and >>> we got back the magic value. So the fact the value changed demonstrates the >>> user's intent to use this feature. In that case if there's a default value >>> we use that, otherwise the user's intent can't be satisfied and we throw >>> OFE. >>> >>> >>>> >>>>> - if we have that worked out, could we then not introduce >>>>>> "${foo.bar?}" as a shorthand for "${foo.bar:$default}"? >>>>>> - should we be concerned with edge cases like "${foo.bar:$def}ault"? >>>>>> >>>>> >>>>> I suspect having that just work would be easier than having it not >>>>> work, and is more consistent. >>>>> >>>>> In the end the parsing stuff comes up with a string and if that >>>>> string is the magic value, then the caller supplies the default or fails if >>>>> there isn't one. So "${foo.bar:$def}ault" would result in "$default" >>>>> which is the request for the default value. >>>>> >>>>> >>>>>> On Tue, Sep 21, 2021 at 3:10 PM David Lloyd <david.lloyd(a)redhat.com&gt; >>>>>> wrote: >>>>>> >>>>>>> On Fri, Sep 17, 2021 at 3:25 PM Brian Stansberry < >>>>>>> brian.stansberry(a)redhat.com&gt; wrote: >>>>>>> >>>>>>>> On Fri, Sep 17, 2021 at 1:31 PM David Lloyd < >>>>>>>> david.lloyd(a)redhat.com&gt; wrote: >>>>>>>> >>>>>>>>> On Fri, Sep 17, 2021 at 12:28 PM Michal Petrov < >>>>>>>>> mpetrov(a)redhat.com&gt; wrote: >>>>>>>>> >>>>>>>>>> Okay, that makes sense except for this bit: >>>>>>>>>> >>>>>>>>>> > Especially strange would be if I had an expression like >>>>>>>>>> "${foo.bar?}baz" and the default value for the property gets put in; >>>>>>>>>> >>>>>>>>>> For one this wouldn't work and I don't think we want it to work, >>>>>>>>>> the default value will be put in only if the attribute resolution is empty >>>>>>>>>> (or if it's the reserved string if we're going with that), so "${foo.bar?}baz" >>>>>>>>>> would resolve to "baz". We're not looking for a way to "inject" the default >>>>>>>>>> value, we want the attribute definition to use its default value because >>>>>>>>>> the user didn't provide anything (as far as the definition is concerned). >>>>>>>>>> As such the optional expression doesn't make sense in any kind of nested >>>>>>>>>> context. >>>>>>>>>> >>>>>>>>> >>>>>>>>> Definitely understood. The use case seems pretty clear: as a >>>>>>>>> user, I need to be able to explicitly specify the default value of a >>>>>>>>> property in the event that another property cannot be expanded. >>>>>>>>> >>>>>>>> >>>>>>>> I think this is what you meant but just in case not.. its "I need >>>>>>>> to be able to explicitly specify the default value of a property *should be >>>>>>>> used* in the event that another property cannot be expanded." For the case >>>>>>>> where the user wants to specify a fallback value, that's the existing >>>>>>>> ${foo.bar:baz}. >>>>>>>> >>>>>>> >>>>>>> Yes, that is what I meant. >>>>>>> >>>>>>> >>>>>>>> >>>>>>>>> However, the proposed implementation *is* defined to use the >>>>>>>>> existing string expression interpolation engine. The contract for the >>>>>>>>> engine is clear: wherever there is an expression, that expression is >>>>>>>>> lexicographically replaced with the exact replacement string. By >>>>>>>>> stipulating a new modified contract based on the use case (specifically, >>>>>>>>> that certain expressions must stand alone), you're introducing a new and >>>>>>>>> arguably unexpected behavior to the existing contract, which brings >>>>>>>>> complications in terms of usability (principle of least surprise for >>>>>>>>> example) as well as implementation (an extra check based on the kind of >>>>>>>>> expression found). >>>>>>>>> >>>>>>>> >>>>>>>> I'll need to check whether WildFly expression resolution results >>>>>>>> in ${foo:} returning an empty string. I thought it didn't but may be wrong. >>>>>>>> If so the '?' approach can be logically seen as saying resolve to 'null', >>>>>>>> not empty string. And then ${foo?}baz resolves to a conceptual >>>>>>>> null.append(baz) i.e. a failure. Any nesting that must use the 'null', e,g >>>>>>>> ${foo?:$baz}} must fail. >>>>>>>> >>>>>>> >>>>>>> That might be an internally consistent rationalization, but does it >>>>>>> really make sense to the user? The user contract of the expression resolver >>>>>>> definitely does not include null values; it replaces a string expression >>>>>>> with another string or else gives a validation error for the particular >>>>>>> expression. >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> The caller of the expression resolution (the AttributeDefinition) >>>>>>>> can deal with the 'null' return, providing the default if one exists, or >>>>>>>> failing. (Or perhaps not setting the attribute to undefined. The situation >>>>>>>> is ambiguous.) >>>>>>>> >>>>>>>> That still feels off to me, but at least I can see how it results >>>>>>>> in an understandable semantic. >>>>>>>> >>>>>>>> I imagine you've had lots of debates in MP Config around these >>>>>>>> null vs empty questions, so I'm prepared to schooled. ;) >>>>>>>> >>>>>>>>> >>>>>>>>> On the other hand, since the contract of the interpolation engine >>>>>>>>> *is* already existent, if the expression engine is used for this purpose in >>>>>>>>> an idiomatic manner (i.e. a special expression argument, which is directly >>>>>>>>> supported already by the expression API), then the user can easily >>>>>>>>> extrapolate that the default value expression is just another expression >>>>>>>>> and can be used in the exact manner of any other expression, and as a >>>>>>>>> result the user has slightly more flexibility than what was originally >>>>>>>>> required (which is not necessarily a bad thing, as long as the implemented >>>>>>>>> functionality can be concisely described in documentation). >>>>>>>>> >>>>>>>> >>>>>>>> I'm not clear on what you're suggesting here. >>>>>>>> >>>>>>> >>>>>>> What I mean is, we have a simple contract for expressions that the >>>>>>> user can understand: _this_ thing is replaced with _that_ thing. Is it so >>>>>>> bad that they would gain the ability to decorate the default value string, >>>>>>> if we used a sentinel variable name like "default"? >>>>>>> >>>>>>> -- >>>>>>> - DML • he/him >>>>>>> >>>>>> >>>>> >>>>> -- >>>>> Brian Stansberry >>>>> Principal Architect, Red Hat JBoss EAP >>>>> He/Him/His >>>>> >>>> >>> >>> -- >>> Brian Stansberry >>> Principal Architect, Red Hat JBoss EAP >>> He/Him/His >>> >> > > -- > Brian Stansberry > Principal Architect, Red Hat JBoss EAP > He/Him/His >