From 6435e96118d7aea3e39ee65fd821be2c1880d262 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 4 Oct 2021 20:20:21 +0200 Subject: [PATCH] Use ApiPath in YangInstanceIdentifierDeserializer We are performing URL parsing in ApiPath, hence we can skip doing the same thing in YangInstanceIdentifierDeserializer. This also improves error reporting, as we provide additional info when we encounter parsing errors. Error messages are also differentiated between parse errors and semantic binding errors. Escaping test has also updated, as ApiPathParser is intelligent enough to understand both : and %3A in identityref name. JIRA: NETCONF-631 Change-Id: I5fdb438d656b79199ea4bf316183d37280261eff Signed-off-by: Robert Varga --- .../YangInstanceIdentifierDeserializer.java | 452 ++++++------------ ...angInstanceIdentifierDeserializerTest.java | 149 +++--- 2 files changed, 228 insertions(+), 373 deletions(-) diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierDeserializer.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierDeserializer.java index 553f86da8c..b3bc922b41 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierDeserializer.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierDeserializer.java @@ -7,44 +7,44 @@ */ package org.opendaylight.restconf.nb.rfc8040.utils.parser; +import static com.google.common.base.Verify.verifyNotNull; import static java.util.Objects.requireNonNull; -import com.google.common.base.CharMatcher; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import java.text.ParseException; import java.util.ArrayList; -import java.util.Iterator; import java.util.List; import java.util.Optional; +import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.restconf.common.errors.RestconfDocumentedException; import org.opendaylight.restconf.common.util.RestUtil; -import org.opendaylight.restconf.common.util.RestconfSchemaUtil; +import org.opendaylight.restconf.nb.rfc8040.ApiPath; +import org.opendaylight.restconf.nb.rfc8040.ApiPath.ListInstance; +import org.opendaylight.restconf.nb.rfc8040.ApiPath.Step; import org.opendaylight.restconf.nb.rfc8040.codecs.RestCodec; import org.opendaylight.yangtools.yang.common.ErrorTag; import org.opendaylight.yangtools.yang.common.ErrorType; import org.opendaylight.yangtools.yang.common.QName; -import org.opendaylight.yangtools.yang.common.YangNames; +import org.opendaylight.yangtools.yang.common.QNameModule; 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.util.DataSchemaContextNode; import org.opendaylight.yangtools.yang.data.util.DataSchemaContextTree; import org.opendaylight.yangtools.yang.model.api.ActionDefinition; import org.opendaylight.yangtools.yang.model.api.ActionNodeContainer; -import org.opendaylight.yangtools.yang.model.api.ContainerLike; -import org.opendaylight.yangtools.yang.model.api.DataNodeContainer; import org.opendaylight.yangtools.yang.model.api.DataSchemaNode; import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext; -import org.opendaylight.yangtools.yang.model.api.IdentitySchemaNode; import org.opendaylight.yangtools.yang.model.api.LeafListSchemaNode; import org.opendaylight.yangtools.yang.model.api.LeafSchemaNode; import org.opendaylight.yangtools.yang.model.api.ListSchemaNode; -import org.opendaylight.yangtools.yang.model.api.Module; import org.opendaylight.yangtools.yang.model.api.RpcDefinition; import org.opendaylight.yangtools.yang.model.api.SchemaContext; -import org.opendaylight.yangtools.yang.model.api.SchemaNode; import org.opendaylight.yangtools.yang.model.api.TypeDefinition; +import org.opendaylight.yangtools.yang.model.api.stmt.IdentityEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.type.IdentityrefTypeDefinition; import org.opendaylight.yangtools.yang.model.api.type.LeafrefTypeDefinition; import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack; @@ -53,119 +53,108 @@ import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack; * Deserializer for {@link String} to {@link YangInstanceIdentifier} for restconf. */ public final class YangInstanceIdentifierDeserializer { - private static final CharMatcher IDENTIFIER_PREDICATE = - CharMatcher.noneOf(ParserConstants.RFC3986_RESERVED_CHARACTERS).precomputed(); - private static final CharMatcher PERCENT_ENCODING = CharMatcher.is('%'); - // position of the first encoded char after percent sign in percent encoded string - private static final int FIRST_ENCODED_CHAR = 1; - // position of the last encoded char after percent sign in percent encoded string - private static final int LAST_ENCODED_CHAR = 3; - // percent encoded radix for parsing integers - private static final int PERCENT_ENCODED_RADIX = 16; + private final @NonNull EffectiveModelContext schemaContext; + private final @NonNull ApiPath apiPath; - private final EffectiveModelContext schemaContext; - private final String data; - - private DataSchemaContextNode current; - private int offset; - - private YangInstanceIdentifierDeserializer(final EffectiveModelContext schemaContext, final String data) { + private YangInstanceIdentifierDeserializer(final EffectiveModelContext schemaContext, final ApiPath apiPath) { this.schemaContext = requireNonNull(schemaContext); - this.data = requireNonNull(data); - current = DataSchemaContextTree.from(schemaContext).getRoot(); + this.apiPath = requireNonNull(apiPath); } /** - * Method to create {@link Iterable} from {@link PathArgument} which are parsing from data by {@link SchemaContext}. + * Method to create {@link List} from {@link PathArgument} which are parsing from data by {@link SchemaContext}. * * @param schemaContext for validate of parsing path arguments * @param data path to data, in URL string form * @return {@link Iterable} of {@link PathArgument} + * @throws RestconfDocumentedException the path is not valid */ public static List create(final EffectiveModelContext schemaContext, final String data) { - return new YangInstanceIdentifierDeserializer(schemaContext, data).parse(); + final ApiPath path; + try { + path = ApiPath.parse(requireNonNull(data)); + } catch (ParseException e) { + throw new RestconfDocumentedException("Invalid path '" + data + "' at offset " + e.getErrorOffset(), + ErrorType.PROTOCOL, ErrorTag.MALFORMED_MESSAGE, e); + } + return create(schemaContext, path); } - private List parse() { - final List path = new ArrayList<>(); + public static List create(final EffectiveModelContext schemaContext, final ApiPath path) { + return new YangInstanceIdentifierDeserializer(schemaContext, path).parse(); + } - while (!allCharsConsumed()) { - validArg(); - final QName qname = prepareQName(); + private ImmutableList parse() { + final var steps = apiPath.steps(); + if (steps.isEmpty()) { + return ImmutableList.of(); + } - // this is the last identifier (input is consumed) or end of identifier (slash) - if (allCharsConsumed() || currentChar() == '/') { - prepareIdentifier(qname, path); - path.add(current == null ? NodeIdentifier.create(qname) : current.getIdentifier()); - } else if (currentChar() == '=') { - if (nextContextNode(qname, path).getDataSchemaNode() instanceof ListSchemaNode) { - prepareNodeWithPredicates(qname, path, (ListSchemaNode) current.getDataSchemaNode()); - } else { - prepareNodeWithValue(qname, path); + final List path = new ArrayList<>(); + DataSchemaContextNode parentNode = DataSchemaContextTree.from(schemaContext).getRoot(); + QNameModule parentNs = null; + for (Step step : steps) { + final var module = step.module(); + final QNameModule ns; + if (module == null) { + if (parentNs == null) { + throw new RestconfDocumentedException( + "First member must use namespace-qualified form, '" + step.identifier() + "' does not", + ErrorType.PROTOCOL, ErrorTag.MALFORMED_MESSAGE); } + ns = parentNs; } else { - throw getParsingCharFailedException(); + ns = resolveNamespace(module); } - } - - return ImmutableList.copyOf(path); - } - - private void prepareNodeWithPredicates(final QName qname, final List path, - final ListSchemaNode listSchemaNode) { - checkValid(listSchemaNode != null, ErrorTag.MALFORMED_MESSAGE, "Data schema node is null"); - final Iterator keys = listSchemaNode.getKeyDefinition().iterator(); - final ImmutableMap.Builder values = ImmutableMap.builder(); - - // skip already expected equal sign - skipCurrentChar(); - - // read key value separated by comma - while (keys.hasNext() && !allCharsConsumed() && currentChar() != '/') { - - // empty key value - if (currentChar() == ',') { - values.put(keys.next(), ""); - skipCurrentChar(); - continue; + final QName qname = step.identifier().bindTo(ns); + final DataSchemaContextNode childNode = nextContextNode(parentNode, qname, path); + final PathArgument pathArg; + if (step instanceof ListInstance) { + final var values = ((ListInstance) step).keyValues(); + final var schema = childNode.getDataSchemaNode(); + pathArg = schema instanceof ListSchemaNode + ? prepareNodeWithPredicates(qname, (ListSchemaNode) schema, values) + : prepareNodeWithValue(qname, schema, values); + } else if (childNode != null) { + RestconfDocumentedException.throwIf(childNode.isKeyedEntry(), + ErrorType.PROTOCOL, ErrorTag.MISSING_ATTRIBUTE, + "Entry '%s' requires key or value predicate to be present.", qname); + pathArg = childNode.getIdentifier(); + } else { + // FIXME: this should be a hard error here, as we cannot resolve the node correctly! + pathArg = NodeIdentifier.create(qname); } - // check if next value is parsable - checkValid(IDENTIFIER_PREDICATE.matches(currentChar()), ErrorTag.MALFORMED_MESSAGE, - "Value that starts with character %c is not parsable.", currentChar()); - - // parse value - final QName key = keys.next(); - Optional leafSchemaNode = listSchemaNode.findDataChildByName(key); - RestconfDocumentedException.throwIf(leafSchemaNode.isEmpty(), ErrorType.PROTOCOL, ErrorTag.BAD_ELEMENT, - "Schema not found for %s", key); + path.add(pathArg); + parentNode = childNode; + parentNs = ns; + } - final String value = findAndParsePercentEncoded(nextIdentifierFromNextSequence(IDENTIFIER_PREDICATE)); - final Object valueByType = prepareValueByType(leafSchemaNode.get(), value); - values.put(key, valueByType); + return ImmutableList.copyOf(path); + } - // skip comma - if (keys.hasNext() && !allCharsConsumed() && currentChar() == ',') { - skipCurrentChar(); - } + private NodeIdentifierWithPredicates prepareNodeWithPredicates(final QName qname, + final @NonNull ListSchemaNode schema, final List<@NonNull String> keyValues) { + final var keyDef = schema.getKeyDefinition(); + final var keySize = keyDef.size(); + final var varSize = keyValues.size(); + if (keySize != varSize) { + throw new RestconfDocumentedException( + "Schema for " + qname + " requires " + keySize + " key values, " + varSize + " supplied", + ErrorType.PROTOCOL, keySize > varSize ? ErrorTag.MISSING_ATTRIBUTE : ErrorTag.UNKNOWN_ATTRIBUTE); } - // the last key is considered to be empty - if (keys.hasNext()) { - // at this point, it must be true that current char is '/' or all chars have already been consumed - values.put(keys.next(), ""); - - // there should be no more missing keys - RestconfDocumentedException.throwIf(keys.hasNext(), ErrorType.PROTOCOL, ErrorTag.MISSING_ATTRIBUTE, - "Cannot parse input identifier '%s'. Key value is missing for QName: %s", data, qname); + final var values = ImmutableMap.builderWithExpectedSize(keySize); + for (int i = 0; i < keySize; ++i) { + final QName keyName = keyDef.get(i); + values.put(keyName, prepareValueByType(schema.getDataChildByName(keyName), keyValues.get(i))); } - path.add(YangInstanceIdentifier.NodeIdentifierWithPredicates.of(qname, values.build())); + return NodeIdentifierWithPredicates.of(qname, values.build()); } - private Object prepareValueByType(final DataSchemaNode schemaNode, final String value) { - Object decoded; + private Object prepareValueByType(final DataSchemaNode schemaNode, final @NonNull String value) { TypeDefinition> typedef; if (schemaNode instanceof LeafListSchemaNode) { @@ -178,259 +167,94 @@ public final class YangInstanceIdentifierDeserializer { typedef = SchemaInferenceStack.ofInstantiatedPath(schemaContext, schemaNode.getPath()) .resolveLeafref((LeafrefTypeDefinition) baseType); } - decoded = RestCodec.from(typedef, null, schemaContext).deserialize(value); - if (decoded == null && typedef instanceof IdentityrefTypeDefinition) { - decoded = toIdentityrefQName(value, schemaNode); - } - return decoded; - } - - private QName prepareQName() { - checkValidIdentifierStart(); - final String preparedPrefix = nextIdentifierFromNextSequence(ParserConstants.YANG_IDENTIFIER_PART); - final String prefix; - final String localName; - if (allCharsConsumed()) { - return getQNameOfDataSchemaNode(preparedPrefix); + if (typedef instanceof IdentityrefTypeDefinition) { + return toIdentityrefQName(value, schemaNode); } - - switch (currentChar()) { - case '/': - case '=': - prefix = preparedPrefix; - return getQNameOfDataSchemaNode(prefix); - case ':': - prefix = preparedPrefix; - skipCurrentChar(); - checkValidIdentifierStart(); - localName = nextIdentifierFromNextSequence(ParserConstants.YANG_IDENTIFIER_PART); - - if (!allCharsConsumed() && currentChar() == '=') { - return getQNameOfDataSchemaNode(localName); - } else { - final Module module = moduleForPrefix(prefix); - RestconfDocumentedException.throwIf(module == null, ErrorType.PROTOCOL, ErrorTag.UNKNOWN_ELEMENT, - "Failed to lookup for module with name '%s'.", prefix); - return QName.create(module.getQNameModule(), localName); - } - default: - throw getParsingCharFailedException(); + try { + return RestCodec.from(typedef, null, schemaContext).deserialize(value); + } catch (IllegalArgumentException e) { + throw new RestconfDocumentedException("Invalid value '" + value + "' for " + schemaNode.getQName(), + ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE, e); } } - private void prepareNodeWithValue(final QName qname, final List path) { - skipCurrentChar(); - final String value = nextIdentifierFromNextSequence(IDENTIFIER_PREDICATE); - - // exception if value attribute is missing - RestconfDocumentedException.throwIf(value.isEmpty(), ErrorType.PROTOCOL, ErrorTag.MISSING_ATTRIBUTE, - "Cannot parse input identifier '%s' - value is missing for QName: %s.", data, qname); - final DataSchemaNode dataSchemaNode = current.getDataSchemaNode(); - final Object valueByType = prepareValueByType(dataSchemaNode, findAndParsePercentEncoded(value)); - path.add(new YangInstanceIdentifier.NodeWithValue<>(qname, valueByType)); - } - - private void prepareIdentifier(final QName qname, final List path) { - final DataSchemaContextNode currentNode = nextContextNode(qname, path); - if (currentNode != null) { - checkValid(!currentNode.isKeyedEntry(), ErrorTag.MISSING_ATTRIBUTE, - "Entry '%s' requires key or value predicate to be present.", qname); - } + private NodeWithValue prepareNodeWithValue(final QName qname, final DataSchemaNode schema, + final List keyValues) { + // TODO: qname should be always equal to schema.getQName(), right? + return new NodeWithValue<>(qname, prepareValueByType(schema, + // FIXME: ahem: we probably want to do something differently here + keyValues.get(0))); } - @SuppressFBWarnings(value = "NP_NULL_ON_SOME_PATH", - justification = "code does check for null 'current' but FB doesn't recognize it") - private DataSchemaContextNode nextContextNode(final QName qname, final List path) { - final DataSchemaContextNode initialContext = current; - final DataSchemaNode initialDataSchema = initialContext.getDataSchemaNode(); - - current = initialContext.getChild(qname); - - if (current == null) { - final Optional module = schemaContext.findModule(qname.getModule()); + private DataSchemaContextNode nextContextNode(final DataSchemaContextNode parent, final QName qname, + final List path) { + final var found = parent.getChild(qname); + if (found == null) { + // FIXME: why are we making this special case here, especially with ... + final var module = schemaContext.findModule(qname.getModule()); if (module.isPresent()) { for (final RpcDefinition rpcDefinition : module.get().getRpcs()) { + // ... this comparison? if (rpcDefinition.getQName().getLocalName().equals(qname.getLocalName())) { return null; } } } - if (findActionDefinition(initialDataSchema, qname.getLocalName()).isPresent()) { + if (findActionDefinition(parent.getDataSchemaNode(), qname.getLocalName()).isPresent()) { return null; } - } - checkValid(current != null, ErrorTag.MALFORMED_MESSAGE, "'%s' is not correct schema node identifier.", qname); - while (current.isMixin()) { - path.add(current.getIdentifier()); - current = current.getChild(qname); - } - return current; - } - private Module moduleForPrefix(final String prefix) { - return schemaContext.findModules(prefix).stream().findFirst().orElse(null); - } - - private boolean allCharsConsumed() { - return offset == data.length(); - } - - private void checkValid(final boolean condition, final ErrorTag errorTag, final String errorMsg) { - if (!condition) { - throw createParsingException(errorTag, errorMsg); + throw new RestconfDocumentedException("Schema for '" + qname + "' not found", + ErrorType.PROTOCOL, ErrorTag.DATA_MISSING); } - } - private void checkValid(final boolean condition, final ErrorTag errorTag, final String fmt, final Object arg) { - if (!condition) { - throw createParsingException(errorTag, String.format(fmt, arg)); + var result = found; + while (result.isMixin()) { + path.add(result.getIdentifier()); + result = verifyNotNull(found.getChild(qname), "Mixin %s is missing child for %s while resolving %s", + result, qname, found); } + return result; } - private void checkValidIdentifierStart() { - checkValid(YangNames.IDENTIFIER_START.matches(currentChar()), ErrorTag.MALFORMED_MESSAGE, - "Identifier must start with character from set 'a-zA-Z_'"); - } - - private RestconfDocumentedException getParsingCharFailedException() { - return createParsingException(ErrorTag.MALFORMED_MESSAGE, - "Bad char '" + currentChar() + "' on the current position."); - } - - private RestconfDocumentedException createParsingException(final ErrorTag errorTag, final String reason) { - return new RestconfDocumentedException( - "Could not parse Instance Identifier '" + data + "'. Offset: '" + offset + "' : Reason: " + reason, - ErrorType.PROTOCOL, errorTag); - } - - private char currentChar() { - return data.charAt(offset); - } - - private void skipCurrentChar() { - offset++; - } - - private String nextIdentifierFromNextSequence(final CharMatcher matcher) { - final int start = offset; - while (!allCharsConsumed() && matcher.matches(currentChar())) { - skipCurrentChar(); - } - return data.substring(start, offset); - } - - private void validArg() { - // every identifier except of the first MUST start with slash - if (offset != 0) { - checkValid('/' == currentChar(), ErrorTag.MALFORMED_MESSAGE, "Identifier must start with '/'."); - - // skip consecutive slashes, users often assume restconf URLs behave just as HTTP does by squashing - // multiple slashes into a single one - while (!allCharsConsumed() && '/' == currentChar()) { - skipCurrentChar(); - } - - // check if slash is not also the last char in identifier - checkValid(!allCharsConsumed(), ErrorTag.MALFORMED_MESSAGE, "Identifier cannot end with '/'."); - } - } - - private QName getQNameOfDataSchemaNode(final String nodeName) { - final DataSchemaNode dataSchemaNode = current.getDataSchemaNode(); - if (dataSchemaNode instanceof ContainerLike) { - return getQNameOfDataSchemaNode((ContainerLike) dataSchemaNode, nodeName); - } else if (dataSchemaNode instanceof ListSchemaNode) { - return getQNameOfDataSchemaNode((ListSchemaNode) dataSchemaNode, nodeName); - } - - throw new UnsupportedOperationException("Unsupported schema node " + dataSchemaNode); - } - - private static QName getQNameOfDataSchemaNode( - final T parent, final String nodeName) { - final Optional actionDef = findActionDefinition(parent, nodeName); - final SchemaNode node; - if (actionDef.isPresent()) { - node = actionDef.get(); - } else { - node = RestconfSchemaUtil.findSchemaNodeInCollection(parent.getChildNodes(), nodeName); - } - return node.getQName(); - } - - private static Optional findActionDefinition(final SchemaNode dataSchemaNode, + private static Optional findActionDefinition(final DataSchemaNode dataSchemaNode, + // FIXME: this should be using a namespace final String nodeName) { - requireNonNull(dataSchemaNode, "DataSchema Node must not be null."); if (dataSchemaNode instanceof ActionNodeContainer) { return ((ActionNodeContainer) dataSchemaNode).getActions().stream() - .filter(actionDef -> actionDef.getQName().getLocalName().equals(nodeName)).findFirst(); + .filter(actionDef -> actionDef.getQName().getLocalName().equals(nodeName)) + .findFirst(); } return Optional.empty(); } - private static String findAndParsePercentEncoded(final String preparedPrefix) { - if (preparedPrefix.indexOf('%') == -1) { - return preparedPrefix; - } - - // FIXME: this is extremely inefficient: we should be converting ranges of characters, not driven by - // CharMatcher, but by String.indexOf() - final StringBuilder parsedPrefix = new StringBuilder(preparedPrefix); - while (PERCENT_ENCODING.matchesAnyOf(parsedPrefix)) { - final int percentCharPosition = PERCENT_ENCODING.indexIn(parsedPrefix); - parsedPrefix.replace(percentCharPosition, percentCharPosition + LAST_ENCODED_CHAR, - String.valueOf((char) Integer.parseInt(parsedPrefix.substring( - percentCharPosition + FIRST_ENCODED_CHAR, percentCharPosition + LAST_ENCODED_CHAR), - PERCENT_ENCODED_RADIX))); - } - - return parsedPrefix.toString(); - } - private QName toIdentityrefQName(final String value, final DataSchemaNode schemaNode) { - final String moduleName = toModuleName(value); - final String nodeName = toNodeName(value); - final Iterator modulesIterator = schemaContext.findModules(moduleName).iterator(); - if (!modulesIterator.hasNext()) { - throw new RestconfDocumentedException(String.format("Cannot decode value '%s' for identityref type " - + "in %s. Make sure reserved characters such as comma, single-quote, double-quote, colon," - + " double-quote, space, and forward slash (,'\":\" /) are percent-encoded," - + " for example ':' is '%%3A'", value, current.getIdentifier().getNodeType()), - ErrorType.PROTOCOL, ErrorTag.BAD_ELEMENT); - } - for (final IdentitySchemaNode identitySchemaNode : modulesIterator.next().getIdentities()) { - final QName qName = identitySchemaNode.getQName(); - if (qName.getLocalName().equals(nodeName)) { - return qName; - } - } - return QName.create(schemaNode.getQName().getNamespace(), schemaNode.getQName().getRevision(), nodeName); - } - - private static String toNodeName(final String str) { - final int idx = str.indexOf(':'); - if (idx == -1) { - return str; - } - - if (str.indexOf(':', idx + 1) != -1) { - return str; + final QNameModule namespace; + final String localName; + final int firstColon = value.indexOf(':'); + if (firstColon != -1) { + namespace = resolveNamespace(value.substring(0, firstColon)); + localName = value.substring(firstColon + 1); + } else { + namespace = schemaNode.getQName().getModule(); + localName = value; } - return str.substring(idx + 1); + return schemaContext.getModuleStatement(namespace) + .streamEffectiveSubstatements(IdentityEffectiveStatement.class) + .map(IdentityEffectiveStatement::argument) + .filter(qname -> localName.equals(qname.getLocalName())) + .findFirst() + .orElseThrow(() -> new RestconfDocumentedException( + "No identity found for '" + localName + "' in namespace " + namespace, + ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE)); } - private static String toModuleName(final String str) { - final int idx = str.indexOf(':'); - if (idx == -1) { - return null; - } - - if (str.indexOf(':', idx + 1) != -1) { - return null; - } - - return str.substring(0, idx); + private @NonNull QNameModule resolveNamespace(final String moduleName) { + final var modules = schemaContext.findModules(moduleName); + RestconfDocumentedException.throwIf(modules.isEmpty(), ErrorType.PROTOCOL, ErrorTag.UNKNOWN_ELEMENT, + "Failed to lookup for module with name '%s'.", moduleName); + return modules.iterator().next().getQNameModule(); } } diff --git a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierDeserializerTest.java b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierDeserializerTest.java index 926efef2c9..1e57c6c084 100644 --- a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierDeserializerTest.java +++ b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierDeserializerTest.java @@ -15,6 +15,7 @@ import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableMap; import java.io.FileNotFoundException; +import java.text.ParseException; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -24,6 +25,7 @@ import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; import org.opendaylight.restconf.common.errors.RestconfDocumentedException; +import org.opendaylight.restconf.nb.rfc8040.ApiPath; import org.opendaylight.restconf.nb.rfc8040.TestRestconfUtils; import org.opendaylight.yangtools.yang.common.ErrorTag; import org.opendaylight.yangtools.yang.common.ErrorType; @@ -199,9 +201,9 @@ public class YangInstanceIdentifierDeserializerTest { * {@code Iterable}. */ @Test - public void deserializeMultipleSlashesTest() { + public void deserializeMultipleSlashesTest() throws ParseException { final var result = YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, - "deserializer-test:contA////list-A=40//list-key"); + ApiPath.parseUrl("deserializer-test:contA////list-A=40//list-key")); assertEquals(4, result.size()); // container @@ -232,7 +234,7 @@ public class YangInstanceIdentifierDeserializerTest { @Test public void nullDataNegativeNegativeTest() { assertThrows(NullPointerException.class, - () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, null)); + () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, (String) null)); } /** @@ -244,12 +246,13 @@ public class YangInstanceIdentifierDeserializerTest { final var ex = assertThrows(RestconfDocumentedException.class, () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, "deserializer-test:cont*leaf-A")); assertEquals("errors: [RestconfError [error-type: protocol, error-tag: malformed-message, " - + "error-message: Could not parse Instance Identifier 'deserializer-test:cont*leaf-A'. Offset: '22' : " - + "Reason: Bad char '*' on the current position.]]", ex.getMessage()); + + "error-message: Invalid path 'deserializer-test:cont*leaf-A' at offset 22, " + + "error-info: Expecting [a-zA-Z_.-], not '*']]", ex.getMessage()); final var errors = ex.getErrors(); assertEquals(1, errors.size()); assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); assertEquals(ErrorTag.MALFORMED_MESSAGE, errors.get(0).getErrorTag()); + assertEquals("Expecting [a-zA-Z_.-], not '*'", errors.get(0).getErrorInfo()); } /** @@ -261,12 +264,13 @@ public class YangInstanceIdentifierDeserializerTest { final var ex = assertThrows(RestconfDocumentedException.class, () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, "deserializer-test:contA/")); assertEquals("errors: [RestconfError [error-type: protocol, error-tag: malformed-message, " - + "error-message: Could not parse Instance Identifier 'deserializer-test:contA/'. Offset: '24' : " - + "Reason: Identifier cannot end with '/'.]]", ex.getMessage()); + + "error-message: Invalid path 'deserializer-test:contA/' at offset 24, " + + "error-info: Identifier may not be empty]]", ex.getMessage()); final var errors = ex.getErrors(); assertEquals(1, errors.size()); assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); assertEquals(ErrorTag.MALFORMED_MESSAGE, errors.get(0).getErrorTag()); + assertEquals("Identifier may not be empty", errors.get(0).getErrorInfo()); } /** @@ -278,12 +282,13 @@ public class YangInstanceIdentifierDeserializerTest { final var ex = assertThrows(RestconfDocumentedException.class, () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, "deserializer-test:contA///")); assertEquals("errors: [RestconfError [error-type: protocol, error-tag: malformed-message, " - + "error-message: Could not parse Instance Identifier 'deserializer-test:contA///'. Offset: '26' : " - + "Reason: Identifier cannot end with '/'.]]", ex.getMessage()); + + "error-message: Invalid path 'deserializer-test:contA///' at offset 24, " + + "error-info: Identifier may not be empty]]", ex.getMessage()); final var errors = ex.getErrors(); assertEquals(1, errors.size()); assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); assertEquals(ErrorTag.MALFORMED_MESSAGE, errors.get(0).getErrorTag()); + assertEquals("Identifier may not be empty", errors.get(0).getErrorInfo()); } /** @@ -295,12 +300,13 @@ public class YangInstanceIdentifierDeserializerTest { final var ex = assertThrows(RestconfDocumentedException.class, () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, "deserializer-test:list-one-key=value/")); assertEquals("errors: [RestconfError [error-type: protocol, error-tag: malformed-message, " - + "error-message: Could not parse Instance Identifier 'deserializer-test:list-one-key=value/'. " - + "Offset: '37' : Reason: Identifier cannot end with '/'.]]", ex.getMessage()); + + "error-message: Invalid path 'deserializer-test:list-one-key=value/' at offset 37, " + + "error-info: Identifier may not be empty]]", ex.getMessage()); final var errors = ex.getErrors(); assertEquals(1, errors.size()); assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); assertEquals(ErrorTag.MALFORMED_MESSAGE, errors.get(0).getErrorTag()); + assertEquals("Identifier may not be empty", errors.get(0).getErrorInfo()); } /** @@ -312,12 +318,13 @@ public class YangInstanceIdentifierDeserializerTest { final var ex = assertThrows(RestconfDocumentedException.class, () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, "deserializer-test:list-one-key=value//")); assertEquals("errors: [RestconfError [error-type: protocol, error-tag: malformed-message, " - + "error-message: Could not parse Instance Identifier 'deserializer-test:list-one-key=value//'. " - + "Offset: '38' : Reason: Identifier cannot end with '/'.]]", ex.getMessage()); + + "error-message: Invalid path 'deserializer-test:list-one-key=value//' at offset 37, " + + "error-info: Identifier may not be empty]]", ex.getMessage()); final var errors = ex.getErrors(); assertEquals(1, errors.size()); assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); assertEquals(ErrorTag.MALFORMED_MESSAGE, errors.get(0).getErrorTag()); + assertEquals("Identifier may not be empty", errors.get(0).getErrorInfo()); } /** @@ -329,12 +336,13 @@ public class YangInstanceIdentifierDeserializerTest { final var ex = assertThrows(RestconfDocumentedException.class, () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, "/")); assertEquals("errors: [RestconfError [error-type: protocol, error-tag: malformed-message, " - + "error-message: Could not parse Instance Identifier '/'. Offset: '0' : " - + "Reason: Identifier must start with character from set 'a-zA-Z_']]", ex.getMessage()); + + "error-message: Invalid path '/' at offset 0, " + + "error-info: Identifier may not be empty]]", ex.getMessage()); final var errors = ex.getErrors(); assertEquals(1, errors.size()); assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); assertEquals(ErrorTag.MALFORMED_MESSAGE, errors.get(0).getErrorTag()); + assertEquals("Identifier may not be empty", errors.get(0).getErrorInfo()); } /** @@ -346,12 +354,13 @@ public class YangInstanceIdentifierDeserializerTest { final var ex = assertThrows(RestconfDocumentedException.class, () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, "deserializer-test*contA")); assertEquals("errors: [RestconfError [error-type: protocol, error-tag: malformed-message, " - + "error-message: Could not parse Instance Identifier 'deserializer-test*contA'. Offset: '17' : " - + "Reason: Bad char '*' on the current position.]]", ex.getMessage()); + + "error-message: Invalid path 'deserializer-test*contA' at offset 17, " + + "error-info: Expecting [a-zA-Z_.-], not '*']]", ex.getMessage()); final var errors = ex.getErrors(); assertEquals(1, errors.size()); assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); assertEquals(ErrorTag.MALFORMED_MESSAGE, errors.get(0).getErrorTag()); + assertEquals("Expecting [a-zA-Z_.-], not '*'", errors.get(0).getErrorInfo()); } /** @@ -379,12 +388,13 @@ public class YangInstanceIdentifierDeserializerTest { final var ex = assertThrows(RestconfDocumentedException.class, () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, "deserializer-test:*not-parsable-identifier")); assertEquals("errors: [RestconfError [error-type: protocol, error-tag: malformed-message, " - + "error-message: Could not parse Instance Identifier 'deserializer-test:*not-parsable-identifier'. " - + "Offset: '18' : Reason: Identifier must start with character from set 'a-zA-Z_']]", ex.getMessage()); + + "error-message: Invalid path 'deserializer-test:*not-parsable-identifier' at offset 18, " + + "error-info: Expecting [a-zA-Z_], not '*']]", ex.getMessage()); final var errors = ex.getErrors(); assertEquals(1, errors.size()); assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); assertEquals(ErrorTag.MALFORMED_MESSAGE, errors.get(0).getErrorTag()); + assertEquals("Expecting [a-zA-Z_], not '*'", errors.get(0).getErrorInfo()); } /** @@ -393,9 +403,16 @@ public class YangInstanceIdentifierDeserializerTest { */ @Test public void prepareQnameErrorParsingNegativeTest() { - // FIXME: this is just wrong - assertThrows(StringIndexOutOfBoundsException.class, + final var ex = assertThrows(RestconfDocumentedException.class, () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, "deserializer-test:")); + assertEquals("errors: [RestconfError [error-type: protocol, error-tag: malformed-message, " + + "error-message: Invalid path 'deserializer-test:' at offset 18, " + + "error-info: Identifier may not be empty]]", ex.getMessage()); + final var errors = ex.getErrors(); + assertEquals(1, errors.size()); + assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); + assertEquals(ErrorTag.MALFORMED_MESSAGE, errors.get(0).getErrorTag()); + assertEquals("Identifier may not be empty", errors.get(0).getErrorInfo()); } /** @@ -408,7 +425,7 @@ public class YangInstanceIdentifierDeserializerTest { final var ex = assertThrows(RestconfDocumentedException.class, () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, "deserializer-test:contA/leafB")); assertEquals("errors: [RestconfError [error-type: protocol, error-tag: data-missing, " - + "error-message: Schema node leafB does not exist in module.]]", ex.getMessage()); + + "error-message: Schema for '(deserializer:test?revision=2016-06-06)leafB' not found]]", ex.getMessage()); final var errors = ex.getErrors(); assertEquals(1, errors.size()); assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); @@ -438,9 +455,8 @@ public class YangInstanceIdentifierDeserializerTest { final var ex = assertThrows(RestconfDocumentedException.class, () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, "deserializer-test:list-one-key")); assertEquals("errors: [RestconfError [error-type: protocol, error-tag: missing-attribute, " - + "error-message: Could not parse Instance Identifier 'deserializer-test:list-one-key'. Offset: '30' : " - + "Reason: Entry '(deserializer:test?revision=2016-06-06)list-one-key' requires key or value predicate " - + "to be present.]]", ex.getMessage()); + + "error-message: Entry '(deserializer:test?revision=2016-06-06)list-one-key' requires key or value " + + "predicate to be present.]]", ex.getMessage()); final var errors = ex.getErrors(); assertEquals(1, errors.size()); assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); @@ -449,20 +465,41 @@ public class YangInstanceIdentifierDeserializerTest { /** * Negative test when there is a comma also after the last key. Test is expected to fail with - * RestconfDocumentedException. + * RestconfDocumentedException. Last comma indicates a fourth key, which is a mismatch with schema. */ @Test - public void deserializeKeysEndsWithComaNegativeTest() { + public void deserializeKeysEndsWithCommaTooManyNegativeTest() { final var ex = assertThrows(RestconfDocumentedException.class, () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, "deserializer-test:list-multiple-keys=value,100,false,")); - assertEquals("errors: [RestconfError [error-type: protocol, error-tag: malformed-message, error-message: " - + "Could not parse Instance Identifier 'deserializer-test:list-multiple-keys=value,100,false,'. " - + "Offset: '52' : Reason: Identifier must start with '/'.]]", ex.getMessage()); + assertEquals("errors: [RestconfError [error-type: protocol, error-tag: unknown-attribute, " + + "error-message: Schema for (deserializer:test?revision=2016-06-06)list-multiple-keys " + + "requires 3 key values, 4 supplied]]", ex.getMessage()); final var errors = ex.getErrors(); assertEquals(1, errors.size()); assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); - assertEquals(ErrorTag.MALFORMED_MESSAGE, errors.get(0).getErrorTag()); + assertEquals(ErrorTag.UNKNOWN_ATTRIBUTE, errors.get(0).getErrorTag()); + } + + /** + * Negative test when there is a comma also after the last key. Test is expected to fail with + * RestconfDocumentedException. Last comma indicates a third key, whose is a mismatch with schema. + */ + @Test + public void deserializeKeysEndsWithCommaIllegalNegativeTest() { + final var ex = assertThrows(RestconfDocumentedException.class, + () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, + "deserializer-test:list-multiple-keys=value,100,")); + assertEquals("errors: [RestconfError [error-type: protocol, error-tag: invalid-value, " + + "error-message: Invalid value '' for (deserializer:test?revision=2016-06-06)enabled, " + + "error-info: Invalid value '' for boolean type. Allowed values are 'true' and 'false']]", + ex.getMessage()); + final var errors = ex.getErrors(); + assertEquals(1, errors.size()); + assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); + assertEquals(ErrorTag.INVALID_VALUE, errors.get(0).getErrorTag()); + assertEquals("Invalid value '' for boolean type. Allowed values are 'true' and 'false'", + errors.get(0).getErrorInfo()); } /** @@ -474,11 +511,11 @@ public class YangInstanceIdentifierDeserializerTest { final QName list = QName.create("deserializer:test", "2016-06-06", "list-multiple-keys"); final Map values = ImmutableMap.of( QName.create(list, "name"), ":foo", - QName.create(list, "number"), "", - QName.create(list, "enabled"), ""); + QName.create(list, "number"), Uint8.ONE, + QName.create(list, "enabled"), false); final var result = YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, - "deserializer-test:list-multiple-keys=%3Afoo,,/string-value"); + "deserializer-test:list-multiple-keys=%3Afoo,1,false/string-value"); assertEquals(3, result.size()); // list assertEquals(NodeIdentifier.create(list), result.get(0)); @@ -528,11 +565,11 @@ public class YangInstanceIdentifierDeserializerTest { final QName list = QName.create("deserializer:test", "2016-06-06", "list-multiple-keys"); final Map values = ImmutableMap.of( QName.create(list, "name"), "", - QName.create(list, "number"), "", - QName.create(list, "enabled"), ""); + QName.create(list, "number"), Uint8.ZERO, + QName.create(list, "enabled"), true); final var result = YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, - "deserializer-test:list-multiple-keys=,,"); + "deserializer-test:list-multiple-keys=,0,true"); assertEquals(2, result.size()); assertEquals(NodeIdentifier.create(list), result.get(0)); assertEquals(NodeIdentifierWithPredicates.of(list, values), result.get(1)); @@ -548,7 +585,7 @@ public class YangInstanceIdentifierDeserializerTest { final var ex = assertThrows(RestconfDocumentedException.class, () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, "deserializer-test:leaf-list-0=")); assertEquals(ErrorType.PROTOCOL, ex.getErrors().get(0).getErrorType()); - assertEquals(ErrorTag.MISSING_ATTRIBUTE, ex.getErrors().get(0).getErrorTag()); + assertEquals(ErrorTag.INVALID_VALUE, ex.getErrors().get(0).getErrorTag()); } /** @@ -557,7 +594,7 @@ public class YangInstanceIdentifierDeserializerTest { @Test public void deserializePartInOtherModuleTest() { final List result = YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, - "deserializer-test-included:augmented-list=100/augmented-leaf"); + "deserializer-test-included:augmented-list=100/deserializer-test:augmented-leaf"); assertEquals(4, result.size()); @@ -579,8 +616,21 @@ public class YangInstanceIdentifierDeserializerTest { */ @Test public void deserializePathWithIdentityrefKeyValueTest() { - final var pathArgs = YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, + assertIdentityrefKeyValue( "deserializer-test-included:refs/list-with-identityref=deserializer-test%3Aderived-identity/foo"); + } + + /** + * Identityref key value is not encoded correctly - ':' character must be encoded as '%3A'. + */ + @Test + public void deserializePathWithInvalidIdentityrefKeyValueTest() { + assertIdentityrefKeyValue( + "deserializer-test-included:refs/list-with-identityref=deserializer-test:derived-identity/foo"); + } + + private static void assertIdentityrefKeyValue(final String path) { + final var pathArgs = YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, path); assertEquals(4, pathArgs.size()); assertEquals("refs", pathArgs.get(0).getNodeType().getLocalName()); @@ -597,23 +647,4 @@ public class YangInstanceIdentifierDeserializerTest { assertEquals("foo", pathArgs.get(3).getNodeType().getLocalName()); } - - /** - * Identityref key value is not encoded correctly - ':' character must be encoded as '%3A'. - */ - @Test - public void deserializePathWithInvalidIdentityrefKeyValueTest() { - final var ex = assertThrows(RestconfDocumentedException.class, - () -> YangInstanceIdentifierDeserializer.create(SCHEMA_CONTEXT, - "deserializer-test-included:refs/list-with-identityref=deserializer-test:derived-identity/foo")); - assertEquals("errors: [RestconfError [error-type: protocol, error-tag: bad-element, " - + "error-message: Cannot decode value 'deserializer-test' for identityref type in " - + "(deserializer:test:included?revision=2016-06-06)list-with-identityref. Make sure reserved characters " - + "such as comma, single-quote, double-quote, colon, double-quote, space, and forward slash (,'\":\" /) " - + "are percent-encoded, for example ':' is '%3A']]", ex.getMessage()); - final var errors = ex.getErrors(); - assertEquals(1, errors.size()); - assertEquals(ErrorType.PROTOCOL, errors.get(0).getErrorType()); - assertEquals(ErrorTag.BAD_ELEMENT, errors.get(0).getErrorTag()); - } } \ No newline at end of file -- 2.36.6