From 9f6b16d6e8ade6dfa40e9bbf4196d55adf8f2fec Mon Sep 17 00:00:00 2001 From: Maros Marsalek Date: Wed, 29 Jun 2016 09:14:51 +0200 Subject: HONEYCOMB-94 Reimplement writer registry with better ordering options Now the registry is flat and allows for full control of writer execution order Change-Id: I864e1d676588ffe59b596145e0829e81b1a1ed2f Signed-off-by: Maros Marsalek --- .../data/impl/ModifiableDataTreeDelegator.java | 228 ++++++--------------- 1 file changed, 60 insertions(+), 168 deletions(-) (limited to 'v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ModifiableDataTreeDelegator.java') diff --git a/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ModifiableDataTreeDelegator.java b/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ModifiableDataTreeDelegator.java index 91de1885e..77aa12aba 100644 --- a/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ModifiableDataTreeDelegator.java +++ b/v3po/data-impl/src/main/java/io/fd/honeycomb/v3po/data/impl/ModifiableDataTreeDelegator.java @@ -16,38 +16,36 @@ package io.fd.honeycomb.v3po.data.impl; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.util.concurrent.Futures.immediateCheckedFuture; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Optional; -import com.google.common.collect.Sets; +import com.google.common.collect.HashMultimap; +import com.google.common.collect.Multimap; import com.google.common.util.concurrent.CheckedFuture; import io.fd.honeycomb.v3po.data.DataModification; import io.fd.honeycomb.v3po.data.ReadableDataManager; import io.fd.honeycomb.v3po.translate.TranslationException; +import io.fd.honeycomb.v3po.translate.util.RWUtils; import io.fd.honeycomb.v3po.translate.util.write.TransactionMappingContext; import io.fd.honeycomb.v3po.translate.util.write.TransactionWriteContext; +import io.fd.honeycomb.v3po.translate.write.DataObjectUpdate; import io.fd.honeycomb.v3po.translate.write.WriteContext; import io.fd.honeycomb.v3po.translate.write.WriterRegistry; -import java.util.Collections; -import java.util.EnumSet; -import java.util.HashMap; import java.util.Map; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException; import org.opendaylight.controller.md.sal.dom.api.DOMDataReadOnlyTransaction; import org.opendaylight.yangtools.binding.data.codec.api.BindingNormalizedNodeSerializer; import org.opendaylight.yangtools.yang.binding.DataObject; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; -import org.opendaylight.yangtools.yang.data.api.schema.LeafNode; 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.ModificationType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -110,6 +108,7 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager @Override protected void processCandidate(final DataTreeCandidate candidate) throws TranslationException { + final DataTreeCandidateNode rootNode = candidate.getRootNode(); final YangInstanceIdentifier rootPath = candidate.getRootPath(); LOG.trace("ConfigDataTree.modify() rootPath={}, rootNode={}, dataBefore={}, dataAfter={}", @@ -119,13 +118,12 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager ModificationDiff.recursivelyFromCandidate(YangInstanceIdentifier.EMPTY, rootNode); LOG.debug("ConfigDataTree.modify() diff: {}", modificationDiff); - final Map, DataObject> nodesBefore = toBindingAware(modificationDiff.getModificationsBefore()); - LOG.debug("ConfigDataTree.modify() extracted nodesBefore={}", nodesBefore.keySet()); - final Map, DataObject> nodesAfter = toBindingAware(modificationDiff.getModificationsAfter()); - LOG.debug("ConfigDataTree.modify() extracted nodesAfter={}", nodesAfter.keySet()); + // Distinguish between updates (create + update) and deletes + final WriterRegistry.DataObjectUpdates baUpdates = toBindingAware(modificationDiff.getUpdates()); + LOG.debug("ConfigDataTree.modify() extracted updates={}", baUpdates); try (final WriteContext ctx = getTransactionWriteContext()) { - writerRegistry.update(nodesBefore, nodesAfter, ctx); + writerRegistry.update(baUpdates, ctx); final CheckedFuture contextUpdateResult = ((TransactionMappingContext) ctx.getMappingContext()).submit(); @@ -152,7 +150,7 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager LOG.error(msg, e); throw new TranslationException(msg, e); } catch (TranslationException e) { - LOG.error("Error while processing data change (before={}, after={})", nodesBefore, nodesAfter, e); + LOG.error("Error while processing data change (updates={})", baUpdates, e); throw e; } } @@ -167,177 +165,71 @@ public final class ModifiableDataTreeDelegator extends ModifiableDataTreeManager return new TransactionWriteContext(serializer, beforeTx, afterTx, mappingContext); } - private Map, DataObject> toBindingAware(final Map> biNodes) { + private WriterRegistry.DataObjectUpdates toBindingAware( + final Map biNodes) { return ModifiableDataTreeDelegator.toBindingAware(biNodes, serializer); } } @VisibleForTesting - static Map, DataObject> toBindingAware(final Map> biNodes, - final BindingNormalizedNodeSerializer serializer) { - final HashMap, DataObject> transformed = new HashMap<>(biNodes.size()); - for (Map.Entry> biEntry : biNodes.entrySet()) { - final Map.Entry, DataObject> baEntry = serializer.fromNormalizedNode(biEntry.getKey(), biEntry.getValue()); - if (baEntry != null) { - transformed.put(baEntry.getKey(), baEntry.getValue()); + static WriterRegistry.DataObjectUpdates toBindingAware( + final Map biNodes, + final BindingNormalizedNodeSerializer serializer) { + + final Multimap, DataObjectUpdate> dataObjectUpdates = HashMultimap.create(); + final Multimap, DataObjectUpdate.DataObjectDelete> dataObjectDeletes = + HashMultimap.create(); + + for (Map.Entry biEntry : biNodes.entrySet()) { + final InstanceIdentifier unkeyedIid = + RWUtils.makeIidWildcarded(serializer.fromYangInstanceIdentifier(biEntry.getKey())); + + ModificationDiff.NormalizedNodeUpdate normalizedNodeUpdate = biEntry.getValue(); + final DataObjectUpdate dataObjectUpdate = toDataObjectUpdate(normalizedNodeUpdate, serializer); + if (dataObjectUpdate != null) { + if (dataObjectUpdate instanceof DataObjectUpdate.DataObjectDelete) { + dataObjectDeletes.put(unkeyedIid, ((DataObjectUpdate.DataObjectDelete) dataObjectUpdate)); + } else { + dataObjectUpdates.put(unkeyedIid, dataObjectUpdate); + } } } - return transformed; + return new WriterRegistry.DataObjectUpdates(dataObjectUpdates, dataObjectDeletes); } - @VisibleForTesting - static final class ModificationDiff { - - private static final ModificationDiff EMPTY_DIFF = new ModificationDiff(Collections.emptyMap(), Collections.emptyMap()); - private static final EnumSet LEAF_MODIFICATIONS = EnumSet.of(ModificationType.WRITE, ModificationType.DELETE); - - private final Map> modificationsBefore; - private final Map> modificationsAfter; + @Nullable + private static DataObjectUpdate toDataObjectUpdate( + final ModificationDiff.NormalizedNodeUpdate normalizedNodeUpdate, + final BindingNormalizedNodeSerializer serializer) { - private ModificationDiff(@Nonnull final Map> modificationsBefore, - @Nonnull final Map> modificationsAfter) { - this.modificationsBefore = modificationsBefore; - this.modificationsAfter = modificationsAfter; - } - - Map> getModificationsBefore() { - return modificationsBefore; - } + InstanceIdentifier baId = serializer.fromYangInstanceIdentifier(normalizedNodeUpdate.getId()); + checkNotNull(baId, "Unable to transform instance identifier: %s into BA", normalizedNodeUpdate.getId()); - Map> getModificationsAfter() { - return modificationsAfter; - } - - private ModificationDiff merge(final ModificationDiff other) { - if (this == EMPTY_DIFF) { - return other; - } + DataObject dataObjectBefore = getDataObject(serializer, + normalizedNodeUpdate.getDataBefore(), normalizedNodeUpdate.getId()); + DataObject dataObjectAfter = + getDataObject(serializer, normalizedNodeUpdate.getDataAfter(), normalizedNodeUpdate.getId()); - if (other == EMPTY_DIFF) { - return this; - } - - return new ModificationDiff(join(modificationsBefore, other.modificationsBefore), - join(modificationsAfter, other.modificationsAfter)); - } - - private static Map> join( - final Map> mapOne, - final Map> mapTwo) { - // Check unique modifications - // TODO Probably not necessary to check - final Sets.SetView duplicates = Sets.intersection(mapOne.keySet(), mapTwo.keySet()); - checkArgument(duplicates.size() == 0, "Duplicates detected: %s. In maps: %s and %s", duplicates, mapOne, mapTwo); - final HashMap> joined = new HashMap<>(); - joined.putAll(mapOne); - joined.putAll(mapTwo); - return joined; - } - - private static ModificationDiff createFromBefore(YangInstanceIdentifier idBefore, DataTreeCandidateNode candidate) { - return new ModificationDiff( - Collections.singletonMap(idBefore, candidate.getDataBefore().get()), - Collections.emptyMap()); - } - - private static ModificationDiff create(YangInstanceIdentifier id, DataTreeCandidateNode candidate) { - return new ModificationDiff( - Collections.singletonMap(id, candidate.getDataBefore().get()), - Collections.singletonMap(id, candidate.getDataAfter().get())); - } - - private static ModificationDiff createFromAfter(YangInstanceIdentifier idAfter, DataTreeCandidateNode candidate) { - return new ModificationDiff( - Collections.emptyMap(), - Collections.singletonMap(idAfter, candidate.getDataAfter().get())); - } - - /** - * Produce a diff from a candidate node recursively. - */ - @Nonnull - static ModificationDiff recursivelyFromCandidate(@Nonnull final YangInstanceIdentifier yangIid, - @Nonnull final DataTreeCandidateNode currentCandidate) { - switch (currentCandidate.getModificationType()) { - case APPEARED: - case DISAPPEARED: - case UNMODIFIED: { - // (dis)appeared nodes are not important, no real data to process - return ModificationDiff.EMPTY_DIFF; - } - case WRITE: { - return currentCandidate.getDataBefore().isPresent() - ? ModificationDiff.create(yangIid, currentCandidate) - : ModificationDiff.createFromAfter(yangIid, currentCandidate); - // TODO HONEYCOMB-94 process children recursively to get modifications for child nodes - } - case DELETE: - return ModificationDiff.createFromBefore(yangIid, currentCandidate); - case SUBTREE_MODIFIED: { - // Modifications here are presented also for leaves. However that kind of granularity is not required - // So if there's a modified leaf, mark current complex node also as modification - java.util.Optional leavesModified = currentCandidate.getChildNodes().stream() - .filter(ModificationDiff::isLeaf) - // For some reason, we get modifications on unmodified list keys TODO debug and report ODL bug - // and that messes up our modifications collection here, so we need to skip - .filter(ModificationDiff::isModification) - .map(child -> LEAF_MODIFICATIONS.contains(child.getModificationType())) - .reduce((boolOne, boolTwo) -> boolOne || boolTwo); - - if (leavesModified.isPresent() && leavesModified.get()) { - return ModificationDiff.create(yangIid, currentCandidate); - // TODO HONEYCOMB-94 process children recursively to get modifications for child nodes even if current - // was modified - } else { - // SUBTREE MODIFIED, no modification on current, but process children recursively - return currentCandidate.getChildNodes().stream() - // not interested in modifications to leaves - .filter(child -> !isLeaf(child)) - .map(candidate -> recursivelyFromCandidate(yangIid.node(candidate.getIdentifier()), candidate)) - .reduce(ModificationDiff::merge) - .orElse(EMPTY_DIFF); - } - } - default: - throw new IllegalStateException("Unknown modification type: " - + currentCandidate.getModificationType() + ". Unsupported"); - } - } + return dataObjectBefore == null && dataObjectAfter == null + ? null + : DataObjectUpdate.create(baId, dataObjectBefore, dataObjectAfter); + } - /** - * Check whether candidate.before and candidate.after is different. If not - * return false. - */ - private static boolean isModification(final DataTreeCandidateNode candidateNode) { - if (candidateNode.getDataBefore().isPresent()) { - if (candidateNode.getDataAfter().isPresent()) { - return !candidateNode.getDataAfter().get().equals(candidateNode.getDataBefore().get()); - } else { - return true; - } + @Nullable + private static DataObject getDataObject(@Nonnull final BindingNormalizedNodeSerializer serializer, + @Nullable final NormalizedNode data, + @Nonnull final YangInstanceIdentifier id) { + DataObject dataObject = null; + if (data != null) { + final Map.Entry, DataObject> dataObjectEntry = + serializer.fromNormalizedNode(id, data); + if (dataObjectEntry != null) { + dataObject = dataObjectEntry.getValue(); } - - // considering not a modification if data after is also null - return candidateNode.getDataAfter().isPresent(); - } - - /** - * Check whether candidate node is for a leaf type node. - */ - private static boolean isLeaf(final DataTreeCandidateNode candidateNode) { - // orNull intentional, some candidate nodes have both data after and data before null - return candidateNode.getDataAfter().orNull() instanceof LeafNode - || candidateNode.getDataBefore().orNull() instanceof LeafNode; - } - - @Override - public String toString() { - return "ModificationDiff{" - + "modificationsBefore=" + modificationsBefore - + ", modificationsAfter=" + modificationsAfter - + '}'; } + return dataObject; } + } -- cgit 1.2.3-korg