Fix ActionProviderService(Adapter)

ActionProviderServiceAdapter is always passing an empty set of
instances to DOMActionProviderService, which is a direct violation of
API contract. Unfortunately the implementation interprets empty set as
a no-op, resulting in the violation being ignored silently and routing
not working.

JIRA: MDSAL-679
Change-Id: I53a8977365166228ed30130519a2024372d89365
Signed-off-by: PeterSuna <Peter.Suna@pantheon.tech>
Signed-off-by: Ivan Hrasko <ivan.hrasko@pantheon.tech>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit cbf88a5bd758d3cab9652b02b4d91b43f3cc939b)

diff --git a/binding/mdsal-binding-api/src/main/java/org/opendaylight/mdsal/binding/api/ActionProviderService.java b/binding/mdsal-binding-api/src/main/java/org/opendaylight/mdsal/binding/api/ActionProviderService.java
index 3f4e737fd133cbe636270eabe0f8cddb59668564..64ce3ee89499fbcd13bf1792b1151d25e852f39e 100644 (file)
--- a/binding/mdsal-binding-api/src/main/java/org/opendaylight/mdsal/binding/api/ActionProviderService.java
+++ b/binding/mdsal-binding-api/src/main/java/org/opendaylight/mdsal/binding/api/ActionProviderService.java
@@ -10,7 +10,7 @@ package org.opendaylight.mdsal.binding.api;
 import com.google.common.annotations.Beta;
 import com.google.common.collect.ImmutableSet;
 import java.util.Set;
-import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.yangtools.concepts.ObjectRegistration;
 import org.opendaylight.yangtools.yang.binding.Action;
@@ -26,7 +26,6 @@ import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
  * @author Robert Varga
  */
 @Beta
