Clarify specification in ABD#addBean

[manovotn]

The original issue and discussion can be found under weld/core#3191.

The disputed specification text is the following section for AfterBeanDiscovery (mind the emphasis made in the quote):

addBean() fires an event of type ProcessSyntheticBean containing the given Bean *and then* registers the Bean with the container, thereby making it available for injection into other beans. The given Bean may implement Interceptor or Decorator.

This wording makes a difference in case two or more extensions (or rather observers) are attempting to both, monitor the ProcessSyntheticBean event and conditionally register synthetic bean. I.e. should the ProcessSyntheticBean be fired in between notifications of other AfterBeanDiscovery observers or should all the ABD observers be fired first, and then fire ProcessSyntheticBean events?

Weld chooses to notify all of the ABD observers before firing ProcessSyntheticBean events. Below is the case I presented in the linked issue.

Imagine the following situation, should you be allowed to intertwine various lifecycle observer notifications:

• Two CDI extensions, let's call them ExtensionA a ExtensionB
  • They have no ordering of events nor do they know about each other (which is pretty common and perfectly fine in bigger apps)
• ExtensionA always unconditionally registers a bean with type Foo (and no qualifiers) as a synthetic bean during ABD event
• ExtensionB monitors ProcessSyntheticBean<Foo>, make note of its presence/absence and then in ABD attempt to add a synthetic bean Foo if it was missing

Now, since CDI imposes exactly no rules on ordering of extensions, two situations can occur.

First one:

1. ExtensionA is notified first and registers synth bean Foo
2. ExtensionB gets the ProcessSyntheticBean event delivered, makes note of it and in its own execution skip the registration of the bean
3. Your app runs fine

Second one:

1. ExtensionB is notified first; as there is no Foo bean yet, it registers it
2. ExtensionA then registers the bean as well
3. You now have ambiguous dependency resolution

Cc @ljnelson and @Ladicek who participated in the discussion. Let's move it here and see what others think about it. We could/should also discuss it in the next CDI call.

As a side note - a very similar situation applies to registering a synthetic annotated type (which triggers PAT event).

[Azquelt]

Now, since CDI imposes exactly no rules on ordering of extensions

This is not true.

For portable extensions:

The notification order for observer methods within extensions follows the same ordering rule as defined in Observer ordering for non-extension observers. The priority of an observer method may be declared using the @Priority annotation.

For build compatible extensions:

The invocation order for extension methods may be controlled using the @Priority annotation. If an extension method does not have the @Priority annotation, the default priority jakarta.interceptor.Interceptor.Priority.APPLICATION + 500 is assumed. If two extension methods have equal priority, the ordering between them is undefined. Note that priority only affects order of extension methods in a single phase.

For this use case, I would expect ExtensionB to be observing with a high priority so that it gets the AfterBeanDiscovery event after ExtensionA has added the Foo bean.

However, this would only work if the ProcessSyntheticBean event is fired when ExtensionA adds the bean and before another AfterBeanDiscovery event is fired. If Weld fires all the AfterBeanDiscovery events first, then this strategy would not work.

Aside from that, it's still not ideal because we don't have any real benchmark for what is a "high" priority.

[manovotn]

Yes, you can order observer method and I do not deny that at all, but in my example I stated the following assumption:

They [the extensions] have no ordering of events nor do they know about each other (which is pretty common and perfectly fine in bigger apps)

What I mean is that there is no "implicit" ordering in case priority isn't specified and you cannot rely on, say, ExtensionA always going before ExtensionB which then leads to nondeterministic outputs with the example above.

[Azquelt]

Yes, that's true, if you don't apply a priority then they could run in either order.

However, I would argue that if you're attempting to do something like "if no bean of this type exists, add a bean", then you know that you want to run after other extensions since they may add beans, so even if you're not aware of those extensions you should be setting a high priority.

Another case that might occur is when two extensions are both trying to do this kind of check - "if a bean of type T was not discovered, add a bean". Consider what happens:

• If all AfterBeanDiscovery observers are run before any ProcessSyntheticBean observers
  • both extensions add a bean
  • resolution of that bean is ambiguous
• If ProcessSyntheticBean events are observed between calls to AfterBeanDiscovery observers
  • whichever extension observes AfterBeanDiscovery first will add a bean and the other will not
  • you'll get one or other of the synthetic beans, but it's undefined as to which one

For this reason, calling ProcessSyntheticBean between AfterBeanDiscovery observers seems better to me.

We also discussed this issue on the call and made the following observations:

• Build compatible extensions appear to be defined differently. The javadoc for Registration says

  This phase is executed twice. For non-synthetic beans and observers, this phase is executed before synthesis. For synthetic beans and observers, this phase is executed after synthesis.

  By defining it as phases, it's more clear that all of the Registration calls for synthetic beans and observers should happen after all of the Synthesis calls.

