Fix action invocation and registration 00/98800/1
authorRobert Varga <robert.varga@pantheon.tech>
Thu, 25 Nov 2021 15:28:33 +0000 (16:28 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Thu, 2 Dec 2021 10:00:27 +0000 (11:00 +0100)
Binding actions involving groupings are a bit more complicated when it
comes to DOM mapping. An instantiated Action can correspond to any
number of ActionEffectiveStatements, but we skimped that over and used
the SchemaPath (and after that, an invalid SchemaNodeIdentifier) to
identify them.

Correct this by requiring users to provide an ActionSpec, which is a
combination of an Action interface and a corresponding instantiation
path.

JIRA: MDSAL-712
Change-Id: I632c0f51b2e71fa1b0a04e43d5b1c50286430b21
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit 54cf838b1a16c1f911963e23e738b1eef753c421)

16 files changed:
binding/mdsal-binding-api/src/main/java/module-info.java
binding/mdsal-binding-api/src/main/java/org/opendaylight/mdsal/binding/api/ActionProviderService.java
binding/mdsal-binding-api/src/main/java/org/opendaylight/mdsal/binding/api/ActionService.java
binding/mdsal-binding-api/src/main/java/org/opendaylight/mdsal/binding/api/ActionSpec.java [new file with mode: 0644]
binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/ActionAdapter.java
binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/ActionAdapterFilter.java [new file with mode: 0644]
binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/ActionProviderServiceAdapter.java
binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/ActionServiceAdapter.java
binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/CurrentAdapterSerializer.java
binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/osgi/OSGiActionProviderService.java
binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/osgi/OSGiActionService.java
binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ActionLookupTest.java
binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ActionProviderServiceAdapterTest.java
binding/mdsal-binding-dom-adapter/src/test/java/org/opendaylight/mdsal/binding/dom/adapter/ActionServiceAdapterTest.java
binding/mdsal-binding-runtime-api/src/main/java/org/opendaylight/mdsal/binding/runtime/api/BindingRuntimeContext.java
binding/mdsal-binding-test-model/src/main/yang/actions.yang

