# HG changeset patch # User Omair Majid # Date 1366223678 14400 # Node ID f0f0027e072466d148555c1918b955bae28df982 # Parent c8d427902c6bda4230e33e3aba44969495e5d301 Hide CommandInfo and reduce Command interface Reviewed-by: vanaltj Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-April/006349.html diff -r c8d427902c6b -r f0f0027e0724 agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/Activator.java --- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/Activator.java Mon Apr 15 15:45:04 2013 +0200 +++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/Activator.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,8 +36,6 @@ package com.redhat.thermostat.agent.cli.impl; -import java.util.Arrays; - import org.osgi.framework.BundleActivator; import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceReference; @@ -58,15 +56,17 @@ @SuppressWarnings({ "rawtypes", "unchecked" }) @Override public void start(final BundleContext context) throws Exception { + reg = new CommandRegistryImpl(context); + exitStatusTracker = new ServiceTracker(context, ExitStatus.class, null) { @Override public Object addingService(ServiceReference reference) { ExitStatus exitStatus = (ExitStatus)context.getService(reference); - reg = new CommandRegistryImpl(context); agentApplication = new AgentApplication(context, exitStatus); - reg.registerCommands(Arrays.asList(new ServiceCommand(context), - new StorageCommand(exitStatus), agentApplication)); + reg.registerCommand("service", new ServiceCommand(context)); + reg.registerCommand("storage", new StorageCommand(exitStatus)); + reg.registerCommand("agent", agentApplication); return exitStatus; } diff -r c8d427902c6b -r f0f0027e0724 agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java --- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java Mon Apr 15 15:45:04 2013 +0200 +++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java Wed Apr 17 14:34:38 2013 -0400 @@ -76,8 +76,6 @@ @SuppressWarnings("restriction") public final class AgentApplication extends AbstractStateNotifyingCommand { - private static final String NAME = "agent"; - private final BundleContext bundleContext; private final ConfigurationCreator configurationCreator; @@ -189,11 +187,6 @@ throw new CommandException(ex); } } - - @Override - public String getName() { - return NAME; - } public void shutdown() { // Exit application diff -r c8d427902c6b -r f0f0027e0724 agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/ServiceCommand.java --- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/ServiceCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/ServiceCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -64,8 +64,6 @@ private static final Translate translator = LocaleResources.createLocalizer(); - private static final String NAME = "service"; - private List> listeners; private Semaphore agentBarrier = new Semaphore(0); private BundleContext context; @@ -122,11 +120,6 @@ } @Override - public String getName() { - return NAME; - } - - @Override public boolean isAvailableInShell() { return false; } diff -r c8d427902c6b -r f0f0027e0724 agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommand.java --- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -51,8 +51,6 @@ public class StorageCommand extends AbstractStateNotifyingCommand { - private static final String NAME = "storage"; - private DBStartupConfiguration configuration; private DBOptionParser parser; private final ExitStatus exitStatus; @@ -163,11 +161,6 @@ } @Override - public String getName() { - return NAME; - } - - @Override public boolean isStorageRequired() { return false; } diff -r c8d427902c6b -r f0f0027e0724 agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ActivatorTest.java --- a/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ActivatorTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ActivatorTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,17 +36,14 @@ package com.redhat.thermostat.agent.cli.impl; +import static com.redhat.thermostat.testutils.Asserts.assertCommandIsRegistered; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.when; import static org.mockito.Mockito.mock; import org.junit.Test; import com.redhat.thermostat.agent.cli.impl.db.StorageCommand; import com.redhat.thermostat.common.ExitStatus; -import com.redhat.thermostat.common.cli.Command; import com.redhat.thermostat.testutils.StubBundleContext; public class ActivatorTest { @@ -61,9 +58,11 @@ activator.start(bundleContext); - assertTrue(bundleContext.isServiceRegistered(Command.class.getName(), AgentApplication.class)); - assertTrue(bundleContext.isServiceRegistered(Command.class.getName(), ServiceCommand.class)); - assertTrue(bundleContext.isServiceRegistered(Command.class.getName(), StorageCommand.class)); + assertCommandIsRegistered(bundleContext, "agent", AgentApplication.class); + + assertCommandIsRegistered(bundleContext, "service", ServiceCommand.class); + + assertCommandIsRegistered(bundleContext, "storage", StorageCommand.class); activator.stop(bundleContext); diff -r c8d427902c6b -r f0f0027e0724 agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java --- a/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -37,7 +37,6 @@ package com.redhat.thermostat.agent.cli.impl; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; @@ -135,20 +134,6 @@ } @Test - public void testName() { - AgentApplication agent = new AgentApplication(context, exitStatus, configCreator, dbServiceFactory); - String name = agent.getName(); - assertEquals("agent", name); - } - - @Test - public void testDescAndUsage() { - AgentApplication agent = new AgentApplication(context, exitStatus, configCreator, dbServiceFactory); - assertNotNull(agent.getDescription()); - assertNotNull(agent.getUsage()); - } - - @Test public void testAgentStartup() throws CommandException, InterruptedException { final AgentApplication agent = new AgentApplication(context, exitStatus, configCreator, dbServiceFactory); final CountDownLatch latch = new CountDownLatch(1); diff -r c8d427902c6b -r f0f0027e0724 agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ServiceCommandTest.java --- a/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ServiceCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ServiceCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,13 +36,8 @@ package com.redhat.thermostat.agent.cli.impl; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; - -import org.apache.commons.cli.Options; import org.junit.After; import org.junit.Before; -import org.junit.Test; import com.redhat.thermostat.testutils.StubBundleContext; @@ -61,23 +56,6 @@ thermostatService = null; } - @Test - public void testName() { - String name = thermostatService.getName(); - assertEquals("service", name); - } - - @Test - public void testDescAndUsage() { - assertNotNull(thermostatService.getDescription()); - assertNotNull(thermostatService.getUsage()); - } - - @Test - public void testOptions() { - Options options = thermostatService.getOptions(); - assertNotNull(options); - assertEquals(0, options.getOptions().size()); - } + // TODO really need to add more tests for service } diff -r c8d427902c6b -r f0f0027e0724 agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommandTest.java --- a/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -37,7 +37,6 @@ package com.redhat.thermostat.agent.cli.impl.db; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -322,18 +321,5 @@ return ctx; } - @Test - public void testName() { - StorageCommand dbService = new StorageCommand(exitStatus); - String name = dbService.getName(); - assertEquals("storage", name); - } - - @Test - public void testDescAndUsage() { - StorageCommand dbService = new StorageCommand(exitStatus); - assertNotNull(dbService.getDescription()); - assertNotNull(dbService.getUsage()); - } } diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/Activator.java --- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/Activator.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/Activator.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,12 +36,9 @@ package com.redhat.thermostat.client.cli.internal; -import java.util.ServiceLoader; - import org.osgi.framework.BundleActivator; import org.osgi.framework.BundleContext; -import com.redhat.thermostat.common.cli.Command; import com.redhat.thermostat.common.cli.CommandRegistry; import com.redhat.thermostat.common.cli.CommandRegistryImpl; @@ -52,8 +49,13 @@ @Override public void start(BundleContext context) throws Exception { reg = new CommandRegistryImpl(context); - ServiceLoader cmds = ServiceLoader.load(Command.class, getClass().getClassLoader()); - reg.registerCommands(cmds); + + reg.registerCommand("list-vms", new ListVMsCommand()); + reg.registerCommand("shell", new ShellCommand()); + reg.registerCommand("vm-info", new VMInfoCommand()); + reg.registerCommand("vm-stat", new VMStatCommand()); + reg.registerCommand("disconnect", new DisconnectCommand()); + reg.registerCommand("connect", new ConnectCommand()); } @Override diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ConnectCommand.java --- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ConnectCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ConnectCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -67,8 +67,6 @@ private static final Translate translator = LocaleResources.createLocalizer(); - private static final String NAME = "connect"; - private ClientPreferences prefs; private BundleContext context; private DbServiceFactory dbServiceFactory; @@ -121,11 +119,6 @@ } @Override - public String getName() { - return NAME; - } - - @Override public boolean isAvailableOutsideShell() { return false; } diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/DisconnectCommand.java --- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/DisconnectCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/DisconnectCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,7 +36,6 @@ package com.redhat.thermostat.client.cli.internal; -import org.apache.commons.cli.Options; import org.osgi.framework.BundleContext; import org.osgi.framework.FrameworkUtil; import org.osgi.framework.ServiceReference; @@ -52,8 +51,6 @@ private static final Translate translator = LocaleResources.createLocalizer(); - private static final String NAME = "disconnect"; - private BundleContext context; public DisconnectCommand() { @@ -83,16 +80,6 @@ } @Override - public String getName() { - return NAME; - } - - @Override - public Options getOptions() { - return new Options(); - } - - @Override public boolean isAvailableOutsideShell() { return false; } diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ListVMsCommand.java --- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ListVMsCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ListVMsCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -56,8 +56,6 @@ private static final Translate translator = LocaleResources.createLocalizer(); - private static final String NAME = "list-vms"; - private final BundleContext context; public ListVMsCommand() { @@ -96,10 +94,6 @@ context.ungetService(vmsDAORef); } - @Override - public String getName() { - return NAME; - } } diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ShellCommand.java --- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ShellCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ShellCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -68,8 +68,6 @@ private static final String[] exitKeywords = { "exit", "quit", "q" }; - private static final String NAME = "shell"; - private static final String PROMPT = "Thermostat > "; private HistoryProvider historyProvider; @@ -177,11 +175,6 @@ } @Override - public String getName() { - return NAME; - } - - @Override public boolean isStorageRequired() { return false; } diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/VMInfoCommand.java --- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/VMInfoCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/VMInfoCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -60,7 +60,6 @@ private static final Translate translator = LocaleResources.createLocalizer(); - private static final String NAME = "vm-info"; private static final String STILL_ALIVE = translator.localize(LocaleResources.VM_STOP_TIME_RUNNING); private final BundleContext context; @@ -128,10 +127,5 @@ table.render(out); } - @Override - public String getName() { - return NAME; - } - } diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/VMStatCommand.java --- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/VMStatCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/VMStatCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -63,7 +63,6 @@ public class VMStatCommand extends AbstractCommand { private static final Logger log = LoggingUtils.getLogger(VMStatCommand.class); - private static final String CMD_NAME = "vm-stat"; private List delegates; private BundleContext context; @@ -145,9 +144,5 @@ context.ungetService(ref); } - @Override - public String getName() { - return CMD_NAME; - } } diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/main/resources/META-INF/services/com.redhat.thermostat.common.cli.Command --- a/client/cli/src/main/resources/META-INF/services/com.redhat.thermostat.common.cli.Command Mon Apr 15 15:45:04 2013 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,6 +0,0 @@ -com.redhat.thermostat.client.cli.internal.ListVMsCommand -com.redhat.thermostat.client.cli.internal.ShellCommand -com.redhat.thermostat.client.cli.internal.VMInfoCommand -com.redhat.thermostat.client.cli.internal.VMStatCommand -com.redhat.thermostat.client.cli.internal.DisconnectCommand -com.redhat.thermostat.client.cli.internal.ConnectCommand \ No newline at end of file diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ActivatorTest.java --- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ActivatorTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ActivatorTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,8 +36,8 @@ package com.redhat.thermostat.client.cli.internal; +import static com.redhat.thermostat.testutils.Asserts.assertCommandIsRegistered; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; @@ -51,7 +51,6 @@ import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; -import com.redhat.thermostat.common.cli.Command; import com.redhat.thermostat.testutils.StubBundleContext; @RunWith(PowerMockRunner.class) @@ -74,16 +73,17 @@ activator.start(ctx); - assertTrue(ctx.isServiceRegistered(Command.class.getName(), ConnectCommand.class)); - assertTrue(ctx.isServiceRegistered(Command.class.getName(), DisconnectCommand.class)); - assertTrue(ctx.isServiceRegistered(Command.class.getName(), ListVMsCommand.class)); - assertTrue(ctx.isServiceRegistered(Command.class.getName(), ShellCommand.class)); - assertTrue(ctx.isServiceRegistered(Command.class.getName(), VMInfoCommand.class)); - assertTrue(ctx.isServiceRegistered(Command.class.getName(), VMStatCommand.class)); + assertCommandIsRegistered(ctx, "connect", ConnectCommand.class); + assertCommandIsRegistered(ctx, "disconnect", DisconnectCommand.class); + assertCommandIsRegistered(ctx, "list-vms", ListVMsCommand.class); + assertCommandIsRegistered(ctx, "shell", ShellCommand.class); + assertCommandIsRegistered(ctx, "vm-info", VMInfoCommand.class); + assertCommandIsRegistered(ctx, "vm-stat", VMStatCommand.class); activator.stop(ctx); assertEquals(0, ctx.getAllServices().size()); } + } diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ConnectCommandTest.java --- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ConnectCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ConnectCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -38,7 +38,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.eq; @@ -171,16 +170,5 @@ assertTrue(cmd.isAvailableInShell()); } - @Test - public void testName() { - assertEquals("connect", cmd.getName()); - } - - @Test - public void testDescAndUsage() { - assertNotNull(cmd.getDescription()); - assertNotNull(cmd.getUsage()); - } - } diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/DisconnectCommandTest.java --- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/DisconnectCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/DisconnectCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -38,13 +38,11 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import org.apache.commons.cli.Options; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -114,22 +112,5 @@ assertFalse(cmd.isStorageRequired()); } - @Test - public void testName() { - assertEquals("disconnect", cmd.getName()); - } - - @Test - public void testDescAndUsage() { - assertNotNull(cmd.getDescription()); - assertNotNull(cmd.getUsage()); - } - - @Test - public void testOptions() { - Options options = cmd.getOptions(); - assertNotNull(options); - assertTrue(options.getOptions().size() == 0); - } } diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ListVMsCommandTest.java --- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ListVMsCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ListVMsCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -37,7 +37,6 @@ package com.redhat.thermostat.client.cli.internal; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; import static org.mockito.Matchers.eq; import static org.mockito.Matchers.isA; @@ -46,7 +45,6 @@ import java.util.Arrays; -import org.apache.commons.cli.Options; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -184,22 +182,5 @@ } } - @Test - public void testName() { - assertEquals("list-vms", cmd.getName()); - } - - @Test - public void testDescAndUsage() { - assertNotNull(cmd.getDescription()); - assertNotNull(cmd.getUsage()); - } - - @Test - public void testOptions() { - Options options = cmd.getOptions(); - assertNotNull(options); - assertEquals(0, options.getOptions().size()); - } } diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ShellCommandTest.java --- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ShellCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ShellCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -38,7 +38,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -51,7 +50,6 @@ import jline.UnixTerminal; import jline.console.history.PersistentHistory; -import org.apache.commons.cli.Options; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -226,24 +224,6 @@ } @Test - public void testName() { - assertEquals("shell", cmd.getName()); - } - - @Test - public void testDescAndUsage() { - assertNotNull(cmd.getDescription()); - assertNotNull(cmd.getUsage()); - } - - @Test - public void testOptions() { - Options options = cmd.getOptions(); - assertNotNull(options); - assertEquals(0, options.getOptions().size()); - } - - @Test public void testStorageRequired() { assertFalse(cmd.isStorageRequired()); } diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/VMInfoCommandTest.java --- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/VMInfoCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/VMInfoCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -37,8 +37,6 @@ package com.redhat.thermostat.client.cli.internal; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; @@ -49,12 +47,9 @@ import java.util.HashMap; import java.util.TimeZone; -import org.apache.commons.cli.Option; -import org.apache.commons.cli.Options; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; import com.redhat.thermostat.common.cli.CommandException; @@ -198,13 +193,6 @@ } } - @Test - public void testName() { - context.registerService(VmInfoDAO.class, vmsDAO, null); - cmd = new VMInfoCommand(context); - assertEquals("vm-info", cmd.getName()); - } - @Bug(id="1046", summary="CLI vm-info display wrong stop time for living vms", url="http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1046") @@ -233,36 +221,6 @@ } @Test - public void testDescAndUsage() { - context.registerService(VmInfoDAO.class, vmsDAO, null); - cmd = new VMInfoCommand(context); - assertNotNull(cmd.getDescription()); - assertNotNull(cmd.getUsage()); - } - - @Ignore - @Test - public void testOptions() { - context.registerService(VmInfoDAO.class, vmsDAO, null); - cmd = new VMInfoCommand(context); - Options options = cmd.getOptions(); - assertNotNull(options); - assertEquals(2, options.getOptions().size()); - - assertTrue(options.hasOption("vmId")); - Option vm = options.getOption("vmId"); - assertEquals("the ID of the VM to monitor", vm.getDescription()); - assertFalse(vm.isRequired()); - assertTrue(vm.hasArg()); - - assertTrue(options.hasOption("hostId")); - Option host = options.getOption("hostId"); - assertEquals("the ID of the host to monitor", host.getDescription()); - assertTrue(host.isRequired()); - assertTrue(host.hasArg()); - } - - @Test public void testStorageRequired() { context.registerService(VmInfoDAO.class, vmsDAO, null); cmd = new VMInfoCommand(context); diff -r c8d427902c6b -r f0f0027e0724 client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/VmStatCommandTest.java --- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/VmStatCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/VmStatCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -263,50 +263,6 @@ } @Test - public void testName() { - StubBundleContext context = new StubBundleContext(); - VMStatCommand cmd = new VMStatCommand(context); - assertEquals("vm-stat", cmd.getName()); - } - - @Test - public void testDescAndUsage() { - StubBundleContext context = new StubBundleContext(); - VMStatCommand cmd = new VMStatCommand(context); - assertNotNull(cmd.getDescription()); - assertNotNull(cmd.getUsage()); - } - - @Ignore - @Test - public void testOptions() { - StubBundleContext context = new StubBundleContext(); - VMStatCommand cmd = new VMStatCommand(context); - Options options = cmd.getOptions(); - assertNotNull(options); - assertEquals(3, options.getOptions().size()); - - assertTrue(options.hasOption("vmId")); - Option vm = options.getOption("vmId"); - assertEquals("the ID of the VM to monitor", vm.getDescription()); - assertTrue(vm.isRequired()); - assertTrue(vm.hasArg()); - - assertTrue(options.hasOption("hostId")); - Option host = options.getOption("hostId"); - assertEquals("the ID of the host to monitor", host.getDescription()); - assertTrue(host.isRequired()); - assertTrue(host.hasArg()); - - assertTrue(options.hasOption("continuous")); - Option cont = options.getOption("continuous"); - assertEquals("c", cont.getOpt()); - assertEquals("print data continuously", cont.getDescription()); - assertFalse(cont.isRequired()); - assertFalse(cont.hasArg()); - } - - @Test public void testNoStats() throws CommandException { // Fail stats != null check VMStatPrintDelegate badDelegate = mock(VMStatPrintDelegate.class); diff -r c8d427902c6b -r f0f0027e0724 client/command/src/main/java/com/redhat/thermostat/client/command/cli/PingCommand.java --- a/client/command/src/main/java/com/redhat/thermostat/client/command/cli/PingCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/command/src/main/java/com/redhat/thermostat/client/command/cli/PingCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -64,8 +64,6 @@ private static final Translate translator = LocaleResources.createLocalizer(); - private static final String NAME = "ping"; - private class PongListener implements RequestResponseListener { private PrintStream out; @@ -180,14 +178,9 @@ private void printCustomMessageWithUsage(PrintStream out, String message) { out.println(message); - out.println(getUsage()); + // FIXME add usage back out.println(getUsage()); return; } - @Override - public String getName() { - return NAME; - } - } diff -r c8d427902c6b -r f0f0027e0724 client/command/src/main/java/com/redhat/thermostat/client/command/internal/Activator.java --- a/client/command/src/main/java/com/redhat/thermostat/client/command/internal/Activator.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/command/src/main/java/com/redhat/thermostat/client/command/internal/Activator.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,17 +36,20 @@ package com.redhat.thermostat.client.command.internal; +import org.osgi.framework.BundleActivator; import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceRegistration; import com.redhat.thermostat.client.command.RequestQueue; -import com.redhat.thermostat.common.CommandLoadingBundleActivator; +import com.redhat.thermostat.client.command.cli.PingCommand; +import com.redhat.thermostat.common.cli.CommandRegistryImpl; -public class Activator extends CommandLoadingBundleActivator { +public class Activator implements BundleActivator { private RequestQueueImpl queue; private ServiceRegistration queueRegistration; private ConfigurationRequestContext configContext; + private CommandRegistryImpl reg; public Activator() { configContext = new ConfigurationRequestContext(); @@ -57,7 +60,9 @@ public void start(BundleContext context) throws Exception { queueRegistration = context.registerService(RequestQueue.class.getName(), queue, null); queue.startProcessingRequests(); - super.start(context); + + reg = new CommandRegistryImpl(context); + reg.registerCommand("ping", new PingCommand()); } @Override @@ -67,7 +72,8 @@ queueRegistration.unregister(); } configContext.getBootstrap().releaseExternalResources(); - super.stop(context); + + reg.unregisterCommands(); } } diff -r c8d427902c6b -r f0f0027e0724 client/command/src/main/resources/META-INF/services/com.redhat.thermostat.common.cli.Command --- a/client/command/src/main/resources/META-INF/services/com.redhat.thermostat.common.cli.Command Mon Apr 15 15:45:04 2013 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,1 +0,0 @@ -com.redhat.thermostat.client.command.cli.PingCommand \ No newline at end of file diff -r c8d427902c6b -r f0f0027e0724 client/command/src/test/java/com/redhat/thermostat/client/command/cli/PingCommandTest.java --- a/client/command/src/test/java/com/redhat/thermostat/client/command/cli/PingCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/command/src/test/java/com/redhat/thermostat/client/command/cli/PingCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -60,14 +60,6 @@ private static final String KNOWN_AGENT_ID = "some-agent-id"; @Test - public void testCommandName() { - StubBundleContext context = new StubBundleContext(); - PingCommand command = new PingCommand(context); - - assertEquals("ping", command.getName()); - } - - @Test public void testCommandNeedsAgentId() throws CommandException { StubBundleContext context = new StubBundleContext(); @@ -80,7 +72,7 @@ command.run(factory.createContext(args)); // TODO why doesn't ping throw an exception? - assertEquals("Ping command accepts one and only one argument.\nUsage not available.\n", factory.getOutput()); + assertEquals("Ping command accepts one and only one argument.\n", factory.getOutput()); } @Test diff -r c8d427902c6b -r f0f0027e0724 client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/GUIClientCommand.java --- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/GUIClientCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/GUIClientCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -55,11 +55,6 @@ } @Override - public String getName() { - return "gui"; - } - - @Override public boolean isStorageRequired() { return false; } diff -r c8d427902c6b -r f0f0027e0724 client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/osgi/ThermostatActivator.java --- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/osgi/ThermostatActivator.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/osgi/ThermostatActivator.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,7 +36,6 @@ package com.redhat.thermostat.client.swing.internal.osgi; -import java.util.Arrays; import java.util.Dictionary; import java.util.Hashtable; import java.util.Map; @@ -115,7 +114,7 @@ main = new Main(context, keyring, appSvc, new String[0]); GUIClientCommand cmd = new GUIClientCommand(main); - cmdReg.registerCommands(Arrays.asList(cmd)); + cmdReg.registerCommand("gui", cmd); } @Override diff -r c8d427902c6b -r f0f0027e0724 client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/GUIClientCommandTest.java --- a/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/GUIClientCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/GUIClientCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,14 +36,11 @@ package com.redhat.thermostat.client.swing.internal; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import org.apache.commons.cli.Options; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -83,26 +80,9 @@ } @Test - public void testName() { - assertEquals("gui", cmd.getName()); - } - - @Test - public void testDescAndUsage() { - assertNotNull(cmd.getDescription()); - assertNotNull(cmd.getUsage()); - } - - @Test public void testRequiresStorage() { assertFalse(cmd.isStorageRequired()); } - @Test - public void testOptions() { - Options options = cmd.getOptions(); - assertNotNull(options); - assertEquals(0, options.getOptions().size()); - } } diff -r c8d427902c6b -r f0f0027e0724 common/core/pom.xml --- a/common/core/pom.xml Mon Apr 15 15:45:04 2013 +0200 +++ b/common/core/pom.xml Wed Apr 17 14:34:38 2013 -0400 @@ -113,6 +113,12 @@ test + com.redhat.thermostat + thermostat-common-test + ${project.version} + test + + org.powermock powermock-api-mockito test diff -r c8d427902c6b -r f0f0027e0724 common/core/src/main/java/com/redhat/thermostat/common/CommandLoadingBundleActivator.java --- a/common/core/src/main/java/com/redhat/thermostat/common/CommandLoadingBundleActivator.java Mon Apr 15 15:45:04 2013 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,75 +0,0 @@ -/* - * Copyright 2012, 2013 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 - * . - * - * 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.common; - -import java.util.ServiceLoader; - -import org.osgi.framework.BundleActivator; -import org.osgi.framework.BundleContext; - -import com.redhat.thermostat.common.cli.Command; -import com.redhat.thermostat.common.cli.CommandRegistry; -import com.redhat.thermostat.common.cli.CommandRegistryImpl; - -/** - * Superclass for activators that need to register commands. The bundle for this - * activator should contain a META-INF/services/com.redhat.thermostat.common.cli.Command - * file containing the class names that should be loaded as commands. If this activator - * also needs to create/destroy other resources during start() and stop(), be sure to - * call super() - */ -public abstract class CommandLoadingBundleActivator implements BundleActivator { - - private CommandRegistry registry; - private ServiceLoader commands; - - @Override - public void start(BundleContext context) throws Exception { - registry = new CommandRegistryImpl(context); - commands = ServiceLoader.load(Command.class, getClass().getClassLoader()); - registry.registerCommands(commands); - } - - @Override - public void stop(BundleContext context) throws Exception { - if (registry != null && commands != null) { - registry.unregisterCommands(); - } - } - -} - diff -r c8d427902c6b -r f0f0027e0724 common/core/src/main/java/com/redhat/thermostat/common/cli/AbstractCommand.java --- a/common/core/src/main/java/com/redhat/thermostat/common/cli/AbstractCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/cli/AbstractCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,75 +36,17 @@ package com.redhat.thermostat.common.cli; -import java.util.logging.Logger; - -import org.apache.commons.cli.Options; - -import com.redhat.thermostat.common.utils.LoggingUtils; - /** - * A partial implementation of {@link Command} that most implementations should extend. Includes - * sane behaviour regarding {@link CommandInfo} methods and those getters that return data that - * is included in a {@link CommandInfo object}. By default, any extension of this class will - * require {@link Storage}, and be available both in and out of the Thermostat shell; override - * the appropriate method to return false if other behaviour is needed for a particular {@link Command}. - * The only methods not provided as default implementation are {@link Command#getName()} and - * {@link Command#run(CommandContext)}. + * A partial implementation of {@link Command} that most implementations should + * extend. By default, any extension of this class will require {@link Storage}, + * and be available both in and out of the Thermostat shell. *

