changeset 1558:ffc701452e4b

Split responsibilities in (internal) AbstractCommand Reviewed-by: jerboaa, vanaltj Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2014-November/011636.html
author Omair Majid <omajid@redhat.com>
date Wed, 19 Nov 2014 12:51:27 -0500
parents a95d1e2e3e69
children 5c6937a60c17
files vm-profiler/client-cli/src/main/java/com/redhat/thermostat/vm/profiler/client/cli/internal/AbstractCommand.java vm-profiler/client-cli/src/main/java/com/redhat/thermostat/vm/profiler/client/cli/internal/Activator.java vm-profiler/client-cli/src/main/java/com/redhat/thermostat/vm/profiler/client/cli/internal/ProfileVmCommand.java vm-profiler/client-cli/src/test/java/com/redhat/thermostat/vm/profiler/client/cli/internal/AbstractCommandTest.java
diffstat 4 files changed, 117 insertions(+), 20 deletions(-) [+]
line wrap: on
line diff
--- a/vm-profiler/client-cli/src/main/java/com/redhat/thermostat/vm/profiler/client/cli/internal/AbstractCommand.java	Tue Nov 18 16:07:35 2014 -0500
+++ b/vm-profiler/client-cli/src/main/java/com/redhat/thermostat/vm/profiler/client/cli/internal/AbstractCommand.java	Wed Nov 19 12:51:27 2014 -0500
@@ -38,6 +38,7 @@
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
@@ -46,7 +47,7 @@
 
     // TODO these changes should probably be promoted to the AbstractCommand public API
 
-    protected Map<Class<?>, BlockingQueue<?>> serviceHolder = new HashMap<>();
+    private Map<Class<?>, BlockingQueue<?>> serviceHolder = new HashMap<>();
 
     private <T> BlockingQueue<T> getHolder(Class<T> serviceClass) {
         if (!serviceHolder.containsKey(serviceClass)) {
@@ -55,22 +56,24 @@
         return (BlockingQueue<T>) serviceHolder.get(serviceClass);
     }
 
-    /** Insert {@code item} if it's non-{@code null} otherwise remove it */
-    protected <T> void addOrRemoveService(Class<T> serviceClass, T item) {
+    protected <T> void addService(Class<T> serviceClass, T item) {
+        Objects.requireNonNull(item);
         BlockingQueue<T> holder = getHolder(serviceClass);
-        if (item == null) {
-            if (holder.peek() != null) {
-                holder.remove();
-            }
-        } else {
-            try {
-                holder.put(item);
-            } catch (InterruptedException e) {
-                throw new AssertionError("Should not happen");
-            }
+        try {
+            holder.put(item);
+        } catch (InterruptedException e) {
+            throw new AssertionError("Should not happen");
         }
     }
 
+    protected <T> void removeService(Class<T> serviceClass) {
+        BlockingQueue<T> holder = getHolder(serviceClass);
+        if (holder.peek() != null) {
+            holder.remove();
+        }
+    }
+
+    /** @return the service, or <code>null</code> */
     protected <T> T getService(Class<T> serviceClass) {
         BlockingQueue<T> holder = getHolder(serviceClass);
         try {
--- a/vm-profiler/client-cli/src/main/java/com/redhat/thermostat/vm/profiler/client/cli/internal/Activator.java	Tue Nov 18 16:07:35 2014 -0500
+++ b/vm-profiler/client-cli/src/main/java/com/redhat/thermostat/vm/profiler/client/cli/internal/Activator.java	Wed Nov 19 12:51:27 2014 -0500
@@ -89,9 +89,10 @@
 
             @Override
             public void dependenciesUnavailable() {
-                command.setAgentInfoDAO(null);
-                command.setVmInfoDAO(null);
-                command.setRequestQueue(null);
+                command.unsetAgentInfoDAO();
+                command.unsetVmInfoDAO();
+                command.unsetRequestQueue();
+                command.unsetProfileDAO();
             }
         });
 
--- a/vm-profiler/client-cli/src/main/java/com/redhat/thermostat/vm/profiler/client/cli/internal/ProfileVmCommand.java	Tue Nov 18 16:07:35 2014 -0500
+++ b/vm-profiler/client-cli/src/main/java/com/redhat/thermostat/vm/profiler/client/cli/internal/ProfileVmCommand.java	Wed Nov 19 12:51:27 2014 -0500
@@ -185,18 +185,34 @@
     }
 
     void setAgentInfoDAO(AgentInfoDAO dao) {
-        addOrRemoveService(AgentInfoDAO.class, dao);
+        addService(AgentInfoDAO.class, dao);
+    }
+
+    void unsetAgentInfoDAO() {
+        removeService(AgentInfoDAO.class);
     }
 
     void setVmInfoDAO(VmInfoDAO dao) {
-        addOrRemoveService(VmInfoDAO.class, dao);
+        addService(VmInfoDAO.class, dao);
+    }
+
+    void unsetVmInfoDAO() {
+        removeService(VmInfoDAO.class);
     }
 
     void setRequestQueue(RequestQueue queue) {
-        addOrRemoveService(RequestQueue.class, queue);
+        addService(RequestQueue.class, queue);
+    }
+
+    void unsetRequestQueue() {
+        removeService(RequestQueue.class);
     }
 
     void setProfileDAO(ProfileDAO dao) {
-        addOrRemoveService(ProfileDAO.class, dao);
+        addService(ProfileDAO.class, dao);
+    }
+
+    void unsetProfileDAO() {
+        removeService(ProfileDAO.class);
     }
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/vm-profiler/client-cli/src/test/java/com/redhat/thermostat/vm/profiler/client/cli/internal/AbstractCommandTest.java	Wed Nov 19 12:51:27 2014 -0500
@@ -0,0 +1,77 @@
+/*
+ * Copyright 2012-2014 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Linking this code with other modules is making a combined work
+ * based on this code.  Thus, the terms and conditions of the GNU
+ * General Public License cover the whole combination.
+ *
+ * As a special exception, the copyright holders of this code give
+ * you permission to link this code with independent modules to
+ * produce an executable, regardless of the license terms of these
+ * independent modules, and to copy and distribute the resulting
+ * executable under terms of your choice, provided that you also
+ * meet, for each linked independent module, the terms and conditions
+ * of the license of that module.  An independent module is a module
+ * which is not derived from or based on this code.  If you modify
+ * this code, you may extend this exception to your version of the
+ * library, but you are not obligated to do so.  If you do not wish
+ * to do so, delete this exception statement from your version.
+ */
+
+package com.redhat.thermostat.vm.profiler.client.cli.internal;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import org.junit.Test;
+
+import com.redhat.thermostat.common.cli.CommandContext;
+import com.redhat.thermostat.common.cli.CommandException;
+
+public class AbstractCommandTest {
+
+    static class TestCommand extends AbstractCommand {
+        @Override
+        public void run(CommandContext ctx) throws CommandException { /* do nothing */ }
+    }
+
+    @Test
+    public void uninjectedServiceIsNotAvailable() throws Exception {
+        AbstractCommand command = new TestCommand();
+
+        assertNull(command.getService(Object.class));
+    }
+
+    @Test
+    public void injectedServiceIsAvailable() throws Exception {
+        AbstractCommand command = new TestCommand();
+        command.addService(Object.class, command);
+
+        assertEquals(command, command.getService(Object.class));
+    }
+
+    @Test
+    public void injectedAndRemovedServiceIsNotAvailable() {
+        AbstractCommand command = new TestCommand();
+        command.addService(Object.class, command);
+
+        command.removeService(Object.class);
+        assertNull(command.getService(Object.class));
+    }
+}