Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#30, Jersey 2.26 support on 2.x #36

Closed
wants to merge 3 commits into from
Closed

#30, Jersey 2.26 support on 2.x #36

wants to merge 3 commits into from

Conversation

pierre-l
Copy link
Contributor

Here is a proposal for the upgrade to Jersey 2.26.
This requires a JAX-RS upgrade, which implies a Resteasy upgrade too.

The JAX-RS upgrade breaks the classloader injection hack in the ContainerRule. I replaced it using a temporary file. Any suggestion or would be welcome.

Once reviewed and approved, I intend to apply this to the master branch too.

…r when using Jersey 2.26

Upgrade to Jersey 2.26.
…r when using Jersey 2.26

Fix the classpath issue in the tests (force again the jersey tests to use the jersey client).
Update Jax-rs to 2.1
Update Resteasy and Grizzly to match the jax-rs and jersey versions
Fix the ProfileManager injection
@victornoel
Copy link
Member

thank @pierre-l, I will take a look at it this week-end.

The question of bumping to jersey 2.26 is complex, in particular because it will break compatibility with stable dropwizard, but we can try to tackle it.

…r when using Jersey 2.26

findbugs compliance.
@pierre-l
Copy link
Contributor Author

Hi @victornoel, thank you for this quick answer.
It seems to me that this compatibility issue would be solved using a multimodule project:

  • core
  • test-tools
  • jersey225
  • jersey226
  • resteasy

This would also reduce future classpath issues. You may want it to be done before the Jersey upgrade though.

@victornoel
Copy link
Member

@pierre-l yes, I agree, I have been thinking about that for a while (in particular for the tests) and your PR is making me wonder about the best way to handle the transition for jax-rs-pac4j 2.x... I wouldn't want to break backward compatibility, but maybe if we combine a multi-module project + a legacy jax-rs-pac4j artifact that just bring everything together (except for the container specific dependencies), this could work.

@victornoel
Copy link
Member

by the way, @pierre-l, if you have feedbacks on things to change in jax-rs-pac4j that break backward compatibility, don't hesitate to open issues before 3.x is released (I suppose it should happen in not so long...) :)

@pierre-l
Copy link
Contributor Author

maybe if we combine a multi-module project + a legacy jax-rs-pac4j artifact that just bring everything together (except for the container specific dependencies), this could work.

Do you mean having a sixth module, the "legacy" one, only on 2.x, containing the current mix of dependencies?

if you have feedbacks on things to change in jax-rs-pac4j that break backward compatibility

I haven't yet, but to be honest I haven't looked for it. I'll keep this in mind ;)

@victornoel
Copy link
Member

Do you mean having a sixth module, the "legacy" one, only on 2.x, containing the current mix of dependencies?

Exactly, I did some tests and it should work :)

@leleuj
Copy link
Member

leleuj commented Mar 21, 2018

ETA for pac4j v3.0.0: early May

@victornoel
Copy link
Member

@pierre-l the new module organisation is merged in master and 2.x, could you rebase this branch over it?

I feel like we should do the following (it's open to discussion):

  • for 2.x, we should introduce a jersey226-pac4j module, so that jersey-pac4j is backward compatible until pac4j 3.0 is released. After that, we won't touch so much 2.x I think so it will stay like this.
  • for master, we should move the current jersey-pac4j to jersey225-pac4j and start supporting jersey 2.26 in jersey-pac4j. Don't forget to change the dependency in dropwizard-pac4j to jersey225-pac4j then. With time jersey225-pac4j will disappear I think.

Concerning jax-rs: even though 2.1 is different, it is backward compatible with 2.0.1 I suppose (is it?), so for now, I feel like we only need jax-rs 2.0.1 APIs, so the core module can continue to relies on jax-rs 2.0.1 and all the integrations should provide at least 2.0.1 compatibility but can add new things for 2.1 (not sure the need exists though right now).

Concerning Resteasy, if there is a concrete API change that motivates to also change our code for latest version of resteasy, then let's do it in another PR maybe, and following the same strategy as with jersey then.

What do you think @leleuj and @pierre-l?

@pierre-l
Copy link
Contributor Author

@victornoel Seems good to me. I have the same feeling about JAX-RS.

@pierre-l
Copy link
Contributor Author

Closing in favor of #40, #41

@pierre-l pierre-l closed this Apr 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants