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> wrote:
On Wed, Oct 20, 2021 at 10:37 AM Michal Petrov
<mpetrov(a)redhat.com> 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> wrote:
>
>>
>>
>> On Thu, Sep 30, 2021 at 1:17 PM Michal Petrov <mpetrov(a)redhat.com>
>> wrote:
>>
>>> On Thu, Sep 30, 2021 at 5:41 PM Brian Stansberry <
>>> brian.stansberry(a)redhat.com> wrote:
>>>
>>>>
>>>>
>>>> On Thu, Sep 30, 2021 at 10:05 AM Michal Petrov
<mpetrov(a)redhat.com>
>>>> 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>
>>>>> wrote:
>>>>>
>>>>>> On Fri, Sep 17, 2021 at 3:25 PM Brian Stansberry <
>>>>>> brian.stansberry(a)redhat.com> wrote:
>>>>>>
>>>>>>> On Fri, Sep 17, 2021 at 1:31 PM David Lloyd
<david.lloyd(a)redhat.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> On Fri, Sep 17, 2021 at 12:28 PM Michal Petrov
<mpetrov(a)redhat.com>
>>>>>>>> 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