From b7c373b1edb9864feb6811bb6c370943d3e8f75f Mon Sep 17 00:00:00 2001 From: Tony Tkacik Date: Sun, 11 May 2014 22:46:24 +0200 Subject: [PATCH 1/1] Bug 981: Fixed accidental shaddowing of data in Data Change Event. Not all Normalized DOM InstanceIdentifier has their counterpart in Binding-Aware representation. Added explicit check to detect mixin instance-identifiers which could lead to incorrect deserialization into maps and shadowing parent data. Change-Id: I66166f50c636ce0dfcd962efac7fc488da622336 Signed-off-by: Tony Tkacik --- .../impl/AbstractForwardedDataBroker.java | 27 ++-- .../impl/BindingToNormalizedNodeCodec.java | 115 ++++++++++++------ 2 files changed, 97 insertions(+), 45 deletions(-) diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedDataBroker.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedDataBroker.java index fa48416484..3accaef5e9 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedDataBroker.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedDataBroker.java @@ -96,8 +96,12 @@ public abstract class AbstractForwardedDataBroker implements Delegator> entry : normalized .entrySet()) { try { - Entry, DataObject> binding = getCodec().toBinding(entry); - newMap.put(binding.getKey(), binding.getValue()); + Optional, DataObject>> potential = getCodec().toBinding( + entry); + if (potential.isPresent()) { + Entry, DataObject> binding = potential.get(); + newMap.put(binding.getKey(), binding.getValue()); + } } catch (DeserializationException e) { LOG.warn("Failed to transform {}, omitting it", entry, e); } @@ -110,8 +114,11 @@ public abstract class AbstractForwardedDataBroker implements Delegator> hashSet = new HashSet<>(); for (org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalizedPath : normalized) { try { - InstanceIdentifier binding = getCodec().toBinding(normalizedPath); - hashSet.add(binding); + Optional> potential = getCodec().toBinding(normalizedPath); + if (potential.isPresent()) { + InstanceIdentifier binding = potential.get(); + hashSet.add(binding); + } } catch (DeserializationException e) { LOG.warn("Failed to transform {}, omitting it", normalizedPath, e); } @@ -120,7 +127,7 @@ public abstract class AbstractForwardedDataBroker implements Delegator toBindingData(final InstanceIdentifier path, final NormalizedNode data) { - if(path.isWildcarded()) { + if (path.isWildcarded()) { return Optional.absent(); } @@ -224,11 +231,11 @@ public abstract class AbstractForwardedDataBroker implements Delegator binding) { - // Used instance-identifier codec do not support serialization of last path + // Used instance-identifier codec do not support serialization of last + // path // argument if it is Augmentation (behaviour expected by old datastore) // in this case, we explicitly check if last argument is augmentation // to process it separately @@ -103,13 +106,24 @@ public class BindingToNormalizedNodeCodec implements SchemaContextListener { } - public InstanceIdentifier toBinding( + /** + * + * Returns a Binding-Aware instance identifier from normalized + * instance-identifier if it is possible to create representation. + * + * Returns Optional.absent for cases where target is mixin node except + * augmentation. + * + */ + public Optional> toBinding( final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalized) throws DeserializationException { PathArgument lastArgument = Iterables.getLast(normalized.getPath()); - // Used instance-identifier codec do not support serialization of last path - // argument if it is AugmentationIdentifier (behaviour expected by old datastore) + // Used instance-identifier codec do not support serialization of last + // path + // argument if it is AugmentationIdentifier (behaviour expected by old + // datastore) // in this case, we explicitly check if last argument is augmentation // to process it separately if (lastArgument instanceof AugmentationIdentifier) { @@ -118,46 +132,71 @@ public class BindingToNormalizedNodeCodec implements SchemaContextListener { return toBindingImpl(normalized); } - private InstanceIdentifier toBindingAugmented( - final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalized) throws DeserializationException { - InstanceIdentifier potential = toBindingImpl(normalized); + private Optional> toBindingAugmented( + final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalized) + throws DeserializationException { + Optional> potential = toBindingImpl(normalized); // Shorthand check, if codec already supports deserialization // of AugmentationIdentifier we will return - if(isAugmentationIdentifier(potential)) { + if (potential.isPresent() && isAugmentationIdentifier(potential.get())) { return potential; } AugmentationIdentifier lastArgument = (AugmentationIdentifier) Iterables.getLast(normalized.getPath()); - // Here we employ small trick - Binding-aware Codec injects an pointer to augmentation class - // if child is referenced - so we will reference child and then shorten path. + // Here we employ small trick - Binding-aware Codec injects an pointer + // to augmentation class + // if child is referenced - so we will reference child and then shorten + // path. for (QName child : lastArgument.getPossibleChildNames()) { org.opendaylight.yangtools.yang.data.api.InstanceIdentifier childPath = new org.opendaylight.yangtools.yang.data.api.InstanceIdentifier( - ImmutableList. builder() - .addAll(normalized.getPath()).add(new NodeIdentifier(child)).build()); + ImmutableList. builder().addAll(normalized.getPath()).add(new NodeIdentifier(child)) + .build()); try { - - InstanceIdentifier potentialPath = shortenToLastAugment(toBindingImpl(childPath)); - return potentialPath; + if (!isChoiceOrCasePath(childPath)) { + InstanceIdentifier potentialPath = shortenToLastAugment(toBindingImpl( + childPath).get()); + return Optional.> of(potentialPath); + } } catch (Exception e) { - LOG.trace("Unable to deserialize aug. child path for {}",childPath,e); + LOG.trace("Unable to deserialize aug. child path for {}", childPath, e); } } return toBindingImpl(normalized); } - private InstanceIdentifier toBindingImpl( + private Optional> toBindingImpl( final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalized) throws DeserializationException { org.opendaylight.yangtools.yang.data.api.InstanceIdentifier legacyPath; + try { + if (isChoiceOrCasePath(normalized)) { + return Optional.absent(); + } 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); + return Optional.> of(bindingToLegacy.fromDataDom(legacyPath)); + } + + private boolean isChoiceOrCasePath(final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalized) + throws DataNormalizationException { + DataNormalizationOperation op = findNormalizationOperation(normalized); + return op.isMixin() && op.getIdentifier() instanceof NodeIdentifier; + } + + private DataNormalizationOperation findNormalizationOperation( + final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalized) + throws DataNormalizationException { + DataNormalizationOperation current = legacyToNormalized.getRootOperation(); + for (PathArgument arg : normalized.getPath()) { + current = current.getChild(arg); + } + return current; } private static final Entry, DataObject> toEntry( @@ -170,7 +209,7 @@ public class BindingToNormalizedNodeCodec implements SchemaContextListener { public DataObject toBinding(final InstanceIdentifier path, final NormalizedNode normalizedNode) throws DeserializationException { CompositeNode legacy = null; - if(isAugmentationIdentifier(path) && normalizedNode instanceof AugmentationNode) { + if (isAugmentationIdentifier(path) && normalizedNode instanceof AugmentationNode) { QName augIdentifier = BindingReflections.findQName(path.getTargetType()); ContainerNode virtualNode = Builders.containerBuilder() // .withNodeIdentifier(new NodeIdentifier(augIdentifier)) // @@ -188,12 +227,20 @@ public class BindingToNormalizedNodeCodec implements SchemaContextListener { return legacyToNormalized; } - public Entry, DataObject> toBinding( + public Optional, DataObject>> toBinding( final Entry> normalized) throws DeserializationException { - InstanceIdentifier bindingPath = toBinding(normalized.getKey()); - DataObject bindingData = toBinding(bindingPath, normalized.getValue()); - return toEntry(bindingPath, bindingData); + Optional> potentialPath = toBinding(normalized.getKey()); + if (potentialPath.isPresent()) { + InstanceIdentifier bindingPath = potentialPath.get(); + DataObject bindingData = toBinding(bindingPath, normalized.getValue()); + if (bindingData == null) { + LOG.warn("Failed to deserialize {} to Binding format. Binding path is: {}", normalized, bindingPath); + } + return Optional.of(toEntry(bindingPath, bindingData)); + } else { + return Optional.absent(); + } } @Override @@ -206,14 +253,17 @@ public class BindingToNormalizedNodeCodec implements SchemaContextListener { org.opendaylight.yangtools.yang.data.api.InstanceIdentifier processed = toNormalizedImpl(augPath); // If used instance identifier codec added supports for deserialization // of last AugmentationIdentifier we will just reuse it - if(isAugmentationIdentifier(processed)) { + if (isAugmentationIdentifier(processed)) { return processed; } - // Here we employ small trick - DataNormalizer injecst augmentation identifier if child is - // also part of the path (since using a child we can safely identify augmentation) + // Here we employ small trick - DataNormalizer injecst augmentation + // identifier if child is + // also part of the path (since using a child we can safely identify + // augmentation) // so, we scan augmentation for children add it to path // and use original algorithm, then shorten it to last augmentation - for (@SuppressWarnings("rawtypes") Class augChild : getAugmentationChildren(augPath.getTargetType())) { + for (@SuppressWarnings("rawtypes") + Class augChild : getAugmentationChildren(augPath.getTargetType())) { @SuppressWarnings("unchecked") InstanceIdentifier childPath = augPath.child(augChild); org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalized = toNormalizedImpl(childPath); @@ -225,8 +275,6 @@ public class BindingToNormalizedNodeCodec implements SchemaContextListener { return processed; } - - private org.opendaylight.yangtools.yang.data.api.InstanceIdentifier shortenToLastAugmentation( final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalized) { int position = 0; @@ -248,7 +296,7 @@ public class BindingToNormalizedNodeCodec implements SchemaContextListener { final InstanceIdentifier binding) { int position = 0; int foundPosition = -1; - for(org.opendaylight.yangtools.yang.binding.InstanceIdentifier.PathArgument arg : binding.getPathArguments()) { + for (org.opendaylight.yangtools.yang.binding.InstanceIdentifier.PathArgument arg : binding.getPathArguments()) { position++; if (isAugmentation(arg.getType())) { foundPosition = position; @@ -257,8 +305,6 @@ public class BindingToNormalizedNodeCodec implements SchemaContextListener { return InstanceIdentifier.create(Iterables.limit(binding.getPathArguments(), foundPosition)); } - - private org.opendaylight.yangtools.yang.data.api.InstanceIdentifier toNormalizedImpl( final InstanceIdentifier binding) { final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier legacyPath = bindingToLegacy @@ -326,13 +372,12 @@ public class BindingToNormalizedNodeCodec implements SchemaContextListener { return InstanceIdentifier.create(wildArgs); } - private static boolean isAugmentation(final Class type) { return Augmentation.class.isAssignableFrom(type); } - private static boolean isAugmentationIdentifier(final InstanceIdentifier path) { - return Augmentation.class.isAssignableFrom(path.getTargetType()); + private static boolean isAugmentationIdentifier(final InstanceIdentifier potential) { + return Augmentation.class.isAssignableFrom(potential.getTargetType()); } private boolean isAugmentationIdentifier(final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier processed) { -- 2.36.6