From 2fa378970cb5c946d9b4eabd08f90a8c34b96629 Mon Sep 17 00:00:00 2001 From: Maros Marsalek Date: Mon, 18 Apr 2016 09:58:58 +0200 Subject: [PATCH] Check augments for equality in LazyDataObject LazyDataObject did not check augments when performing equals() returing true for un-equal augmentable dataObjects Hashcode already includes augments in the calculation Change-Id: Ic57c3c6f70eaea26de938ecb4e3ca67026e1965d Signed-off-by: Maros Marsalek (cherry picked from commit bbb707d67bf55f055c2c8790392f2fab03233a1d) --- .../data/codec/impl/LazyDataObject.java | 19 ++- ...ormalizedNodeSerializeDeserializeTest.java | 116 +++++++++++++++++- .../yang/opendaylight-mdsal-binding-test.yang | 12 ++ 3 files changed, 140 insertions(+), 7 deletions(-) diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/LazyDataObject.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/LazyDataObject.java index 1e891c70de..732903d9b9 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/LazyDataObject.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/yangtools/binding/data/codec/impl/LazyDataObject.java @@ -23,6 +23,7 @@ import org.opendaylight.yangtools.binding.data.codec.util.AugmentationReader; import org.opendaylight.yangtools.yang.binding.Augmentable; import org.opendaylight.yangtools.yang.binding.Augmentation; import org.opendaylight.yangtools.yang.binding.DataObject; +import org.opendaylight.yangtools.yang.binding.util.BindingReflections; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer; @@ -86,7 +87,13 @@ class LazyDataObject implements InvocationHandler, Augment for (final Method m : context.getHashCodeAndEqualsMethods()) { final Object thisValue = getBindingData(m); final Object otherValue = m.invoke(other); - if(!Objects.equals(thisValue, otherValue)) { + if (!Objects.equals(thisValue, otherValue)) { + return false; + } + } + + if (Augmentable.class.isAssignableFrom(context.getBindingClass())) { + if (!getAugmentationsImpl().equals(getAllAugmentations(other))) { return false; } } @@ -97,6 +104,16 @@ class LazyDataObject implements InvocationHandler, Augment return true; } + private static Map>, Augmentation> getAllAugmentations(Object dataObject) { + if (dataObject instanceof AugmentationReader) { + return ((AugmentationReader) dataObject).getAugmentations(dataObject); + } else if (dataObject instanceof Augmentable){ + return BindingReflections.getAugmentations((Augmentable) dataObject); + } + + throw new IllegalArgumentException("Unable to get all augmentations from " + dataObject); + } + private Integer bindingHashCode() { final Integer ret = cachedHashcode; if (ret != null) { diff --git a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/NormalizedNodeSerializeDeserializeTest.java b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/NormalizedNodeSerializeDeserializeTest.java index efc5077877..9c027af0f8 100644 --- a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/NormalizedNodeSerializeDeserializeTest.java +++ b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/yangtools/binding/data/codec/test/NormalizedNodeSerializeDeserializeTest.java @@ -7,7 +7,9 @@ */ package org.opendaylight.yangtools.binding.data.codec.test; +import static java.util.Collections.singleton; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; import static org.opendaylight.mdsal.binding.test.model.util.ListsBindingUtils.top; @@ -20,8 +22,10 @@ import static org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes.ma import static org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes.mapEntryBuilder; import static org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes.mapNodeBuilder; +import com.google.common.collect.Maps; import com.google.common.collect.Sets; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -43,6 +47,10 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.te import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.ChoiceContainer; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.ChoiceContainerBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.Top; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.Top1; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.Top1Builder; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.Top2; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.Top2Builder; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.TopBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.choice.identifier.ExtendedBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.mdsal.test.binding.rev140701.choice.identifier.extended.ExtendedIdBuilder; @@ -56,6 +64,7 @@ import org.opendaylight.yang.gen.v1.urn.test.foo4798.rev160101.Root; import org.opendaylight.yangtools.binding.data.codec.gen.impl.StreamWriterGenerator; import org.opendaylight.yangtools.binding.data.codec.impl.BindingNormalizedNodeCodecRegistry; import org.opendaylight.yangtools.sal.binding.generator.util.JavassistUtils; +import org.opendaylight.yangtools.yang.binding.Augmentation; import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.common.QName; @@ -64,6 +73,8 @@ import org.opendaylight.yangtools.yang.data.api.schema.AugmentationNode; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; +import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContainerNodeAttrBuilder; import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableAugmentationNodeBuilder; import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableChoiceNodeBuilder; import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableContainerNodeBuilder; @@ -121,21 +132,114 @@ public class NormalizedNodeSerializeDeserializeTest extends AbstractBindingRunti public void containerToNormalized() { final Map.Entry> entry = registry.toNormalizedNode(InstanceIdentifier.create(Top.class), top()); - final ContainerNode topNormalized = ImmutableContainerNodeBuilder.create() - .withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(TOP_QNAME)) - .withChild(mapNodeBuilder(TOP_LEVEL_LIST_QNAME).build()).build(); + final ContainerNode topNormalized = getEmptyTop(); assertEquals(topNormalized, entry.getValue()); } @Test public void containerFromNormalized() { - final ContainerNode topNormalized = ImmutableContainerNodeBuilder.create() - .withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(TOP_QNAME)) - .withChild(mapNodeBuilder(TOP_LEVEL_LIST_QNAME).build()).build(); + final ContainerNode topNormalized = getEmptyTop(); final Map.Entry, DataObject> entry = registry.fromNormalizedNode(BI_TOP_PATH, topNormalized); assertEquals(top(), entry.getValue()); } + private ContainerNode getEmptyTop() { + return ImmutableContainerNodeBuilder.create() + .withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(TOP_QNAME)) + .withChild(mapNodeBuilder(TOP_LEVEL_LIST_QNAME).build()).build(); + } + + private static final QName AGUMENT_STRING_Q = QName.create(TOP_QNAME, "augmented-string"); + private static final String AUGMENT_STRING_VALUE = "testingEquals"; + private static final QName AUGMENT_INT_Q = QName.create(TOP_QNAME, "augmented-int"); + private static final int AUGMENT_INT_VALUE = 44; + + @Test + public void equalsWithAugment() { + final ContainerNode topNormalizedWithAugments = getNormalizedTopWithAugments( + augmentationBuilder() + .withNodeIdentifier(new YangInstanceIdentifier.AugmentationIdentifier(singleton(AGUMENT_STRING_Q))) + .withChild(ImmutableNodes.leafNode(AGUMENT_STRING_Q, AUGMENT_STRING_VALUE)) + .build()); + final ContainerNode topNormalized = getEmptyTop(); + + final Map.Entry, DataObject> entry = registry.fromNormalizedNode(BI_TOP_PATH, topNormalized); + final Map.Entry, DataObject> entryWithAugments = registry.fromNormalizedNode(BI_TOP_PATH, topNormalizedWithAugments); + + // Equals on other with no augmentation should be false + assertNotEquals(top(), entryWithAugments.getValue()); + // Equals on other(reversed) with no augmentation should be false + assertNotEquals(entryWithAugments.getValue(), top()); + // Equals on other(lazy) with no augmentation should be false + assertNotEquals(entry.getValue(), entryWithAugments.getValue()); + // Equals on other(lazy, reversed) with no augmentation should be false + assertNotEquals(entryWithAugments.getValue(), entry.getValue()); + + final Top topWithAugments = topWithAugments(Collections.>, Augmentation> + singletonMap(Top1.class, new Top1Builder().setAugmentedString(AUGMENT_STRING_VALUE).build())); + // Equals other with same augment should be true + assertEquals(topWithAugments, entryWithAugments.getValue()); + // Equals other with same augment should be true + assertEquals(entryWithAugments.getValue(), topWithAugments); + // Equals on self should be true + assertEquals(entryWithAugments.getValue(), entryWithAugments.getValue()); + + final Top topWithAugmentsDiffValue = topWithAugments(Collections.>, Augmentation> + singletonMap(Top1.class, new Top1Builder().setAugmentedString("differentValue").build())); + assertNotEquals(topWithAugmentsDiffValue, entryWithAugments.getValue()); + assertNotEquals(entryWithAugments.getValue(), topWithAugmentsDiffValue); + } + + @Test + public void equalsWithMultipleAugments() { + final ContainerNode topNormalizedWithAugments = getNormalizedTopWithAugments( + augmentationBuilder() + .withNodeIdentifier( + new YangInstanceIdentifier.AugmentationIdentifier(singleton(AGUMENT_STRING_Q))) + .withChild(ImmutableNodes.leafNode(AGUMENT_STRING_Q, AUGMENT_STRING_VALUE)) + .build(), + augmentationBuilder() + .withNodeIdentifier(new YangInstanceIdentifier.AugmentationIdentifier(singleton(AUGMENT_INT_Q))) + .withChild(ImmutableNodes.leafNode(AUGMENT_INT_Q, AUGMENT_INT_VALUE)) + .build()); + + final Map.Entry, DataObject> entryWithAugments = registry.fromNormalizedNode(BI_TOP_PATH, topNormalizedWithAugments); + Map>, Augmentation> augments = Maps.newHashMap(); + augments.put(Top1.class, new Top1Builder().setAugmentedString(AUGMENT_STRING_VALUE).build()); + augments.put(Top2.class, new Top2Builder().setAugmentedInt(AUGMENT_INT_VALUE).build()); + Top topWithAugments = topWithAugments(augments); + + assertEquals(topWithAugments, entryWithAugments.getValue()); + assertEquals(entryWithAugments.getValue(), topWithAugments); + + augments = Maps.newHashMap(); + augments.put(Top1.class, new Top1Builder().setAugmentedString(AUGMENT_STRING_VALUE).build()); + augments.put(Top2.class, new Top2Builder().setAugmentedInt(999).build()); + topWithAugments = topWithAugments(augments); + + assertNotEquals(topWithAugments, entryWithAugments.getValue()); + assertNotEquals(entryWithAugments.getValue(), topWithAugments); + } + + private ContainerNode getNormalizedTopWithAugments(final AugmentationNode... augChild) { + final DataContainerNodeAttrBuilder + builder = ImmutableContainerNodeBuilder.create(); + + for (AugmentationNode augmentationNode : augChild) { + builder.withChild(augmentationNode); + } + return builder.withNodeIdentifier(new YangInstanceIdentifier.NodeIdentifier(TOP_QNAME)) + .withChild(mapNodeBuilder(TOP_LEVEL_LIST_QNAME).build()).build(); + } + + private static Top topWithAugments(final Map>, ? extends Augmentation> augments) { + final TopBuilder topBuilder = new TopBuilder(); + for (Map.Entry>, ? extends Augmentation> augment : augments.entrySet()) { + topBuilder.addAugmentation(augment.getKey(), augment.getValue()); + } + return topBuilder.setTopLevelList(Collections.emptyList()).build(); + } + @Test public void listWithKeysToNormalized() { final Map.Entry> entry = diff --git a/binding/mdsal-binding-test-model/src/main/yang/opendaylight-mdsal-binding-test.yang b/binding/mdsal-binding-test-model/src/main/yang/opendaylight-mdsal-binding-test.yang index e1b5e9c7e3..0942c6d523 100644 --- a/binding/mdsal-binding-test-model/src/main/yang/opendaylight-mdsal-binding-test.yang +++ b/binding/mdsal-binding-test-model/src/main/yang/opendaylight-mdsal-binding-test.yang @@ -121,4 +121,16 @@ module opendaylight-mdsal-binding-test { uses two-level-list; } } + + augment "/list-test:top" { + leaf augmented-string { + type string; + } + } + + augment "/list-test:top" { + leaf augmented-int { + type int32; + } + } } -- 2.36.6