Merge "Bug 279, 1390: Used Guava Cache r for lazy-loading of RPC Routers"
authorTom Pantelis <tpanteli@brocade.com>
Wed, 23 Jul 2014 14:30:44 +0000 (14:30 +0000)
committerGerrit Code Review <gerrit@opendaylight.org>
Wed, 23 Jul 2014 14:30:44 +0000 (14:30 +0000)
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/RpcIsNotRoutedException.java [new file with mode: 0644]
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/RuntimeCodeGenerator.java
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/impl/AbstractRuntimeCodeGenerator.java
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/impl/RpcProviderRegistryImpl.java
opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/impl/test/RpcProviderRegistryTest.java

diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/RpcIsNotRoutedException.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/sal/binding/codegen/RpcIsNotRoutedException.java
new file mode 100644 (file)
index 0000000..5317324
--- /dev/null
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2013 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.controller.sal.binding.codegen;
+
+import com.google.common.base.Preconditions;
+
+/**
+ * Exception is raised when supplied Bidning Aware
+ * RPCService class is not routed and was used in context
+ * where routed RPCs should only be used.
+ *
+ */
+public class RpcIsNotRoutedException extends IllegalStateException {
+
+    private static final long serialVersionUID = 1L;
+
+    public RpcIsNotRoutedException(final String message, final Throwable cause) {
+        super(Preconditions.checkNotNull(message), cause);
+    }
+
+    public RpcIsNotRoutedException(final String message) {
+        super(Preconditions.checkNotNull(message));
+    }
+}
index 9b7b8e28c90657aedc2f53c8c17d1697a0cd6e62..c7c5f10f71bd1073a9e6040ce62b23bbd39c0bc3 100644 (file)
@@ -84,8 +84,9 @@ public interface RuntimeCodeGenerator {
      *            - Subclass of RpcService for which Router is to be generated.
      * @return Instance of RpcService of provided serviceType which implements
      *         also {@link RpcRouter}<T> and {@link org.opendaylight.controller.sal.binding.spi.DelegateProxy}
+     * @throws RpcIsNotRoutedException
      */
-    <T extends RpcService> RpcRouter<T> getRouterFor(Class<T> serviceType,String name) throws IllegalArgumentException;
+    <T extends RpcService> RpcRouter<T> getRouterFor(Class<T> serviceType,String name) throws IllegalArgumentException, RpcIsNotRoutedException;
 
     NotificationInvokerFactory getInvokerFactory();
 }
index 9605a4d3723b6f21ccf8ce5810ad078ba3e3bbd6..8fa88ead0797a69df10c65f63078bac05476b479 100644 (file)
@@ -19,8 +19,10 @@ import javax.annotation.concurrent.GuardedBy;
 
 import org.eclipse.xtext.xbase.lib.Extension;
 import org.opendaylight.controller.sal.binding.api.rpc.RpcRouter;
+import org.opendaylight.controller.sal.binding.codegen.RpcIsNotRoutedException;
 import org.opendaylight.controller.sal.binding.spi.NotificationInvokerFactory;
 import org.opendaylight.yangtools.sal.binding.generator.util.JavassistUtils;
+import org.opendaylight.yangtools.yang.binding.BindingMapping;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.binding.NotificationListener;
 import org.opendaylight.yangtools.yang.binding.RpcService;
@@ -56,11 +58,11 @@ abstract class AbstractRuntimeCodeGenerator implements org.opendaylight.controll
     protected abstract <T extends RpcService> Supplier<T> directProxySupplier(final Class<T> serviceType);
     protected abstract <T extends RpcService> Supplier<T> routerSupplier(final Class<T> serviceType, RpcServiceMetadata metadata);
 
