[RESTEASY-1533] Public/private module separation

[asoldano]

This PR provides a proposal for defining a public spi for the current resteasy-jaxrs and resteasy-client modules. This is a requirement for clearly stating what we guarantee to be stable and only possibly changing in major releases, versus what might changes in minor and micros too. Proper user expectations, easier project evolution and maintenance.

resteasy-jaxrs

This is now split into resteasy-jaxrs and resteasy-jaxrs-impl. The former includes the following packages:

• org.jboss.resteasy.annotations
• org.jboss.resteasy.api.validation
• org.jboss.resteasy.spi
• org.jboss.resteasy.plugins.providers.validation

The latter includes:

• org.jboss.resteasy.core
• org.jboss.resteasy.mock
• org.jboss.resteasy.plugins
• org.jboss.resteasy.specimpl
• org.jboss.resteasy.tracing
• org.jboss.resteasy.util

resteasy-client

This is now split into resteasy-client and resteasy-jaxrs-client-api . The latter only includes the following classes that are used for configuring the RESTEasy client additions:

• ClientHttpEngine & ClientHttpEngineBuilder
• ProxyBuilder & ProxyConfig
• ResteasyClient
• ResteasyClientBuilder
• ResteasyWebTarget

JBoss Modules

There's a new org.jboss.resteasy.resteasy-jaxrs-public module including resteasy-jaxrs and resteasy-jaxrs-client-api. The already existing org.jboss.resteasy.resteasy-jaxrs as well as all other org.jboss.resteasy.* modules should become private.

Non backward compatible changes

Clearly, the refactoring comes with some unavoidable class moves and changes that imply breaking backward comaptibility (this is the reason why this only fits master). I tried to avoid breaking anything as much as possible, but sometimes there's no way (we also have to avoid splitting packages because of JDK 9 changes).

• Some classes have been moved from package org.jboss.resteasy.util to package org.jboss.resteasy.spi.util: Types, FindAnnotation, MethodHashing and PickConstructor.
• Some classes have been moved from package org.jboss.resteasy.spi.touri to package org.jboss.resteasy.plugins.touri: AbstractURITemplateAnnotationReader*, MappedByAnnotationReader, ObjectToURI, URITemplateAnnotationReader and URIableURIResolver.
• HTTPResponseCodes has moved from package org.jboss.resteasy.util to package org.jboss.resteasy.spi.
• ResponseInvoker has moved from package org.jboss.resteasy.core to package org.jboss.resteasy.spi and has broader request and response types on invoke(..) methods.
• The getMethod and setMethod methods of ResteasyAsynchronousResponse have been factored out to new org.jboss.resteasy.core.ResourceMethodInvokerAwareResponse.
• InternalDispatcher has moved from pachage org,jboss.resteasy.spi to package org.jboss.resteasy.core.
• ValueInjector and Dispatcher have moved from pachage org,jboss.resteasy.core to package org.jboss.resteasy.spi.
• org.jboss.resteasy.spi.LinkHeaders has been removed.
• ResteasyUriInfo has moved from package org.jboss.resteasy.spi to package org.jboss.resteasy.specimpl.
• org.jboss.resteasy.spi.ResteasyDeployment has turned into an interface with the same methods it previously had; it's now implemented by org.jboss.resteasy.core.ResteasyDeploymentImpl
• org.jboss.resteasy.spi.ProviderFactory is now an abstract class; all non-static methods are not abstract and implemented in org.jboss.resteasy.core.ResteasyProviderFactoryImpl.
• JaxrsInterceptorRegistry and JaxrsInterceptorRegistryListener have moved from package org.jboss.resteasy.core.interception.jaxrs to package org.jboss.resteasy.spi.interception; the former is also turned into an interface with the same method it had before; it's implemented by JaxrsInterceptorRegistryImpl in package org.jboss.resteasy.core.interception.jaxrs.
• ClientRequestFilterRegistry, ClientResponseFilterRegistry, ContainerRequestFilterRegistry, ContainerResponseFilterRegistry, ReaderInterceptorRegistry and WriterInterceptorRegistry in package org.jboss.resteasy.core.interception.jaxrs have been renamed to ClientRequestFilterRegistryImpl, ClientResponseFilterRegistryImpl, ContainerRequestFilterRegistryImpl, ContainerResponseFilterRegistryImpl, ReaderInterceptorRegistryImpl and WriterInterceptorRegistryImpl respectively, same package.
• ResteasyUriBuilder has moved from package org.jboss.resteasy.specimpl to package org.jboss.resteasy.spi and is now abstract; it's extended by non-abstract class org.jboss.resteasy.specimpl.ResteasyUriBuilderImpl.
• org.jboss.resteasy.client.jaxrs.ProxyBuilder is now abstract; it's extended by non-abstract class org.jboss.resteasy.client.jaxrs.internal.proxy.ProxyBuilderImpl.
• Some classes have been moved from package org.jboss.resteasy.client.jaxrs to package org.jboss.resteasy.client.jaxrs.engines: AsyncClientHttpEngine, ClientHttpEngineBuilder43, *VerifierWrapper and BasicAuthentication.
• org.jboss.resteasy.client.jaxrs.ResteasyClient is now abstract, witth all non static methods as abstract ones; it's extended by non-abstract class org.jboss.resteasy.client.jaxrs.internal.ResteasyClientImpl.
• org.jboss.resteasy.client.jaxrs.ResteasyClientBuilder is now abstract, witth all non static methods as abstract ones; it's extended by non-abstract class org.jboss.resteasy.client.jaxrs.internal.ResteasyClientBuilderImpl.
• ViolationsContainer has been removed
• ConstraintTypeUtil11 has moved from package org.jboss.resteasy.plugins.providers.validation to package org.jboss.resteasy.plugins.validation.
• ConstraintTypeUtil has moved from package org.jboss.resteasy.plugins.providers.validation to package org.jboss.resteasy.spi.validation.
• ResteasyViolationException is not abstract and extended by ResteasyViolationExceptionImpl