index c61351a2c91b4fbce679a63aa315aed9192a1ef2..26ccc9e7f0f3dc3ac15d78f9d84cb8035cd4a4fe 100644 (file)
@@ -9,6 +9,7 @@ module org.opendaylight.mdsal.binding.api {
     exports org.opendaylight.mdsal.binding.api;
     exports org.opendaylight.mdsal.binding.api.query;
 
+    requires transitive org.opendaylight.yangtools.concepts;
     requires transitive org.opendaylight.yangtools.yang.binding;
     requires transitive org.opendaylight.mdsal.common.api;
     requires org.opendaylight.yangtools.util;
index 64ce3ee89499fbcd13bf1792b1151d25e852f39e..0b546110680d93b8c2ae0f5770072b36251129b1 100644 (file)
@@ -30,7 +30,7 @@ public interface ActionProviderService extends BindingService {
     /**
      * Register an implementation of an action, potentially constrained to a set of nodes.
      *
-     * @param actionInterface Generated Action interface
+     * @param spec Action instance specification
      * @param implementation Implementation of {@code actionInterface}
      * @param datastore {@link LogicalDatastoreType} on which the implementation operates
      * @param validNodes Set of nodes this implementation is constrained to, empty if this implementation can handle
@@ -40,20 +40,19 @@ public interface ActionProviderService extends BindingService {
      * @throws IllegalArgumentException if any of the {@code validNodes} does not match {@code datastore}
      * @throws UnsupportedOperationException if this service cannot handle requested datastore
      */
-    <O extends DataObject, P extends InstanceIdentifier<O>, T extends Action<P, ?, ?>, S extends T>
-        @NonNull ObjectRegistration<S> registerImplementation(@NonNull Class<T> actionInterface,
-            @NonNull S implementation, @NonNull LogicalDatastoreType datastore,
-            @NonNull Set<InstanceIdentifier<O>> validNodes);
+    <P extends DataObject, A extends Action<InstanceIdentifier<P>, ?, ?>, S extends A>
+        @NonNull ObjectRegistration<S> registerImplementation(@NonNull ActionSpec<A, P> spec, @NonNull S implementation,
+            @NonNull LogicalDatastoreType datastore, @NonNull Set<InstanceIdentifier<P>> validNodes);
 
-    default <O extends DataObject, P extends InstanceIdentifier<O>, T extends Action<P, ?, ?>, S extends T>
-        @NonNull ObjectRegistration<S> registerImplementation(final @NonNull Class<T> actionInterface,
+    default <P extends DataObject, T extends Action<InstanceIdentifier<P>, ?, ?>, S extends T>
+        @NonNull ObjectRegistration<S> registerImplementation(final @NonNull ActionSpec<T, P> spec,
             final @NonNull S implementation, final @NonNull LogicalDatastoreType datastore) {
-        return registerImplementation(actionInterface, implementation, datastore, ImmutableSet.of());
+        return registerImplementation(spec, implementation, datastore, ImmutableSet.of());
     }
 
-    default <O extends DataObject, P extends InstanceIdentifier<O>, T extends Action<P, ?, ?>, S extends T>
-        @NonNull ObjectRegistration<S> registerImplementation(final @NonNull Class<T> actionInterface,
+    default <P extends DataObject, T extends Action<InstanceIdentifier<P>, ?, ?>, S extends T>
+        @NonNull ObjectRegistration<S> registerImplementation(final @NonNull ActionSpec<T, P> spec,
             final @NonNull S implementation) {
-        return registerImplementation(actionInterface, implementation, LogicalDatastoreType.OPERATIONAL);
+        return registerImplementation(spec, implementation, LogicalDatastoreType.OPERATIONAL);
     }
 }
index 6642c34e6e7de0e6575cc1cdc1154a9d29ab541e..be0b85c9071b882bb904d2615a3df7bc8598e127 100644 (file)
@@ -47,32 +47,32 @@ public interface ActionService extends BindingService {
      * The returned proxy is automatically updated with the most recent registered implementation, hence there is no
      * guarantee that multiple consecutive invocations will be handled by the same implementation.
      *
-     * @param actionInterface Generated Action interface
+     * @param spec Action instance specification
      * @param validNodes Set of nodes this service will be constrained to, empty if no constraints are known
      * @return A proxy implementation of the generated interface
      * @throws NullPointerException if {@code actionInterface} is null
      * @throws IllegalArgumentException when {@code actionInterface} does not conform to the Binding Specification
      */
-    <O extends DataObject, T extends Action<?, ?, ?>> T getActionHandle(Class<T> actionInterface,
-            Set<DataTreeIdentifier<O>> validNodes);
+    <P extends DataObject, A extends Action<InstanceIdentifier<P>, ?, ?>> A getActionHandle(ActionSpec<A, P> spec,
+        Set<DataTreeIdentifier<P>> validNodes);
 
-    default <O extends DataObject, P extends InstanceIdentifier<O>, T extends Action<P, ?, ?>> T getActionHandle(
-            final Class<T> actionInterface) {
-        return getActionHandle(actionInterface, ImmutableSet.of());
+    default <P extends DataObject, A extends Action<InstanceIdentifier<P>, ?, ?>> A getActionHandle(
+            final ActionSpec<A, P> spec) {
+        return getActionHandle(spec, ImmutableSet.of());
     }
 
-    default <O extends DataObject, P extends InstanceIdentifier<O>, T extends Action<P, ?, ?>> T getActionHandle(
-            final Class<T> actionInterface, final LogicalDatastoreType dataStore, final P path) {
-        return getActionHandle(actionInterface, ImmutableSet.of(DataTreeIdentifier.create(dataStore, path)));
+    default <P extends DataObject, A extends Action<InstanceIdentifier<P>, ?, ?>> A getActionHandle(
+            final ActionSpec<A, P> spec, final LogicalDatastoreType dataStore, final InstanceIdentifier<P> path) {
+        return getActionHandle(spec, ImmutableSet.of(DataTreeIdentifier.create(dataStore, path)));
     }
 
-    default <O extends DataObject, P extends InstanceIdentifier<O>, T extends Action<P, ?, ?>> T getActionHandle(
-            final Class<T> actionInterface, final P path) {
-        return getActionHandle(actionInterface, LogicalDatastoreType.OPERATIONAL, path);
+    default <P extends DataObject, A extends Action<InstanceIdentifier<P>, ?, ?>> A getActionHandle(
+            final ActionSpec<A, P> spec, final InstanceIdentifier<P> path) {
+        return getActionHandle(spec, LogicalDatastoreType.OPERATIONAL, path);
     }
 
-    default <O extends DataObject, P extends InstanceIdentifier<O>, T extends Action<P, ?, ?>> T getActionHandle(
-            final Class<T> actionInterface, @SuppressWarnings("unchecked") final DataTreeIdentifier<O>... nodes) {
-        return getActionHandle(actionInterface, ImmutableSet.copyOf(nodes));
+    default <P extends DataObject, A extends Action<InstanceIdentifier<P>, ?, ?>> A getActionHandle(
+            final ActionSpec<A, P> spec, @SuppressWarnings("unchecked") final DataTreeIdentifier<P>... nodes) {
+        return getActionHandle(spec, ImmutableSet.copyOf(nodes));
     }
 }
diff --git a/binding/mdsal-binding-api/src/main/java/org/opendaylight/mdsal/binding/api/ActionSpec.java b/binding/mdsal-binding-api/src/main/java/org/opendaylight/mdsal/binding/api/ActionSpec.java
new file mode 100644 (file)
index 0000000..206fed3
--- /dev/null
@@ -0,0 +1,120 @@
+/*
+ * 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.api;
+
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.annotations.Beta;
+import com.google.common.base.MoreObjects;
+import java.util.Objects;
+import org.eclipse.jdt.annotation.NonNull;
+import org.opendaylight.yangtools.concepts.Immutable;
+import org.opendaylight.yangtools.concepts.Mutable;
+import org.opendaylight.yangtools.yang.binding.Action;
+import org.opendaylight.yangtools.yang.binding.Augmentation;
+import org.opendaylight.yangtools.yang.binding.ChildOf;
+import org.opendaylight.yangtools.yang.binding.ChoiceIn;
+import org.opendaylight.yangtools.yang.binding.DataObject;
+import org.opendaylight.yangtools.yang.binding.DataRoot;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier.InstanceIdentifierBuilder;
+
+/**
+ * A combination of an {@link Action} class and its corresponding instantiation wildcard, expressed as
+ * an {@link InstanceIdentifier}. This glue is required because action interfaces are generated at the place of their
+ * definition, most importantly in {@code grouping} and we actually need to bind to a particular instantiation (e.g. a
+ * place where {@code uses} references that grouping).
+ *
+ * @param <A> Generated Action interface type
+ * @param <P> Action parent type
+ */
+@Beta
+public final class ActionSpec<A extends Action<InstanceIdentifier<P>, ?, ?>, P extends DataObject>
+        implements Immutable {
+    private final @NonNull InstanceIdentifier<P> path;
+    private final @NonNull Class<A> type;
+
+    private ActionSpec(final Class<A> type, final InstanceIdentifier<P> path) {
+        this.type = requireNonNull(type);
+        this.path = requireNonNull(path);
+    }
+
+    public static <P extends ChildOf<? extends DataRoot>> @NonNull Builder<P> builder(final Class<P> container) {
+        return new Builder<>(InstanceIdentifier.builder(container));
+    }
+
+    public static <C extends ChoiceIn<? extends DataRoot> & DataObject, P extends ChildOf<? super C>>
+            @NonNull Builder<P> builder(final Class<C> caze, final Class<P> container) {
+        return new Builder<>(InstanceIdentifier.builder(caze, container));
+    }
+
+    public @NonNull InstanceIdentifier<P> path() {
+        return path;
+    }
+
+    public @NonNull Class<A> type() {
+        return type;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(type, path);
+    }
+
+    @Override
+    public boolean equals(final Object obj) {
+        if (obj == this) {
+            return true;
+        }
+        if (!(obj instanceof ActionSpec)) {
+            return false;
+        }
+        final var other = (ActionSpec<?, ?>) obj;
+        return type.equals(other.type) && path.equals(other.path);
+    }
+
+    @Override
+    public String toString() {
+        return MoreObjects.toStringHelper(this).add("action", type).add("path", path).toString();
+    }
+
+    @Beta
+    public static final class Builder<P extends DataObject> implements Mutable {
+        private final InstanceIdentifierBuilder<P> pathBuilder;
+
+        Builder(final InstanceIdentifierBuilder<P> pathBuilder) {
+            this.pathBuilder = requireNonNull(pathBuilder);
+        }
+
+        public <N extends ChildOf<? super P>> @NonNull Builder<N> withPathChild(final Class<N> container) {
+            pathBuilder.child(container);
+            return castThis();
+        }
+
+        public <C extends ChoiceIn<? super P> & DataObject, N extends ChildOf<? super C>>
+                @NonNull Builder<N> withPathChild(final Class<C> caze, final Class<N> container) {
+            pathBuilder.child(caze, container);
+            return castThis();
+        }
+
+        public <N extends DataObject & Augmentation<? super P>> @NonNull Builder<N> withPathAugmentation(
+                final Class<N> container) {
+            pathBuilder.augmentation(container);
+            return castThis();
+        }
+
+        public <A extends Action<InstanceIdentifier<P>, ?, ?>> @NonNull ActionSpec<A, P> build(final Class<A> type) {
+            return new ActionSpec<>(type, pathBuilder.build());
+        }
+
+        @SuppressWarnings("unchecked")
+        private <N extends DataObject> @NonNull Builder<N> castThis() {
+            return (Builder<N>) this;
+        }
+    }
+}
index a8ccb88d196526d9204e98131941e7e88073bfc3..e8d9e95eaf6654ded084d097134bbd1242f8260c 100644 (file)
@@ -16,35 +16,31 @@ import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
-import org.eclipse.jdt.annotation.NonNullByDefault;
-import org.eclipse.jdt.annotation.Nullable;
+import org.eclipse.jdt.annotation.NonNull;
+import org.opendaylight.mdsal.binding.api.ActionSpec;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.mdsal.dom.api.DOMActionResult;
 import org.opendaylight.mdsal.dom.api.DOMActionService;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier;
-import org.opendaylight.yangtools.yang.binding.Action;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.binding.RpcInput;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Absolute;
 
-@NonNullByDefault
 final class ActionAdapter extends AbstractBindingAdapter<DOMActionService> implements InvocationHandler {
-    private final Class<? extends Action<?, ?, ?>> type;
-    private final NodeIdentifier inputName;
-    private final Absolute actionPath;
+    private final @NonNull ActionSpec<?, ?> spec;
+    private final @NonNull NodeIdentifier inputName;
+    private final @NonNull Absolute actionPath;
 
-    ActionAdapter(final AdapterContext codec, final DOMActionService delegate,
-            final Class<? extends Action<?, ?, ?>> type) {
+    ActionAdapter(final AdapterContext codec, final DOMActionService delegate, final ActionSpec<?, ?> spec) {
         super(codec, delegate);
-        this.type = requireNonNull(type);
-        this.actionPath = currentSerializer().getActionPath(type);
-        this.inputName = NodeIdentifier.create(operationInputQName(actionPath.lastNodeIdentifier().getModule()));
+        this.spec = requireNonNull(spec);
+        actionPath = currentSerializer().getActionPath(spec);
+        inputName = NodeIdentifier.create(operationInputQName(actionPath.lastNodeIdentifier().getModule()));
     }
 
     @Override
-    public @Nullable Object invoke(final @Nullable Object proxy, final @Nullable Method method,
-            final Object @Nullable [] args) throws Throwable {
+    public Object invoke(final Object proxy, final Method method, final Object [] args) throws NoSuchMethodError {
         switch (method.getName()) {
             case "equals":
                 if (args.length == 1) {
@@ -58,7 +54,7 @@ final class ActionAdapter extends AbstractBindingAdapter<DOMActionService> imple
                 break;
             case "toString":
                 if (args.length == 0) {
-                    return type.getName() + "$Adapter{delegate=" + getDelegate() + "}";
+                    return spec.type().getName() + "$Adapter{delegate=" + getDelegate() + "}";
                 }
                 break;
             case "invoke":
@@ -69,7 +65,7 @@ final class ActionAdapter extends AbstractBindingAdapter<DOMActionService> imple
                     final ListenableFuture<? extends DOMActionResult> future = getDelegate().invokeAction(actionPath,
                         new DOMDataTreeIdentifier(LogicalDatastoreType.OPERATIONAL,
                             serializer.toYangInstanceIdentifier(path)),
-                        serializer.toLazyNormalizedNodeActionInput(type, inputName, input));
+                        serializer.toLazyNormalizedNodeActionInput(spec.type(), inputName, input));
 
                     // Invocation returned a future we know about -- return that future instead
                     if (ENABLE_CODEC_SHORTCUT && future instanceof BindingRpcFutureAware) {
@@ -78,7 +74,7 @@ final class ActionAdapter extends AbstractBindingAdapter<DOMActionService> imple
 
                     return Futures.transform(future,
                         dom -> RpcResultUtil.rpcResultFromDOM(dom.getErrors(), dom.getOutput()
-                            .map(output -> serializer.fromNormalizedNodeActionOutput(type, output))
+                            .map(output -> serializer.fromNormalizedNodeActionOutput(spec.type(), output))
                             .orElse(null)),
                         MoreExecutors.directExecutor());
                 }
diff --git a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/ActionAdapterFilter.java b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/ActionAdapterFilter.java
new file mode 100644 (file)
index 0000000..b71187a
--- /dev/null
@@ -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 com.google.common.base.Preconditions.checkState;
+import static java.util.Objects.requireNonNull;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.Set;
+import org.opendaylight.mdsal.binding.api.DataTreeIdentifier;
+import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+
+final class ActionAdapterFilter implements InvocationHandler {
+    private final Set<DataTreeIdentifier<?>> nodes;
+    private final ActionAdapter delegate;
+
+    ActionAdapterFilter(final ActionAdapter delegate, final Set<DataTreeIdentifier<?>> nodes) {
+        this.delegate = requireNonNull(delegate);
+        this.nodes = requireNonNull(nodes);
+    }
+
+    @Override
+    public Object invoke(final Object proxy, final Method method, final Object[] args) throws NoSuchMethodError {
+        if (method.getName().equals("invoke") && args.length == 2) {
+            final InstanceIdentifier<?> path = (InstanceIdentifier<?>) requireNonNull(args[0]);
+            checkState(nodes.contains(DataTreeIdentifier.create(LogicalDatastoreType.OPERATIONAL, path)),
+                "Cannot service %s", path);
+        }
+        return delegate.invoke(proxy, method, args);
+    }
+}
index dbf8a0369a84d7085610343181cc3aa8143a44af..8381b216e6edf4176fda65cf9839245cd6293cfa 100644 (file)
@@ -17,6 +17,7 @@ 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.ActionSpec;
 import org.opendaylight.mdsal.binding.api.DataTreeIdentifier;
 import org.opendaylight.mdsal.binding.dom.adapter.BindingDOMAdapterBuilder.Factory;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
@@ -65,12 +66,12 @@ 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<InstanceIdentifier<O>> validNodes) {
+    public <P extends DataObject, A extends Action<InstanceIdentifier<P>, ?, ?>, S extends A>
+            ObjectRegistration<S> registerImplementation(final ActionSpec<A, P> spec, final S implementation,
+                final LogicalDatastoreType datastore, final Set<InstanceIdentifier<P>> validNodes) {
         final CurrentAdapterSerializer serializer = currentSerializer();
-        final Absolute actionPath = serializer.getActionPath(actionInterface);
-        final Impl impl = new Impl(adapterContext(), actionPath, actionInterface, implementation);
+        final Absolute actionPath = serializer.getActionPath(spec);
+        final Impl impl = new Impl(adapterContext(), actionPath, spec.type(), implementation);
         final DOMActionInstance instance = validNodes.isEmpty()
             // Register on the entire datastore
             ? DOMActionInstance.of(actionPath, new DOMDataTreeIdentifier(datastore, YangInstanceIdentifier.empty()))
@@ -99,7 +100,7 @@ public final class ActionProviderServiceAdapter extends AbstractBindingAdapter<D
         Impl(final AdapterContext adapterContext, final Absolute actionPath,
                 final Class<? extends Action<?, ?, ?>> actionInterface, final Action<?, ?, ?> implementation) {
             this.adapterContext = requireNonNull(adapterContext);
-            this.outputName = NodeIdentifier.create(
+            outputName = NodeIdentifier.create(
                 YangConstants.operationOutputQName(actionPath.lastNodeIdentifier().getModule()));
             this.actionInterface = requireNonNull(actionInterface);
             this.implementation = requireNonNull(implementation);
index deabcd10c4d999b9dd5365fe7513609333e830d5..77416b5f1e6c8a5b1590bffa477f95cf71d43e40 100644 (file)
@@ -8,33 +8,26 @@
 package org.opendaylight.mdsal.binding.dom.adapter;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkState;
-import static java.util.Objects.requireNonNull;
 
 import com.google.common.collect.ClassToInstanceMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.util.concurrent.ListenableFuture;
 import java.lang.reflect.Proxy;
 import java.util.Set;
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.opendaylight.mdsal.binding.api.ActionService;
+import org.opendaylight.mdsal.binding.api.ActionSpec;
 import org.opendaylight.mdsal.binding.api.DataTreeIdentifier;
 import org.opendaylight.mdsal.binding.dom.adapter.BindingDOMAdapterBuilder.Factory;
 import org.opendaylight.mdsal.binding.spec.reflect.BindingReflections;
-import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.mdsal.dom.api.DOMActionService;
 import org.opendaylight.mdsal.dom.api.DOMService;
-import org.opendaylight.yangtools.concepts.Delegator;
 import org.opendaylight.yangtools.yang.binding.Action;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
-import org.opendaylight.yangtools.yang.binding.RpcInput;
-import org.opendaylight.yangtools.yang.binding.RpcOutput;
-import org.opendaylight.yangtools.yang.common.RpcResult;
 
 @NonNullByDefault
 final class ActionServiceAdapter
-        extends AbstractBindingLoadingAdapter<DOMActionService, Class<? extends Action<?, ?, ?>>, ActionAdapter>
+        extends AbstractBindingLoadingAdapter<DOMActionService, ActionSpec<?, ?>, ActionAdapter>
         implements ActionService {
     private static final class Builder extends BindingDOMAdapterBuilder<ActionService> {
         Builder(final AdapterContext adapterContext) {
@@ -52,29 +45,6 @@ final class ActionServiceAdapter
         }
     }
 
-    private static final class ConstrainedAction implements Delegator<Action<?, ?, ?>>,
-            Action<InstanceIdentifier<?>, RpcInput, RpcOutput> {
-        private final Action<InstanceIdentifier<?>, RpcInput, RpcOutput> delegate;
-        private final Set<? extends DataTreeIdentifier<?>> nodes;
-
-        ConstrainedAction(final Action<?, ?, ?> delegate, final Set<? extends DataTreeIdentifier<?>> nodes) {
-            this.delegate = requireNonNull((Action) delegate);
-            this.nodes = requireNonNull(nodes);
-        }
-
-        @Override
-        public ListenableFuture<RpcResult<RpcOutput>> invoke(final InstanceIdentifier<?> path, final RpcInput input) {
-            checkState(nodes.contains(DataTreeIdentifier.create(LogicalDatastoreType.OPERATIONAL, path)),
-                "Cannot service %s", path);
-            return delegate.invoke(path, input);
-        }
-
-        @Override
-        public Action<?, ?, ?> getDelegate() {
-            return delegate;
-        }
-    }
-
     static final Factory<ActionService> BUILDER_FACTORY = Builder::new;
 
     ActionServiceAdapter(final AdapterContext adapterContext, final DOMActionService delegate) {
@@ -82,17 +52,19 @@ final class ActionServiceAdapter
     }
 
     @Override
-    public <O extends DataObject, T extends Action<?, ?, ?>> T getActionHandle(final Class<T> actionInterface,
-            final Set<DataTreeIdentifier<O>> nodes) {
-        return !nodes.isEmpty() ? (T) new ConstrainedAction(getActionHandle(actionInterface, ImmutableSet.of()), nodes)
-                : (T) Proxy.newProxyInstance(actionInterface.getClassLoader(), new Class[] { actionInterface },
-                    getAdapter(actionInterface));
+    public <P extends DataObject, A extends Action<InstanceIdentifier<P>, ?, ?>> A getActionHandle(
+            final ActionSpec<A, P> spec, final Set<DataTreeIdentifier<P>> nodes) {
+        final var type = spec.type();
+        final var adapter = getAdapter(spec);
+        return type.cast(Proxy.newProxyInstance(type.getClassLoader(), new Class[] { type },
+            nodes.isEmpty() ? adapter : new ActionAdapterFilter(adapter, Set.copyOf(nodes))));
     }
 
     @Override
-    ActionAdapter loadAdapter(final Class<? extends Action<?, ?, ?>> key) {
-        checkArgument(BindingReflections.isBindingClass(key));
-        checkArgument(key.isInterface(), "Supplied Action type must be an interface.");
+    ActionAdapter loadAdapter(final ActionSpec<?, ?> key) {
+        final var type = key.type();
+        checkArgument(BindingReflections.isBindingClass(type));
+        checkArgument(type.isInterface(), "Supplied Action type must be an interface.");
         return new ActionAdapter(adapterContext(), getDelegate(), key);
     }
 }
index cea844371fdda9e5f6c2169c6810c74fe0e50074..f444c1cf8e3529b135a35c5ee4b156b5eadd6320 100644 (file)
@@ -7,7 +7,7 @@
  */
 package org.opendaylight.mdsal.binding.dom.adapter;
 
-import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Verify.verify;
 import static com.google.common.base.Verify.verifyNotNull;
 import static java.util.Objects.requireNonNull;
 
@@ -21,6 +21,7 @@ import java.lang.reflect.Method;
 import java.util.Collection;
 import java.util.stream.Collectors;
 import org.eclipse.jdt.annotation.NonNull;
+import org.opendaylight.mdsal.binding.api.ActionSpec;
 import org.opendaylight.mdsal.binding.api.DataTreeIdentifier;
 import org.opendaylight.mdsal.binding.dom.codec.spi.BindingDOMCodecServices;
 import org.opendaylight.mdsal.binding.dom.codec.spi.ForwardingBindingDOMCodecServices;
@@ -28,16 +29,20 @@ import org.opendaylight.mdsal.binding.runtime.api.BindingRuntimeContext;
 import org.opendaylight.mdsal.binding.spec.naming.BindingMapping;
 import org.opendaylight.mdsal.binding.spec.reflect.BindingReflections;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeIdentifier;
-import org.opendaylight.yangtools.yang.binding.Action;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.binding.RpcService;
 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.AugmentationIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.model.api.Module;
 import org.opendaylight.yangtools.yang.model.api.RpcDefinition;
+import org.opendaylight.yangtools.yang.model.api.stmt.ActionEffectiveStatement;
+import org.opendaylight.yangtools.yang.model.api.stmt.ListEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Absolute;
+import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -81,10 +86,38 @@ public final class CurrentAdapterSerializer extends ForwardingBindingDOMCodecSer
         return subtrees.stream().map(this::toDOMDataTreeIdentifier).collect(Collectors.toSet());
     }
 
-    @NonNull Absolute getActionPath(final @NonNull Class<? extends Action<?, ?, ?>> type) {
-        final Absolute identifier = getRuntimeContext().getActionIdentifier(type);
-        checkArgument(identifier != null, "Failed to find schema for %s", type);
-        return identifier;
+    @NonNull Absolute getActionPath(final @NonNull ActionSpec<?, ?> spec) {
+        final var stack = SchemaInferenceStack.of(getRuntimeContext().getEffectiveModelContext());
+        final var it = toYangInstanceIdentifier(spec.path()).getPathArguments().iterator();
+        verify(it.hasNext(), "Unexpected empty instance identifier for %s", spec);
+
+        QNameModule lastNamespace;
+        do {
+            final var arg = it.next();
+            if (arg instanceof AugmentationIdentifier) {
+                final var augChildren = ((AugmentationIdentifier) arg).getPossibleChildNames();
+                verify(!augChildren.isEmpty(), "Invalid empty augmentation %s", arg);
+                lastNamespace = augChildren.iterator().next().getModule();
+                continue;
+            }
+
+            final var qname = arg.getNodeType();
+            final var stmt = stack.enterDataTree(qname);
+            lastNamespace = qname.getModule();
+            if (stmt instanceof ListEffectiveStatement) {
+                // Lists have two steps
+                verify(it.hasNext(), "Unexpected list termination at %s in %s", stmt, spec);
+                // Verify just to make sure we are doing the right thing
+                final var skipped = it.next();
+                verify(skipped instanceof NodeIdentifier, "Unexpected skipped list entry item %s in %s", skipped, spec);
+                verify(stmt.argument().equals(skipped.getNodeType()), "Mismatched list entry item %s in %s", skipped,
+                    spec);
+            }
+        } while (it.hasNext());
+
+        final var stmt = stack.enterSchemaTree(BindingReflections.findQName(spec.type()).bindTo(lastNamespace));
+        verify(stmt instanceof ActionEffectiveStatement, "Action %s resolved to unexpected statement %s", spec, stmt);
+        return stack.toSchemaNodeIdentifier();
     }
 
     // FIXME: This should be probably part of Binding Runtime context
index f0a44b2c8186e975ee20cc7310b94e8f85d67238..90995d0446fa452d6912e9b70c34fd0edc017d1b 100644 (file)
@@ -11,6 +11,7 @@ import com.google.common.annotations.Beta;
 import java.util.Map;
 import java.util.Set;
 import org.opendaylight.mdsal.binding.api.ActionProviderService;
+import org.opendaylight.mdsal.binding.api.ActionSpec;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.yangtools.concepts.ObjectRegistration;
 import org.opendaylight.yangtools.yang.binding.Action;
@@ -32,10 +33,10 @@ public final class OSGiActionProviderService extends AbstractAdaptedService<Acti
     }
 
     @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<InstanceIdentifier<O>> validNodes) {
-        return delegate().registerImplementation(actionInterface, implementation, datastore, validNodes);
+    public <P extends DataObject, A extends Action<InstanceIdentifier<P>, ?, ?>, S extends A>
+        ObjectRegistration<S> registerImplementation(final ActionSpec<A, P> spec, final S implementation,
+            final LogicalDatastoreType datastore, final Set<InstanceIdentifier<P>> validNodes) {
+        return delegate().registerImplementation(spec, implementation, datastore, validNodes);
     }
 
     @Activate
index 5eb5740dc6ec91981d2c401c27f775794d70a0aa..0b9999f286dfa53185456c08413a69f171ce7270 100644 (file)
@@ -11,15 +11,19 @@ import com.google.common.annotations.Beta;
 import java.util.Map;
 import java.util.Set;
 import org.eclipse.jdt.annotation.NonNull;
+import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.opendaylight.mdsal.binding.api.ActionService;
+import org.opendaylight.mdsal.binding.api.ActionSpec;
 import org.opendaylight.mdsal.binding.api.DataTreeIdentifier;
 import org.opendaylight.yangtools.yang.binding.Action;
 import org.opendaylight.yangtools.yang.binding.DataObject;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Deactivate;
 
 @Beta
+@NonNullByDefault
 @Component(factory = OSGiActionService.FACTORY_NAME)
 public final class OSGiActionService extends AbstractAdaptedService<ActionService> implements ActionService {
     // OSGi DS Component Factory name
@@ -30,9 +34,9 @@ public final class OSGiActionService extends AbstractAdaptedService<ActionServic
     }
 
     @Override
-    public <O extends @NonNull DataObject, T extends @NonNull Action<?, ?, ?>> T getActionHandle(
-            final Class<T> actionInterface, final Set<@NonNull DataTreeIdentifier<O>> validNodes) {
-        return delegate().getActionHandle(actionInterface, validNodes);
+    public <P extends DataObject, A extends Action<InstanceIdentifier<P>, ?, ?>> A getActionHandle(
+            final ActionSpec<A, P> spec, final Set<@NonNull DataTreeIdentifier<P>> validNodes) {
+        return delegate().getActionHandle(spec, validNodes);
     }
 
     @Activate
index fd9c54c9fad42bbc9659c132829ed83d57afcde0..223995f25661b84691ccc22fa8ba753f91e67e7e 100644 (file)
@@ -10,13 +10,16 @@ package org.opendaylight.mdsal.binding.dom.adapter;
 import static org.junit.Assert.assertEquals;
 
 import org.junit.Test;
+import org.opendaylight.mdsal.binding.api.ActionSpec;
 import org.opendaylight.mdsal.binding.dom.codec.impl.BindingCodecContext;
 import org.opendaylight.mdsal.binding.runtime.spi.BindingRuntimeHelpers;
 import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.Cont;
 import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.Grpcont;
+import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.Nestedcont;
 import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.Othercont;
 import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.cont.Foo;
 import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.grpcont.Bar;
+import org.opendaylight.yang.gen.v1.urn.odl.actions.norev.nested.Baz;
 import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Absolute;
 
 public class ActionLookupTest {
@@ -25,9 +28,15 @@ public class ActionLookupTest {
         CurrentAdapterSerializer codec = new CurrentAdapterSerializer(new BindingCodecContext(
             BindingRuntimeHelpers.createRuntimeContext()));
 
-        assertEquals(Absolute.of(Cont.QNAME, Foo.QNAME), codec.getActionPath(Foo.class));
-        assertEquals(Absolute.of(Grpcont.QNAME, Bar.QNAME), codec.getActionPath(Bar.class));
-        assertEquals(Absolute.of(Othercont.QNAME, Bar.QNAME),
-            codec.getActionPath(org.opendaylight.yang.gen.v1.urn.odl.actions.norev.othercont.Bar.class));
+        assertEquals(Absolute.of(Cont.QNAME, Foo.QNAME), codec.getActionPath(
+            ActionSpec.builder(Cont.class).build(Foo.class)));
+        assertEquals(Absolute.of(Grpcont.QNAME, Bar.QNAME), codec.getActionPath(
+            ActionSpec.builder(Grpcont.class).build(Bar.class)));
+        assertEquals(Absolute.of(Othercont.QNAME, Bar.QNAME), codec.getActionPath(
+            ActionSpec.builder(Othercont.class).build(
+                org.opendaylight.yang.gen.v1.urn.odl.actions.norev.othercont.Bar.class)));
+        assertEquals(Absolute.of(Nestedcont.QNAME, Baz.QNAME, Bar.QNAME), codec.getActionPath(
+            ActionSpec.builder(Nestedcont.class).withPathChild(Baz.class).build(
+                org.opendaylight.yang.gen.v1.urn.odl.actions.norev.nested.baz.Bar.class)));
     }
 }
index abacf151394d4de241340ee415f13e2a3de3b3f5..0abf3f6f7e2bfc8db26416bbdd5cc7046dab4ea2 100644 (file)
@@ -19,6 +19,7 @@ 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.binding.api.ActionSpec;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.mdsal.dom.api.DOMActionInstance;
 import org.opendaylight.mdsal.dom.api.DOMActionProviderService;
@@ -47,8 +48,8 @@ public class ActionProviderServiceAdapterTest extends AbstractActionAdapterTest
 
     @Test
     public void testInstanceRegistration() {
-        adapter.registerImplementation(Foo.class, FOO, LogicalDatastoreType.OPERATIONAL,
-            Set.of(InstanceIdentifier.create(Cont.class)));
+        adapter.registerImplementation(ActionSpec.builder(Cont.class).build(Foo.class), FOO,
+            LogicalDatastoreType.OPERATIONAL, Set.of(InstanceIdentifier.create(Cont.class)));
 
         verify(actionProvider).registerActionImplementation(any(), eq(DOMActionInstance.of(FOO_PATH,
             LogicalDatastoreType.OPERATIONAL, YangInstanceIdentifier.create(new NodeIdentifier(Cont.QNAME)))));
@@ -56,7 +57,7 @@ public class ActionProviderServiceAdapterTest extends AbstractActionAdapterTest
 
     @Test
     public void testWildcardRegistration() {
-        adapter.registerImplementation(Foo.class, FOO);
+        adapter.registerImplementation(ActionSpec.builder(Cont.class).build(Foo.class), FOO);
         verify(actionProvider).registerActionImplementation(any(), eq(DOMActionInstance.of(FOO_PATH,
             LogicalDatastoreType.OPERATIONAL, YangInstanceIdentifier.empty())));
     }
index 244ce9b9902caadeba1ad8bb324689da4c038d25..76e28aa80f5fe071c699f0a32de2160f2a90cea7 100644 (file)
@@ -17,7 +17,6 @@ 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;
@@ -25,6 +24,7 @@ import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 import org.opendaylight.mdsal.binding.api.ActionService;
+import org.opendaylight.mdsal.binding.api.ActionSpec;
 import org.opendaylight.mdsal.dom.api.DOMActionResult;
 import org.opendaylight.mdsal.dom.api.DOMActionService;
 import org.opendaylight.mdsal.dom.spi.SimpleDOMActionResult;
@@ -56,7 +56,7 @@ public class ActionServiceAdapterTest extends AbstractActionAdapterTest {
 
     @Test
     public void testInvocation() throws ExecutionException {
-        final Foo handle = service.getActionHandle(Foo.class, Set.of());
+        final Foo handle = service.getActionHandle(ActionSpec.builder(Cont.class).build(Foo.class));
         final ListenableFuture<RpcResult<Output>> future = handle.invoke(InstanceIdentifier.create(Cont.class),
             BINDING_FOO_INPUT);
         assertNotNull(future);
index 66830d30e5a175960248920d90ff7b46519f55dd..15892d17036931a8755f4538e7770e21d10e86c1 100644 (file)
@@ -99,6 +99,7 @@ public interface BindingRuntimeContext extends EffectiveModelContextProvider, Im
 
     @Nullable ActionDefinition getActionDefinition(Class<? extends Action<?, ?, ?>> cls);
 
+    @Deprecated(forRemoval = true)
     @Nullable Absolute getActionIdentifier(Class<? extends Action<?, ?, ?>> cls);
 
     @NonNull Entry<AugmentationIdentifier, AugmentationSchemaNode> getResolvedAugmentationSchema(
index 299519677e63a25983ed3b6bbbdefa3d2182bcf2..3dd56c527592836500702aabb8656bd7a64e8740 100644 (file)
@@ -56,6 +56,12 @@ module actions {
         uses grp;
     }
 
+    grouping nested {
+        container baz {
+            uses other;
+        }
+    }
+
     container grpcont {
         uses grp;
     }
@@ -71,5 +77,9 @@ module actions {
     container othercont {
         uses other;
     }
+
+    container nestedcont {
+        uses nested;
+    }
 }