Bug 967: Fixed incorrect delete was addressing augmentation of augmentation

DataNormalizationOperation was constructed incorrectly, if augmentation
was already augmented, which leaded to incorrect construction of instance
identifier for delete.

Patchset 2: Fixed normalization in cases of mixin inside mixin
e.g. augmentation containing choice with cases.

Change-Id: I580e5dfe41ce504775ac454b66368037f4d707bb
Signed-off-by: Tony Tkacik <ttkacik@cisco.com>

diff --git a/opendaylight/md-sal/sal-binding-dom-it/src/test/java/org/opendaylight/controller/sal/binding/test/bugfix/DeleteNestedAugmentationListenParentTest.java b/opendaylight/md-sal/sal-binding-dom-it/src/test/java/org/opendaylight/controller/sal/binding/test/bugfix/DeleteNestedAugmentationListenParentTest.java
new file mode 100644 (file)
index 0000000..c1eba3e
--- /dev/null
+++ b/opendaylight/md-sal/sal-binding-dom-it/src/test/java/org/opendaylight/controller/sal/binding/test/bugfix/DeleteNestedAugmentationListenParentTest.java
@@ -0,0 +1,96 @@
+package org.opendaylight.controller.sal.binding.test.bugfix;
+
+import static org.junit.Assert.assertFalse;
+
+import java.util.concurrent.ExecutionException;
+
+import org.junit.Test;
+import org.opendaylight.controller.md.sal.common.api.data.DataChangeEvent;
+import org.opendaylight.controller.sal.binding.api.data.DataChangeListener;
+import org.opendaylight.controller.sal.binding.api.data.DataModificationTransaction;
+import org.opendaylight.controller.sal.binding.test.AbstractDataServiceTest;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowCapableNode;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowId;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.Table;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.TableKey;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.table.Flow;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.table.FlowBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.table.FlowKey;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.statistics.rev130819.FlowStatisticsData;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.statistics.rev130819.FlowStatisticsDataBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.statistics.rev130819.flow.statistics.FlowStatisticsBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.flow.MatchBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.Nodes;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeKey;
+import org.opendaylight.yangtools.concepts.ListenerRegistration;
+import org.opendaylight.yangtools.yang.binding.DataObject;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+
+import com.google.common.util.concurrent.SettableFuture;
+
+public class DeleteNestedAugmentationListenParentTest extends AbstractDataServiceTest {
+
+    private static final NodeKey NODE_KEY = new NodeKey(new NodeId("foo"));
+
+    private static final TableKey TABLE_KEY = new TableKey((short) 0);
+
+    private static final FlowKey FLOW_KEY = new FlowKey(new FlowId("100"));
+
+    private static final InstanceIdentifier<FlowCapableNode> LISTENER_PATH = InstanceIdentifier.builder(Nodes.class) //
+            .child(Node.class)
+            .augmentation(FlowCapableNode.class).build();
+
+
+    private static final InstanceIdentifier<FlowCapableNode> NODE_AUGMENT_PATH = InstanceIdentifier.builder(Nodes.class)
+            .child(Node.class,NODE_KEY)
+            .augmentation(FlowCapableNode.class)
+            .build();
+
+    private static final InstanceIdentifier<Flow> FLOW_PATH = NODE_AUGMENT_PATH.builder()
+            .child(Table.class,TABLE_KEY)
+            .child(Flow.class,FLOW_KEY)
+            .build();
+
+
+    @Test
+    public void deleteChildListenParent() throws InterruptedException, ExecutionException {
+        DataModificationTransaction initTx = baDataService.beginTransaction();
+
+        initTx.putOperationalData(FLOW_PATH, flow());
+        initTx.commit().get();
+
+        final SettableFuture<DataChangeEvent<InstanceIdentifier<?>, DataObject>> event = SettableFuture.create();
+
+        ListenerRegistration<DataChangeListener> listenerReg = baDataService.registerDataChangeListener(FLOW_PATH, new DataChangeListener() {
+
+            @Override
+            public void onDataChanged(final DataChangeEvent<InstanceIdentifier<?>, DataObject> change) {
+                event.set(change);
+            }
+        });
+
+        DataModificationTransaction deleteTx = baDataService.beginTransaction();
+        deleteTx.removeOperationalData(FLOW_PATH.augmentation(FlowStatisticsData.class));
+        deleteTx.commit().get();
+
+        DataChangeEvent<InstanceIdentifier<?>, DataObject> receivedEvent = event.get();
+        assertFalse(receivedEvent.getRemovedOperationalData().contains(NODE_AUGMENT_PATH));
+    }
+
+    private Flow flow() {
+        FlowBuilder builder = new FlowBuilder()
+            .setKey(FLOW_KEY)
+            .addAugmentation(FlowStatisticsData.class,new FlowStatisticsDataBuilder()
+                    .setFlowStatistics(new FlowStatisticsBuilder()
+                        .setBarrier(true)
+                        .setMatch(new MatchBuilder()
+                            .build())
+                        .build())
+                    .build())
+        ;//.build();
+        return builder.build();
+    }
+
+}
\ No newline at end of file

