Bug 509: Fixed Backwards compatibility bugs with datastore. 05/6605/1
authorTony Tkacik <ttkacik@cisco.com>
Tue, 29 Apr 2014 16:57:08 +0000 (18:57 +0200)
committerTony Tkacik <ttkacik@cisco.com>
Tue, 29 Apr 2014 16:57:12 +0000 (18:57 +0200)
Fixed bug in write and merge of leaf set children,
which prevented update of leaf sets.

Change-Id: I61c130b9d40876ae50468ebfbd2cf15208f1249c
Signed-off-by: Tony Tkacik <ttkacik@cisco.com>
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedTransaction.java
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/ForwardedBackwardsCompatibleDataBroker.java
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ResolveDataChangeEventsTask.java
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/SchemaAwareApplyOperation.java

index 6bd6e6a..cc9c6eb 100644 (file)
@@ -82,7 +82,7 @@ public class AbstractForwardedTransaction<T extends AsyncTransaction<org.openday
                 try {
                     final DataObject dataObject = normalizedNode.isPresent() ? codec.toBinding(path,
                             normalizedNode.get()) : null;
-                    if(dataObject != null) {
+                    if (dataObject != null) {
                         updateCache(store, path, dataObject);
                     }
                     return Optional.fromNullable(dataObject);
@@ -109,6 +109,27 @@ public class AbstractForwardedTransaction<T extends AsyncTransaction<org.openday
                 .toNormalizedNode(path, data);
 
         org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalizedPath = normalized.getKey();
+        ensureParentsByMerge(writeTransaction, store, normalized.getKey(), path);
+        LOG.debug("Tx: {} : Putting data {}",getDelegate().getIdentifier(),normalized.getKey());
+        writeTransaction.put(store, normalized.getKey(), normalized.getValue());
+    }
+
+    protected void doMergeWithEnsureParents(final DOMDataReadWriteTransaction writeTransaction,
+            final LogicalDatastoreType store, final InstanceIdentifier<?> path, final DataObject data) {
+        invalidateCache(store, path);
+        final Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> normalized = codec
+                .toNormalizedNode(path, data);
+
+        org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalizedPath = normalized.getKey();
+        ensureParentsByMerge(writeTransaction, store, normalized.getKey(), path);
+        LOG.debug("Tx: {} : Merge data {}",getDelegate().getIdentifier(),normalized.getKey());
+        writeTransaction.merge(store, normalized.getKey(), normalized.getValue());
+    }
+
+    private void ensureParentsByMerge(final DOMDataReadWriteTransaction writeTransaction,
+            final LogicalDatastoreType store,
+            final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalizedPath,
+            final InstanceIdentifier<?> path) {
         List<PathArgument> currentArguments = new ArrayList<>();
         DataNormalizationOperation<?> currentOp = codec.getDataNormalizer().getRootOperation();
         Iterator<PathArgument> iterator = normalizedPath.getPath().iterator();
@@ -135,8 +156,6 @@ public class AbstractForwardedTransaction<T extends AsyncTransaction<org.openday
                 writeTransaction.merge(store, currentPath, currentOp.createDefault(currentArg));
             }
         }
