X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=blobdiff_plain;f=singleton-service%2Fmdsal-singleton-dom-impl%2Fsrc%2Fmain%2Fjava%2Forg%2Fopendaylight%2Fmdsal%2Fsingleton%2Fdom%2Fimpl%2FClusterSingletonServiceGroupImpl.java;h=2fae806534da672d4c6ec555ddf6353175c8c835;hb=ecdcdb340457ded6898193197e88ea33c84d58ae;hp=7570a4bbfeb3e08a19078c31af97eec5e5905db5;hpb=0f351bbc28ddf2cddfe30c8d018646d81953fa17;p=mdsal.git

diff --git a/singleton-service/mdsal-singleton-dom-impl/src/main/java/org/opendaylight/mdsal/singleton/dom/impl/ClusterSingletonServiceGroupImpl.java b/singleton-service/mdsal-singleton-dom-impl/src/main/java/org/opendaylight/mdsal/singleton/dom/impl/ClusterSingletonServiceGroupImpl.java
index 7570a4bbfe..2fae806534 100644
--- a/singleton-service/mdsal-singleton-dom-impl/src/main/java/org/opendaylight/mdsal/singleton/dom/impl/ClusterSingletonServiceGroupImpl.java
+++ b/singleton-service/mdsal-singleton-dom-impl/src/main/java/org/opendaylight/mdsal/singleton/dom/impl/ClusterSingletonServiceGroupImpl.java
@@ -22,6 +22,7 @@ import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.common.util.concurrent.SettableFuture;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -31,9 +32,8 @@ import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
 import java.util.concurrent.atomic.AtomicReference;
-import javax.annotation.CheckReturnValue;
-import javax.annotation.concurrent.GuardedBy;
-import javax.annotation.concurrent.ThreadSafe;
+import org.checkerframework.checker.lock.qual.GuardedBy;
+import org.checkerframework.checker.lock.qual.Holding;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.mdsal.eos.common.api.CandidateAlreadyRegisteredException;
 import org.opendaylight.mdsal.eos.common.api.EntityOwnershipChangeState;
@@ -44,12 +44,12 @@ import org.opendaylight.mdsal.eos.common.api.GenericEntityOwnershipListener;
 import org.opendaylight.mdsal.eos.common.api.GenericEntityOwnershipService;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonService;
 import org.opendaylight.mdsal.singleton.common.api.ClusterSingletonServiceRegistration;
-import org.opendaylight.yangtools.concepts.Path;
+import org.opendaylight.yangtools.concepts.HierarchicalIdentifier;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * Implementation of {@link ClusterSingletonServiceGroup} on top of the Entitiy Ownership Service. Since EOS is atomic
+ * Implementation of {@link ClusterSingletonServiceGroup} on top of the Entity Ownership Service. Since EOS is atomic
  * in its operation and singleton services incur startup and most notably cleanup, we need to do something smart here.
  *
  * <p>
@@ -70,8 +70,7 @@ import org.slf4j.LoggerFactory;
  * @param <G> the GenericEntityOwnershipListener type
  * @param <S> the GenericEntityOwnershipService type
  */
-@ThreadSafe
-final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends GenericEntity<P>,
+final class ClusterSingletonServiceGroupImpl<P extends HierarchicalIdentifier<P>, E extends GenericEntity<P>,
         C extends GenericEntityOwnershipChange<P, E>,  G extends GenericEntityOwnershipListener<P, C>,
         S extends GenericEntityOwnershipService<P, E, G>> extends ClusterSingletonServiceGroup<P, E, C> {
 
@@ -261,8 +260,7 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
 
     @Override
     void registerService(final ClusterSingletonServiceRegistration reg) {
-        final ClusterSingletonService service = reg.getInstance();
-        verify(identifier.equals(service.getIdentifier().getValue()));
+        final ClusterSingletonService service = verifyRegistration(reg);
         checkNotClosed();
 
         checkState(initialized, "Service group %s is not initialized yet", identifier);
@@ -280,11 +278,9 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
         reconcileState();
     }
 
-    @CheckReturnValue
     @Override
     ListenableFuture<?> unregisterService(final ClusterSingletonServiceRegistration reg) {
-        final ClusterSingletonService service = reg.getInstance();
-        verify(identifier.equals(service.getIdentifier().getValue()));
+        verifyRegistration(reg);
         checkNotClosed();
 
         verify(members.remove(reg));
@@ -305,6 +301,12 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
         return null;
     }
 
+    private ClusterSingletonService verifyRegistration(final ClusterSingletonServiceRegistration reg) {
+        final ClusterSingletonService service = reg.getInstance();
+        verify(identifier.equals(service.getIdentifier().getName()));
+        return service;
+    }
+
     private synchronized @NonNull ListenableFuture<?> destroyGroup() {
         final SettableFuture<Void> future = SettableFuture.create();
         if (!closeFuture.compareAndSet(null, future)) {
@@ -332,7 +334,7 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
 
         if (isDirty()) {
             if (!tryLock()) {
-                LOG.debug("Service group {} postponing ownership change sync");
+                LOG.debug("Service group {} postponing ownership change sync", identifier);
                 return;
             }
 
@@ -346,7 +348,7 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
      *
      * @param ownershipChange reported change
      */
-    @GuardedBy("this")
+    @Holding("this")
     private void lockedOwnershipChanged(final C ownershipChange) {
         final E entity = ownershipChange.getEntity();
         if (serviceEntity.equals(entity)) {
@@ -360,7 +362,7 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
         }
     }
 
-    @GuardedBy("this")
+    @Holding("this")
     private void cleanupCandidateOwnershipChanged(final EntityOwnershipChangeState state, final boolean jeopardy) {
         if (jeopardy) {
             switch (state) {
@@ -405,7 +407,7 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
         }
     }
 
-    @GuardedBy("this")
+    @Holding("this")
     private void serviceOwnershipChanged(final EntityOwnershipChangeState state, final boolean jeopardy) {
         if (jeopardy) {
             LOG.info("Service group {} service entity ownership uncertain", identifier);
@@ -489,6 +491,8 @@ final class ClusterSingletonServiceGroupImpl<P extends Path<P>, E extends Generi
         }
     }
 
+    @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD",
+            justification = "https://github.com/spotbugs/spotbugs/issues/811")
     private void serviceTransitionCompleted() {
         markDirty();
         if (tryLock()) {