Is there a test of the beans.xml bean-discovery-mode attribute in Lite?

[starksm64]

Are we validating the Lite implementations need to parse the beans.xml bean-discovery-mode attribute in Lite?

[manovotn]

I am not sure that's testable universally for Lite and Full. But maybe I am just missing something. Here are my thoughts...

There is 3 values it can have:

• none
  • The issue with this is that Lite aims to create a singular bean archive and if you create a test that has one archive with discovery none, then you don't have any bean archive at all
  • And if memory serves, I have come across an issue on WFLY where I found out that they have optimizations where in such case they don't even start CDI container for that deployment (which isn't wrong, there is no point in booting it)
  • However, that makes it pretty untestable
• annotated
  • There is little point in doing this because the behavior is exactly the same as empty beans.xml meaning the test could easily show positive result even if implementor doesn't parse it
• anything else; since no othermode is not supported in CDI Lite
  • Here I am not sure CDI spec says what kind of error to throw here if any
  • Again, note that if you add something like mode all, then any CDI Full implementation running this test will correctly pass it

[starksm64]

So we have this statement in the beans_4_0.xsd schema:

      <xs:attribute name="bean-discovery-mode" use="optional" default="annotated">
        <xs:annotation>
          <xs:documentation>
            It is strongly recommended you use "annotated". This is now the default and it
            is also the default as of 4.0 when an empty beans.xml file is seen. When running
            in a CDI Lite  environment, this is the only aspect of the beans.xml file that
            is used.

            If the bean discovery mode is "all", then all types in this
            archive will be considered. If the bean discovery mode is
            "annotated", then only those types with bean defining annotations will be
            considered. If the bean discovery mode is "none", then no
            types will be considered.
          </xs:documentation>

Should we just be saying that in a Lite environment this is not read?

[Ladicek]

It needs to be read in Lite, but portable testing of the none value seems hard, per Matej's explanation above.

Hmm maybe we could have a test with 2 archives, both of them having beans.xml, one with annotated bean discovery mode, the other with none (but also some classes with BDAs). Then we could test that only beans from the first archvie exist. @manovotn, do you think the TCK can do that? I think we often use a WAR structure, so we could put some archives into WEB-INF/lib?

[manovotn]

Hmm maybe we could have a test with 2 archives, both of them having beans.xml, one with annotated bean discovery mode, the other with none (but also some classes with BDAs). Then we could test that only beans from the first archvie exist. @manovotn, do you think the TCK can do that? I think we often use a WAR structure, so we could put some archives into WEB-INF/lib?

Sure that's definitely possible but I didn't bring it up because I thought we don't want to do that for Lite. Apparently, I missed the fact that we already have lots of Lite tests with WAR.
We just need to make sure that the deployment structure is valid for Full - meaning that the WAR should have annotated discovery and the library should have none else you'd bump into visibility issues between WAR and its lib when attempting resolution.

It needs to be read in Lite

I agree but we might want to change the wording in schema so that we mention all isn't supported in Lite? Or, better still, that it is undefined?

[Emily-Jiang]

Hmm maybe we could have a test with 2 archives, both of them having beans.xml, one with annotated bean discovery mode, the other with none (but also some classes with BDAs). Then we could test that only beans from the first archvie exist. @manovotn, do you think the TCK can do that? I think we often use a WAR structure, so we could put some archives into WEB-INF/lib?

Sure that's definitely possible but I didn't bring it up because I thought we don't want to do that for Lite. Apparently, I missed the fact that we already have lots of Lite tests with WAR. We just need to make sure that the deployment structure is valid for Full - meaning that the WAR should have annotated discovery and the library should have none else you'd bump into visibility issues between WAR and its lib when attempting resolution.

+1 on adding tests to test none and others.

It needs to be read in Lite

I agree but we might want to change the wording in schema so that we mention all isn't supported in Lite? Or, better still, that it is undefined?

I am unclear about the all as bean-discovery-mode in lite. Do we treat as if saying annoted or we fail with invalid beans.xml. We need to clarify the behaviour. What about other invalid values such as mispelt anotated? I think it is better to thrown error message for anthing other than none or annotated in lite.

[manovotn]

I am unclear about the all as bean-discovery-mode in lite. Do we treat as if saying annoted

No because that would create a non-portable behavior between Lite and Full (i.e. taking the same deployment to CDI Full would result in different behavior).

