Bug 509: Fixed small discrepancies in Binding Data Change Events. 60/6160/6
authorRobert Varga <rovarga@cisco.com>
Mon, 14 Apr 2014 15:46:07 +0000 (17:46 +0200)
committerTony Tkacik <ttkacik@cisco.com>
Tue, 15 Apr 2014 13:18:36 +0000 (15:18 +0200)
DataNormalizer which was responsible to render representation
of Instance Identifier in original format was introducing
additional node, when translation happened from normalized
to legacy, which is only triggered by Data Change Events.

Implementation of responsible algorithm was changed to
use schema in order to derive legacy InstanceIdentifier.

Data Change Events

 - Translated Data Change events were deserialized from
   DOM format on each access to fields, which may introduced
   CPU time overhead.

   Immutability of backing Data Change Events allows to
   add lazy caching of translated TOs, since event and data
   contained in Event Object are immutable.

 - getUpdated*Data in original Binding APIs returned
   both created and modified, so backwards-compatible fix
   was introduced.

Change-Id: Idab3dffb8651d50cca79a22ddcd43af29561b80e
Signed-off-by: Tony Tkacik <ttkacik@cisco.com>
Signed-off-by: Robert Varga <rovarga@cisco.com>
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedDataBroker.java
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingToNormalizedNodeCodec.java
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/LegacyDataChangeEvent.java
opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizer.java

index 8a32b0b..685a919 100644 (file)
@@ -7,13 +7,13 @@
  */
 package org.opendaylight.controller.md.sal.binding.impl;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
 
-import org.eclipse.xtext.xbase.lib.Exceptions;
 import org.opendaylight.controller.md.sal.binding.api.BindingDataChangeListener;
 import org.opendaylight.controller.md.sal.common.api.data.AsyncDataBroker.DataChangeScope;
 import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeEvent;
