[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:
-----------------------------------
[~manovotn] In OWB we still return the same instance of the configurator. This is perfectly solvable.
And no, I didn't avoid to answer Martins question.
Whether a user manipulates the AnnotatedType via setAnnotatedType or configureAnnotatedType MUST make absolutely no difference.
By acknowledging that we can further safely assume that both ways should be interchangeably from a user pov.
As shown with the setAnnotatedType: whatever happens as last step will be the full and only truth. Any AT change inbetween - which is not being wrapped - will be ultimately lost. That's the same with setAnnotatedType and I see no way nor reason to change this for configureAnnotatedType.
Mathematically speaking: if B is an inclusive superset of A then we can widen our solution from A to B and ensure 100% backward compatibility.
> 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)
6 years, 11 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:
-----------------------------------
bq. ... "observer invocation" is highly ambiguous....
Not really unless you are looking for a way to bend it on purpose. And we also agreed on that wording and it's meaning back there and from what I recall, there were quite long discussions on the CDI-558 PR(s). None of those discussions was concerned about the ambiguity of this observer statement.
bq. And again: this is not a backward incompatible change!
And again I beg to differ. Especially since we already have a [TCK test|https://github.com/cdi-spec/cdi-tck/blob/master/impl/src/main/java/o...] verifying that this exact behaviour doesn't work. This test was also recently slightly reworked on your own request from which I judge you didn't have a problem with this behaviour. This is basically a 180° turn; going from _this must not work_ to _this can work just fine_ - this discussion probably should have happened a year and half ago.
Last but not least - Martin's question was neatly avoided, so I will just go ahead and repeat that:
(NOTE: it isn't the same as Mark's code below, this one is obtaining the configurator twice which is what disputes spec wording)
bq. 3. In the snippet below, what happens on the line with configurator.methods()?
{code}
void observe(@Observes ProcessAnnotatedType<Foo> event) {
AnnotatedTypeConfigurator<Foo> configurator = event.configureAnnotatedType().add(RequestScoped.Literal.INSTANCE);
event.setAnnotatedType(...);
configurator.methods().forEach(m -> m.add(Whatever.Literal.INSTANCE));
}
{code}
With current spec wording, namely the part that says that no matter how many times you obtain configurator it will always be the same - it breaks the compatibility yet again. Two things can happen; either you return configurator or the newly set AT (which violets current wording and hence backward compat) or you return the previous one overriding the AT yet again (which is bewildering).
> 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)
6 years, 11 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:
----------------------------------
bq. "observer invocation" is highly ambiguous. It might be interpreted as "ObserverMethod invocation" but also as "em.fire()"
[~struberg] I don't think so. "observer invocation" != "event.fire()", that would be "observer notification", at least from the spec POV - see also 10.5. Observer notification.
I insist that this is a backward incompatible change.
> 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)
6 years, 11 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:
-----------------------------------
[~antoinesabot-durand] the intention is to get this done for the next MR indeed!
And again: this is *not* a backward incompatible change!
[~mkouba] As noted before: "observer invocation" is highly ambiguous. It might be interpreted as "ObserverMethod invocation" but also as "em.fire()". And no, it's no change in behaviour as the internas are not visible from outside anyway
> 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)
6 years, 11 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:
----------------------------------
WRT snippet - so you're saying that newAT_2 is the final value and configurator.methods() part is simply ignored? That's imho pretty confusing.
bq. Can you please point me to the section where you did read this?
11.5.6. ProcessAnnotatedType event, _"configureAnnotatedType() returns an AnnotatedTypeConfigurator (as defined in AnnotatedTypeConfigurator SPI) initialized with the AnnotatedType processed by the event to easily configure the AnnotatedType *which will be used to replace the original one at the end of observer invocation*. The method always returns *the same* AnnotatedTypeConfigurator"_, but I agree that it's defined a little vaugely.
In any case, we would have to change the wording about replacing the original AT at the end of observer invocation which is IMHO incompatible behavioral change.
> 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)
6 years, 11 months
[JBoss JIRA] (CDI-721) configureAnnotatedType vs setAnnotatedType restrition is unecessarily strict
by Antoine Sabot-Durand (JIRA)
[ https://issues.jboss.org/browse/CDI-721?page=com.atlassian.jira.plugin.sy... ]
Antoine Sabot-Durand commented on CDI-721:
------------------------------------------
Discussion on this topic took place nearly 18 months ago and decision was made to go that way.
Now that spec, TCK and API (including javadoc) are published, it is too late to ask for such a modification.
BTW at this step even a typo in the spec couldn't be officially released without a MR, so I don't understand how you can imagine that such a modification can be performed.
This said, I agree that giving the possibility to reuse an {{AnnotatedTypeConfigurator}} could be useful, but I disagree on mixing approach in a given {{ProcessAnnotatedType}} observer. Providing access to a reusable {{AnnotatedTypeConfigurator}} in future CDI version could make sense (if I remember we decided that it would be better to wait for user feedback to introduce such a feature).
So I suggest that instead of asking non backward compatibility change, we start thinking of pros and cons and eventually means to give access to a part of configurator API from outside container events.
> 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)
6 years, 11 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:
-----------------------------------
Another observation
> A configurator instance is currently scoped to a container lifecycle observer method invocation
Can you please point me to the section where you did read this? I'm not able to find it
> and if we change this in CDI 2.x I think we would break backward compatibility.
No, we just remove a synthetic restriction. This would be perfectly backward compatible.
> 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)
6 years, 11 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:
-----------------------------------
[~mkouba] your sample above is exactly as the following:
{code:java}
void observe(@Observes ProcessAnnotatedType<Foo> event) {
event.setAnnotatedType(newAT_1);
event.setAnnotatedType(newAT_2);
configurator.methods().forEach(m -> m.add(Whatever.Literal.INSTANCE));
}
{code}
Whether this makes sense or not is TOTALLY up to what happens between those 2 setAnnotatedType calls. Maybe the user wants to *intentionally* replace newAT_1 later? How could you know?
It is *not* upon us to judge whether that code makes sense or not - it is purely a matter of the business logic of that project!
> 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)
6 years, 11 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:
----------------------------------------
If you call getAnnotatedType in your ... it is all good, otherwise failing is fine. This simple and easy to impl rule makes 1 true.
> 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)
6 years, 11 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:
----------------------------------
Ok, so I'll repeat once again:
1. {{getAnnotatedType()}} is not a "terminal" operation and cannot be used as a "build()" method
2. We're talking about the scope of a single container lifecycle method invocation
3. In the snippet below, what happens on the line with {{configurator.methods()}}?
{code:java}
void observe(@Observes ProcessAnnotatedType<Foo> event) {
AnnotatedTypeConfigurator<Foo> configurator = event.configureAnnotatedType().add(RequestScoped.Literal.INSTANCE);
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)
6 years, 11 months