Bug 563987 - Handle service rankings
The implementation did not track service ranking. If rankings changed
such that the cached service is no longer the highest ranked the
implementation would keep returning the lower ranked instance.
Change-Id: I086ed3e9b7f9ca2e2005cb304b3c8e8348c9be1a
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
diff --git a/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/ServiceCallerTest.java b/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/ServiceCallerTest.java
index b77e5bf..1be98fb 100644
--- a/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/ServiceCallerTest.java
+++ b/bundles/org.eclipse.equinox.common.tests/src/org/eclipse/equinox/common/tests/ServiceCallerTest.java
@@ -13,7 +13,9 @@
*******************************************************************************/
package org.eclipse.equinox.common.tests;
+import static java.util.Collections.singletonMap;
import static org.junit.Assert.assertNotEquals;
+import static org.osgi.framework.FrameworkUtil.asDictionary;
import java.io.IOException;
import java.util.Arrays;
@@ -29,6 +31,7 @@
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import org.osgi.framework.BundleException;
+import org.osgi.framework.Constants;
import org.osgi.framework.FrameworkUtil;
import org.osgi.framework.InvalidSyntaxException;
import org.osgi.framework.ServiceFactory;
@@ -207,6 +210,64 @@
}
}
+ public void testRank() {
+ Bundle bundle = FrameworkUtil.getBundle(ServiceCallerTest.class);
+ assertNotNull("Test only works under an OSGi runtime", bundle);
+ BundleContext context = bundle.getBundleContext();
+ ServiceExample s1 = new ServiceExample();
+ ServiceExample s2 = new ServiceExample();
+ ServiceExample s3 = new ServiceExample();
+ ServiceRegistration<IServiceExample> reg1 = null;
+ ServiceRegistration<IServiceExample> reg2 = null;
+ ServiceRegistration<IServiceExample> reg3 = null;
+ try {
+ reg1 = context.registerService(IServiceExample.class, s1,
+ asDictionary(singletonMap(Constants.SERVICE_RANKING, 1)));
+ reg2 = context.registerService(IServiceExample.class, s2,
+ asDictionary(singletonMap(Constants.SERVICE_RANKING, 2)));
+ reg3 = context.registerService(IServiceExample.class, s3,
+ asDictionary(singletonMap(Constants.SERVICE_RANKING, 3)));
+ ServiceCaller<IServiceExample> caller = new ServiceCaller<>(getClass(), IServiceExample.class);
+
+ assertHighestRankedCalled(caller, s3, s2, s1, reg3, reg2, reg1);
+
+ reg1.setProperties(asDictionary(singletonMap(Constants.SERVICE_RANKING, 10)));
+ assertHighestRankedCalled(caller, s1, s2, s3, reg1, reg2, reg3);
+
+ reg2.setProperties(asDictionary(singletonMap(Constants.SERVICE_RANKING, 10)));
+ reg1.setProperties(asDictionary(singletonMap(Constants.SERVICE_RANKING, 11)));
+ assertHighestRankedCalled(caller, s1, s2, s3, reg1, reg2, reg3);
+
+ reg1.setProperties(asDictionary(singletonMap(Constants.SERVICE_RANKING, 1)));
+ assertHighestRankedCalled(caller, s2, s1, s3, reg2, reg1, reg3);
+ } finally {
+ if (reg1 != null) {
+ reg1.unregister();
+ }
+ if (reg2 != null) {
+ reg2.unregister();
+ }
+ if (reg3 != null) {
+ reg3.unregister();
+ }
+ }
+ }
+
+ private void assertHighestRankedCalled(ServiceCaller<IServiceExample> caller, ServiceExample highest,
+ ServiceExample lower1, ServiceExample lower2, ServiceRegistration<IServiceExample> highestReg,
+ ServiceRegistration<IServiceExample> lower1Reg, ServiceRegistration<IServiceExample> lower2Reg) {
+ assertTrue("Did not call service.", caller.call(IServiceExample::call));
+ assertTrue("Highest Ranked not called.", highest.called);
+ assertFalse("Lower1 called.", lower1.called);
+ assertFalse("Lower2 called.", lower2.called);
+ assertNotNull("No users of highest.", highestReg.getReference().getUsingBundles());
+ assertNull("Users of lower ranked.", lower1Reg.getReference().getUsingBundles());
+ assertNull("Users of lower ranked.", lower2Reg.getReference().getUsingBundles());
+ highest.called = false;
+ lower1.called = false;
+ lower2.called = false;
+ }
+
interface IServiceExample {
void call();
}
diff --git a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ServiceCaller.java b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ServiceCaller.java
index 37f7570..bd472f5 100644
--- a/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ServiceCaller.java
+++ b/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/ServiceCaller.java
@@ -103,15 +103,25 @@
return new ServiceCaller<>(caller, serviceType).getCallUnget(consumer);
}
+ static int getRank(ServiceReference<?> ref) {
+ Object rank = ref.getProperty(Constants.SERVICE_RANKING);
+ if (rank instanceof Integer) {
+ return ((Integer) rank).intValue();
+ }
+ return 0;
+ }
+
class ReferenceAndService implements SynchronousBundleListener, ServiceListener {
final BundleContext context;
final ServiceReference<Service> ref;
final Service instance;
+ final int rank;
public ReferenceAndService(final BundleContext context, ServiceReference<Service> ref, Service instance) {
this.context = context;
this.ref = ref;
this.instance = instance;
+ this.rank = getRank(ref);
}
void unget() {
@@ -126,17 +136,28 @@
@Override
public void bundleChanged(BundleEvent e) {
if (bundle.equals(e.getBundle()) && e.getType() == BundleEvent.STOPPING) {
- untrack();
+ unget();
}
}
@Override
public void serviceChanged(ServiceEvent e) {
- if (e.getType() == ServiceEvent.UNREGISTERING) {
- untrack();
- }
- if (filter != null && e.getType() == ServiceEvent.MODIFIED_ENDMATCH) {
- untrack();
+ if (e.getServiceReference().equals(ref)) {
+ if (e.getType() == ServiceEvent.UNREGISTERING) {
+ unget();
+ } else if (filter != null && e.getType() == ServiceEvent.MODIFIED_ENDMATCH) {
+ unget();
+ } else if (e.getType() == ServiceEvent.MODIFIED) {
+ if (getRank(ref) != rank) {
+ // rank changed; untrack to force a new ReferenceAndService with new rank
+ unget();
+ }
+ }
+ } else if (e.getType() == ServiceEvent.MODIFIED) {
+ if (getRank(e.getServiceReference()) > rank) {
+ // Another service with higher rank is available
+ unget();
+ }
}
}
@@ -144,18 +165,19 @@
Optional<ReferenceAndService> track() {
try {
ServiceCaller.this.service = this;
- // Filter specific to this service reference ID
context.addServiceListener(this, "(&" //$NON-NLS-1$
+ "(objectClass=" + serviceType.getName() + ")" // //$NON-NLS-1$ //$NON-NLS-2$
- + "(service.id=" + ref.getProperty(Constants.SERVICE_ID) + ")" // //$NON-NLS-1$ //$NON-NLS-2$
+ (filter == null ? "" : filter) // //$NON-NLS-1$
+ ")"); //$NON-NLS-1$
context.addBundleListener(this);
- if (ref.getBundle() == null || context.getBundle() == null && ServiceCaller.this.service == this) {
+ if ((ref.getBundle() == null || context.getBundle() == null) && ServiceCaller.this.service == this) {
// service should have been untracked but we may have missed the event
// before we could added the listeners
- untrack();
- return Optional.empty();
+ unget();
+ }
+ if (getRank(ref) != rank) {
+ // ranking has changed; unget to force reget in case the ranking is not the highest
+ unget();
}
} catch (InvalidSyntaxException e) {
// really should never happen with our own filter above.
@@ -164,8 +186,12 @@
} catch (IllegalStateException e) {
// bundle was stopped before we could get listeners added/removed
ServiceCaller.this.service = null;
- return Optional.empty();
}
+ // Note that we always return this ReferenceAndService
+ // even for cases where the instance was unget
+ // It is way complicated to try again and
+ // even if we did the returned value can become
+ // stale right after return.
return Optional.of(this);
}
@@ -247,7 +273,9 @@
* <ul>
* <li>The {@link #unget()} method is called.</li>
* <li>The service is unregistered.</li>
+ * <li>The service properties change such that this {@code ServiceCaller} filter no longer matches.
* <li>The caller bundle is stopped.</li>
+ * <li>The service rankings have changed.</li>
* </ul>
*
* After one of these conditions occur subsequent calls to this method will try to acquire the