@@ -36,7 +36,11 @@ import org.opendaylight.yangtools.yang.model.api.SchemaContextListener;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public abstract class AbstractForwardedDataBroker implements Delegator<DOMDataBroker>, DomForwardedBroker, SchemaContextListener {
+import com.google.common.base.Objects;
+import com.google.common.base.Optional;
+
+public abstract class AbstractForwardedDataBroker implements Delegator<DOMDataBroker>, DomForwardedBroker,
+        SchemaContextListener {
 
     private static final Logger LOG = LoggerFactory.getLogger(AbstractForwardedDataBroker.class);
     // The Broker to whom we do all forwarding
@@ -81,11 +85,12 @@ public abstract class AbstractForwardedDataBroker implements Delegator<DOMDataBr
         DOMDataChangeListener domDataChangeListener = new TranslatingDataChangeInvoker(store, path, listener,
                 triggeringScope);
         org.opendaylight.yangtools.yang.data.api.InstanceIdentifier domPath = codec.toNormalized(path);
-        ListenerRegistration<DOMDataChangeListener> domRegistration = domDataBroker.registerDataChangeListener(store, domPath, domDataChangeListener, triggeringScope);
+        ListenerRegistration<DOMDataChangeListener> domRegistration = domDataBroker.registerDataChangeListener(store,
+                domPath, domDataChangeListener, triggeringScope);
         return new ListenerRegistrationImpl(listener, domRegistration);
     }
 
-    protected Map<InstanceIdentifier<?>, DataObject> fromDOMToData(
+    protected Map<InstanceIdentifier<?>, DataObject> toBinding(
             final Map<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, ? extends NormalizedNode<?, ?>> normalized) {
         Map<InstanceIdentifier<?>, DataObject> newMap = new HashMap<>();
         for (Map.Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, ? extends NormalizedNode<?, ?>> entry : normalized
@@ -94,12 +99,38 @@ public abstract class AbstractForwardedDataBroker implements Delegator<DOMDataBr
                 Entry<InstanceIdentifier<? extends DataObject>, DataObject> binding = getCodec().toBinding(entry);
                 newMap.put(binding.getKey(), binding.getValue());
             } catch (DeserializationException e) {
-                LOG.debug("Ommiting {}",entry,e);
+                LOG.debug("Omitting {}", entry, e);
             }
         }
         return newMap;
     }
 
+    protected Set<InstanceIdentifier<?>> toBinding(
+            final Set<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier> normalized) {
+        Set<InstanceIdentifier<?>> hashSet = new HashSet<>();
+        for (org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalizedPath : normalized) {
+            try {
+                InstanceIdentifier<? extends DataObject> binding = getCodec().toBinding(normalizedPath);
+                hashSet.add(binding);
+            } catch (DeserializationException e) {
+                LOG.debug("Omitting {}", normalizedPath, e);
+            }
+        }
+        return hashSet;
+    }
+
+    protected Optional<DataObject> toBindingData(final InstanceIdentifier<?> path, final NormalizedNode<?, ?> data) {
+        if(path.isWildcarded()) {
+            return Optional.absent();
+        }
+
+        try {
+            return Optional.fromNullable(getCodec().toBinding(path, data));
+        } catch (DeserializationException e) {
+            return Optional.absent();
+        }
+    }
+
     private class TranslatingDataChangeInvoker implements DOMDataChangeListener {
         private final BindingDataChangeListener bindingDataChangeListener;
         private final LogicalDatastoreType store;
@@ -117,18 +148,20 @@ public abstract class AbstractForwardedDataBroker implements Delegator<DOMDataBr
         @Override
         public void onDataChanged(
                 final AsyncDataChangeEvent<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> change) {
-            bindingDataChangeListener.onDataChanged(new TranslatedDataChangeEvent(change,path));
+            bindingDataChangeListener.onDataChanged(new TranslatedDataChangeEvent(change, path));
         }
     }
 
     private class TranslatedDataChangeEvent implements AsyncDataChangeEvent<InstanceIdentifier<?>, DataObject> {
         private final AsyncDataChangeEvent<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> domEvent;
-        private InstanceIdentifier<?> path;
+        private final InstanceIdentifier<?> path;
 
-        public TranslatedDataChangeEvent(
-                final AsyncDataChangeEvent<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> change) {
-            this.domEvent = change;
-        }
+        private Map<InstanceIdentifier<?>, DataObject> createdCache;
+        private Map<InstanceIdentifier<?>, DataObject> updatedCache;
+        private Map<InstanceIdentifier<?>, ? extends DataObject> originalCache;
+        private Set<InstanceIdentifier<?>> removedCache;
+        private Optional<DataObject> originalDataCache;
+        private Optional<DataObject> updatedDataCache;
 
         public TranslatedDataChangeEvent(
                 final AsyncDataChangeEvent<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> change,
@@ -139,52 +172,63 @@ public abstract class AbstractForwardedDataBroker implements Delegator<DOMDataBr
 
         @Override
         public Map<InstanceIdentifier<?>, DataObject> getCreatedData() {
-            return fromDOMToData(domEvent.getCreatedData());
+            if (createdCache == null) {
+                createdCache = Collections.unmodifiableMap(toBinding(domEvent.getCreatedData()));
+            }
+            return createdCache;
         }
 
         @Override
         public Map<InstanceIdentifier<?>, DataObject> getUpdatedData() {
-            return fromDOMToData(domEvent.getUpdatedData());
+            if (updatedCache == null) {
+                updatedCache = Collections.unmodifiableMap(toBinding(domEvent.getUpdatedData()));
+            }
+            return updatedCache;
 
         }
 
         @Override
         public Set<InstanceIdentifier<?>> getRemovedPaths() {
-            final Set<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier> removedPaths = domEvent
-                    .getRemovedPaths();
-            final Set<InstanceIdentifier<?>> output = new HashSet<>();
-            for (org.opendaylight.yangtools.yang.data.api.InstanceIdentifier instanceIdentifier : removedPaths) {
-                try {
-                    output.add(mappingService.fromDataDom(instanceIdentifier));
-                } catch (DeserializationException e) {
-                    Exceptions.sneakyThrow(e);
-                }
+            if (removedCache == null) {
+                removedCache = Collections.unmodifiableSet(toBinding(domEvent.getRemovedPaths()));
             }
-
-            return output;
+            return removedCache;
         }
 
         @Override
         public Map<InstanceIdentifier<?>, ? extends DataObject> getOriginalData() {
-            return fromDOMToData(domEvent.getOriginalData());
+            if (originalCache == null) {
+                originalCache = Collections.unmodifiableMap(toBinding(domEvent.getOriginalData()));
+            }
+            return originalCache;
 
         }
 
         @Override
         public DataObject getOriginalSubtree() {
-
-            return toBindingData(path,domEvent.getOriginalSubtree());
+            if (originalDataCache == null) {
+                originalDataCache = toBindingData(path, domEvent.getOriginalSubtree());
+            }
+            return originalDataCache.orNull();
         }
 
         @Override
         public DataObject getUpdatedSubtree() {
+            if (updatedDataCache == null) {
+                updatedDataCache = toBindingData(path, domEvent.getUpdatedSubtree());
+            }
 
-            return toBindingData(path,domEvent.getUpdatedSubtree());
+            return updatedDataCache.orNull();
         }
 
         @Override
         public String toString() {
-            return "TranslatedDataChangeEvent [domEvent=" + domEvent + "]";
+            return Objects.toStringHelper(TranslatedDataChangeEvent.class) //
+                .add("created", getCreatedData()) //
+                .add("updated", getUpdatedData()) //
+                .add("removed", getRemovedPaths()) //
+                .add("dom", domEvent) //
+                .toString();
         }
     }
 
@@ -203,15 +247,6 @@ public abstract class AbstractForwardedDataBroker implements Delegator<DOMDataBr
         }
     }
 
-    protected DataObject toBindingData(final InstanceIdentifier<?> path, final NormalizedNode<?, ?> data) {
-        try {
-            return getCodec().toBinding(path, data);
-        } catch (DeserializationException e) {
-            return null;
-        }
-    }
-
-
     @Override
     public BindingIndependentConnector getConnector() {
         return this.connector;
@@ -229,7 +264,7 @@ public abstract class AbstractForwardedDataBroker implements Delegator<DOMDataBr
 
     @Override
     public void setDomProviderContext(final ProviderSession domProviderContext) {
-       this.context = domProviderContext;
+        this.context = domProviderContext;
     }
 
     @Override
@@ -237,7 +272,4 @@ public abstract class AbstractForwardedDataBroker implements Delegator<DOMDataBr
         // NOOP
     }
 
-
-
-
 }
