Deprecate leak methods in DOMMountPointServiceImpl 09/77609/5
authorJakub Morvay <jmorvay@frinx.io>
Thu, 8 Nov 2018 09:41:30 +0000 (10:41 +0100)
committerJakub Morvay <jmorvay@frinx.io>
Thu, 8 Nov 2018 09:54:16 +0000 (10:54 +0100)
This deprecates DOMMountPointServiceImpl registerMountPoint() and
unregisterMountPoint() methods as they are leaks and should never have
been exposed publicly.

Change-Id: Iba5a270d15ca882eb08372f1f6911af55d298f8d
Signed-off-by: Jakub Morvay <jmorvay@frinx.io>
dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/DOMMountPointServiceImpl.java
dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/MountPointServiceTest.java

index 4c5f6152c07493ab31ebe3c4f8175784dc4a708d..ecf6f7a5c3fa7cfef0c17b299812a997e4f5b2d6 100644 (file)
@@ -12,6 +12,7 @@ import static com.google.common.base.Preconditions.checkState;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 import com.google.common.collect.MutableClassToInstanceMap;
 import java.util.HashMap;
 import java.util.Map;
@@ -53,7 +54,21 @@ public class DOMMountPointServiceImpl implements DOMMountPointService {
         return listeners.register(listener);
     }
 
+    /**
+     * Deprecated.
+     *
+     * @deprecated this method should never have been exposed publicly - registration should be done via the public
+     *             {@link #createMountPoint} interface. As such, this method expects the {@code mountPoint} part to be
+     *             of type {@link SimpleDOMMountPoint}.
+     */
+    @Deprecated
     public ObjectRegistration<DOMMountPoint> registerMountPoint(final DOMMountPoint mountPoint) {
+        Preconditions.checkArgument(mountPoint instanceof SimpleDOMMountPoint,
+                "Expected mountpoint argument to be of type SimpleDOMMountPoint");
+        return doRegisterMountPoint((SimpleDOMMountPoint) mountPoint);
+    }
+
+    private ObjectRegistration<DOMMountPoint> doRegisterMountPoint(final SimpleDOMMountPoint mountPoint) {
         final YangInstanceIdentifier mountPointId = mountPoint.getIdentifier();
         synchronized (mountPoints) {
             final DOMMountPoint prev = mountPoints.putIfAbsent(mountPointId, mountPoint);
@@ -64,15 +79,29 @@ public class DOMMountPointServiceImpl implements DOMMountPointService {
         return new AbstractObjectRegistration<DOMMountPoint>(mountPoint) {
             @Override
             protected void removeRegistration() {
-                unregisterMountPoint(getInstance().getIdentifier());
+                doUnregisterMountPoint(getInstance().getIdentifier());
             }
         };
     }
 
+    /**
+     * Unregisters mountpoint.
+     *
+     * @param mountPointId Mountpoint identifier.
+     *
+     * @deprecated this method should never have been exposed publicly - mountpoint should be unregistered by simply
+     *             closing its registration.
+     *
+     */
+    @Deprecated
     public void unregisterMountPoint(final YangInstanceIdentifier mountPointId) {
+        doUnregisterMountPoint(mountPointId);
+    }
+
+    private void doUnregisterMountPoint(final YangInstanceIdentifier mountPointId) {
         synchronized (mountPoints) {
             if (mountPoints.remove(mountPointId) == null) {
-                LOG.warn("Removed non-existend mount point {} at", mountPointId, new Throwable());
+                LOG.warn("Removing non-existent mount point {} at", mountPointId, new Throwable());
                 return;
             }
         }
@@ -118,7 +147,7 @@ public class DOMMountPointServiceImpl implements DOMMountPointService {
         public ObjectRegistration<DOMMountPoint> register() {
             checkState(mountPoint == null, "Mount point is already built.");
             mountPoint = SimpleDOMMountPoint.create(path, services,schemaContext);
-            return registerMountPoint(mountPoint);
+            return doRegisterMountPoint(mountPoint);
         }
     }
 }
index adea584bc89d04b2c7657df3bad85f903cb5d6de..1f554414710e4f1f78ed64203cf57522225cf649 100644 (file)
@@ -41,7 +41,7 @@ public class MountPointServiceTest {
     }
 
     @Test
-    public void createSimpleMountPoint() throws Exception {
+    public void createSimpleMountPoint() {
         final DOMMountPointListener listener = mock(DOMMountPointListener.class);
         doNothing().when(listener).onMountPointCreated(PATH);
         mountService.registerProvisionListener(listener);
@@ -54,23 +54,23 @@ public class MountPointServiceTest {
     }
 
     @Test
-    public void unregisterTest() throws Exception {
+    public void unregisterTest() {
         final DOMMountPointListener listener = mock(DOMMountPointListener.class);
         doNothing().when(listener).onMountPointCreated(PATH);
         doNothing().when(listener).onMountPointRemoved(PATH);
         final DOMMountPointServiceImpl service = new DOMMountPointServiceImpl();
         service.registerProvisionListener(listener);
-        service.createMountPoint(PATH).register();
+        final ObjectRegistration<DOMMountPoint> mountPointReg = service.createMountPoint(PATH).register();
 
         assertTrue(service.getMountPoint(PATH).isPresent());
 
-        service.unregisterMountPoint(PATH);
+        mountPointReg.close();
 
         assertFalse(service.getMountPoint(PATH).isPresent());
     }
 
     @Test
-    public void mountRegistrationTest() throws Exception {
+    public void mountRegistrationTest() {
         final DOMMountPointBuilder mountBuilder = mountService.createMountPoint(PATH);
         final ObjectRegistration<DOMMountPoint> objectRegistration = mountBuilder.register();
 
@@ -83,7 +83,7 @@ public class MountPointServiceTest {
     }
 
     @Test
-    public void mountBuilderTest() throws Exception {
+    public void mountBuilderTest() {
         final DOMMountPointBuilderImpl mountBuilder = (DOMMountPointBuilderImpl) mountService.createMountPoint(PATH);
         mountBuilder.register();