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.
- 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