Broken ParamConverterProvider ordering in 2.26

[jerseyrobot]

It looks like there are a few issues regarding provider ordering in 2.26. Although I think older versions are also affected.

I created a minimal sample app to reproduce the issue: https://github.com/chkal/jersey-prio

Basically I'm trying to register a custom ParamConverter which overrides the default converter for the data type java.lang.Integer.

First issue:

The first issue is that whether the custom provider or the default provider is used randomly changes between deployments. That's caused by the use of a HashSet instead of a LinkedHashSet in ParamConverterFactory. My pull request #3669 should fix this.

Second issue

The second problem is more difficult to reason about. After merging #3669 you will see that Jersey will always pick the default provider and ignores the custom provider deployed with the app.

I'll try to explain what I debugged so far. Actually the code in the ParamConverterFactory tries to handle the case of custom providers overriding default providers correctly:

https://github.com/jersey/jersey/blob/4ecf20c7585abfe1bd2c312d890943e884a9eb3b/core-server/src/main/java/org/glassfish/jersey/server/internal/inject/ParamConverterFactory.java#L74-L81

The weird thing is that providers deployed with the app always end up in the providers set in the constructor and NOT in the customProviders set. The ParamConverterFactory is created here:

https://github.com/jersey/jersey/blob/4ecf20c7585abfe1bd2c312d890943e884a9eb3b/core-server/src/main/java/org/glassfish/jersey/server/internal/inject/ParamExtractorConfigurator.java#L68-L71

If you dig deeper into the Provider.getProviders() and Provider.getCustomProviders() code, you will see that getCustomProviders() will look for providers with a qualifier Custom. I'm not sure under which circumstances providers get this qualifier.