or we fail with invalid beans.xml.

This is the tricky part - you cannot fail. Remember that all CDI Lite tests are executed by CDI Full implementations as well and those have to understand all mode and pass the test! Hence this one case is not testable within TCKs which is why I suggested to say it is undefined. However, Lite implementations can (and probably should) choose to react somehow and failure seems like a good option.

What about other invalid values such as mispelt anotated?

This remains the same as it was up until now.

[Ladicek]

I think we could possibly go one step further and define how exactly a bean discovery mode of "all" is non-portable.

We currently say:

Any other archive which contains a beans.xml file is not portable in CDI Lite. More kinds of bean archives exist in CDI Full.

We could say something like:

Any other archive which contains a beans.xml file is not portable in CDI Lite. More kinds of bean archives exist in CDI Full.

Implementations that do not support CDI Full are required to detect presence of an archive which contains a beans.xml file that has bean-discovery-mode attribute set to all and treat it as a deployment problem.

That said, I don't think this is a big deal.

[manovotn]

We could say something like:

We could, but it will, again, not be testable so I don't think it's worth the hassle. Alternatively we could just say that those impls are "encouraged" to treat it as deployment problem.

The only way we could test these things would be a property indicating mode in which the TCK is executed and the assertions tuned to that. But I would be -100 on that one because it just makes TCK way less readable and the results dependent on setting some knobs.

Hmm maybe we could have a test with 2 archives, both of them having beans.xml, one with annotated bean discovery mode, the other with none (but also some classes with BDAs).

@Ladicek I was thinking about this some more and we should try it but I am not sure its runnable everywhere.
As in, the most commonly used weld-embedded mode might fail, because arquillian-container-weld simplifies the deployment and merges it into one archive. That also means, it has to merge its beans.xml into one.
Looking at current TCKs, I can see we have BeanDiscoveryTest, see here. This is a Full test with all kinds of archives but half of its tests are marked as group INTEGRATION and I think this is precisely the reason.

[Emily-Jiang]

What is the Lite behaviour when it sees bean-discovery-mode=all? I don't think it is enough to say non-portable behaviour occurs. What is the Weld defined behaviour?

[manovotn]

What is the Weld defined behaviour?

Weld is a CDI Full implementation. As such it understands all as any CDI implementation did up until now.

What is the Lite behaviour when it sees bean-discovery-mode=all?

Please see the spec doc, 2.11.1. Bean archives.
PDF version from the last release has a link mentioned here - https://eclipse-ee4j.github.io/cdi/announcement/2022/01/25/400-RC3-spec.html

That is, to my knowledge, all the definition we have.
Since we wanted Lite to be subset of Full and to ensure portability, we instead approached this from the opposite direction and stated which archives are legitimate (and hence subsequently portable) for CDI Lite.

[Ladicek]

I don't think it is enough to say non-portable behaviour occurs.

It can't possibly be portable between across Lite-only and Full, so we have to say it's non-portable. What do you suggest we say on top of that?

Hmm maybe we could have a test with 2 archives, both of them having beans.xml, one with annotated bean discovery mode, the other with none (but also some classes with BDAs).

@Ladicek I was thinking about this some more and we should try it but I am not sure its runnable everywhere. As in, the most commonly used weld-embedded mode might fail, because arquillian-container-weld simplifies the deployment and merges it into one archive. That also means, it has to merge its beans.xml into one. Looking at current TCKs, I can see we have BeanDiscoveryTest, see here. This is a Full test with all kinds of archives but half of its tests are marked as group INTEGRATION and I think this is precisely the reason.

Fair enough. I'm fine with keeping the situation as is, or we could have a group LITE_INTEGRATION? :-)

[manovotn]

or we could have a group LITE_INTEGRATION

Crossed my mind as well but then it gets complicated for Lite implementation to tell which groups they need to execute and even worse for Full impls.
Frankly, I'm too +1 for keeping it as is.

[Emily-Jiang]

What is the Weld defined behaviour?

Weld is a CDI Full implementation. As such it understands all as any CDI implementation did up until now.

What is the Lite behaviour when it sees bean-discovery-mode=all?

Please see the spec doc, 2.11.1. Bean archives. PDF version from the last release has a link mentioned here - https://eclipse-ee4j.github.io/cdi/announcement/2022/01/25/400-RC3-spec.html

