TCK Challenge: CDI tests should inject Maps into parameterized types

[brideck]

Challenged Tests: faces23 - ee.jakarta.tck.faces.test.javaee8.cdi.* faces40 - ee.jakarta.tck.faces.test.javaee8.cdi.*

TCK Version: Jakarta Faces 4.0.x

Tested Implementation: Open Liberty -- containing MyFaces 4.0

Description: The intent of the Faces specification regarding the injection of the various supported Map types is unclear to us. Section 5.6.1 of the Faces specification states: "The annotations in package jakarta.faces.annotation are used to cause @ Inject injection of the corresponding Map into a field. Generics may be used."

Looking at the API docs for RequestCookieMap (as an example), it is clearly stated that the annotation should be "on a field of type Map<String, Object>," likewise with the corresponding getter on ExternalContext.

However, the TCK injects these Maps into raw types in faces23 and faces40. One could argue that support of injection into raw types is implied by the specification, although it is not outright stated. Our contention is that the current TCK should only be using parameterized types for these Maps, as that is what is explicitly supported in the Faces API. If the intent is to support both types, then the specification should be clarified in a future release when the raw type tests can be re-enabled.

MyFaces currently only supports these parameterized types, so attempts to resolve the TCK's raw type injections are dutifully rejected by CDI with an error:

org.jboss.weld.exceptions.DeploymentException: WELD-001408: Unsatisfied dependencies for type Map with qualifiers @RequestCookieMap
  at injection point [BackedAnnotatedField] @RequestCookieMap @Inject ee.jakarta.tck.faces.test.javaee8.cdi.InjectRequestCookieMapBean.requestCookieMap
  at ee.jakarta.tck.faces.test.javaee8.cdi.InjectRequestCookieMapBean.requestCookieMap(InjectRequestCookieMapBean.java:0)

[pnicolucci]

@arjantijms @BalusC @tandraschko what do you think?

[arjantijms]

@pnicolucci it's an interesting case. I remember we added that one after some initial discussion, as it's indeed not immediately obvious it should be supported or not. I think @mnriem took the lead on this at the time (he was spec lead for 2.3), so would be good to hear from him.

[mnriem]

From my perspective while not specifically stated it should be supported as a matter of convenience to the user.

[pnicolucci]

@arjantijms @mnriem from my perspective, since it is not specifically stated in the Faces Specification, then this is just a "value add" feature that an implementation could implement but nothing that is required by the specification. As such I don't agree that this should be part of the TCK without any change to the Specification to justify such a test. Thus, I'd still say this is a valid challenge! MyFaces has implemented it the current way using CDI's @Produces for multiple versions of the specification.

[manovotn]

Hi,

from CDI perspective, it is correct that this injection doesn't work. Relevant CDI specification was already mentioned in this ticket but just for extra clarity, let me link it again - https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html#assignable_parameters

Specifically, the following sentence:

A parameterized bean type is considered assignable to a raw required type if the raw types are identical and all type parameters of the bean type are either unbounded type variables or java.lang.Object.

This requirement is not met by the TCK as the parameters are <String, Object>.

I should also note that when using dynamic resolution, the TCK can levarage TypeLiteral so that it can define a parameterized bean type along with its parameters.

"The annotations in package jakarta.faces.annotation are used to cause @ Inject injection of the corresponding Map into a field. Generics may be used."

Using raw types is not a good idea and I don't think a spec should encourage that but I do have a very limited understanding of faces spec :)

[arjantijms]

Thanks for chiming in @manovotn. There's a few things to say perhaps, but maybe it's also not for much reward in the end. I've accepted the challenge, and let's simply remove the raw types test. Thanks!

[brideck]

@arjantijms Re-opened because https://github.com/jakartaee/faces/blob/4.0.1/tck/faces23/cdi/src/main/java/ee/jakarta/tck/faces/test/javaee8/cdi/InjectApplicationMap2Bean.java#L35 needs to be updated with the same fix that you put in for the faces40 version.

[arjantijms]

@brideck additional issue has been addressed. Closing again, but if there's anything still to be done it can be re-opened.

[brideck]

Please reopen. One bean file was missed -- https://github.com/jakartaee/faces/blob/master/tck/faces40/cdi/src/main/java/ee/jakarta/tck/faces/test/javaee8/cdi/InjectHeaderMapBean.java

It would help if I tested the actual PR content instead of what I think should have been in it.

[pnicolucci]

I spoke with @brideck this morning and he confirmed everything has been resolved for this Challenge. I'm going to close it.