diff --git a/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationOperation.java b/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationOperation.java
index a36d984fa75c88e50360b7da64a8c8bbb7dcd832..663493adcc55c92ef0dbadb0923d132b77eb2a61 100644 (file)
--- a/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationOperation.java
+++ b/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationOperation.java
@@ -45,6 +45,7 @@ import org.opendaylight.yangtools.yang.model.api.LeafSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.ListSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 
 import org.opendaylight.yangtools.yang.model.api.ListSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.SchemaContext;
 

+import com.google.common.base.Optional;

 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;


@@ -223,20 +224,32 @@ public abstract class DataNormalizationOperation<T extends PathArgument> impleme
             if (potential != null) {
                 return potential;
             }
             if (potential != null) {
                 return potential;
             }

-            potential = fromSchema(schema, child);
+            potential = fromLocalSchema(child);

             return register(potential);
         }
 
             return register(potential);
         }
 

+        private DataNormalizationOperation<?> fromLocalSchema(final PathArgument child) throws DataNormalizationException {
+            if (child instanceof AugmentationIdentifier) {
+                return fromSchemaAndQNameChecked(schema, ((AugmentationIdentifier) child).getPossibleChildNames()
+                        .iterator().next());
+            }
+            return fromSchemaAndQNameChecked(schema, child.getNodeType());
+        }
+

         @Override
         public DataNormalizationOperation<?> getChild(final QName child) throws DataNormalizationException {
             DataNormalizationOperation<?> potential = byQName.get(child);
             if (potential != null) {
                 return potential;
             }
         @Override
         public DataNormalizationOperation<?> getChild(final QName child) throws DataNormalizationException {
             DataNormalizationOperation<?> potential = byQName.get(child);
             if (potential != null) {
                 return potential;
             }

-            potential = fromSchemaAndPathArgument(schema, child);
+            potential = fromLocalSchemaAndQName(schema, child);

             return register(potential);
         }
 
             return register(potential);
         }
 

+        protected DataNormalizationOperation<?> fromLocalSchemaAndQName(final DataNodeContainer schema2, final QName child) throws DataNormalizationException {
+            return fromSchemaAndQNameChecked(schema2, child);
+        }
+

         private DataNormalizationOperation<?> register(final DataNormalizationOperation<?> potential) {
             if (potential != null) {
                 byArg.put(potential.getIdentifier(), potential);
         private DataNormalizationOperation<?> register(final DataNormalizationOperation<?> potential) {
             if (potential != null) {
                 byArg.put(potential.getIdentifier(), potential);


@@ -398,38 +411,34 @@ public abstract class DataNormalizationOperation<T extends PathArgument> impleme
         }
     }
 
         }
     }
 

-    private static final class AugmentationNormalization extends MixinNormalizationOp<AugmentationIdentifier> {
-
-        private final Map<QName, DataNormalizationOperation<?>> byQName;
-        private final Map<PathArgument, DataNormalizationOperation<?>> byArg;
+    private static final class AugmentationNormalization extends DataContainerNormalizationOperation<AugmentationIdentifier> {

 
         public AugmentationNormalization(final AugmentationSchema augmentation, final DataNodeContainer schema) {
 
         public AugmentationNormalization(final AugmentationSchema augmentation, final DataNodeContainer schema) {

-            super(augmentationIdentifierFrom(augmentation));
-
-            ImmutableMap.Builder<QName, DataNormalizationOperation<?>> byQNameBuilder = ImmutableMap.builder();
-            ImmutableMap.Builder<PathArgument, DataNormalizationOperation<?>> byArgBuilder = ImmutableMap.builder();
-
-            for (DataSchemaNode augNode : augmentation.getChildNodes()) {
-                DataSchemaNode resolvedNode = schema.getDataChildByName(augNode.getQName());
-                DataNormalizationOperation<?> resolvedOp = fromDataSchemaNode(resolvedNode);
-                byArgBuilder.put(resolvedOp.getIdentifier(), resolvedOp);
-                for (QName resQName : resolvedOp.getQNameIdentifiers()) {
-                    byQNameBuilder.put(resQName, resolvedOp);
-                }
-            }
-            byQName = byQNameBuilder.build();
-            byArg = byArgBuilder.build();
-
+            //super();
+            super(augmentationIdentifierFrom(augmentation), augmentationProxy(augmentation,schema));

         }
 
         @Override
         }
 
         @Override

-        public DataNormalizationOperation<?> getChild(final PathArgument child) {
-            return byArg.get(child);
+        public boolean isMixin() {
+            return true;

         }
 
         }
 

+
+

         @Override
         @Override

-        public DataNormalizationOperation<?> getChild(final QName child) {
-            return byQName.get(child);
+        protected DataNormalizationOperation<?> fromLocalSchemaAndQName(final DataNodeContainer schema, final QName child)
+                throws DataNormalizationException {
+            Optional<DataSchemaNode> potential = findChildSchemaNode(schema, child);
+            if (!potential.isPresent()) {
+                return null;
+            }
+
+            DataSchemaNode result = potential.get();
+            // We try to look up if this node was added by augmentation
+            if ((schema instanceof DataSchemaNode) && result.isAugmenting()) {
+                return fromAugmentation(schema, (AugmentationTarget) schema, result);
+            }
+            return fromDataSchemaNode(result);

         }
 
         @Override
         }
 
         @Override


@@ -591,23 +600,30 @@ public abstract class DataNormalizationOperation<T extends PathArgument> impleme
         }
     }
 
         }
     }
 

