I'm inclined to agree, but glad I proposed the PR. Its a shame that
the org.apache.cxf.jaxrs.provider.ProviderFactory.MessageBodyReaderComparator
and org.apache.cxf.jaxrs.provider.ProviderFactory.MessageBodyWriterComparator
in CXF are private, otherwise we could most likely call those directly.
I did try a straight comparator on the @Priority value directly, which did
work for Michel's case, but lead to a number of test failures across the
openejb-cxf-rs module, the worst of which seemed to be related to malformed
JSON in the Johnzon provider due to things happening in the wrong order.
Doing this (ordering by class name as well as priority), the results are
less bad:
private static final class PriorityProviderComparator implements
Comparator<ProviderInfo<?>> {
@Override
public int compare(final ProviderInfo<?> o1, final ProviderInfo<?>
o2) {
final int p1 = getPriority(o1.getProvider().getClass());
final int p2 = getPriority(o2.getProvider().getClass());
final int compare = Integer.compare(p1, p2);
if (compare != 0) {
return compare;
}
return
o1.getProvider().getClass().getName().compareTo(o2.getProvider().getClass().getName());
}
}
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] AdvancedProviderConfigTest.check:103 expected:<[tru]e> but
was:<[fals]e>
[ERROR] CDIProviderTest.isCdi:74 expected:<[Oh Yeah!]> but was:<[failed]>
[ERROR] EJBProviderTest.isEJB:66 expected:<[Oh Yeah!]> but was:<[failed]>
[INFO]
[ERROR] Tests run: 106, Failures: 3, Errors: 0, Skipped: 1
I suspect this is more down to luck as opposed to being a good solution
though.
Michel - I'll have a go at pushing a PR to CXF for this one this evening.
Cheers
Jon
On Thu, Jul 27, 2017 at 5:39 AM, Romain Manni-Bucau <[email protected]>
wrote:
> Hi Jon
>
> If we do it this way - jaxrs 2.1 - I d be to PR cxf instead of tomee. We
> fork too much code to stay sane here.
>
> What about just sorting by priority blindly? Cxf will handle the q and type
> sorting anyway, no?
>
> Le 26 juil. 2017 22:18, "Jonathan Gallimore" <[email protected]
> >
> a écrit :
>
> > Hi,
> >
> > I've had a go at this: https://github.com/apache/tomee/pull/96
> >
> > This creates a default comparator when no setting for
> > cxf.jaxrs.provider-comparator is specified. This will order by priority
> > first, but will still incorporate the default CXF sorting where
> priorities
> > are the same for backwards compatibility.
> >
> > Michel's test case appears to work ok with this patch.
> >
> > Any feedback would be most welcome.
> >
> > Many thanks
> >
> > Jon
> >
> > On Tue, Jul 25, 2017 at 5:02 PM, Jonathan Gallimore <
> > [email protected]> wrote:
> >
> > > Yep - that's what I had in my mind. Makes sense if a comparator is
> > > specified, we should use that instead of the @Priority ordering.
> > >
> > > Jon
> > >
> > > On Tue, Jul 25, 2017 at 4:59 PM, Romain Manni-Bucau <
> > [email protected]
> > > > wrote:
> > >
> > >> +1
> > >>
> > >> What's do we want to do? Looks like a EE 8 feature we can eagerly
> > support
> > >> as a default comparator (if the comparator is set we must bypass it to
> > >> respect the comparator IMHO)
> > >>
> > >>
> > >> Romain Manni-Bucau
> > >> @rmannibucau <https://twitter.com/rmannibucau> | Blog
> > >> <https://blog-rmannibucau.rhcloud.com> | Old Blog
> > >> <http://rmannibucau.wordpress.com> | Github <
> > >> https://github.com/rmannibucau> |
> > >> LinkedIn <https://www.linkedin.com/in/rmannibucau> | JavaEE Factory
> > >> <https://javaeefactory-rmannibucau.rhcloud.com>
> > >>
> > >> 2017-07-25 17:03 GMT+02:00 Jonathan Gallimore <
> > >> [email protected]>
> > >> :
> > >>
> > >> > I'm happy to take a look at this one, unless someone else is already
> > >> > working on it?
> > >> >
> > >> > Jon
> > >> >
> > >>
> > >
> > >
> >
>
