Fix NPE when creating a producer after shard was unregistered
[mdsal.git] / dom / mdsal-dom-broker / src / main / java / org / opendaylight / mdsal / dom / broker / ShardedDOMDataTree.java
index 9544cbd114d1f8f8e9622a51a5ea7ee00e3051e5..2ef26a9dc34d7ffa581ec0853f4a51b9fc9951a0 100644 (file)
@@ -8,12 +8,12 @@
 package org.opendaylight.mdsal.dom.broker;
 
 import com.google.common.base.Preconditions;
+import com.google.common.collect.Iterables;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Set;
-import java.util.TreeMap;
 import javax.annotation.concurrent.GuardedBy;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeListener;
@@ -23,6 +23,8 @@ import org.opendaylight.mdsal.dom.api.DOMDataTreeService;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeShard;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeShardingConflictException;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeShardingService;
+import org.opendaylight.mdsal.dom.spi.DOMDataTreePrefixTable;
+import org.opendaylight.mdsal.dom.spi.DOMDataTreePrefixTableEntry;
 import org.opendaylight.mdsal.dom.spi.store.DOMStoreTreeChangePublisher;
 import org.opendaylight.yangtools.concepts.AbstractListenerRegistration;
 import org.opendaylight.yangtools.concepts.ListenerRegistration;
@@ -33,9 +35,9 @@ public final class ShardedDOMDataTree implements DOMDataTreeService, DOMDataTree
     private static final Logger LOG = LoggerFactory.getLogger(ShardedDOMDataTree.class);
 
     @GuardedBy("this")
-    private final ShardingTable<ShardRegistration<?>> shards = ShardingTable.create();
+    private final DOMDataTreePrefixTable<ShardRegistration<?>> shards = DOMDataTreePrefixTable.create();
     @GuardedBy("this")
-    private final Map<DOMDataTreeIdentifier, DOMDataTreeProducer> idToProducer = new TreeMap<>();
+    private final DOMDataTreePrefixTable<DOMDataTreeProducer> producers = DOMDataTreePrefixTable.create();
 
 
     void removeShard(final ShardRegistration<?> reg) {
@@ -59,7 +61,12 @@ public final class ShardedDOMDataTree implements DOMDataTreeService, DOMDataTree
     }
 
     @Override