-    private static DataNormalizationOperation<?> fromSchemaAndPathArgument(final DataNodeContainer schema,
-            final QName child) throws DataNormalizationException {
-        DataSchemaNode potential = schema.getDataChildByName(child);
+    private static final Optional<DataSchemaNode> findChildSchemaNode(final DataNodeContainer parent,final QName child) {
+        DataSchemaNode potential = parent.getDataChildByName(child);

         if (potential == null) {
             Iterable<org.opendaylight.yangtools.yang.model.api.ChoiceNode> choices = FluentIterable.from(
         if (potential == null) {
             Iterable<org.opendaylight.yangtools.yang.model.api.ChoiceNode> choices = FluentIterable.from(

-                    schema.getChildNodes()).filter(org.opendaylight.yangtools.yang.model.api.ChoiceNode.class);
+                    parent.getChildNodes()).filter(org.opendaylight.yangtools.yang.model.api.ChoiceNode.class);

             potential = findChoice(choices, child);
         }
             potential = findChoice(choices, child);
         }

+        return Optional.fromNullable(potential);
+    }

 
 

-        if (potential == null) {
+    private static DataNormalizationOperation<?> fromSchemaAndQNameChecked(final DataNodeContainer schema,
+            final QName child) throws DataNormalizationException {
+
+        Optional<DataSchemaNode> potential = findChildSchemaNode(schema, child);
+        if (!potential.isPresent()) {

             throw new DataNormalizationException(String.format("Supplied QName %s is not valid according to schema %s, potential children nodes: %s", child, schema,schema.getChildNodes()));
         }
 
             throw new DataNormalizationException(String.format("Supplied QName %s is not valid according to schema %s, potential children nodes: %s", child, schema,schema.getChildNodes()));
         }
 

-        if ((schema instanceof DataSchemaNode) && !((DataSchemaNode) schema).isAugmenting() && potential.isAugmenting()) {
-            return fromAugmentation(schema, (AugmentationTarget) schema, potential);
+        DataSchemaNode result = potential.get();
+        // We try to look up if this node was added by augmentation
+        if ((schema instanceof DataSchemaNode) && result.isAugmenting()) {
+            return fromAugmentation(schema, (AugmentationTarget) schema, result);

         }
         }

-        return fromDataSchemaNode(potential);
+        return fromDataSchemaNode(result);

     }
 
     private static org.opendaylight.yangtools.yang.model.api.ChoiceNode findChoice(
     }
 
     private static org.opendaylight.yangtools.yang.model.api.ChoiceNode findChoice(


@@ -615,7 +631,7 @@ public abstract class DataNormalizationOperation<T extends PathArgument> impleme
         org.opendaylight.yangtools.yang.model.api.ChoiceNode foundChoice = null;
         choiceLoop: for (org.opendaylight.yangtools.yang.model.api.ChoiceNode choice : choices) {
             for (ChoiceCaseNode caze : choice.getCases()) {
         org.opendaylight.yangtools.yang.model.api.ChoiceNode foundChoice = null;
         choiceLoop: for (org.opendaylight.yangtools.yang.model.api.ChoiceNode choice : choices) {
             for (ChoiceCaseNode caze : choice.getCases()) {

-                if (caze.getDataChildByName(child) != null) {
+                if (findChildSchemaNode(caze, child).isPresent()) {

                     foundChoice = choice;
                     break choiceLoop;
                 }
                     foundChoice = choice;
                     break choiceLoop;
                 }


@@ -632,30 +648,44 @@ public abstract class DataNormalizationOperation<T extends PathArgument> impleme
         return new AugmentationIdentifier(null, potentialChildren.build());
     }
 
         return new AugmentationIdentifier(null, potentialChildren.build());
     }
 

-    private static AugmentationNormalization fromAugmentation(final DataNodeContainer schema,
-            final AugmentationTarget augments, final DataSchemaNode potential) {
+    private static DataNodeContainer augmentationProxy(final AugmentationSchema augmentation, final DataNodeContainer schema) {
+        Set<DataSchemaNode> children = new HashSet<>();
+        for (DataSchemaNode augNode : augmentation.getChildNodes()) {
+            children.add(schema.getDataChildByName(augNode.getQName()));
+        }
+        return new DataSchemaContainerProxy(children);
+    }
+
+    /**
+     * Returns a DataNormalizationOperation for provided child node
+     *
+     * If supplied child is added by Augmentation this operation returns
+     * a DataNormalizationOperation for augmentation,
+     * otherwise returns a DataNormalizationOperation for child as
+     * call for {@link #fromDataSchemaNode(DataSchemaNode)}.
+     *
+     *
+     * @param parent
+     * @param parentAug
+     * @param child
+     * @return
+     */
+    private static DataNormalizationOperation<?> fromAugmentation(final DataNodeContainer parent,
+            final AugmentationTarget parentAug, final DataSchemaNode child) {

         AugmentationSchema augmentation = null;
         AugmentationSchema augmentation = null;

-        for (AugmentationSchema aug : augments.getAvailableAugmentations()) {
-            DataSchemaNode child = aug.getDataChildByName(potential.getQName());
-            if (child != null) {
+        for (AugmentationSchema aug : parentAug.getAvailableAugmentations()) {
+            DataSchemaNode potential = aug.getDataChildByName(child.getQName());
+            if (potential != null) {

                 augmentation = aug;
                 break;
             }
 
         }
         if (augmentation != null) {
                 augmentation = aug;
                 break;
             }
 
         }
         if (augmentation != null) {

-            return new AugmentationNormalization(augmentation, schema);
+            return new AugmentationNormalization(augmentation, parent);

         } else {
         } else {

-            return null;
-        }
-    }
-
-    private static DataNormalizationOperation<?> fromSchema(final DataNodeContainer schema, final PathArgument child) throws DataNormalizationException {
-        if (child instanceof AugmentationIdentifier) {
-            return fromSchemaAndPathArgument(schema, ((AugmentationIdentifier) child).getPossibleChildNames()
-                    .iterator().next());
+            return fromDataSchemaNode(child);

         }
         }

-        return fromSchemaAndPathArgument(schema, child.getNodeType());

     }
 
     public static DataNormalizationOperation<?> fromDataSchemaNode(final DataSchemaNode potential) {
     }
 
     public static DataNormalizationOperation<?> fromDataSchemaNode(final DataSchemaNode potential) {

diff --git a/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataSchemaContainerProxy.java b/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataSchemaContainerProxy.java
new file mode 100644 (file)
index 0000000..d243c88
--- /dev/null
+++ b/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataSchemaContainerProxy.java
@@ -0,0 +1,65 @@
+/*
+ * Copyright (c) 2014 Cisco Systems, Inc. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.controller.md.sal.common.impl.util.compat;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.model.api.DataNodeContainer;
+import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
+import org.opendaylight.yangtools.yang.model.api.GroupingDefinition;
+import org.opendaylight.yangtools.yang.model.api.TypeDefinition;
+import org.opendaylight.yangtools.yang.model.api.UsesNode;
+
+class DataSchemaContainerProxy implements DataNodeContainer {
+
+    private final Set<DataSchemaNode> realChildSchemas;
+    private final Map<QName, DataSchemaNode> mappedChildSchemas;
+
+    public DataSchemaContainerProxy(final Set<DataSchemaNode> realChildSchema) {
+        realChildSchemas = realChildSchema;
+        mappedChildSchemas = new HashMap<QName, DataSchemaNode>();
+        for(DataSchemaNode schema : realChildSchemas) {
+            mappedChildSchemas.put(schema.getQName(),schema);
+        }
+    }
+
+    @Override
+    public DataSchemaNode getDataChildByName(final QName name) {
+        return mappedChildSchemas.get(name);
+    }
+
+    @Override
+    public Set<DataSchemaNode> getChildNodes() {
+        return realChildSchemas;
+    }
+
+    @Override
+    public DataSchemaNode getDataChildByName(final String arg0) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public Set<GroupingDefinition> getGroupings() {
+        return Collections.emptySet();
+    }
+
+    @Override
+    public Set<TypeDefinition<?>> getTypeDefinitions() {
+        return Collections.emptySet();
+    }
+
+    @Override
+    public Set<UsesNode> getUses() {
+        return Collections.emptySet();
+    }
+
+}