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

Using class instead of object for registering ServletJaxRsContextFactoryProvider #39

Merged
merged 2 commits into from Apr 22, 2018

Conversation

ganeshs
Copy link

@ganeshs ganeshs commented Apr 22, 2018

Raising #38 for master

@victornoel
Copy link
Member

@ganeshs can you rebase on master? just fixed the dependencies :)

@ganeshs
Copy link
Author

ganeshs commented Apr 22, 2018

Thanks. I see the build passing now.

@victornoel victornoel merged commit d5208a1 into pac4j:master Apr 22, 2018
@victornoel
Copy link
Member

thanks a lot @ganeshs

@ganeshs
Copy link
Author

ganeshs commented Apr 23, 2018

@victornoel just wanted to check when the next release is planned.

@ganeshs
Copy link
Author

ganeshs commented Apr 23, 2018

The issue is not properly fixed. We will have to reopen this. The HttpServletRequest object is not injected every time. I'm checking this. Will raise a new PR. You can revert this one @victornoel

@victornoel
Copy link
Member

Damned! thx for the feedback. If you manage to introduce a test that currently fail, I would love it!

As for the release, once we are sure it's ok, it can happen very fast :)

Should I revert or we can simply work on top of this?

@ganeshs
Copy link
Author

ganeshs commented Apr 24, 2018

@victornoel We will have to revert this fix. The fix is better done in the jax-rs-pac4j module. I already see similar fix done for grizzly jaxrs module GrizzlyJaxRsContextFactoryProvider.

@ganeshs
Copy link
Author

ganeshs commented Apr 24, 2018

Doing the fix from GrizzlyJaxRsContextFactoryProvider breaks resteasy tests but works for others.

@victornoel
Copy link
Member

@ganeshs ok, I will revert the PRs. Meanwhile, could you open an issue describing clearly the problem in dropwizard-pac4j or jax-rs-pac4j before we start to think of a solution? Thanks :)

@ganeshs
Copy link
Author

ganeshs commented Apr 24, 2018

Done. Created an issue for this #43

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.

None yet

2 participants