-    private RpcServiceMetadata getRpcMetadata(final CtClass iface) throws ClassNotFoundException, NotFoundException {
+    private RpcServiceMetadata getRpcMetadata(final CtClass iface) throws ClassNotFoundException, NotFoundException, RpcIsNotRoutedException {
         final RpcServiceMetadata metadata = new RpcServiceMetadata();
 
         for (CtMethod method : iface.getMethods()) {
-            if (iface.equals(method.getDeclaringClass()) && method.getParameterTypes().length == 1) {
+            if (isRpcMethodWithInput(iface, method)) {
                 final RpcMetadata routingPair = getRpcMetadata(method);
                 if (routingPair != null) {
                     metadata.addContext(routingPair.getContext());
@@ -81,6 +83,8 @@ abstract class AbstractRuntimeCodeGenerator implements org.opendaylight.controll
                      *        remains to be investigated.
                      */
                     Thread.currentThread().getContextClassLoader().loadClass(routingPair.getInputType().getName());
+                } else {
+                    throw new RpcIsNotRoutedException("RPC " + method.getName() + " from "+ iface.getName() +" is not routed");
                 }
             }
         }
@@ -88,6 +92,18 @@ abstract class AbstractRuntimeCodeGenerator implements org.opendaylight.controll
         return metadata;
     }
 
+
+    private boolean isRpcMethodWithInput(final CtClass iface, final CtMethod method) throws NotFoundException {
+        if(iface.equals(method.getDeclaringClass())
+                && method.getParameterTypes().length == 1) {
+            final CtClass onlyArg = method.getParameterTypes()[0];
+            if(onlyArg.isInterface() && onlyArg.getName().endsWith(BindingMapping.RPC_INPUT_SUFFIX)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     private RpcMetadata getRpcMetadata(final CtMethod method) throws NotFoundException {
         final CtClass inputClass = method.getParameterTypes()[0];
         return rpcMethodMetadata(inputClass, inputClass, method.getName());
@@ -152,7 +168,7 @@ abstract class AbstractRuntimeCodeGenerator implements org.opendaylight.controll
     }
 
     @Override
-    public final <T extends RpcService> RpcRouter<T> getRouterFor(final Class<T> serviceType, final String name) {
+    public final <T extends RpcService> RpcRouter<T> getRouterFor(final Class<T> serviceType, final String name) throws RpcIsNotRoutedException {
         final RpcServiceMetadata metadata = ClassLoaderUtils.withClassLoader(serviceType.getClassLoader(), new Supplier<RpcServiceMetadata>() {
             @Override
             public RpcServiceMetadata get() {
index c61ec4926a65f58f31fd03657162b0af70b23c0e..f2e467038f003cbc3acaa9dad3f4d108b0df691c 100644 (file)
@@ -9,16 +9,14 @@ package org.opendaylight.controller.sal.binding.impl;
 
 import static com.google.common.base.Preconditions.checkState;
 
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
-
 import java.util.EventListener;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
-import java.util.WeakHashMap;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.opendaylight.controller.md.sal.common.api.routing.RouteChange;
 import org.opendaylight.controller.md.sal.common.api.routing.RouteChangeListener;
@@ -29,6 +27,7 @@ import org.opendaylight.controller.sal.binding.api.BindingAwareBroker.RpcRegistr
 import org.opendaylight.controller.sal.binding.api.RpcProviderRegistry;
 import org.opendaylight.controller.sal.binding.api.rpc.RpcContextIdentifier;
 import org.opendaylight.controller.sal.binding.api.rpc.RpcRouter;
+import org.opendaylight.controller.sal.binding.codegen.RpcIsNotRoutedException;
 import org.opendaylight.controller.sal.binding.codegen.RuntimeCodeGenerator;
 import org.opendaylight.controller.sal.binding.codegen.RuntimeCodeHelper;
 import org.opendaylight.controller.sal.binding.codegen.impl.SingletonHolder;
@@ -41,6 +40,13 @@ import org.opendaylight.yangtools.yang.binding.RpcService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Throwables;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.util.concurrent.UncheckedExecutionException;
+
 public class RpcProviderRegistryImpl implements RpcProviderRegistry, RouteChangePublisher<RpcContextIdentifier, InstanceIdentifier<?>> {
 
     private RuntimeCodeGenerator rpcFactory = SingletonHolder.RPC_GENERATOR_IMPL;
@@ -56,7 +62,9 @@ public class RpcProviderRegistryImpl implements RpcProviderRegistry, RouteChange
                 }
             });
 
-    private final Map<Class<? extends RpcService>, RpcRouter<?>> rpcRouters = new WeakHashMap<>();
+    private final Cache<Class<? extends RpcService>, RpcRouter<?>> rpcRouters = CacheBuilder.newBuilder().weakKeys()
+            .build();
+
     private final ListenerRegistry<RouteChangeListener<RpcContextIdentifier, InstanceIdentifier<?>>> routeChangeListeners = ListenerRegistry
             .create();
     private final ListenerRegistry<RouterInstantiationListener> routerInstantiationListener = ListenerRegistry.create();
@@ -85,12 +93,20 @@ public class RpcProviderRegistryImpl implements RpcProviderRegistry, RouteChange
     @Override
     public final <T extends RpcService> RpcRegistration<T> addRpcImplementation(final Class<T> type, final T implementation)
             throws IllegalStateException {
-        @SuppressWarnings("unchecked")
-        RpcRouter<T> potentialRouter = (RpcRouter<T>) rpcRouters.get(type);
-        if (potentialRouter != null) {
+
+        // FIXME: This should be well documented - addRpcImplementation for
+        // routed RPCs
+        try {
+            // Note: If RPC is really global, expected count of registrations
+            // of this method is really low.
+            RpcRouter<T> potentialRouter = getRpcRouter(type);
             checkState(potentialRouter.getDefaultService() == null,
-                    "Default service for routed RPC already registered.");
+                        "Default service for routed RPC already registered.");
             return potentialRouter.registerDefaultService(implementation);
+        } catch (RpcIsNotRoutedException e) {
+            // NOOP - we could safely continue, since RPC is not routed
+            // so we fallback to global routing.
+            LOG.debug("RPC is not routed. Using global registration.",e);
         }
         T publicProxy = getRpcService(type);
         RpcService currentDelegate = RuntimeCodeHelper.getDelegate(publicProxy);
@@ -107,28 +123,37 @@ public class RpcProviderRegistryImpl implements RpcProviderRegistry, RouteChange
         return (T) publicProxies.getUnchecked(type);
     }
 
-    @SuppressWarnings({ "unchecked", "rawtypes" })
+
     public <T extends RpcService> RpcRouter<T> getRpcRouter(final Class<T> type) {
-        RpcRouter<?> potentialRouter = rpcRouters.get(type);
-        if (potentialRouter != null) {
-            return (RpcRouter<T>) potentialRouter;
-        }
-        synchronized (this) {
-            /**
-             * Potential Router could be instantiated by other thread while we
-             * were waiting for the lock.
-             */
-            potentialRouter = rpcRouters.get(type);
-            if (potentialRouter != null) {
-                return (RpcRouter<T>) potentialRouter;
+        try {
+            final AtomicBoolean created = new AtomicBoolean(false);
+            @SuppressWarnings( "unchecked")
+            // LoadingCache is unsuitable for RpcRouter since we need to distinguish
+            // first creation of RPC Router, so that is why
+            // we are using normal cache with load API and shared AtomicBoolean
+            // for this call, which will be set to true if router was created.
+            RpcRouter<T> router = (RpcRouter<T>) rpcRouters.get(type,new Callable<RpcRouter<?>>() {
+
+                @Override
+                public org.opendaylight.controller.sal.binding.api.rpc.RpcRouter<?> call()  {
+                    RpcRouter<?> router = rpcFactory.getRouterFor(type, name);
+                    router.registerRouteChangeListener(new RouteChangeForwarder<T>(type));
+                    LOG.debug("Registering router {} as global implementation of {} in {}", router, type.getSimpleName(), this);
+                    RuntimeCodeHelper.setDelegate(getRpcService(type), router.getInvocationProxy());
+                    created.set(true);
+                    return router;
+                }
+            });
+            if(created.get()) {
+                notifyListenersRoutedCreated(router);
             }
-            RpcRouter<T> router = rpcFactory.getRouterFor(type, name);
-            router.registerRouteChangeListener(new RouteChangeForwarder(type));
-            LOG.debug("Registering router {} as global implementation of {} in {}", router, type.getSimpleName(), this);
-            RuntimeCodeHelper.setDelegate(getRpcService(type), router.getInvocationProxy());
-            rpcRouters.put(type, router);
-            notifyListenersRoutedCreated(router);
             return router;
+        } catch (ExecutionException | UncheckedExecutionException e) {
+            // We rethrow Runtime Exceptions which were wrapped by
+            // Execution Exceptions
+            // otherwise we throw IllegalStateException with original
+            Throwables.propagateIfPossible(e.getCause());
+            throw new IllegalStateException("Could not load RPC Router for "+type.getName(),e);
         }
     }
 
@@ -159,7 +184,7 @@ public class RpcProviderRegistryImpl implements RpcProviderRegistry, RouteChange
             final RouterInstantiationListener listener) {
         ListenerRegistration<RouterInstantiationListener> reg = routerInstantiationListener.register(listener);
         try {
-            for (RpcRouter<?> router : rpcRouters.values()) {
+            for (RpcRouter<?> router : rpcRouters.asMap().values()) {
                 listener.onRpcRouterCreated(router);
             }
         } catch (Exception e) {
@@ -225,7 +250,7 @@ public class RpcProviderRegistryImpl implements RpcProviderRegistry, RouteChange
                 try {
                     listener.getInstance().onRouteChange(toPublish);
                 } catch (Exception e) {
-                    e.printStackTrace();
+                    LOG.error("Unhandled exception during invoking listener",listener.getInstance(),e);
                 }
             }
         }
index 110e5b4dce3f02e7f32aad00d0c8915ea188bb47..8782046eeef3c13da933bc01ab8d68476b0a0664 100644 (file)
@@ -23,6 +23,7 @@ import org.opendaylight.controller.sal.binding.api.BindingAwareBroker.RoutedRpcR
 import org.opendaylight.controller.sal.binding.api.BindingAwareBroker.RpcRegistration;
 import org.opendaylight.controller.sal.binding.api.rpc.RpcContextIdentifier;
 import org.opendaylight.controller.sal.binding.api.rpc.RpcRouter;
+import org.opendaylight.controller.sal.binding.codegen.RpcIsNotRoutedException;
 import org.opendaylight.controller.sal.binding.impl.RpcProviderRegistryImpl;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.bi.ba.rpcservice.rev140701.OpendaylightTestRpcServiceService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.two.level.list.TopLevelList;
@@ -77,6 +78,28 @@ public class RpcProviderRegistryTest {
         assertNotNull(regTwo);
     }
 
+    @Test
+    public void routedRpcRegisteredUsingGlobalAsDefaultInstance() throws Exception {
+        OpendaylightTestRoutedRpcService def = Mockito.mock(OpendaylightTestRoutedRpcService.class);
+        rpcRegistry.addRpcImplementation(OpendaylightTestRoutedRpcService.class, def);
+        RpcRouter<OpendaylightTestRoutedRpcService> router = rpcRegistry.getRpcRouter(OpendaylightTestRoutedRpcService.class);
+        assertEquals(def, router.getDefaultService());
+    }
+
+    @Test
+    public void nonRoutedRegisteredAsRouted() {
+        OpendaylightTestRpcServiceService one = Mockito.mock(OpendaylightTestRpcServiceService.class);
+        try {
+            rpcRegistry.addRoutedRpcImplementation(OpendaylightTestRpcServiceService.class, one);
+            fail("RpcIsNotRoutedException should be thrown");
+        } catch (RpcIsNotRoutedException e) {
+            assertNotNull(e.getMessage());
+        } catch (Exception e) {
+            fail("RpcIsNotRoutedException should be thrown");
+        }
+
+    }
+
     @Test
     public void testRpcRouterInstance() throws Exception  {
         OpendaylightTestRoutedRpcService def = Mockito.mock(OpendaylightTestRoutedRpcService.class);