That is, to my knowledge, all the definition we have. Since we wanted Lite to be subset of Full and to ensure portability, we instead approached this from the opposite direction and stated which archives are legitimate (and hence subsequently portable) for CDI Lite.

The lite does not say anything about what other value except "annotated" behaves. Some runtime only suppots Lite only and I think the value of all maybe can be translated to annotated as Lite only supports annoated or none. If we add. a test, the test should check different results based on whether running in lite or full. By the way, the none value as bean-discovery-mode was not speced in the Lite section.

[manovotn]

By the way, the none value as bean-discovery-mode was not speced in the Lite section.

2.11.1. Bean archives, quoting:

An archive which:
• contains a beans.xml file with the bean-discovery-mode of none, or,
• contains a portable extension or a build compatible extension and no beans.xml file
113
is not a bean archive.

[manovotn]

Some runtime only suppots Lite only and I think the value of all maybe can be translated to annotated as Lite only supports annoated or none

Nope. This break portability from Lite to Full.
Imagine you have an interface SomeBean and then you have @Dependent class BeanImpl1 implements SomeBean and also class BeanImpl2 implements SomBean. Furthermore, assume somewhere in your application there is @Inject SomeBean. Lastly, the archive has beans.xml with discovery mode all.

With your suggestion, the deployment will work with CDI Lite, but will result in ambiguous resolution in CDI Full because BeanImpl2 would suddenly be discovered as well. We simply cannot do that.

If we add. a test, the test should check different results based on whether running in lite or full.

There is no telling, from inside a test, in which "mode" you run. CDI Full implementations do not have a concept of running in Lite mode either, they are just Full which means they by definition support everything Lite has.
That is also a reason why we defined what is a bean archive in Lite as opposed to defining what isn't because the perception will be different in CDI Full.

[Emily-Jiang]

I understood the challenge. I think leaving the bean-discovery-mode with anything except none or annotated unspeced would cause confusions.

Some runtime only suppots Lite only and I think the value of all maybe can be translated to annotated as Lite only supports annoated or none

Nope. This break portability from Lite to Full. Imagine you have an interface SomeBean and then you have @Dependent class BeanImpl1 implements SomeBean and also class BeanImpl2 implements SomBean. Furthermore, assume somewhere in your application there is @Inject SomeBean. Lastly, the archive has beans.xml with discovery mode all.

With your suggestion, the deployment will work with CDI Lite, but will result in ambiguous resolution in CDI Full because BeanImpl2 would suddenly be discovered as well. We simply cannot do that.

What is your proposal for maintaining portability from Lite to Full. I feel not specing the behaviour in Lite is even worse as the end users do not know what it will happen in lite when using Full as bean-discovery-mode.

If we add. a test, the test should check different results based on whether running in lite or full.

There is no telling, from inside a test, in which "mode" you run. CDI Full implementations do not have a concept of running in Lite mode either, they are just Full which means they by definition support everything Lite has. That is also a reason why we defined what is a bean archive in Lite as opposed to defining what isn't because the perception will be different in CDI Full.

We need to agree on a behaviour and then add a test. I am not sure what is the speced behaviour for the bean-discovery-mode=all.

[Ladicek]

I believe we have to distinguish 2 things:

1. our ability to specify the desired behavior,
2. our ability to test that an implementation conforms to the specification.

I submitted jakartaee/cdi#591 with my proposal for dealing with item no 1. Feel free to comment there.

When it comes to item no 2., we may have to admit that testing deployment problems is always hard and certain things may just be left untested.

[manovotn]

What is your proposal for maintaining portability from Lite to Full.

We already have all we need to maintain portability from Lite to Full. So long as the Lite application conforms to defined behavior, it is portable. The point of this issue is with mode all which is not ATM specced as valid value for Lite.

We need to agree on a behaviour and then add a test. I am not sure what is the speced behaviour for the bean-discovery-mode=all.

The inability of testing whatever behavior we agree on is the crux of this issue. And the reason why you cannot test it is precisely portability and the general ability to execute Lite under Full. Such test would inevitably have to expect contradicting results under Lite and Full.

I believe we have to distinguish 2 things:

Yes, that's good way of putting it, thank you.
We can adjust the wording if we decide to but we have to acknowledge that we still won't be able to enforce it via TCKs.