From 7f1cd0306b107bf71ee931af676e82d8d2366352 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Tue, 22 Feb 2022 12:09:56 +0100 Subject: [PATCH] Separate out RPC input/output codecs This partially reverts 7a17eba49deb73733bdbb9579d2edd672bb0d71e, as leads to false sharing and corresponding problems with invalid InstanceIdentifiers not being caught. JIRA: MDSAL-724 Change-Id: I0ddf62efffb23e793b6727e0afcf50303f7aea74 Signed-off-by: Robert Varga --- .../codec/impl/SchemaRootCodecContext.java | 59 ++++++++++--------- .../binding/dom/codec/impl/Mdsal724Test.java | 28 +++++++++ 2 files changed, 60 insertions(+), 27 deletions(-) create mode 100644 binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal724Test.java diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/SchemaRootCodecContext.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/SchemaRootCodecContext.java index e438ed54dd..be48e153fd 100644 --- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/SchemaRootCodecContext.java +++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/SchemaRootCodecContext.java @@ -68,32 +68,6 @@ final class SchemaRootCodecContext extends DataContainerCo () -> new IllegalArgumentException("Supplied " + key + " is not valid notification")); return new NotificationCodecContext<>(key, schema, factory()); } - if (RpcInput.class.isAssignableFrom(key) || RpcOutput.class.isAssignableFrom(key)) { - final QName qname = BindingReflections.findQName(key); - final QNameModule qnameModule = qname.getModule(); - final Module module = getSchema().findModule(qnameModule).orElseThrow( - () -> new IllegalArgumentException("Failed to find module for " + qnameModule)); - final String className = BindingMapping.getClassName(qname); - - for (final RpcDefinition potential : module.getRpcs()) { - final QName potentialQName = potential.getQName(); - /* - * Check if rpc and class represents data from same module and then checks if rpc local name - * produces same class name as class name appended with Input/Output based on QName associated - * with binding class. - * - * FIXME: Rework this to have more precise logic regarding Binding Specification. - */ - if (key.getSimpleName().equals(BindingMapping.getClassName(potentialQName) + className)) { - final ContainerLike schema = getRpcDataSchema(potential, qname); - checkArgument(schema != null, "Schema for %s does not define input / output.", - potentialQName); - return DataContainerCodecPrototype.from(key, schema, factory()).get(); - } - } - - throw new IllegalArgumentException("Supplied class " + key + " is not valid RPC class."); - } return createDataTreeChildContext(key); } }); @@ -114,6 +88,37 @@ final class SchemaRootCodecContext extends DataContainerCo } }); + private final LoadingCache, ContainerNodeCodecContext> rpcDataByClass = CacheBuilder.newBuilder() + .build(new CacheLoader, ContainerNodeCodecContext>() { + @Override + public ContainerNodeCodecContext load(final Class key) { + final QName qname = BindingReflections.findQName(key); + final QNameModule qnameModule = qname.getModule(); + final Module module = getSchema().findModule(qnameModule).orElseThrow( + () -> new IllegalArgumentException("Failed to find module for " + qnameModule)); + final String className = BindingMapping.getClassName(qname); + + for (final RpcDefinition potential : module.getRpcs()) { + final QName potentialQName = potential.getQName(); + /* + * Check if rpc and class represents data from same module and then checks if rpc local name + * produces same class name as class name appended with Input/Output based on QName associated + * with binding class. + * + * FIXME: Rework this to have more precise logic regarding Binding Specification. + */ + if (key.getSimpleName().equals(BindingMapping.getClassName(potentialQName) + className)) { + final ContainerLike schema = getRpcDataSchema(potential, qname); + checkArgument(schema != null, "Schema for %s does not define input / output.", potentialQName); + return (ContainerNodeCodecContext) DataContainerCodecPrototype.from(key, schema, factory()) + .get(); + } + } + + throw new IllegalArgumentException("Supplied class " + key + " is not valid RPC class."); + } + }); + private final LoadingCache> childrenByQName = CacheBuilder.newBuilder().build(new CacheLoader<>() { @Override @@ -208,7 +213,7 @@ final class SchemaRootCodecContext extends DataContainerCo } ContainerNodeCodecContext getRpc(final Class rpcInputOrOutput) { - return (ContainerNodeCodecContext) streamChild((Class)rpcInputOrOutput); + return getOrRethrow(rpcDataByClass, rpcInputOrOutput); } RpcInputCodec getRpc(final Absolute containerPath) { diff --git a/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal724Test.java b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal724Test.java new file mode 100644 index 0000000000..0da0bed50f --- /dev/null +++ b/binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal724Test.java @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2022 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.mdsal.binding.dom.codec.impl; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; + +import org.junit.Test; +import org.opendaylight.mdsal.binding.dom.codec.api.IncorrectNestingException; +import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.md.sal.knock.knock.rev180723.KnockKnockInput; +import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; + +public class Mdsal724Test extends AbstractBindingCodecTest { + @Test + public void testRpcInputInstanceIdentifier() { + // An InstanceIdentifier pointing at a notification, unsafe to create + @SuppressWarnings({ "rawtypes", "unchecked" }) + final var iid = InstanceIdentifier.create((Class) KnockKnockInput.class); + final var ex = assertThrows(IncorrectNestingException.class, () -> codecContext.toYangInstanceIdentifier(iid)); + assertEquals("interface org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.md.sal.knock.knock." + + "rev180723.KnockKnockInput is not top-level item.", ex.getMessage()); + } +} -- 2.36.6