From: Tony Tkacik Date: Thu, 21 Aug 2014 07:18:03 +0000 (+0000) Subject: Merge "BUG-1493: split off recursion tracking and rework it" X-Git-Tag: release/beryllium~149^2~226 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=b1260bc5317766dac365758afd4b73da5cb47a42;hp=8a771d49a615fccfa4f3e40e32a16db0b4a3e6db;p=mdsal.git Merge "BUG-1493: split off recursion tracking and rework it" --- diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChangeListenerNotifyTask.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChangeListenerNotifyTask.java index ac1f2e32d5..536cfa0081 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChangeListenerNotifyTask.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ChangeListenerNotifyTask.java @@ -7,6 +7,8 @@ */ package org.opendaylight.controller.md.sal.dom.store.impl; +import com.google.common.base.Preconditions; + import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeEvent; import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeListener; import org.opendaylight.yangtools.util.concurrent.NotificationManager; @@ -16,35 +18,37 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; class ChangeListenerNotifyTask implements Runnable { - private static final Logger LOG = LoggerFactory.getLogger(ChangeListenerNotifyTask.class); - private final Iterable> listeners; - private final AsyncDataChangeEvent> event; - @SuppressWarnings("rawtypes") - private final NotificationManager - notificationMgr; + private final NotificationManager notificationMgr; + private final AsyncDataChangeEvent> event; + private final DataChangeListenerRegistration listener; @SuppressWarnings("rawtypes") - public ChangeListenerNotifyTask(final Iterable> listeners, + public ChangeListenerNotifyTask(final DataChangeListenerRegistration listener, final AsyncDataChangeEvent> event, final NotificationManager notificationMgr) { - this.listeners = listeners; - this.event = event; - this.notificationMgr = notificationMgr; + this.notificationMgr = Preconditions.checkNotNull(notificationMgr); + this.listener = Preconditions.checkNotNull(listener); + this.event = Preconditions.checkNotNull(event); } @Override public void run() { - - for (DataChangeListenerRegistration listener : listeners) { - notificationMgr.submitNotification(listener.getInstance(), event); + final AsyncDataChangeListener> l = listener.getInstance(); + if (l == null) { + LOG.trace("Skipping event delivery to unregistered listener {}", l); + return; } + LOG.trace("Listener {} event {}", l, event); + + // FIXME: Yo dawg I heard you like queues, so this was queued to be queued + notificationMgr.submitNotification(l, event); } @Override public String toString() { - return "ChangeListenerNotifyTask [listeners=" + listeners + ", event=" + event + "]"; + return "ChangeListenerNotifyTask [listener=" + listener + ", event=" + event + "]"; } } diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java index 5faebcef36..f457e3b9e9 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/DOMImmutableDataChangeEvent.java @@ -7,6 +7,8 @@ */ package org.opendaylight.controller.md.sal.dom.store.impl; +import com.google.common.base.Preconditions; + import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -19,8 +21,6 @@ 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 com.google.common.base.Preconditions; - public final class DOMImmutableDataChangeEvent implements AsyncDataChangeEvent> { @@ -184,6 +184,10 @@ public final class DOMImmutableDataChangeEvent implements updated.put(path, after); return this; } + + public boolean isEmpty() { + return created.isEmpty() && removed.isEmpty() && updated.isEmpty(); + } } private static final class RemoveEventFactory implements SimpleEventFactory { diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java index d0d3fe9e6a..129013378e 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/InMemoryDOMDataStore.java @@ -49,7 +49,6 @@ import org.slf4j.LoggerFactory; import javax.annotation.concurrent.GuardedBy; -import java.util.Collections; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; @@ -176,7 +175,7 @@ public class InMemoryDOMDataStore implements DOMStore, Identifiable, Sch .addCreated(path, data) // .build(); - new ChangeListenerNotifyTask(Collections.singletonList(reg), event, + new ChangeListenerNotifyTask(reg, event, dataChangeListenerNotificationManager).run(); } } diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ResolveDataChangeEventsTask.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ResolveDataChangeEventsTask.java index d8feaa71f6..a4e8c86aa8 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ResolveDataChangeEventsTask.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ResolveDataChangeEventsTask.java @@ -7,36 +7,24 @@ */ package org.opendaylight.controller.md.sal.dom.store.impl; -import static org.opendaylight.controller.md.sal.dom.store.impl.DOMImmutableDataChangeEvent.builder; - import com.google.common.base.Optional; import com.google.common.base.Preconditions; -import com.google.common.collect.HashMultimap; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; +import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.Multimap; +import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; -import java.util.LinkedList; -import java.util.List; import java.util.Map.Entry; -import java.util.Set; import java.util.concurrent.Callable; +import org.opendaylight.controller.md.sal.common.api.data.AsyncDataBroker.DataChangeScope; import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeEvent; import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeListener; -import org.opendaylight.controller.md.sal.common.api.data.AsyncDataBroker.DataChangeScope; import org.opendaylight.controller.md.sal.dom.store.impl.DOMImmutableDataChangeEvent.Builder; import org.opendaylight.controller.md.sal.dom.store.impl.DOMImmutableDataChangeEvent.SimpleEventFactory; import org.opendaylight.controller.md.sal.dom.store.impl.tree.ListenerTree; -import org.opendaylight.controller.md.sal.dom.store.impl.tree.ListenerTree.Node; import org.opendaylight.controller.md.sal.dom.store.impl.tree.ListenerTree.Walker; import org.opendaylight.yangtools.util.concurrent.NotificationManager; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue; 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; @@ -54,14 +42,13 @@ import org.slf4j.LoggerFactory; */ final class ResolveDataChangeEventsTask implements Callable> { private static final Logger LOG = LoggerFactory.getLogger(ResolveDataChangeEventsTask.class); - private static final DOMImmutableDataChangeEvent NO_CHANGE = builder(DataChangeScope.BASE).build(); - private final Multimap events = HashMultimap.create(); + @SuppressWarnings("rawtypes") + private final NotificationManager notificationMgr; private final DataTreeCandidate candidate; private final ListenerTree listenerRoot; - @SuppressWarnings("rawtypes") - private final NotificationManager notificationMgr; + private Multimap, DOMImmutableDataChangeEvent> collectedEvents; @SuppressWarnings("rawtypes") public ResolveDataChangeEventsTask(final DataTreeCandidate candidate, final ListenerTree listenerTree, @@ -81,153 +68,42 @@ final class ResolveDataChangeEventsTask implements Callable call() { + public synchronized Iterable call() { try (final Walker w = listenerRoot.getWalker()) { - resolveAnyChangeEvent(candidate.getRootPath(), Collections.singleton(w.getRootNode()), candidate.getRootNode()); - return createNotificationTasks(); - } - } - - /** - * - * Walks map of listeners to data change events, creates notification - * delivery tasks. - * - * Walks map of registered and affected listeners and creates notification - * tasks from set of listeners and events to be delivered. - * - * If set of listeners has more then one event (applicable to wildcarded - * listeners), merges all data change events into one, final which contains - * all separate updates. - * - * Dispatch between merge variant and reuse variant of notification task is - * done in - * {@link #addNotificationTask(com.google.common.collect.ImmutableList.Builder, Node, java.util.Collection)} - * - * @return Collection of notification tasks. - */ - private Collection createNotificationTasks() { - ImmutableList.Builder taskListBuilder = ImmutableList.builder(); - for (Entry> entry : events.asMap().entrySet()) { - addNotificationTask(taskListBuilder, entry.getKey(), entry.getValue()); - } - return taskListBuilder.build(); - } - - /** - * Adds notification task to task list. - * - * If entry collection contains one event, this event is reused and added to - * notification tasks for listeners (see - * {@link #addNotificationTaskByScope(com.google.common.collect.ImmutableList.Builder, Node, DOMImmutableDataChangeEvent)} - * . Otherwise events are merged by scope and distributed between listeners - * to particular scope. See - * {@link #addNotificationTasksAndMergeEvents(com.google.common.collect.ImmutableList.Builder, Node, java.util.Collection)} - * . - * - * @param taskListBuilder - * @param listeners - * @param entries - */ - private void addNotificationTask(final ImmutableList.Builder taskListBuilder, - final ListenerTree.Node listeners, final Collection entries) { - - if (!entries.isEmpty()) { - if (entries.size() == 1) { - addNotificationTaskByScope(taskListBuilder, listeners, Iterables.getOnlyElement(entries)); - } else { - addNotificationTasksAndMergeEvents(taskListBuilder, listeners, entries); - } - } - } + // Defensive: reset internal state + collectedEvents = ArrayListMultimap.create(); - /** - * - * Add notification deliveries task to the listener. - * - * - * @param taskListBuilder - * @param listeners - * @param event - */ - private void addNotificationTaskByScope( - final ImmutableList.Builder taskListBuilder, final ListenerTree.Node listeners, - final DOMImmutableDataChangeEvent event) { - DataChangeScope eventScope = event.getScope(); - for (DataChangeListenerRegistration listenerReg : listeners.getListeners()) { - DataChangeScope listenerScope = listenerReg.getScope(); - List> listenerSet = Collections - .> singletonList(listenerReg); - if (eventScope == DataChangeScope.BASE) { - taskListBuilder.add(new ChangeListenerNotifyTask(listenerSet, event, notificationMgr)); - } else if (eventScope == DataChangeScope.ONE && listenerScope != DataChangeScope.BASE) { - taskListBuilder.add(new ChangeListenerNotifyTask(listenerSet, event, notificationMgr)); - } else if (eventScope == DataChangeScope.SUBTREE && listenerScope == DataChangeScope.SUBTREE) { - taskListBuilder.add(new ChangeListenerNotifyTask(listenerSet, event, notificationMgr)); - } - } - } + // Run through the tree + final ResolveDataChangeState s = ResolveDataChangeState.initial(candidate.getRootPath(), w.getRootNode()); + resolveAnyChangeEvent(s, candidate.getRootNode()); - /** - * - * Add notification tasks with merged event - * - * Separate Events by scope and creates merged notification tasks for each - * and every scope which is present. - * - * Adds merged events to task list based on scope requested by client. - * - * @param taskListBuilder - * @param listeners - * @param entries - */ - private void addNotificationTasksAndMergeEvents( - final ImmutableList.Builder taskListBuilder, final ListenerTree.Node listeners, - final Collection entries) { - - final Builder baseBuilder = builder(DataChangeScope.BASE); - final Builder oneBuilder = builder(DataChangeScope.ONE); - final Builder subtreeBuilder = builder(DataChangeScope.SUBTREE); - - boolean baseModified = false; - boolean oneModified = false; - boolean subtreeModified = false; - for (final DOMImmutableDataChangeEvent entry : entries) { - switch (entry.getScope()) { - // Absence of breaks is intentional here. Subtree contains base and - // one, one also contains base - case BASE: - baseBuilder.merge(entry); - baseModified = true; - case ONE: - oneBuilder.merge(entry); - oneModified = true; - case SUBTREE: - subtreeBuilder.merge(entry); - subtreeModified = true; + /* + * Convert to tasks, but be mindful of multiple values -- those indicate multiple + * wildcard matches, which need to be merged. + */ + final Collection ret = new ArrayList<>(); + for (Entry, Collection> e : collectedEvents.asMap().entrySet()) { + final Collection col = e.getValue(); + final DOMImmutableDataChangeEvent event; + + if (col.size() != 1) { + final Builder b = DOMImmutableDataChangeEvent.builder(DataChangeScope.BASE); + for (DOMImmutableDataChangeEvent i : col) { + b.merge(i); + } + + event = b.build(); + LOG.trace("Merged events {} into event {}", col, event); + } else { + event = col.iterator().next(); + } + + ret.add(new ChangeListenerNotifyTask(e.getKey(), event, notificationMgr)); } - } - if (baseModified) { - addNotificationTaskExclusively(taskListBuilder, listeners, baseBuilder.build()); - } - if (oneModified) { - addNotificationTaskExclusively(taskListBuilder, listeners, oneBuilder.build()); - } - if (subtreeModified) { - addNotificationTaskExclusively(taskListBuilder, listeners, subtreeBuilder.build()); - } - } - - private void addNotificationTaskExclusively( - final ImmutableList.Builder taskListBuilder, final Node listeners, - final DOMImmutableDataChangeEvent event) { - for (DataChangeListenerRegistration listener : listeners.getListeners()) { - if (listener.getScope() == event.getScope()) { - Set> listenerSet = Collections - .> singleton(listener); - taskListBuilder.add(new ChangeListenerNotifyTask(listenerSet, event, notificationMgr)); - } + // FIXME: so now we have tasks to submit tasks... Inception-style! + LOG.debug("Created tasks {}", ret); + return ret; } } @@ -245,94 +121,90 @@ final class ResolveDataChangeEventsTask implements Callable listeners, final DataTreeCandidateNode node) { - + private boolean resolveAnyChangeEvent(final ResolveDataChangeState state, final DataTreeCandidateNode node) { if (node.getModificationType() != ModificationType.UNMODIFIED && !node.getDataAfter().isPresent() && !node.getDataBefore().isPresent()) { LOG.debug("Modification at {} has type {}, but no before- and after-data. Assuming unchanged.", - path, node.getModificationType()); - return NO_CHANGE; + state.getPath(), node.getModificationType()); + return false; } // no before and after state is present switch (node.getModificationType()) { case SUBTREE_MODIFIED: - return resolveSubtreeChangeEvent(path, listeners, node); + return resolveSubtreeChangeEvent(state, node); case MERGE: case WRITE: Preconditions.checkArgument(node.getDataAfter().isPresent(), - "Modification at {} has type {} but no after-data", path, node.getModificationType()); - if (node.getDataBefore().isPresent()) { - return resolveReplacedEvent(path, listeners, node.getDataBefore().get(), node.getDataAfter().get()); - } else { - return resolveCreateEvent(path, listeners, node.getDataAfter().get()); + "Modification at {} has type {} but no after-data", state.getPath(), node.getModificationType()); + if (!node.getDataBefore().isPresent()) { + resolveCreateEvent(state, node.getDataAfter().get()); + return true; } + + return resolveReplacedEvent(state, node.getDataBefore().get(), node.getDataAfter().get()); case DELETE: Preconditions.checkArgument(node.getDataBefore().isPresent(), - "Modification at {} has type {} but no before-data", path, node.getModificationType()); - return resolveDeleteEvent(path, listeners, node.getDataBefore().get()); + "Modification at {} has type {} but no before-data", state.getPath(), node.getModificationType()); + resolveDeleteEvent(state, node.getDataBefore().get()); + return true; case UNMODIFIED: - return NO_CHANGE; + return false; } - throw new IllegalStateException(String.format("Unhandled node state %s at %s", node.getModificationType(), path)); + throw new IllegalStateException(String.format("Unhandled node state %s at %s", node.getModificationType(), state.getPath())); } - private DOMImmutableDataChangeEvent resolveReplacedEvent(final YangInstanceIdentifier path, - final Collection listeners, final NormalizedNode beforeData, - final NormalizedNode afterData) { - - // FIXME: BUG-1493: check the listeners to prune unneeded changes: - // for subtrees, we have to do all - // for one, we need to expand children - // for base, we just report replacement + private boolean resolveReplacedEvent(final ResolveDataChangeState state, + final NormalizedNode beforeData, final NormalizedNode afterData) { if (beforeData instanceof NormalizedNodeContainer) { - // Node is container (contains child) and we have interested - // listeners registered for it, that means we need to do - // resolution of changes on children level and can not - // shortcut resolution. - LOG.trace("Resolving subtree replace event for {} before {}, after {}",path,beforeData,afterData); + /* + * Node is a container (contains a child) and we have interested + * listeners registered for it, that means we need to do + * resolution of changes on children level and can not + * shortcut resolution. + */ + LOG.trace("Resolving subtree replace event for {} before {}, after {}", state.getPath(), beforeData, afterData); @SuppressWarnings("unchecked") NormalizedNodeContainer> beforeCont = (NormalizedNodeContainer>) beforeData; @SuppressWarnings("unchecked") NormalizedNodeContainer> afterCont = (NormalizedNodeContainer>) afterData; - return resolveNodeContainerReplaced(path, listeners, beforeCont, afterCont); - } else if (!beforeData.equals(afterData)) { - // Node is Leaf type (does not contain child nodes) - // so normal equals method is sufficient for determining change. - LOG.trace("Resolving leaf replace event for {} , before {}, after {}",path,beforeData,afterData); - DOMImmutableDataChangeEvent event = builder(DataChangeScope.BASE).setBefore(beforeData).setAfter(afterData) - .addUpdated(path, beforeData, afterData).build(); - addPartialTask(listeners, event); - return event; - } else { - return NO_CHANGE; + return resolveNodeContainerReplaced(state, beforeCont, afterCont); } + + // Node is a Leaf type (does not contain child nodes) + // so normal equals method is sufficient for determining change. + if (beforeData.equals(afterData)) { + LOG.trace("Skipping equal leaf {}", state.getPath()); + return false; + } + + LOG.trace("Resolving leaf replace event for {} , before {}, after {}", state.getPath(), beforeData, afterData); + DOMImmutableDataChangeEvent event = DOMImmutableDataChangeEvent.builder(DataChangeScope.BASE).addUpdated(state.getPath(), beforeData, afterData).build(); + state.addEvent(event); + state.collectEvents(beforeData, afterData, collectedEvents); + return true; } - private DOMImmutableDataChangeEvent resolveNodeContainerReplaced(final YangInstanceIdentifier path, - final Collection listeners, + private boolean resolveNodeContainerReplaced(final ResolveDataChangeState state, final NormalizedNodeContainer> beforeCont, final NormalizedNodeContainer> afterCont) { - final List childChanges = new LinkedList<>(); + if (!state.needsProcessing()) { + LOG.trace("Not processing replaced container {}", state.getPath()); + return true; + } // We look at all children from before and compare it with after state. + boolean childChanged = false; for (NormalizedNode beforeChild : beforeCont.getValue()) { final PathArgument childId = beforeChild.getIdentifier(); - YangInstanceIdentifier childPath = path.node(childId); - Collection childListeners = getListenerChildrenWildcarded(listeners, childId); - Optional> afterChild = afterCont.getChild(childId); - DOMImmutableDataChangeEvent childChange = resolveNodeContainerChildUpdated(childPath, childListeners, - beforeChild, afterChild); - // If change is empty (equals to NO_CHANGE) - if (childChange != NO_CHANGE) { - childChanges.add(childChange); + if (resolveNodeContainerChildUpdated(state.child(childId), beforeChild, afterCont.getChild(childId))) { + childChanged = true; } } @@ -345,187 +217,120 @@ final class ResolveDataChangeEventsTask implements Callable childListeners = getListenerChildrenWildcarded(listeners, childId); - YangInstanceIdentifier childPath = path.node(childId); - childChanges.add(resolveSameEventRecursivelly(childPath , childListeners, afterChild, - DOMImmutableDataChangeEvent.getCreateEventFactory())); + resolveSameEventRecursivelly(state.child(childId), afterChild, DOMImmutableDataChangeEvent.getCreateEventFactory()); + childChanged = true; } } - if (childChanges.isEmpty()) { - return NO_CHANGE; - } - Builder eventBuilder = builder(DataChangeScope.BASE) // - .setBefore(beforeCont) // - .setAfter(afterCont) - .addUpdated(path, beforeCont, afterCont); - for (DOMImmutableDataChangeEvent childChange : childChanges) { - eventBuilder.merge(childChange); + if (childChanged) { + DOMImmutableDataChangeEvent event = DOMImmutableDataChangeEvent.builder(DataChangeScope.BASE) + .addUpdated(state.getPath(), beforeCont, afterCont).build(); + state.addEvent(event); } - DOMImmutableDataChangeEvent replaceEvent = eventBuilder.build(); - addPartialTask(listeners, replaceEvent); - return replaceEvent; + state.collectEvents(beforeCont, afterCont, collectedEvents); + return childChanged; } - private DOMImmutableDataChangeEvent resolveNodeContainerChildUpdated(final YangInstanceIdentifier path, - final Collection listeners, final NormalizedNode before, - final Optional> after) { - + private boolean resolveNodeContainerChildUpdated(final ResolveDataChangeState state, + final NormalizedNode before, final Optional> after) { if (after.isPresent()) { // REPLACE or SUBTREE Modified - return resolveReplacedEvent(path, listeners, before, after.get()); - - } else { - // AFTER state is not present - child was deleted. - return resolveSameEventRecursivelly(path, listeners, before, - DOMImmutableDataChangeEvent.getRemoveEventFactory()); + return resolveReplacedEvent(state, before, after.get()); } + + // AFTER state is not present - child was deleted. + resolveSameEventRecursivelly(state, before, DOMImmutableDataChangeEvent.getRemoveEventFactory()); + return true; } /** * Resolves create events deep down the interest listener tree. * - * * @param path * @param listeners * @param afterState * @return */ - private DOMImmutableDataChangeEvent resolveCreateEvent(final YangInstanceIdentifier path, - final Collection listeners, final NormalizedNode afterState) { + private void resolveCreateEvent(final ResolveDataChangeState state, final NormalizedNode afterState) { @SuppressWarnings({ "unchecked", "rawtypes" }) final NormalizedNode node = (NormalizedNode) afterState; - return resolveSameEventRecursivelly(path, listeners, node, DOMImmutableDataChangeEvent.getCreateEventFactory()); + resolveSameEventRecursivelly(state, node, DOMImmutableDataChangeEvent.getCreateEventFactory()); } - private DOMImmutableDataChangeEvent resolveDeleteEvent(final YangInstanceIdentifier path, - final Collection listeners, final NormalizedNode beforeState) { - + private void resolveDeleteEvent(final ResolveDataChangeState state, final NormalizedNode beforeState) { @SuppressWarnings({ "unchecked", "rawtypes" }) final NormalizedNode node = (NormalizedNode) beforeState; - return resolveSameEventRecursivelly(path, listeners, node, DOMImmutableDataChangeEvent.getRemoveEventFactory()); + resolveSameEventRecursivelly(state, node, DOMImmutableDataChangeEvent.getRemoveEventFactory()); } - private DOMImmutableDataChangeEvent resolveSameEventRecursivelly(final YangInstanceIdentifier path, - final Collection listeners, final NormalizedNode node, - final SimpleEventFactory eventFactory) { - final DOMImmutableDataChangeEvent event = eventFactory.create(path, node); - DOMImmutableDataChangeEvent propagateEvent = event; + private void resolveSameEventRecursivelly(final ResolveDataChangeState state, + final NormalizedNode node, final SimpleEventFactory eventFactory) { + if (!state.needsProcessing()) { + LOG.trace("Skipping child {}", state.getPath()); + return; + } + // We have listeners for this node or it's children, so we will try // to do additional processing if (node instanceof NormalizedNodeContainer) { - LOG.trace("Resolving subtree recursive event for {}, type {}", path, eventFactory); - - Builder eventBuilder = builder(DataChangeScope.BASE); - eventBuilder.merge(event); - eventBuilder.setBefore(event.getOriginalSubtree()); - eventBuilder.setAfter(event.getUpdatedSubtree()); + LOG.trace("Resolving subtree recursive event for {}, type {}", state.getPath(), eventFactory); // Node has children, so we will try to resolve it's children // changes. @SuppressWarnings("unchecked") NormalizedNodeContainer> container = (NormalizedNodeContainer>) node; for (NormalizedNode child : container.getValue()) { - PathArgument childId = child.getIdentifier(); + final PathArgument childId = child.getIdentifier(); + LOG.trace("Resolving event for child {}", childId); - Collection childListeners = getListenerChildrenWildcarded(listeners, childId); - eventBuilder.merge(resolveSameEventRecursivelly(path.node(childId), childListeners, child, eventFactory)); + resolveSameEventRecursivelly(state.child(childId), child, eventFactory); } - propagateEvent = eventBuilder.build(); } - if (!listeners.isEmpty()) { - addPartialTask(listeners, propagateEvent); - } - return propagateEvent; - } - private DOMImmutableDataChangeEvent resolveSubtreeChangeEvent(final YangInstanceIdentifier path, - final Collection listeners, final DataTreeCandidateNode modification) { + final DOMImmutableDataChangeEvent event = eventFactory.create(state.getPath(), node); + LOG.trace("Adding event {} at path {}", event, state.getPath()); + state.addEvent(event); + state.collectEvents(event.getOriginalSubtree(), event.getUpdatedSubtree(), collectedEvents); + } - Preconditions.checkArgument(modification.getDataBefore().isPresent(), "Subtree change with before-data not present at path %s", path); - Preconditions.checkArgument(modification.getDataAfter().isPresent(), "Subtree change with after-data not present at path %s", path); + private boolean resolveSubtreeChangeEvent(final ResolveDataChangeState state, final DataTreeCandidateNode modification) { + Preconditions.checkArgument(modification.getDataBefore().isPresent(), "Subtree change with before-data not present at path %s", state.getPath()); + Preconditions.checkArgument(modification.getDataAfter().isPresent(), "Subtree change with after-data not present at path %s", state.getPath()); - Builder one = builder(DataChangeScope.ONE). - setBefore(modification.getDataBefore().get()). - setAfter(modification.getDataAfter().get()); - Builder subtree = builder(DataChangeScope.SUBTREE). - setBefore(modification.getDataBefore().get()). - setAfter(modification.getDataAfter().get()); - boolean oneModified = false; + DataChangeScope scope = null; for (DataTreeCandidateNode childMod : modification.getChildNodes()) { - PathArgument childId = childMod.getIdentifier(); - YangInstanceIdentifier childPath = path.node(childId); - Collection childListeners = getListenerChildrenWildcarded(listeners, childId); - + final ResolveDataChangeState childState = state.child(childMod.getIdentifier()); switch (childMod.getModificationType()) { case WRITE: case MERGE: case DELETE: - one.merge(resolveAnyChangeEvent(childPath, childListeners, childMod)); - oneModified = true; + if (resolveAnyChangeEvent(childState, childMod)) { + scope = DataChangeScope.ONE; + } break; case SUBTREE_MODIFIED: - subtree.merge(resolveSubtreeChangeEvent(childPath, childListeners, childMod)); + if (resolveSubtreeChangeEvent(childState, childMod) && scope == null) { + scope = DataChangeScope.SUBTREE; + } break; case UNMODIFIED: // no-op break; } } - final DOMImmutableDataChangeEvent oneChangeEvent; - if(oneModified) { - one.addUpdated(path, modification.getDataBefore().get(), modification.getDataAfter().get()); - oneChangeEvent = one.build(); - subtree.merge(oneChangeEvent); - } else { - oneChangeEvent = null; - subtree.addUpdated(path, modification.getDataBefore().get(), modification.getDataAfter().get()); - } - DOMImmutableDataChangeEvent subtreeEvent = subtree.build(); - if (!listeners.isEmpty()) { - if(oneChangeEvent != null) { - addPartialTask(listeners, oneChangeEvent); - } - addPartialTask(listeners, subtreeEvent); - } - return subtreeEvent; - } - private DOMImmutableDataChangeEvent addPartialTask(final Collection listeners, - final DOMImmutableDataChangeEvent event) { - for (ListenerTree.Node listenerNode : listeners) { - if (!listenerNode.getListeners().isEmpty()) { - LOG.trace("Adding event {} for listeners {}",event,listenerNode); - events.put(listenerNode, event); - } - } - return event; - } + final NormalizedNode before = modification.getDataBefore().get(); + final NormalizedNode after = modification.getDataAfter().get(); - private static Collection getListenerChildrenWildcarded(final Collection parentNodes, - final PathArgument child) { - if (parentNodes.isEmpty()) { - return Collections.emptyList(); - } - com.google.common.collect.ImmutableList.Builder result = ImmutableList.builder(); - if (child instanceof NodeWithValue || child instanceof NodeIdentifierWithPredicates) { - NodeIdentifier wildcardedIdentifier = new NodeIdentifier(child.getNodeType()); - addChildrenNodesToBuilder(result, parentNodes, wildcardedIdentifier); + if (scope != null) { + DOMImmutableDataChangeEvent one = DOMImmutableDataChangeEvent.builder(scope).addUpdated(state.getPath(), before, after).build(); + state.addEvent(one); } - addChildrenNodesToBuilder(result, parentNodes, child); - return result.build(); - } - private static void addChildrenNodesToBuilder(final ImmutableList.Builder result, - final Collection parentNodes, final PathArgument childIdentifier) { - for (ListenerTree.Node node : parentNodes) { - Optional child = node.getChild(childIdentifier); - if (child.isPresent()) { - result.add(child.get()); - } - } + state.collectEvents(before, after, collectedEvents); + return scope != null; } @SuppressWarnings("rawtypes") diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ResolveDataChangeState.java b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ResolveDataChangeState.java new file mode 100644 index 0000000000..dca2eff705 --- /dev/null +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/main/java/org/opendaylight/controller/md/sal/dom/store/impl/ResolveDataChangeState.java @@ -0,0 +1,227 @@ +/* + * 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.dom.store.impl; + +import com.google.common.base.Optional; +import com.google.common.base.Preconditions; +import com.google.common.collect.Iterables; +import com.google.common.collect.Multimap; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; + +import org.opendaylight.controller.md.sal.common.api.data.AsyncDataBroker.DataChangeScope; +import org.opendaylight.controller.md.sal.dom.store.impl.DOMImmutableDataChangeEvent.Builder; +import org.opendaylight.controller.md.sal.dom.store.impl.tree.ListenerTree.Node; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Recursion state used in {@link ResolveDataChangeEventsTask}. Instances of this + * method track which listeners are affected by a particular change node. It takes + * care of properly inheriting SUB/ONE listeners and also provides a means to + * understand when actual processing need not occur. + */ +final class ResolveDataChangeState { + private static final Logger LOG = LoggerFactory.getLogger(ResolveDataChangeState.class); + /** + * Inherited from all parents + */ + private final Iterable inheritedSub; + /** + * Inherited from immediate parent + */ + private final Iterable inheritedOne; + private final YangInstanceIdentifier nodeId; + private final Collection nodes; + + private final Map, Builder> subBuilders = new HashMap<>(); + private final Map, Builder> oneBuilders = new HashMap<>(); + private final Map, Builder> baseBuilders = new HashMap<>(); + + private ResolveDataChangeState(final YangInstanceIdentifier nodeId, + final Iterable inheritedSub, final Iterable inheritedOne, + final Collection nodes) { + this.nodeId = Preconditions.checkNotNull(nodeId); + this.nodes = Preconditions.checkNotNull(nodes); + this.inheritedSub = Preconditions.checkNotNull(inheritedSub); + this.inheritedOne = Preconditions.checkNotNull(inheritedOne); + + /* + * Collect the nodes which need to be propagated from us to the child. + */ + for (Node n : nodes) { + for (DataChangeListenerRegistration l : n.getListeners()) { + final Builder b = DOMImmutableDataChangeEvent.builder(DataChangeScope.BASE); + switch (l.getScope()) { + case BASE: + baseBuilders.put(l, b); + break; + case ONE: + oneBuilders.put(l, b); + break; + case SUBTREE: + subBuilders.put(l, b); + break; + } + } + } + } + + /** + * Create an initial state handle at a particular root node. + * + * @param rootId root instance identifier + * @param root root node + * @return + */ + public static ResolveDataChangeState initial(final YangInstanceIdentifier rootId, final Node root) { + return new ResolveDataChangeState(rootId, Collections.emptyList(), + Collections.emptyList(), Collections.singletonList(root)); + } + + /** + * Create a state handle for iterating over a particular child. + * + * @param childId ID of the child + * @return State handle + */ + public ResolveDataChangeState child(final PathArgument childId) { + return new ResolveDataChangeState(nodeId.node(childId), + Iterables.concat(inheritedSub, subBuilders.values()), + oneBuilders.values(), getListenerChildrenWildcarded(nodes, childId)); + } + + /** + * Get the current path + * + * @return Current path. + */ + public YangInstanceIdentifier getPath() { + return nodeId; + } + + /** + * Check if this child needs processing. + * + * @return True if processing needs to occur, false otherwise. + */ + public boolean needsProcessing() { + // May have underlying listeners, so we need to process + if (!nodes.isEmpty()) { + return true; + } + // Have SUBTREE listeners + if (!Iterables.isEmpty(inheritedSub)) { + return true; + } + // Have ONE listeners + if (!Iterables.isEmpty(inheritedOne)) { + return true; + } + + // FIXME: do we need anything else? If not, flip this to 'false' + return true; + } + + /** + * Add an event to all current listeners. + * + * @param event + */ + public void addEvent(final DOMImmutableDataChangeEvent event) { + // Subtree builders get always notified + for (Builder b : subBuilders.values()) { + b.merge(event); + } + for (Builder b : inheritedSub) { + b.merge(event); + } + + if (event.getScope() == DataChangeScope.ONE || event.getScope() == DataChangeScope.BASE) { + for (Builder b : oneBuilders.values()) { + b.merge(event); + } + } + + if (event.getScope() == DataChangeScope.BASE) { + for (Builder b : inheritedOne) { + b.merge(event); + } + for (Builder b : baseBuilders.values()) { + b.merge(event); + } + } + } + + /** + * Gather all non-empty events into the provided map. + * + * @param before before-image + * @param after after-image + * @param map target map + */ + public void collectEvents(final NormalizedNode before, final NormalizedNode after, + final Multimap, DOMImmutableDataChangeEvent> map) { + for (Entry, Builder> e : baseBuilders.entrySet()) { + final Builder b = e.getValue(); + if (!b.isEmpty()) { + map.put(e.getKey(), b.setBefore(before).setAfter(after).build()); + } + } + for (Entry, Builder> e : oneBuilders.entrySet()) { + final Builder b = e.getValue(); + if (!b.isEmpty()) { + map.put(e.getKey(), b.setBefore(before).setAfter(after).build()); + } + } + for (Entry, Builder> e : subBuilders.entrySet()) { + final Builder b = e.getValue(); + if (!b.isEmpty()) { + map.put(e.getKey(), b.setBefore(before).setAfter(after).build()); + } + } + + LOG.trace("Collected events {}", map); + } + + private static Collection getListenerChildrenWildcarded(final Collection parentNodes, + final PathArgument child) { + if (parentNodes.isEmpty()) { + return Collections.emptyList(); + } + + final List result = new ArrayList<>(); + if (child instanceof NodeWithValue || child instanceof NodeIdentifierWithPredicates) { + NodeIdentifier wildcardedIdentifier = new NodeIdentifier(child.getNodeType()); + addChildNodes(result, parentNodes, wildcardedIdentifier); + } + addChildNodes(result, parentNodes, child); + return result; + } + + private static void addChildNodes(final List result, final Collection parentNodes, final PathArgument childIdentifier) { + for (Node node : parentNodes) { + Optional child = node.getChild(childIdentifier); + if (child.isPresent()) { + result.add(child.get()); + } + } + } +} diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/WildcardedScopeBaseTest.java b/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/WildcardedScopeBaseTest.java index cdf465aace..ddbba76ae0 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/WildcardedScopeBaseTest.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/WildcardedScopeBaseTest.java @@ -34,8 +34,11 @@ public class WildcardedScopeBaseTest extends DefaultDataChangeListenerTestSuite assertNotNull(change); - assertNotContains(change.getCreatedData(), TOP_LEVEL); - assertContains(change.getCreatedData(), path(FOO), path(FOO, BAR)); + /* + * Created data must not contain nested-list item, since that is two-level deep. + */ + assertNotContains(change.getCreatedData(), TOP_LEVEL,path(FOO, BAR)); + assertContains(change.getCreatedData(), path(FOO) ); assertEmpty(change.getUpdatedData()); assertEmpty(change.getRemovedPaths()); @@ -48,11 +51,18 @@ public class WildcardedScopeBaseTest extends DefaultDataChangeListenerTestSuite AsyncDataChangeEvent> change = task.getChangeEvent(); assertNotNull(change); - - assertContains(change.getCreatedData(), path(FOO, BAZ)); + /* + * Created data must NOT contain nested-list item since scope is base, and change is two + * level deep. + */ + assertNotContains(change.getCreatedData(), path(FOO, BAZ)); assertContains(change.getUpdatedData(), path(FOO)); assertNotContains(change.getUpdatedData(), TOP_LEVEL); - assertContains(change.getRemovedPaths(), path(FOO, BAR)); + /* + * Removed data must NOT contain nested-list item since scope is base, and change is two + * level deep. + */ + assertNotContains(change.getRemovedPaths(), path(FOO, BAR)); } @@ -64,8 +74,9 @@ public class WildcardedScopeBaseTest extends DefaultDataChangeListenerTestSuite assertNotNull(change); assertFalse(change.getCreatedData().isEmpty()); - assertContains(change.getCreatedData(), path(FOO), path(FOO, BAR), path(FOO, BAZ)); - assertNotContains(change.getCreatedData(), TOP_LEVEL); + // Base event should contain only changed item, no details about child. + assertContains(change.getCreatedData(), path(FOO)); + assertNotContains(change.getCreatedData(), TOP_LEVEL,path(FOO, BAR), path(FOO, BAZ)); assertEmpty(change.getUpdatedData()); assertEmpty(change.getRemovedPaths()); @@ -95,7 +106,12 @@ public class WildcardedScopeBaseTest extends DefaultDataChangeListenerTestSuite assertEmpty(change.getUpdatedData()); assertNotContains(change.getUpdatedData(), TOP_LEVEL); - assertContains(change.getRemovedPaths(), path(FOO),path(FOO, BAZ),path(FOO,BAR)); + /* + * Scope base listener event should contain top-level-list item and nested list path + * and should not contain baz, bar which are two-level deep + */ + assertContains(change.getRemovedPaths(), path(FOO)); + assertNotContains(change.getRemovedPaths(),path(FOO, BAZ),path(FOO,BAR)); } @Override diff --git a/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/WildcardedScopeOneTest.java b/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/WildcardedScopeOneTest.java index 3407e0ffa4..75f9fce612 100644 --- a/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/WildcardedScopeOneTest.java +++ b/opendaylight/md-sal/sal-inmemory-datastore/src/test/java/org/opendaylight/controller/md/sal/dom/store/impl/WildcardedScopeOneTest.java @@ -14,6 +14,7 @@ import java.util.concurrent.ExecutionException; import org.opendaylight.controller.md.sal.common.api.data.AsyncDataBroker.DataChangeScope; import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeEvent; import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.two.level.list.TopLevelList; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.two.level.list.top.level.list.NestedList; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; @@ -34,8 +35,8 @@ public class WildcardedScopeOneTest extends DefaultDataChangeListenerTestSuite { assertNotNull(change); - assertNotContains(change.getCreatedData(), TOP_LEVEL); - assertContains(change.getCreatedData(), path(FOO), path(FOO, BAR)); + assertNotContains(change.getCreatedData(), TOP_LEVEL,path(FOO, BAR)); + assertContains(change.getCreatedData(), path(FOO), path(FOO).node(NestedList.QNAME)); assertEmpty(change.getUpdatedData()); assertEmpty(change.getRemovedPaths()); @@ -48,11 +49,18 @@ public class WildcardedScopeOneTest extends DefaultDataChangeListenerTestSuite { AsyncDataChangeEvent> change = task.getChangeEvent(); assertNotNull(change); - - assertContains(change.getCreatedData(), path(FOO, BAZ)); - assertContains(change.getUpdatedData(), path(FOO)); + /* + * Created data must NOT contain nested-list item since scope is base, and change is two + * level deep. + */ + assertNotContains(change.getCreatedData(), path(FOO, BAZ)); + assertContains(change.getUpdatedData(), path(FOO),path(FOO).node(NestedList.QNAME)); assertNotContains(change.getUpdatedData(), TOP_LEVEL); - assertContains(change.getRemovedPaths(), path(FOO, BAR)); + /* + * Removed data must NOT contain nested-list item since scope is base, and change is two + * level deep. + */ + assertNotContains(change.getRemovedPaths(), path(FOO, BAR)); } @@ -64,8 +72,9 @@ public class WildcardedScopeOneTest extends DefaultDataChangeListenerTestSuite { assertNotNull(change); assertFalse(change.getCreatedData().isEmpty()); - assertContains(change.getCreatedData(), path(FOO), path(FOO, BAR), path(FOO, BAZ)); - assertNotContains(change.getCreatedData(), TOP_LEVEL); + // Base event should contain only changed item, and details about immediate child. + assertContains(change.getCreatedData(), path(FOO),path(FOO).node(NestedList.QNAME)); + assertNotContains(change.getCreatedData(), TOP_LEVEL,path(FOO, BAR), path(FOO, BAZ)); assertEmpty(change.getUpdatedData()); assertEmpty(change.getRemovedPaths()); @@ -96,7 +105,8 @@ public class WildcardedScopeOneTest extends DefaultDataChangeListenerTestSuite { assertEmpty(change.getUpdatedData()); assertNotContains(change.getUpdatedData(), TOP_LEVEL); - assertContains(change.getRemovedPaths(), path(FOO),path(FOO, BAZ),path(FOO,BAR)); + assertContains(change.getRemovedPaths(), path(FOO),path(FOO).node(NestedList.QNAME)); + assertNotContains(change.getRemovedPaths(), path(FOO, BAZ),path(FOO,BAR)); } @Override