[RESTEASY-2026]: Priority override MUST be taken into account when registering

[NicoNes]

No description provided.

[NicoNes]

@asoldano With the changes I did in this PR, I had to modify some test classes you wrote for unit test ProviderContextInjectionTest:
Both ContextResolver ProviderContextInjectionTextPlainEnumContextResolver and ProviderContextInjectionEnumContextResolver are providers for the same type ProviderContextInjectionEnumProvider but with different media types.
Acording to the spec (4.3.1) _"a provider that explicitly lists a media type is sorted before a provider that lists */*." So in your test case ProviderContextInjectionTest.isRegisteredWildCardContextResolverTest() ProviderContextInjectionTextPlainEnumContextResolver should be the provider that will be selected and used.
It used to work before because of the loop I removed https://github.com/resteasy/Resteasy/pull/1709/files#diff-be425f1e75ab8a4f6b0a674dcc8d5136L1245 that was reversing the sorted list of ContextResolver.

[asoldano]

@NicoNes thanks for your PR, can you please look at the conflict and resolve it? (master recently went through a bit of refactoring, sorry)
@ronsigal can you please have a look at this PR and perhaps check the TCK? I believe the mentioned test was written by QA (I think I added this stuff copying from the internal QA repo back at the time of 3.1) to cover some CTS checks.. so better verifying.

[NicoNes]

Hi @asoldano,

Yes sure. I've just fixed the conflicts, everything is OK now.

[ronsigal]

@asoldano, I'll take a look.

[asoldano]

@ronsigal any update here?

[ronsigal]

@asoldano, Ooops. Will do.

[ronsigal]

Hi @NicoNes.

I'm trying to understand why you took this section out. I don't see any broken tests, but could you explain your thinking?

Thanks,
Ron

[NicoNes]

Hey @ronsigal ,

Actually there were two problems with the old code:

• The priority was correctly computed by getPriority(...) method at lines https://github.com/resteasy/Resteasy/pull/1709/files#diff-c684d1c557d88c213e775fefc4fb0f5eR1535 and https://github.com/resteasy/Resteasy/pull/1709/files#diff-c684d1c557d88c213e775fefc4fb0f5eR1768 but it was not used by addContextResolver(...). That's what I fixed by adding priority argument to addContextResolver(...) signature.
  This being done, I was expecting my PriorityTest.testContextResolverPriorityOverride() to pass since the priority I provided was now used by ResteasyProviderFactoryImpl.SortedKey to sort registered ContextResolver. Unfortunately the test was still failling.
• Then I realize that it was because of the if section at line https://github.com/resteasy/Resteasy/pull/1709/files#diff-c684d1c557d88c213e775fefc4fb0f5eL1165 which was turning, the already well sorted list, upside down. Thats why I remove this if section and only take the else bloc.

So to be precise PriorityTest.testContextResolverPriorityOverride() test should work with only fixing the second problem (the upside down): without fixing the first problem both ContextResolver are registered with the same priority (5000) so the first one registered will be used first.

To really test the first problem I should enrich the test registering K first with priority 1 then O with priority 0 as I did for the others tests:

 webTarget.register(new ContextResolver<String>()
         {
            @Override
            public String getContext(Class<?> type)
            {
               result.append("K");
               return null;
            }
         }, 1);
         webTarget.register(new ContextResolver<String>()
         {
            @Override
            public String getContext(Class<?> type)
            {
               result.append("O");
               return null;
            }
         }, 0);

Let me know if it's clearer.

[ronsigal]

Hi @NicoNes,

re: "That's what I fixed by adding priority argument to addContextResolver(...) signature."

I took that to be an efficiency measure, since line 1118 recalculates priority.

re: "Then I realize that it was because of the if section at line https://github.com/resteasy/Resteasy/pull/1709/files#diff-c684d1c557d88c213e775fefc4fb0f5eL1165 which was turning, the already well sorted list, upside down. Thats why I remove this if section and only take the else bloc."

It's the removal of the if section that concerns me, since someone (probably Bill Burke) gave a justification for the backwards sort. On the other hand, the javadoc for Providers.getContextResolver() says

If more than one resolver matches then the list of matching resolvers is ordered with those 
with the best matching values of {@link javax.ws.rs.Produces} (x/y > x/* > */*) 
sorted first. 

The comment in ResteasyProviderFactoryImpl.getContextResolvers() seems to say that, if "*/*" is passed in, a resolver with @produces("*/*") would be preferred over one with @produces("text/plain"), which contradicts the javadoc rule.

So I guess I'm willing to say that the TCK issue is no longer applicable.

-Ron

[NicoNes]

Hey @ronsigal

I took that to be an efficiency measure, since line 1118 recalculates priority.

Yep, it was calculated at this line but without considering user priorityOverride

It's the removal of the if section that concerns me, since someone (probably Bill Burke) gave a justification for the backwards sort. On the other hand, the javadoc for Providers.getContextResolver() says

Actually I was not really confident neither when I removed it until I saw the spec section you talked about. Then I made the change and also review test ProviderContextInjectionTest.isRegisteredWildCardContextResolverTest(), modifying both ProviderContextInjectionTextPlainEnumContextResolver and ProviderContextInjectionEnumContextResolver since the wrong ContextResolver was expected in regards to the spec.

So I guess I'm willing to say that the TCK issue is no longer applicable.

Unless it has already been done, is it possible to run the TCK tests to be sure ?

[marekkopecky]

I believe the mentioned test was written by QA

This test has been originally written by Bill here

[ronsigal]

@NicoNes, for legal reasons, I'm not supposed to talk about the TCK publicly. But I can say that your changes are correct. Thank you, as always.