index f885b32..35ebe76 100644 (file)
@@ -10,6 +10,7 @@ package org.opendaylight.controller.md.sal.binding.impl;
 import java.util.AbstractMap.SimpleEntry;
 import java.util.Map.Entry;
 
+import org.opendaylight.controller.md.sal.common.impl.util.compat.DataNormalizationException;
 import org.opendaylight.controller.md.sal.common.impl.util.compat.DataNormalizer;
 import org.opendaylight.yangtools.yang.binding.Augmentation;
 import org.opendaylight.yangtools.yang.binding.DataObject;
@@ -43,7 +44,10 @@ public class BindingToNormalizedNodeCodec implements SchemaContextListener {
 
     public org.opendaylight.yangtools.yang.data.api.InstanceIdentifier toNormalized(
             final InstanceIdentifier<? extends DataObject> binding) {
-        return legacyToNormalized.toNormalized(bindingToLegacy.toDataDom(binding));
+        final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier legacyPath = bindingToLegacy.toDataDom(binding);
+        final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalized = legacyToNormalized.toNormalized(legacyPath);
+        LOG.trace("InstanceIdentifier Path {} Serialization: Legacy representation {}, Normalized representation: {}",binding,legacyPath,normalized);
+        return normalized;
     }
 
     public Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> toNormalizedNode(
@@ -54,7 +58,9 @@ public class BindingToNormalizedNodeCodec implements SchemaContextListener {
 
     public Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> toNormalizedNode(
             final Entry<org.opendaylight.yangtools.yang.binding.InstanceIdentifier<? extends DataObject>, DataObject> binding) {
-        Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> normalizedEntry = legacyToNormalized.toNormalized(bindingToLegacy.toDataDom(binding));
+        Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, CompositeNode> legacyEntry = bindingToLegacy.toDataDom(binding);
+        Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> normalizedEntry = legacyToNormalized.toNormalized(legacyEntry);
+        LOG.trace("Serialization of {}, Legacy Representation: {}, Normalized Representation: {}",binding,legacyEntry,normalizedEntry);
         if(Augmentation.class.isAssignableFrom(binding.getKey().getTargetType())) {
 
             for(DataContainerChild<? extends PathArgument, ?> child : ((DataContainerNode<?>) normalizedEntry.getValue()).getValue()) {
@@ -78,8 +84,13 @@ public class BindingToNormalizedNodeCodec implements SchemaContextListener {
             final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalized)
             throws DeserializationException {
 
-        org.opendaylight.yangtools.yang.data.api.InstanceIdentifier legacyPath = legacyToNormalized
-                .toLegacy(normalized);
+        org.opendaylight.yangtools.yang.data.api.InstanceIdentifier legacyPath;
+        try {
+            legacyPath = legacyToNormalized.toLegacy(normalized);
+        } catch (DataNormalizationException e) {
+            throw new IllegalStateException("Could not denormalize path.",e);
+        }
+        LOG.trace("InstanceIdentifier Path Deserialization: Legacy representation {}, Normalized representation: {}",legacyPath,normalized);
         return bindingToLegacy.fromDataDom(legacyPath);
     }
 
index 8cb4a70..ce1ff45 100644 (file)
@@ -8,6 +8,7 @@
 package org.opendaylight.controller.md.sal.binding.impl;
 
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
 
@@ -96,6 +97,7 @@ public abstract class LegacyDataChangeEvent implements
     private final static class OperationalChangeEvent extends LegacyDataChangeEvent {
 
         private final AsyncDataChangeEvent<InstanceIdentifier<?>, DataObject> delegate;
+        private Map<InstanceIdentifier<?>, DataObject> updatedCache;
 
         public OperationalChangeEvent(final AsyncDataChangeEvent<InstanceIdentifier<?>, DataObject> change) {
             this.delegate = change;
@@ -128,7 +130,15 @@ public abstract class LegacyDataChangeEvent implements
 
         @Override
         public Map<InstanceIdentifier<?>, DataObject> getUpdatedOperationalData() {
-            return delegate.getUpdatedData();
+            if(updatedCache == null) {
+                Map<InstanceIdentifier<?>, DataObject> created = delegate.getCreatedData();
+                Map<InstanceIdentifier<?>, DataObject> updated = delegate.getUpdatedData();
+                HashMap<InstanceIdentifier<?>, DataObject> updatedComposite = new HashMap<>(created.size() + updated.size());
+                updatedComposite.putAll(created);
+                updatedComposite.putAll(updated);
+                updatedCache = Collections.unmodifiableMap(updatedComposite);
+            }
+            return updatedCache;
         }
 
         @Override
@@ -142,6 +152,7 @@ public abstract class LegacyDataChangeEvent implements
     private final static class ConfigurationChangeEvent extends LegacyDataChangeEvent {
 
         private final AsyncDataChangeEvent<InstanceIdentifier<?>, DataObject> delegate;
+        private Map<InstanceIdentifier<?>, DataObject> updatedCache;
 
         public ConfigurationChangeEvent(final AsyncDataChangeEvent<InstanceIdentifier<?>, DataObject> change) {
             this.delegate = change;
@@ -174,7 +185,15 @@ public abstract class LegacyDataChangeEvent implements
 
         @Override
         public Map<InstanceIdentifier<?>, DataObject> getUpdatedConfigurationData() {
-            return delegate.getUpdatedData();
+            if(updatedCache == null) {
+                Map<InstanceIdentifier<?>, DataObject> created = delegate.getCreatedData();
+                Map<InstanceIdentifier<?>, DataObject> updated = delegate.getUpdatedData();
+                HashMap<InstanceIdentifier<?>, DataObject> updatedComposite = new HashMap<>(created.size() + updated.size());
+                updatedComposite.putAll(created);
+                updatedComposite.putAll(updated);
+                updatedCache = Collections.unmodifiableMap(updatedComposite);
+            }
+            return updatedCache;
         }
 
         @Override
index e52e196..8fb6ff3 100644 (file)
@@ -18,8 +18,6 @@ import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.CompositeNode;
 import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.AugmentationIdentifier;
-import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.NodeIdentifier;
-import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.NodeIdentifierWithPredicates;
 import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.api.Node;
 import org.opendaylight.yangtools.yang.data.api.SimpleNode;
@@ -117,25 +115,15 @@ public class DataNormalizer {
                 currentOp.normalize(legacyData));
     }
 
-    public InstanceIdentifier toLegacy(final InstanceIdentifier normalized) {
+    public InstanceIdentifier toLegacy(final InstanceIdentifier normalized) throws DataNormalizationException {
         ImmutableList.Builder<PathArgument> legacyArgs = ImmutableList.builder();
         PathArgument previous = null;
+        DataNormalizationOperation<?> currentOp = operation;
         for (PathArgument normalizedArg : normalized.getPath()) {
-            if (normalizedArg instanceof NodeIdentifier) {
-                if (previous != null) {
-                    legacyArgs.add(previous);
-                }
-                previous = normalizedArg;
-            } else if (normalizedArg instanceof NodeIdentifierWithPredicates) {
-                // We skip previous node, which was mixin.
-                previous = normalizedArg;
-            } else if (normalizedArg instanceof AugmentationIdentifier) {
-                // We ignore argument
+            currentOp = currentOp.getChild(normalizedArg);
+            if(!currentOp.isMixin()) {
+                legacyArgs.add(normalizedArg);
             }
-            // FIXME : Add option for reading choice
-        }
-        if (previous != null) {
-            legacyArgs.add(previous);
         }
         return new InstanceIdentifier(legacyArgs.build());
     }

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.