ONOS-3321 Added ability for applications to register a deactivation hook.

Change-Id: I284321474dbf9e39e8b61f7b907f637ba6b80aaf

@@ -67,4 +67,11 @@ public interface ApplicationService
      */
     Set<Permission> getPermissions(ApplicationId appId);
 
+    /**
+     * Registers application pre-deactivation processing hook.
+     *
+     * @param appId application identifier
+     * @param hook  pre-deactivation hook
+     */
+    void registerDeactivateHook(ApplicationId appId, Runnable hook);
 }

@@ -50,6 +50,7 @@ public interface CoreService {
 
     /**
      * Returns an existing application id from a given id.
+     *
      * @param id the short value of the id
      * @return an application id
      */
@@ -57,6 +58,7 @@ public interface CoreService {
 
     /**
      * Returns an existing application id from a given id.
+     *
      * @param name the name portion of the ID to look up
      * @return an application id
      */
@@ -67,10 +69,21 @@ public interface CoreService {
      * to follow the reverse DNS convention, e.g.
      * {@code org.flying.circus.app}
      *
-     * @param identifier string identifier
+     * @param name string identifier
+     * @return the application id
+     */
+    ApplicationId registerApplication(String name);
+
+    /**
+     * Registers a new application by its name, which is expected
+     * to follow the reverse DNS convention, e.g.
+     * {@code org.flying.circus.app}, along with its pre-deactivation hook.
+     *
+     * @param name          string identifier
+     * @param preDeactivate pre-deactivation hook
      * @return the application id
      */
-    ApplicationId registerApplication(String identifier);
+    ApplicationId registerApplication(String name, Runnable preDeactivate);
 
     /**
      * Returns an id generator for a given topic.

@@ -51,6 +51,10 @@ public class ApplicationServiceAdapter implements ApplicationService {
     }
 
     @Override
+    public void registerDeactivateHook(ApplicationId appId, Runnable hook) {
+    }
+
+    @Override
     public void addListener(ApplicationListener listener) {
     }
 

@@ -43,7 +43,12 @@ public class CoreServiceAdapter implements CoreService {
     }
 
     @Override
-    public ApplicationId registerApplication(String identifier) {
+    public ApplicationId registerApplication(String name) {
+        return null;
+    }
+
+    @Override
+    public ApplicationId registerApplication(String name, Runnable preDeactivate) {
         return null;
     }
 

@@ -15,6 +15,7 @@
  */
 package org.onosproject.app.impl;
 
+import com.google.common.collect.Maps;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
@@ -30,20 +31,21 @@ import org.onosproject.app.ApplicationService;
 import org.onosproject.app.ApplicationState;
 import org.onosproject.app.ApplicationStore;
 import org.onosproject.app.ApplicationStoreDelegate;
-import org.onosproject.event.AbstractListenerManager;
 import org.onosproject.core.Application;
 import org.onosproject.core.ApplicationId;
+import org.onosproject.event.AbstractListenerManager;
 import org.onosproject.security.Permission;
 import org.onosproject.security.SecurityUtil;
 import org.slf4j.Logger;
 
 import java.io.InputStream;
+import java.util.Map;
 import java.util.Set;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.onosproject.app.ApplicationEvent.Type.*;
-import static org.onosproject.security.AppPermission.Type.*;
 import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.APP_READ;
 import static org.slf4j.LoggerFactory.getLogger;
 
 /**
@@ -69,6 +71,9 @@ public class ApplicationManager
 
     private boolean initializing;
 
+    // Application supplied hooks for pre-activation processing.
+    private final Map<String, Runnable> deactivateHooks = Maps.newConcurrentMap();
+
     @Activate
     public void activate() {
         eventDispatcher.addSink(ApplicationEvent.class, listenerRegistry);
@@ -122,6 +127,14 @@ public class ApplicationManager
     }
 
     @Override
+    public void registerDeactivateHook(ApplicationId appId, Runnable hook) {
+        checkPermission(APP_READ);
+        checkNotNull(appId, APP_ID_NULL);
+        checkNotNull(hook, "Hook cannot be null");
+        deactivateHooks.put(appId.name(), hook);
+    }
+
+    @Override
     public Application install(InputStream appDescStream) {
         checkNotNull(appDescStream, "Application archive stream cannot be null");
         Application app = store.create(appDescStream);
@@ -235,6 +248,7 @@ public class ApplicationManager
 
     private synchronized boolean uninstallAppFeatures(Application app) throws Exception {
         boolean changed = false;
+        invokeHook(deactivateHooks.get(app.id().name()), app.id());
         for (String name : app.features()) {
             Feature feature = featuresService.getFeature(name);
             if (feature != null && featuresService.isInstalled(feature)) {
@@ -247,4 +261,16 @@ public class ApplicationManager
         return changed;
     }
 
+    // Invokes the specified function, if not null.
+    private void invokeHook(Runnable hook, ApplicationId appId) {
+        if (hook != null) {
+            try {
+                hook.run();
+            } catch (Exception e) {
+                log.warn("Deactivate hook for application {} encountered an error",
+                         appId.name(), e);
+            }
+        }
+    }
+
 }

@@ -24,6 +24,7 @@ import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.Service;
 import org.onlab.util.SharedExecutors;
+import org.onosproject.app.ApplicationService;
 import org.onosproject.cfg.ComponentConfigService;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.ApplicationIdStore;
@@ -48,7 +49,7 @@ import java.util.Set;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static org.onosproject.security.AppGuard.checkPermission;
-import static org.onosproject.security.AppPermission.Type.*;
+import static org.onosproject.security.AppPermission.Type.APP_READ;
 
 
 
@@ -71,6 +72,9 @@ public class CoreManager implements CoreService {
     protected IdBlockStore idBlockStore;
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+    protected ApplicationService appService;
+
+    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected ComponentConfigService cfgService;
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
@@ -111,28 +115,24 @@ public class CoreManager implements CoreService {
     @Override
     public Version version() {
         checkPermission(APP_READ);
-
         return version;
     }
 
     @Override
     public Set<ApplicationId> getAppIds() {
         checkPermission(APP_READ);
-
         return applicationIdStore.getAppIds();
     }
 
     @Override
     public ApplicationId getAppId(Short id) {
         checkPermission(APP_READ);
-
         return applicationIdStore.getAppId(id);
     }
 
     @Override
     public ApplicationId getAppId(String name) {
         checkPermission(APP_READ);
-
         return applicationIdStore.getAppId(name);
     }
 
@@ -144,6 +144,13 @@ public class CoreManager implements CoreService {
     }
 
     @Override
+    public ApplicationId registerApplication(String name, Runnable preDeactivate) {
+        ApplicationId id = registerApplication(name);
+        appService.registerDeactivateHook(id, preDeactivate);
+        return id;
+    }
+
+    @Override
     public IdGenerator getIdGenerator(String topic) {
         IdBlockAllocator allocator = new StoreBasedIdBlockAllocator(topic, idBlockStore);
         return new BlockAllocatorBasedIdGenerator(allocator);
@@ -185,10 +192,10 @@ public class CoreManager implements CoreService {
      */
     private static Integer getIntegerProperty(Dictionary<?, ?> properties,
                                               String propertyName) {
-        Integer value = null;
+        Integer value;
         try {
             String s = (String) properties.get(propertyName);
-            value = isNullOrEmpty(s) ? value : Integer.parseInt(s.trim());
+            value = isNullOrEmpty(s) ? null : Integer.parseInt(s.trim());
         } catch (NumberFormatException | ClassCastException e) {
             value = null;
         }

@@ -24,11 +24,11 @@ import org.onosproject.app.ApplicationListener;
 import org.onosproject.app.ApplicationState;
 import org.onosproject.app.ApplicationStoreAdapter;
 import org.onosproject.common.app.ApplicationArchive;
+import org.onosproject.common.event.impl.TestEventDispatcher;
 import org.onosproject.core.Application;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.DefaultApplication;
 import org.onosproject.core.DefaultApplicationId;
-import org.onosproject.common.event.impl.TestEventDispatcher;
 
 import java.io.InputStream;
 import java.net.URI;
@@ -36,7 +36,7 @@ import java.util.HashSet;
 import java.util.Optional;
 import java.util.Set;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
 import static org.onosproject.app.ApplicationEvent.Type.*;
 import static org.onosproject.app.ApplicationState.ACTIVE;
 import static org.onosproject.app.ApplicationState.INSTALLED;
@@ -53,6 +53,8 @@ public class ApplicationManagerTest {
     private ApplicationManager mgr = new ApplicationManager();
     private ApplicationListener listener = new TestListener();
 
+    private boolean deactivated = false;
+
     @Before
     public void setUp() {
         injectEventDispatcher(mgr, new TestEventDispatcher());
@@ -88,6 +90,11 @@ public class ApplicationManagerTest {
         assertEquals("incorrect app count", 1, mgr.getApplications().size());
         assertEquals("incorrect app", app, mgr.getApplication(APP_ID));
         assertEquals("incorrect app state", INSTALLED, mgr.getState(APP_ID));
+        mgr.registerDeactivateHook(app.id(), this::deactivateHook);
+    }
+
+    private void deactivateHook() {
+        deactivated = true;
     }
 
     @Test
@@ -102,6 +109,7 @@ public class ApplicationManagerTest {
         install();
         mgr.activate(APP_ID);
         assertEquals("incorrect app state", ACTIVE, mgr.getState(APP_ID));
+        assertFalse("preDeactivate hook wrongly called", deactivated);
     }
 
     @Test
@@ -109,6 +117,7 @@ public class ApplicationManagerTest {
         activate();
         mgr.deactivate(APP_ID);
         assertEquals("incorrect app state", INSTALLED, mgr.getState(APP_ID));
+        assertTrue("preDeactivate hook not called", deactivated);
     }
 
 

@@ -23,10 +23,8 @@ import org.onlab.junit.TestUtils;
 import org.onlab.packet.IpPrefix;
 import org.onosproject.common.event.impl.TestEventDispatcher;
 import org.onosproject.core.ApplicationId;
-import org.onosproject.core.CoreService;
+import org.onosproject.core.CoreServiceAdapter;
 import org.onosproject.core.DefaultApplicationId;
-import org.onosproject.core.IdGenerator;
-import org.onosproject.core.Version;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.mcast.McastEvent;
@@ -35,7 +33,6 @@ import org.onosproject.net.mcast.McastRoute;
 import org.onosproject.store.service.TestStorageService;
 
 import java.util.List;
-import java.util.Set;
 
 import static junit.framework.Assert.fail;
 import static junit.framework.TestCase.assertEquals;
@@ -152,49 +149,23 @@ public class MulticastRouteManagerTest {
 
         for (int i = 0; i < evs.length; i++) {
             if (evs[i] != events.get(i).type()) {
-                fail(String.format("Mismtached events# obtained -> %s : expected %s",
+                fail(String.format("Mismatched events# obtained -> %s : expected %s",
                                    events, evs));
             }
         }
     }
 
     class TestMulticastListener implements McastListener {
-
         @Override
         public void event(McastEvent event) {
             events.add(event);
         }
     }
 
-    private class TestCoreService implements CoreService {
-        @Override
-        public Version version() {
-            return null;
-        }
-
-        @Override
-        public Set<ApplicationId> getAppIds() {
-            return null;
-        }
-
-        @Override
-        public ApplicationId getAppId(Short id) {
-            return null;
-        }
-
-        @Override
-        public ApplicationId getAppId(String name) {
-            return null;
-        }
-
-        @Override
-        public ApplicationId registerApplication(String identifier) {
-            return new DefaultApplicationId(0, identifier);
-        }
-
+    private class TestCoreService extends CoreServiceAdapter {
         @Override
-        public IdGenerator getIdGenerator(String topic) {
-            return null;
+        public ApplicationId registerApplication(String name) {
+            return new DefaultApplicationId(0, name);
         }
     }
 }