- * Concrete implementations must be registered as OSGi services with {@link Command} as the - * class. This may be done through the use of a BundleActivator which descends from - * {@link CommandLoadingBundleActivator} - * + * Concrete implementations must be registered as OSGi services with + * {@link Command} as the class. This may be done through a + * {@link CommandRegistry}. */ public abstract class AbstractCommand implements Command { - private static final Logger logger = LoggingUtils.getLogger(AbstractCommand.class); - private CommandInfo info; - private static final String noDesc = "Description not available."; - private static final String noUsage = "Usage not available."; - - public void setCommandInfo(CommandInfo info) { - this.info = info; - } - - public boolean hasCommandInfo() { - return info != null; - } - - @Override - public String getDescription() { - String desc = null; - if (hasCommandInfo()) { - desc = info.getDescription(); - } - if (desc == null) { - desc = noDesc; - } - return desc; - } - - @Override - public String getUsage() { - String usage = null; - if (hasCommandInfo()) { - usage = info.getUsage(); - } - if (usage == null) { - usage = noUsage; - } - return usage; - } - - @Override - public Options getOptions() { - try { - return info.getOptions(); - } catch (NullPointerException e) { - logger.warning("CommandInfo not yet set, returning empty Options."); - return new Options(); - } - } - @Override public boolean isStorageRequired() { return true; diff -r c8d427902c6b -r f0f0027e0724 common/core/src/main/java/com/redhat/thermostat/common/cli/AbstractStateNotifyingCommand.java --- a/common/core/src/main/java/com/redhat/thermostat/common/cli/AbstractStateNotifyingCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/cli/AbstractStateNotifyingCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -46,8 +46,9 @@ * to their runtime status to other components. *

* Concrete implementations must be registered as OSGi services with {@link Command} as the - * class. This may be done through the use of a BundleActivator which descends from - * {@link CommandLoadingBundleActivator} + * class. + * + * @see Command */ public abstract class AbstractStateNotifyingCommand extends AbstractCommand { diff -r c8d427902c6b -r f0f0027e0724 common/core/src/main/java/com/redhat/thermostat/common/cli/Command.java --- a/common/core/src/main/java/com/redhat/thermostat/common/cli/Command.java Mon Apr 15 15:45:04 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/cli/Command.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,21 +36,13 @@ package com.redhat.thermostat.common.cli; -import org.apache.commons.cli.Options; - import com.redhat.thermostat.annotations.ExtensionPoint; /** * Represents a command on the command line. *

* In order to be runnable, a command must be registered as an OSGi service - * with the {@link #NAME} set to the value of {@link #getName()}. If this is - * done directly, the developer is then responsible for enabling and disabling - * it at the appropriate times. - *

- * Bundles which provide {@link Command}s may choose to have their BundleActivator - * extend from {@link CommandLoadingBundleActivator} and in so doing avoid needing to - * explicitly register their {@link Command} implementations as services. + * with the {@link #NAME} set to the name of the command. *

* It is also possible to use an instance of {@link CommandRegistry}, registering the * {@link Command}s when the bundle starts and and unregistering them when the @@ -60,7 +52,6 @@ * or {@link AbstractStateNotifyingCommand} classes to descend from, as they provide * sensible default implementations of most methods and/or provide some other functionality. *

- * @see CommandLoadingBundleActivator * @see CommandRegistry * @see AbstractCommand * @see AbstractStateNotifyingCommand @@ -76,29 +67,6 @@ public void run(CommandContext ctx) throws CommandException; /** - * Returns a name for this command. This will be used by the user to select - * this command. - */ - public String getName(); - - /** - * A short description for the command indicating what it does. - */ - public String getDescription(); - - /** - * How the user should invoke this command - */ - public String getUsage(); - - /** - * Returns the Options that the command is prepared to handle. - * If the user provides unknown or malformed arguments, this command will - * not be invoked. - */ - public Options getOptions(); - - /** * Whether the command depends on {@link Storage} * @return true if {@link Storage} is required. */ diff -r c8d427902c6b -r f0f0027e0724 common/core/src/main/java/com/redhat/thermostat/common/cli/CommandContext.java --- a/common/core/src/main/java/com/redhat/thermostat/common/cli/CommandContext.java Mon Apr 15 15:45:04 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/cli/CommandContext.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,7 +36,6 @@ package com.redhat.thermostat.common.cli; - public interface CommandContext { Console getConsole(); diff -r c8d427902c6b -r f0f0027e0724 common/core/src/main/java/com/redhat/thermostat/common/cli/CommandInfo.java --- a/common/core/src/main/java/com/redhat/thermostat/common/cli/CommandInfo.java Mon Apr 15 15:45:04 2013 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,57 +0,0 @@ -/* - * Copyright 2012, 2013 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 - * . - * - * 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.common.cli; - -import java.util.List; - -import org.apache.commons.cli.Options; - -public interface CommandInfo { - - public String getName(); - - public String getDescription(); - - public String getUsage(); - - public Options getOptions(); - - /** Returns a list of jar that this command depends on */ - public List getDependencyResourceNames(); - -} - diff -r c8d427902c6b -r f0f0027e0724 common/core/src/main/java/com/redhat/thermostat/common/cli/CommandInfoNotFoundException.java --- a/common/core/src/main/java/com/redhat/thermostat/common/cli/CommandInfoNotFoundException.java Mon Apr 15 15:45:04 2013 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,53 +0,0 @@ -/* - * Copyright 2012, 2013 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 - * . - * - * 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.common.cli; - -public class CommandInfoNotFoundException extends RuntimeException { - - private String commandName = null; - - public CommandInfoNotFoundException(String commandName) { - super("data for command '" + commandName + "' not found"); - - this.commandName = commandName; - } - - public String getCommandName() { - return commandName; - } -} - diff -r c8d427902c6b -r f0f0027e0724 common/core/src/main/java/com/redhat/thermostat/common/cli/CommandInfoSource.java --- a/common/core/src/main/java/com/redhat/thermostat/common/cli/CommandInfoSource.java Mon Apr 15 15:45:04 2013 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,48 +0,0 @@ -/* - * Copyright 2012, 2013 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 - * . - * - * 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.common.cli; - -import java.util.Collection; - -public interface CommandInfoSource { - - public CommandInfo getCommandInfo(String name) throws CommandInfoNotFoundException; - - public Collection getCommandInfos(); - -} - diff -r c8d427902c6b -r f0f0027e0724 common/core/src/main/java/com/redhat/thermostat/common/cli/CommandRegistry.java --- a/common/core/src/main/java/com/redhat/thermostat/common/cli/CommandRegistry.java Mon Apr 15 15:45:04 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/cli/CommandRegistry.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,19 +36,11 @@ package com.redhat.thermostat.common.cli; -import java.util.Collection; - public interface CommandRegistry { - public void registerCommand(Command cmd); - - public void registerCommands(Iterable cmds); + public void registerCommand(String name, Command cmd); public void unregisterCommands(); - public Command getCommand(String name); - - public Collection getRegisteredCommands(); - } diff -r c8d427902c6b -r f0f0027e0724 common/core/src/main/java/com/redhat/thermostat/common/cli/CommandRegistryImpl.java --- a/common/core/src/main/java/com/redhat/thermostat/common/cli/CommandRegistryImpl.java Mon Apr 15 15:45:04 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/common/cli/CommandRegistryImpl.java Wed Apr 17 14:34:38 2013 -0400 @@ -38,70 +38,41 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Hashtable; +import java.util.Objects; import org.osgi.framework.BundleContext; -import org.osgi.framework.ServiceReference; - -import com.redhat.thermostat.common.utils.ServiceRegistry; +import org.osgi.framework.ServiceRegistration; /** - * This class mainly wraps around a ServiceRegistry object, handling the additional - * non-osgi-specific task of enable/disable of Command objects as they are registered - * or unregistered. + * Handles registration and un-registration of {@link Command}s. */ public class CommandRegistryImpl implements CommandRegistry { private BundleContext context; - private ServiceRegistry proxy; - private Collection myRegisteredCommands; + private Collection myRegisteredCommands; public CommandRegistryImpl(BundleContext ctx) { context = ctx; - proxy = new ServiceRegistry(ctx, Command.class.getName()); myRegisteredCommands = new ArrayList<>(); } @Override - public void registerCommand(Command cmd) { - proxy.registerService(cmd, cmd.getName()); - myRegisteredCommands.add(cmd); - } + public void registerCommand(String name, Command cmd) { + Objects.requireNonNull(name); + Objects.requireNonNull(cmd); - @Override - public void registerCommands(Iterable cmds) { - for (Command cmd : cmds) { - registerCommand(cmd); - } + Hashtable props = new Hashtable<>(); + props.put(Command.NAME, name); + ServiceRegistration registration = context.registerService(Command.class.getName(), cmd, props); + myRegisteredCommands.add(registration); } @Override public void unregisterCommands() { - proxy.unregisterAll(); - } - - @Override - public Command getCommand(String name) { - Command command = proxy.getService(name); - if (command != null && command instanceof AbstractCommand) { - initializeCommandInfo((AbstractCommand) command); + for (ServiceRegistration reg : myRegisteredCommands) { + reg.unregister(); } - return command; - } - - void initializeCommandInfo(AbstractCommand command) { - if (!command.hasCommandInfo()) { - @SuppressWarnings("rawtypes") - ServiceReference infosRef = context.getServiceReference(CommandInfoSource.class); - @SuppressWarnings("unchecked") - CommandInfoSource infos = (CommandInfoSource) context.getService(infosRef); - command.setCommandInfo(infos.getCommandInfo(command.getName())); - context.ungetService(infosRef); - } - } - - @Override - public Collection getRegisteredCommands() { - return proxy.getRegisteredServices(); } } diff -r c8d427902c6b -r f0f0027e0724 common/core/src/main/java/com/redhat/thermostat/test/TestCommandContextFactory.java --- a/common/core/src/main/java/com/redhat/thermostat/test/TestCommandContextFactory.java Mon Apr 15 15:45:04 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/test/TestCommandContextFactory.java Wed Apr 17 14:34:38 2013 -0400 @@ -65,8 +65,6 @@ reset(); } - private CommandRegistry commandRegistry = new TestCommandRegistry(); - private TestConsole console; private PipedOutputStream inOut; @@ -105,7 +103,7 @@ @Override public CommandRegistry getCommandRegistry() { - return commandRegistry; + return TestCommandContextFactory.this.getCommandRegistry(); } @Override @@ -116,11 +114,6 @@ }; } - @Override - public CommandRegistry getCommandRegistry() { - return commandRegistry; - } - public String getOutput() { return new String(out.toByteArray()); } diff -r c8d427902c6b -r f0f0027e0724 common/core/src/main/java/com/redhat/thermostat/test/TestCommandRegistry.java --- a/common/core/src/main/java/com/redhat/thermostat/test/TestCommandRegistry.java Mon Apr 15 15:45:04 2013 +0200 +++ b/common/core/src/main/java/com/redhat/thermostat/test/TestCommandRegistry.java Wed Apr 17 14:34:38 2013 -0400 @@ -55,8 +55,8 @@ return commands.values(); } - public void registerCommand(Command cmd) { - commands.put(cmd.getName(), cmd); + public void registerCommand(String name, Command cmd) { + commands.put(name, cmd); } @Override @@ -64,11 +64,5 @@ commands.clear(); } - @Override - public void registerCommands(Iterable cmds) { - for (Command cmd : cmds) { - registerCommand(cmd); - } - } } diff -r c8d427902c6b -r f0f0027e0724 common/core/src/test/java/com/redhat/thermostat/common/cli/AbstractCommandTest.java --- a/common/core/src/test/java/com/redhat/thermostat/common/cli/AbstractCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/common/core/src/test/java/com/redhat/thermostat/common/cli/AbstractCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,56 +36,17 @@ package com.redhat.thermostat.common.cli; -import org.apache.commons.cli.Option; -import org.apache.commons.cli.Options; -import org.junit.Test; +import static org.junit.Assert.assertTrue; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import org.junit.Test; public class AbstractCommandTest { - @Test - public void testHasCommandInfo() { - AbstractCommand command = createCommandForTest(); - CommandInfo info = mock(CommandInfo.class); - assertFalse(command.hasCommandInfo()); - command.setCommandInfo(info); - assertTrue(command.hasCommandInfo()); - } - - @Test - public void testGettersReturnCorrectly() { - AbstractCommand command = createCommandForTest(); - - CommandInfo info = mock(CommandInfo.class); - when(info.getDescription()).thenReturn("The description"); - when(info.getUsage()).thenReturn("The usage."); - Options options = new Options(); - Option option = new Option("option", "description"); - options.addOption(option); - when(info.getOptions()).thenReturn(options); - command.setCommandInfo(info); - - // The values that should be returned based on the CommandInfo supplied. - assertTrue(command.hasCommandInfo()); - assertEquals(options, command.getOptions()); - assertEquals("The description", command.getDescription()); - assertEquals("The usage.", command.getUsage()); - } @Test public void testDefaultReturnValues() { AbstractCommand command = createCommandForTest(); - // The default values used before CommandInfo injected. - assertEquals("Description not available.", command.getDescription()); - assertEquals("Usage not available.", command.getUsage()); - Options options = command.getOptions(); - assertTrue(options.getOptions().isEmpty()); assertTrue(command.isStorageRequired()); assertTrue(command.isAvailableInShell()); assertTrue(command.isAvailableOutsideShell()); @@ -93,17 +54,10 @@ private AbstractCommand createCommandForTest() { return new AbstractCommand() { - @Override public void run(CommandContext ctx) throws CommandException { // Do nothing. } - - @Override - public String getName() { - return "name"; - } - }; } } diff -r c8d427902c6b -r f0f0027e0724 common/core/src/test/java/com/redhat/thermostat/common/cli/AbstractStateNotifyingCommandTest.java --- a/common/core/src/test/java/com/redhat/thermostat/common/cli/AbstractStateNotifyingCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/common/core/src/test/java/com/redhat/thermostat/common/cli/AbstractStateNotifyingCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -38,7 +38,6 @@ import static org.junit.Assert.assertNotNull; -import org.apache.commons.cli.Options; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -61,26 +60,6 @@ } @Override - public String getName() { - return null; - } - - @Override - public String getDescription() { - return null; - } - - @Override - public String getUsage() { - return null; - } - - @Override - public Options getOptions() { - return new Options(); - } - - @Override public boolean isStorageRequired() { return false; } diff -r c8d427902c6b -r f0f0027e0724 common/core/src/test/java/com/redhat/thermostat/common/cli/CommandRegistryImplTest.java --- a/common/core/src/test/java/com/redhat/thermostat/common/cli/CommandRegistryImplTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/common/core/src/test/java/com/redhat/thermostat/common/cli/CommandRegistryImplTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,47 +36,27 @@ package com.redhat.thermostat.common.cli; +import static com.redhat.thermostat.testutils.Asserts.assertCommandIsRegistered; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; - -import java.util.Arrays; -import java.util.Collection; -import java.util.Hashtable; import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.osgi.framework.BundleContext; import org.osgi.framework.InvalidSyntaxException; -import org.osgi.framework.ServiceReference; -import org.osgi.framework.ServiceRegistration; -import com.redhat.thermostat.common.utils.ServiceRegistry; +import com.redhat.thermostat.testutils.StubBundleContext; public class CommandRegistryImplTest { private CommandRegistryImpl commandRegistry; - private BundleContext bundleContext; + private StubBundleContext bundleContext; @Before public void setUp() { - bundleContext = mock(BundleContext.class); + bundleContext = new StubBundleContext(); commandRegistry = new CommandRegistryImpl(bundleContext); - ServiceReference infoSourceReference = mock(ServiceReference.class); - CommandInfoSource infoSource = mock(CommandInfoSource.class); - CommandInfo info = mock(CommandInfo.class); - when(infoSource.getCommandInfo(any(String.class))).thenReturn(info); - when(bundleContext.getServiceReference(eq(CommandInfoSource.class))).thenReturn(infoSourceReference); - when(bundleContext.getService(infoSourceReference)).thenReturn(infoSource); } @After @@ -87,150 +67,35 @@ @Test public void testRegisterCommands() { Command cmd1 = mock(Command.class); - when(cmd1.getName()).thenReturn("test1"); + String name1 = "test1"; Command cmd2 = mock(Command.class); - when(cmd2.getName()).thenReturn("test2"); - - commandRegistry.registerCommands(Arrays.asList(cmd1, cmd2)); + String name2 = "test2"; - Hashtable props1 = new Hashtable<>(); - props1.put(ServiceRegistry.SERVICE_NAME, "test1"); - Hashtable props2 = new Hashtable<>(); - props2.put(ServiceRegistry.SERVICE_NAME, "test2"); - verify(bundleContext).registerService(Command.class.getName(), cmd1, props1); - verify(bundleContext).registerService(Command.class.getName(), cmd2, props2); + commandRegistry.registerCommand(name1, cmd1); + commandRegistry.registerCommand(name2, cmd2); - verifyNoMoreInteractions(bundleContext); + assertCommandIsRegistered(bundleContext, "test1", cmd1.getClass()); + assertCommandIsRegistered(bundleContext, name2, cmd2.getClass()); + assertEquals(2, bundleContext.getAllServices().size()); } @Test public void testUnregisterCommand() throws InvalidSyntaxException { Command cmd1 = mock(Command.class); - when(cmd1.getName()).thenReturn("test1"); + String name1 = "test1"; Command cmd2 = mock(Command.class); - when(cmd2.getName()).thenReturn("test2"); - - ServiceReference cmd1Reference = mock(ServiceReference.class); - ServiceReference cmd2Reference = mock(ServiceReference.class); - - ServiceRegistration cmd1Reg = mock(ServiceRegistration.class); - when(cmd1Reg.getReference()).thenReturn(cmd1Reference); - ServiceRegistration cmd2Reg = mock(ServiceRegistration.class); - when(cmd2Reg.getReference()).thenReturn(cmd2Reference); + String name2 = "test2"; - Hashtable props1 = new Hashtable<>(); - props1.put(ServiceRegistry.SERVICE_NAME, cmd1.getName()); - Hashtable props2 = new Hashtable<>(); - props2.put(ServiceRegistry.SERVICE_NAME, cmd2.getName()); - - when(bundleContext.registerService(Command.class.getName(), cmd1, props1)).thenReturn(cmd1Reg); - when(bundleContext.registerService(Command.class.getName(), cmd2, props2)).thenReturn(cmd2Reg); + commandRegistry.registerCommand(name1, cmd1); + commandRegistry.registerCommand(name2, cmd2); - commandRegistry.registerCommands(Arrays.asList(cmd1, cmd2)); - - verify(bundleContext).registerService(Command.class.getName(), cmd1, props1); - verify(bundleContext).registerService(Command.class.getName(), cmd2, props2); - - when(bundleContext.getService(eq(cmd1Reference))).thenReturn(cmd1); - when(bundleContext.getService(eq(cmd2Reference))).thenReturn(cmd2); - when(bundleContext.getServiceReferences(Command.class.getName(), (String) null)).thenReturn(new ServiceReference[] {cmd1Reference, cmd2Reference}); + assertEquals(2, bundleContext.getAllServices().size()); commandRegistry.unregisterCommands(); - verify(cmd1Reg).unregister(); - verify(cmd2Reg).unregister(); - - verifyNoMoreInteractions(bundleContext); - } - - @Test - public void testGetCommand() throws InvalidSyntaxException { - ServiceReference ref1 = mock(ServiceReference.class); - when(bundleContext.getServiceReferences(Command.class.getName(), "(&(objectclass=*)(servicename=test1))")).thenReturn(new ServiceReference[] { ref1 }); - Command cmd1 = mock(Command.class); - when(bundleContext.getService(ref1)).thenReturn(cmd1); - - Command cmd = commandRegistry.getCommand("test1"); - - assertSame(cmd1, cmd); - } - - @Test - public void testNotRegisteredCommand() throws InvalidSyntaxException { - ServiceReference ref1 = mock(ServiceReference.class); - Command cmd1 = mock(Command.class); - when(bundleContext.getService(ref1)).thenReturn(cmd1); - - Command cmd = commandRegistry.getCommand("test1"); - - assertNull(cmd); - } - - @Test - public void testNotRegisteredCommandEmptyList() throws InvalidSyntaxException { - when(bundleContext.getServiceReferences(Command.class.getName(), "(&(objectclass=*)(servicename=test1))")).thenReturn(new ServiceReference[0]); - - Command cmd = commandRegistry.getCommand("test1"); - - assertNull(cmd); + assertEquals(0, bundleContext.getAllServices().size()); } - @Test - public void testDoubleRegisteredCommand() throws InvalidSyntaxException { - ServiceReference ref1 = mock(ServiceReference.class); - ServiceReference ref2 = mock(ServiceReference.class); - Command cmd1 = mock(Command.class); - Command cmd2 = mock(Command.class); - when(bundleContext.getServiceReferences(Command.class.getName(), "(&(objectclass=*)(servicename=test1))")).thenReturn(new ServiceReference[] { ref1, ref2 }); - when(bundleContext.getService(ref1)).thenReturn(cmd1); - when(bundleContext.getService(ref2)).thenReturn(cmd2); - - Command cmd = commandRegistry.getCommand("test1"); - - assertSame(cmd1, cmd); - } - - @Test(expected=InternalError.class) - public void testGetCommandInvalidSyntax() throws InvalidSyntaxException { - when(bundleContext.getServiceReferences(Command.class.getName(), "(&(objectclass=*)(servicename=test1))")).thenThrow(new InvalidSyntaxException("test", "test")); - - commandRegistry.getCommand("test1"); - } - - @Test - public void testRegisteredCommands() throws InvalidSyntaxException { - ServiceReference ref1 = mock(ServiceReference.class); - ServiceReference ref2 = mock(ServiceReference.class); - Command cmd1 = mock(Command.class); - Command cmd2 = mock(Command.class); - when(bundleContext.getServiceReferences(Command.class.getName(), null)).thenReturn(new ServiceReference[] { ref1, ref2 }); - when(bundleContext.getService(ref1)).thenReturn(cmd1); - when(bundleContext.getService(ref2)).thenReturn(cmd2); - - Collection cmds = commandRegistry.getRegisteredCommands(); - - assertEquals(2, cmds.size()); - assertTrue(cmds.contains(cmd1)); - assertTrue(cmds.contains(cmd2)); - } - - @Test - public void testInitializeCommandInfo() { - CommandInfoSource infos = mock(CommandInfoSource.class); - ServiceReference infosRef = mock(ServiceReference.class); - CommandInfo info = mock(CommandInfo.class); - when(infos.getCommandInfo("name")).thenReturn(info); - AbstractCommand command = mock(AbstractCommand.class); - when(command.getName()).thenReturn("name"); - when(command.hasCommandInfo()).thenReturn(false); - - when(bundleContext.getServiceReference(CommandInfoSource.class)).thenReturn(infosRef); - when(bundleContext.getService(infosRef)).thenReturn(infos); - - commandRegistry.initializeCommandInfo(command); - verify(command).setCommandInfo(info); - verify(bundleContext).ungetService(infosRef); - } } diff -r c8d427902c6b -r f0f0027e0724 common/core/src/test/java/com/redhat/thermostat/common/cli/SimpleArgumentsTest.java --- a/common/core/src/test/java/com/redhat/thermostat/common/cli/SimpleArgumentsTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/common/core/src/test/java/com/redhat/thermostat/common/cli/SimpleArgumentsTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -47,8 +47,6 @@ import org.junit.Before; import org.junit.Test; -import com.redhat.thermostat.common.cli.SimpleArguments; - public class SimpleArgumentsTest { private SimpleArguments args; diff -r c8d427902c6b -r f0f0027e0724 common/test/src/main/java/com/redhat/thermostat/testutils/Asserts.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/common/test/src/main/java/com/redhat/thermostat/testutils/Asserts.java Wed Apr 17 14:34:38 2013 -0400 @@ -0,0 +1,52 @@ +/* + * Copyright 2013 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 + * . + * + * 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.testutils; + +import java.util.Hashtable; + +public class Asserts { + + public static void assertCommandIsRegistered(StubBundleContext context, String name, Class klass) { + // The Command class is not visible to this module, so we have to live + // with hardcoding some details here + Hashtable props = new Hashtable<>(); + props.put("COMMAND_NAME", name); + if (!context.isServiceRegistered("com.redhat.thermostat.common.cli.Command", klass, props)) { + throw new AssertionError("Command " + name + " is not registered"); + } + } +} diff -r c8d427902c6b -r f0f0027e0724 common/test/src/main/java/com/redhat/thermostat/testutils/StubBundleContext.java --- a/common/test/src/main/java/com/redhat/thermostat/testutils/StubBundleContext.java Mon Apr 15 15:45:04 2013 +0200 +++ b/common/test/src/main/java/com/redhat/thermostat/testutils/StubBundleContext.java Wed Apr 17 14:34:38 2013 -0400 @@ -48,6 +48,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; @@ -364,6 +365,33 @@ return false; } + /** + * @returns true if an instance of implementationClass is registered as + * service serviceName with properties that is a superset of props + */ + public boolean isServiceRegistered(String serviceName, Class implementationClass, Dictionary props) { + for (ServiceInformation info : registeredServices) { + for (String serviceInterface : (String[]) info.properties.get(Constants.OBJECTCLASS)) { + if (serviceInterface.equals(serviceName) + && info.implementation.getClass().equals(implementationClass)) { + boolean propsMatch = true; + Enumeration keys = props.keys(); + while (keys.hasMoreElements()) { + String key = keys.nextElement(); + if (!Objects.equals(props.get(key), info.properties.get(key))) { + propsMatch = false; + break; + } + } + if (propsMatch) { + return true; + } + } + } + } + return false; + } + public Collection getAllServices() { return registeredServices; } diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/BundleManager.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/BundleManager.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/BundleManager.java Wed Apr 17 14:34:38 2013 -0400 @@ -43,8 +43,6 @@ import org.osgi.framework.launch.Framework; import com.redhat.thermostat.annotations.Service; -import com.redhat.thermostat.common.cli.CommandInfoNotFoundException; -import com.redhat.thermostat.common.cli.CommandInfoSource; import com.redhat.thermostat.common.config.Configuration; import com.redhat.thermostat.launcher.internal.BundleLoader; @@ -56,9 +54,7 @@ public abstract void setPrintOSGiInfo(boolean printOSGiInfo); - public abstract void setCommandInfoSource(CommandInfoSource source); - - public abstract void addBundlesFor(String commandName) throws BundleException, CommandInfoNotFoundException, IOException; + public abstract void addBundles(List dependencies) throws BundleException, IOException; public static void preLoadBundles(Framework framework, List bundleLocations, boolean printOSGiInfo) throws BundleException { diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java Wed Apr 17 14:34:38 2013 -0400 @@ -48,7 +48,6 @@ import com.redhat.thermostat.common.ExitStatus; import com.redhat.thermostat.common.Launcher; import com.redhat.thermostat.common.cli.CommandContextFactory; -import com.redhat.thermostat.common.cli.CommandInfoSource; import com.redhat.thermostat.common.cli.CommandRegistry; import com.redhat.thermostat.common.cli.CommandRegistryImpl; import com.redhat.thermostat.common.config.Configuration; @@ -85,12 +84,11 @@ config.getLibRoot(), config.getPluginRoot()); CommandInfoSource commands = new CompoundCommandInfoSource(builtInCommandSource, pluginCommandSource); + cmdInfoReg = context.registerService(CommandInfoSource.class, commands, null); - cmdInfoReg = context.registerService(CommandInfoSource.class, commands, null); - bundleService.setCommandInfoSource(commands); // Register Launcher service since FrameworkProvider is waiting for it blockingly. LauncherImpl launcher = new LauncherImpl(context, - new CommandContextFactory(context), bundleService); + new CommandContextFactory(context), bundleService, commands); launcherReg = context.registerService(Launcher.class.getName(), launcher, null); bundleManReg = context.registerService(BundleManager.class, bundleService, null); ExitStatus exitStatus = new ExitStatusImpl(ExitStatus.EXIT_SUCCESS); @@ -149,7 +147,7 @@ commandInfoSourceTracker.open(); registry = new CommandRegistryImpl(context); - registry.registerCommand(helpCommand); + registry.registerCommand("help", helpCommand); } @Override diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/internal/BasicCommandInfo.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/BasicCommandInfo.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/BasicCommandInfo.java Wed Apr 17 14:34:38 2013 -0400 @@ -40,7 +40,6 @@ import org.apache.commons.cli.Options; -import com.redhat.thermostat.common.cli.CommandInfo; public class BasicCommandInfo implements CommandInfo { diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/internal/BuiltInCommandInfo.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/BuiltInCommandInfo.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/BuiltInCommandInfo.java Wed Apr 17 14:34:38 2013 -0400 @@ -49,7 +49,6 @@ import org.apache.commons.cli.OptionGroup; import org.apache.commons.cli.Options; -import com.redhat.thermostat.common.cli.CommandInfo; import com.redhat.thermostat.common.utils.LoggingUtils; public class BuiltInCommandInfo implements CommandInfo { diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/internal/BuiltInCommandInfoSource.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/BuiltInCommandInfoSource.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/BuiltInCommandInfoSource.java Wed Apr 17 14:34:38 2013 -0400 @@ -46,9 +46,6 @@ import java.util.Properties; import java.util.logging.Logger; -import com.redhat.thermostat.common.cli.CommandInfo; -import com.redhat.thermostat.common.cli.CommandInfoNotFoundException; -import com.redhat.thermostat.common.cli.CommandInfoSource; import com.redhat.thermostat.common.utils.LoggingUtils; public class BuiltInCommandInfoSource implements CommandInfoSource { diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/internal/BundleManagerImpl.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/BundleManagerImpl.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/BundleManagerImpl.java Wed Apr 17 14:34:38 2013 -0400 @@ -50,14 +50,11 @@ import org.osgi.framework.FrameworkUtil; import org.osgi.framework.launch.Framework; -import com.redhat.thermostat.common.cli.CommandInfoNotFoundException; -import com.redhat.thermostat.common.cli.CommandInfoSource; import com.redhat.thermostat.common.config.Configuration; import com.redhat.thermostat.launcher.BundleManager; public class BundleManagerImpl extends BundleManager { - private CommandInfoSource commandInfos; private Map loaded; private Configuration configuration; private BundleLoader loader; @@ -83,16 +80,7 @@ } @Override - public void setCommandInfoSource(CommandInfoSource source) { - this.commandInfos = source; - } - - @Override - public void addBundlesFor(String commandName) throws BundleException, IOException, CommandInfoNotFoundException { - if (configuration.getPrintOSGiInfo()) { - System.out.println("Loading additional bundles for: " + commandName); - } - List requiredBundles = commandInfos.getCommandInfo(commandName).getDependencyResourceNames(); + public void addBundles(List requiredBundles) throws BundleException, IOException { List bundlesToLoad = new ArrayList<>(); if (requiredBundles != null) { for (String resource : requiredBundles) { diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandInfo.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandInfo.java Wed Apr 17 14:34:38 2013 -0400 @@ -0,0 +1,71 @@ +/* + * Copyright 2012, 2013 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 + * . + * + * 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.launcher.internal; + +import java.util.List; + +import org.apache.commons.cli.Options; + +public interface CommandInfo { + /** + * Returns a name for this command. This will be used by the user to select + * this command. + */ + public String getName(); + + /** + * A short description for the command indicating what it does. + */ + public String getDescription(); + + /** + * How the user should invoke this command + */ + public String getUsage(); + + /** + * Returns the Options that the command is prepared to handle. + * If the user provides unknown or malformed arguments, this command will + * not be invoked. + */ + public Options getOptions(); + + /** Returns a list of jar that this command depends on */ + public List getDependencyResourceNames(); + +} + diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandInfoNotFoundException.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandInfoNotFoundException.java Wed Apr 17 14:34:38 2013 -0400 @@ -0,0 +1,53 @@ +/* + * Copyright 2012, 2013 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 + * . + * + * 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.launcher.internal; + +public class CommandInfoNotFoundException extends RuntimeException { + + private String commandName = null; + + public CommandInfoNotFoundException(String commandName) { + super("data for command '" + commandName + "' not found"); + + this.commandName = commandName; + } + + public String getCommandName() { + return commandName; + } +} + diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandInfoSource.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandInfoSource.java Wed Apr 17 14:34:38 2013 -0400 @@ -0,0 +1,49 @@ +/* + * Copyright 2012, 2013 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 + * . + * + * 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.launcher.internal; + +import java.util.Collection; + + +public interface CommandInfoSource { + + public CommandInfo getCommandInfo(String name) throws CommandInfoNotFoundException; + + public Collection getCommandInfos(); + +} + diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandSource.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CommandSource.java Wed Apr 17 14:34:38 2013 -0400 @@ -0,0 +1,49 @@ +package com.redhat.thermostat.launcher.internal; + +import java.util.Arrays; +import java.util.logging.Logger; + +import org.osgi.framework.BundleContext; +import org.osgi.framework.Constants; +import org.osgi.framework.InvalidSyntaxException; +import org.osgi.framework.ServiceReference; + +import com.redhat.thermostat.common.cli.Command; +import com.redhat.thermostat.common.cli.CommandRegistryImpl; +import com.redhat.thermostat.common.utils.LoggingUtils; + +/** + * Provides {@link Command} and {@link CommandInfo} objects for a given command + * name. + * + * @see CommandRegistryImpl + */ +public class CommandSource { + + private static final Logger logger = LoggingUtils.getLogger(CommandSource.class); + + private final BundleContext context; + + public CommandSource(BundleContext context) { + this.context = context; + } + + public Command getCommand(String name) { + try { + ServiceReference[] refs = context.getAllServiceReferences(Command.class.getName(), "(" + Command.NAME + "=" + name + ")"); + if (refs == null) { + return null; + } else if (refs.length > 1) { + logger.warning("More than one matching command implementation found for " + name); + for (int i = 0; i < refs.length; i++) { + logger.warning(name + " is provided by + " + Arrays.toString((String[])refs[i].getProperty(Constants.OBJECTCLASS))); + } + } + ServiceReference ref = refs[0]; + return (Command) context.getService(ref); + } catch (InvalidSyntaxException e) { + throw new IllegalArgumentException("bad name for command: " + name, e); + } + } + +} diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/internal/CompoundCommandInfoSource.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CompoundCommandInfoSource.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/CompoundCommandInfoSource.java Wed Apr 17 14:34:38 2013 -0400 @@ -45,9 +45,6 @@ import org.apache.commons.cli.Options; -import com.redhat.thermostat.common.cli.CommandInfo; -import com.redhat.thermostat.common.cli.CommandInfoNotFoundException; -import com.redhat.thermostat.common.cli.CommandInfoSource; /** * Presents multiple {@link CommandInfoSource}s as one diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/internal/HelpCommand.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/HelpCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/HelpCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -48,9 +48,6 @@ import com.redhat.thermostat.common.cli.Arguments; import com.redhat.thermostat.common.cli.CommandContext; -import com.redhat.thermostat.common.cli.CommandInfo; -import com.redhat.thermostat.common.cli.CommandInfoNotFoundException; -import com.redhat.thermostat.common.cli.CommandInfoSource; import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.TableRenderer; import com.redhat.thermostat.common.locale.Translate; @@ -60,7 +57,6 @@ private static final Translate translator = LocaleResources.createLocalizer(); private static final int COMMANDS_COLUMNS_WIDTH = 14; - private static final String NAME = "help"; private static final String APP_NAME = "thermostat"; private static final CommandInfoComparator comparator = new CommandInfoComparator(); @@ -129,11 +125,6 @@ } @Override - public String getName() { - return NAME; - } - - @Override public boolean isStorageRequired() { return false; } @@ -146,10 +137,10 @@ if (o1.getName().equals(o2.getName())) { return 0; } - if (o1.getName().equals(NAME)) { + if (o1.getName().equals("help")) { return -1; } - if (o2.getName().equals(NAME)) { + if (o2.getName().equals("help")) { return 1; } diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java Wed Apr 17 14:34:38 2013 -0400 @@ -60,8 +60,6 @@ import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandContextFactory; import com.redhat.thermostat.common.cli.CommandException; -import com.redhat.thermostat.common.cli.CommandInfoNotFoundException; -import com.redhat.thermostat.common.cli.CommandRegistry; import com.redhat.thermostat.common.config.ClientPreferences; import com.redhat.thermostat.common.config.InvalidConfigurationException; import com.redhat.thermostat.common.locale.Translate; @@ -88,21 +86,28 @@ private final CommandContextFactory cmdCtxFactory; private final DbServiceFactory dbServiceFactory; private final Version coreVersion; + private final CommandSource commandSource; + private final CommandInfoSource commandInfoSource; /** MUST be mutated in a 'synchronized (this)' block */ private ClientPreferences prefs; - public LauncherImpl(BundleContext context, CommandContextFactory cmdCtxFactory, BundleManager registry) { - this(context, cmdCtxFactory, registry, new LoggingInitializer(), new DbServiceFactory(), new Version()); + + public LauncherImpl(BundleContext context, CommandContextFactory cmdCtxFactory, BundleManager registry, CommandInfoSource infoSource) { + this(context, cmdCtxFactory, registry, infoSource, new CommandSource(context), new LoggingInitializer(), new DbServiceFactory(), new Version()); } LauncherImpl(BundleContext context, CommandContextFactory cmdCtxFactory, BundleManager registry, + CommandInfoSource commandInfoSource, CommandSource commandSource, LoggingInitializer loggingInitializer, DbServiceFactory dbServiceFactory, Version version) { this.context = context; this.cmdCtxFactory = cmdCtxFactory; this.registry = registry; this.dbServiceFactory = dbServiceFactory; this.coreVersion = version; + this.commandSource = commandSource; + this.commandInfoSource = commandInfoSource; + loggingInitializer.initialize(); } @@ -201,24 +206,32 @@ PrintStream out = cmdCtxFactory.getConsole().getOutput(); PrintStream err = cmdCtxFactory.getConsole().getError(); + + CommandInfo cmdInfo; try { - registry.addBundlesFor(cmdName); + cmdInfo = commandInfoSource.getCommandInfo(cmdName); + } catch (CommandInfoNotFoundException commandNotFound) { + runHelpCommandFor(cmdName); + return; + } + + try { + registry.addBundles(cmdInfo.getDependencyResourceNames()); } catch (BundleException | IOException e) { // If this happens we definitely need to do something about it, and the // trace will be immeasurably helpful in figuring out what is wrong. out.println(t.localize(LocaleResources.COMMAND_COULD_NOT_LOAD_BUNDLES, cmdName)); e.printStackTrace(out); return; - } catch (CommandInfoNotFoundException commandNotFound) { - runHelpCommandFor(cmdName); - return; } - Command cmd = getCommand(cmdName); + Command cmd = commandSource.getCommand(cmdName); + if (cmd == null) { err.println(t.localize(LocaleResources.COMMAND_DESCRIBED_BUT_NOT_AVAILALBE, cmdName)); return; } + if ((inShell && !cmd.isAvailableInShell()) || (!inShell && !cmd.isAvailableOutsideShell())) { outputBadShellContext(inShell, out, cmdName); return; @@ -230,7 +243,7 @@ notifier.addActionListener(listener); } } - Options options = cmd.getOptions(); + Options options = cmdInfo.getOptions(); Arguments args = null; try { args = parseCommandArguments(cmdArgs, options); @@ -270,13 +283,6 @@ } } - private Command getCommand(String cmdName) { - - CommandRegistry registry = cmdCtxFactory.getCommandRegistry(); - Command cmd = registry.getCommand(cmdName); - return cmd; - } - private Arguments parseCommandArguments(String[] cmdArgs, Options options) throws CommandLineArgumentParseException { diff -r c8d427902c6b -r f0f0027e0724 launcher/src/main/java/com/redhat/thermostat/launcher/internal/PluginCommandInfoSource.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/PluginCommandInfoSource.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/PluginCommandInfoSource.java Wed Apr 17 14:34:38 2013 -0400 @@ -49,9 +49,6 @@ import java.util.logging.Level; import java.util.logging.Logger; -import com.redhat.thermostat.common.cli.CommandInfo; -import com.redhat.thermostat.common.cli.CommandInfoNotFoundException; -import com.redhat.thermostat.common.cli.CommandInfoSource; import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.launcher.internal.PluginConfiguration.CommandExtensions; import com.redhat.thermostat.launcher.internal.PluginConfiguration.NewCommand; diff -r c8d427902c6b -r f0f0027e0724 launcher/src/test/java/com/redhat/thermostat/launcher/TestCommand.java --- a/launcher/src/test/java/com/redhat/thermostat/launcher/TestCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,142 +0,0 @@ -/* - * Copyright 2012, 2013 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 - * . - * - * 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.launcher; - -import org.apache.commons.cli.Option; -import org.apache.commons.cli.Options; - -import com.redhat.thermostat.common.cli.Command; -import com.redhat.thermostat.common.cli.CommandContext; -import com.redhat.thermostat.common.cli.CommandException; -import com.redhat.thermostat.common.cli.CommandInfo; - - -public class TestCommand implements Command { - - private String name; - private Handle handle; - private String description; - private String usage; - private boolean storageRequired; - private boolean availableInShell = true; - private boolean availableOutsideShell = true; - private CommandInfo info; - - private Options options = new Options(); - - public static interface Handle { - public void run(CommandContext ctx) throws CommandException; - } - - public TestCommand(String name) { - this(name, null); - } - - public TestCommand(String name, Handle r) { - this.name = name; - this.handle = r; - } - - @Override - public void run(CommandContext ctx) throws CommandException { - if (handle != null) { - handle.run(ctx); - } - } - - @Override - public String getName() { - return name; - } - - @Override - public String getDescription() { - return description; - } - - public void setDescription(String desc) { - description = desc; - } - - @Override - public String getUsage() { - return usage; - } - - public void setUsage(String usage) { - this.usage = usage; - } - - @Override - public Options getOptions() { - return options; - } - - public void addOptions(Option... options) { - for (Option option : options) { - this.options.addOption(option); - } - } - - @Override - public boolean isStorageRequired() { - return storageRequired; - } - - public void setStorageRequired(boolean storageRequired) { - this.storageRequired = storageRequired; - } - - @Override - public boolean isAvailableInShell() { - return availableInShell; - } - - void setAvailableInShell(boolean available) { - this.availableInShell = available; - } - - @Override - public boolean isAvailableOutsideShell() { - return availableOutsideShell; - } - - void setAvailableOutsideShell(boolean available) { - this.availableOutsideShell = available; - } -} - diff -r c8d427902c6b -r f0f0027e0724 launcher/src/test/java/com/redhat/thermostat/launcher/internal/ActivatorTest.java --- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/ActivatorTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/ActivatorTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,6 +36,7 @@ package com.redhat.thermostat.launcher.internal; +import static com.redhat.thermostat.testutils.Asserts.assertCommandIsRegistered; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -43,7 +44,6 @@ import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Matchers.isA; -import static org.mockito.Matchers.isNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -53,11 +53,9 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; -import java.util.Dictionary; import java.util.Hashtable; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; @@ -66,7 +64,6 @@ import org.osgi.framework.FrameworkUtil; import org.osgi.framework.InvalidSyntaxException; import org.osgi.framework.ServiceReference; -import org.osgi.framework.ServiceRegistration; import org.osgi.framework.launch.Framework; import org.osgi.util.tracker.ServiceTracker; import org.osgi.util.tracker.ServiceTrackerCustomizer; @@ -79,10 +76,7 @@ import com.redhat.thermostat.common.MultipleServiceTracker; import com.redhat.thermostat.common.MultipleServiceTracker.Action; import com.redhat.thermostat.common.cli.Command; -import com.redhat.thermostat.common.cli.CommandInfo; -import com.redhat.thermostat.common.cli.CommandInfoSource; import com.redhat.thermostat.common.config.Configuration; -import com.redhat.thermostat.common.utils.ServiceRegistry; import com.redhat.thermostat.launcher.BundleManager; import com.redhat.thermostat.launcher.internal.Activator.RegisterLauncherCustomizer; import com.redhat.thermostat.testutils.StubBundleContext; @@ -92,10 +86,8 @@ @PrepareForTest({Activator.class, Activator.RegisterLauncherCustomizer.class, FrameworkUtil.class}) public class ActivatorTest { - private BundleContext context; + private StubBundleContext context; private MultipleServiceTracker tracker; - private ServiceReference registryServiceReference, helpCommandReference; - private ServiceRegistration launcherServiceRegistration, helpCommandRegistration; private BundleManager registryService; private Command helpCommand; @@ -104,25 +96,16 @@ Path tempDir = createStubThermostatHome(); System.setProperty("THERMOSTAT_HOME", tempDir.toString()); - context = mock(BundleContext.class); + context = new StubBundleContext(); setupOsgiRegistryImplMock(); - registryServiceReference = mock(ServiceReference.class); - launcherServiceRegistration = mock(ServiceRegistration.class); registryService = mock(BundleManager.class); - when(context.getServiceReference(eq(BundleManager.class))).thenReturn(registryServiceReference); - when(context.getService(eq(registryServiceReference))).thenReturn(registryService); - when(context.registerService(eq(Launcher.class.getName()), any(), (Dictionary) isNull())). - thenReturn(launcherServiceRegistration); + context.registerService(BundleManager.class, registryService, null); - helpCommandRegistration = mock(ServiceRegistration.class); - helpCommandReference = mock(ServiceReference.class); helpCommand = mock(Command.class); - when(helpCommandRegistration.getReference()).thenReturn(helpCommandReference); - when(context.registerService(eq(Command.class.getName()), any(), isA(Dictionary.class))). - thenReturn(helpCommandRegistration); - when(context.getService(helpCommandReference)).thenReturn(helpCommand); - when(context.getServiceReferences(Command.class.getName(), null)).thenReturn(new ServiceReference[] {helpCommandReference}); + Hashtable props = new Hashtable<>(); + props.put(Command.NAME, "help"); + context.registerService(Command.class, helpCommand, props); Configuration config = mock(Configuration.class); when(config.getThermostatHome()).thenReturn(""); @@ -164,9 +147,7 @@ Activator activator = new Activator(); activator.start(context); - Hashtable props = new Hashtable<>(); - props.put(ServiceRegistry.SERVICE_NAME, "help"); - verify(context).registerService(eq(Command.class.getName()), isA(HelpCommand.class), eq(props)); + assertCommandIsRegistered(context, "help", HelpCommand.class); verify(mockTracker).open(); @@ -239,9 +220,8 @@ when(FrameworkUtil.getBundle(BundleManagerImpl.class)).thenReturn(mockBundle); when(mockBundle.getBundleContext()).thenReturn(context); Bundle mockFramework = mock(Framework.class); - when(context.getBundle(0)).thenReturn(mockFramework); + context.setBundle(0, mockFramework); when(mockFramework.getBundleContext()).thenReturn(context); - when(context.getBundles()).thenReturn(new Bundle[0]); } } diff -r c8d427902c6b -r f0f0027e0724 launcher/src/test/java/com/redhat/thermostat/launcher/internal/BuiltInCommandInfoSourceTest.java --- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/BuiltInCommandInfoSourceTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/BuiltInCommandInfoSourceTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -50,7 +50,6 @@ import org.junit.Before; import org.junit.Test; -import com.redhat.thermostat.common.cli.CommandInfo; import com.redhat.thermostat.launcher.internal.BuiltInCommandInfoSource; public class BuiltInCommandInfoSourceTest { diff -r c8d427902c6b -r f0f0027e0724 launcher/src/test/java/com/redhat/thermostat/launcher/internal/BundleManagerImplTest.java --- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/BundleManagerImplTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/BundleManagerImplTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -58,8 +58,6 @@ import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; -import com.redhat.thermostat.common.cli.CommandInfo; -import com.redhat.thermostat.common.cli.CommandInfoSource; import com.redhat.thermostat.common.config.Configuration; @RunWith(PowerMockRunner.class) @@ -123,12 +121,7 @@ when(FrameworkUtil.getBundle(any(Class.class))).thenReturn(theBundle); BundleManagerImpl registry = new BundleManagerImpl(conf); - CommandInfoSource infos = mock(CommandInfoSource.class); - CommandInfo info = mock(CommandInfo.class); - when (info.getDependencyResourceNames()).thenReturn(bundleLocs); - when (infos.getCommandInfo(cmdName)).thenReturn(info); - registry.setCommandInfoSource(infos); - registry.addBundlesFor(cmdName); + registry.addBundles(bundleLocs); verify(loader).installAndStartBundles(any(Framework.class), eq(locationsNeeded)); } @@ -147,7 +140,7 @@ when(FrameworkUtil.getBundle(any(Class.class))).thenReturn(theBundle); Object registry = new BundleManagerImpl(conf); - Class clazz = registry.getClass(); + Class clazz = registry.getClass(); Method m = clazz.getMethod("setPrintOSGiInfo", Boolean.TYPE); m.invoke(registry, true); // If this fails, then API has changed in ways that break FrameworkProvider. } diff -r c8d427902c6b -r f0f0027e0724 launcher/src/test/java/com/redhat/thermostat/launcher/internal/CommandSourceTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/CommandSourceTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -0,0 +1,70 @@ +package com.redhat.thermostat.launcher.internal; + +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.mockito.Mockito.mock; + +import java.util.Hashtable; + +import org.junit.Before; +import org.junit.Test; +import org.osgi.framework.InvalidSyntaxException; +import org.osgi.framework.ServiceRegistration; + +import com.redhat.thermostat.common.cli.Command; +import com.redhat.thermostat.testutils.StubBundleContext; + +public class CommandSourceTest { + + private CommandSource commandSource; + + private StubBundleContext bundleContext; + private CommandInfoSource infoSource; + + @Before + public void setUp() { + bundleContext = new StubBundleContext(); + commandSource = new CommandSource(bundleContext); + + infoSource = mock(CommandInfoSource.class); + + bundleContext.registerService(CommandInfoSource.class, infoSource, null); + } + + @Test + public void testGetNotRegisteredCommand() throws InvalidSyntaxException { + Command result = commandSource.getCommand("test1"); + + assertNull(result); + } + + @Test + public void testGetCommandAndInfo() throws InvalidSyntaxException { + Command cmd = mock(Command.class); + registerCommand("test", cmd); + + Command result = commandSource.getCommand("test1"); + + assertSame(cmd, result); + } + + @Test + public void testDoubleRegisteredCommand() throws InvalidSyntaxException { + Command cmd1 = mock(Command.class); + Command cmd2 = mock(Command.class); + + registerCommand("test1", cmd1); + registerCommand("test1", cmd2); + + Command cmd = commandSource.getCommand("test1"); + + assertSame(cmd1, cmd); + } + + private ServiceRegistration registerCommand(String name, Command cmd) { + Hashtable props = new Hashtable<>(); + props.put(Command.NAME, "test1"); + return bundleContext.registerService(Command.class, cmd, props); + } + + } diff -r c8d427902c6b -r f0f0027e0724 launcher/src/test/java/com/redhat/thermostat/launcher/internal/CompoundCommandInfoSourceTest.java --- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/CompoundCommandInfoSourceTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/CompoundCommandInfoSourceTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -49,9 +49,6 @@ import org.junit.Before; import org.junit.Test; -import com.redhat.thermostat.common.cli.CommandInfo; -import com.redhat.thermostat.common.cli.CommandInfoNotFoundException; -import com.redhat.thermostat.common.cli.CommandInfoSource; public class CompoundCommandInfoSourceTest { @@ -149,6 +146,6 @@ when(source2.getCommandInfos()).thenReturn(Arrays.asList(cmdInfo21, cmdInfo22)); Collection results = compoundSource.getCommandInfos(); - + assertEquals(3, results.size()); } } diff -r c8d427902c6b -r f0f0027e0724 launcher/src/test/java/com/redhat/thermostat/launcher/internal/HelpCommandTest.java --- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/HelpCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/HelpCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -38,7 +38,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -52,9 +51,6 @@ import org.junit.Test; import com.redhat.thermostat.common.cli.Arguments; -import com.redhat.thermostat.common.cli.CommandInfo; -import com.redhat.thermostat.common.cli.CommandInfoNotFoundException; -import com.redhat.thermostat.common.cli.CommandInfoSource; import com.redhat.thermostat.common.cli.SimpleArguments; import com.redhat.thermostat.test.TestCommandContextFactory; @@ -71,19 +67,6 @@ } @Test - public void verifyName() { - HelpCommand cmd = new HelpCommand(); - assertEquals("help", cmd.getName()); - } - - @Test - public void verifyDescAndUsage() { - HelpCommand cmd = new HelpCommand(); - assertNotNull(cmd.getDescription()); - assertNotNull(cmd.getUsage()); - } - - @Test public void verifyCommandIsAvailableEverywhere() { HelpCommand cmd = new HelpCommand(); assertTrue(cmd.isAvailableInShell()); diff -r c8d427902c6b -r f0f0027e0724 launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java --- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -40,16 +40,12 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.anyString; -import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.io.IOException; -import java.security.Permission; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.concurrent.ExecutorService; import java.util.logging.Level; @@ -62,8 +58,6 @@ import org.junit.BeforeClass; import org.junit.Test; import org.mockito.ArgumentCaptor; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import org.osgi.framework.Bundle; import org.osgi.framework.BundleException; @@ -77,13 +71,10 @@ import com.redhat.thermostat.common.cli.Command; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; -import com.redhat.thermostat.common.cli.CommandInfo; -import com.redhat.thermostat.common.cli.CommandInfoNotFoundException; -import com.redhat.thermostat.common.cli.CommandInfoSource; +import com.redhat.thermostat.common.cli.CommandRegistry; import com.redhat.thermostat.common.config.ClientPreferences; import com.redhat.thermostat.common.tools.ApplicationState; import com.redhat.thermostat.launcher.BundleManager; -import com.redhat.thermostat.launcher.TestCommand; import com.redhat.thermostat.launcher.internal.DisallowSystemExitSecurityManager.ExitException; import com.redhat.thermostat.launcher.internal.HelpCommand; import com.redhat.thermostat.launcher.internal.LauncherImpl; @@ -157,7 +148,7 @@ public void setUp() throws CommandInfoNotFoundException, BundleException, IOException { setupCommandContextFactory(); - TestCommand cmd1 = new TestCommand(name1, new TestCmd1()); + TestCommand cmd1 = new TestCommand(new TestCmd1()); CommandInfo info1 = mock(CommandInfo.class); when(info1.getName()).thenReturn(name1); when(info1.getUsage()).thenReturn(name1 + " <--arg1 > [--arg2 ]"); @@ -171,11 +162,10 @@ // option is properly set up Option logLevel = new Option("l", "logLevel", true, null); options1.addOption(logLevel); - cmd1.addOptions(opt1, opt2, logLevel); - cmd1.setDescription("description 1"); when(info1.getDescription()).thenReturn("description 1"); when(info1.getOptions()).thenReturn(options1); - TestCommand cmd2 = new TestCommand("test2", new TestCmd2()); + + TestCommand cmd2 = new TestCommand(new TestCmd2()); CommandInfo info2 = mock(CommandInfo.class); when(info2.getName()).thenReturn(name2); Options options2 = new Options(); @@ -183,33 +173,28 @@ options2.addOption(opt3); Option opt4 = new Option(null, "arg4", true, null); options2.addOption(opt4); - cmd2.addOptions(opt3, opt4); - cmd2.setDescription("description 2"); when(info2.getDescription()).thenReturn("description 2"); when(info2.getOptions()).thenReturn(options2); - TestCommand cmd3 = new TestCommand(name3); + TestCommand cmd3 = new TestCommand(); CommandInfo info3 = mock(CommandInfo.class); when(info3.getName()).thenReturn(name3); cmd3.setStorageRequired(true); - cmd3.setDescription("description 3"); when(info3.getDescription()).thenReturn("description 3"); when(info3.getOptions()).thenReturn(new Options()); AbstractStateNotifyingCommand basicCmd = mock(AbstractStateNotifyingCommand.class); CommandInfo basicInfo = mock(CommandInfo.class); - when(basicCmd.getName()).thenReturn("basic"); when(basicInfo.getName()).thenReturn("basic"); - when(basicCmd.getDescription()).thenReturn("nothing that means anything"); when(basicInfo.getDescription()).thenReturn("nothing that means anything"); when(basicCmd.isStorageRequired()).thenReturn(false); when(basicCmd.isAvailableInShell()).thenReturn(true); when(basicCmd.isAvailableOutsideShell()).thenReturn(true); Options options = new Options(); - when(basicCmd.getOptions()).thenReturn(options); when(basicInfo.getOptions()).thenReturn(options); notifier = mock(ActionNotifier.class); when(basicCmd.getNotifier()).thenReturn(notifier); + CommandInfo helpCommandInfo = mock(CommandInfo.class); when(helpCommandInfo.getName()).thenReturn("help"); when(helpCommandInfo.getDescription()).thenReturn("print help information"); @@ -219,9 +204,15 @@ HelpCommand helpCommand = new HelpCommand(); - ctxFactory.getCommandRegistry().registerCommands(Arrays.asList(helpCommand, cmd1, cmd2, cmd3, basicCmd)); + CommandRegistry reg = ctxFactory.getCommandRegistry(); + reg.registerCommand("help", helpCommand); + reg.registerCommand(name1, cmd1); + reg.registerCommand(name2, cmd2); + reg.registerCommand(name3, cmd3); + reg.registerCommand("basic", basicCmd); infos = mock(CommandInfoSource.class); + bundleContext.registerService(CommandInfoSource.class, infos, null); when(infos.getCommandInfo(name1)).thenReturn(info1); when(infos.getCommandInfo(name2)).thenReturn(info2); when(infos.getCommandInfo(name3)).thenReturn(info3); @@ -239,15 +230,6 @@ helpCommand.setCommandInfoSource(infos); registry = mock(BundleManager.class); - doAnswer(new Answer() { - @Override - public Void answer(InvocationOnMock invocation) throws Throwable { - // simulate the real BundleManager which tries to find a CommandInfo - // needed to propagate/handle exceptions properly - infos.getCommandInfo((String) invocation.getArguments()[0]); - return null; - } - }).when(registry).addBundlesFor(anyString()); timerFactory = new TestTimerFactory(); ExecutorService exec = mock(ExecutorService.class); @@ -260,7 +242,7 @@ dbServiceFactory = mock(DbServiceFactory.class); version = mock(Version.class); - launcher = new LauncherImpl(bundleContext, ctxFactory, registry, loggingInitializer, dbServiceFactory, version); + launcher = new LauncherImpl(bundleContext, ctxFactory, registry, infos, new CommandSource(bundleContext), loggingInitializer, dbServiceFactory, version); Keyring keyring = mock(Keyring.class); launcher.setPreferences(new ClientPreferences(keyring)); @@ -395,7 +377,6 @@ @Test public void testCommandInfoNotFound() throws CommandInfoNotFoundException, BundleException, IOException { when(infos.getCommandInfo("foo")).thenThrow(new CommandInfoNotFoundException("foo")); - doThrow(new CommandInfoNotFoundException("foo")).when(registry).addBundlesFor("foo"); String expected = "unknown command 'foo'\n" + "list of commands:\n\n" @@ -409,7 +390,7 @@ @Test public void testMainExceptionInCommand() { - TestCommand errorCmd = new TestCommand("error", new TestCommand.Handle() { + TestCommand errorCmd = new TestCommand(new TestCommand.Handle() { @Override public void run(CommandContext ctx) throws CommandException { @@ -417,7 +398,11 @@ } }); - ctxFactory.getCommandRegistry().registerCommands(Arrays.asList(errorCmd)); + ctxFactory.getCommandRegistry().registerCommand("error", errorCmd); + CommandInfo cmdInfo = mock(CommandInfo.class); + when(cmdInfo.getName()).thenReturn("error"); + when(cmdInfo.getOptions()).thenReturn(new Options()); + when(infos.getCommandInfo("error")).thenReturn(cmdInfo); wrappedRun(launcher, new String[] { "error" }, false); assertEquals("test error\n", ctxFactory.getError()); @@ -461,15 +446,17 @@ @Test public void verifyDbServiceConnectIsCalledForStorageCommand() throws Exception { Command mockCmd = mock(Command.class); - when(mockCmd.getName()).thenReturn("dummy"); when(mockCmd.isStorageRequired()).thenReturn(true); - Options options = mock(Options.class); - when(mockCmd.getOptions()).thenReturn(options); when(mockCmd.isAvailableInShell()).thenReturn(true); when(mockCmd.isAvailableOutsideShell()).thenReturn(true); - ctxFactory.getCommandRegistry().registerCommand(mockCmd); + ctxFactory.getCommandRegistry().registerCommand("dummy", mockCmd); + CommandInfo cmdInfo = mock(CommandInfo.class); + when(cmdInfo.getName()).thenReturn("dummy"); + when(cmdInfo.getOptions()).thenReturn(new Options()); + when(infos.getCommandInfo("dummy")).thenReturn(cmdInfo); + DbService dbService = mock(DbService.class); when(dbServiceFactory.createDbService(anyString(), anyString(), anyString())).thenReturn(dbService); @@ -551,14 +538,16 @@ private void runWithShellStatus(boolean isInShell, String cmdName, boolean isAvailableInShell, boolean isAvailableOutsideShell, String expected) { Command mockCmd = mock(Command.class); - when(mockCmd.getName()).thenReturn(cmdName); when(mockCmd.isStorageRequired()).thenReturn(false); - Options options = mock(Options.class); - when(mockCmd.getOptions()).thenReturn(options); when(mockCmd.isAvailableInShell()).thenReturn(isAvailableInShell); when(mockCmd.isAvailableOutsideShell()).thenReturn(isAvailableOutsideShell); - ctxFactory.getCommandRegistry().registerCommand(mockCmd); + CommandInfo cmdInfo = mock(CommandInfo.class); + when(cmdInfo.getName()).thenReturn(cmdName); + when(cmdInfo.getOptions()).thenReturn(new Options()); + when(infos.getCommandInfo(cmdName)).thenReturn(cmdInfo); + + ctxFactory.getCommandRegistry().registerCommand(cmdName, mockCmd); runAndVerifyCommand(new String[] { cmdName }, expected, isInShell); } } diff -r c8d427902c6b -r f0f0027e0724 launcher/src/test/java/com/redhat/thermostat/launcher/internal/PluginCommandInfoSourceTest.java --- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/PluginCommandInfoSourceTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/PluginCommandInfoSourceTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -61,8 +61,6 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; -import com.redhat.thermostat.common.cli.CommandInfo; -import com.redhat.thermostat.common.cli.CommandInfoNotFoundException; import com.redhat.thermostat.launcher.internal.PluginConfiguration.CommandExtensions; import com.redhat.thermostat.launcher.internal.PluginConfiguration.NewCommand; diff -r c8d427902c6b -r f0f0027e0724 launcher/src/test/java/com/redhat/thermostat/launcher/internal/TestCommand.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/TestCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -0,0 +1,100 @@ +/* + * Copyright 2012, 2013 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 + * . + * + * 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.launcher.internal; + + +import com.redhat.thermostat.common.cli.Command; +import com.redhat.thermostat.common.cli.CommandContext; +import com.redhat.thermostat.common.cli.CommandException; + + +public class TestCommand implements Command { + + private Handle handle; + + private boolean storageRequired; + private boolean availableInShell = true; + private boolean availableOutsideShell = true; + + + public static interface Handle { + public void run(CommandContext ctx) throws CommandException; + } + + public TestCommand() { + this(null); + } + + public TestCommand(Handle r) { + this.handle = r; + } + + @Override + public void run(CommandContext ctx) throws CommandException { + if (handle != null) { + handle.run(ctx); + } + } + + @Override + public boolean isStorageRequired() { + return storageRequired; + } + + public void setStorageRequired(boolean storageRequired) { + this.storageRequired = storageRequired; + } + + @Override + public boolean isAvailableInShell() { + return availableInShell; + } + + void setAvailableInShell(boolean available) { + this.availableInShell = available; + } + + @Override + public boolean isAvailableOutsideShell() { + return availableOutsideShell; + } + + void setAvailableOutsideShell(boolean available) { + this.availableOutsideShell = available; + } +} + diff -r c8d427902c6b -r f0f0027e0724 launcher/src/test/java/com/redhat/thermostat/launcher/internal/TestCommandInfo.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/TestCommandInfo.java Wed Apr 17 14:34:38 2013 -0400 @@ -0,0 +1,96 @@ +/* + * Copyright 2012, 2013 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 + * . + * + * 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.launcher.internal; + +import java.util.Collections; +import java.util.List; + +import org.apache.commons.cli.Option; +import org.apache.commons.cli.Options; + +public class TestCommandInfo implements CommandInfo { + + private String name; + private String description; + private String usage; + + private Options options = new Options(); + + public TestCommandInfo(String name) { + this.name = name; + } + + @Override + public String getName() { + return name; + } + + @Override + public String getDescription() { + return description; + } + + public void setDescription(String desc) { + description = desc; + } + + @Override + public String getUsage() { + return usage; + } + + public void setUsage(String usage) { + this.usage = usage; + } + + @Override + public Options getOptions() { + return options; + } + + public void addOptions(Option... options) { + for (Option option : options) { + this.options.addOption(option); + } + } + + @Override + public List getDependencyResourceNames() { + return Collections.emptyList(); + } + +} diff -r c8d427902c6b -r f0f0027e0724 main/src/main/java/com/redhat/thermostat/main/Thermostat.java --- a/main/src/main/java/com/redhat/thermostat/main/Thermostat.java Mon Apr 15 15:45:04 2013 +0200 +++ b/main/src/main/java/com/redhat/thermostat/main/Thermostat.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,33 +36,22 @@ package com.redhat.thermostat.main; -import java.io.FileNotFoundException; -import java.io.IOException; -import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Arrays; import java.util.Iterator; import java.util.List; -import org.osgi.framework.BundleContext; -import org.osgi.framework.BundleException; -import org.osgi.util.tracker.ServiceTracker; - -import com.redhat.thermostat.common.Launcher; import com.redhat.thermostat.common.config.Configuration; import com.redhat.thermostat.main.impl.FrameworkProvider; public class Thermostat { - private static Configuration config; + /** + * @param args the arguments to the program + */ + public static void main(String[] args) { - /** - * @param args - * @throws Exception - */ - public static void main(String[] args) throws Exception { - - config = new Configuration(); + Configuration config = new Configuration(); List toProcess = new ArrayList<>(Arrays.asList(args)); Iterator iter = toProcess.iterator(); @@ -76,10 +65,7 @@ FrameworkProvider frameworkProvider = new FrameworkProvider(config); frameworkProvider.start(toProcess.toArray(new String[0])); - } - - } diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/Activator.java --- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/Activator.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/Activator.java Wed Apr 17 14:34:38 2013 -0400 @@ -36,8 +36,6 @@ package com.redhat.thermostat.vm.heap.analysis.command.internal; -import java.util.ServiceLoader; - import org.osgi.framework.BundleActivator; import org.osgi.framework.BundleContext; @@ -52,9 +50,18 @@ @Override public void start(final BundleContext context) throws Exception { reg = new CommandRegistryImpl(context); - ServiceLoader cmds = ServiceLoader.load(Command.class, - getClass().getClassLoader()); - reg.registerCommands(cmds); + + registerCommand("dump-heap", new DumpHeapCommand()); + registerCommand("list-heap-dumps", new ListHeapDumpsCommand()); + registerCommand("save-heap-dump-to-file", new SaveHeapDumpToFileCommand()); + registerCommand("show-heap-histogram", new ShowHeapHistogramCommand()); + registerCommand("find-objects", new FindObjectsCommand()); + registerCommand("object-info", new ObjectInfoCommand()); + registerCommand("find-root", new FindRootCommand()); + } + + private void registerCommand(String name, Command command) { + reg.registerCommand(name, command); } @Override diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommand.java --- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -55,7 +55,6 @@ public class DumpHeapCommand extends AbstractCommand { private static final Translate translator = LocaleResources.createLocalizer(); - private static final String NAME = "dump-heap"; private BundleContext context; private final DumpHeapHelper implementation; @@ -70,11 +69,6 @@ } @Override - public String getName() { - return NAME; - } - - @Override public void run(final CommandContext ctx) throws CommandException { final HostVMArguments args = new HostVMArguments(ctx.getArguments()); diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindObjectsCommand.java --- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindObjectsCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindObjectsCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -60,7 +60,6 @@ private static final String HEAP_ID_ARG = "heapId"; private static final String LIMIT_ARG = "limit"; - private static final String NAME = "find-objects"; private static final String HEADER_OBJECT_ID = translator.localize(LocaleResources.HEADER_OBJECT_ID); private static final String HEADER_TYPE = translator.localize(LocaleResources.HEADER_OBJECT_TYPE); private static final int DEFAULT_LIMIT = 10; @@ -139,10 +138,5 @@ } return limit; } - - @Override - public String getName() { - return NAME; - } } diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindRootCommand.java --- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindRootCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindRootCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -62,7 +62,6 @@ private static final Translate translator = LocaleResources.createLocalizer(); private static final String ALL_ARG = "all"; - private static final String NAME = "find-root"; private BundleContext context; @@ -140,10 +139,5 @@ } } - @Override - public String getName() { - return NAME; - } - } diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommand.java --- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -62,8 +62,6 @@ private static final Translate translator = LocaleResources.createLocalizer(); - private static final String NAME = "list-heap-dumps"; - private static final String[] COLUMN_NAMES = { translator.localize(LocaleResources.HEADER_HOST_ID), translator.localize(LocaleResources.HEADER_VM_ID), @@ -83,11 +81,6 @@ } @Override - public String getName() { - return NAME; - } - - @Override public void run(CommandContext ctx) throws CommandException { HostVMArguments args = new HostVMArguments(ctx.getArguments(), false, false); diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ObjectInfoCommand.java --- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ObjectInfoCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ObjectInfoCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -61,8 +61,6 @@ private static final Translate translator = LocaleResources.createLocalizer(); - private static final String NAME = "object-info"; - private final BundleContext context; private Snapshot snapshot; @@ -142,10 +140,5 @@ return "[" + from.describeReferenceTo(to, snapshot) + "]"; } - @Override - public String getName() { - return NAME; - } - } diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/SaveHeapDumpToFileCommand.java --- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/SaveHeapDumpToFileCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/SaveHeapDumpToFileCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -62,8 +62,6 @@ private static final Translate translator = LocaleResources.createLocalizer(); - private static final String NAME = "save-heap-dump-to-file"; - private static final String HEAP_ID_ARGUMENT = "heapId"; private static final String FILE_NAME_ARGUMENT = "file"; @@ -80,12 +78,6 @@ } @Override - public String getName() { - return NAME; - } - - @Override - public void run(CommandContext ctx) throws CommandException { ServiceReference ref = context.getServiceReference(HeapDAO.class.getName()); if (ref == null) { diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ShowHeapHistogramCommand.java --- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ShowHeapHistogramCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ShowHeapHistogramCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -58,8 +58,6 @@ private static final Translate translator = LocaleResources.createLocalizer(); - private static final String NAME = "show-heap-histogram"; - private final BundleContext context; public ShowHeapHistogramCommand() { @@ -71,11 +69,6 @@ } @Override - public String getName() { - return NAME; - } - - @Override public void run(CommandContext ctx) throws CommandException { ServiceReference ref = context.getServiceReference(HeapDAO.class.getName()); if (ref == null) { diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/main/resources/META-INF/services/com.redhat.thermostat.common.cli.Command --- a/vm-heap-analysis/command/src/main/resources/META-INF/services/com.redhat.thermostat.common.cli.Command Mon Apr 15 15:45:04 2013 +0200 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,7 +0,0 @@ -com.redhat.thermostat.vm.heap.analysis.command.internal.DumpHeapCommand -com.redhat.thermostat.vm.heap.analysis.command.internal.ListHeapDumpsCommand -com.redhat.thermostat.vm.heap.analysis.command.internal.SaveHeapDumpToFileCommand -com.redhat.thermostat.vm.heap.analysis.command.internal.ShowHeapHistogramCommand -com.redhat.thermostat.vm.heap.analysis.command.internal.FindObjectsCommand -com.redhat.thermostat.vm.heap.analysis.command.internal.ObjectInfoCommand -com.redhat.thermostat.vm.heap.analysis.command.internal.FindRootCommand \ No newline at end of file diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ActivatorTest.java --- a/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ActivatorTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ActivatorTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -1,5 +1,5 @@ /* - * Copyright 2012, 2013 Red Hat, Inc. + * Copyright 201trin2, 2013 Red Hat, Inc. * * This file is part of Thermostat. * @@ -36,23 +36,21 @@ package com.redhat.thermostat.vm.heap.analysis.command.internal; +import static com.redhat.thermostat.testutils.Asserts.assertCommandIsRegistered; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import static org.mockito.Mockito.mock; import org.junit.Test; import org.junit.runner.RunWith; import org.osgi.framework.Bundle; -import org.osgi.framework.Filter; import org.osgi.framework.FrameworkUtil; import org.osgi.framework.InvalidSyntaxException; import org.powermock.api.mockito.PowerMockito; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; -import com.redhat.thermostat.common.cli.Command; import com.redhat.thermostat.testutils.StubBundleContext; @RunWith(PowerMockRunner.class) @@ -63,42 +61,39 @@ public void testCommandsRegistered() throws Exception { StubBundleContext ctx = new StubBundleContext(); - makeServiceLoaderHappy(ctx); + makeConstructorsHappy(ctx); Activator activator = new Activator(); activator.start(ctx); - - assertTrue(ctx.isServiceRegistered(Command.class.getName(), DumpHeapCommand.class)); - assertTrue(ctx.isServiceRegistered(Command.class.getName(), FindObjectsCommand.class)); - assertTrue(ctx.isServiceRegistered(Command.class.getName(), FindRootCommand.class)); - assertTrue(ctx.isServiceRegistered(Command.class.getName(), ListHeapDumpsCommand.class)); - assertTrue(ctx.isServiceRegistered(Command.class.getName(), ObjectInfoCommand.class)); - assertTrue(ctx.isServiceRegistered(Command.class.getName(), SaveHeapDumpToFileCommand.class)); - assertTrue(ctx.isServiceRegistered(Command.class.getName(), ShowHeapHistogramCommand.class)); + + assertCommandIsRegistered(ctx, "dump-heap", DumpHeapCommand.class); + assertCommandIsRegistered(ctx, "find-objects", FindObjectsCommand.class); + assertCommandIsRegistered(ctx, "find-root", FindRootCommand.class); + assertCommandIsRegistered(ctx, "list-heap-dumps", ListHeapDumpsCommand.class); + assertCommandIsRegistered(ctx, "object-info", ObjectInfoCommand.class); + assertCommandIsRegistered(ctx, "save-heap-dump-to-file", SaveHeapDumpToFileCommand.class); + assertCommandIsRegistered(ctx, "show-heap-histogram", ShowHeapHistogramCommand.class); activator.stop(ctx); assertEquals(0, ctx.getAllServices().size()); } - private void makeServiceLoaderHappy(StubBundleContext ctx) { + private void makeConstructorsHappy(StubBundleContext ctx) { // Commands' no-arg constructors use FrameworkUtil to get - // the bundle context. This results in NPEs when ServiceLoader - // attempts to load Command classes. Note that client-cli is - // a dep of this bundle and hence ServiceLoader wants to instantiate - // Command classes from there too. + // the bundle context. This results in NPEs when those Command classes + // are created PowerMockito.mockStatic(FrameworkUtil.class); Bundle mockBundle = mock(Bundle.class); when(FrameworkUtil.getBundle(any(Class.class))).thenReturn(mockBundle); when(mockBundle.getBundleContext()).thenReturn(ctx); - Filter mockFilter = mock(Filter.class); - // StubBundleContext.createFilter() returns null if FrameworkUtil is - // mocked, so mock the offending static call too. + // StubBundleContext.createFilter() needs if FrameworkUtil.createFilter + // to work, so fix that. try { - when(FrameworkUtil.createFilter(any(String.class))).thenReturn(mockFilter); + when(FrameworkUtil.createFilter(any(String.class))).thenCallRealMethod(); } catch (InvalidSyntaxException e) { - // ignored + throw new AssertionError("mocked method threw invalid syntax exception"); } } } diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommandTest.java --- a/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -37,7 +37,6 @@ package com.redhat.thermostat.vm.heap.analysis.command.internal; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; @@ -53,7 +52,6 @@ import org.mockito.stubbing.Answer; import com.redhat.thermostat.client.command.RequestQueue; -import com.redhat.thermostat.common.cli.Command; import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.cli.SimpleArguments; import com.redhat.thermostat.common.locale.Translate; @@ -69,17 +67,6 @@ .createLocalizer(); @Test - public void testBasics() { - StubBundleContext context = new StubBundleContext(); - DumpHeapHelper impl = mock(DumpHeapHelper.class); - - Command command = new DumpHeapCommand(context, impl); - assertEquals("dump-heap", command.getName()); - assertNotNull(command.getDescription()); - assertNotNull(command.getUsage()); - } - - @Test public void verifyAcuallyCallsWorker() throws CommandException { AgentInfoDAO agentInfoDao = mock(AgentInfoDAO.class); RequestQueue queue = mock(RequestQueue.class); diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindObjectsCommandTest.java --- a/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindObjectsCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindObjectsCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -37,8 +37,6 @@ package com.redhat.thermostat.vm.heap.analysis.command.internal; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.isA; import static org.mockito.Mockito.mock; @@ -46,10 +44,7 @@ import java.util.Arrays; -import org.apache.commons.cli.Option; -import org.apache.commons.cli.Options; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import com.redhat.thermostat.common.cli.CommandException; @@ -119,36 +114,6 @@ } @Test - public void testName() { - assertEquals("find-objects", cmd.getName()); - } - - @Test - public void testDescAndUsage() { - assertNotNull(cmd.getDescription()); - assertNotNull(cmd.getUsage()); - } - - @Ignore - @Test - public void testOptions() { - Options options = cmd.getOptions(); - assertEquals(2, options.getOptions().size()); - - assertTrue(options.hasOption("heapId")); - Option heapOption = options.getOption("heapId"); - assertEquals("the ID of the heapdump to analyze", heapOption.getDescription()); - assertTrue(heapOption.isRequired()); - assertTrue(heapOption.hasArg()); - - assertTrue(options.hasOption("limit")); - Option limitOption = options.getOption("limit"); - assertEquals("limit search to top N results, defaults to 10", limitOption.getDescription()); - assertFalse(limitOption.isRequired()); - assertTrue(limitOption.hasArg()); - } - - @Test public void testStorageRequired() { assertTrue(cmd.isStorageRequired()); } diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindRootCommandTest.java --- a/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindRootCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindRootCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -37,8 +37,6 @@ package com.redhat.thermostat.vm.heap.analysis.command.internal; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; @@ -46,14 +44,10 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import java.util.Collection; import java.util.Enumeration; -import org.apache.commons.cli.Option; -import org.apache.commons.cli.Options; import org.junit.After; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.mockito.stubbing.OngoingStubbing; @@ -183,47 +177,6 @@ } @Test - public void testName() { - assertEquals("find-root", cmd.getName()); - } - - @Test - public void testDescAndUsage() { - assertNotNull(cmd.getDescription()); - assertNotNull(cmd.getUsage()); - } - - @Ignore - @Test - public void testOptions() { - String heapIdOption = "heapId"; - String objectIdOption = "objectId"; - String allOption = "all"; - Options options = cmd.getOptions(); - @SuppressWarnings("unchecked") - Collection theOptions = options.getOptions(); - assertEquals(3, theOptions.size()); - - assertTrue(options.hasOption(heapIdOption)); - Option heapOption = options.getOption(heapIdOption); - assertEquals("the ID of the heapdump to analyze", heapOption.getDescription()); - assertTrue(heapOption.isRequired()); - assertTrue(heapOption.hasArg()); - - assertTrue(options.hasOption(objectIdOption)); - Option objectOption = options.getOption(objectIdOption); - assertEquals("the ID of the object to query", objectOption.getDescription()); - assertTrue(heapOption.isRequired()); - assertTrue(heapOption.hasArg()); - - assertTrue(options.hasOption(allOption)); - Option all = options.getOption(allOption); - assertEquals("finds all paths to GC roots", all.getDescription()); - assertFalse(all.isRequired()); - assertFalse(all.hasArg()); - } - - @Test public void testStorageRequired() { assertTrue(cmd.isStorageRequired()); } diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommandTest.java --- a/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -37,7 +37,6 @@ package com.redhat.thermostat.vm.heap.analysis.command.internal; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; import static org.mockito.Matchers.isA; import static org.mockito.Mockito.mock; @@ -47,10 +46,8 @@ import java.util.Calendar; import java.util.TimeZone; -import org.apache.commons.cli.Options; import org.junit.AfterClass; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; import com.redhat.thermostat.common.cli.Command; @@ -81,25 +78,6 @@ } @Test - public void verifyBasics() { - StubBundleContext context = new StubBundleContext(); - Command command = new ListHeapDumpsCommand(context); - assertEquals("list-heap-dumps", command.getName()); - assertNotNull(command.getDescription()); - assertNotNull(command.getUsage()); - } - - @Ignore - @Test - public void verifyOptions() { - StubBundleContext context = new StubBundleContext(); - Command command = new ListHeapDumpsCommand(context); - Options options = command.getOptions(); - assertNotNull(options); - assertEquals(2, options.getOptions().size()); - } - - @Test public void verifyFailsWithoutHostDao() throws Exception { StubBundleContext context = new StubBundleContext(); Command command = new ListHeapDumpsCommand(context); diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ObjectInfoCommandTest.java --- a/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ObjectInfoCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ObjectInfoCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -37,7 +37,6 @@ package com.redhat.thermostat.vm.heap.analysis.command.internal; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; @@ -48,11 +47,8 @@ import java.util.Enumeration; -import org.apache.commons.cli.Option; -import org.apache.commons.cli.Options; import org.junit.After; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -147,42 +143,6 @@ } @Test - public void testName() { - StubBundleContext context = new StubBundleContext(); - cmd = new ObjectInfoCommand(context); - assertEquals("object-info", cmd.getName()); - } - - @Test - public void testDescAndUsage() { - StubBundleContext context = new StubBundleContext(); - cmd = new ObjectInfoCommand(context); - assertNotNull(cmd.getDescription()); - assertNotNull(cmd.getUsage()); - } - - @Ignore - @Test - public void testOptions() { - StubBundleContext context = new StubBundleContext(); - cmd = new ObjectInfoCommand(context); - Options options = cmd.getOptions(); - assertEquals(2, options.getOptions().size()); - - assertTrue(options.hasOption("heapId")); - Option heapOption = options.getOption("heapId"); - assertEquals("the ID of the heapdump to analyze", heapOption.getDescription()); - assertTrue(heapOption.isRequired()); - assertTrue(heapOption.hasArg()); - - assertTrue(options.hasOption("objectId")); - Option objOption = options.getOption("objectId"); - assertEquals("the ID of the object to query", objOption.getDescription()); - assertTrue(objOption.isRequired()); - assertTrue(objOption.hasArg()); - } - - @Test public void testStorageRequired() { StubBundleContext context = new StubBundleContext(); cmd = new ObjectInfoCommand(context); diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/SaveHeapDumpToFileCommandTest.java --- a/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/SaveHeapDumpToFileCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/SaveHeapDumpToFileCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -38,7 +38,6 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -73,14 +72,6 @@ context = new StubBundleContext(); } - @Test - public void verifyBasicInformation() { - Command command = new SaveHeapDumpToFileCommand(context, mock(FileStreamCreator.class)); - assertEquals("save-heap-dump-to-file", command.getName()); - assertNotNull(command.getDescription()); - assertNotNull(command.getUsage()); - } - @Test (expected=CommandException.class) public void verifyMissingHeapIdThrowsException() throws CommandException { TestCommandContextFactory factory = new TestCommandContextFactory(); diff -r c8d427902c6b -r f0f0027e0724 vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ShowHeapHistogramCommandTest.java --- a/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ShowHeapHistogramCommandTest.java Mon Apr 15 15:45:04 2013 +0200 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ShowHeapHistogramCommandTest.java Wed Apr 17 14:34:38 2013 -0400 @@ -37,7 +37,6 @@ package com.redhat.thermostat.vm.heap.analysis.command.internal; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; @@ -63,16 +62,6 @@ private static final Translate translator = LocaleResources.createLocalizer(); @Test - public void verifyBasics() { - StubBundleContext context = new StubBundleContext(); - Command command = new ShowHeapHistogramCommand(context); - - assertEquals("show-heap-histogram", command.getName()); - assertNotNull(command.getDescription()); - assertNotNull(command.getUsage()); - } - - @Test public void verifyWorks() throws CommandException { ObjectHistogram histo = new ObjectHistogram(); diff -r c8d427902c6b -r f0f0027e0724 web/cmd/src/main/java/com/redhat/thermostat/web/cmd/Activator.java --- a/web/cmd/src/main/java/com/redhat/thermostat/web/cmd/Activator.java Mon Apr 15 15:45:04 2013 +0200 +++ b/web/cmd/src/main/java/com/redhat/thermostat/web/cmd/Activator.java Wed Apr 17 14:34:38 2013 -0400 @@ -37,8 +37,6 @@ package com.redhat.thermostat.web.cmd; -import java.util.Arrays; - import org.osgi.framework.BundleActivator; import org.osgi.framework.BundleContext; @@ -52,7 +50,7 @@ @Override public void start(BundleContext context) throws Exception { reg = new CommandRegistryImpl(context); - reg.registerCommands(Arrays.asList(new WebServiceCommand())); + reg.registerCommand("webservice", new WebServiceCommand()); } @Override diff -r c8d427902c6b -r f0f0027e0724 web/cmd/src/main/java/com/redhat/thermostat/web/cmd/WebServiceCommand.java --- a/web/cmd/src/main/java/com/redhat/thermostat/web/cmd/WebServiceCommand.java Mon Apr 15 15:45:04 2013 +0200 +++ b/web/cmd/src/main/java/com/redhat/thermostat/web/cmd/WebServiceCommand.java Wed Apr 17 14:34:38 2013 -0400 @@ -85,11 +85,6 @@ } @Override - public String getName() { - return "webservice"; - } - - @Override public boolean isStorageRequired() { return false; }