From b9b7c857c35cfebcdebdb8f3488debc6332fd487 Mon Sep 17 00:00:00 2001 From: Tony Tkacik Date: Wed, 7 May 2014 13:10:40 +0200 Subject: [PATCH] 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 --- ...eteNestedAugmentationListenParentTest.java | 96 +++++++++++++ .../compat/DataNormalizationOperation.java | 130 +++++++++++------- .../util/compat/DataSchemaContainerProxy.java | 65 +++++++++ 3 files changed, 241 insertions(+), 50 deletions(-) create mode 100644 opendaylight/md-sal/sal-binding-dom-it/src/test/java/org/opendaylight/controller/sal/binding/test/bugfix/DeleteNestedAugmentationListenParentTest.java create mode 100644 opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataSchemaContainerProxy.java 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 index 0000000000..c1eba3ee25 --- /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 LISTENER_PATH = InstanceIdentifier.builder(Nodes.class) // + .child(Node.class) + .augmentation(FlowCapableNode.class).build(); + + + private static final InstanceIdentifier NODE_AUGMENT_PATH = InstanceIdentifier.builder(Nodes.class) + .child(Node.class,NODE_KEY) + .augmentation(FlowCapableNode.class) + .build(); + + private static final InstanceIdentifier 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, DataObject>> event = SettableFuture.create(); + + ListenerRegistration listenerReg = baDataService.registerDataChangeListener(FLOW_PATH, new DataChangeListener() { + + @Override + public void onDataChanged(final DataChangeEvent, DataObject> change) { + event.set(change); + } + }); + + DataModificationTransaction deleteTx = baDataService.beginTransaction(); + deleteTx.removeOperationalData(FLOW_PATH.augmentation(FlowStatisticsData.class)); + deleteTx.commit().get(); + + DataChangeEvent, 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 a36d984fa7..663493adcc 100644 --- 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 com.google.common.base.Optional; 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 impleme if (potential != null) { return potential; } - potential = fromSchema(schema, child); + potential = fromLocalSchema(child); 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; } - potential = fromSchemaAndPathArgument(schema, child); + potential = fromLocalSchemaAndQName(schema, child); 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); @@ -398,38 +411,34 @@ public abstract class DataNormalizationOperation impleme } } - private static final class AugmentationNormalization extends MixinNormalizationOp { - - private final Map> byQName; - private final Map> byArg; + private static final class AugmentationNormalization extends DataContainerNormalizationOperation { public AugmentationNormalization(final AugmentationSchema augmentation, final DataNodeContainer schema) { - super(augmentationIdentifierFrom(augmentation)); - - ImmutableMap.Builder> byQNameBuilder = ImmutableMap.builder(); - ImmutableMap.Builder> 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 - public DataNormalizationOperation getChild(final PathArgument child) { - return byArg.get(child); + public boolean isMixin() { + return true; } + + @Override - public DataNormalizationOperation getChild(final QName child) { - return byQName.get(child); + protected DataNormalizationOperation fromLocalSchemaAndQName(final DataNodeContainer schema, final QName child) + throws DataNormalizationException { + Optional 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 @@ -591,23 +600,30 @@ public abstract class DataNormalizationOperation impleme } } - private static DataNormalizationOperation fromSchemaAndPathArgument(final DataNodeContainer schema, - final QName child) throws DataNormalizationException { - DataSchemaNode potential = schema.getDataChildByName(child); + private static final Optional findChildSchemaNode(final DataNodeContainer parent,final QName child) { + DataSchemaNode potential = parent.getDataChildByName(child); if (potential == null) { Iterable 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); } + return Optional.fromNullable(potential); + } - if (potential == null) { + private static DataNormalizationOperation fromSchemaAndQNameChecked(final DataNodeContainer schema, + final QName child) throws DataNormalizationException { + + Optional 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())); } - 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( @@ -615,7 +631,7 @@ public abstract class DataNormalizationOperation 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()) { - if (caze.getDataChildByName(child) != null) { + if (findChildSchemaNode(caze, child).isPresent()) { foundChoice = choice; break choiceLoop; } @@ -632,30 +648,44 @@ public abstract class DataNormalizationOperation impleme 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 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; - 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) { - return new AugmentationNormalization(augmentation, schema); + return new AugmentationNormalization(augmentation, parent); } 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) { 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 index 0000000000..d243c888bd --- /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 realChildSchemas; + private final Map mappedChildSchemas; + + public DataSchemaContainerProxy(final Set realChildSchema) { + realChildSchemas = realChildSchema; + mappedChildSchemas = new HashMap(); + for(DataSchemaNode schema : realChildSchemas) { + mappedChildSchemas.put(schema.getQName(),schema); + } + } + + @Override + public DataSchemaNode getDataChildByName(final QName name) { + return mappedChildSchemas.get(name); + } + + @Override + public Set getChildNodes() { + return realChildSchemas; + } + + @Override + public DataSchemaNode getDataChildByName(final String arg0) { + throw new UnsupportedOperationException(); + } + + @Override + public Set getGroupings() { + return Collections.emptySet(); + } + + @Override + public Set> getTypeDefinitions() { + return Collections.emptySet(); + } + + @Override + public Set getUses() { + return Collections.emptySet(); + } + +} -- 2.36.6