[JBoss JIRA] (CDI-721) configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
by Mark Struberg (JIRA)
[ https://issues.jboss.org/browse/CDI-721?page=com.atlassian.jira.plugin.sy... ]
Mark Struberg commented on CDI-721:
-----------------------------------
Btw, just to complete the picture:
it works the same if someone _first_ uses setAnnotatedType() and only later uses configureAnnotatedType.
In that case configureAnnotatedType will clear the fixed AT and create a Configurator based on this AT.
So you don' loose any information, regardless how you mix them up.
> configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
> ----------------------------------------------------------------------------
>
> Key: CDI-721
> URL: https://issues.jboss.org/browse/CDI-721
> Project: CDI Specification Issues
> Issue Type: Bug
> Components: Portable Extensions
> Affects Versions: 2.0 .Final
> Reporter: Mark Struberg
>
> {noformat}
> Any observer of this event is permitted to wrap and/or replace the AnnotatedType by calling either setAnnotatedType() or configureAnnotatedType(). If both methods are called within an observer notification an IllegalStateException is thrown.
> {noformat}
> This rule is way too strict without any real reason.
> Any CDI container must support that both methods are being called on the same event payload anyway. Because we did not forbid that observerMethod1 invokes setAnnotatedType and observerMethod2 uses configureAnnoatedType. And that's good that way, otherwise the pluggability would be lost.
> We should delete this sentence without any substitution.
> The same applies to similar configurator methods like configureBeanAttributes, etc.
--
This message was sent by Atlassian JIRA
(v7.5.0#75005)
5 years, 4 months
[JBoss JIRA] (CDI-721) configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
by Romain Manni-Bucau (JIRA)
[ https://issues.jboss.org/browse/CDI-721?page=com.atlassian.jira.plugin.sy... ]
Romain Manni-Bucau commented on CDI-721:
----------------------------------------
[~mkouba] well it is close enough of a builder pattern - in particular with its fluent API - to be perceived as such. We only miss getAnnotatedType() to be the build() method somehow. Also it is not that countra productive since it is just a clarification and not a change in the API as Mark explained. Said otherwise, it can already be in use.
> configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
> ----------------------------------------------------------------------------
>
> Key: CDI-721
> URL: https://issues.jboss.org/browse/CDI-721
> Project: CDI Specification Issues
> Issue Type: Bug
> Components: Portable Extensions
> Affects Versions: 2.0 .Final
> Reporter: Mark Struberg
>
> {noformat}
> Any observer of this event is permitted to wrap and/or replace the AnnotatedType by calling either setAnnotatedType() or configureAnnotatedType(). If both methods are called within an observer notification an IllegalStateException is thrown.
> {noformat}
> This rule is way too strict without any real reason.
> Any CDI container must support that both methods are being called on the same event payload anyway. Because we did not forbid that observerMethod1 invokes setAnnotatedType and observerMethod2 uses configureAnnoatedType. And that's good that way, otherwise the pluggability would be lost.
> We should delete this sentence without any substitution.
> The same applies to similar configurator methods like configureBeanAttributes, etc.
--
This message was sent by Atlassian JIRA
(v7.5.0#75005)
5 years, 4 months
[JBoss JIRA] (CDI-721) configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
by Martin Kouba (JIRA)
[ https://issues.jboss.org/browse/CDI-721?page=com.atlassian.jira.plugin.sy... ]
Martin Kouba commented on CDI-721:
----------------------------------
[~struberg]
bq. ...we already have it: {{getAnnotatedType()}}
This is not a "terminal" method. You don't even need to call it at all. See also the example in my previous comment - what happens with the configurator reference after I call {{setAnnotatedType()}}?
bq. What if some extension calls setAnnotatedType multiple times?
Yes, this is how it is defined.
bq. But if the Extension is well built ...
We cannot rely on this.
bq. Please chedk how we did it in OWB...
I will do.
[~rmannibucau]
bq. This is like a builder pattern based on immutability...
It's actually not a builder, that's why we call it configurators...
bq. It is beneficial for developers...
This is disputable. If we don't cover all possible corner cases properly it could be even contra productive.
> configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
> ----------------------------------------------------------------------------
>
> Key: CDI-721
> URL: https://issues.jboss.org/browse/CDI-721
> Project: CDI Specification Issues
> Issue Type: Bug
> Components: Portable Extensions
> Affects Versions: 2.0 .Final
> Reporter: Mark Struberg
>
> {noformat}
> Any observer of this event is permitted to wrap and/or replace the AnnotatedType by calling either setAnnotatedType() or configureAnnotatedType(). If both methods are called within an observer notification an IllegalStateException is thrown.
> {noformat}
> This rule is way too strict without any real reason.
> Any CDI container must support that both methods are being called on the same event payload anyway. Because we did not forbid that observerMethod1 invokes setAnnotatedType and observerMethod2 uses configureAnnoatedType. And that's good that way, otherwise the pluggability would be lost.
> We should delete this sentence without any substitution.
> The same applies to similar configurator methods like configureBeanAttributes, etc.
--
This message was sent by Atlassian JIRA
(v7.5.0#75005)
5 years, 4 months
[JBoss JIRA] (CDI-721) configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
by Romain Manni-Bucau (JIRA)
[ https://issues.jboss.org/browse/CDI-721?page=com.atlassian.jira.plugin.sy... ]
Romain Manni-Bucau commented on CDI-721:
----------------------------------------
Hi
kind of agree with Mark. This is like a builder pattern based on immutability (like in JSON-P). It works well and nothing technically justifies to prevent it.
It is beneficial for developers and users so we should support it IMHO.
Romain
> configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
> ----------------------------------------------------------------------------
>
> Key: CDI-721
> URL: https://issues.jboss.org/browse/CDI-721
> Project: CDI Specification Issues
> Issue Type: Bug
> Components: Portable Extensions
> Affects Versions: 2.0 .Final
> Reporter: Mark Struberg
>
> {noformat}
> Any observer of this event is permitted to wrap and/or replace the AnnotatedType by calling either setAnnotatedType() or configureAnnotatedType(). If both methods are called within an observer notification an IllegalStateException is thrown.
> {noformat}
> This rule is way too strict without any real reason.
> Any CDI container must support that both methods are being called on the same event payload anyway. Because we did not forbid that observerMethod1 invokes setAnnotatedType and observerMethod2 uses configureAnnoatedType. And that's good that way, otherwise the pluggability would be lost.
> We should delete this sentence without any substitution.
> The same applies to similar configurator methods like configureBeanAttributes, etc.
--
This message was sent by Atlassian JIRA
(v7.5.0#75005)
5 years, 4 months
[JBoss JIRA] (CDI-721) configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
by Mark Struberg (JIRA)
[ https://issues.jboss.org/browse/CDI-721?page=com.atlassian.jira.plugin.sy... ]
Mark Struberg commented on CDI-721:
-----------------------------------
At first I also thought that we would need such a terminal() method.
But after further thinking about it my conclusion is that we do *not*!
Or better we already have it: getAnnotatedType().
What if some extension calls setAnnotatedType multiple times?
The last one wins!
But if the Extension is well built (and not just random nonsense) then it will first get the previous AT via getAnnotatedType(). Then modify this information and use it for setAnnotatedType.
Please chedk how we did it in OWB:
https://github.com/apache/openwebbeans/blob/trunk/webbeans-impl/src/main/...
> configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
> ----------------------------------------------------------------------------
>
> Key: CDI-721
> URL: https://issues.jboss.org/browse/CDI-721
> Project: CDI Specification Issues
> Issue Type: Bug
> Components: Portable Extensions
> Affects Versions: 2.0 .Final
> Reporter: Mark Struberg
>
> {noformat}
> Any observer of this event is permitted to wrap and/or replace the AnnotatedType by calling either setAnnotatedType() or configureAnnotatedType(). If both methods are called within an observer notification an IllegalStateException is thrown.
> {noformat}
> This rule is way too strict without any real reason.
> Any CDI container must support that both methods are being called on the same event payload anyway. Because we did not forbid that observerMethod1 invokes setAnnotatedType and observerMethod2 uses configureAnnoatedType. And that's good that way, otherwise the pluggability would be lost.
> We should delete this sentence without any substitution.
> The same applies to similar configurator methods like configureBeanAttributes, etc.
--
This message was sent by Atlassian JIRA
(v7.5.0#75005)
5 years, 4 months
[JBoss JIRA] (CDI-721) configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
by Martin Kouba (JIRA)
[ https://issues.jboss.org/browse/CDI-721?page=com.atlassian.jira.plugin.sy... ]
Martin Kouba commented on CDI-721:
----------------------------------
Mark, I understand that you found a good use case where the current approach does not work. But configurators were not meant to replace {{setAnnotatedType()}} and to cover all possible use cases. A configurator instance is currently *scoped to a container lifecycle observer method invocation* and if we change this in CDI 2.x I think we would break backward compatibility. Also we would have to add some "terminal" operation to the configurator API so that it would be clear when a configurator result should be used to replace the original AT:
{code:java}
void observe(@Observes ProcessAnnotatedType<Foo> event) {
AnnotatedTypeConfigurator<Foo> configurator = event.configureAnnotatedType().add(RequestScoped.Literal.INSTANCE);
// At this time we cannot "terminate" the configurator!
event.setAnnotatedType(...);
configurator.methods().forEach(m -> m.add(Whatever.Literal.INSTANCE));
}
{code}
> configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
> ----------------------------------------------------------------------------
>
> Key: CDI-721
> URL: https://issues.jboss.org/browse/CDI-721
> Project: CDI Specification Issues
> Issue Type: Bug
> Components: Portable Extensions
> Affects Versions: 2.0 .Final
> Reporter: Mark Struberg
>
> {noformat}
> Any observer of this event is permitted to wrap and/or replace the AnnotatedType by calling either setAnnotatedType() or configureAnnotatedType(). If both methods are called within an observer notification an IllegalStateException is thrown.
> {noformat}
> This rule is way too strict without any real reason.
> Any CDI container must support that both methods are being called on the same event payload anyway. Because we did not forbid that observerMethod1 invokes setAnnotatedType and observerMethod2 uses configureAnnoatedType. And that's good that way, otherwise the pluggability would be lost.
> We should delete this sentence without any substitution.
> The same applies to similar configurator methods like configureBeanAttributes, etc.
--
This message was sent by Atlassian JIRA
(v7.5.0#75005)
5 years, 4 months
[JBoss JIRA] (CDI-721) configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
by Mark Struberg (JIRA)
[ https://issues.jboss.org/browse/CDI-721?page=com.atlassian.jira.plugin.sy... ]
Mark Struberg commented on CDI-721:
-----------------------------------
So it seems we all do agree that this restriction is technically not needed, right?
And here is why I came across it:
Customer wanted to extend his old code (using getAnnoatedType() + wrapping it + setAnnotatedType(wrappedAT)).
Of course he want's to leverage CDI-2.0 configureAnnotatedType. But for performance reasons and ease of reproducibility (observer ordering) they only have 1 PAT observer where they simply call the old method and the new method.
It's not always nonsense to mix those 2 approaches. Just because we didn't think about it doesn't mean we should forbid it!
> configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
> ----------------------------------------------------------------------------
>
> Key: CDI-721
> URL: https://issues.jboss.org/browse/CDI-721
> Project: CDI Specification Issues
> Issue Type: Bug
> Components: Portable Extensions
> Affects Versions: 2.0 .Final
> Reporter: Mark Struberg
>
> {noformat}
> Any observer of this event is permitted to wrap and/or replace the AnnotatedType by calling either setAnnotatedType() or configureAnnotatedType(). If both methods are called within an observer notification an IllegalStateException is thrown.
> {noformat}
> This rule is way too strict without any real reason.
> Any CDI container must support that both methods are being called on the same event payload anyway. Because we did not forbid that observerMethod1 invokes setAnnotatedType and observerMethod2 uses configureAnnoatedType. And that's good that way, otherwise the pluggability would be lost.
> We should delete this sentence without any substitution.
> The same applies to similar configurator methods like configureBeanAttributes, etc.
--
This message was sent by Atlassian JIRA
(v7.5.0#75005)
5 years, 4 months
[JBoss JIRA] (CDI-721) configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
by Matej Novotny (JIRA)
[ https://issues.jboss.org/browse/CDI-721?page=com.atlassian.jira.plugin.sy... ]
Matej Novotny commented on CDI-721:
-----------------------------------
To underline it, this is about invoking the {{set}} and {{configure}} methods _within one observer invocation_.
Across multiple methods (be it in one or multiple extensions) you can do as you please even now.
To me this restriction seems OK and I think we should not remove it. While the reason behind it might not be technical, it enforces clearer code.
There is no reason known to me where you (*within one method*) need to start configuring the object (say AT for now) and then you suddenly change you mind and call the {{setAT}} method and replace it altogether. That means the previous configuration was a waste of time and resources because you just tossed it away. Not to mention it is easy to refactor your code not to invoke both methods and first check what is it you want to do with that given AT.
Therefore most people who write 'sane code' would not even bump into this and the rest will at least realize they are doing some extraneous work.
We have yet to see a case where this restriction would block user from achieving something.
> configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
> ----------------------------------------------------------------------------
>
> Key: CDI-721
> URL: https://issues.jboss.org/browse/CDI-721
> Project: CDI Specification Issues
> Issue Type: Bug
> Components: Portable Extensions
> Affects Versions: 2.0 .Final
> Reporter: Mark Struberg
>
> {noformat}
> Any observer of this event is permitted to wrap and/or replace the AnnotatedType by calling either setAnnotatedType() or configureAnnotatedType(). If both methods are called within an observer notification an IllegalStateException is thrown.
> {noformat}
> This rule is way too strict without any real reason.
> Any CDI container must support that both methods are being called on the same event payload anyway. Because we did not forbid that observerMethod1 invokes setAnnotatedType and observerMethod2 uses configureAnnoatedType. And that's good that way, otherwise the pluggability would be lost.
> We should delete this sentence without any substitution.
> The same applies to similar configurator methods like configureBeanAttributes, etc.
--
This message was sent by Atlassian JIRA
(v7.5.0#75005)
5 years, 4 months
[JBoss JIRA] (CDI-721) configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
by Mark Struberg (JIRA)
[ https://issues.jboss.org/browse/CDI-721?page=com.atlassian.jira.plugin.sy... ]
Mark Struberg commented on CDI-721:
-----------------------------------
The AT configurator will build the AT when the AT is needed the next time.
That might be at the end of _all_ events get invoked (when the AT is further processed by the CDI runtime) or when getAnnotatedType() is invoked on the pat the next time.
Technically it doesn't make any difference whether this happens in the next observer method or in the same one.
The CDI container, or resp the ProcessAnnotatedType implementation, has to deal with it anyway.
So this restriction technically makes no sense.
> configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
> ----------------------------------------------------------------------------
>
> Key: CDI-721
> URL: https://issues.jboss.org/browse/CDI-721
> Project: CDI Specification Issues
> Issue Type: Bug
> Components: Portable Extensions
> Affects Versions: 2.0 .Final
> Reporter: Mark Struberg
>
> {noformat}
> Any observer of this event is permitted to wrap and/or replace the AnnotatedType by calling either setAnnotatedType() or configureAnnotatedType(). If both methods are called within an observer notification an IllegalStateException is thrown.
> {noformat}
> This rule is way too strict without any real reason.
> Any CDI container must support that both methods are being called on the same event payload anyway. Because we did not forbid that observerMethod1 invokes setAnnotatedType and observerMethod2 uses configureAnnoatedType. And that's good that way, otherwise the pluggability would be lost.
> We should delete this sentence without any substitution.
> The same applies to similar configurator methods like configureBeanAttributes, etc.
--
This message was sent by Atlassian JIRA
(v7.5.0#75005)
5 years, 4 months
[JBoss JIRA] (CDI-721) configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
by Martin Kouba (JIRA)
[ https://issues.jboss.org/browse/CDI-721?page=com.atlassian.jira.plugin.sy... ]
Martin Kouba commented on CDI-721:
----------------------------------
Well, I don't think we should remove this restriction. The reason was to simplify the contract and avoid possible confusion (see also CDI-596). Note that the result of configurator should replace the original one at the end of the observer invocation (there is no terminal operation in configurator API).
E.g. in this example:
{code:java}
void observePat(@Observes ProcessAnnotatedType<Foo> event) {
event.configureAnnotatedType().add(RequestScoped.Literal.INSTANCE);
event.setAnnotatedType(new FooAnnotatedType());
}
{code}
The configurator must be discarded completely. So we would have to change the wording of {{ProcessAnnotatedType.configureAnnotatedType()}} too. Something like _"... configurator AnnotatedType will replace the original one at the end of the observer invocation or when setAnnotatedType() is called..."_.
Anyway, I don't see a benefit in allowing both methods to be called within a single observer method invocation.
> configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
> ----------------------------------------------------------------------------
>
> Key: CDI-721
> URL: https://issues.jboss.org/browse/CDI-721
> Project: CDI Specification Issues
> Issue Type: Bug
> Components: Portable Extensions
> Affects Versions: 2.0 .Final
> Reporter: Mark Struberg
>
> {noformat}
> Any observer of this event is permitted to wrap and/or replace the AnnotatedType by calling either setAnnotatedType() or configureAnnotatedType(). If both methods are called within an observer notification an IllegalStateException is thrown.
> {noformat}
> This rule is way too strict without any real reason.
> Any CDI container must support that both methods are being called on the same event payload anyway. Because we did not forbid that observerMethod1 invokes setAnnotatedType and observerMethod2 uses configureAnnoatedType. And that's good that way, otherwise the pluggability would be lost.
> We should delete this sentence without any substitution.
> The same applies to similar configurator methods like configureBeanAttributes, etc.
--
This message was sent by Atlassian JIRA
(v7.5.0#75005)
5 years, 4 months