• The situation where two extensions both add synthetic beans which cause ambiguity could be avoided by both extensions making their synthetic bean an alternative with a priority so that if another extension registers the same bean, they will not be ambiguous. (Of course, both extensions could choose the same priority and then you'd still be stuck)

[manovotn]

Firstly, sorry to have missed the call, I was hoping to go through it there but did a poor job planing other colliding duties.

For this reason, calling ProcessSyntheticBean between AfterBeanDiscovery observers seems better to me.

I am not sure I understand. How is it better to have a non deterministic outcome rather than seeing an error showing you the bean has defined twice.
The latter makes the situation actionable in multiple ways (alternative, [default bean,] defining your own bean earlier via annotated type, skipping the definition entirely,...) whereas the former you won't directly observe and it will be very hard to decipher assuming you needn't be aware all of the extensions potentially registering that bean. It will "just work" until it wont so to speak :)

Build compatible extensions appear to be defined differently. The javadoc for Registration says

That's an interesting observation.
We should probably unify this across the extensions (whichever way we choose).
And I need to take a look at both, Weld and ArC to see if there is a hint as to why they both do the notifications in batches.

The situation where two extensions both add synthetic beans which cause ambiguity could be avoided by both extensions making their synthetic bean an alternative with a priority so that if another extension registers the same bean, they will not be ambiguous. (Of course, both extensions could choose the same priority and then you'd still be stuck)

Yes, and if we manage to add the notion of default beans, this problem might vanish entirely although it would still make sense to unify it somehow.

[Azquelt]

For this reason, calling ProcessSyntheticBean between AfterBeanDiscovery observers seems better to me.

I am not sure I understand. How is it better to have a non deterministic outcome rather than seeing an error showing you the bean has defined twice. The latter makes the situation actionable in multiple ways (alternative, [default bean,] defining your own bean earlier via annotated type, skipping the definition entirely,...) whereas the former you won't directly observe and it will be very hard to decipher assuming you needn't be aware all of the extensions potentially registering that bean. It will "just work" until it wont so to speak :)

We have two extensions trying to perform an operation without knowledge of the other. These could be libraries which the application author does not control

• AfterBeanDiscovery observers are run before any ProcessSyntheticBean observers
  • This scenario prevents the two libraries from working together. The second library to have their AfterBeanDiscovery called is operating without knowledge of the beans that have already been registered and thus is unable to take the correct action.
• ProcessSyntheticBean observers are called between AfterBeanDiscovery observers
  • This scenario allows the two libraries to work together if both are written correctly. When AfterBeanDiscovery is called, each library has full information about the beans that have already been defined and is able to take the correct action.

It's also worth noting that either extension could raise a validation error if it detects that something unexpected has happened. E.g. if a ProcessSyntheticBean event occurs after AfterBeanDiscovery has been called which means the extension has made the wrong choice.

[manovotn]

Some more notes as I spent the afternoon digging in this...

Firstly, in the original issue, @ljnelson said:

The specification says "addBean() fires an event of type ProcessSyntheticBean containing the given Bean and then registers the Bean with the container". It makes it sound like, during the lifetime of an addBean() invocation, both things are supposed to happen. The specification statement, in other words, is a statement about what addBean() must do while it is "running".

I should note that this is definitely not possible - not in case of BeanConfigurator anyway. During the notification of the ABD OM, the configurator stays mutable (it has no complete() method to mark when modifications are done). We therefore have to wait with bean registration after the OM method exits.
If the current spec wording seems to imply that PSB happens during the notification, we should amend that.
Needless to add we ought to treat both addBean() methods the same way to keep it consistent.

FTR, the javadoc of the method is already pretty clear in that [emphasis is mine]:

Obtains a new {@link BeanConfigurator} to configure a new {@link Bean} and add it at the end of the observer invocation.

Perhaps we should use similar wording in the specification text as well? Thoughts?

Secondly, I went digging into history of Weld and CDI to see if I can find the roots of this.

• This behavior was introduced into Weld in 2.2.0.Final version with WELD-1646.
  • There isn't many details there, but it does lead to CDI-392 which it aims to implement.
• CDI-392 is more interesting and has a long discussion about which BM method can be invoked at which point in container bootstrap
  • My understanding is that this was the basis for the current behavior as Weld attempts to "normalize" the view of the container regardless of when your extension OM gets invoked (the non-guaranteed order of OM invocation with no priority that was mentioned earlier in this issue)

Thirdly, I went again through the specification texts and I don't see Weld's behavior as breaking the specification.
Specification only goes to say that there is PSB event fired before registering the bean and that much is honored.
I didn't find any part that would assume and/or guarantee that you can rely on ABD and PSB timing.

The examples @Azquelt and I presented indicate that both approaches have their ups and downs and both are solvable one way or the other. I, however, still stand behind my opinion that the current Weld approach is "safer" simply because it eliminates the non-deterministic element of unordered extensions adding the same beans.

[manovotn]

Removing from the Milestone as per discussion on the CDI mtg today (Sep 23, 2025).
The issue was inspired by a use case which basically resulted in us discussing the concept of "default" beans which are being added via a separate issue.
This is hence low priority.