#6 - add a priority field

[dmlloyd]

(Fixes issue #6)

[jglick]

Seems not very useful, because priorities would only be compared within a single JAR.

Referring to the more sophisticated implementation of the idea behind this library in the NetBeans Platform, ServiceProvider.position works (across JARs) only because MetaInfServicesLookup interprets this optional metadata. java.util.ServiceLoader does not.

In general, it is more fruitful to define richer semantics on the service interface itself: minimally a position/priority attribute, but often a filter allowing it to specify what conditions it is suited to.

[dmlloyd]

It is useful for us. We are implementing providers for an interface we do not control, and our class loading is isolated with a clear and predictable dependency order. Adding in ordering solves a real world problem for us and allows us to use this project instead of coding it by hand.

This isn't a hypothetical.

[jglick]

Not saying it “useless”, just not as useful as it would sound (to someone coming across this feature), because you cannot force the caller to see one impl from jar A, then an impl from jar B, then another impl from jar A. In other words, you cannot freely refactor code but need to consider where it is being compiled.

So if the library is to have this restricted functionality it must minimally document that restriction. Ideally it would actually enforce it. For example,

package jarA.p1;
import …;
@MetaInfServices(before=FallbackImpl.class)
public class QuickImpl implements SomeService {…}

package jarA.p2;
import …;
@MetaInfServices(after=QuickImpl.class)
public class FallbackImpl implements SomeService {…}

would statically ensure that the ordering really works as written, by making you write the doubly-linked list within a single source tree.

[dmlloyd]

Maybe. Experience has taught me that simplicity is more important in this sort of case though. It's simpler for me to just use an int field, and push up a -jboss-1 release to our Maven repo, and not spend any more time dickering about subtle ideology.

[kohsuke]

I'm bit more sympathetic to this use case than Jesse, for I believe in "this may not be perfect but it does what it does and that's useful for me" philosophy.

But the code as it is written breaks down with incremental compilation. The priority value is not remembered once written to the service maniest entry, so when it's read back that original priority is lost.

(Obviously, Jesse is right that ServiceLoader generally doesn't commit to anything about loading order, and any "proper" ordering semantics should be defined above ServiceLoader mechanism.)

So I'm closing this PR with regret.

[dmlloyd]

FWIW I have fixed this locally by writing the priority out as a comment. The better solution is still to do this in one pass though.

[jglick]

I have fixed this locally by writing the priority out as a comment

Do you have that available as an add-on commit?

I would still recommend that priority() sternly document its limitation, and that a test (using Hickory, or com.google.auto:auto-common) verify proper behavior during incremental compilation.

[jglick]

Possibly this whole project should be deprecated in favor of com.google.auto.service:auto-service.