[marekkopecky]

Module naming is a little bit inconsistent:
API: resteasy-jaxrs, resteasy-jaxrs-client-api
IMPL: resteasy-jaxrs-impl, resteasy-client

Jax-rs api doesn't have a suffix (-api), but client api has a suffix (-api). Jax-rs impl has a suffix (-impl), but client impl doesn't have a suffix (-impl).

What about make some naming convention, like "resteasy-jaxrs, resteasy-jaxrs-impl, resteasy-client, resteasy-client-impl", or "resteasy-jaxrs-api, resteasy-jaxrs, resteasy-client-api, resteasy-client", or something similar? WDYT?

[marekkopecky]

What about to add more final keyword to API classes? Like CorsHeaders, ConstraintTypeUtil11, etc?

[marekkopecky]

If you have a time ... you can also check checkstyle, current master should be aligned with this checkstyle.xml: https://issues.jboss.org/browse/RESTEASY-1974?focusedCommentId=13610933&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13610933
(this checkstyle file contains just subset of rules from checkstyle in resteasy repo)

[asoldano]

Module naming is a little bit inconsistent:
API: resteasy-jaxrs, resteasy-jaxrs-client-api
IMPL: resteasy-jaxrs-impl, resteasy-client

Jax-rs api doesn't have a suffix (-api), but client api has a suffix (-api). Jax-rs impl has a suffix (-impl), but client impl doesn't have a suffix (-impl).

What about make some naming convention, like "resteasy-jaxrs, resteasy-jaxrs-impl, resteasy-client, resteasy-client-impl", or "resteasy-jaxrs-api, resteasy-jaxrs, resteasy-client-api, resteasy-client", or something similar? WDYT?

I've been thinking about this; resteasy-jaxrs is not really an api, though, while resteasy-jaxrs-client-api is.
If we want to keep an easy upgrade path, we can do:

• resteasy-jaxrs-spi
• resteasy-jaxrs
• resteasy-client-api
• resteasy-client

Otherwise a better naming could be:

• resteasy-core-spi
• resteasy-core
• resteasy-client-api
• resteasy-client

