From b14e3fb6a934e373c2f88d9d58c6585acddb81f5 Mon Sep 17 00:00:00 2001 From: lubos-cicut Date: Tue, 26 Mar 2024 15:27:41 +0100 Subject: [PATCH] Rework YangInstanceIdentifier/URI path conversion YangInstanceIdentifierSerializer has a rather ambiguous name and description, but essentially it does what it advertizes: it takes a YangInstanceIdentifier and creates a String corresponding to ApiPath.parse(). Unfortunately this process handles values through String.valueOf(), which obviously does the wrong thing for non-trivial values, such as identityref and instance-identifier. We already have a component handling ApiPath -> YangInstanceIdentifier conversion, the ApiPathNormalizer. This patch completely replaces YangInstanceIdentifierSerializer.serializePath() with ApiPathNormalizer.canonicalize(). It essentially does the same thing, but returns an ApiPath as the result. Users can use ApiPath.toString() to then get the String representation, if needed. JIRA: NETCONF-1264 Change-Id: I1093eb2cbd84ae269430f918e48b98cc2fb4e805 Signed-off-by: lubos-cicut Signed-off-by: Robert Varga --- .../rests/transactions/RestconfStrategy.java | 5 +- .../YangInstanceIdentifierSerializer.java | 151 ------------------ .../restconf/server/api/PatchBody.java | 7 +- .../SubscribeDeviceNotificationRpc.java | 4 +- .../CreateDataChangeEventSubscriptionRpc.java | 4 +- .../server/spi/ApiPathNormalizer.java | 149 +++++++++++++++++ .../restconf/server/spi/HackJsonWriter.java | 95 +++++++++++ .../YangInstanceIdentifierSerializerTest.java | 139 ++++++++-------- .../serializer/serializer-test-included.yang | 14 ++ .../parser/serializer/serializer-test.yang | 10 ++ 10 files changed, 353 insertions(+), 225 deletions(-) delete mode 100644 restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierSerializer.java create mode 100644 restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/spi/HackJsonWriter.java diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java index cb175b55d4..c00b903a25 100644 --- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java +++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java @@ -68,7 +68,6 @@ import org.opendaylight.restconf.nb.rfc8040.Insert; import org.opendaylight.restconf.nb.rfc8040.legacy.ErrorTags; import org.opendaylight.restconf.nb.rfc8040.legacy.NormalizedNodePayload; import org.opendaylight.restconf.nb.rfc8040.legacy.QueryParameters; -import org.opendaylight.restconf.nb.rfc8040.utils.parser.YangInstanceIdentifierSerializer; import org.opendaylight.restconf.server.api.ChildBody; import org.opendaylight.restconf.server.api.ConfigurationMetadata; import org.opendaylight.restconf.server.api.DataGetParams; @@ -514,9 +513,9 @@ public abstract class RestconfStrategy { Futures.addCallback(future, new FutureCallback() { @Override public void onSuccess(final CommitInfo result) { - ret.set(new CreateResource(new YangInstanceIdentifierSerializer(databind).serializePath( + ret.set(new CreateResource(new ApiPathNormalizer(databind).canonicalize( data instanceof MapNode mapData && !mapData.isEmpty() - ? path.node(mapData.body().iterator().next().name()) : path))); + ? path.node(mapData.body().iterator().next().name()) : path).toString())); } @Override diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierSerializer.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierSerializer.java deleted file mode 100644 index bb9d4b4a9d..0000000000 --- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierSerializer.java +++ /dev/null @@ -1,151 +0,0 @@ -/* - * Copyright (c) 2016 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.restconf.nb.rfc8040.utils.parser; - -import static java.util.Objects.requireNonNull; - -import java.util.Map.Entry; -import java.util.Set; -import org.eclipse.jdt.annotation.NonNull; -import org.opendaylight.restconf.api.ApiPath; -import org.opendaylight.restconf.common.errors.RestconfDocumentedException; -import org.opendaylight.restconf.server.api.DatabindContext; -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.QNameModule; -import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; -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.DataSchemaContext; -import org.opendaylight.yangtools.yang.data.util.DataSchemaContext.PathMixin; -import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext; - -/** - * Serializer for {@link YangInstanceIdentifier} to {@link String} for restconf. - */ -public final class YangInstanceIdentifierSerializer { - private final @NonNull DatabindContext databind; - - public YangInstanceIdentifierSerializer(final DatabindContext databind) { - this.databind = requireNonNull(databind); - } - - /** - * Method to create String from {@link Iterable} of {@link PathArgument} which are parsing from data with the help - * of an {@link EffectiveModelContext}. - * - * @param path path to data - * @return {@link String} - */ - public String serializePath(final YangInstanceIdentifier path) { - if (path.isEmpty()) { - return ""; - } - - final var current = databind.schemaTree().getRoot(); - final var variables = new MainVarsWrapper(current); - final var sb = new StringBuilder(); - - QNameModule parentModule = null; - for (var arg : path.getPathArguments()) { - // get module of the parent - final var currentContext = variables.getCurrent(); - - if (!(currentContext instanceof PathMixin)) { - parentModule = currentContext.dataSchemaNode().getQName().getModule(); - } - - final var childContext = currentContext instanceof DataSchemaContext.Composite composite - ? composite.childByArg(arg) : null; - if (childContext == null) { - throw new RestconfDocumentedException( - "Invalid input '%s': schema for argument '%s' (after '%s') not found".formatted(path, arg, sb), - ErrorType.APPLICATION, ErrorTag.UNKNOWN_ELEMENT); - } - - variables.setCurrent(childContext); - if (childContext instanceof PathMixin) { - continue; - } - - // append namespace before every node which is defined in other module than its parent - // condition is satisfied also for the first path argument - if (!arg.getNodeType().getModule().equals(parentModule)) { - // append slash if it is not the first path argument - if (sb.length() > 0) { - sb.append('/'); - } - - sb.append(prefixForNamespace(arg.getNodeType())).append(':'); - } else { - sb.append('/'); - } - - sb.append(arg.getNodeType().getLocalName()); - if (arg instanceof NodeIdentifierWithPredicates withPredicates) { - prepareNodeWithPredicates(sb, withPredicates.entrySet()); - } else if (arg instanceof NodeWithValue withValue) { - prepareNodeWithValue(sb, withValue.getValue()); - } - } - - return sb.toString(); - } - - private static void prepareNodeWithValue(final StringBuilder path, final Object value) { - path.append('='); - - // FIXME: this is quite fishy - final var str = String.valueOf(value); - path.append(ApiPath.PERCENT_ESCAPER.escape(str)); - } - - private static void prepareNodeWithPredicates(final StringBuilder path, final Set> entries) { - final var iterator = entries.iterator(); - if (iterator.hasNext()) { - path.append('='); - } - - while (iterator.hasNext()) { - // FIXME: this is quite fishy - final var str = String.valueOf(iterator.next().getValue()); - path.append(ApiPath.PERCENT_ESCAPER.escape(str)); - if (iterator.hasNext()) { - path.append(','); - } - } - } - - /** - * Create prefix of namespace from {@link QName}. - * - * @param qname {@link QName} - * @return {@link String} - */ - private String prefixForNamespace(final QName qname) { - return databind.modelContext().findModuleStatement(qname.getModule()).orElseThrow().argument().getLocalName(); - } - - private static final class MainVarsWrapper { - private DataSchemaContext current; - - MainVarsWrapper(final DataSchemaContext current) { - setCurrent(current); - } - - public DataSchemaContext getCurrent() { - return current; - } - - public void setCurrent(final DataSchemaContext current) { - this.current = current; - } - } -} diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/api/PatchBody.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/api/PatchBody.java index 3660239b0b..1c156eeaf7 100644 --- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/api/PatchBody.java +++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/api/PatchBody.java @@ -16,7 +16,6 @@ import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.restconf.api.ApiPath; import org.opendaylight.restconf.common.errors.RestconfDocumentedException; import org.opendaylight.restconf.common.patch.PatchContext; -import org.opendaylight.restconf.nb.rfc8040.utils.parser.YangInstanceIdentifierSerializer; import org.opendaylight.restconf.server.spi.ApiPathNormalizer; import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.patch.rev170222.yang.patch.yang.patch.Edit.Operation; import org.opendaylight.yangtools.yang.common.ErrorTag; @@ -50,16 +49,16 @@ public abstract sealed class PatchBody extends AbstractBody permits JsonPatchBod // FIXME: NETCONF-1157: these two operations should really be a single ApiPathNormalizer step -- but for that // we need to switch to ApiPath forms - final var databind = path.databind(); + final var pathNormalizer = new ApiPathNormalizer(path.databind()); final String targetUrl; if (urlPath.isEmpty()) { targetUrl = target.startsWith("/") ? target.substring(1) : target; } else { - targetUrl = new YangInstanceIdentifierSerializer(databind).serializePath(urlPath) + target; + targetUrl = pathNormalizer.canonicalize(urlPath).toString() + target; } try { - return new ApiPathNormalizer(databind).normalizeDataPath(ApiPath.parse(targetUrl)).instance(); + return pathNormalizer.normalizeDataPath(ApiPath.parse(targetUrl)).instance(); } catch (ParseException | RestconfDocumentedException e) { throw new RestconfDocumentedException("Failed to parse target " + target, ErrorType.RPC, ErrorTag.MALFORMED_MESSAGE, e); diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/mdsal/streams/devnotif/SubscribeDeviceNotificationRpc.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/mdsal/streams/devnotif/SubscribeDeviceNotificationRpc.java index 9978133ed9..e0518e2bc3 100644 --- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/mdsal/streams/devnotif/SubscribeDeviceNotificationRpc.java +++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/mdsal/streams/devnotif/SubscribeDeviceNotificationRpc.java @@ -15,8 +15,8 @@ import javax.inject.Singleton; import org.opendaylight.mdsal.dom.api.DOMMountPointService; import org.opendaylight.restconf.common.errors.RestconfDocumentedException; import org.opendaylight.restconf.common.errors.RestconfFuture; -import org.opendaylight.restconf.nb.rfc8040.utils.parser.YangInstanceIdentifierSerializer; import org.opendaylight.restconf.server.api.OperationsPostResult; +import org.opendaylight.restconf.server.spi.ApiPathNormalizer; import org.opendaylight.restconf.server.spi.OperationInput; import org.opendaylight.restconf.server.spi.RestconfStream; import org.opendaylight.restconf.server.spi.RpcImplementation; @@ -81,7 +81,7 @@ public final class SubscribeDeviceNotificationRpc extends RpcImplementation { return streamRegistry.createStream(restconfURI, new DeviceNotificationSource(mountPointService, path), "All YANG notifications occuring on mount point /" - + new YangInstanceIdentifierSerializer(input.databind()).serializePath(path)) + + new ApiPathNormalizer(input.databind()).canonicalize(path).toString()) .transform(stream -> input.newOperationOutput(ImmutableNodes.newContainerBuilder() .withNodeIdentifier(new NodeIdentifier(SubscribeDeviceNotificationOutput.QNAME)) .withChild(ImmutableNodes.leafNode(DEVICE_NOTIFICATION_STREAM_NAME_NODEID, stream.name())) diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/mdsal/streams/dtcl/CreateDataChangeEventSubscriptionRpc.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/mdsal/streams/dtcl/CreateDataChangeEventSubscriptionRpc.java index 9c98f4143c..e7aa20bb75 100644 --- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/mdsal/streams/dtcl/CreateDataChangeEventSubscriptionRpc.java +++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/mdsal/streams/dtcl/CreateDataChangeEventSubscriptionRpc.java @@ -18,8 +18,8 @@ import org.opendaylight.mdsal.dom.api.DOMDataBroker; import org.opendaylight.mdsal.dom.api.DOMDataBroker.DataTreeChangeExtension; import org.opendaylight.restconf.common.errors.RestconfDocumentedException; import org.opendaylight.restconf.common.errors.RestconfFuture; -import org.opendaylight.restconf.nb.rfc8040.utils.parser.YangInstanceIdentifierSerializer; import org.opendaylight.restconf.server.api.OperationsPostResult; +import org.opendaylight.restconf.server.spi.ApiPathNormalizer; import org.opendaylight.restconf.server.spi.DatabindProvider; import org.opendaylight.restconf.server.spi.OperationInput; import org.opendaylight.restconf.server.spi.RestconfStream; @@ -111,7 +111,7 @@ public final class CreateDataChangeEventSubscriptionRpc extends RpcImplementatio return streamRegistry.createStream(restconfURI, new DataTreeChangeSource(databindProvider, changeService, datastore, path), "Events occuring in " + datastore + " datastore under /" - + new YangInstanceIdentifierSerializer(input.databind()).serializePath(path)) + + new ApiPathNormalizer(input.databind()).canonicalize(path).toString()) .transform(stream -> input.newOperationOutput(ImmutableNodes.newContainerBuilder() .withNodeIdentifier(OUTPUT_NODEID) .withChild(ImmutableNodes.leafNode(STREAM_NAME_NODEID, stream.name())) diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/spi/ApiPathNormalizer.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/spi/ApiPathNormalizer.java index d2841c4343..421a549c67 100644 --- a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/spi/ApiPathNormalizer.java +++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/spi/ApiPathNormalizer.java @@ -12,13 +12,17 @@ import static com.google.common.base.Verify.verifyNotNull; import static java.util.Objects.requireNonNull; import com.google.common.base.VerifyException; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import java.io.IOException; import java.util.ArrayList; import java.util.List; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.NonNullByDefault; import org.opendaylight.restconf.api.ApiPath; +import org.opendaylight.restconf.api.ApiPath.ApiIdentifier; import org.opendaylight.restconf.api.ApiPath.ListInstance; +import org.opendaylight.restconf.api.ApiPath.Step; import org.opendaylight.restconf.common.errors.RestconfDocumentedException; import org.opendaylight.restconf.nb.rfc8040.Insert.PointNormalizer; import org.opendaylight.restconf.server.api.DatabindContext; @@ -30,15 +34,19 @@ import org.opendaylight.yangtools.yang.common.ErrorType; import org.opendaylight.yangtools.yang.common.QName; 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.codec.gson.JSONCodec; import org.opendaylight.yangtools.yang.data.util.DataSchemaContext; +import org.opendaylight.yangtools.yang.data.util.DataSchemaContext.Composite; import org.opendaylight.yangtools.yang.data.util.DataSchemaContext.PathMixin; import org.opendaylight.yangtools.yang.model.api.ActionNodeContainer; import org.opendaylight.yangtools.yang.model.api.DataSchemaNode; import org.opendaylight.yangtools.yang.model.api.EffectiveStatementInference; 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.TypedDataSchemaNode; import org.opendaylight.yangtools.yang.model.api.stmt.ActionEffectiveStatement; @@ -393,6 +401,137 @@ public final class ApiPathNormalizer implements PointNormalizer { throw new RestconfDocumentedException("Unexpected path " + path, ErrorType.PROTOCOL, ErrorTag.DATA_MISSING); } + /** + * Return the canonical {@link ApiPath} for specified {@link YangInstanceIdentifier}. + * + * @param path {@link YangInstanceIdentifier} to canonicalize + * @return An {@link ApiPath} + */ + public @NonNull ApiPath canonicalize(final YangInstanceIdentifier path) { + final var it = path.getPathArguments().iterator(); + if (!it.hasNext()) { + return ApiPath.empty(); + } + + final var stack = SchemaInferenceStack.of(databind.modelContext()); + final var builder = ImmutableList.builder(); + DataSchemaContext context = databind.schemaTree().getRoot(); + QNameModule parentModule = null; + do { + final var arg = it.next(); + + // get module of the parent + if (!(context instanceof PathMixin)) { + parentModule = context.dataSchemaNode().getQName().getModule(); + } + + final var childContext = context instanceof Composite composite ? composite.enterChild(stack, arg) : null; + if (childContext == null) { + throw new RestconfDocumentedException( + "Invalid input '%s': schema for argument '%s' (after '%s') not found".formatted(path, arg, + ApiPath.of(builder.build())), ErrorType.APPLICATION, ErrorTag.UNKNOWN_ELEMENT); + } + + context = childContext; + if (childContext instanceof PathMixin) { + // This PathArgument is a mixed-in YangInstanceIdentifier, do not emit anything and continue + continue; + } + + builder.add(canonicalize(arg, parentModule, stack, context)); + } while (it.hasNext()); + + return new ApiPath(builder.build()); + } + + private @NonNull Step canonicalize(final PathArgument arg, final QNameModule prevNamespace, + final SchemaInferenceStack stack, final DataSchemaContext context) { + // append namespace before every node which is defined in other module than its parent + // condition is satisfied also for the first path argument + final var nodeType = arg.getNodeType(); + final var module = nodeType.getModule().equals(prevNamespace) ? null : resolvePrefix(nodeType); + final var identifier = nodeType.unbind(); + + // NodeIdentifier maps to an ApiIdentifier + if (arg instanceof NodeIdentifier) { + return new ApiIdentifier(module, identifier); + } + + // NodeWithValue addresses a LeafSetEntryNode and maps to a ListInstance with a single value + final var schema = context.dataSchemaNode(); + if (arg instanceof NodeWithValue withValue) { + if (!(schema instanceof LeafListSchemaNode leafList)) { + throw new RestconfDocumentedException( + "Argument '%s' does not map to a leaf-list, but %s".formatted(arg, schema), + ErrorType.APPLICATION, ErrorTag.INVALID_VALUE); + } + return ListInstance.of(module, identifier, encodeValue(stack, leafList, withValue.getValue())); + } + + // The only remaining case is NodeIdentifierWrithPredicates, verify that instead of an explicit cast + if (!(arg instanceof NodeIdentifierWithPredicates withPredicates)) { + throw new VerifyException("Unhandled " + arg); + } + // A NodeIdentifierWithPredicates adresses a MapEntryNode and maps to a ListInstance with one or more values: + // 1) schema has to be a ListSchemaNode + if (!(schema instanceof ListSchemaNode list)) { + throw new RestconfDocumentedException( + "Argument '%s' does not map to a list, but %s".formatted(arg, schema), + ErrorType.APPLICATION, ErrorTag.INVALID_VALUE); + } + // 2) the key definition must be non-empty + final var keyDef = list.getKeyDefinition(); + final var size = keyDef.size(); + if (size == 0) { + throw new RestconfDocumentedException( + "Argument '%s' maps a list without any keys %s".formatted(arg, schema), + ErrorType.APPLICATION, ErrorTag.INVALID_VALUE); + } + // 3) the number of predicates has to match the number of keys + if (size != withPredicates.size()) { + throw new RestconfDocumentedException( + "Argument '%s' does not match required keys %s".formatted(arg, keyDef), + ErrorType.APPLICATION, ErrorTag.INVALID_VALUE); + } + + // ListSchemaNode implies the context is a composite, verify that instead of an unexplained cast when we look + // up the schema for individual keys + if (!(context instanceof Composite composite)) { + throw new VerifyException("Unexpected non-composite " + context + " with " + list); + } + + final var builder = ImmutableList.builderWithExpectedSize(size); + for (var key : keyDef) { + final var value = withPredicates.getValue(key); + if (value == null) { + throw new RestconfDocumentedException("Argument '%s' is missing predicate for %s".formatted(arg, key), + ErrorType.APPLICATION, ErrorTag.INVALID_VALUE); + } + + final var tmpStack = stack.copy(); + final var keyContext = composite.enterChild(tmpStack, key); + if (keyContext == null) { + throw new VerifyException("Failed to find key " + key + " in " + composite); + } + if (!(keyContext.dataSchemaNode() instanceof LeafSchemaNode leaf)) { + throw new VerifyException("Key " + key + " maps to non-leaf context " + keyContext); + } + builder.add(encodeValue(tmpStack, leaf, value)); + } + return ListInstance.of(module, identifier, builder.build()); + } + + private String encodeValue(final SchemaInferenceStack stack, final TypedDataSchemaNode schema, final Object value) { + @SuppressWarnings("unchecked") + final var codec = (JSONCodec) databind.jsonCodecs().codecFor(schema, stack); + try (var jsonWriter = new HackJsonWriter()) { + codec.writeValue(jsonWriter, value); + return jsonWriter.acquireCaptured().rawString(); + } catch (IOException e) { + throw new IllegalStateException("Failed to serialize '" + value + "'", e); + } + } + private NodeIdentifierWithPredicates prepareNodeWithPredicates(final SchemaInferenceStack stack, final QName qname, final @NonNull ListSchemaNode schema, final List<@NonNull String> keyValues) { final var keyDef = schema.getKeyDefinition(); @@ -450,4 +589,14 @@ public final class ApiPathNormalizer implements PointNormalizer { throw new RestconfDocumentedException("Failed to lookup for module with name '" + moduleName + "'.", ErrorType.PROTOCOL, ErrorTag.UNKNOWN_ELEMENT); } + + /** + * Create prefix of namespace from {@link QName}. + * + * @param qname {@link QName} + * @return {@link String} + */ + private @NonNull String resolvePrefix(final QName qname) { + return databind.modelContext().findModuleStatement(qname.getModule()).orElseThrow().argument().getLocalName(); + } } diff --git a/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/spi/HackJsonWriter.java b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/spi/HackJsonWriter.java new file mode 100644 index 0000000000..2b11c57c88 --- /dev/null +++ b/restconf/restconf-nb/src/main/java/org/opendaylight/restconf/server/spi/HackJsonWriter.java @@ -0,0 +1,95 @@ +/* + * Copyright (c) 2024 PANTHEON.tech, s.r.o. 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.restconf.server.spi; + +import static java.util.Objects.requireNonNull; + +import com.google.gson.stream.JsonWriter; +import java.io.IOException; +import java.io.StringWriter; +import org.eclipse.jdt.annotation.NonNull; +import org.opendaylight.yangtools.yang.data.codec.gson.JSONCodec; + +/** + * A hack to intercept {@link JSONCodec#writeValue(JsonWriter, Object)} output. This class is closely tailored to + * respond implementation behaviour of JSONCodecs. + */ +// FIXME: remove this class once we have YANGTOOLS-1569 +final class HackJsonWriter extends JsonWriter { + record Value(String rawString, Kind kind) { + enum Kind { + BOOLEAN, + NULL, + NUMBER, + STRING + } + + Value { + requireNonNull(rawString); + requireNonNull(kind); + } + } + + private static final Value FALSE = new Value("false", Value.Kind.BOOLEAN); + private static final Value TRUE = new Value("true", Value.Kind.BOOLEAN); + private static final Value NULL = new Value("[null]", Value.Kind.NULL); + + private Value captured = null; + + HackJsonWriter() { + super(new StringWriter()); + } + + @Override + public JsonWriter nullValue() throws IOException { + capture(NULL); + return super.nullValue(); + } + + @Override + public JsonWriter value(final boolean value) throws IOException { + capture(value ? TRUE : FALSE); + return super.value(value); + } + + @Override + public JsonWriter value(final Boolean value) throws IOException { + // We assume non-null values + return value(value.booleanValue()); + } + + @Override + public JsonWriter value(final Number value) throws IOException { + capture(new Value(value.toString(), Value.Kind.NUMBER)); + return super.value(value); + } + + @Override + public JsonWriter value(final String value) throws IOException { + if (value == null) { + return nullValue(); + } + capture(new Value(value, Value.Kind.STRING)); + return super.value(value); + } + + @NonNull Value acquireCaptured() throws IOException { + final var local = captured; + if (local == null) { + throw new IOException("No value set"); + } + return local; + } + + private void capture(final Value newValue) throws IOException { + if (captured != null) { + throw new IOException("Value already set to " + captured); + } + captured = newValue; + } +} diff --git a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierSerializerTest.java b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierSerializerTest.java index 810bff040a..1685d7a43b 100644 --- a/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierSerializerTest.java +++ b/restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierSerializerTest.java @@ -14,7 +14,6 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.ImmutableMap; import java.text.ParseException; import java.util.Map; -import org.eclipse.jdt.annotation.NonNull; import org.junit.jupiter.api.Test; import org.opendaylight.restconf.api.ApiPath; import org.opendaylight.restconf.common.errors.RestconfDocumentedException; @@ -36,9 +35,8 @@ import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils; * Unit tests for {@link YangInstanceIdentifierSerializer}. */ class YangInstanceIdentifierSerializerTest { - private static final @NonNull DatabindContext DATABIND = DatabindContext.ofModel( - YangParserTestUtils.parseYangResourceDirectory("/restconf/parser/serializer")); - private static final YangInstanceIdentifierSerializer SERIALIZER = new YangInstanceIdentifierSerializer(DATABIND); + private static final ApiPathNormalizer NORMALIZER = new ApiPathNormalizer(DatabindContext.ofModel( + YangParserTestUtils.parseYangResourceDirectory("/restconf/parser/serializer"))); /** * Positive test of serialization of YangInstanceIdentifier containing container node to @@ -46,8 +44,8 @@ class YangInstanceIdentifierSerializerTest { */ @Test void serializeContainerTest() { - assertEquals("serializer-test:contA", SERIALIZER.serializePath( - YangInstanceIdentifier.of(QName.create("serializer:test", "2016-06-06", "contA")))); + assertApiPath("serializer-test:contA", + YangInstanceIdentifier.of(QName.create("serializer:test", "2016-06-06", "contA"))); } /** @@ -56,10 +54,10 @@ class YangInstanceIdentifierSerializerTest { */ @Test void serializeContainerWithLeafTest() { - assertEquals("serializer-test:contA/leaf-A", SERIALIZER.serializePath( + assertApiPath("serializer-test:contA/leaf-A", YangInstanceIdentifier.of( QName.create("serializer:test", "2016-06-06", "contA"), - QName.create("serializer:test", "2016-06-06", "leaf-A")))); + QName.create("serializer:test", "2016-06-06", "leaf-A"))); } /** @@ -71,14 +69,14 @@ class YangInstanceIdentifierSerializerTest { final var list = QName.create("serializer:test", "2016-06-06", "list-A"); final var leafList = QName.create("serializer:test", "2016-06-06", "leaf-list-AA"); - assertEquals("serializer-test:contA/list-A=100/leaf-list-AA=instance", SERIALIZER.serializePath( + assertApiPath("serializer-test:contA/list-A=100/leaf-list-AA=instance", YangInstanceIdentifier.builder() .node(QName.create("serializer:test", "2016-06-06", "contA")) .node(list) .node(NodeIdentifierWithPredicates.of(list, QName.create(list, "list-key"), 100)) .node(leafList) .node(new NodeWithValue<>(leafList, "instance")) - .build())); + .build()); } /** @@ -88,24 +86,27 @@ class YangInstanceIdentifierSerializerTest { */ @Test void serializeListWithNoKeysTest() { - assertEquals("serializer-test:list-no-key", SERIALIZER.serializePath( + assertApiPath("serializer-test:list-no-key", YangInstanceIdentifier.of( QName.create("serializer:test", "2016-06-06", "list-no-key"), - QName.create("serializer:test", "2016-06-06", "list-no-key")))); + QName.create("serializer:test", "2016-06-06", "list-no-key"))); } /** - * Positive test of serialization of YangInstanceIdentifier to String when serialized - * YangInstanceIdentifier contains a keyed list, but the path argument does not specify them. Returned - * String is compared to have expected value. + * Negative test of serialization of YangInstanceIdentifier to String when serialized + * YangInstanceIdentifier contains a keyed list, but the path argument does not specify them. */ @Test void serializeMapWithNoKeysTest() { - assertEquals("serializer-test:list-one-key", SERIALIZER.serializePath( - YangInstanceIdentifier.builder() - .node(QName.create("serializer:test", "2016-06-06", "list-one-key")) - .nodeWithKey(QName.create("serializer:test", "2016-06-06", "list-one-key"), Map.of()) - .build())); + final var error = assertError(YangInstanceIdentifier.builder() + .node(QName.create("serializer:test", "2016-06-06", "list-one-key")) + .nodeWithKey(QName.create("serializer:test", "2016-06-06", "list-one-key"), Map.of()) + .build()); + assertEquals(""" + Argument '(serializer:test?revision=2016-06-06)list-one-key[{}]' does not match required keys \ + [(serializer:test?revision=2016-06-06)name]""", error.getErrorMessage()); + assertEquals(ErrorType.APPLICATION, error.getErrorType()); + assertEquals(ErrorTag.INVALID_VALUE, error.getErrorTag()); } /** @@ -115,12 +116,12 @@ class YangInstanceIdentifierSerializerTest { */ @Test void serializeMapWithOneKeyTest() { - assertEquals("serializer-test:list-one-key=value", SERIALIZER.serializePath( + assertApiPath("serializer-test:list-one-key=value", YangInstanceIdentifier.builder() .node(QName.create("serializer:test", "2016-06-06", "list-one-key")) .nodeWithKey(QName.create("serializer:test", "2016-06-06", "list-one-key"), QName.create("serializer:test", "2016-06-06", "name"), "value") - .build())); + .build()); } /** @@ -132,14 +133,38 @@ class YangInstanceIdentifierSerializerTest { void serializeMapWithMultipleKeysTest() { final var list = QName.create("serializer:test", "2016-06-06", "list-multiple-keys"); - assertEquals("serializer-test:list-multiple-keys=value-1,2,true", SERIALIZER.serializePath( + assertApiPath("serializer-test:list-multiple-keys=value-1,2,true", YangInstanceIdentifier.builder() .node(list) .nodeWithKey(list, ImmutableMap.of( QName.create(list, "name"), "value-1", QName.create(list, "number"), Uint8.TWO, QName.create(list, "enabled"), Boolean.TRUE)) - .build())); + .build()); + } + + /** + * Positive test of serialization of YangInstanceIdentifier to String when serialized + * YangInstanceIdentifier contains list with YangInstanceIdentifier as key. + * Returned String is compared to have expected value. + */ + @Test + void serializeMapWithIIDKeyTest() { + assertApiPath(""" + serializer-test:container-iid-key/list-iid-key=%2Fserializer-test-included%3Aiid-container%2Fiid%5Bid%3D%27\ + 0%27%5D""", + YangInstanceIdentifier.builder() + .node(QName.create("serializer:test", "2016-06-06", "container-iid-key")) + .node(QName.create("serializer:test", "2016-06-06", "list-iid-key")) + .nodeWithKey(QName.create("serializer:test", "2016-06-06", "list-iid-key"), + QName.create("serializer:test", "2016-06-06", "name"), + YangInstanceIdentifier.builder() + .node(QName.create("serializer:test:included", "2016-06-06", "iid-container")) + .node(QName.create("serializer:test:included", "2016-06-06", "iid")) + .nodeWithKey(QName.create("serializer:test:included", "2016-06-06", "iid"), + QName.create("serializer:test:included", "2016-06-06", "id"), 0) + .build()) + .build()); } /** @@ -149,8 +174,8 @@ class YangInstanceIdentifierSerializerTest { */ @Test void serializeLeafTest() { - assertEquals("serializer-test:leaf-0", SERIALIZER.serializePath( - YangInstanceIdentifier.of(QName.create("serializer:test", "2016-06-06", "leaf-0")))); + assertApiPath("serializer-test:leaf-0", + YangInstanceIdentifier.of(QName.create("serializer:test", "2016-06-06", "leaf-0"))); } /** @@ -160,21 +185,11 @@ class YangInstanceIdentifierSerializerTest { */ @Test void serializeLeafListTest() { - assertEquals("serializer-test:leaf-list-0=true", SERIALIZER.serializePath( + assertApiPath("serializer-test:leaf-list-0=true", YangInstanceIdentifier.builder() .node(QName.create("serializer:test", "2016-06-06", "leaf-list-0")) .node(new NodeWithValue<>(QName.create("serializer:test", "2016-06-06", "leaf-list-0"), Boolean.TRUE)) - .build())); - } - - /** - * Negative test of serialization YangInstanceIdentifier to String when - * SchemaContext is null. Test is expected to fail with - * NullPointerException. - */ - @Test - void serializeNullSchemaContextNegativeTest() { - assertThrows(NullPointerException.class, () -> new YangInstanceIdentifierSerializer(null)); + .build()); } /** @@ -184,7 +199,7 @@ class YangInstanceIdentifierSerializerTest { */ @Test void serializeNullDataNegativeTest() { - assertThrows(NullPointerException.class, () -> SERIALIZER.serializePath(null)); + assertThrows(NullPointerException.class, () -> NORMALIZER.canonicalize(null)); } /** @@ -194,7 +209,7 @@ class YangInstanceIdentifierSerializerTest { */ @Test void serializeEmptyDataTest() { - assertEquals("", SERIALIZER.serializePath(YangInstanceIdentifier.of())); + assertApiPath("", YangInstanceIdentifier.of()); } /** @@ -219,12 +234,12 @@ class YangInstanceIdentifierSerializerTest { */ @Test void serializePercentEncodingTest() { - assertEquals("serializer-test:list-one-key=foo%3Afoo bar%2Ffoo%2Cbar%2F%27bar%27", SERIALIZER.serializePath( + assertApiPath("serializer-test:list-one-key=foo%3Afoo bar%2Ffoo%2Cbar%2F%27bar%27", YangInstanceIdentifier.builder() .node(QName.create("serializer:test", "2016-06-06", "list-one-key")) .nodeWithKey(QName.create("serializer:test", "2016-06-06", "list-one-key"), QName.create("serializer:test", "2016-06-06", "name"), "foo:foo bar/foo,bar/'bar'") - .build())); + .build()); } /** @@ -232,12 +247,12 @@ class YangInstanceIdentifierSerializerTest { */ @Test void serializeNoPercentEncodingTest() { - assertEquals("serializer-test:list-one-key=foo\"b\"bar", SERIALIZER.serializePath( + assertApiPath("serializer-test:list-one-key=foo\"b\"bar", YangInstanceIdentifier.builder() .node(QName.create("serializer:test", "2016-06-06", "list-one-key")) .nodeWithKey(QName.create("serializer:test", "2016-06-06", "list-one-key"), QName.create("serializer:test", "2016-06-06", "name"), "foo\"b\"bar") - .build())); + .build()); } /** @@ -249,12 +264,12 @@ class YangInstanceIdentifierSerializerTest { final var list = QName.create("serializer:test:included", "2016-06-06", "augmented-list"); final var child = QName.create("serializer:test", "2016-06-06", "augmented-leaf"); - assertEquals("serializer-test-included:augmented-list=100/serializer-test:augmented-leaf", - SERIALIZER.serializePath(YangInstanceIdentifier.builder() + assertApiPath("serializer-test-included:augmented-list=100/serializer-test:augmented-leaf", + YangInstanceIdentifier.builder() .node(list) .node(NodeIdentifierWithPredicates.of(list, QName.create(list, "list-key"), 100)) .node(child) - .build())); + .build()); } /** @@ -305,8 +320,7 @@ class YangInstanceIdentifierSerializerTest { QName.create(list2, "key5"), "b")) .node(QName.create("list:test", "2016-04-29", "result")) .build(), dataYangII); - assertEquals("list-test:top/list1=%2C%27\"%3A\" %2F,,foo/list2=a,b/result", - SERIALIZER.serializePath(dataYangII)); + assertApiPath("list-test:top/list1=%2C%27\"%3A\" %2F,,foo/list2=a,b/result", dataYangII); } /** @@ -324,30 +338,29 @@ class YangInstanceIdentifierSerializerTest { .node(y) .node(new NodeWithValue<>(y, Uint32.valueOf(4))) .build(), dataYangII); - assertEquals(str, SERIALIZER.serializePath(dataYangII)); + assertApiPath(str, dataYangII); } - /** - * Positive test of serialization of an empty {@link YangInstanceIdentifier}. - */ - @Test - void codecDeserializeAndSerializeEmptyTest() { - assertEquals("", SERIALIZER.serializePath(YangInstanceIdentifier.of())); + private static void assertApiPath(final String expected, final YangInstanceIdentifier path) { + assertEquals(newApiPath(expected), NORMALIZER.canonicalize(path)); } private static YangInstanceIdentifier assertNormalized(final String str) { - try { - return assertInstanceOf(Data.class, new ApiPathNormalizer(DATABIND).normalizePath(ApiPath.parse(str))) - .instance(); - } catch (ParseException e) { - throw new AssertionError(e); - } + return assertInstanceOf(Data.class, NORMALIZER.normalizePath(newApiPath(str))).instance(); } private static RestconfError assertError(final YangInstanceIdentifier path) { - final var ex = assertThrows(RestconfDocumentedException.class, () -> SERIALIZER.serializePath(path)); + final var ex = assertThrows(RestconfDocumentedException.class, () -> NORMALIZER.canonicalize(path)); final var errors = ex.getErrors(); assertEquals(1, errors.size()); return errors.get(0); } + + private static ApiPath newApiPath(final String apiPath) { + try { + return ApiPath.parse(apiPath); + } catch (ParseException e) { + throw new AssertionError(e); + } + } } diff --git a/restconf/restconf-nb/src/test/resources/restconf/parser/serializer/serializer-test-included.yang b/restconf/restconf-nb/src/test/resources/restconf/parser/serializer/serializer-test-included.yang index c404aeb8af..c5a8b676b8 100644 --- a/restconf/restconf-nb/src/test/resources/restconf/parser/serializer/serializer-test-included.yang +++ b/restconf/restconf-nb/src/test/resources/restconf/parser/serializer/serializer-test-included.yang @@ -8,6 +8,20 @@ module serializer-test-included { "Initial revision."; } + typedef iid-ref { + type instance-identifier; + } + + container iid-container { + list iid { + key id; + + leaf id { + type uint32; + } + } + } + list augmented-list { key list-key; diff --git a/restconf/restconf-nb/src/test/resources/restconf/parser/serializer/serializer-test.yang b/restconf/restconf-nb/src/test/resources/restconf/parser/serializer/serializer-test.yang index 691e4dc8e2..e7eaf591fb 100644 --- a/restconf/restconf-nb/src/test/resources/restconf/parser/serializer/serializer-test.yang +++ b/restconf/restconf-nb/src/test/resources/restconf/parser/serializer/serializer-test.yang @@ -78,6 +78,16 @@ module serializer-test { } } + container container-iid-key { + list list-iid-key { + key name; + + leaf name { + type sti:iid-ref; + } + } + } + augment "/sti:augmented-list" { leaf augmented-leaf { type string; -- 2.36.6