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

Added integration test to reproduce #4099 #4101

Merged
merged 1 commit into from
Apr 24, 2019

Conversation

chkal
Copy link
Contributor

@chkal chkal commented Apr 14, 2019

This pull request contains a new integration test which reproduces #4099. Please see #4099 for a detailed description of the problem.

Steps to reproduce:

  • Remove the @Ignored annotation in PriorityAnnotationOnExceptionMappersTest
  • Run the test a few times
  • You will see that the test randomly passes or fails

Setting a breakpoint here will show the root cause of the problem:

private static int getPriority(Class<?> serviceClass) {
Priority annotation = serviceClass.getAnnotation(Priority.class);
if (annotation != null) {
return annotation.value();
}
// default priority
return Priorities.USER;
}

The method scans InstanceSupplierFactoryBridge instead of the actual implementation class for the @Priority annotation.

@chkal
Copy link
Contributor Author

chkal commented Apr 24, 2019

I updated the pull request and removed the @Ignore annotation on the test.

@jansupol jansupol merged commit cbd9040 into eclipse-ee4j:master Apr 24, 2019
@alawrenc
Copy link

I know I'm coming in way too late, but doesn't this integration test demonstrate exactly the wrong behavior for exception mappers?
Per the jax-rs spec, post-request filters (including exception mappers by my understanding) are sorted in reverse order. https://docs.oracle.com/javaee/7/api/javax/ws/rs/Priorities.html
So a higher priority value should take precedence in this case

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.

4 participants