org.jboss.cdi.tck.tests.deployment.discovery.EmptyBeansXmlDiscoveryTest breaks backward compat

[struberg]

Can someone please explain to me the rational behind org.jboss.cdi.tck.tests.deployment.discovery.EmptyBeansXmlDiscoveryTest?

An empty beans.xml file always behaved as marker file for a ALL!

What you wrote in CDI-4.0 is that "A new beans.xml 4.0 schema file has been added and the namespace of the beans_4_0.xsd schema file is xmlns:jakartaee="https://jakarta.ee/xml/ns/jakartaee", the same as 3.0. The key changes in the schema are to make the bean-discovery-mode attribute default to annotated and to use annotated as the default when an empty beans.xml is seen in a deployment. "

So if a beans.xml with a version="4.0" gets detected then the bean discovery mode gets changed. Otherwise not.

If you really intend to change the default in CDI-4.0 for a completely empty beans.xml file then it will not even be backward compat with CDI-3.0. That would imo be a major pain for real world adoption and a very stupid move.

[manovotn]

The test is changed to correspond to changes summarized in #500

The initial description as well as my comment (#500 (comment)) should explain the current state in CDI 4.
In short, yes this is breaking for fraction of CDI applications and this was expected.
The above is also a reason why there is also a mandatory compatibility switch for EE integrators. However, the TCK is unable to test this as the config is implementation specific.

Since the test IMO reflects current specification requirements, I'll close this issue.
If you mean to challenge the design as such, I suggest we keep the conversation in one place - the CDI issue.

[struberg]

This breaks backward compatibility. Got the requirement for backward compatibility with deprecation and pruning phases ditched in JakartaEE? If not we should escalate this question to the council. It might not be a failure of the TCK, but from the CDI spec though

[struberg]

BTW, the Jakarta rules explicitly forbid such changes.
https://eclipse-ee4j.github.io/jakartaee-platform/CompatibilityRequirements

[dblevins]

I do agree with @struberg that the backwards incompatibility is unfortunate, hence we did not vote +1 on CDI. That said it is now part of the ratified spec.

The TCK test challenge process explicitly calls out that the spec itself cannot be challenged. See "Invalid Challenges", bullet two:

• https://jakarta.ee/committees/specification/tckprocess/

Can we agree that what's being challenged is the spec itself? If so, we can keep this issue open but we'd need to move it to the spec repo and address it through the spec process in a future version. I would definitely recommend a flag to portably turn on the CDI 3.0 and before behavior.

[manovotn]

Can we agree that what's being challenged is the spec itself? If so, we can keep this issue open but we'd need to move it to the spec repo and address it through the spec process in a future version. I would definitely recommend a flag to portably turn on the CDI 3.0 and before behavior.

@dblevins this flag is anchored in the spec text of 4.0 and required to be implemented by EE containers.

To quote, the spec says:

For compatibility with CDI versions prior to 4.0, CDI Full products must contain an option that causes an archive with empty beans.xml to be considered an explicit bean archive.

[dblevins]

@manovotn Understood. I referred to a portable flag vs an implementation-specific flag. We could then test it, users can also get the benefit. If I can think of any good options and find the time to contribute language/tests, I'll create a new ticket.

[manovotn]

True, this has come up during development and I agree a "portable flag" would be a better option.
However, CDI itself doesn't have an app wide config mechanism through which we could introduce such a flag.
We'd have to introduce some such mechanism (since I assume we don't want to bring in dep to something like MP config) for just this one option.

How we landed on this was partly because of what I said above and partly because previous versions of the spec had exactly this kind of flag for years. Since CDI 1.1, there was this section in the spec which stated:

For compatibility with Contexts and Dependency 1.0, products must contain an option to cause an archive to be ignored by the container when no beans.xml is present.

This also couldn't be tested and was vendor specific but was mandated by the spec nonetheless.

But maybe we could introduce a way to assert the new flag. Perhaps adding a test that asserts the old behavior and a TCK API/SPI that will have to be implemented by whoever runs the TCK? We already have some such SPIs in TCK, the Contexts one being an example.

[Ladicek]

This not a valid TCK challenge. I'm moving the issue to the cdi repository for further discussion.

[manovotn]

Is there further discussion to be had @dblevins?

We have separate issue for updating TCK to provide the ability to test the switch. The only other thing we could do is to enforce a CDI config for this - currently only available as something as system property because CDI has no config API of its own (nor could it use Jakarta or MP config as it can be used in both environments independently).

[arjantijms]

since I assume we don't want to bring in dep to something like MP config) for just this one option.

Maybe not just for this option, but together with some other use cases that surfaced recent in Telemtry, it might be time to work towards this indeed.