Separate out RPC input/output codecs 38/99838/3
authorRobert Varga <robert.varga@pantheon.tech>
Tue, 22 Feb 2022 11:09:56 +0000 (12:09 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Tue, 22 Feb 2022 13:45:53 +0000 (14:45 +0100)
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 <robert.varga@pantheon.tech>
binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/impl/SchemaRootCodecContext.java
binding/mdsal-binding-dom-codec/src/test/java/org/opendaylight/mdsal/binding/dom/codec/impl/Mdsal724Test.java [new file with mode: 0644]

index e438ed54dd090c16d5a7f52207071f556bbb1ae4..be48e153fd1820893683c14fca66ded6817fe4c0 100644 (file)
@@ -68,32 +68,6 @@ final class SchemaRootCodecContext<D extends DataObject> 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<D extends DataObject> extends DataContainerCo
             }
         });
 
+    private final LoadingCache<Class<?>, ContainerNodeCodecContext<?>> rpcDataByClass = CacheBuilder.newBuilder()
+        .build(new CacheLoader<Class<?>, 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<QName, DataContainerCodecContext<?,?>> childrenByQName =
         CacheBuilder.newBuilder().build(new CacheLoader<>() {
             @Override
@@ -208,7 +213,7 @@ final class SchemaRootCodecContext<D extends DataObject> extends DataContainerCo
     }
 
     ContainerNodeCodecContext<?> getRpc(final Class<? extends DataContainer> rpcInputOrOutput) {
-        return (ContainerNodeCodecContext<?>) streamChild((Class<? extends DataObject>)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 (file)
index 0000000..0da0bed
--- /dev/null
@@ -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());
+    }
+}