[CXF-7638] Only register provider if it implements specified contracts

[andymc12]

Per JAX-RS javadoc, all registered components must implement the
contracts specified in the passed in array of Classes or the map of
contracts to priorities. CXF should also log a warning when an invalid
contract is passed in to alert the user that the call to register has
been ignored.

Since ServerProviderFactory and CdiServerConfigurableFactory also used
this same registration mechanism (passing in server side filters/
interceptors, these parts also needed changes.

Looking for a review because I'm not 100% certain what the supportedContracts flow was really doing or if there are any negative effects from removing it.

https://issues.apache.org/jira/browse/CXF-7638

[sberyozkin]

This can be quite sensitive as users are also registering MBRs/MBWs and indeed all other standard interfaces now via the feature

[andymc12]

@sberyozkin Right, I'm not worried about the cases where they register via Feature or without specifying a contract interface. I'm more concerned about the case where ServerProviderFactory was creating the ConfigurableImpl with a List of built-in interfaces - then that List was used to register providers - even if those providers didn't implement the interfaces in the List. Do you know what the point of that List was? If it is essential, then I'll need to come up with an alternate fix, because this fix just gets rid of the List altogether. Thanks, Andy

[sberyozkin]

Hi, I was referring to a case where a provider implementing multiple interfaces has to be registered under a specific contract(s) only... May be I'm missing what the problem is. Can you please type the code showing where the issue is for me to look at the master code with the better focus :-) ? Thanks

[sberyozkin]

Sorry, looking at the test in this PR which explains the issue...

[sberyozkin]

OK, I've debugged the test and recall a bit better now what I was trying to do with these supported types. FYI, I found implementing all of these Configuration/Configurable very challenging, I recall typing and thinking I was not really sure if I was on the wrong or right path :-) as the original docs were quite compact, and the interpretation of some signatures can vary even today.

My own initial understanding was that it was primarily about supporting new filters/interceptors, given that in 2.0 (and in 2.1 for the most non-filter/non-interceptor interfaces) no priorities existed for MBR/MBW (and today for ex for ExceptionMapper or ParamConverterProvider) while many register methods accept the priorities or have the default priorities which have no any processing reqs for the last 2 interfaces and in 2.0 - for MBR/MBW as well. Hence those supported types lists (for client/server) only included the new 2.0 filter/interceptors and only later I started adding the support for the wider set of standard types.

But anyway, the patch looks fine, except that the supported contracts still need to be passed in, this is to protect Server-side Configuration from accepting ClientRequest/ResponseFilter and the client side Configuration - from ContainerRequest/ResponseFilter. These supported types lists may need to be expanded for the client and server (to include all other standard types which make sense on the client and the server) such that a provider can not be registered under SomeNonJaxrsContract - which should be rejected

Thanks

[sberyozkin]

Hmm...May be the registration of the unknown types should not be blocked as I suspect this is how many users are already registering CXF specific providers with register(...) without the base implementation having any knowledge of the scope of such providers.
So IMHO it should only be limited to blocking the well-known server interfaces in the client scope and vice versa.

[andymc12]

Hi @sberyozkin - that makes sense. I'll update with a new commit shortly. Thanks!

[andymc12]

@sberyozkin I updated the code with your suggestions. Do you want to take another look?

I also fixed the isEnabled behavior - we also ran into a couple of TCK/CTS failures with that method.

[andymc12]

One failure - org.apache.cxf.systest.ws.rm.SequenceTest.testOnewayAnonymousAcksSuppressedAsyncExecutor - which I've seen fail in other builds (like from #380) that are unrelated to these changes, so I think it should be safe to merge.