-    public <T extends DOMDataTreeShard> ListenerRegistration<T> registerDataTreeShard(final DOMDataTreeIdentifier prefix, final T shard) throws DOMDataTreeShardingConflictException {
+    public <T extends DOMDataTreeShard> ListenerRegistration<T> registerDataTreeShard(final DOMDataTreeIdentifier prefix, final T shard, final DOMDataTreeProducer producer) throws DOMDataTreeShardingConflictException {
+
+        final DOMDataTreeIdentifier firstSubtree = Iterables.getOnlyElement(((ShardedDOMDataTreeProducer) producer).getSubtrees());
+        Preconditions.checkArgument(firstSubtree != null, "Producer that is used to verify namespace claim can only claim a single namespace");
+        Preconditions.checkArgument(prefix.equals(firstSubtree), "Trying to register shard to a different namespace than the producer has claimed");
+
         final ShardRegistration<T> reg;
         final ShardRegistration<?> parentReg;
 
@@ -69,7 +76,7 @@ public final class ShardedDOMDataTree implements DOMDataTreeService, DOMDataTree
              * and if it exists, check if its registration prefix does not collide with
              * this registration.
              */
-            final ShardingTableEntry<ShardRegistration<?>> parent = shards.lookup(prefix);
+            final DOMDataTreePrefixTableEntry<ShardRegistration<?>> parent = shards.lookup(prefix);
             if (parent != null) {
                 parentReg = parent.getValue();
                 if (parentReg != null && prefix.equals(parentReg.getPrefix())) {
@@ -86,7 +93,7 @@ public final class ShardedDOMDataTree implements DOMDataTreeService, DOMDataTree
 
             shards.store(prefix, reg);
 
-            // FIXME: update any producers/registrations
+            ((ShardedDOMDataTreeProducer) producer).subshardAdded(Collections.singletonMap(prefix, shard));
         }
 
         // Notify the parent shard
@@ -99,30 +106,26 @@ public final class ShardedDOMDataTree implements DOMDataTreeService, DOMDataTree
 
     @GuardedBy("this")
     private DOMDataTreeProducer findProducer(final DOMDataTreeIdentifier subtree) {
-        for (final Entry<DOMDataTreeIdentifier, DOMDataTreeProducer> e : idToProducer.entrySet()) {
-            if (e.getKey().contains(subtree)) {
-                return e.getValue();
-            }
-        }
 
+        final DOMDataTreePrefixTableEntry<DOMDataTreeProducer> producerEntry = producers.lookup(subtree);
+        if (producerEntry != null) {
+            return producerEntry.getValue();
+        }
         return null;
     }
 
     synchronized void destroyProducer(final ShardedDOMDataTreeProducer producer) {
         for (final DOMDataTreeIdentifier s : producer.getSubtrees()) {
-            final DOMDataTreeProducer r = idToProducer.remove(s);
-            if (!producer.equals(r)) {
-                LOG.error("Removed producer {} on subtree {} while removing {}", r, s, producer);
-            }
+            producers.remove(s);
         }
     }
 
     @GuardedBy("this")
-    private DOMDataTreeProducer createProducer(final Map<DOMDataTreeIdentifier, DOMDataTreeShard> shardMap) {
+    private DOMDataTreeProducer createProducer(final Collection<DOMDataTreeIdentifier> subtrees, final Map<DOMDataTreeIdentifier, DOMDataTreeShard> shardMap) {
         // Record the producer's attachment points
-        final DOMDataTreeProducer ret = ShardedDOMDataTreeProducer.create(this, shardMap);
-        for (final DOMDataTreeIdentifier s : shardMap.keySet()) {
-            idToProducer.put(s, ret);
+        final DOMDataTreeProducer ret = ShardedDOMDataTreeProducer.create(this, subtrees, shardMap);
+        for (final DOMDataTreeIdentifier subtree : subtrees) {
+            producers.store(subtree, ret);
         }
 
         return ret;
@@ -133,15 +136,18 @@ public final class ShardedDOMDataTree implements DOMDataTreeService, DOMDataTree
         Preconditions.checkArgument(!subtrees.isEmpty(), "Subtrees may not be empty");
 
         final Map<DOMDataTreeIdentifier, DOMDataTreeShard> shardMap = new HashMap<>();
-        for (final DOMDataTreeIdentifier s : subtrees) {
+        for (final DOMDataTreeIdentifier subtree : subtrees) {
             // Attempting to create a disconnected producer -- all subtrees have to be unclaimed
-            final DOMDataTreeProducer producer = findProducer(s);
-            Preconditions.checkArgument(producer == null, "Subtree %s is attached to producer %s", s, producer);
+            final DOMDataTreeProducer producer = findProducer(subtree);
+            Preconditions.checkArgument(producer == null, "Subtree %s is attached to producer %s", subtree, producer);
 
-            shardMap.put(s, shards.lookup(s).getValue().getInstance());
+            final DOMDataTreePrefixTableEntry<ShardRegistration<?>> possibleShardReg = shards.lookup(subtree);
+            if (possibleShardReg != null && possibleShardReg.getValue() != null) {
+                shardMap.put(subtree, possibleShardReg.getValue().getInstance());
+            }
         }
 
-        return createProducer(shardMap);
+        return createProducer(subtrees, shardMap);
     }
 
     synchronized DOMDataTreeProducer createProducer(final ShardedDOMDataTreeProducer parent, final Collection<DOMDataTreeIdentifier> subtrees) {
@@ -152,7 +158,7 @@ public final class ShardedDOMDataTree implements DOMDataTreeService, DOMDataTree
             shardMap.put(s, shards.lookup(s).getValue().getInstance());
         }
 
-        return createProducer(shardMap);
+        return createProducer(subtrees, shardMap);
     }
 
     @Override
@@ -165,16 +171,16 @@ public final class ShardedDOMDataTree implements DOMDataTreeService, DOMDataTree
                 ShardedDOMDataTreeListenerContext.create(listener, subtrees, allowRxMerges);
         try {
             // FIXME: Add attachment of producers
-            for (DOMDataTreeProducer producer : producers) {
+            for (final DOMDataTreeProducer producer : producers) {
                 Preconditions.checkArgument(producer instanceof ShardedDOMDataTreeProducer);
-                ShardedDOMDataTreeProducer castedProducer = ((ShardedDOMDataTreeProducer) producer);
+                final ShardedDOMDataTreeProducer castedProducer = ((ShardedDOMDataTreeProducer) producer);
                 simpleLoopCheck(subtrees, castedProducer.getSubtrees());
                 // FIXME: We should also unbound listeners
                 castedProducer.boundToListener(listenerContext);
             }
 
-            for (DOMDataTreeIdentifier subtree : subtrees) {
-                DOMDataTreeShard shard = shards.lookup(subtree).getValue().getInstance();
+            for (final DOMDataTreeIdentifier subtree : subtrees) {
+                final DOMDataTreeShard shard = shards.lookup(subtree).getValue().getInstance();
                 // FIXME: What should we do if listener is wildcard? And shards are on per
                 // node basis?
                 Preconditions.checkArgument(shard instanceof DOMStoreTreeChangePublisher,
@@ -182,7 +188,7 @@ public final class ShardedDOMDataTree implements DOMDataTreeService, DOMDataTree
 
                 listenerContext.register(subtree, (DOMStoreTreeChangePublisher) shard);
             }
-        } catch (Exception e) {
+        } catch (final Exception e) {
             listenerContext.close();
             throw e;
         }
@@ -194,10 +200,10 @@ public final class ShardedDOMDataTree implements DOMDataTreeService, DOMDataTree
         };
     }
 
-    private static void simpleLoopCheck(Collection<DOMDataTreeIdentifier> listen, Set<DOMDataTreeIdentifier> writes)
+    private static void simpleLoopCheck(final Collection<DOMDataTreeIdentifier> listen, final Set<DOMDataTreeIdentifier> writes)
             throws DOMDataTreeLoopException {
-        for(DOMDataTreeIdentifier listenPath : listen) {
-            for (DOMDataTreeIdentifier writePath : writes) {
+        for(final DOMDataTreeIdentifier listenPath : listen) {
+            for (final DOMDataTreeIdentifier writePath : writes) {
                 if (listenPath.contains(writePath)) {
                     throw new DOMDataTreeLoopException(String.format(
                             "Listener must not listen on parent (%s), and also writes child (%s)", listenPath,
@@ -211,7 +217,7 @@ public final class ShardedDOMDataTree implements DOMDataTreeService, DOMDataTree
         }
     }
 
-    void removeListener(ShardedDOMDataTreeListenerContext<?> listener) {
+    void removeListener(final ShardedDOMDataTreeListenerContext<?> listener) {
         // FIXME: detach producers
         listener.close();
     }