From 28e0ef3b11e12112c0acc89ba07f59cc742f7417 Mon Sep 17 00:00:00 2001 From: Jakub Morvay Date: Mon, 1 Aug 2016 08:24:25 +0200 Subject: [PATCH] Make AbstractDOMShardTreeChangePublisher advertise initial data change Change-Id: Ia9d5a7fcb8bedcb3494b4acc55c372ca5e812424 Signed-off-by: Jakub Morvay --- ...rdedDOMDataTreeProducerMultiShardTest.java | 18 ++-- .../dom/broker/ShardedDOMDataTreeTest.java | 6 +- .../AbstractDOMShardTreeChangePublisher.java | 87 +++++++++++++++++-- ...stractDOMShardTreeChangePublisherTest.java | 49 +++++++++-- .../InMemoryDOMDataTreeShardTest.java | 2 + 5 files changed, 135 insertions(+), 27 deletions(-) diff --git a/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeProducerMultiShardTest.java b/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeProducerMultiShardTest.java index 444e0771e7..02c1ff9ba0 100644 --- a/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeProducerMultiShardTest.java +++ b/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeProducerMultiShardTest.java @@ -179,7 +179,7 @@ public class ShardedDOMDataTreeProducerMultiShardTest { transaction.ready(); transaction.submit(); - verify(mockedDataTreeListener, timeout(1000)).onDataTreeChanged(captorForChanges.capture(), captorForSubtrees.capture()); + verify(mockedDataTreeListener, timeout(1000).times(2)).onDataTreeChanged(captorForChanges.capture(), captorForSubtrees.capture()); final Collection capturedValue = captorForChanges.getValue(); assertTrue(capturedValue.size() == 1); @@ -199,7 +199,7 @@ public class ShardedDOMDataTreeProducerMultiShardTest { final DOMDataTreeShardWriteTransaction transaction = producer.createTransaction(); writeCrossShardContainer(transaction); - verify(mockedDataTreeListener, timeout(1000)).onDataTreeChanged(captorForChanges.capture(), captorForSubtrees.capture()); + verify(mockedDataTreeListener, timeout(1000).times(2)).onDataTreeChanged(captorForChanges.capture(), captorForSubtrees.capture()); final Collection capturedValue = captorForChanges.getValue(); assertTrue(capturedValue.size() == 1); @@ -236,19 +236,19 @@ public class ShardedDOMDataTreeProducerMultiShardTest { .build(); //verify listeners have been notified - verify(mockedDataTreeListener, timeout(1000).times(2)).onDataTreeChanged(captorForChanges.capture(), captorForSubtrees.capture()); + verify(mockedDataTreeListener, timeout(1000).times(4)).onDataTreeChanged(captorForChanges.capture(), captorForSubtrees.capture()); final List> capturedChanges = captorForChanges.getAllValues(); final List>> capturedSubtrees = captorForSubtrees.getAllValues(); - final DataTreeCandidate firstNotificationCandidate = capturedChanges.get(0).iterator().next(); + final DataTreeCandidate firstNotificationCandidate = capturedChanges.get(2).iterator().next(); - assertTrue(capturedSubtrees.get(0).size() == 1); + assertTrue(capturedSubtrees.get(2).size() == 1); assertEquals(testContainerVerificationNode, firstNotificationCandidate.getRootNode().getDataAfter().get()); - assertEquals(testContainerVerificationNode, capturedSubtrees.get(0).get(TEST_ID)); + assertEquals(testContainerVerificationNode, capturedSubtrees.get(2).get(TEST_ID)); - final DataTreeCandidate secondNotificationCandidate = capturedChanges.get(1).iterator().next(); - assertTrue(capturedSubtrees.get(1).size() == 1); + final DataTreeCandidate secondNotificationCandidate = capturedChanges.get(3).iterator().next(); + assertTrue(capturedSubtrees.get(3).size() == 1); assertEquals(crossShardContainer, secondNotificationCandidate.getRootNode().getDataAfter().get()); - assertEquals(crossShardContainer, capturedSubtrees.get(1).get(TEST_ID)); + assertEquals(crossShardContainer, capturedSubtrees.get(3).get(TEST_ID)); verifyNoMoreInteractions(mockedDataTreeListener); } diff --git a/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeTest.java b/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeTest.java index b7383043b5..1ffa904ef4 100644 --- a/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeTest.java +++ b/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/ShardedDOMDataTreeTest.java @@ -141,11 +141,11 @@ public class ShardedDOMDataTreeTest { cursor.close(); tx.submit().checkedGet(); - verify(mockedDataTreeListener, timeout(1000).times(2)).onDataTreeChanged(captorForChanges.capture(), captorForSubtrees.capture()); + verify(mockedDataTreeListener, timeout(1000).times(3)).onDataTreeChanged(captorForChanges.capture(), captorForSubtrees.capture()); final List> capturedValue = captorForChanges.getAllValues(); - assertTrue(capturedValue.size() == 2); + assertTrue(capturedValue.size() == 3); - final ContainerNode capturedChange = (ContainerNode) capturedValue.get(0).iterator().next().getRootNode().getDataAfter().get(); + final ContainerNode capturedChange = (ContainerNode) capturedValue.get(1).iterator().next().getRootNode().getDataAfter().get(); final ContainerNode innerContainerVerify = (ContainerNode) crossShardContainer.getChild(TestModel.INNER_CONTAINER_PATH.getLastPathArgument()).get(); assertEquals(innerContainerVerify, capturedChange); diff --git a/dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/AbstractDOMShardTreeChangePublisher.java b/dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/AbstractDOMShardTreeChangePublisher.java index 086689f2c1..e24cdea9ec 100644 --- a/dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/AbstractDOMShardTreeChangePublisher.java +++ b/dom/mdsal-dom-inmemory-datastore/src/main/java/org/opendaylight/mdsal/dom/store/inmemory/AbstractDOMShardTreeChangePublisher.java @@ -8,6 +8,7 @@ package org.opendaylight.mdsal.dom.store.inmemory; +import com.google.common.base.Optional; import com.google.common.base.Preconditions; import java.util.ArrayList; import java.util.Collection; @@ -18,9 +19,9 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import org.opendaylight.mdsal.dom.api.DOMDataTreeChangeListener; import org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier; -import org.opendaylight.mdsal.dom.api.DOMDataTreeListener; import org.opendaylight.mdsal.dom.spi.AbstractDOMDataTreeChangeListenerRegistration; import org.opendaylight.mdsal.dom.spi.RegistrationTreeNode; import org.opendaylight.mdsal.dom.spi.store.AbstractDOMStoreTreeChangePublisher; @@ -28,12 +29,14 @@ import org.opendaylight.mdsal.dom.spi.store.DOMStoreTreeChangePublisher; import org.opendaylight.yangtools.concepts.ListenerRegistration; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; 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.tree.DataTree; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidates; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeModification; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException; +import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,7 +44,7 @@ abstract class AbstractDOMShardTreeChangePublisher extends AbstractDOMStoreTreeC private static final Logger LOG = LoggerFactory.getLogger(AbstractDOMShardTreeChangePublisher.class); - private YangInstanceIdentifier shardPath; + private final YangInstanceIdentifier shardPath; private final Map childShards; private final DataTree dataTree; @@ -66,7 +69,7 @@ abstract class AbstractDOMShardTreeChangePublisher extends AbstractDOMStoreTreeC private AbstractDOMDataTreeChangeListenerRegistration setupListenerContext(final YangInstanceIdentifier listenerPath, final L listener) { // we need to register the listener registration path based on the shards root // we have to strip the shard path from the listener path and then register - YangInstanceIdentifier strippedIdentifier = listenerPath; + YangInstanceIdentifier strippedIdentifier = listenerPath; if (!shardPath.isEmpty()) { strippedIdentifier = YangInstanceIdentifier.create(stripShardPath(listenerPath)); } @@ -76,19 +79,36 @@ abstract class AbstractDOMShardTreeChangePublisher extends AbstractDOMStoreTreeC for (final ChildShardContext maybeAffected : childShards.values()) { if (listenerPath.contains(maybeAffected.getPrefix().getRootIdentifier())) { - // consumer has a subshard somewhere on lower level + // consumer has initialDataChangeEvent subshard somewhere on lower level // register to the notification manager with snapshot and forward child notifications to parent LOG.debug("Adding new subshard{{}} to listener at {}", maybeAffected.getPrefix(), listenerPath); subshardListener.addSubshard(maybeAffected); } else if (maybeAffected.getPrefix().getRootIdentifier().contains(listenerPath)) { // bind path is inside subshard // TODO can this happen? seems like in ShardedDOMDataTree we are already registering to the lowest shard possible - throw new UnsupportedOperationException("Listener should be registered directly into a subshard"); + throw new UnsupportedOperationException("Listener should be registered directly into initialDataChangeEvent subshard"); } } + + initialDataChangeEvent(listenerPath, listener); + return reg; } + private void initialDataChangeEvent(final YangInstanceIdentifier listenerPath, final L listener) { + // FIXME Add support for wildcard listeners + final Optional> preExistingData = dataTree.takeSnapshot().readNode(listenerPath); + final DataTreeCandidate initialCandidate; + if (preExistingData.isPresent()) { + initialCandidate = DataTreeCandidates.fromNormalizedNode(listenerPath, preExistingData.get()); + } else { + initialCandidate = DataTreeCandidates.newDataTreeCandidate(listenerPath, + new EmptyDataTreeCandidateNode(listenerPath.getLastPathArgument())); + } + + listener.onDataTreeChanged(Collections.singleton(initialCandidate)); + } + private AbstractDOMDataTreeChangeListenerRegistration setupContextWithoutSubshards(final YangInstanceIdentifier listenerPath, final DOMDataTreeListenerWithSubshards listener) { LOG.debug("Registering root listener at {}", listenerPath); final RegistrationTreeNode> node = findNodeFor(listenerPath.getPathArguments()); @@ -156,7 +176,7 @@ abstract class AbstractDOMShardTreeChangePublisher extends AbstractDOMStoreTreeC void addSubshard(final ChildShardContext context) { Preconditions.checkState(context.getShard() instanceof DOMStoreTreeChangePublisher, - "All subshards that are a part of ListenerContext need to be listenable"); + "All subshards that are initialDataChangeEvent part of ListenerContext need to be listenable"); final DOMStoreTreeChangePublisher listenableShard = (DOMStoreTreeChangePublisher) context.getShard(); // since this is going into subshard we want to listen for ALL changes in the subshard @@ -181,7 +201,7 @@ abstract class AbstractDOMShardTreeChangePublisher extends AbstractDOMStoreTreeC modification.ready(); try { dataTree.validate(modification); - } catch (DataValidationFailedException e) { + } catch (final DataValidationFailedException e) { LOG.error("Validation failed for built modification", e); throw new RuntimeException("Notification validation failed", e); } @@ -189,12 +209,61 @@ abstract class AbstractDOMShardTreeChangePublisher extends AbstractDOMStoreTreeC // strip nodes we dont need since this listener doesn't have to be registered at the root of the DataTree DataTreeCandidateNode modifiedChild = dataTree.prepare(modification).getRootNode(); for (final PathArgument pathArgument : listenerPath.getPathArguments()) { - // there should be no null pointers since we wouldn't get a notification change - // if there was no node modified at the listener's location modifiedChild = modifiedChild.getModifiedChild(pathArgument); } + if (modifiedChild == null) { + modifiedChild = new EmptyDataTreeCandidateNode(listenerPath.getLastPathArgument()); + } + return DataTreeCandidates.newDataTreeCandidate(listenerPath, modifiedChild); } } + + private static final class EmptyDataTreeCandidateNode implements DataTreeCandidateNode { + + private final PathArgument identifier; + + EmptyDataTreeCandidateNode(final PathArgument identifier) { + Preconditions.checkNotNull(identifier, "Identifier should not be null"); + this.identifier = identifier; + } + + @Nonnull + @Override + public PathArgument getIdentifier() { + return identifier; + } + + @Nonnull + @Override + public Collection getChildNodes() { + return Collections.emptySet(); + } + + @Nullable + @Override + public DataTreeCandidateNode getModifiedChild(final PathArgument identifier) { + return null; + } + + @Nonnull + @Override + public ModificationType getModificationType() { + return ModificationType.UNMODIFIED; + } + + @Nonnull + @Override + public Optional> getDataAfter() { + return Optional.absent(); + } + + @Nonnull + @Override + public Optional> getDataBefore() { + return Optional.absent(); + } + } + } \ No newline at end of file diff --git a/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/AbstractDOMShardTreeChangePublisherTest.java b/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/AbstractDOMShardTreeChangePublisherTest.java index 5f26ee11d9..e1635dc787 100644 --- a/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/AbstractDOMShardTreeChangePublisherTest.java +++ b/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/AbstractDOMShardTreeChangePublisherTest.java @@ -7,20 +7,29 @@ */ package org.opendaylight.mdsal.dom.store.inmemory; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.opendaylight.mdsal.dom.store.inmemory.TestUtils.DATA_TREE; import static org.opendaylight.mdsal.dom.store.inmemory.TestUtils.resetMocks; +import com.google.common.base.Optional; import com.google.common.collect.ImmutableMap; import java.util.Collection; import java.util.Map; import javax.annotation.Nonnull; import org.junit.After; +import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.MockitoAnnotations; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; import org.opendaylight.mdsal.dom.api.DOMDataTreeChangeListener; import org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier; @@ -28,7 +37,10 @@ import org.opendaylight.mdsal.dom.spi.AbstractDOMDataTreeChangeListenerRegistrat import org.opendaylight.yangtools.concepts.ListenerRegistration; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidate; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeCandidateNode; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataTreeSnapshot; public class AbstractDOMShardTreeChangePublisherTest extends AbstractDOMShardTreeChangePublisher { @@ -47,15 +59,40 @@ public class AbstractDOMShardTreeChangePublisherTest extends AbstractDOMShardTre private static final Map CHILD_SHARDS = ImmutableMap.of(DOM_DATA_TREE_IDENTIFIER, CHILD_SHARD_CONTEXT); + @Captor + private ArgumentCaptor> captorForChanges; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + } + @Test public void registerTreeChangeListenerTest() throws Exception { final DOMDataTreeChangeListener domDataTreeChangeListener = mock(DOMDataTreeChangeListener.class); final ListenerRegistration listenerRegistration = mock(ListenerRegistration.class); + final DataTreeSnapshot initialSnapshot = mock(DataTreeSnapshot.class); + final NormalizedNode initialData = mock(NormalizedNode.class); + doReturn(initialSnapshot).when(DATA_TREE).takeSnapshot(); + doReturn(Optional.of(initialData)).when(initialSnapshot).readNode(any()); + doNothing().when(domDataTreeChangeListener).onDataTreeChanged(any()); + doReturn(listenerRegistration) .when(READABLE_WRITEABLE_DOM_DATA_TREE_SHARD).registerTreeChangeListener(any(), any()); assertNotNull(this.registerTreeChangeListener(YANG_INSTANCE_IDENTIFIER, domDataTreeChangeListener)); verify(READABLE_WRITEABLE_DOM_DATA_TREE_SHARD).registerTreeChangeListener(any(), any()); + + verify(domDataTreeChangeListener) + .onDataTreeChanged(captorForChanges.capture()); + + final Collection initialChange = captorForChanges.getValue(); + + assertTrue(initialChange.size() == 1); + initialChange.forEach(dataTreeCandidate -> + assertEquals(dataTreeCandidate.getRootPath(), YANG_INSTANCE_IDENTIFIER)); + initialChange.forEach(dataTreeCandidate -> + assertSame(dataTreeCandidate.getRootNode().getDataAfter().get(), initialData)); } @Test(expected = UnsupportedOperationException.class) @@ -71,7 +108,7 @@ public class AbstractDOMShardTreeChangePublisherTest extends AbstractDOMShardTre final Map childShardContextMap = ImmutableMap.of(domDataTreeIdentifier, childShardContext); - AbstractDOMShardTreeChangePublisherTest abstractDOMShardTreeChangePublisherTest = + final AbstractDOMShardTreeChangePublisherTest abstractDOMShardTreeChangePublisherTest = new AbstractDOMShardTreeChangePublisherTest(childShardContextMap); abstractDOMShardTreeChangePublisherTest .registerTreeChangeListener(YANG_INSTANCE_IDENTIFIER, domDataTreeChangeListener); @@ -81,23 +118,23 @@ public class AbstractDOMShardTreeChangePublisherTest extends AbstractDOMShardTre super(DATA_TREE, YANG_INSTANCE_IDENTIFIER, CHILD_SHARDS); } - private AbstractDOMShardTreeChangePublisherTest(Map childShardContextMap) { + private AbstractDOMShardTreeChangePublisherTest(final Map childShardContextMap) { super(DATA_TREE, YANG_INSTANCE_IDENTIFIER, childShardContextMap); } @Override - protected void notifyListeners(@Nonnull Collection> registrations, - @Nonnull YangInstanceIdentifier path, @Nonnull DataTreeCandidateNode node) { + protected void notifyListeners(@Nonnull final Collection> registrations, + @Nonnull final YangInstanceIdentifier path, @Nonnull final DataTreeCandidateNode node) { // NOOP } @Override - protected void registrationRemoved(@Nonnull AbstractDOMDataTreeChangeListenerRegistration registration) { + protected void registrationRemoved(@Nonnull final AbstractDOMDataTreeChangeListenerRegistration registration) { // NOOP } @After - public void reset(){ + public void reset() { resetMocks(); } } \ No newline at end of file diff --git a/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMDataTreeShardTest.java b/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMDataTreeShardTest.java index e25c708766..aabd3725c8 100644 --- a/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMDataTreeShardTest.java +++ b/dom/mdsal-dom-inmemory-datastore/src/test/java/org/opendaylight/mdsal/dom/store/inmemory/InMemoryDOMDataTreeShardTest.java @@ -12,6 +12,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -62,6 +63,7 @@ public class InMemoryDOMDataTreeShardTest { final DOMDataTreeChangeListener domDataTreeChangeListener = mock(DOMDataTreeChangeListener.class); final ListenerRegistration listenerRegistration = mock(ListenerRegistration.class); doReturn(listenerRegistration).when(domDataTreeShard).registerTreeChangeListener(any(), any()); + doNothing().when(domDataTreeChangeListener).onDataTreeChanged(any()); inMemoryDOMDataTreeShard.registerTreeChangeListener(YangInstanceIdentifier.EMPTY, domDataTreeChangeListener); verify(domDataTreeShard, atLeastOnce()).registerTreeChangeListener(any(), any()); -- 2.36.6