Hi, I'm looking for feedback on my issue - https://issues.redhat.com/browse/WFCORE-5339, analysis document at https://github.com/wildfly/wildfly-proposals/pull/423 Thanks, Michal _______________________________________________ wildfly-dev mailing list -- wildfly-dev(a)lists.jboss.org To unsubscribe send an email to wildfly-dev-leave(a)lists.jboss.org %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

It's not, the attribute definition method would still have to be changed. But it would no longer be possible to set the attribute value to an empty string using this expression, isn't that a problem? On Tue, Sep 7, 2021 at 6:55 PM David Lloyd <david.lloyd(a)redhat.com&gt; wrote: > I think the proposed functionality should already mostly be present, as > you can supply an empty default value. For example "${foo.bar:}" will not > throw an exception if `foo.bar` is missing but rather return an empty > string. Is an empty string sufficient to yield the default value of an > attribute? > > On Tue, Sep 7, 2021 at 11:32 AM Michal Petrov <mpetrov(a)redhat.com&gt; wrote: > >> Hi, >> >> I'm looking for feedback on my issue - >> https://issues.redhat.com/browse/WFCORE-5339, analysis document at >> https://github.com/wildfly/wildfly-proposals/pull/423 >> >> Thanks, >> Michal >> _______________________________________________ >> wildfly-dev mailing list -- wildfly-dev(a)lists.jboss.org >> To unsubscribe send an email to wildfly-dev-leave(a)lists.jboss.org >> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s > > > > -- > - DML • he/him >

Well I guess we can either have a custom syntax for the optional expression or a custom syntax (reserved string) for the default value, I went with an option that is more concise. On Thu, Sep 9, 2021 at 2:50 PM David Lloyd <david.lloyd(a)redhat.com&gt; wrote: > Yes that would be a problem. Mainly I'm trying to think of an > alternative solution that doesn't involve a custom syntax though. Maybe a > special variable called "default" could be used e.g. > `${foo.bar:${default}}`? Is that too convoluted? > > On Thu, Sep 9, 2021 at 6:23 AM Michal Petrov <mpetrov(a)redhat.com&gt; wrote: > >> It's not, the attribute definition method would still have to be >> changed. But it would no longer be possible to set the attribute value to >> an empty string using this expression, isn't that a problem? >> >> On Tue, Sep 7, 2021 at 6:55 PM David Lloyd <david.lloyd(a)redhat.com&gt; >> wrote: >> >>> I think the proposed functionality should already mostly be present, as >>> you can supply an empty default value. For example "${foo.bar:}" will not >>> throw an exception if `foo.bar` is missing but rather return an empty >>> string. Is an empty string sufficient to yield the default value of an >>> attribute? >>> >>> On Tue, Sep 7, 2021 at 11:32 AM Michal Petrov <mpetrov(a)redhat.com&gt; >>> wrote: >>> >>>> Hi, >>>> >>>> I'm looking for feedback on my issue - >>>> https://issues.redhat.com/browse/WFCORE-5339, analysis document at >>>> https://github.com/wildfly/wildfly-proposals/pull/423 >>>> >>>> Thanks, >>>> Michal >>>> _______________________________________________ >>>> wildfly-dev mailing list -- wildfly-dev(a)lists.jboss.org >>>> To unsubscribe send an email to wildfly-dev-leave(a)lists.jboss.org >>>> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s >>> >>> >>> >>> -- >>> - DML • he/him >>> >> > > -- > - DML • he/him >

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.

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

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

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.

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

a shorthand for "${foo.bar:$default}"? - should we be concerned with edge cases like "${foo.bar:$def}ault"?

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 >

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"?

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

Sorry for the late reply.

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.

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.

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 >

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 >