Applications are required to have a public no-arg constructor. The Ec…

[jamezp]

…hoApplication must be public.

If this needs an issue file please let me know.

resolves #1099

[jamezp]

@mkarg I didn't see anything in the specification which directly referenced an Application. However, resources and providers are required to have a public constructor. It seems applications should as well.

[mkarg]

As you did not find an unumbiguous spec wording, I do not see why the TCK needs to be changed. Unless it is explicitly forbidden by the spec, it is clearly valid to do what the TCK currently does. If you want to change the spec, please open an issue for a spec change. Thanks.

[jamezp]

The TCK did not include a test like this before and has not tested a non-public application before. While it may be ambiguous, I don't see how it could be considered good practice. If resources and providers explicitly state they require public constructors why would it be assumed an application can be private?

3.1.2. Constructors

Root resource classes are instantiated by the JAX-RS runtime and MUST have a public constructor for which the JAX-RS >runtime can provide all parameter values. Note that a zero argument constructor is permissible under this rule.

[jansupol]

Section 11.2.3:

In a product that supports CDI, implementations MUST support the use of CDI-style Beans as root resource classes, providers and Application subclasses.

As argued before with the test to instantiate the class using the most arguments constructor - When CDI is used, we need to be able to instantiate the classes with public no-arg constructors. RestEasy was able to turn off CDI with a proper beans.xml that is added to the tests. But we are approaching CDI-only injection times...

[jamezp]

Section 11.2.3:

In a product that supports CDI, implementations MUST support the use of CDI-style Beans as root resource classes, providers and Application subclasses.

As argued before with the test to instantiate the class using the most arguments constructor - When CDI is used, we need to be able to instantiate the classes with public no-arg constructors. RestEasy was able to turn off CDI with a proper beans.xml that is added to the tests. But we are approaching CDI-only injection times...

I will say RESTEasy has a ways to go to truly support CDI like it should be supported IMO. That said this test does not include a beans.xml file and does not qualify for an empty beans.xml file: https://jakarta.ee/specifications/cdi/3.0/jakarta-cdi-spec-3.0.html#what_classes_are_beans.

I really don't understand why making this public is a big deal. It changes nothing about what the test is actually testing.

[jimma]

Except this EchoApplication and StatusApplication in ./src/main/java/ee/jakarta/tck/ws/rs/spec/client/exceptions/ClientExceptionsIT.java, other tck application classes are all public. Is there particular reason we set EchoApplication and StatusApplication as private ?

[mkarg]

The TCK test are spec compliant. The spec is the sole source of truth, not the TCK. So if we want to enforce public, we break backwards compatibility of existing applications running on Jersey. Hence, that change is a spec change. Hence we need a spec change proposal. Unless that proposal is agreed, the TCK is correct as-is, including private.

[jansupol]

So if we want to enforce public, we break backwards compatibility of existing applications running on Jersey

No, that is not what this is about. This is about not enforcing private so that each implementation can instantiate the class.

[spericas]

@mkarg Can you please review your vote?

[jansupol]

Except this EchoApplication and StatusApplication in ./src/main/java/ee/jakarta/tck/ws/rs/spec/client/exceptions/ClientExceptionsIT.java, other tck application classes are all public.

@jamezp @jimma Are you going to add the second Application to this PR?

[jamezp]

Except this EchoApplication and StatusApplication in ./src/main/java/ee/jakarta/tck/ws/rs/spec/client/exceptions/ClientExceptionsIT.java, other tck application classes are all public.

Are you going to add the second Application to this PR?

@jansupol This one is kind of weird. It's not in the downloaded artifact. It's also got a @since 4.0 so I'm not sure if it's intended to target 3.1 or not. However, I'm happy to make the change there too.

[mkarg]

I am sorry but as long as the spec does not clearly say that Application MUST be public, any application programmer is free to use private Application, so the TCK ensures this is possible. I have not problem chaning the spec to allow this, but first spec change, then TCK change.

[mkarg]

To quickly resolve the current situation I propose that @jamezp adds a commit into this PR that changes the wording of the spec accordingly. I am +1 for the spec change + TCK change, but -1 for just a TCK change.

[spericas]

I am sorry but as long as the spec does not clearly say that Application MUST be public, any application programmer is free to use private Application, so the TCK ensures this is possible. I have not problem chaning the spec to allow this, but first spec change, then TCK change.

Does this imply that we need TCK tests for all things that the spec is not clear about? This makes no sense, and it would be a never ending task. As explained by others, the TCKs test spec assertions, and this is not one (al least not yet). Only this TCK needs updating for 3.1.

[jamezp]

I've updated the PR to also fix the ClientExceptionsIT.

[jimma]

+1 for this change.

[arjantijms]

Thanks everyone!

[jamezp]

Thank you all! This is much appreciated.