-        //LOG .info("Tx: {} : Putting data {}",getDelegate().getIdentifier(),normalized.getKey());
-        writeTransaction.put(store, normalized.getKey(), normalized.getValue());
     }
 
     protected void doMerge(final DOMDataWriteTransaction writeTransaction, final LogicalDatastoreType store,
index ab0dc6e..0ad5927 100644 (file)
@@ -218,13 +218,17 @@ public class ForwardedBackwardsCompatibleDataBroker extends AbstractForwardedDat
 
         @Override
         public void putOperationalData(final InstanceIdentifier<? extends DataObject> path, final DataObject data) {
-            posponedRemovedOperational.remove(path);
-            doPutWithEnsureParents(getDelegate(), LogicalDatastoreType.OPERATIONAL, path, data);
+            boolean previouslyRemoved = posponedRemovedOperational.remove(path);
+            if(previouslyRemoved) {
+                doPutWithEnsureParents(getDelegate(), LogicalDatastoreType.OPERATIONAL, path, data);
+            } else {
+                doMergeWithEnsureParents(getDelegate(), LogicalDatastoreType.OPERATIONAL, path, data);
+            }
         }
 
         @Override
         public void putConfigurationData(final InstanceIdentifier<? extends DataObject> path, final DataObject data) {
-            posponedRemovedConfiguration.remove(path);
+            boolean previouslyRemoved = posponedRemovedConfiguration.remove(path);
             DataObject originalObj = readConfigurationData(path);
             if (originalObj != null) {
                 original.put(path, originalObj);
@@ -233,7 +237,11 @@ public class ForwardedBackwardsCompatibleDataBroker extends AbstractForwardedDat
                 created.put(path, data);
             }
             updated.put(path, data);
-            doPutWithEnsureParents(getDelegate(), LogicalDatastoreType.CONFIGURATION, path, data);
+            if(previouslyRemoved) {
+                doPutWithEnsureParents(getDelegate(), LogicalDatastoreType.CONFIGURATION, path, data);
+            } else {
+                doMergeWithEnsureParents(getDelegate(), LogicalDatastoreType.CONFIGURATION, path, data);
+            }
         }
 
         @Override
index c62c27d..e74ba95 100644 (file)
@@ -394,7 +394,8 @@ public class ResolveDataChangeEventsTask implements Callable<Iterable<ChangeList
 
         Builder eventBuilder = builder(potentialScope) //
                 .setBefore(beforeCont) //
-                .setAfter(afterCont);
+                .setAfter(afterCont)
+                .addUpdated(path, beforeCont, afterCont);
         for (DOMImmutableDataChangeEvent childChange : childChanges) {
             eventBuilder.merge(childChange);
         }
@@ -447,27 +448,34 @@ public class ResolveDataChangeEventsTask implements Callable<Iterable<ChangeList
             final Collection<Node> listeners, final NormalizedNode<PathArgument, ?> node,
             final SimpleEventFactory eventFactory) {
 
-        DOMImmutableDataChangeEvent event = eventFactory.create(path, node);
-
-        if (!listeners.isEmpty()) {
+        final DOMImmutableDataChangeEvent event = eventFactory.create(path, node);
+        DOMImmutableDataChangeEvent propagateEvent = event;
             // We have listeners for this node or it's children, so we will try
             // to do additional processing
-            if (node instanceof NormalizedNodeContainer<?, ?, ?>) {
-                // Node has children, so we will try to resolve it's children
-                // changes.
-                @SuppressWarnings("unchecked")
-                NormalizedNodeContainer<?, PathArgument, NormalizedNode<PathArgument, ?>> container = (NormalizedNodeContainer<?, PathArgument, NormalizedNode<PathArgument, ?>>) node;
-                for (NormalizedNode<PathArgument, ?> child : container.getValue()) {
-                    PathArgument childId = child.getIdentifier();
-                    Collection<Node> childListeners = getListenerChildrenWildcarded(listeners, childId);
-                    if (!childListeners.isEmpty()) {
-                        resolveSameEventRecursivelly(append(path, childId), childListeners, child, eventFactory);
-                    }
-                }
+        if (node instanceof NormalizedNodeContainer<?, ?, ?>) {
+            Builder eventBuilder = builder(DataChangeScope.BASE);
+            eventBuilder.merge(event);
+            eventBuilder.setBefore(event.getOriginalSubtree());
+            eventBuilder.setAfter(event.getUpdatedSubtree());
+
+            // Node has children, so we will try to resolve it's children
+            // changes.
+            @SuppressWarnings("unchecked")
+            NormalizedNodeContainer<?, PathArgument, NormalizedNode<PathArgument, ?>> container = (NormalizedNodeContainer<?, PathArgument, NormalizedNode<PathArgument, ?>>) node;
+            for (NormalizedNode<PathArgument, ?> child : container.getValue()) {
+                PathArgument childId = child.getIdentifier();
+                Collection<Node> childListeners = getListenerChildrenWildcarded(listeners, childId);
+                eventBuilder.merge(resolveSameEventRecursivelly(append(path, childId), childListeners, child, eventFactory));
             }
+            propagateEvent = eventBuilder.build();
+        } else {
+            // We do not dispatch leaf events since Binding Aware components do not support them.
+            propagateEvent = builder(DataChangeScope.BASE).build();
+        }
+        if (!listeners.isEmpty()) {
             addPartialTask(listeners, event);
         }
-        return event;
+        return propagateEvent;
     }
 
     private DOMImmutableDataChangeEvent resolveSubtreeChangeEvent(final InstanceIdentifier path,
@@ -476,7 +484,7 @@ public class ResolveDataChangeEventsTask implements Callable<Iterable<ChangeList
 
         Builder one = builder(DataChangeScope.ONE).setBefore(before.getData()).setAfter(after.getData());
 
-        Builder subtree = builder(DataChangeScope.SUBTREE);
+        Builder subtree = builder(DataChangeScope.SUBTREE).setBefore(before.getData()).setAfter(after.getData());
 
         for (NodeModification childMod : modification.getModifications()) {
             PathArgument childId = childMod.getIdentifier();
index a5c9b79..85dc711 100644 (file)
@@ -36,6 +36,7 @@ import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableCo
 import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableLeafSetNodeBuilder;
 import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableMapEntryNodeBuilder;
 import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableMapNodeBuilder;
+import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableOrderedLeafSetNodeBuilder;
 import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableOrderedMapNodeBuilder;
 import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableUnkeyedListEntryNodeBuilder;
 import org.opendaylight.yangtools.yang.model.api.AugmentationSchema;
@@ -70,7 +71,7 @@ public abstract class SchemaAwareApplyOperation implements ModificationApplyOper
         } else if (schemaNode instanceof ChoiceNode) {
             return new ChoiceModificationStrategy((ChoiceNode) schemaNode);
         } else if (schemaNode instanceof LeafListSchemaNode) {
-            return new LeafSetEntryModificationStrategy((LeafListSchemaNode) schemaNode);
+            return fromLeafListSchemaNode((LeafListSchemaNode) schemaNode);
         } else if (schemaNode instanceof LeafSchemaNode) {
             return new LeafModificationStrategy((LeafSchemaNode) schemaNode);
         }
@@ -89,6 +90,15 @@ public abstract class SchemaAwareApplyOperation implements ModificationApplyOper
         return new UnorderedMapModificationStrategy(schemaNode);
     }
 
+    private static SchemaAwareApplyOperation fromLeafListSchemaNode(final LeafListSchemaNode schemaNode) {
+        if(schemaNode.isUserOrdered()) {
+            return new OrderedLeafSetModificationStrategy(schemaNode);
+        } else {
+            return new UnorderedLeafSetModificationStrategy(schemaNode);
+        }
+    }
+
+
     public static SchemaAwareApplyOperation from(final DataNodeContainer resolvedTree,
             final AugmentationTarget augSchemas, final AugmentationIdentifier identifier) {
         AugmentationSchema augSchema = null;
@@ -563,12 +573,12 @@ public abstract class SchemaAwareApplyOperation implements ModificationApplyOper
 
     }
 
-    public static class LeafSetModificationStrategy extends NormalizedNodeContainerModificationStrategy {
+    public static class UnorderedLeafSetModificationStrategy extends NormalizedNodeContainerModificationStrategy {
 
         private final Optional<ModificationApplyOperation> entryStrategy;
 
         @SuppressWarnings({ "unchecked", "rawtypes" })
-        protected LeafSetModificationStrategy(final LeafListSchemaNode schema) {
+        protected UnorderedLeafSetModificationStrategy(final LeafListSchemaNode schema) {
             super((Class) LeafSetNode.class);
             entryStrategy = Optional.<ModificationApplyOperation> of(new LeafSetEntryModificationStrategy(schema));
         }
@@ -589,6 +599,32 @@ public abstract class SchemaAwareApplyOperation implements ModificationApplyOper
 
     }
 
+    public static class OrderedLeafSetModificationStrategy extends NormalizedNodeContainerModificationStrategy {
+
+        private final Optional<ModificationApplyOperation> entryStrategy;
+
+        @SuppressWarnings({ "unchecked", "rawtypes" })
+        protected OrderedLeafSetModificationStrategy(final LeafListSchemaNode schema) {
+            super((Class) LeafSetNode.class);
+            entryStrategy = Optional.<ModificationApplyOperation> of(new LeafSetEntryModificationStrategy(schema));
+        }
+
+        @SuppressWarnings("rawtypes")
+        @Override
+        protected NormalizedNodeContainerBuilder createBuilder(final PathArgument identifier) {
+            return ImmutableOrderedLeafSetNodeBuilder.create().withNodeIdentifier((NodeIdentifier) identifier);
+        }
+
+        @Override
+        public Optional<ModificationApplyOperation> getChild(final PathArgument identifier) {
+            if (identifier instanceof NodeWithValue) {
+                return entryStrategy;
+            }
+            return Optional.absent();
+        }
+
+    }
+
     public static class UnkeyedListModificationStrategy extends SchemaAwareApplyOperation {
 
         private final Optional<ModificationApplyOperation> entryStrategy;