Improve DOMMountPointServiceImpl error message

The checkState() we perform is rather ugly, as it does not provide
enough diagnostic information. We already have a nicer checkState()
in the register() path. Extract it to a common method and reuse it from
both places.

Also improve safety by checking for null path before anything and also
take the lock to ensure serialization with others.

JIRA: NETCONF-621
Change-Id: I55845f9bbd7b3f4dbfa06a91724abcb601dc4f7f
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/DOMMountPointServiceImpl.java b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/DOMMountPointServiceImpl.java
index 80d0c80ee34a53e1420714bce6b476188dbb5b57..843edcec4a03e6851d4379a2914b431d598b4599 100644 (file)
--- a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/DOMMountPointServiceImpl.java
+++ b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/DOMMountPointServiceImpl.java
@@ -48,8 +48,8 @@ public final class DOMMountPointServiceImpl implements DOMMountPointService {
     }
 
     @Override
-    public DOMMountPointBuilder createMountPoint(final YangInstanceIdentifier path) {
-        checkState(!mountPoints.containsKey(path), "Mount point already exists");
+    public synchronized DOMMountPointBuilder createMountPoint(final YangInstanceIdentifier path) {
+        checkNotExists(path, mountPoints.get(requireNonNull(path)));
         return new DOMMountPointBuilderImpl(path);
     }
 
@@ -74,8 +74,7 @@ public final class DOMMountPointServiceImpl implements DOMMountPointService {
     private ObjectRegistration<DOMMountPoint> registerMountPoint(final SimpleDOMMountPoint mountPoint) {
         final YangInstanceIdentifier mountPointId = mountPoint.getIdentifier();
         synchronized (mountPoints) {
-            final DOMMountPoint prev = mountPoints.putIfAbsent(mountPointId, mountPoint);
-            checkState(prev == null, "Mount point %s already exists as %s", mountPointId, prev);
+            checkNotExists(mountPointId, mountPoints.putIfAbsent(mountPointId, mountPoint));
         }
         listeners.streamListeners().forEach(listener -> {
             try {
@@ -111,8 +110,11 @@ public final class DOMMountPointServiceImpl implements DOMMountPointService {
         });
     }
 
-    private final class DOMMountPointBuilderImpl implements DOMMountPointBuilder {
+    private static void checkNotExists(final YangInstanceIdentifier id, final DOMMountPoint mountPoint) {
+        checkState(mountPoint == null, "Mount point %s already exists as %s", id, mountPoint);
+    }
 
+    private final class DOMMountPointBuilderImpl implements DOMMountPointBuilder {
         private final MutableClassToInstanceMap<DOMService> services = MutableClassToInstanceMap.create();
         private final YangInstanceIdentifier path;