... dropping the jaxrs which is kind of assumed. WDYT?

[rsvoboda]

I like naming without jaxrs part a bit more

[rsearls]

This restructuring looks good to me.
You are the new record holder for number of files changed in a single commit.

[asoldano]

@marekkopecky I've re-written two commits fixing the resteasy-jaxrs and resteasy-jaxrs-client-api modules according to the checkstyle.
As for the module names, @rsvoboda , I also have a slight preference for the resteasy-core and resteasy-core-spi ones; it's one additional thing to change for those migrating to version 4 (most will have the resteasy-jaxrs dependency in their projects), but that would probably raise awareness on the fact that something internal and hence potentially changing in future (even if we'll strive to avoid that) is used.

[ronsigal]

There's a lot to absorb. Now THIS is "epic". ;-)

One point, for now: Seeing org.jboss.resteasy.plugins.providers.validation in public was a little jarring, and I think it can be done away with. As for ConstraintTypeUtil and ConstraintTypeUtil11, once upon a time there were two versions of the Resteasy validation layer, resteasy-validator-provider-11 for Hibernate Validator 5 (Bean Validation 1.1) and another one for Hibernate Validator 4 (Bean Validation 1.0). That's why there were two versions of ConstraintTypeUtil. Now that the BV 1.0 module is gone, I think it would be safe to move ConstraintTypeUtil and ConstraintTypeUtil11 to the resteasy-validator-provider-11 module. Also, it looks like ViolationsContainer, which is used only by two org.jboss.resteasy.api.validation.ResteasyViolationException (resteasy-jaxrs) constructors, can be removed since those constructors are no longer called.

Alessio, I can make these changes in a separate pull request, or you can just make them here.

-Ron

[asoldano]

@ronsigal thanks Ron, I agree on your proposals. I'm basically off for today, so if you still have some time, feel free to create a commit on top of mine and I'll cherry-pick it here. Otherwise we can work on this next week.

[ronsigal]

@asoldano, I've created #1698 with the validation changes. The reason there are validation classes in resteasy-jaxrs was, as I mentioned before, that there used to be two different implementations of validation. I was thinking of moving everything into resteasy-validator-provider-11, but I see that there is now CDI 2.0. I guess, in principle, that we could have a new validation module based on CDI 2.0, so maybe it makes sense to leave things spread out, with API and SPI classes in resteasy-jaxrs.

-Ron

[asoldano]

Thanks @ronsigal , I've cherry-picked the last commit from your PR 👍

[marekkopecky]

I check quickstarts and grep TS logs for "Deployment * is using a private module". I don't find anything relevant, just this one:

testsuite/integration-tests/target/surefire-reports/null-output.txt:10:03:49,003 WARN  [org.jboss.as.dependency.private] (MSC service thread 1-6) WFLYSRV0018: Deployment "deployment.war_with_jsonb.war" is using a private module ("org.jboss.resteasy.resteasy-json-binding-provider") which may be changed or removed in future versions without notice.

But this message is printed in current master as well, so it is not relevant with current PR.

[asoldano]

I've added another commit that moves the contextual data management out of the ResteasyProviderFactory (almost all static methods except for the get/newInstance) to a dedicated ResteasyContext class, out of the SPI. That also allows removing the ThreadLocalStack class from the SPI too. Perhaps there's much that could now be turned into "private" in the ResteasyProviderFactory interface and move out of the SPI...

[asoldano]

@marekkopecky , do you know/have you checked if that warning is printed only when the module dependency is explicitly set by the user? (versus automatically added by wildfly jaxrs integration)

[asoldano]

Having heard no further opinions on the module naming topic, I've renamed resteasy-jaxrs -> resteasy-core-spi, resteasy-jaxrs-impl -> resteasy-core, resteasy-jaxrs-client-api -> resteasy-client-api.

At this point, I'm planning to leave this PR here for review for a couple of days, so that it can be reviewed, please let me know.

[asoldano]

Rebased to fix conflicts..

[marekkopecky]

@asoldano warning is printed only when some class from private module is used in deployment code ...