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

Provider ordering issue with Jersey #37

Closed
chkal opened this issue Jan 26, 2019 · 5 comments
Closed

Provider ordering issue with Jersey #37

chkal opened this issue Jan 26, 2019 · 5 comments
Labels
bug Something isn't working help wanted Extra attention is needed tck
Milestone

Comments

@chkal
Copy link
Contributor

chkal commented Jan 26, 2019

There are a few situations for which we rely on correct JAX-RS provider ordering via @Priority. The ability to order providers using this annotation has been introduced in JAX-RS 2.1.

Unfortunately this doesn't seem to work correctly in Jersey. I already filed eclipse-ee4j/jersey#3670, but this issue hasn't been addressed yet. To be honest, I'm quite surprised that this issue wasn't detected by the JAX-RS TCK.

Currently it looks like a random provider wins and @Priority is being ignored. This leads to the following issues:

  • The MVC parameter binding rules (i18n support + binding errors) aren't always applied correctly. That's because we provide our own ParamConverterProvider for standard Java types. If our provider is picked, everything works fine, but if the Jersey builtin provider is used, the binding is broken. This changes at random between deployments.
  • We provide a default ExceptionMapper for CsrfValidationException, but the developer can also provide his own mapper to change the default behavior. The user provided mapper should always be preferred, but which one gets executed changes at random.

For the latter case we already have a TCK test which sometimes works and sometimes fails:

https://i.imgur.com/N26A3sP.png

This is the most critical issue when running Krazo on Glassfish/Jersey. It would be great to find a solution here. Either by helping to fix the Jersey bug, or by finding a workaround on our side.

@chkal chkal added bug Something isn't working help wanted Extra attention is needed tck labels Jan 26, 2019
@chkal chkal added this to the 1.0.0-m06 milestone Jan 26, 2019
@jansupol
Copy link

jansupol commented Feb 7, 2019

There is a test in JAX-RS TCK that tests the @Priority. Jersey needed to pass the test. What is the difference to the MVC TCK test?

@chkal
Copy link
Contributor Author

chkal commented Feb 7, 2019

@jansupol Thanks for pointing us to the corresponding TCK test. I created the original Jersey issue before the TCK was transferred. I'll see if I find any difference.

@chkal
Copy link
Contributor Author

chkal commented Feb 23, 2019

I posed a few details about my findings here. Any help for this is welcome.

@chkal
Copy link
Contributor Author

chkal commented Mar 7, 2019

Great news. Looks like this issue will be fixed with eclipse-ee4j/jersey#4073

@chkal
Copy link
Contributor Author

chkal commented Apr 19, 2019

Krazo now passes the TCK on Eclipse Glassfish with a patched version of Jersey containing backports for two bugfixes from the master branch. So I guess we can close this issue now.

@chkal chkal closed this as completed Apr 19, 2019
@chkal chkal modified the milestones: 1.0.0-m06, 1.0.0-m05 Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed tck
Projects
None yet
Development

No branches or pull requests

2 participants