From bb928c712fa55fa5a7d5814d501510be90acb02f Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 14 Apr 2014 18:13:00 +0200 Subject: [PATCH] BUG-509: improve normalization error reporting This patch introduces an explicit checked exception for reporting normalization errors, declares it at strategic places and makes use of it for generating better error reports than: Caused by: java.lang.IllegalArgumentException: Supplied QName (urn:opendaylight:params:xml:ns:yang:pcep:crabbe:initiated:00?revision=2014-01-13)initiation is not valid according to schema container stateful Change-Id: I2020c9e6ef85035259558a32f141a132b6321e0e Signed-off-by: Robert Varga --- .../impl/AbstractForwardedTransaction.java | 7 ++- .../compat/DataNormalizationException.java | 20 +++++++ .../compat/DataNormalizationOperation.java | 38 +++++++++---- .../impl/util/compat/DataNormalizer.java | 54 ++++++++++++++----- .../BackwardsCompatibleTransaction.java | 14 ++++- 5 files changed, 106 insertions(+), 27 deletions(-) create mode 100644 opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationException.java diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedTransaction.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedTransaction.java index e39ebfbc71..6334457fd4 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedTransaction.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractForwardedTransaction.java @@ -21,6 +21,7 @@ import org.eclipse.xtext.xbase.lib.Exceptions; import org.opendaylight.controller.md.sal.common.api.TransactionStatus; import org.opendaylight.controller.md.sal.common.api.data.AsyncTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; +import org.opendaylight.controller.md.sal.common.impl.util.compat.DataNormalizationException; import org.opendaylight.controller.md.sal.common.impl.util.compat.DataNormalizationOperation; import org.opendaylight.controller.md.sal.dom.api.DOMDataReadTransaction; import org.opendaylight.controller.md.sal.dom.api.DOMDataReadWriteTransaction; @@ -113,7 +114,11 @@ public class AbstractForwardedTransaction iterator = normalizedPath.getPath().iterator(); while (iterator.hasNext()) { PathArgument currentArg = iterator.next(); - currentOp = currentOp.getChild(currentArg); + try { + currentOp = currentOp.getChild(currentArg); + } catch (DataNormalizationException e) { + throw new IllegalArgumentException(String.format("Invalid child encountered in path %s", path), e); + } currentArguments.add(currentArg); org.opendaylight.yangtools.yang.data.api.InstanceIdentifier currentPath = new org.opendaylight.yangtools.yang.data.api.InstanceIdentifier( currentArguments); diff --git a/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationException.java b/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationException.java new file mode 100644 index 0000000000..f7a15b614e --- /dev/null +++ b/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationException.java @@ -0,0 +1,20 @@ +/* + * 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.common.impl.util.compat; + +public class DataNormalizationException extends Exception { + private static final long serialVersionUID = 1L; + + public DataNormalizationException(String message) { + super(message); + } + + public DataNormalizationException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationOperation.java b/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationOperation.java index 941f2fdb39..1055fa81fd 100644 --- a/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationOperation.java +++ b/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizationOperation.java @@ -1,3 +1,10 @@ +/* + * 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.common.impl.util.compat; import static com.google.common.base.Preconditions.checkArgument; @@ -70,9 +77,9 @@ public abstract class DataNormalizationOperation impleme return Collections.singleton(identifier.getNodeType()); } - public abstract DataNormalizationOperation getChild(final PathArgument child); + public abstract DataNormalizationOperation getChild(final PathArgument child) throws DataNormalizationException; - public abstract DataNormalizationOperation getChild(QName child); + public abstract DataNormalizationOperation getChild(QName child) throws DataNormalizationException; public abstract NormalizedNode normalize(Node legacyData); @@ -162,7 +169,13 @@ public abstract class DataNormalizationOperation impleme Set> usedMixins = new HashSet<>(); for (Node childLegacy : compositeNode.getValue()) { - DataNormalizationOperation childOp = getChild(childLegacy.getNodeType()); + final DataNormalizationOperation childOp; + + try { + childOp = getChild(childLegacy.getNodeType()); + } catch (DataNormalizationException e) { + throw new IllegalArgumentException(String.format("Failed to normalize data %s", compositeNode.getValue()), e); + } // We skip unknown nodes if this node is mixin since // it's nodes and parent nodes are interleaved @@ -175,8 +188,7 @@ public abstract class DataNormalizationOperation impleme if (childOp.isMixin()) { if (usedMixins.contains(childOp)) { // We already run / processed that mixin, so to avoid - // dupliciry we are - // skiping next nodes. + // duplicity we are skipping next nodes. continue; } builder.addChild(childOp.normalize(compositeNode)); @@ -208,7 +220,7 @@ public abstract class DataNormalizationOperation impleme } @Override - public DataNormalizationOperation getChild(final PathArgument child) { + public DataNormalizationOperation getChild(final PathArgument child) throws DataNormalizationException { DataNormalizationOperation potential = byArg.get(child); if (potential != null) { return potential; @@ -218,7 +230,7 @@ public abstract class DataNormalizationOperation impleme } @Override - public DataNormalizationOperation getChild(final QName child) { + public DataNormalizationOperation getChild(final QName child) throws DataNormalizationException { DataNormalizationOperation potential = byQName.get(child); if (potential != null) { return potential; @@ -486,15 +498,19 @@ public abstract class DataNormalizationOperation impleme } } - public static DataNormalizationOperation fromSchemaAndPathArgument(final DataNodeContainer schema, - final QName child) { + private static DataNormalizationOperation fromSchemaAndPathArgument(final DataNodeContainer schema, + final QName child) throws DataNormalizationException { DataSchemaNode potential = schema.getDataChildByName(child); if (potential == null) { Iterable choices = FluentIterable.from( schema.getChildNodes()).filter(org.opendaylight.yangtools.yang.model.api.ChoiceNode.class); potential = findChoice(choices, child); } - checkArgument(potential != null, "Supplied QName %s is not valid according to schema %s", child, schema); + + if (potential == null) { + throw new DataNormalizationException(String.format("Supplied QName %s is not valid according to schema %s", child, schema)); + } + if ((schema instanceof DataSchemaNode) && !((DataSchemaNode) schema).isAugmenting() && potential.isAugmenting()) { return fromAugmentation(schema, (AugmentationTarget) schema, potential); } @@ -541,7 +557,7 @@ public abstract class DataNormalizationOperation impleme } } - private static DataNormalizationOperation fromSchema(final DataNodeContainer schema, final PathArgument child) { + private static DataNormalizationOperation fromSchema(final DataNodeContainer schema, final PathArgument child) throws DataNormalizationException { if (child instanceof AugmentationIdentifier) { return fromSchemaAndPathArgument(schema, ((AugmentationIdentifier) child).getPossibleChildNames() .iterator().next()); diff --git a/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizer.java b/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizer.java index 28b2bde26d..e1fc3c3cdb 100644 --- a/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizer.java +++ b/opendaylight/md-sal/sal-common-impl/src/main/java/org/opendaylight/controller/md/sal/common/impl/util/compat/DataNormalizer.java @@ -1,3 +1,10 @@ +/* + * 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.common.impl.util.compat; import static com.google.common.base.Preconditions.checkArgument; @@ -7,6 +14,7 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.Map; +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; @@ -44,18 +52,24 @@ public class DataNormalizer { DataNormalizationOperation currentOp = operation; Iterator arguments = legacy.getPath().iterator(); - while ( arguments.hasNext() ) { - PathArgument legacyArg = arguments.next(); - currentOp = currentOp.getChild(legacyArg); - checkArgument(currentOp != null, "Legacy Instance Identifier %s is not correct. Normalized Instance Identifier so far %s",legacy,normalizedArgs.build()); - while (currentOp.isMixin()) { - normalizedArgs.add(currentOp.getIdentifier()); - currentOp = currentOp.getChild(legacyArg.getNodeType()); - } - if(arguments.hasNext() || (!currentOp.isKeyedEntry() || legacyArg instanceof NodeIdentifierWithPredicates || legacyArg instanceof NodeWithValue)) { - normalizedArgs.add(legacyArg); + + try { + while ( arguments.hasNext() ) { + PathArgument legacyArg = arguments.next(); + currentOp = currentOp.getChild(legacyArg); + checkArgument(currentOp != null, "Legacy Instance Identifier %s is not correct. Normalized Instance Identifier so far %s",legacy,normalizedArgs.build()); + while (currentOp.isMixin()) { + normalizedArgs.add(currentOp.getIdentifier()); + currentOp = currentOp.getChild(legacyArg.getNodeType()); + } + if(arguments.hasNext() || (!currentOp.isKeyedEntry() || legacyArg instanceof NodeIdentifierWithPredicates || legacyArg instanceof NodeWithValue)) { + normalizedArgs.add(legacyArg); + } } + } catch (DataNormalizationException e) { + throw new IllegalArgumentException(String.format("Failed to normalize path %s", legacy), e); } + return new InstanceIdentifier(normalizedArgs.build()); } @@ -69,12 +83,24 @@ public class DataNormalizer { DataNormalizationOperation currentOp = operation; for (PathArgument arg : normalizedPath.getPath()) { - currentOp = currentOp.getChild(arg); + try { + currentOp = currentOp.getChild(arg); + } catch (DataNormalizationException e) { + throw new IllegalArgumentException(String.format("Failed to validate normalized path %s", normalizedPath), e); + } } - // Write Augmentaiton data resolution + + // Write Augmentation data resolution if (legacyData.getChildren().size() == 1) { - DataNormalizationOperation potentialOp = currentOp.getChild(legacyData.getChildren().get(0) - .getNodeType()); + final DataNormalizationOperation potentialOp; + + try { + final QName childType = legacyData.getChildren().get(0).getNodeType(); + potentialOp = currentOp.getChild(childType); + } catch (DataNormalizationException e) { + throw new IllegalArgumentException(String.format("Failed to get child operation for %s", legacyData), e); + } + if(potentialOp.getIdentifier() instanceof AugmentationIdentifier) { currentOp = potentialOp; ArrayList reworkedArgs = new ArrayList<>(normalizedPath.getPath()); diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/compat/BackwardsCompatibleTransaction.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/compat/BackwardsCompatibleTransaction.java index fce2494554..b905d2f673 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/compat/BackwardsCompatibleTransaction.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/compat/BackwardsCompatibleTransaction.java @@ -1,3 +1,10 @@ +/* + * 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.broker.impl.compat; import static com.google.common.base.Preconditions.checkNotNull; @@ -14,6 +21,7 @@ import java.util.concurrent.Future; import org.opendaylight.controller.md.sal.common.api.TransactionStatus; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; +import org.opendaylight.controller.md.sal.common.impl.util.compat.DataNormalizationException; import org.opendaylight.controller.md.sal.common.impl.util.compat.DataNormalizationOperation; import org.opendaylight.controller.md.sal.common.impl.util.compat.DataNormalizer; import org.opendaylight.controller.md.sal.dom.api.DOMDataReadTransaction; @@ -232,7 +240,11 @@ public abstract class BackwardsCompatibleTransaction iterator = normalizedPath.getPath().iterator(); while(iterator.hasNext()) { PathArgument currentArg = iterator.next(); - currentOp = currentOp.getChild(currentArg); + try { + currentOp = currentOp.getChild(currentArg); + } catch (DataNormalizationException e) { + throw new IllegalArgumentException(String.format("Invalid child encountered in path %s", normalizedPath), e); + } currentArguments.add(currentArg); InstanceIdentifier currentPath = new InstanceIdentifier(currentArguments); boolean isPresent = getDelegate().read(store, currentPath).get().isPresent(); -- 2.36.6