-@NonNullByDefault
 public interface ActionProviderService extends BindingService {
     /**
      * Register an implementation of an action, potentially constrained to a set of nodes.
@@ -42,17 +41,19 @@ public interface ActionProviderService extends BindingService {
      * @throws UnsupportedOperationException if this service cannot handle requested datastore
      */
     <O extends DataObject, P extends InstanceIdentifier<O>, T extends Action<P, ?, ?>, S extends T>
-        ObjectRegistration<S> registerImplementation(Class<T> actionInterface, S implementation,
-                LogicalDatastoreType datastore, Set<DataTreeIdentifier<O>> validNodes);
+        @NonNull ObjectRegistration<S> registerImplementation(@NonNull Class<T> actionInterface,
+            @NonNull S implementation, @NonNull LogicalDatastoreType datastore,
+            @NonNull Set<InstanceIdentifier<O>> validNodes);
 
     default <O extends DataObject, P extends InstanceIdentifier<O>, T extends Action<P, ?, ?>, S extends T>
-        ObjectRegistration<S> registerImplementation(final Class<T> actionInterface, final S implementation,
-                final LogicalDatastoreType datastore) {
+        @NonNull ObjectRegistration<S> registerImplementation(final @NonNull Class<T> actionInterface,
+            final @NonNull S implementation, final @NonNull LogicalDatastoreType datastore) {
         return registerImplementation(actionInterface, implementation, datastore, ImmutableSet.of());
     }
 
     default <O extends DataObject, P extends InstanceIdentifier<O>, T extends Action<P, ?, ?>, S extends T>
-        ObjectRegistration<S> registerImplementation(final Class<T> actionInterface, final S implementation) {
+        @NonNull ObjectRegistration<S> registerImplementation(final @NonNull Class<T> actionInterface,
+            final @NonNull S implementation) {
         return registerImplementation(actionInterface, implementation, LogicalDatastoreType.OPERATIONAL);
     }
 }

diff --git a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/ActionProviderServiceAdapter.java b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/ActionProviderServiceAdapter.java
index 43525b3048d49d56e4aeb4041efed3fa796ea82c..cef0c0bca211c9633df8354ef4ca3ad090c5e96c 100644 (file)
--- a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/ActionProviderServiceAdapter.java
+++ b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/ActionProviderServiceAdapter.java
@@ -14,12 +14,14 @@ import com.google.common.collect.ClassToInstanceMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.util.concurrent.ListenableFuture;
 import java.util.Set;
+import java.util.stream.Collectors;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.opendaylight.mdsal.binding.api.ActionProviderService;
 import org.opendaylight.mdsal.binding.api.DataTreeIdentifier;
 import org.opendaylight.mdsal.binding.dom.adapter.BindingDOMAdapterBuilder.Factory;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.mdsal.dom.api.DOMActionImplementation;
+import org.opendaylight.mdsal.dom.api.DOMActionInstance;
 import org.opendaylight.mdsal.dom.api.DOMActionProviderService;
 import org.opendaylight.mdsal.dom.api.DOMActionResult;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier;
@@ -64,12 +66,16 @@ public final class ActionProviderServiceAdapter extends AbstractBindingAdapter<D
     @Override
     public <O extends DataObject, P extends InstanceIdentifier<O>, T extends Action<P, ?, ?>, S extends T>
             ObjectRegistration<S> registerImplementation(final Class<T> actionInterface, final S implementation,
-                final LogicalDatastoreType datastore, final Set<DataTreeIdentifier<O>> validNodes) {
-        final Absolute path = currentSerializer().getActionPath(actionInterface);
-        final ObjectRegistration<DOMActionImplementation> reg = getDelegate().registerActionImplementation(
-            new Impl(adapterContext(),
-                NodeIdentifier.create(YangConstants.operationOutputQName(path.lastNodeIdentifier().getModule())),
-                actionInterface, implementation), ImmutableSet.of());
+                final LogicalDatastoreType datastore, final Set<InstanceIdentifier<O>> validNodes) {
+        final CurrentAdapterSerializer serializer = currentSerializer();
+        final Absolute actionPath = serializer.getActionPath(actionInterface);
+        final Impl impl = new Impl(adapterContext(), actionPath, actionInterface, implementation);
+
+        final ObjectRegistration<DOMActionImplementation> reg = getDelegate().registerActionImplementation(impl,
+            DOMActionInstance.of(actionPath, validNodes.stream()
+                .map(instance -> serializer.toDOMDataTreeIdentifier(DataTreeIdentifier.create(datastore, instance)))
+                .collect(Collectors.toUnmodifiableSet())));
+
         return new AbstractObjectRegistration<>(implementation) {
             @Override
             protected void removeRegistration() {
@@ -84,10 +90,11 @@ public final class ActionProviderServiceAdapter extends AbstractBindingAdapter<D
         private final Action implementation;
         private final NodeIdentifier outputName;
 
-        Impl(final AdapterContext adapterContext, final NodeIdentifier outputName,
+        Impl(final AdapterContext adapterContext, final Absolute actionPath,
                 final Class<? extends Action<?, ?, ?>> actionInterface, final Action<?, ?, ?> implementation) {
             this.adapterContext = requireNonNull(adapterContext);
-            this.outputName = requireNonNull(outputName);
+            this.outputName = NodeIdentifier.create(
+                YangConstants.operationOutputQName(actionPath.lastNodeIdentifier().getModule()));
             this.actionInterface = requireNonNull(actionInterface);
             this.implementation = requireNonNull(implementation);
         }

diff --git a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/osgi/OSGiActionProviderService.java b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/osgi/OSGiActionProviderService.java
index 45a41430b4d1ef4c653e19e98630d9e25fef8471..f0a44b2c8186e975ee20cc7310b94e8f85d67238 100644 (file)
--- a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/osgi/OSGiActionProviderService.java
+++ b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/osgi/OSGiActionProviderService.java
@@ -10,9 +10,7 @@ package org.opendaylight.mdsal.binding.dom.adapter.osgi;
 import com.google.common.annotations.Beta;
 import java.util.Map;
 import java.util.Set;
-import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.mdsal.binding.api.ActionProviderService;
-import org.opendaylight.mdsal.binding.api.DataTreeIdentifier;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.yangtools.concepts.ObjectRegistration;
 import org.opendaylight.yangtools.yang.binding.Action;
@@ -34,10 +32,9 @@ public final class OSGiActionProviderService extends AbstractAdaptedService<Acti
     }
 
     @Override
-    public <O extends @NonNull DataObject, P extends @NonNull InstanceIdentifier<O>,
-            T extends @NonNull Action<P, ?, ?>, S extends T> @NonNull ObjectRegistration<S> registerImplementation(
-                    final Class<T> actionInterface, final S implementation, final LogicalDatastoreType datastore,
-                    final Set<@NonNull DataTreeIdentifier<O>> validNodes) {
+    public <O extends DataObject, P extends InstanceIdentifier<O>, T extends Action<P, ?, ?>, S extends T>
+        ObjectRegistration<S> registerImplementation(final Class<T> actionInterface, final S implementation,
+            final LogicalDatastoreType datastore, final Set<InstanceIdentifier<O>> validNodes) {
         return delegate().registerImplementation(actionInterface, implementation, datastore, validNodes);
     }
 

diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/AbstractActionAdapterTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/AbstractActionAdapterTest.java
new file mode 100644 (file)
index 0000000..23ce80d
--- /dev/null
+++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/AbstractActionAdapterTest.java
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2021 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.adapter;
+
+import static org.opendaylight.yangtools.yang.common.YangConstants.operationInputQName;
+import static org.opendaylight.yangtools.yang.common.YangConstants.operationOutputQName;
+import static org.opendaylight.yangtools.yang.data.impl.schema.Builders.containerBuilder;
+import static org.opendaylight.yangtools.yang.data.impl.schema.Builders.leafBuilder;
+
+import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.Cont;
+import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.cont.Foo;
+import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.cont.foo.Input;
+import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.cont.foo.InputBuilder;
+import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.cont.foo.Output;
+import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.cont.foo.OutputBuilder;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
+import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
+import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Absolute;
+
+public abstract class AbstractActionAdapterTest extends AbstractAdapterTest  {
+    protected static final Absolute FOO_PATH = Absolute.of(Cont.QNAME, Foo.QNAME);
+    protected static final NodeIdentifier FOO_INPUT = new NodeIdentifier(operationInputQName(Foo.QNAME.getModule()));
+    protected static final NodeIdentifier FOO_OUTPUT = new NodeIdentifier(operationOutputQName(Foo.QNAME.getModule()));
+    protected static final NodeIdentifier FOO_XYZZY = new NodeIdentifier(QName.create(Foo.QNAME, "xyzzy"));
+    protected static final ContainerNode DOM_FOO_INPUT = containerBuilder().withNodeIdentifier(FOO_INPUT)
+            .withChild(leafBuilder().withNodeIdentifier(FOO_XYZZY).withValue("xyzzy").build())
+            .build();
+    protected static final ContainerNode DOM_FOO_OUTPUT = containerBuilder().withNodeIdentifier(FOO_OUTPUT).build();
+    protected static final Input BINDING_FOO_INPUT = new InputBuilder().setXyzzy("xyzzy").build();
+    protected static final Output BINDING_FOO_OUTPUT = new OutputBuilder().build();
+
+}

diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ActionProviderServiceAdapterTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ActionProviderServiceAdapterTest.java
new file mode 100644 (file)
index 0000000..9fa7fd2
--- /dev/null
+++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ActionProviderServiceAdapterTest.java
@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2021 PANTHEON.tech s.r.o. 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.adapter;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.verify;
+
+import java.util.Set;
+import org.eclipse.jdt.annotation.NonNull;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+import org.opendaylight.mdsal.binding.api.ActionProviderService;
+import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
+import org.opendaylight.mdsal.dom.api.DOMActionInstance;
+import org.opendaylight.mdsal.dom.api.DOMActionProviderService;
+import org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier;
+import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.Cont;
+import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.cont.Foo;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+import org.opendaylight.yangtools.yang.common.RpcResultBuilder;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
+
+@RunWith(MockitoJUnitRunner.StrictStubs.class)
+public class ActionProviderServiceAdapterTest extends AbstractActionAdapterTest {
+    private static final @NonNull Foo FOO = (path, input) -> RpcResultBuilder.success(BINDING_FOO_OUTPUT).buildFuture();
+
+    @Mock
+    private DOMActionProviderService actionProvider;
+
+    private ActionProviderService adapter;
+
+    @Override
+    @Before
+    public void before() {
+        super.before();
+        adapter = new ActionProviderServiceAdapter(codec, actionProvider);
+    }
+
+    @Test
+    public void testInstanceRegistration() {
+        adapter.registerImplementation(Foo.class, FOO, LogicalDatastoreType.OPERATIONAL,
+            Set.of(InstanceIdentifier.create(Cont.class)));
+
+        verify(actionProvider).registerActionImplementation(any(), eq(DOMActionInstance.of(FOO_PATH,
+            new DOMDataTreeIdentifier(LogicalDatastoreType.OPERATIONAL,
+                YangInstanceIdentifier.create(new NodeIdentifier(Cont.QNAME))))));
+    }
+}

diff --git a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ActionServiceAdapterTest.java b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ActionServiceAdapterTest.java
index 7e759c627400bfb24801cef6e9e562387a6e71ce..244ce9b9902caadeba1ad8bb324689da4c038d25 100644 (file)
--- a/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ActionServiceAdapterTest.java
+++ b/binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ActionServiceAdapterTest.java
@@ -12,16 +12,12 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doReturn;
-import static org.opendaylight.yangtools.yang.common.YangConstants.operationInputQName;
-import static org.opendaylight.yangtools.yang.common.YangConstants.operationOutputQName;
-import static org.opendaylight.yangtools.yang.data.impl.schema.Builders.containerBuilder;
-import static org.opendaylight.yangtools.yang.data.impl.schema.Builders.leafBuilder;
 
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
+import java.util.List;
+import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import org.junit.Before;
 import org.junit.Test;
@@ -34,29 +30,12 @@ import org.opendaylight.mdsal.dom.api.DOMActionService;
 import org.opendaylight.mdsal.dom.spi.SimpleDOMActionResult;
 import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.Cont;
 import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.cont.Foo;
-import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.cont.foo.Input;
-import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.cont.foo.InputBuilder;
 import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.cont.foo.Output;
-import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.cont.foo.OutputBuilder;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
-import org.opendaylight.yangtools.yang.binding.RpcOutput;
-import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.common.RpcResult;
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
-import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode;
 
 @RunWith(MockitoJUnitRunner.StrictStubs.class)
-public class ActionServiceAdapterTest extends AbstractAdapterTest {
-    private static final NodeIdentifier FOO_INPUT = NodeIdentifier.create(operationInputQName(Foo.QNAME.getModule()));
-    private static final NodeIdentifier FOO_OUTPUT = NodeIdentifier.create(operationOutputQName(Foo.QNAME.getModule()));
-    private static final NodeIdentifier FOO_XYZZY = NodeIdentifier.create(QName.create(Foo.QNAME, "xyzzy"));
-    private static final ContainerNode DOM_FOO_INPUT = containerBuilder().withNodeIdentifier(FOO_INPUT)
-            .withChild(leafBuilder().withNodeIdentifier(FOO_XYZZY).withValue("xyzzy").build())
-            .build();
-    private static final ContainerNode DOM_FOO_OUTPUT = containerBuilder().withNodeIdentifier(FOO_OUTPUT).build();
-    private static final Input BINDING_FOO_INPUT = new InputBuilder().setXyzzy("xyzzy").build();
-    private static final RpcOutput BINDING_FOO_OUTPUT = new OutputBuilder().build();
-
+public class ActionServiceAdapterTest extends AbstractActionAdapterTest {
     @Mock
     private DOMActionService delegate;
 
@@ -77,16 +56,15 @@ public class ActionServiceAdapterTest extends AbstractAdapterTest {
 
     @Test
     public void testInvocation() throws ExecutionException {
-        final Foo handle = service.getActionHandle(Foo.class, ImmutableSet.of());
+        final Foo handle = service.getActionHandle(Foo.class, Set.of());
         final ListenableFuture<RpcResult<Output>> future = handle.invoke(InstanceIdentifier.create(Cont.class),
             BINDING_FOO_INPUT);
         assertNotNull(future);
         assertFalse(future.isDone());
-        domResult.set(new SimpleDOMActionResult(DOM_FOO_OUTPUT, ImmutableList.of()));
+        domResult.set(new SimpleDOMActionResult(DOM_FOO_OUTPUT, List.of()));
         final RpcResult<Output> bindingResult = Futures.getDone(future);
 
-        assertEquals(ImmutableList.of(), bindingResult.getErrors());
+        assertEquals(List.of(), bindingResult.getErrors());
         assertEquals(BINDING_FOO_OUTPUT, bindingResult.getResult());
     }
-
 }