Also noteworthy: Only getCustomProviders() seems to apply the ordering algorithm specified in JAX-RS 2.1 (see jax-rs/api#538). As custom providers aren't found by this method, the ordering algorithm isn't applied at all.

I hope this description helps to debug the problem. Let me know if there is anything I can do to help with this.

[jerseyrobot]

• Issue Imported From: https://github.com/jersey/jersey/issues/3670
• Original Issue Raised By:@chkal
• Original Issue Assigned To: @jansupol

[chkal]

Hi all. As open merge requests haven't been migrated from the jersey/jersey repository, I create a new pull request #3955 which contains the changes from jersey/jersey#3669. This pull request fixes the first issue described above.

It would be great to get some feedback from the Jersey team regarding the second issue.

[jansupol]

Thanks. As there historically were PRs in Jersey (and other projects) that were years old and possibly were no longer valid, the decision was not to transfer them. PR needed by the community should have been recreated in the new repos.

[chkal]

@jansupol Sure, starting from the scratch makes sense, especially if there are many old pull request!

Any chance that somebody could take a look at the "second issue" described above? I'm pretty sure that this is a compatibility issue regarding the @Priority ordering requirement for providers which has been introduced in JAX-RS 2.1. I would love to hear other thoughts on this.

[jansupol]

@chkal What providers do you describe here? Is it related to any provider ordering or is it ParamConverterProvider specific only? Jersey 2.26 changed the behaviour of @Priority, previous versions could behave differently.

[chkal]

@jansupol I ran into issues with both ParamConverterProvider and ExceptionMapper. In both cases one provider was packaged in WEB-INF/classes and the other one in a JAR placed in WEB-INF/lib (or was part of Jersey itself). So maybe this is the cause for the weird behavior described above!?

[chkal]

@jansupol I spent some more time debugging this issue. I'm still able to reproduce it with Eclipse Glassfish 5.1.0 which includes Eclipse Jersey 2.28. See the following description for more details.

Jersey looks up providers (like ExceptionMapper in my case) like this:

Providers.getAllServiceHolders(injectionManager, ExceptionMapper.class);

This lookup method works like this:

jersey/core-common/src/main/java/org/glassfish/jersey/internal/inject/Providers.java

Lines 286 to 301 in 374e90f

	public static <T> Collection<ServiceHolder<T>> getAllServiceHolders(InjectionManager injectionManager, Class<T> contract) {
	List<ServiceHolder<T>> providers = getServiceHolders(injectionManager,
	contract,
	Comparator.comparingInt(Providers::getPriority),
	CustomAnnotationLiteral.INSTANCE);
	providers.addAll(getServiceHolders(injectionManager, contract));
	LinkedHashSet<ServiceHolder<T>> providersSet = new LinkedHashSet<>();
	for (ServiceHolder<T> provider : providers) {
	if (!providersSet.contains(provider)) {
	providersSet.add(provider);
	}
	}
	return providersSet;
	}

As you see, this method actually performs two lookups:

jersey/core-common/src/main/java/org/glassfish/jersey/internal/inject/Providers.java

Lines 287 to 290 in 374e90f

	List<ServiceHolder<T>> providers = getServiceHolders(injectionManager,
	contract,
	Comparator.comparingInt(Providers::getPriority),
	CustomAnnotationLiteral.INSTANCE);

This code looks up provider implementations having the Custom qualifier. Please note that the supplied comparator will order the matching providers correctly according to their priority.

Then there is a second lookup:

jersey/core-common/src/main/java/org/glassfish/jersey/internal/inject/Providers.java

Line 291 in 374e90f

providers.addAll(getServiceHolders(injectionManager, contract));

My understanding is, that this lookup adds all other providers (without the Custom qualifier) to the list. Please note that these providers are NOT ordered according to their priority.

In my case I'm having two ExceptionMapper implementations for the same exception type. One implementation is located in WEB-INF/classes, the other one in a library located in WEB-INF/lib.

The interesting fact is that both are NOT found in the first lookup (which would order them correctly), but instead found in the second lookup (which doesn't do any ordering).

I'm not familiar with how the component model and discovery works for Jersey in detail, but to me it looks like my implementations are not qualified with Custom and therefore treated as Jersey-internal providers which doesn't seem to get ordered correctly according to @Priority.

Sorry for my lengthy description, but I want to share as much of my findings as possible. Does this help in any way? Any idea what is causing this? Any hint on how to continue with this?

[chkal]

I'm currently working on finishing the TCK for JSR-371 and I'm getting more and tests failing on Glassfish because of this issue. Is there any chance that somebody from the Jersey team could have a look at my findings posted in the last comment and share some thoughts?

[senivam]

for now I've discovered that the only difference which makes it work (on GF) is removing the root / from the @ Path in MyResource. So, the resource looks like

`
@ Path("param/{value}")
@ValidateOnExecution(type = ExecutableType.NONE)
public class MyResource {

@PathParam("value")
private Integer value;

@GET
public String get() {
    return "Value injected via @PathParam: " + value;
}

}`

[chkal]

@senivam Thanks a lot for looking into this. Does this really change the behavior I described above (custom provider not qualified with @Custom)? I'm just asking because I was seeing very unreproducible behavior and sometimes even changing a log message (and therefore recompiling the class files) resulted in a different ordering.

[senivam]

For now, as I was looking into this more deeply, when you take clean Tomcat and add required dependencies to your reproducer, it works for the described case like a charm (independently from a value in the Path annotation).

When you rewrite the reproducer into Jersey's test and/or example it works as well.

Further more, the provider is located in correct Custom providers map (without any custom annotation). So, it comes the first in the ordering and all works.

But when you run the reproducer in Glassfish (5.1.0 or 5.0.1) where Jersey is bundled along with another modules, something brakes Jersey's behavior and Custom provider gets to the end of internal providers map which is wrong (does not depend on a value of the Path annotation).

So, I will continue investigation what causes this behavior.

[chkal]

Thanks a lot for the update and for working on this. This is really a weird behavior. But I'm happy to hear that you are able to reproduce it.

[senivam]

I suppose it is solved. I've found the reason. I've submited a PR for that whith your reproducer as integration test. If you do not mind to use your code inside Jersey...

[chkal]

Sure, feel free to use the code as you like. Thanks a lot for your help.