WELD-2775 Make sure BM#getEvent() adds @Default qualifier correctly

[manovotn]

Attempts to fix the missing @Default qualifier when firing events via BeanManager.getEvent() API.

[mkouba]

Looks good.

Although I don't think the requirement for BM#getEvent() is correct. It should be more like BM#createInstance()...

[manovotn]

Looks good.

Although I don't think the requirement for BM#getEvent() is correct. It should be more like BM#createInstance()...

I agree, but for now this is what we're stuck with 🤷

[Ladicek]

I wonder (here and in EventMetadataImpl), what if qualifiers contains @Any (and nothing else)?

These are event qualifiers, so @Any is always present (that is, there's no difference between user specifying @Any and user not specifying @Any), so perhaps @Default should be added too in this case?

[mkouba]

I wonder (here and in EventMetadataImpl), what if qualifiers contains @Any (and nothing else)?

These are event qualifiers, so @Any is always present (that is, there's no difference between user specifying @Any and user not specifying @Any), so perhaps @Default should be added too in this case?

Why? If you do @Inject @Any Event<Foo> event and then event.fire(new Foo()) then the @Default should not be present, or?

[manovotn]

I wonder (here and in EventMetadataImpl), what if qualifiers contains @Any (and nothing else)?

Had the same idea and my previous version of this code did exactly what you're suggesting but there were TCKs failures.
IIRC it was the EventMetadataTest#testSimpleEvent which doesn't expect @Default to be present when you use something like @Inject @Any Event<SimpleEvent> and fire event from there.

[Ladicek]

OK, that makes sense. @Inject Event<Foo> is equivalent to @Inject @Default Event<Foo>, so the set of specified qualifiers is {@Default} and the set of event qualifiers is {@Default, @Any}.

So the qualifiers here are not event qualifiers, they are specified qualifiers.

The problem is that

slightly different behaviour applies to observers with @default qualifier
...
Such observer will only be notified for events having either no qualifiers or only @Default qualifier

talks about event qualifiers (which are never {} or {@Default}), even though it probably should talk about specified qualifiers (which are also never {}, but may be {@Default}).

(Technically, that paragraph should probably never have been added. Looking at the history, this was an attempt to clarify that @Inject Event<Foo> is equivalent to @Inject @Default Event<Foo> due to how injection points work, but that attempt unintentionally added a new provision to the specification that triggers in some weird situation. I actually don't know what that situation is anymore, considering that BM.getEvent() should unconditionally add @Default to the set of specified qualifiers...)

[Ladicek]

Also maybe fix the comment at BeanManagerImpl.EventInjectionPoint.getQualifiers(), because for events, ResolvableBuilder.create() is never called with an empty set of qualifiers (hence this PR).

[manovotn]

Also maybe fix the comment at BeanManagerImpl.EventInjectionPoint.getQualifiers(), because for events, ResolvableBuilder.create() is never called with an empty set of qualifiers (hence this PR).

I still think the an empty set makes the most sense - this is just a representation of a synthetic IP in case it gets introspected (via EventMetadata I assume). The exact same happens for InstanceInjectionPoint.

[Ladicek]

No, that's just an implementation comment. Why are you explicitly adding @Default to the ResolvableBuilder in this PR? Because we never call ResolvableBuilder.create() with an empty set of qualifiers when it comes to events.

[manovotn]

No, that's just an implementation comment. Why are you explicitly adding @Default to the ResolvableBuilder in this PR? Because we never call ResolvableBuilder.create() with an empty set of qualifiers when it comes to events.

Ok, I'll change it to:

@Default and @Any are added as needed in ObserverQualifier#buildEventResolvable