# HG changeset patch # User Elliott Baron # Date 1363968095 14400 # Node ID 3526e4183714ca694572aa929476918fe41dd90d # Parent c6c9f2c7961ce81dff2a424a269bd51c45dc821b Remove OSGIUtils This commit removes OSGIUtils. This class duplicated a lot of functionality from BundleContext, and hides some important details about OSGi such as where the underlying BundleContext comes from. For instance, when developing the Eclipse client, at first the common-core bundle wasn't started. This lead to NPEs in OSGIUtils because it uses the BundleContext of the common-core bundle and not the bundle of the calling class or the bundle containing the requested service. Another problem with OSGIUtils is from several cases in Thermostat where we are using getService instead of getServiceAllowNull, which throws NPEs if the requested service is not registered. Uses of OSGIUtils have been replaced with either a ServiceTracker for long-running tasks (e.g. GUI), or for shell commands using a BundleContext and checking the ServiceReference for null and throwing a meaningful exception. Reviewed-by: jerboaa, vanaltj, neugens Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-March/006104.html diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/ServiceCommand.java Fri Mar 22 12:01:35 2013 -0400 @@ -40,18 +40,21 @@ import java.util.List; import java.util.concurrent.Semaphore; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; +import org.osgi.framework.ServiceReference; + import com.redhat.thermostat.agent.cli.impl.db.StorageAlreadyRunningException; import com.redhat.thermostat.agent.cli.impl.db.StorageCommand; import com.redhat.thermostat.agent.cli.impl.locale.LocaleResources; import com.redhat.thermostat.common.ActionEvent; import com.redhat.thermostat.common.ActionListener; import com.redhat.thermostat.common.Launcher; +import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; -import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.locale.Translate; import com.redhat.thermostat.common.tools.ApplicationState; -import com.redhat.thermostat.common.utils.OSGIUtils; /** * Simple service that allows starting Agent and DB Backend @@ -64,24 +67,37 @@ private static final String NAME = "service"; private List> listeners; - private Semaphore agentBarrier = new Semaphore(0); + private BundleContext context; + private Launcher launcher; public ServiceCommand() { + this(FrameworkUtil.getBundle(ServiceCommand.class).getBundleContext()); + } + + public ServiceCommand(BundleContext context) { + this.context = context; listeners = new ArrayList<>(); listeners.add(this); } @Override public void run(CommandContext ctx) throws CommandException { - Launcher launcher = getLauncher(); + ServiceReference launcherRef = context.getServiceReference(Launcher.class); + if (launcherRef == null) { + throw new CommandException(translator.localize(LocaleResources.LAUNCHER_UNAVAILABLE)); + } + launcher = (Launcher) context.getService(launcherRef); String[] storageStartArgs = new String[] { "storage", "--start" }; launcher.setArgs(storageStartArgs); launcher.run(listeners, false); agentBarrier.acquireUninterruptibly(); + String[] storageStopArgs = new String[] { "storage", "--stop" }; launcher.setArgs(storageStopArgs); launcher.run(false); + + context.ungetService(launcherRef); } @Override @@ -95,7 +111,6 @@ switch (actionEvent.getActionId()) { case START: String dbUrl = storage.getConfiguration().getDBConnectionString(); - Launcher launcher = getLauncher(); String[] agentArgs = new String[] {"agent", "-d", dbUrl}; System.err.println(translator.localize(LocaleResources.STARTING_AGENT)); launcher.setArgs(agentArgs); @@ -128,10 +143,5 @@ return false; } - private Launcher getLauncher() { - OSGIUtils osgi = OSGIUtils.getInstance(); - return osgi.getService(Launcher.class); - } - } diff -r c6c9f2c7961c -r 3526e4183714 agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/locale/LocaleResources.java --- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/locale/LocaleResources.java Fri Mar 22 09:24:14 2013 +0100 +++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/locale/LocaleResources.java Fri Mar 22 12:01:35 2013 -0400 @@ -56,7 +56,8 @@ STARTING_STORAGE_SERVER, CANNOT_EXECUTE_PROCESS, SERVER_LISTENING_ON, - PID_IS; + PID_IS, + LAUNCHER_UNAVAILABLE; static final String RESOURCE_BUNDLE = "com.redhat.thermostat.agent.cli.impl.strings"; diff -r c6c9f2c7961c -r 3526e4183714 agent/cli/src/main/resources/com/redhat/thermostat/agent/cli/impl/strings.properties --- a/agent/cli/src/main/resources/com/redhat/thermostat/agent/cli/impl/strings.properties Fri Mar 22 09:24:14 2013 +0100 +++ b/agent/cli/src/main/resources/com/redhat/thermostat/agent/cli/impl/strings.properties Fri Mar 22 12:01:35 2013 -0400 @@ -15,3 +15,4 @@ CANNOT_EXECUTE_PROCESS = can not execute {0} process. is it installed? SERVER_LISTENING_ON = server listening on ip: {0} PID_IS = pid: {0} +LAUNCHER_UNAVAILABLE = Launcher is not available diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ActivatorTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -38,6 +38,7 @@ 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; @@ -62,7 +63,7 @@ PowerMockito.mockStatic(FrameworkUtil.class); Bundle mockBundle = mock(Bundle.class); - when(FrameworkUtil.getBundle(StorageCommand.class)).thenReturn(mockBundle); + when(FrameworkUtil.getBundle(any(Class.class))).thenReturn(mockBundle); StubBundleContext bundleContext = new StubBundleContext(); diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ServiceCommandTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -44,7 +44,7 @@ import org.junit.Before; import org.junit.Test; -import com.redhat.thermostat.agent.cli.impl.ServiceCommand; +import com.redhat.thermostat.testutils.StubBundleContext; public class ServiceCommandTest { @@ -52,7 +52,8 @@ @Before public void setUp() { - thermostatService = new ServiceCommand(); + StubBundleContext context = new StubBundleContext(); + thermostatService = new ServiceCommand(context); } @After diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ConnectCommand.java Fri Mar 22 12:01:35 2013 -0400 @@ -36,12 +36,15 @@ package com.redhat.thermostat.client.cli.internal; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; +import org.osgi.framework.ServiceReference; + +import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; -import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.config.ClientPreferences; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.ConnectionException; import com.redhat.thermostat.storage.core.DbService; import com.redhat.thermostat.storage.core.DbServiceFactory; @@ -67,25 +70,36 @@ private static final String NAME = "connect"; private ClientPreferences prefs; + private BundleContext context; private DbServiceFactory dbServiceFactory; public ConnectCommand() { - this(new DbServiceFactory()); + this(FrameworkUtil.getBundle(ConnectCommand.class).getBundleContext(), new DbServiceFactory()); } - - ConnectCommand(DbServiceFactory dbServiceFactory) { + + ConnectCommand(BundleContext context, DbServiceFactory dbServiceFactory) { + this.context = context; this.dbServiceFactory = dbServiceFactory; } @Override public void run(CommandContext ctx) throws CommandException { - DbService service = OSGIUtils.getInstance().getServiceAllowNull(DbService.class); - if (service != null) { + ServiceReference dbServiceRef = context.getServiceReference(DbService.class); + if (dbServiceRef != null) { + DbService service = (DbService) context.getService(dbServiceRef); + String connectionUrl = service.getConnectionUrl(); + context.ungetService(dbServiceRef); // Already connected, bail out - throw new CommandException(translator.localize(LocaleResources.COMMAND_CONNECT_ALREADY_CONNECTED, service.getConnectionUrl())); + throw new CommandException(translator.localize(LocaleResources.COMMAND_CONNECT_ALREADY_CONNECTED, connectionUrl)); } if (prefs == null) { - prefs = new ClientPreferences(OSGIUtils.getInstance().getService(Keyring.class)); + ServiceReference keyringRef = context.getServiceReference(Keyring.class); + if (keyringRef == null) { + throw new CommandException(translator.localize(LocaleResources.COMMAND_CONNECT_NO_KEYRING)); + } + Keyring keyring = (Keyring) context.getService(keyringRef); + prefs = new ClientPreferences(keyring); + context.ungetService(keyringRef); } String dbUrl = ctx.getArguments().getArgument(DB_URL_ARG); if (dbUrl == null) { @@ -95,7 +109,7 @@ String password = ctx.getArguments().getArgument(PASSWORD_ARG); try { // may throw StorageException if storage url is not supported - service = dbServiceFactory.createDbService(username, password, dbUrl); + DbService service = dbServiceFactory.createDbService(username, password, dbUrl); service.connect(); } catch (StorageException ex) { throw new CommandException(translator.localize(LocaleResources.COMMAND_CONNECT_INVALID_STORAGE, dbUrl)); diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/DisconnectCommand.java Fri Mar 22 12:01:35 2013 -0400 @@ -37,12 +37,14 @@ 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; +import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; -import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.ConnectionException; import com.redhat.thermostat.storage.core.DbService; @@ -52,17 +54,31 @@ private static final String NAME = "disconnect"; + private BundleContext context; + + public DisconnectCommand() { + this(FrameworkUtil.getBundle(DisconnectCommand.class).getBundleContext()); + } + + DisconnectCommand(BundleContext context) { + this.context = context; + } + @Override public void run(CommandContext ctx) throws CommandException { - DbService service = OSGIUtils.getInstance().getServiceAllowNull(DbService.class); - if (service == null) { + ServiceReference ref = context.getServiceReference(DbService.class.getName()); + if (ref == null) { // not connected throw new CommandException(translator.localize(LocaleResources.COMMAND_DISCONNECT_NOT_CONNECTED)); } + try { + DbService service = (DbService) context.getService(ref); service.disconnect(); } catch (ConnectionException e) { throw new CommandException(translator.localize(LocaleResources.COMMAND_DISCONNECT_ERROR)); + } finally { + context.ungetService(ref); } } diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/ListVMsCommand.java Fri Mar 22 12:01:35 2013 -0400 @@ -38,11 +38,14 @@ import java.util.Collection; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; +import org.osgi.framework.ServiceReference; + import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.HostInfoDAO; @@ -55,30 +58,32 @@ private static final String NAME = "list-vms"; - private final OSGIUtils serviceProvider; + private final BundleContext context; public ListVMsCommand() { - this(OSGIUtils.getInstance()); + this(FrameworkUtil.getBundle(ListVMsCommand.class).getBundleContext()); } - ListVMsCommand(OSGIUtils serviceProvider) { - this.serviceProvider = serviceProvider; + ListVMsCommand(BundleContext context) { + this.context = context; } @Override public void run(CommandContext ctx) throws CommandException { - HostInfoDAO hostsDAO = serviceProvider.getServiceAllowNull(HostInfoDAO.class); - if (hostsDAO == null) { + ServiceReference hostsDAORef = context.getServiceReference(HostInfoDAO.class.getName()); + if (hostsDAORef == null) { throw new CommandException(translator.localize(LocaleResources.HOST_SERVICE_UNAVAILABLE)); } + HostInfoDAO hostsDAO = (HostInfoDAO) context.getService(hostsDAORef); Collection hosts = hostsDAO.getHosts(); - serviceProvider.ungetService(HostInfoDAO.class, hostsDAO); + context.ungetService(hostsDAORef); - VmInfoDAO vmsDAO = serviceProvider.getServiceAllowNull(VmInfoDAO.class); - if (vmsDAO == null) { + ServiceReference vmsDAORef = context.getServiceReference(VmInfoDAO.class.getName()); + if (vmsDAORef == null) { throw new CommandException(translator.localize(LocaleResources.VM_SERVICE_UNAVAILABLE)); } + VmInfoDAO vmsDAO = (VmInfoDAO) context.getService(vmsDAORef); VMListFormatter formatter = new VMListFormatter(); for (HostRef host : hosts) { Collection vms = vmsDAO.getVMs(host); @@ -88,8 +93,7 @@ } } formatter.format(ctx.getConsole().getOutput()); - - serviceProvider.ungetService(VmInfoDAO.class, vmsDAO); + context.ungetService(vmsDAORef); } @Override diff -r c6c9f2c7961c -r 3526e4183714 client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/LocaleResources.java --- a/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/LocaleResources.java Fri Mar 22 09:24:14 2013 +0100 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/LocaleResources.java Fri Mar 22 12:01:35 2013 -0400 @@ -50,6 +50,7 @@ COMMAND_CONNECT_ALREADY_CONNECTED, COMMAND_CONNECT_FAILED_TO_CONNECT, COMMAND_CONNECT_INVALID_STORAGE, + COMMAND_CONNECT_NO_KEYRING, COMMAND_CONNECT_ERROR, COMMAND_DISCONNECT_NOT_CONNECTED, diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/client/cli/src/main/java/com/redhat/thermostat/client/cli/internal/VMInfoCommand.java Fri Mar 22 12:01:35 2013 -0400 @@ -40,13 +40,16 @@ import java.util.Collection; import java.util.Date; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; +import org.osgi.framework.ServiceReference; + import com.redhat.thermostat.client.cli.HostVMArguments; import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.cli.TableRenderer; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.DAOException; @@ -60,23 +63,24 @@ private static final String NAME = "vm-info"; private static final String STILL_ALIVE = translator.localize(LocaleResources.VM_STOP_TIME_RUNNING); - private OSGIUtils serviceProvider; + private final BundleContext context; public VMInfoCommand() { - this(OSGIUtils.getInstance()); + this(FrameworkUtil.getBundle(VMInfoCommand.class).getBundleContext()); } /** For tests only */ - VMInfoCommand(OSGIUtils serviceProvider) { - this.serviceProvider = serviceProvider; + VMInfoCommand(BundleContext context) { + this.context = context; } @Override public void run(CommandContext ctx) throws CommandException { - VmInfoDAO vmsDAO = serviceProvider.getServiceAllowNull(VmInfoDAO.class); - if (vmsDAO == null) { + ServiceReference vmsDAORef = context.getServiceReference(VmInfoDAO.class.getName()); + if (vmsDAORef == null) { throw new CommandException(translator.localize(LocaleResources.VM_SERVICE_UNAVAILABLE)); } + VmInfoDAO vmsDAO = (VmInfoDAO) context.getService(vmsDAORef); HostVMArguments hostVMArgs = new HostVMArguments(ctx.getArguments(), true, false); HostRef host = hostVMArgs.getHost(); @@ -91,7 +95,7 @@ } catch (DAOException ex) { ctx.getConsole().getError().println(ex.getMessage()); } finally { - serviceProvider.ungetService(VmInfoDAO.class, vmsDAO); + context.ungetService(vmsDAORef); } } diff -r c6c9f2c7961c -r 3526e4183714 client/cli/src/main/resources/com/redhat/thermostat/client/cli/strings.properties --- a/client/cli/src/main/resources/com/redhat/thermostat/client/cli/strings.properties Fri Mar 22 09:24:14 2013 +0100 +++ b/client/cli/src/main/resources/com/redhat/thermostat/client/cli/strings.properties Fri Mar 22 12:01:35 2013 -0400 @@ -8,6 +8,7 @@ COMMAND_CONNECT_ALREADY_CONNECTED = Already connected to storage: URL = {0}\nPlease use disconnect command to disconnect. COMMAND_CONNECT_FAILED_TO_CONNECT = Could not connect to db {0} COMMAND_CONNECT_INVALID_STORAGE = Unrecognized storage URL {0} +COMMAND_CONNECT_NO_KEYRING = Unable to retrieve keyring COMMAND_CONNECT_ERROR = Error: {0} COMMAND_DISCONNECT_NOT_CONNECTED = Not connected to storage. You may use the connect command for establishing connections. diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ActivatorTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -38,6 +38,7 @@ 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; import static org.mockito.Mockito.when; @@ -59,12 +60,10 @@ @Test public void testCommandsRegistered() throws Exception { - // Need to mock FrameworkUtil to avoid NPE in ShellCommand and - // VMStatCommand's no-arg constructors + // Need to mock FrameworkUtil to avoid NPE in commands' no-arg constructors PowerMockito.mockStatic(FrameworkUtil.class); Bundle mockBundle = mock(Bundle.class); - when(FrameworkUtil.getBundle(ShellCommand.class)).thenReturn(mockBundle); - when(FrameworkUtil.getBundle(VMStatCommand.class)).thenReturn(mockBundle); + when(FrameworkUtil.getBundle(any(Class.class))).thenReturn(mockBundle); // When we call createFilter, we need a real return value when(FrameworkUtil.createFilter(anyString())).thenCallRealMethod(); diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ConnectCommandTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -40,36 +40,34 @@ 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; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; import org.osgi.framework.Bundle; import org.osgi.framework.BundleContext; -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.CommandContext; import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.cli.SimpleArguments; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.DbService; import com.redhat.thermostat.storage.core.DbServiceFactory; import com.redhat.thermostat.test.TestCommandContextFactory; +import com.redhat.thermostat.testutils.StubBundleContext; +import com.redhat.thermostat.utils.keyring.Keyring; -@RunWith(PowerMockRunner.class) -@PrepareForTest({ OSGIUtils.class, DbServiceFactory.class }) public class ConnectCommandTest { private static final Translate translator = LocaleResources.createLocalizer(); + private StubBundleContext context; private ConnectCommand cmd; private TestCommandContextFactory cmdCtxFactory; private BundleContext bundleContext; @@ -79,8 +77,9 @@ public void setUp() { setupCommandContextFactory(); + context = new StubBundleContext(); dbServiceFactory = mock(DbServiceFactory.class); - cmd = new ConnectCommand(dbServiceFactory); + cmd = new ConnectCommand(context, dbServiceFactory); } private void setupCommandContextFactory() { @@ -99,13 +98,12 @@ @Test public void verifyConnectedThrowsExceptionWithDiagnosticMessage() { + Keyring keyring = mock(Keyring.class); + context.registerService(Keyring.class, keyring, null); String dbUrl = "fluff"; DbService dbService = mock(DbService.class); - OSGIUtils utils = mock(OSGIUtils.class); - PowerMockito.mockStatic(OSGIUtils.class); - when(OSGIUtils.getInstance()).thenReturn(utils); - when(utils.getServiceAllowNull(DbService.class)).thenReturn(dbService); when(dbService.getConnectionUrl()).thenReturn(dbUrl); + context.registerService(DbService.class, dbService, null); SimpleArguments args = new SimpleArguments(); args.addArgument("--dbUrl", dbUrl); @@ -118,11 +116,8 @@ @Test public void verifyNotConnectedConnects() throws CommandException { - OSGIUtils utils = mock(OSGIUtils.class); - PowerMockito.mockStatic(OSGIUtils.class); - when(OSGIUtils.getInstance()).thenReturn(utils); - when(utils.getServiceAllowNull(DbService.class)).thenReturn(null); - + Keyring keyring = mock(Keyring.class); + context.registerService(Keyring.class, keyring, null); DbService dbService = mock(DbService.class); String username = "testuser"; @@ -139,6 +134,29 @@ } @Test + public void verifyNoKeyring() throws CommandException { + DbService dbService = mock(DbService.class); + + String username = "testuser"; + String password = "testpassword"; + String dbUrl = "mongodb://10.23.122.1:12578"; + SimpleArguments args = new SimpleArguments(); + args.addArgument("dbUrl", dbUrl); + args.addArgument("username", username); + args.addArgument("password", password); + CommandContext ctx = cmdCtxFactory.createContext(args); + + try { + cmd.run(ctx); + fail(); + } catch (CommandException e) { + assertEquals(translator.localize(LocaleResources.COMMAND_CONNECT_NO_KEYRING), e.getMessage()); + } + + verify(dbService, never()).connect(); + } + + @Test public void testIsStorageRequired() { assertFalse(cmd.isStorageRequired()); } diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/DisconnectCommandTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -43,51 +43,34 @@ import static org.junit.Assert.fail; 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; -import org.junit.runner.RunWith; -import org.osgi.framework.Bundle; -import org.osgi.framework.BundleContext; -import org.osgi.framework.FrameworkUtil; -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.CommandContext; import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.cli.SimpleArguments; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.DbService; import com.redhat.thermostat.test.TestCommandContextFactory; +import com.redhat.thermostat.testutils.StubBundleContext; -@RunWith(PowerMockRunner.class) -@PrepareForTest({ OSGIUtils.class, FrameworkUtil.class }) public class DisconnectCommandTest { private static final Translate translator = LocaleResources.createLocalizer(); + private StubBundleContext context; private DisconnectCommand cmd; private TestCommandContextFactory cmdCtxFactory; - private BundleContext bundleContext; @Before public void setUp() { - setupCommandContextFactory(); - - cmd = new DisconnectCommand(); - - } + context = new StubBundleContext(); + cmdCtxFactory = new TestCommandContextFactory(context); - private void setupCommandContextFactory() { - Bundle sysBundle = mock(Bundle.class); - bundleContext = mock(BundleContext.class); - when(bundleContext.getBundle(0)).thenReturn(sysBundle); - cmdCtxFactory = new TestCommandContextFactory(bundleContext); + cmd = new DisconnectCommand(context); } @After @@ -98,11 +81,6 @@ @Test public void verifyNotConnectedThrowsException() { - OSGIUtils utils = mock(OSGIUtils.class); - PowerMockito.mockStatic(OSGIUtils.class); - when(OSGIUtils.getInstance()).thenReturn(utils); - when(utils.getServiceAllowNull(DbService.class)).thenReturn(null); - try { cmd.run(cmdCtxFactory.createContext(new SimpleArguments())); fail("cmd.run() should have thrown exception."); @@ -114,10 +92,7 @@ @Test public void verifyConnectedDisconnects() throws CommandException { DbService dbService = mock(DbService.class); - OSGIUtils utils = mock(OSGIUtils.class); - PowerMockito.mockStatic(OSGIUtils.class); - when(OSGIUtils.getInstance()).thenReturn(utils); - when(utils.getServiceAllowNull(DbService.class)).thenReturn(dbService); + context.registerService(DbService.class, dbService, null); CommandContext ctx = cmdCtxFactory.createContext(new SimpleArguments()); cmd.run(ctx); diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ListVMsCommandTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -38,6 +38,7 @@ 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; import static org.mockito.Mockito.mock; @@ -53,34 +54,34 @@ import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.cli.SimpleArguments; -import com.redhat.thermostat.common.utils.OSGIUtils; +import com.redhat.thermostat.common.locale.Translate; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.HostInfoDAO; import com.redhat.thermostat.storage.dao.VmInfoDAO; import com.redhat.thermostat.storage.model.VmInfo; import com.redhat.thermostat.test.TestCommandContextFactory; +import com.redhat.thermostat.testutils.StubBundleContext; public class ListVMsCommandTest { + + private static final Translate translator = LocaleResources.createLocalizer(); private ListVMsCommand cmd; private TestCommandContextFactory cmdCtxFactory; private HostInfoDAO hostsDAO; private VmInfoDAO vmsDAO; - private OSGIUtils serviceProvider; + private StubBundleContext context; @Before public void setUp() { setupCommandContextFactory(); - serviceProvider = mock(OSGIUtils.class); + context = new StubBundleContext(); - cmd = new ListVMsCommand(serviceProvider); + cmd = new ListVMsCommand(context); hostsDAO = mock(HostInfoDAO.class); vmsDAO = mock(VmInfoDAO.class); - - when(serviceProvider.getServiceAllowNull(HostInfoDAO.class)).thenReturn(hostsDAO); - when(serviceProvider.getServiceAllowNull(VmInfoDAO.class)).thenReturn(vmsDAO); } private void setupCommandContextFactory() { @@ -97,6 +98,8 @@ @Test public void verifyOutputFormatOneLine() throws CommandException { + context.registerService(HostInfoDAO.class, hostsDAO, null); + context.registerService(VmInfoDAO.class, vmsDAO, null); HostRef host1 = new HostRef("123", "h1"); VmRef vm1 = new VmRef(host1, 1, "n"); @@ -118,6 +121,8 @@ @Test public void verifyOutputFormatMultiLines() throws CommandException { + context.registerService(HostInfoDAO.class, hostsDAO, null); + context.registerService(VmInfoDAO.class, vmsDAO, null); HostRef host1 = new HostRef("123", "h1"); HostRef host2 = new HostRef("456", "longhostname"); @@ -146,6 +151,38 @@ "123 h1 2 EXITED n1\n" + "456 longhostname 123456 EXITED longvmname\n", output); } + + @Test + public void testNeedHostInfoDAO() throws CommandException { + context.registerService(VmInfoDAO.class, vmsDAO, null); + + SimpleArguments args = new SimpleArguments(); + args.addArgument("--dbUrl", "fluff"); + CommandContext ctx = cmdCtxFactory.createContext(args); + + try { + cmd.run(ctx); + fail(); + } catch (CommandException e) { + assertEquals(translator.localize(LocaleResources.HOST_SERVICE_UNAVAILABLE), e.getMessage()); + } + } + + @Test + public void testNeedVmInfoDAO() throws CommandException { + context.registerService(HostInfoDAO.class, hostsDAO, null); + + SimpleArguments args = new SimpleArguments(); + args.addArgument("--dbUrl", "fluff"); + CommandContext ctx = cmdCtxFactory.createContext(args); + + try { + cmd.run(ctx); + fail(); + } catch (CommandException e) { + assertEquals(translator.localize(LocaleResources.VM_SERVICE_UNAVAILABLE), e.getMessage()); + } + } @Test public void testName() { diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/VMInfoCommandTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -40,6 +40,7 @@ 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.when; @@ -58,7 +59,7 @@ import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.cli.SimpleArguments; -import com.redhat.thermostat.common.utils.OSGIUtils; +import com.redhat.thermostat.common.locale.Translate; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.DAOException; @@ -66,9 +67,11 @@ import com.redhat.thermostat.storage.model.VmInfo; import com.redhat.thermostat.test.Bug; import com.redhat.thermostat.test.TestCommandContextFactory; +import com.redhat.thermostat.testutils.StubBundleContext; public class VMInfoCommandTest { - + + private static final Translate translator = LocaleResources.createLocalizer(); private static TimeZone defaultTimezone; @BeforeClass @@ -86,18 +89,14 @@ private VmInfoDAO vmsDAO; private TestCommandContextFactory cmdCtxFactory; private VmRef vm; + private StubBundleContext context; @Before public void setUp() { + context = new StubBundleContext(); setupCommandContextFactory(); vmsDAO = mock(VmInfoDAO.class); - - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(VmInfoDAO.class)).thenReturn(vmsDAO); - - cmd = new VMInfoCommand(serviceProvider); - setupDAOs(); } @@ -121,6 +120,8 @@ @Test public void testVmInfo() throws CommandException { + context.registerService(VmInfoDAO.class, vmsDAO, null); + cmd = new VMInfoCommand(context); SimpleArguments args = new SimpleArguments(); args.addArgument("vmId", "234"); args.addArgument("hostId", "123"); @@ -135,9 +136,26 @@ "VM arguments: vmArguments\n"; assertEquals(expected, cmdCtxFactory.getOutput()); } + + @Test + public void testNoVmInfoDAO() throws CommandException { + cmd = new VMInfoCommand(context); + SimpleArguments args = new SimpleArguments(); + args.addArgument("vmId", "234"); + args.addArgument("hostId", "123"); + + try { + cmd.run(cmdCtxFactory.createContext(args)); + fail(); + } catch (CommandException e) { + assertEquals(translator.localize(LocaleResources.VM_SERVICE_UNAVAILABLE), e.getMessage()); + } + } @Test public void testAllVmInfoForHost() throws CommandException { + context.registerService(VmInfoDAO.class, vmsDAO, null); + cmd = new VMInfoCommand(context); SimpleArguments args = new SimpleArguments(); args.addArgument("hostId", "123"); cmd.run(cmdCtxFactory.createContext(args)); @@ -154,6 +172,8 @@ @Test public void testVmInfoUnknownVM() throws CommandException { + context.registerService(VmInfoDAO.class, vmsDAO, null); + cmd = new VMInfoCommand(context); SimpleArguments args = new SimpleArguments(); args.addArgument("vmId", "9876"); args.addArgument("hostId", "123"); @@ -165,6 +185,8 @@ @Test public void testVmInfoNonNumericalVMID() throws CommandException { + context.registerService(VmInfoDAO.class, vmsDAO, null); + cmd = new VMInfoCommand(context); SimpleArguments args = new SimpleArguments(); args.addArgument("vmId", "fluff"); args.addArgument("hostId", "123"); @@ -178,6 +200,8 @@ @Test public void testName() { + context.registerService(VmInfoDAO.class, vmsDAO, null); + cmd = new VMInfoCommand(context); assertEquals("vm-info", cmd.getName()); } @@ -186,6 +210,8 @@ url="http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1046") @Test public void testStopTime() throws CommandException { + context.registerService(VmInfoDAO.class, vmsDAO, null); + cmd = new VMInfoCommand(context); Calendar start = Calendar.getInstance(); start.set(2012, 5, 7, 15, 32, 0); VmInfo vmInfo = new VmInfo(234, start.getTimeInMillis(), Long.MIN_VALUE, "vmVersion", "javaHome", "mainClass", "commandLine", "vmName", "vmInfo", "vmVersion", "vmArguments", new HashMap(), new HashMap(), new String[0]); @@ -208,6 +234,8 @@ @Test public void testDescAndUsage() { + context.registerService(VmInfoDAO.class, vmsDAO, null); + cmd = new VMInfoCommand(context); assertNotNull(cmd.getDescription()); assertNotNull(cmd.getUsage()); } @@ -215,6 +243,8 @@ @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()); @@ -234,6 +264,8 @@ @Test public void testStorageRequired() { + context.registerService(VmInfoDAO.class, vmsDAO, null); + cmd = new VMInfoCommand(context); assertTrue(cmd.isStorageRequired()); } } diff -r c6c9f2c7961c -r 3526e4183714 client/command/pom.xml --- a/client/command/pom.xml Fri Mar 22 09:24:14 2013 +0100 +++ b/client/command/pom.xml Fri Mar 22 12:01:35 2013 -0400 @@ -101,6 +101,12 @@ thermostat-common-command ${project.version} + + com.redhat.thermostat + thermostat-common-test + ${project.version} + test + diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/client/command/src/main/java/com/redhat/thermostat/client/command/cli/PingCommand.java Fri Mar 22 12:01:35 2013 -0400 @@ -41,6 +41,10 @@ import java.util.List; import java.util.concurrent.Semaphore; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; +import org.osgi.framework.ServiceReference; + import com.redhat.thermostat.client.command.RequestQueue; import com.redhat.thermostat.client.command.internal.LocaleResources; import com.redhat.thermostat.common.cli.Arguments; @@ -52,7 +56,6 @@ import com.redhat.thermostat.common.command.RequestResponseListener; import com.redhat.thermostat.common.command.Response; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.dao.AgentInfoDAO; import com.redhat.thermostat.storage.dao.HostInfoDAO; @@ -95,14 +98,14 @@ } - private OSGIUtils serviceProvider; + private final BundleContext context; public PingCommand() { - this(OSGIUtils.getInstance()); + this(FrameworkUtil.getBundle(PingCommand.class).getBundleContext()); } - public PingCommand(OSGIUtils serviceProvider) { - this.serviceProvider = serviceProvider; + public PingCommand(BundleContext context) { + this.context = context; } @Override @@ -114,23 +117,25 @@ return; } - HostInfoDAO hostInfoDao = serviceProvider.getServiceAllowNull(HostInfoDAO.class); - if (hostInfoDao == null) { + ServiceReference hostInfoDaoRef = context.getServiceReference(HostInfoDAO.class.getName()); + if (hostInfoDaoRef == null) { throw new CommandException(translator.localize(LocaleResources.COMMAND_PING_NO_HOST_INFO_DAO)); } + HostInfoDAO hostInfoDao = (HostInfoDAO) context.getService(hostInfoDaoRef); HostRef targetHostRef = getHostRef(hostInfoDao, agentId); - serviceProvider.ungetService(HostInfoDAO.class, hostInfoDao); + context.ungetService(hostInfoDaoRef); if (targetHostRef == null) { printCustomMessageWithUsage(out, translator.localize(LocaleResources.COMMAND_PING_INVALID_HOST_ID)); return; } - AgentInfoDAO agentInfoDao = serviceProvider.getService(AgentInfoDAO.class); - if (agentInfoDao == null) { + ServiceReference agentInfoDaoRef = context.getServiceReference(AgentInfoDAO.class.getName()); + if (agentInfoDaoRef == null) { throw new CommandException(translator.localize(LocaleResources.COMMAND_PING_NO_AGENT_INFO_DAO)); } + AgentInfoDAO agentInfoDao = (AgentInfoDAO) context.getService(agentInfoDaoRef); String address = agentInfoDao.getAgentInformation(targetHostRef).getConfigListenAddress(); - serviceProvider.ungetService(AgentInfoDAO.class, agentInfoDao); + context.ungetService(agentInfoDaoRef); String [] host = address.split(":"); InetSocketAddress target = new InetSocketAddress(host[0], Integer.parseInt(host[1])); @@ -139,9 +144,14 @@ final Semaphore responseBarrier = new Semaphore(0); ping.addListener(new PongListener(out, responseBarrier)); - RequestQueue queue = OSGIUtils.getInstance().getService(RequestQueue.class); + ServiceReference queueRef = context.getServiceReference(RequestQueue.class.getName()); + if (queueRef == null) { + throw new CommandException(translator.localize(LocaleResources.COMMAND_PING_NO_REQUEST_QUEUE)); + } + RequestQueue queue = (RequestQueue) context.getService(queueRef); out.println(translator.localize(LocaleResources.COMMAND_PING_QUEUING_REQUEST, target.toString())); queue.putRequest(ping); + context.ungetService(queueRef); try { responseBarrier.acquire(); } catch (InterruptedException e) { diff -r c6c9f2c7961c -r 3526e4183714 client/command/src/main/java/com/redhat/thermostat/client/command/internal/LocaleResources.java --- a/client/command/src/main/java/com/redhat/thermostat/client/command/internal/LocaleResources.java Fri Mar 22 09:24:14 2013 +0100 +++ b/client/command/src/main/java/com/redhat/thermostat/client/command/internal/LocaleResources.java Fri Mar 22 12:01:35 2013 -0400 @@ -47,6 +47,7 @@ COMMAND_PING_INVALID_HOST_ID, COMMAND_PING_NO_HOST_INFO_DAO, COMMAND_PING_NO_AGENT_INFO_DAO, + COMMAND_PING_NO_REQUEST_QUEUE, COMMAND_PING_RESPONSE_ERROR, COMMAND_PING_RESPONSE_OK, diff -r c6c9f2c7961c -r 3526e4183714 client/command/src/main/resources/com/redhat/thermostat/client/command/internal/strings.properties --- a/client/command/src/main/resources/com/redhat/thermostat/client/command/internal/strings.properties Fri Mar 22 09:24:14 2013 +0100 +++ b/client/command/src/main/resources/com/redhat/thermostat/client/command/internal/strings.properties Fri Mar 22 12:01:35 2013 -0400 @@ -4,6 +4,7 @@ COMMAND_PING_INVALID_HOST_ID = Invalid host ID or agent no longer running. See \'help list-vms\' to obtain a valid host ID. COMMAND_PING_NO_HOST_INFO_DAO = Unable to access host information: service not available COMMAND_PING_NO_AGENT_INFO_DAO = Unable to access agent information: service not available +COMMAND_PING_NO_REQUEST_QUEUE = Unable to access command request queue: service not available COMMAND_PING_RESPONSE_ERROR = Error received from: {0} COMMAND_PING_RESPONSE_OK = Response received from: {0} diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/client/command/src/test/java/com/redhat/thermostat/client/command/cli/PingCommandTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -38,6 +38,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -47,10 +48,12 @@ import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.cli.SimpleArguments; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.HostRef; +import com.redhat.thermostat.storage.dao.AgentInfoDAO; import com.redhat.thermostat.storage.dao.HostInfoDAO; +import com.redhat.thermostat.storage.model.AgentInformation; import com.redhat.thermostat.test.TestCommandContextFactory; +import com.redhat.thermostat.testutils.StubBundleContext; public class PingCommandTest { @@ -58,16 +61,17 @@ @Test public void testCommandName() { - PingCommand command = new PingCommand(); + StubBundleContext context = new StubBundleContext(); + PingCommand command = new PingCommand(context); assertEquals("ping", command.getName()); } @Test public void testCommandNeedsAgentId() throws CommandException { - OSGIUtils serviceProvider = mock(OSGIUtils.class); + StubBundleContext context = new StubBundleContext(); - PingCommand command = new PingCommand(serviceProvider); + PingCommand command = new PingCommand(context); TestCommandContextFactory factory = new TestCommandContextFactory(); @@ -81,9 +85,9 @@ @Test public void testCommandWithoutHostDao() throws CommandException { - OSGIUtils serviceProvider = mock(OSGIUtils.class); + StubBundleContext context = new StubBundleContext(); - PingCommand command = new PingCommand(serviceProvider); + PingCommand command = new PingCommand(context); TestCommandContextFactory factory = new TestCommandContextFactory(); @@ -106,10 +110,10 @@ HostInfoDAO hostInfoDao = mock(HostInfoDAO.class); when(hostInfoDao.getAliveHosts()).thenReturn(Arrays.asList(host1)); - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(HostInfoDAO.class)).thenReturn(hostInfoDao); + StubBundleContext context = new StubBundleContext(); + context.registerService(HostInfoDAO.class, hostInfoDao, null); - PingCommand command = new PingCommand(serviceProvider); + PingCommand command = new PingCommand(context); TestCommandContextFactory factory = new TestCommandContextFactory(); @@ -123,6 +127,38 @@ assertEquals("Unable to access agent information: service not available", agentDaoServiceMissing.getMessage()); } } + + @Test + public void testCommandWithoutRequestQueue() throws CommandException { + HostRef host1 = mock(HostRef.class); + when(host1.getAgentId()).thenReturn(KNOWN_AGENT_ID); + + HostInfoDAO hostInfoDao = mock(HostInfoDAO.class); + when(hostInfoDao.getAliveHosts()).thenReturn(Arrays.asList(host1)); + + AgentInfoDAO agentInfoDao = mock(AgentInfoDAO.class); + AgentInformation info = mock(AgentInformation.class); + when(info.getConfigListenAddress()).thenReturn("myHost:9001"); + when(agentInfoDao.getAgentInformation(any(HostRef.class))).thenReturn(info); + + StubBundleContext context = new StubBundleContext(); + context.registerService(HostInfoDAO.class, hostInfoDao, null); + context.registerService(AgentInfoDAO.class, agentInfoDao, null); + + PingCommand command = new PingCommand(context); + + TestCommandContextFactory factory = new TestCommandContextFactory(); + + SimpleArguments args = new SimpleArguments(); + args.addNonOptionArgument(KNOWN_AGENT_ID); + + try { + command.run(factory.createContext(args)); + fail("did not throw expected exception"); + } catch (CommandException e) { + assertEquals("Unable to access command request queue: service not available", e.getMessage()); + } + } // TODO add more tests that check the actual behaviour under valid input } diff -r c6c9f2c7961c -r 3526e4183714 client/core/src/main/java/com/redhat/thermostat/client/ui/AgentInformationDisplayModel.java --- a/client/core/src/main/java/com/redhat/thermostat/client/ui/AgentInformationDisplayModel.java Fri Mar 22 09:24:14 2013 +0100 +++ b/client/core/src/main/java/com/redhat/thermostat/client/ui/AgentInformationDisplayModel.java Fri Mar 22 12:01:35 2013 -0400 @@ -42,7 +42,6 @@ import java.util.List; import java.util.Map; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.dao.AgentInfoDAO; import com.redhat.thermostat.storage.dao.BackendInfoDAO; @@ -61,12 +60,6 @@ private final List agents; private final Map> backends; - public AgentInformationDisplayModel() { - this( - OSGIUtils.getInstance().getService(AgentInfoDAO.class), - OSGIUtils.getInstance().getService(BackendInfoDAO.class)); - } - public AgentInformationDisplayModel(AgentInfoDAO agentInfoDao, BackendInfoDAO backendInfoDao) { this.agentInfoDao = agentInfoDao; this.backendInfoDao = backendInfoDao; diff -r c6c9f2c7961c -r 3526e4183714 client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImpl.java --- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImpl.java Fri Mar 22 09:24:14 2013 +0100 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImpl.java Fri Mar 22 12:01:35 2013 -0400 @@ -88,12 +88,13 @@ import com.redhat.thermostat.common.Timer.SchedulingType; import com.redhat.thermostat.common.config.ClientPreferences; import com.redhat.thermostat.common.utils.LoggingUtils; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.DefaultHostsVMsLoader; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.HostsVMsLoader; import com.redhat.thermostat.storage.core.Ref; import com.redhat.thermostat.storage.core.VmRef; +import com.redhat.thermostat.storage.dao.AgentInfoDAO; +import com.redhat.thermostat.storage.dao.BackendInfoDAO; import com.redhat.thermostat.storage.dao.HostInfoDAO; import com.redhat.thermostat.storage.dao.VmInfoDAO; import com.redhat.thermostat.utils.keyring.Keyring; @@ -112,13 +113,18 @@ private Timer backgroundUpdater; private MainView view; + private Keyring keyring; private HostInfoDAO hostInfoDAO; private VmInfoDAO vmInfoDAO; + private AgentInfoDAO agentInfoDAO; + private BackendInfoDAO backendInfoDAO; private SummaryViewProvider summaryViewProvider; private HostInformationViewProvider hostInfoViewProvider; private VmInformationViewProvider vmInfoViewProvider; + private AgentInformationViewProvider agentInfoViewProvider; + private ClientConfigViewProvider clientConfigViewProvider; private ApplicationInfo appInfo; @@ -224,26 +230,41 @@ this.shutdown = shutdown; Class[] deps = new Class[] { + Keyring.class, HostInfoDAO.class, VmInfoDAO.class, + AgentInfoDAO.class, + BackendInfoDAO.class, SummaryViewProvider.class, HostInformationViewProvider.class, - VmInformationViewProvider.class + VmInformationViewProvider.class, + AgentInformationViewProvider.class, + ClientConfigViewProvider.class, }; depTracker = new MultipleServiceTracker(context, deps, new MultipleServiceTracker.Action() { @Override public void dependenciesAvailable(Map services) { + keyring = (Keyring) services.get(Keyring.class.getName()); + Objects.requireNonNull(keyring); hostInfoDAO = (HostInfoDAO) services.get(HostInfoDAO.class.getName()); Objects.requireNonNull(hostInfoDAO); vmInfoDAO = (VmInfoDAO) services.get(VmInfoDAO.class.getName()); Objects.requireNonNull(vmInfoDAO); + agentInfoDAO = (AgentInfoDAO) services.get(AgentInfoDAO.class.getName()); + Objects.requireNonNull(agentInfoDAO); + backendInfoDAO = (BackendInfoDAO) services.get(BackendInfoDAO.class.getName()); + Objects.requireNonNull(backendInfoDAO); summaryViewProvider = (SummaryViewProvider) services.get(SummaryViewProvider.class.getName()); Objects.requireNonNull(summaryViewProvider); hostInfoViewProvider = (HostInformationViewProvider) services.get(HostInformationViewProvider.class.getName()); Objects.requireNonNull(hostInfoViewProvider); vmInfoViewProvider = (VmInformationViewProvider) services.get(VmInformationViewProvider.class.getName()); Objects.requireNonNull(vmInfoViewProvider); + agentInfoViewProvider = (AgentInformationViewProvider) services.get(AgentInformationViewProvider.class.getName()); + Objects.requireNonNull(agentInfoViewProvider); + clientConfigViewProvider = (ClientConfigViewProvider) services.get(ClientConfigViewProvider.class.getName()); + Objects.requireNonNull(clientConfigViewProvider); initView(view); @@ -501,17 +522,15 @@ } private void showAgentConfiguration() { - AgentInformationDisplayModel model = new AgentInformationDisplayModel(); - AgentInformationViewProvider viewProvider = OSGIUtils.getInstance().getService(AgentInformationViewProvider.class); - AgentInformationDisplayView view = viewProvider.createView(); + AgentInformationDisplayModel model = new AgentInformationDisplayModel(agentInfoDAO, backendInfoDAO); + AgentInformationDisplayView view = agentInfoViewProvider.createView(); AgentInformationDisplayController controller = new AgentInformationDisplayController(model, view); controller.showView(); } private void showConfigureClientPreferences() { - ClientPreferences prefs = new ClientPreferences(OSGIUtils.getInstance().getService(Keyring.class)); - ClientConfigViewProvider viewProvider = OSGIUtils.getInstance().getService(ClientConfigViewProvider.class); - ClientConfigurationView view = viewProvider.createView(); + ClientPreferences prefs = new ClientPreferences(keyring); + ClientConfigurationView view = clientConfigViewProvider.createView(); ClientConfigurationController controller = new ClientConfigurationController(prefs, view); controller.showDialog(); } diff -r c6c9f2c7961c -r 3526e4183714 client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImplTest.java --- a/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImplTest.java Fri Mar 22 09:24:14 2013 +0100 +++ b/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/MainWindowControllerImplTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -65,7 +65,9 @@ import org.osgi.framework.BundleException; import com.redhat.thermostat.client.core.Filter; +import com.redhat.thermostat.client.core.views.AgentInformationViewProvider; import com.redhat.thermostat.client.core.views.BasicView; +import com.redhat.thermostat.client.core.views.ClientConfigViewProvider; import com.redhat.thermostat.client.core.views.HostInformationView; import com.redhat.thermostat.client.core.views.HostInformationViewProvider; import com.redhat.thermostat.client.core.views.SummaryView; @@ -91,11 +93,14 @@ import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.HostsVMsLoader; import com.redhat.thermostat.storage.core.VmRef; +import com.redhat.thermostat.storage.dao.AgentInfoDAO; +import com.redhat.thermostat.storage.dao.BackendInfoDAO; import com.redhat.thermostat.storage.dao.HostInfoDAO; import com.redhat.thermostat.storage.dao.VmInfoDAO; import com.redhat.thermostat.storage.model.VmInfo; import com.redhat.thermostat.test.Bug; import com.redhat.thermostat.testutils.StubBundleContext; +import com.redhat.thermostat.utils.keyring.Keyring; public class MainWindowControllerImplTest { @@ -152,11 +157,19 @@ ApplicationService appSvc = mock(ApplicationService.class); when (appSvc.getTimerFactory()).thenReturn(timerFactory); + Keyring keyring = mock(Keyring.class); + context.registerService(Keyring.class, keyring, null); + mockHostsDAO = mock(HostInfoDAO.class); context.registerService(HostInfoDAO.class, mockHostsDAO, null); mockVmsDAO = mock(VmInfoDAO.class); context.registerService(VmInfoDAO.class, mockVmsDAO, null); + AgentInfoDAO agentInfoDAO = mock(AgentInfoDAO.class); + context.registerService(AgentInfoDAO.class, agentInfoDAO, null); + BackendInfoDAO backendInfoDAO = mock(BackendInfoDAO.class); + context.registerService(BackendInfoDAO.class, backendInfoDAO, null); + SummaryViewProvider summaryViewProvider = mock(SummaryViewProvider.class); context.registerService(SummaryViewProvider.class, summaryViewProvider, null); SummaryView summaryView = mock(SummaryView.class); @@ -171,6 +184,11 @@ context.registerService(VmInformationViewProvider.class, vmInfoViewProvider, null); vmInfoView = mock(VmInformationView.class); when(vmInfoViewProvider.createView()).thenReturn(vmInfoView); + + AgentInformationViewProvider agentInfoViewProvider = mock(AgentInformationViewProvider.class); + context.registerService(AgentInformationViewProvider.class, agentInfoViewProvider, null); + ClientConfigViewProvider clientConfigViewProvider = mock(ClientConfigViewProvider.class); + context.registerService(ClientConfigViewProvider.class, clientConfigViewProvider, null); // Setup View view = mock(MainView.class); diff -r c6c9f2c7961c -r 3526e4183714 common/core/src/main/java/com/redhat/thermostat/common/utils/OSGIUtils.java --- a/common/core/src/main/java/com/redhat/thermostat/common/utils/OSGIUtils.java Fri Mar 22 09:24:14 2013 +0100 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,107 +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.utils; - -import java.util.Dictionary; - -import org.osgi.framework.BundleContext; -import org.osgi.framework.FrameworkUtil; -import org.osgi.framework.ServiceReference; -import org.osgi.framework.ServiceRegistration; - -public class OSGIUtils { - - // TODO: Maybe we should stick this singleton into an ApplicationContext? - private static OSGIUtils instance = new OSGIUtils(); - public static OSGIUtils getInstance() { - return instance; - } - - // This is only here to be used by test code. - public static void setInstance(OSGIUtils utils) { - instance = utils; - } - - /** - * Gets a service. If no matching service is available, throw a NullPointerException. - * - * @param serviceType the class for the service - * @return The best matching service implementation for the requested serviceType. - */ - @SuppressWarnings({ "rawtypes", "unchecked" }) - public E getService(Class serviceType) { - BundleContext ctx = FrameworkUtil.getBundle(getClass()).getBundleContext(); - ServiceReference ref = ctx.getServiceReference(serviceType.getName()); - return (E) ctx.getService(ref); - } - - /** - * Gets a service. If no matching service is available, return {@code null}. - * - * @param serviceType the class for the service - * @return The best matching service implementation for the requested serviceType or null. - */ - @SuppressWarnings({ "rawtypes", "unchecked" }) - public E getServiceAllowNull(Class serviceType) { - BundleContext ctx = FrameworkUtil.getBundle(getClass()).getBundleContext(); - ServiceReference ref = ctx.getServiceReference(serviceType.getName()); - if (ref == null) { - return null; - } - return (E) ctx.getService(ref); - } - - public void ungetService(Class serviceType, Object service) { - BundleContext ctx = FrameworkUtil.getBundle(getClass()).getBundleContext(); - // FIXME the reference returned now may be different from a previous invocation - ServiceReference ref = ctx.getServiceReference(serviceType.getName()); - ctx.ungetService(ref); - } - - @SuppressWarnings("rawtypes") - public ServiceRegistration registerService(Class serviceType, E service) { - return registerService(serviceType, service, null); - } - - @SuppressWarnings("rawtypes") - public ServiceRegistration registerService(Class serviceType, E service, Dictionary properties) { - BundleContext ctx = FrameworkUtil.getBundle(getClass()).getBundleContext(); - return ctx.registerService(serviceType.getName(), service, properties); - } - -} - diff -r c6c9f2c7961c -r 3526e4183714 eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/internal/Activator.java --- a/eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/internal/Activator.java Fri Mar 22 09:24:14 2013 +0100 +++ b/eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/internal/Activator.java Fri Mar 22 12:01:35 2013 -0400 @@ -46,7 +46,6 @@ import org.osgi.framework.ServiceReference; import org.osgi.util.tracker.ServiceTracker; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.eclipse.LoggerFacility; import com.redhat.thermostat.eclipse.internal.views.SWTHostOverviewViewProvider; import com.redhat.thermostat.host.overview.client.core.HostOverviewViewProvider; @@ -186,9 +185,12 @@ } public boolean isDbConnected() { - DbService dbService = OSGIUtils.getInstance().getServiceAllowNull( - DbService.class); - return dbService != null; + boolean result = false; + BundleContext context = getBundle().getBundleContext(); + if (context.getServiceReference(DbService.class) != null) { + result = true; + } + return result; } public Keyring getKeyring() { diff -r c6c9f2c7961c -r 3526e4183714 eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/internal/views/HostsVmsTreeViewPart.java --- a/eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/internal/views/HostsVmsTreeViewPart.java Fri Mar 22 09:24:14 2013 +0100 +++ b/eclipse/com.redhat.thermostat.eclipse/src/com/redhat/thermostat/eclipse/internal/views/HostsVmsTreeViewPart.java Fri Mar 22 12:01:35 2013 -0400 @@ -36,9 +36,13 @@ package com.redhat.thermostat.eclipse.internal.views; +import java.util.Map; +import java.util.Objects; + import org.eclipse.core.runtime.jobs.Job; import org.eclipse.jface.action.Action; import org.eclipse.jface.action.IToolBarManager; +import org.eclipse.jface.dialogs.MessageDialog; import org.eclipse.jface.viewers.TreeViewer; import org.eclipse.swt.SWT; import org.eclipse.swt.layout.RowLayout; @@ -47,12 +51,15 @@ import org.eclipse.swt.widgets.Label; import org.eclipse.swt.widgets.Link; import org.eclipse.swt.widgets.Listener; +import org.eclipse.ui.IViewSite; +import org.eclipse.ui.PartInitException; import org.eclipse.ui.PlatformUI; import org.eclipse.ui.part.PageBook; import org.eclipse.ui.part.ViewPart; +import org.osgi.framework.BundleContext; import com.redhat.thermostat.common.config.ClientPreferences; -import com.redhat.thermostat.common.utils.OSGIUtils; +import com.redhat.thermostat.common.MultipleServiceTracker; import com.redhat.thermostat.eclipse.internal.Activator; import com.redhat.thermostat.eclipse.internal.ConnectionConfiguration; import com.redhat.thermostat.eclipse.internal.controllers.ConnectDBAction; @@ -80,6 +87,10 @@ private Composite connectPage; // Container for tree and connect private PageBook pageBook; + private MultipleServiceTracker tracker; + private HostInfoDAO hostInfoDAO; + private VmInfoDAO vmInfoDAO; + private boolean closing; public HostsVmsTreeViewPart() { ClientPreferences clientPrefs = new ClientPreferences(Activator.getDefault().getKeyring()); @@ -95,6 +106,51 @@ connectJob.addJobChangeListener(new ConnectionJobListener(connectAction, this)); } + @Override + public void init(IViewSite site) throws PartInitException { + super.init(site); + + BundleContext context = Activator.getDefault().getBundle().getBundleContext(); + Class[] deps = new Class[] { + HostInfoDAO.class, + VmInfoDAO.class + }; + tracker = new MultipleServiceTracker(context, deps, new MultipleServiceTracker.Action() { + + @Override + public void dependenciesAvailable(Map services) { + hostInfoDAO = (HostInfoDAO) services.get(HostInfoDAO.class.getName()); + Objects.requireNonNull(hostInfoDAO); + vmInfoDAO = (VmInfoDAO) services.get(VmInfoDAO.class.getName()); + Objects.requireNonNull(vmInfoDAO); + } + + @Override + public void dependenciesUnavailable() { + if (!closing) { + // Show the user an error + PlatformUI.getWorkbench().getDisplay().syncExec(new Runnable() { + + @Override + public void run() { + MessageDialog.openError(null, "Connection Error", "Unable to connect to storage"); + } + }); + // Switch to the connection page + showConnectionPage(); + } + } + }); + tracker.open(); + } + + @Override + public void dispose() { + closing = true; + tracker.close(); + super.dispose(); + } + public void showConnectionPage() { PlatformUI.getWorkbench().getDisplay().asyncExec(new Runnable() { @Override @@ -105,12 +161,8 @@ } public void showHostVmsPage() { - HostInfoDAO hostDAO = OSGIUtils.getInstance().getService( - HostInfoDAO.class); - VmInfoDAO vmsDAO = OSGIUtils.getInstance().getService( - VmInfoDAO.class); - final HostsVMsLoader loader = new DefaultHostsVMsLoader(hostDAO, - vmsDAO, false); + final HostsVMsLoader loader = new DefaultHostsVMsLoader(hostInfoDAO, + vmInfoDAO, false); PlatformUI.getWorkbench().getDisplay().asyncExec(new Runnable() { diff -r c6c9f2c7961c -r 3526e4183714 killvm/client-swing/src/main/java/com/redhat/thermostat/killvm/client/internal/Activator.java --- a/killvm/client-swing/src/main/java/com/redhat/thermostat/killvm/client/internal/Activator.java Fri Mar 22 09:24:14 2013 +0100 +++ b/killvm/client-swing/src/main/java/com/redhat/thermostat/killvm/client/internal/Activator.java Fri Mar 22 12:01:35 2013 -0400 @@ -42,6 +42,7 @@ import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceRegistration; +import com.redhat.thermostat.client.command.RequestQueue; import com.redhat.thermostat.client.ui.VMContextAction; import com.redhat.thermostat.common.MultipleServiceTracker; import com.redhat.thermostat.common.MultipleServiceTracker.Action; @@ -58,6 +59,7 @@ Class[] serviceDeps = new Class[] { AgentInfoDAO.class, VmInfoDAO.class, + RequestQueue.class, }; killVmActionTracker = new MultipleServiceTracker(context, serviceDeps, new Action() { @@ -65,7 +67,8 @@ public void dependenciesAvailable(Map services) { AgentInfoDAO agentDao = (AgentInfoDAO) services.get(AgentInfoDAO.class.getName()); VmInfoDAO vmDao = (VmInfoDAO) services.get(VmInfoDAO.class.getName()); - KillVMAction service = new KillVMAction(agentDao, vmDao, new SwingVMKilledListener()); + RequestQueue queue = (RequestQueue) services.get(RequestQueue.class.getName()); + KillVMAction service = new KillVMAction(agentDao, vmDao, queue, new SwingVMKilledListener()); killActionRegistration = context.registerService(VMContextAction.class.getName(), service, null); } diff -r c6c9f2c7961c -r 3526e4183714 killvm/client-swing/src/main/java/com/redhat/thermostat/killvm/client/internal/KillVMAction.java --- a/killvm/client-swing/src/main/java/com/redhat/thermostat/killvm/client/internal/KillVMAction.java Fri Mar 22 09:24:14 2013 +0100 +++ b/killvm/client-swing/src/main/java/com/redhat/thermostat/killvm/client/internal/KillVMAction.java Fri Mar 22 12:01:35 2013 -0400 @@ -46,7 +46,6 @@ import com.redhat.thermostat.common.command.Request.RequestType; import com.redhat.thermostat.common.command.RequestResponseListener; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.killvm.client.locale.LocaleResources; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.AgentInfoDAO; @@ -63,13 +62,15 @@ private final AgentInfoDAO agentDao; private final VmInfoDAO vmDao; private final Translate t; + private final RequestQueue queue; private final RequestResponseListener listener; - public KillVMAction(AgentInfoDAO agentDao, VmInfoDAO vmDao, RequestResponseListener listener) { + public KillVMAction(AgentInfoDAO agentDao, VmInfoDAO vmDao, RequestQueue queue, RequestResponseListener listener) { Objects.requireNonNull(listener, "Listener can't be null"); this.agentDao = agentDao; this.vmDao = vmDao; this.t = LocaleResources.createLocalizer(); + this.queue = queue; this.listener = listener; } @@ -94,7 +95,6 @@ murderer.setReceiver(RECEIVER); murderer.addListener(listener); - RequestQueue queue = OSGIUtils.getInstance().getService(RequestQueue.class); queue.putRequest(murderer); } diff -r c6c9f2c7961c -r 3526e4183714 killvm/client-swing/src/test/java/com/redhat/thermostat/killvm/client/internal/KillVMActionTest.java --- a/killvm/client-swing/src/test/java/com/redhat/thermostat/killvm/client/internal/KillVMActionTest.java Fri Mar 22 09:24:14 2013 +0100 +++ b/killvm/client-swing/src/test/java/com/redhat/thermostat/killvm/client/internal/KillVMActionTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -47,18 +47,12 @@ import java.net.InetSocketAddress; import org.junit.Test; -import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; -import org.powermock.api.mockito.PowerMockito; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; import com.redhat.thermostat.client.command.RequestQueue; import com.redhat.thermostat.client.core.Filter; import com.redhat.thermostat.common.command.Request; import com.redhat.thermostat.common.command.RequestResponseListener; -import com.redhat.thermostat.common.utils.OSGIUtils; -import com.redhat.thermostat.killvm.client.internal.KillVMAction; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.AgentInfoDAO; @@ -66,8 +60,6 @@ import com.redhat.thermostat.storage.model.AgentInformation; import com.redhat.thermostat.storage.model.VmInfo; -@RunWith(PowerMockRunner.class) -@PrepareForTest(OSGIUtils.class) public class KillVMActionTest { @Test @@ -80,9 +72,10 @@ VmInfo vmInfo = mock(VmInfo.class); when(vmInfoDao.getVmInfo(matching)).thenReturn(vmInfo); + RequestQueue queue = mock(RequestQueue.class); RequestResponseListener listener = mock(RequestResponseListener.class); - KillVMAction action = new KillVMAction(agentDao, vmInfoDao, listener); + KillVMAction action = new KillVMAction(agentDao, vmInfoDao, queue, listener); Filter filter = action.getFilter(); @@ -109,18 +102,14 @@ RequestResponseListener agentResponseListener = mock(RequestResponseListener.class); + RequestQueue queue = mock(RequestQueue.class); final Request req = mock(Request.class); - KillVMAction action = new KillVMAction(agentDao, vmInfoDao, agentResponseListener) { + KillVMAction action = new KillVMAction(agentDao, vmInfoDao, queue, agentResponseListener) { @Override Request getKillRequest(InetSocketAddress target) { return req; } }; - OSGIUtils utils = mock(OSGIUtils.class); - PowerMockito.mockStatic(OSGIUtils.class); - when(OSGIUtils.getInstance()).thenReturn(utils); - RequestQueue queue = mock(RequestQueue.class); - when(utils.getService(RequestQueue.class)).thenReturn(queue); action.execute(ref); ArgumentCaptor vmIdParamCaptor = ArgumentCaptor .forClass(String.class); diff -r c6c9f2c7961c -r 3526e4183714 thread/client-common/pom.xml --- a/thread/client-common/pom.xml Fri Mar 22 09:24:14 2013 +0100 +++ b/thread/client-common/pom.xml Fri Mar 22 12:01:35 2013 -0400 @@ -94,6 +94,13 @@ thermostat-thread-collector ${project.version} + + + com.redhat.thermostat + thermostat-common-test + ${project.version} + test + diff -r c6c9f2c7961c -r 3526e4183714 thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorFactoryImpl.java --- a/thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorFactoryImpl.java Fri Mar 22 09:24:14 2013 +0100 +++ b/thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorFactoryImpl.java Fri Mar 22 12:01:35 2013 -0400 @@ -36,6 +36,9 @@ package com.redhat.thermostat.thread.client.common.collector.impl; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; + import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.AgentInfoDAO; import com.redhat.thermostat.thread.client.common.collector.ThreadCollector; @@ -44,9 +47,18 @@ public class ThreadCollectorFactoryImpl implements ThreadCollectorFactory { + private BundleContext context; private AgentInfoDAO agentDao; private ThreadDao threadDao; + public ThreadCollectorFactoryImpl() { + this(FrameworkUtil.getBundle(ThreadCollectorFactoryImpl.class).getBundleContext()); + } + + ThreadCollectorFactoryImpl(BundleContext context) { + this.context = context; + } + public void setAgentDao(AgentInfoDAO agentDao) { this.agentDao = agentDao; } @@ -58,7 +70,7 @@ @Override public synchronized ThreadCollector getCollector(VmRef reference) { // TODO set the values when the agent/thread dao changes - ThreadMXBeanCollector result = new ThreadMXBeanCollector(reference); + ThreadMXBeanCollector result = new ThreadMXBeanCollector(context, reference); result.setAgentInfoDao(agentDao); result.setThreadDao(threadDao); return result; diff -r c6c9f2c7961c -r 3526e4183714 thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadMXBeanCollector.java --- a/thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadMXBeanCollector.java Fri Mar 22 09:24:14 2013 +0100 +++ b/thread/client-common/src/main/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadMXBeanCollector.java Fri Mar 22 12:01:35 2013 -0400 @@ -39,13 +39,19 @@ import java.net.InetSocketAddress; import java.util.List; import java.util.concurrent.CountDownLatch; +import java.util.logging.Level; +import java.util.logging.Logger; + +import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceReference; import com.redhat.thermostat.client.command.RequestQueue; +import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.command.Request; +import com.redhat.thermostat.common.command.Request.RequestType; import com.redhat.thermostat.common.command.RequestResponseListener; import com.redhat.thermostat.common.command.Response; -import com.redhat.thermostat.common.command.Request.RequestType; -import com.redhat.thermostat.common.utils.OSGIUtils; +import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.AgentInfoDAO; @@ -57,12 +63,16 @@ import com.redhat.thermostat.thread.model.VMThreadCapabilities; public class ThreadMXBeanCollector implements ThreadCollector { + + private static final Logger logger = LoggingUtils.getLogger(ThreadMXBeanCollector.class); private AgentInfoDAO agentDao; private ThreadDao threadDao; + private BundleContext context; private VmRef ref; - public ThreadMXBeanCollector(VmRef ref) { + public ThreadMXBeanCollector(BundleContext context, VmRef ref) { + this.context = context; this.ref = ref; } @@ -112,11 +122,11 @@ } }); - RequestQueue queue = getRequestQueue(); - queue.putRequest(harvester); - try { + enqueueRequest(harvester); latch.await(); + } catch (CommandException e) { + logger.log(Level.WARNING, "Failed to enqueue request", e); } catch (InterruptedException ignore) {} return result[0]; @@ -146,11 +156,11 @@ } }); - RequestQueue queue = getRequestQueue(); - queue.putRequest(harvester); - try { + enqueueRequest(harvester); latch.await(); + } catch (CommandException e) { + logger.log(Level.WARNING, "Failed to enqueue request", e); } catch (InterruptedException ignore) {} return result[0]; } @@ -178,11 +188,11 @@ } }); - RequestQueue queue = getRequestQueue(); - queue.putRequest(harvester); - try { + enqueueRequest(harvester); latch.await(); + } catch (CommandException e) { + logger.log(Level.WARNING, "Failed to enqueue request", e); } catch (InterruptedException ignore) {} return result[0]; } @@ -235,22 +245,30 @@ } }); - RequestQueue queue = getRequestQueue(); - queue.putRequest(harvester); try { + enqueueRequest(harvester); latch.await(); // FIXME there is no guarantee that data is now present in storage caps = threadDao.loadCapabilities(ref); } catch (InterruptedException ignore) { caps = new VMThreadCapabilities(); + } catch (CommandException e) { + logger.log(Level.WARNING, "Failed to enqueue request", e); } } return caps; } - RequestQueue getRequestQueue() { - return OSGIUtils.getInstance().getService(RequestQueue.class); + private void enqueueRequest(Request req) throws CommandException { + ServiceReference ref = context.getServiceReference(RequestQueue.class.getName()); + if (ref == null) { + throw new CommandException("Cannot access command channel"); + } + RequestQueue queue = (RequestQueue) context.getService(ref); + queue.putRequest(req); + context.ungetService(ref); } + } diff -r c6c9f2c7961c -r 3526e4183714 thread/client-common/src/test/java/com/redhat/thermostat/thread/client/common/collector/ThreadCollectorFactoryTest.java --- a/thread/client-common/src/test/java/com/redhat/thermostat/thread/client/common/collector/ThreadCollectorFactoryTest.java Fri Mar 22 09:24:14 2013 +0100 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,73 +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.thread.client.common.collector; - -import static org.junit.Assert.*; -import static org.mockito.Mockito.mock; - -import org.junit.Test; - -import com.redhat.thermostat.storage.core.VmRef; -import com.redhat.thermostat.storage.dao.AgentInfoDAO; -import com.redhat.thermostat.thread.client.common.collector.impl.ThreadCollectorFactoryImpl; -import com.redhat.thermostat.thread.dao.ThreadDao; - -public class ThreadCollectorFactoryTest { - - @Test - public void testThreadCollectorFactory() { - VmRef reference = mock(VmRef.class); - - ThreadCollectorFactory factory = new ThreadCollectorFactoryImpl(); - ThreadCollector collector = factory.getCollector(reference); - assertNotNull(collector); - } - - @Test - public void testThreadCollectorFactoryWithAgentAndThreadDaos() { - AgentInfoDAO agentDao = mock(AgentInfoDAO.class); - ThreadDao threadDao = mock(ThreadDao.class); - VmRef reference = mock(VmRef.class); - - ThreadCollectorFactory factory = new ThreadCollectorFactoryImpl(); - factory.setAgentDao(agentDao); - factory.setThreadDao(threadDao); - ThreadCollector collector = factory.getCollector(reference); - assertNotNull(collector); - } -} - diff -r c6c9f2c7961c -r 3526e4183714 thread/client-common/src/test/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorFactoryImplTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/thread/client-common/src/test/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorFactoryImplTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -0,0 +1,78 @@ +/* + * 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.thread.client.common.collector.impl; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; + +import org.junit.Test; + +import com.redhat.thermostat.storage.core.VmRef; +import com.redhat.thermostat.storage.dao.AgentInfoDAO; +import com.redhat.thermostat.testutils.StubBundleContext; +import com.redhat.thermostat.thread.client.common.collector.ThreadCollector; +import com.redhat.thermostat.thread.client.common.collector.ThreadCollectorFactory; +import com.redhat.thermostat.thread.client.common.collector.impl.ThreadCollectorFactoryImpl; +import com.redhat.thermostat.thread.dao.ThreadDao; + +public class ThreadCollectorFactoryImplTest { + + @Test + public void testThreadCollectorFactory() { + StubBundleContext context = new StubBundleContext(); + VmRef reference = mock(VmRef.class); + + ThreadCollectorFactory factory = new ThreadCollectorFactoryImpl(context); + ThreadCollector collector = factory.getCollector(reference); + assertNotNull(collector); + } + + @Test + public void testThreadCollectorFactoryWithAgentAndThreadDaos() { + StubBundleContext context = new StubBundleContext(); + AgentInfoDAO agentDao = mock(AgentInfoDAO.class); + ThreadDao threadDao = mock(ThreadDao.class); + VmRef reference = mock(VmRef.class); + + ThreadCollectorFactory factory = new ThreadCollectorFactoryImpl(context); + factory.setAgentDao(agentDao); + factory.setThreadDao(threadDao); + ThreadCollector collector = factory.getCollector(reference); + assertNotNull(collector); + } +} + diff -r c6c9f2c7961c -r 3526e4183714 thread/client-common/src/test/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorTest.java --- a/thread/client-common/src/test/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorTest.java Fri Mar 22 09:24:14 2013 +0100 +++ b/thread/client-common/src/test/java/com/redhat/thermostat/thread/client/common/collector/impl/ThreadCollectorTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -36,16 +36,18 @@ package com.redhat.thermostat.thread.client.common.collector.impl; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.fail; - import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.mockito.Mockito.times; +import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.invocation.InvocationOnMock; @@ -56,34 +58,45 @@ import com.redhat.thermostat.common.command.RequestResponseListener; import com.redhat.thermostat.common.command.Response; import com.redhat.thermostat.common.command.Response.ResponseType; -import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.AgentInfoDAO; +import com.redhat.thermostat.testutils.StubBundleContext; import com.redhat.thermostat.thread.client.common.collector.ThreadCollector; -import com.redhat.thermostat.thread.client.common.collector.impl.ThreadMXBeanCollector; import com.redhat.thermostat.thread.collector.HarvesterCommand; import com.redhat.thermostat.thread.dao.ThreadDao; import com.redhat.thermostat.thread.model.VMThreadCapabilities; public class ThreadCollectorTest { + + private StubBundleContext context; + private ThreadDao threadDao; + private VmRef reference; + private Request request; + private AgentInfoDAO agentDao; + + @Before + public void setup() { + context = new StubBundleContext(); + request = mock(Request.class); + agentDao = mock(AgentInfoDAO.class); + threadDao = mock(ThreadDao.class); + reference = mock(VmRef.class); + when(reference.getIdString()).thenReturn("00101010"); + + final Response response = mock(Response.class); + when(response.getType()).thenReturn(ResponseType.OK); + + final ArgumentCaptor captor = ArgumentCaptor.forClass(RequestResponseListener.class); + doNothing().when(request).addListener(captor.capture()); + } @Test public void testVMCapabilitiesNotInDAO() throws Exception { - - HostRef agent = mock(HostRef.class); - when(agent.getAgentId()).thenReturn("42"); - - VmRef reference = mock(VmRef.class); - when(reference.getIdString()).thenReturn("00101010"); - when(reference.getAgent()).thenReturn(agent); - AgentInfoDAO agentDao = mock(AgentInfoDAO.class); - ThreadDao threadDao = mock(ThreadDao.class); - VMThreadCapabilities resCaps = mock(VMThreadCapabilities.class); when(threadDao.loadCapabilities(reference)).thenReturn(null).thenReturn(resCaps); - final Request request = mock(Request.class); final RequestQueue requestQueue = mock(RequestQueue.class); + context.registerService(RequestQueue.class, requestQueue, null); final ArgumentCaptor captor = ArgumentCaptor.forClass(RequestResponseListener.class); doNothing().when(request).addListener(captor.capture()); @@ -101,22 +114,16 @@ } }).when(requestQueue).putRequest(request); - - /* ************* */ - ThreadCollector collector = new ThreadMXBeanCollector(reference) { + ThreadCollector collector = new ThreadMXBeanCollector(context, reference) { @Override Request createRequest() { return request; } - @Override - RequestQueue getRequestQueue() { - return requestQueue; - } }; collector.setAgentInfoDao(agentDao); collector.setThreadDao(threadDao); - + VMThreadCapabilities caps = collector.getVMThreadCapabilities(); verify(request).setParameter(HarvesterCommand.class.getName(), HarvesterCommand.VM_CAPS.name()); @@ -129,26 +136,41 @@ } @Test - public void testVMCapabilitiesInDAO() throws Exception { + public void testVMCapabilitiesNoRequestQueue() throws Exception { + VMThreadCapabilities resCaps = mock(VMThreadCapabilities.class); + when(threadDao.loadCapabilities(reference)).thenReturn(null).thenReturn(resCaps); + + ThreadCollector collector = new ThreadMXBeanCollector(context, reference) { + @Override + Request createRequest() { + return request; + } + }; + collector.setAgentInfoDao(agentDao); + collector.setThreadDao(threadDao); - VmRef reference = mock(VmRef.class); - AgentInfoDAO agentDao = mock(AgentInfoDAO.class); - ThreadDao threadDao = mock(ThreadDao.class); + VMThreadCapabilities caps = collector.getVMThreadCapabilities(); + + verify(request).setParameter(HarvesterCommand.class.getName(), HarvesterCommand.VM_CAPS.name()); + verify(request).setParameter(HarvesterCommand.VM_ID.name(), "00101010"); + + verify(threadDao, times(1)).loadCapabilities(reference); + assertNull(caps); + } + + @Test + public void testVMCapabilitiesInDAO() throws Exception { + StubBundleContext context = new StubBundleContext(); VMThreadCapabilities resCaps = mock(VMThreadCapabilities.class); when(threadDao.loadCapabilities(reference)).thenReturn(resCaps); - ThreadCollector collector = new ThreadMXBeanCollector(reference) { + ThreadCollector collector = new ThreadMXBeanCollector(context, reference) { @Override Request createRequest() { fail(); return null; } - @Override - RequestQueue getRequestQueue() { - fail(); - return null; - } }; collector.setAgentInfoDao(agentDao); @@ -162,17 +184,8 @@ @Test public void testStart() { - - HostRef agent = mock(HostRef.class); - when(agent.getAgentId()).thenReturn("42"); - - final Request request = mock(Request.class); final RequestQueue requestQueue = mock(RequestQueue.class); - AgentInfoDAO agentDao = mock(AgentInfoDAO.class); - ThreadDao threadDao = mock(ThreadDao.class); - VmRef reference = mock(VmRef.class); - when(reference.getIdString()).thenReturn("00101010"); - when(reference.getAgent()).thenReturn(agent); + context.registerService(RequestQueue.class, requestQueue, null); final Response response = mock(Response.class); when(response.getType()).thenReturn(ResponseType.OK); @@ -194,15 +207,11 @@ }).when(requestQueue).putRequest(request); - ThreadCollector collector = new ThreadMXBeanCollector(reference) { + ThreadCollector collector = new ThreadMXBeanCollector(context, reference) { @Override Request createRequest() { return request; } - @Override - RequestQueue getRequestQueue() { - return requestQueue; - } }; collector.setAgentInfoDao(agentDao); collector.setThreadDao(threadDao); @@ -215,16 +224,29 @@ verify(requestQueue).putRequest(request); } - + @Test + public void testStartNoRequestQueue() { + ThreadCollector collector = new ThreadMXBeanCollector(context, reference) { + @Override + Request createRequest() { + return request; + } + }; + collector.setAgentInfoDao(agentDao); + collector.setThreadDao(threadDao); + + boolean result = collector.startHarvester(); + + verify(request).setParameter(HarvesterCommand.class.getName(), HarvesterCommand.START.name()); + verify(request).setParameter(HarvesterCommand.VM_ID.name(), "00101010"); + + assertFalse(result); + } + @Test public void testStop() { - - final Request request = mock(Request.class); final RequestQueue requestQueue = mock(RequestQueue.class); - AgentInfoDAO agentDao = mock(AgentInfoDAO.class); - ThreadDao threadDao = mock(ThreadDao.class); - VmRef reference = mock(VmRef.class); - when(reference.getIdString()).thenReturn("00101010"); + context.registerService(RequestQueue.class, requestQueue, null); final Response response = mock(Response.class); when(response.getType()).thenReturn(ResponseType.OK); @@ -246,15 +268,11 @@ }).when(requestQueue).putRequest(request); - ThreadCollector collector = new ThreadMXBeanCollector(reference) { + ThreadCollector collector = new ThreadMXBeanCollector(context, reference) { @Override Request createRequest() { return request; } - @Override - RequestQueue getRequestQueue() { - return requestQueue; - } }; collector.setAgentInfoDao(agentDao); collector.setThreadDao(threadDao); @@ -264,6 +282,25 @@ verify(request).setParameter(HarvesterCommand.VM_ID.name(), "00101010"); verify(requestQueue).putRequest(request); - } + } + + @Test + public void testStopNoRequestQueue() { + ThreadCollector collector = new ThreadMXBeanCollector(context, reference) { + @Override + Request createRequest() { + return request; + } + }; + collector.setAgentInfoDao(agentDao); + collector.setThreadDao(threadDao); + + boolean result = collector.stopHarvester(); + + verify(request).setParameter(HarvesterCommand.class.getName(), HarvesterCommand.STOP.name()); + verify(request).setParameter(HarvesterCommand.VM_ID.name(), "00101010"); + + assertFalse(result); + } } diff -r c6c9f2c7961c -r 3526e4183714 thread/client-controllers/src/main/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationController.java --- a/thread/client-controllers/src/main/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationController.java Fri Mar 22 09:24:14 2013 +0100 +++ b/thread/client-controllers/src/main/java/com/redhat/thermostat/thread/client/controller/impl/ThreadInformationController.java Fri Mar 22 12:01:35 2013 -0400 @@ -127,7 +127,7 @@ break; default: - logger.log(Level.WARNING, "unkown action: " + actionEvent.getActionId()); + logger.log(Level.WARNING, "unknown action: " + actionEvent.getActionId()); break; } } diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommand.java Fri Mar 22 12:01:35 2013 -0400 @@ -38,12 +38,16 @@ import java.util.concurrent.Semaphore; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; +import org.osgi.framework.ServiceReference; + import com.redhat.thermostat.client.cli.HostVMArguments; +import com.redhat.thermostat.client.command.RequestQueue; import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.dao.AgentInfoDAO; import com.redhat.thermostat.vm.heap.analysis.command.locale.LocaleResources; @@ -53,15 +57,15 @@ private static final Translate translator = LocaleResources.createLocalizer(); private static final String NAME = "dump-heap"; - private final OSGIUtils serviceProvider; + private BundleContext context; private final DumpHeapHelper implementation; public DumpHeapCommand() { - this(OSGIUtils.getInstance(), new DumpHeapHelper()); + this(FrameworkUtil.getBundle(DumpHeapCommand.class).getBundleContext(), new DumpHeapHelper()); } - DumpHeapCommand(OSGIUtils serviceProvider, DumpHeapHelper impl) { - this.serviceProvider = serviceProvider; + DumpHeapCommand(BundleContext context, DumpHeapHelper impl) { + this.context = context; this.implementation = impl; } @@ -92,13 +96,23 @@ } }; - AgentInfoDAO service = serviceProvider.getService(AgentInfoDAO.class); - if (service == null) { - throw new CommandException("Unable to access agent information"); + ServiceReference agentInfoRef = context.getServiceReference(AgentInfoDAO.class.getName()); + if (agentInfoRef == null) { + throw new CommandException(translator.localize(LocaleResources.AGENT_SERVICE_UNAVAILABLE)); } - implementation.execute(service, args.getVM(), successHandler, errorHandler); - serviceProvider.ungetService(AgentInfoDAO.class, service); - + AgentInfoDAO agentInfoDAO = (AgentInfoDAO) context.getService(agentInfoRef); + + ServiceReference requestQueueRef = context.getServiceReference(RequestQueue.class.getName()); + if (requestQueueRef == null) { + throw new CommandException(translator.localize(LocaleResources.REQUEST_QUEUE_UNAVAILABLE)); + } + RequestQueue queue = (RequestQueue) context.getService(requestQueueRef); + + implementation.execute(agentInfoDAO, args.getVM(), queue, successHandler, errorHandler); + + context.ungetService(agentInfoRef); + context.ungetService(requestQueueRef); + try { s.acquire(); } catch (InterruptedException e) { diff -r c6c9f2c7961c -r 3526e4183714 vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapHelper.java --- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapHelper.java Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapHelper.java Fri Mar 22 12:01:35 2013 -0400 @@ -44,7 +44,6 @@ import com.redhat.thermostat.common.command.RequestResponseListener; import com.redhat.thermostat.common.command.Response; import com.redhat.thermostat.common.command.Response.ResponseType; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.AgentInfoDAO; @@ -79,8 +78,9 @@ } - public void execute(AgentInfoDAO agentInfoDAO, VmRef reference, Runnable heapDumpSuccessAction, Runnable heapDumpFailureAction) { - + public void execute(AgentInfoDAO agentInfoDAO, VmRef reference, + RequestQueue queue, Runnable heapDumpSuccessAction, + Runnable heapDumpFailureAction) { HostRef targetHostRef = reference.getAgent(); String address = agentInfoDAO.getAgentInformation(targetHostRef).getConfigListenAddress(); @@ -91,7 +91,6 @@ req.setParameter(VM_ID_PARAM, reference.getIdString()); req.addListener(new HeapDumpListener(heapDumpSuccessAction, heapDumpFailureAction)); - RequestQueue queue = OSGIUtils.getInstance().getService(RequestQueue.class); queue.putRequest(req); } diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindObjectsCommand.java Fri Mar 22 12:01:35 2013 -0400 @@ -39,12 +39,15 @@ import java.util.Collection; import java.util.List; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; +import org.osgi.framework.ServiceReference; + +import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; -import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.TableRenderer; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.vm.heap.analysis.command.locale.LocaleResources; import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO; import com.redhat.thermostat.vm.heap.analysis.common.HeapDump; @@ -62,27 +65,27 @@ private static final String HEADER_TYPE = translator.localize(LocaleResources.HEADER_OBJECT_TYPE); private static final int DEFAULT_LIMIT = 10; - private OSGIUtils serviceProvider; + private BundleContext context; public FindObjectsCommand() { - this(OSGIUtils.getInstance()); + this(FrameworkUtil.getBundle(FindObjectsCommand.class).getBundleContext()); } - FindObjectsCommand(OSGIUtils serviceProvider) { - this.serviceProvider = serviceProvider; + FindObjectsCommand(BundleContext context) { + this.context = context; } @Override public void run(CommandContext ctx) throws CommandException { - - HeapDAO heapDAO = serviceProvider.getServiceAllowNull(HeapDAO.class); - if (heapDAO == null) { + ServiceReference heapDAORef = context.getServiceReference(HeapDAO.class.getName()); + if (heapDAORef == null) { throw new CommandException(translator.localize(LocaleResources.HEAP_SERVICE_UNAVAILABLE)); } + HeapDAO heapDAO = (HeapDAO) context.getService(heapDAORef); try { run(ctx, heapDAO); } finally { - serviceProvider.ungetService(HeapDAO.class, heapDAO); + context.ungetService(heapDAORef); heapDAO = null; } } diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindRootCommand.java Fri Mar 22 12:01:35 2013 -0400 @@ -40,11 +40,14 @@ import java.util.Collection; import java.util.Iterator; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; +import org.osgi.framework.ServiceReference; + +import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; -import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.vm.heap.analysis.command.FindRoot; import com.redhat.thermostat.vm.heap.analysis.command.HeapPath; import com.redhat.thermostat.vm.heap.analysis.command.locale.LocaleResources; @@ -61,27 +64,28 @@ private static final String ALL_ARG = "all"; private static final String NAME = "find-root"; - private OSGIUtils serviceProvider; + private BundleContext context; public FindRootCommand() { - this(OSGIUtils.getInstance()); + this(FrameworkUtil.getBundle(FindRootCommand.class).getBundleContext()); } - FindRootCommand(OSGIUtils serviceProvider) { - this.serviceProvider = serviceProvider; + FindRootCommand(BundleContext context) { + this.context = context; } @Override public void run(CommandContext ctx) throws CommandException { - HeapDAO heapDao = serviceProvider.getServiceAllowNull(HeapDAO.class); - if (heapDao == null) { + ServiceReference heapDaoRef = context.getServiceReference(HeapDAO.class.getName()); + if (heapDaoRef == null) { throw new CommandException(translator.localize(LocaleResources.HEAP_SERVICE_UNAVAILABLE)); } + HeapDAO heapDao = (HeapDAO) context.getService(heapDaoRef); try { run(ctx, heapDao); } finally { - serviceProvider.ungetService(HeapDAO.class, heapDao); + context.ungetService(heapDaoRef); } } diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommand.java Fri Mar 22 12:01:35 2013 -0400 @@ -40,13 +40,16 @@ import java.util.Collection; import java.util.Date; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; +import org.osgi.framework.ServiceReference; + import com.redhat.thermostat.client.cli.HostVMArguments; import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.cli.TableRenderer; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.HostInfoDAO; @@ -68,15 +71,15 @@ translator.localize(LocaleResources.HEADER_TIMESTAMP), }; - private final OSGIUtils serviceProvider; + private final BundleContext context; public ListHeapDumpsCommand() { - this(OSGIUtils.getInstance()); + this(FrameworkUtil.getBundle(ListHeapDumpsCommand.class).getBundleContext()); } /** For tests only */ - ListHeapDumpsCommand(OSGIUtils serviceProvider) { - this.serviceProvider = serviceProvider; + ListHeapDumpsCommand(BundleContext context) { + this.context = context; } @Override @@ -92,20 +95,23 @@ renderer.printLine(COLUMN_NAMES); - HostInfoDAO hostDAO = serviceProvider.getServiceAllowNull(HostInfoDAO.class); - if (hostDAO == null) { + ServiceReference hostDAORef = context.getServiceReference(HostInfoDAO.class.getName()); + if (hostDAORef == null) { throw new CommandException(translator.localize(LocaleResources.HOST_SERVICE_UNAVAILABLE)); } + HostInfoDAO hostDAO = (HostInfoDAO) context.getService(hostDAORef); - VmInfoDAO vmDAO = serviceProvider.getServiceAllowNull(VmInfoDAO.class); - if (vmDAO == null) { + ServiceReference vmDAORef = context.getServiceReference(VmInfoDAO.class.getName()); + if (vmDAORef == null) { throw new CommandException(translator.localize(LocaleResources.VM_SERVICE_UNAVAILABLE)); } + VmInfoDAO vmDAO = (VmInfoDAO) context.getService(vmDAORef); - HeapDAO heapDAO = serviceProvider.getServiceAllowNull(HeapDAO.class); - if (heapDAO == null) { + ServiceReference heapDAORef = context.getServiceReference(HeapDAO.class.getName()); + if (heapDAORef == null) { throw new CommandException(translator.localize(LocaleResources.HEAP_SERVICE_UNAVAILABLE)); } + HeapDAO heapDAO = (HeapDAO) context.getService(heapDAORef); Collection hosts = args.getHost() != null ? Arrays.asList(args.getHost()) : hostDAO.getHosts(); for (HostRef hostRef : hosts) { @@ -115,9 +121,9 @@ } } - serviceProvider.ungetService(HeapDAO.class, heapDAO); - serviceProvider.ungetService(VmInfoDAO.class, vmDAO); - serviceProvider.ungetService(HostInfoDAO.class, hostDAO); + context.ungetService(heapDAORef); + context.ungetService(vmDAORef); + context.ungetService(hostDAORef); renderer.render(ctx.getConsole().getOutput()); } diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ObjectInfoCommand.java Fri Mar 22 12:01:35 2013 -0400 @@ -39,12 +39,15 @@ import java.io.PrintStream; import java.util.Enumeration; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; +import org.osgi.framework.ServiceReference; + +import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; -import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.TableRenderer; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.vm.heap.analysis.command.locale.LocaleResources; import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO; import com.redhat.thermostat.vm.heap.analysis.common.HeapDump; @@ -60,28 +63,29 @@ private static final String NAME = "object-info"; - private OSGIUtils serviceProvider; + private final BundleContext context; private Snapshot snapshot; public ObjectInfoCommand() { - this(OSGIUtils.getInstance()); + this(FrameworkUtil.getBundle(ObjectInfoCommand.class).getBundleContext()); } - ObjectInfoCommand(OSGIUtils serviceProvider) { - this.serviceProvider = serviceProvider; + ObjectInfoCommand(BundleContext context) { + this.context = context; } @Override public void run(CommandContext ctx) throws CommandException { - HeapDAO heapDao = serviceProvider.getServiceAllowNull(HeapDAO.class); - if (heapDao == null) { + ServiceReference heapDaoRef = context.getServiceReference(HeapDAO.class.getName()); + if (heapDaoRef == null) { throw new CommandException(translator.localize(LocaleResources.HEAP_SERVICE_UNAVAILABLE)); } + HeapDAO heapDao = (HeapDAO) context.getService(heapDaoRef); try { run(ctx, heapDao); } finally { - serviceProvider.ungetService(HeapDAO.class, heapDao); + context.ungetService(heapDaoRef); } } diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/SaveHeapDumpToFileCommand.java Fri Mar 22 12:01:35 2013 -0400 @@ -44,12 +44,15 @@ import java.io.InputStream; import java.io.OutputStream; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; +import org.osgi.framework.ServiceReference; + +import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.Arguments; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; -import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.common.utils.StreamUtils; import com.redhat.thermostat.vm.heap.analysis.command.locale.LocaleResources; import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO; @@ -65,14 +68,14 @@ private static final String FILE_NAME_ARGUMENT = "file"; private final FileStreamCreator creator; - private final OSGIUtils serviceProvider; + private final BundleContext context; public SaveHeapDumpToFileCommand() { - this(OSGIUtils.getInstance(), new FileStreamCreator()); + this(FrameworkUtil.getBundle(SaveHeapDumpToFileCommand.class).getBundleContext(), new FileStreamCreator()); } - SaveHeapDumpToFileCommand(OSGIUtils serviceProvider, FileStreamCreator creator) { - this.serviceProvider = serviceProvider; + SaveHeapDumpToFileCommand(BundleContext context, FileStreamCreator creator) { + this.context = context; this.creator = creator; } @@ -84,11 +87,15 @@ @Override public void run(CommandContext ctx) throws CommandException { - HeapDAO heapDAO = serviceProvider.getServiceAllowNull(HeapDAO.class); + ServiceReference ref = context.getServiceReference(HeapDAO.class.getName()); + if (ref == null) { + throw new CommandException(translator.localize(LocaleResources.HEAP_SERVICE_UNAVAILABLE)); + } + HeapDAO heapDAO = (HeapDAO) context.getService(ref); try { run(ctx, heapDAO); } finally { - serviceProvider.ungetService(HeapDAO.class, heapDAO); + context.ungetService(ref); heapDAO = null; } } diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ShowHeapHistogramCommand.java Fri Mar 22 12:01:35 2013 -0400 @@ -38,13 +38,16 @@ import java.io.PrintStream; +import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; +import org.osgi.framework.ServiceReference; + +import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.Arguments; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; -import com.redhat.thermostat.common.cli.AbstractCommand; import com.redhat.thermostat.common.cli.TableRenderer; import com.redhat.thermostat.common.locale.Translate; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.vm.heap.analysis.command.locale.LocaleResources; import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO; import com.redhat.thermostat.vm.heap.analysis.common.HistogramRecord; @@ -57,14 +60,14 @@ private static final String NAME = "show-heap-histogram"; - private OSGIUtils serviceProvider; - + private final BundleContext context; + public ShowHeapHistogramCommand() { - this(OSGIUtils.getInstance()); + this(FrameworkUtil.getBundle(ShowHeapHistogramCommand.class).getBundleContext()); } - ShowHeapHistogramCommand(OSGIUtils serviceProvider) { - this.serviceProvider = serviceProvider; + ShowHeapHistogramCommand(BundleContext context) { + this.context = context; } @Override @@ -74,15 +77,16 @@ @Override public void run(CommandContext ctx) throws CommandException { - HeapDAO heapDAO = serviceProvider.getServiceAllowNull(HeapDAO.class); - if (heapDAO == null) { + ServiceReference ref = context.getServiceReference(HeapDAO.class.getName()); + if (ref == null) { throw new CommandException(translator.localize(LocaleResources.HEAP_SERVICE_UNAVAILABLE)); } + HeapDAO heapDAO = (HeapDAO) context.getService(ref); try { run(ctx, heapDAO); } finally { - serviceProvider.ungetService(HeapDAO.class, heapDAO); + context.ungetService(ref); } } diff -r c6c9f2c7961c -r 3526e4183714 vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/locale/LocaleResources.java --- a/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/locale/LocaleResources.java Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/main/java/com/redhat/thermostat/vm/heap/analysis/command/locale/LocaleResources.java Fri Mar 22 12:01:35 2013 -0400 @@ -42,6 +42,8 @@ HOST_SERVICE_UNAVAILABLE, VM_SERVICE_UNAVAILABLE, HEAP_SERVICE_UNAVAILABLE, + AGENT_SERVICE_UNAVAILABLE, + REQUEST_QUEUE_UNAVAILABLE, HEADER_TIMESTAMP, HEADER_HOST_ID, diff -r c6c9f2c7961c -r 3526e4183714 vm-heap-analysis/command/src/main/resources/com/redhat/thermostat/vm/heap/analysis/command/locale/strings.properties --- a/vm-heap-analysis/command/src/main/resources/com/redhat/thermostat/vm/heap/analysis/command/locale/strings.properties Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/main/resources/com/redhat/thermostat/vm/heap/analysis/command/locale/strings.properties Fri Mar 22 12:01:35 2013 -0400 @@ -1,6 +1,8 @@ HOST_SERVICE_UNAVAILABLE = Unable to access host information (HostInfoDAO unavailable) VM_SERVICE_UNAVAILABLE = Unable to access vm information (VmInfoDAO unavailable) -HEAP_SERVICE_UNAVAILABLE = Unable to access heap informatio (HeapDAO unavailable) +HEAP_SERVICE_UNAVAILABLE = Unable to access heap information (HeapDAO unavailable) +AGENT_SERVICE_UNAVAILABLE = Unable to access agent information (AgentInfoDAO unavailable) +REQUEST_QUEUE_UNAVAILABLE = Unable to access command channel (RequestQueue unavailable) HEADER_TIMESTAMP = TIMESTAMP HEADER_HOST_ID = HOST ID diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ActivatorTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -83,7 +83,7 @@ } private void makeServiceLoaderHappy(StubBundleContext ctx) { - // ShellCommands no-arg constructor uses FrameworkUtil to get + // 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 diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapCommandTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -46,21 +46,21 @@ import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.invocation.InvocationOnMock; 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; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.AgentInfoDAO; import com.redhat.thermostat.test.TestCommandContextFactory; +import com.redhat.thermostat.testutils.StubBundleContext; import com.redhat.thermostat.vm.heap.analysis.command.locale.LocaleResources; public class DumpHeapCommandTest { @@ -70,7 +70,10 @@ @Test public void testBasics() { - Command command = new DumpHeapCommand(); + 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()); @@ -79,8 +82,10 @@ @Test public void verifyAcuallyCallsWorker() throws CommandException { AgentInfoDAO agentInfoDao = mock(AgentInfoDAO.class); - OSGIUtils osgi = mock(OSGIUtils.class); - when(osgi.getService(AgentInfoDAO.class)).thenReturn(agentInfoDao); + RequestQueue queue = mock(RequestQueue.class); + StubBundleContext context = new StubBundleContext(); + context.registerService(AgentInfoDAO.class, agentInfoDao, null); + context.registerService(RequestQueue.class, queue, null); DumpHeapHelper impl = mock(DumpHeapHelper.class); final ArgumentCaptor successHandler = ArgumentCaptor @@ -92,10 +97,10 @@ successHandler.getValue().run(); return null; } - }).when(impl).execute(eq(agentInfoDao), any(VmRef.class), + }).when(impl).execute(eq(agentInfoDao), any(VmRef.class), eq(queue), successHandler.capture(), any(Runnable.class)); - DumpHeapCommand command = new DumpHeapCommand(osgi, impl); + DumpHeapCommand command = new DumpHeapCommand(context, impl); TestCommandContextFactory factory = new TestCommandContextFactory(); @@ -105,7 +110,7 @@ command.run(factory.createContext(args)); - verify(impl).execute(eq(agentInfoDao), isA(VmRef.class), + verify(impl).execute(eq(agentInfoDao), isA(VmRef.class), eq(queue), any(Runnable.class), any(Runnable.class)); assertEquals("Done\n", factory.getOutput()); } @@ -113,11 +118,13 @@ @Test public void verifyNeedsHostAndVmId() throws CommandException { AgentInfoDAO agentInfoDao = mock(AgentInfoDAO.class); - OSGIUtils osgi = mock(OSGIUtils.class); - when(osgi.getService(AgentInfoDAO.class)).thenReturn(agentInfoDao); + RequestQueue queue = mock(RequestQueue.class); + StubBundleContext context = new StubBundleContext(); + context.registerService(AgentInfoDAO.class, agentInfoDao, null); + context.registerService(RequestQueue.class, queue, null); DumpHeapHelper impl = mock(DumpHeapHelper.class); - DumpHeapCommand command = new DumpHeapCommand(osgi, impl); + DumpHeapCommand command = new DumpHeapCommand(context, impl); TestCommandContextFactory factory = new TestCommandContextFactory(); @@ -133,10 +140,12 @@ @Test public void verifyFailsIfAgentDaoIsNotAvailable() { - OSGIUtils osgi = mock(OSGIUtils.class); + RequestQueue queue = mock(RequestQueue.class); + StubBundleContext context = new StubBundleContext(); + context.registerService(RequestQueue.class, queue, null); DumpHeapHelper impl = mock(DumpHeapHelper.class); - DumpHeapCommand command = new DumpHeapCommand(osgi, impl); + DumpHeapCommand command = new DumpHeapCommand(context, impl); TestCommandContextFactory factory = new TestCommandContextFactory(); @@ -148,7 +157,30 @@ command.run(factory.createContext(args)); assertTrue("should not reach here", false); } catch (CommandException ce) { - assertEquals("Unable to access agent information", ce.getMessage()); + assertEquals(TRANSLATOR.localize(LocaleResources.AGENT_SERVICE_UNAVAILABLE), ce.getMessage()); + } + } + + @Test + public void verifyFailsIfRequestQueueIsNotAvailable() { + AgentInfoDAO agentInfoDao = mock(AgentInfoDAO.class); + StubBundleContext context = new StubBundleContext(); + context.registerService(AgentInfoDAO.class, agentInfoDao, null); + + DumpHeapHelper impl = mock(DumpHeapHelper.class); + DumpHeapCommand command = new DumpHeapCommand(context, impl); + + TestCommandContextFactory factory = new TestCommandContextFactory(); + + SimpleArguments args = new SimpleArguments(); + args.addArgument("hostId", "foo"); + args.addArgument("vmId", "0"); + + try { + command.run(factory.createContext(args)); + fail(); + } catch (CommandException ce) { + assertEquals(TRANSLATOR.localize(LocaleResources.REQUEST_QUEUE_UNAVAILABLE), ce.getMessage()); } } @@ -157,11 +189,13 @@ final String HOST_ID = "myHost"; final int VM_ID = 9001; AgentInfoDAO agentInfoDao = mock(AgentInfoDAO.class); - OSGIUtils osgi = mock(OSGIUtils.class); - when(osgi.getService(AgentInfoDAO.class)).thenReturn(agentInfoDao); + RequestQueue queue = mock(RequestQueue.class); + StubBundleContext context = new StubBundleContext(); + context.registerService(AgentInfoDAO.class, agentInfoDao, null); + context.registerService(RequestQueue.class, queue, null); DumpHeapHelper impl = mock(DumpHeapHelper.class); - DumpHeapCommand command = new DumpHeapCommand(osgi, impl); + DumpHeapCommand command = new DumpHeapCommand(context, impl); TestCommandContextFactory factory = new TestCommandContextFactory(); SimpleArguments args = new SimpleArguments(); @@ -172,11 +206,11 @@ @Override public Object answer(InvocationOnMock invocation) throws Throwable { - Runnable failRunnable = (Runnable) invocation.getArguments()[3]; + Runnable failRunnable = (Runnable) invocation.getArguments()[4]; failRunnable.run(); return null; } - }).when(impl).execute(any(AgentInfoDAO.class), any(VmRef.class), + }).when(impl).execute(any(AgentInfoDAO.class), any(VmRef.class), any(RequestQueue.class), any(Runnable.class), any(Runnable.class)); try { diff -r c6c9f2c7961c -r 3526e4183714 vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapHelperTest.java --- a/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapHelperTest.java Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/DumpHeapHelperTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -58,12 +58,10 @@ import com.redhat.thermostat.common.command.RequestResponseListener; import com.redhat.thermostat.common.command.Response; import com.redhat.thermostat.common.command.Response.ResponseType; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.AgentInfoDAO; import com.redhat.thermostat.storage.model.AgentInformation; -import com.redhat.thermostat.vm.heap.analysis.command.internal.DumpHeapHelper; public class DumpHeapHelperTest { @@ -77,9 +75,6 @@ @Before public void setUp() { reqQueue = mock(RequestQueue.class); - OSGIUtils osgiUtils = mock(OSGIUtils.class); - when(osgiUtils.getService(RequestQueue.class)).thenReturn(reqQueue); - OSGIUtils.setInstance(osgiUtils); HostRef host = mock(HostRef.class); @@ -108,7 +103,7 @@ @Test public void testExecute() { - cmd.execute(agentInfoDao, vmRef, heapDumpCompleteAction, heapDumpFailedAction); + cmd.execute(agentInfoDao, vmRef, reqQueue, heapDumpCompleteAction, heapDumpFailedAction); ArgumentCaptor reqArg = ArgumentCaptor.forClass(Request.class); verify(reqQueue).putRequest(reqArg.capture()); @@ -130,7 +125,7 @@ @Test public void testExecuteFailure() { - cmd.execute(agentInfoDao, vmRef, heapDumpCompleteAction, heapDumpFailedAction); + cmd.execute(agentInfoDao, vmRef, reqQueue, heapDumpCompleteAction, heapDumpFailedAction); ArgumentCaptor reqArg = ArgumentCaptor.forClass(Request.class); verify(reqQueue).putRequest(reqArg.capture()); diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindObjectsCommandTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -54,8 +54,8 @@ import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.cli.SimpleArguments; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.test.TestCommandContextFactory; +import com.redhat.thermostat.testutils.StubBundleContext; import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO; import com.redhat.thermostat.vm.heap.analysis.common.HeapDump; import com.redhat.thermostat.vm.heap.analysis.common.model.HeapInfo; @@ -78,10 +78,10 @@ setupDAO(); - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(HeapDAO.class)).thenReturn(dao); + StubBundleContext context = new StubBundleContext(); + context.registerService(HeapDAO.class, dao, null); - cmd = new FindObjectsCommand(serviceProvider); + cmd = new FindObjectsCommand(context); } diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/FindRootCommandTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -59,8 +59,8 @@ import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.cli.SimpleArguments; -import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.test.TestCommandContextFactory; +import com.redhat.thermostat.testutils.StubBundleContext; import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO; import com.redhat.thermostat.vm.heap.analysis.common.HeapDump; import com.redhat.thermostat.vm.heap.analysis.common.model.HeapInfo; @@ -92,10 +92,10 @@ HeapDump heapDump = setupHeapDump(); setupDAO(heapDump); - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(HeapDAO.class)).thenReturn(dao); + StubBundleContext context = new StubBundleContext(); + context.registerService(HeapDAO.class, dao, null); - cmd = new FindRootCommand(serviceProvider); + cmd = new FindRootCommand(context); } @After diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ListHeapDumpsCommandTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -38,6 +38,7 @@ 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; import static org.mockito.Mockito.when; @@ -55,13 +56,12 @@ 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.utils.OSGIUtils; import com.redhat.thermostat.storage.core.HostRef; import com.redhat.thermostat.storage.core.VmRef; import com.redhat.thermostat.storage.dao.HostInfoDAO; import com.redhat.thermostat.storage.dao.VmInfoDAO; import com.redhat.thermostat.test.TestCommandContextFactory; -import com.redhat.thermostat.vm.heap.analysis.command.internal.ListHeapDumpsCommand; +import com.redhat.thermostat.testutils.StubBundleContext; import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO; import com.redhat.thermostat.vm.heap.analysis.common.model.HeapInfo; @@ -82,7 +82,8 @@ @Test public void verifyBasics() { - Command command = new ListHeapDumpsCommand(); + StubBundleContext context = new StubBundleContext(); + Command command = new ListHeapDumpsCommand(context); assertEquals("list-heap-dumps", command.getName()); assertNotNull(command.getDescription()); assertNotNull(command.getUsage()); @@ -91,7 +92,8 @@ @Ignore @Test public void verifyOptions() { - Command command = new ListHeapDumpsCommand(); + StubBundleContext context = new StubBundleContext(); + Command command = new ListHeapDumpsCommand(context); Options options = command.getOptions(); assertNotNull(options); assertEquals(2, options.getOptions().size()); @@ -99,12 +101,13 @@ @Test public void verifyFailsWithoutHostDao() throws Exception { - OSGIUtils serviceProvider = mock(OSGIUtils.class); - - Command command = new ListHeapDumpsCommand(serviceProvider); + StubBundleContext context = new StubBundleContext(); + Command command = new ListHeapDumpsCommand(context); + TestCommandContextFactory factory = new TestCommandContextFactory(); try { command.run(factory.createContext(new SimpleArguments())); + fail(); } catch (CommandException hostDaoNotAvailableException) { assertEquals("Unable to access host information (HostInfoDAO unavailable)", hostDaoNotAvailableException.getMessage()); @@ -117,12 +120,12 @@ VmInfoDAO vmInfo = mock(VmInfoDAO.class); HeapDAO heapDao = mock(HeapDAO.class); - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(HostInfoDAO.class)).thenReturn(hostInfo); - when(serviceProvider.getServiceAllowNull(VmInfoDAO.class)).thenReturn(vmInfo); - when(serviceProvider.getServiceAllowNull(HeapDAO.class)).thenReturn(heapDao); + StubBundleContext context = new StubBundleContext(); + context.registerService(HostInfoDAO.class, hostInfo, null); + context.registerService(VmInfoDAO.class, vmInfo, null); + context.registerService(HeapDAO.class, heapDao, null); - Command command = new ListHeapDumpsCommand(serviceProvider); + Command command = new ListHeapDumpsCommand(context); TestCommandContextFactory factory = new TestCommandContextFactory(); command.run(factory.createContext(new SimpleArguments())); assertEquals("HOST ID VM ID HEAP ID TIMESTAMP\n", factory.getOutput()); @@ -151,12 +154,12 @@ when(heapDao.getAllHeapInfo(vmRef)).thenReturn(Arrays.asList(heapInfo)); - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(HostInfoDAO.class)).thenReturn(hostInfo); - when(serviceProvider.getServiceAllowNull(VmInfoDAO.class)).thenReturn(vmInfo); - when(serviceProvider.getServiceAllowNull(HeapDAO.class)).thenReturn(heapDao); + StubBundleContext context = new StubBundleContext(); + context.registerService(HostInfoDAO.class, hostInfo, null); + context.registerService(VmInfoDAO.class, vmInfo, null); + context.registerService(HeapDAO.class, heapDao, null); - Command command = new ListHeapDumpsCommand(serviceProvider); + Command command = new ListHeapDumpsCommand(context); TestCommandContextFactory factory = new TestCommandContextFactory(); command.run(factory.createContext(new SimpleArguments())); @@ -195,12 +198,12 @@ when(heapDao.getAllHeapInfo(vmRef1)).thenReturn(Arrays.asList(heapInfo)); when(heapDao.getAllHeapInfo(vmRef2)).thenReturn(Arrays.asList(heapInfo)); - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(HostInfoDAO.class)).thenReturn(hostInfo); - when(serviceProvider.getServiceAllowNull(VmInfoDAO.class)).thenReturn(vmInfo); - when(serviceProvider.getServiceAllowNull(HeapDAO.class)).thenReturn(heapDao); + StubBundleContext context = new StubBundleContext(); + context.registerService(HostInfoDAO.class, hostInfo, null); + context.registerService(VmInfoDAO.class, vmInfo, null); + context.registerService(HeapDAO.class, heapDao, null); - Command command = new ListHeapDumpsCommand(serviceProvider); + Command command = new ListHeapDumpsCommand(context); TestCommandContextFactory factory = new TestCommandContextFactory(); SimpleArguments args = new SimpleArguments(); @@ -244,12 +247,12 @@ when(heapDao.getAllHeapInfo(vmRef1)).thenReturn(Arrays.asList(heapInfo)); when(heapDao.getAllHeapInfo(vmRef2)).thenReturn(Arrays.asList(heapInfo)); - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(HostInfoDAO.class)).thenReturn(hostInfo); - when(serviceProvider.getServiceAllowNull(VmInfoDAO.class)).thenReturn(vmInfo); - when(serviceProvider.getServiceAllowNull(HeapDAO.class)).thenReturn(heapDao); + StubBundleContext context = new StubBundleContext(); + context.registerService(HostInfoDAO.class, hostInfo, null); + context.registerService(VmInfoDAO.class, vmInfo, null); + context.registerService(HeapDAO.class, heapDao, null); - Command command = new ListHeapDumpsCommand(serviceProvider); + Command command = new ListHeapDumpsCommand(context); TestCommandContextFactory factory = new TestCommandContextFactory(); SimpleArguments args = new SimpleArguments(); diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ObjectInfoCommandTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -59,8 +59,10 @@ import com.redhat.thermostat.common.cli.CommandException; import com.redhat.thermostat.common.cli.SimpleArguments; -import com.redhat.thermostat.common.utils.OSGIUtils; +import com.redhat.thermostat.common.locale.Translate; import com.redhat.thermostat.test.TestCommandContextFactory; +import com.redhat.thermostat.testutils.StubBundleContext; +import com.redhat.thermostat.vm.heap.analysis.command.locale.LocaleResources; import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO; import com.redhat.thermostat.vm.heap.analysis.common.HeapDump; import com.redhat.thermostat.vm.heap.analysis.common.model.HeapInfo; @@ -71,6 +73,7 @@ public class ObjectInfoCommandTest { + private static final Translate translator = LocaleResources.createLocalizer(); private static final String HEAP_ID = "TEST_HEAP_ID"; private ObjectInfoCommand cmd; @@ -82,11 +85,6 @@ public void setUp() { setupHeapDump(); setupDAO(); - - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(HeapDAO.class)).thenReturn(dao); - - cmd = new ObjectInfoCommand(serviceProvider); } @After @@ -150,11 +148,15 @@ @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()); } @@ -162,6 +164,8 @@ @Ignore @Test public void testOptions() { + StubBundleContext context = new StubBundleContext(); + cmd = new ObjectInfoCommand(context); Options options = cmd.getOptions(); assertEquals(2, options.getOptions().size()); @@ -180,11 +184,17 @@ @Test public void testStorageRequired() { + StubBundleContext context = new StubBundleContext(); + cmd = new ObjectInfoCommand(context); assertTrue(cmd.isStorageRequired()); } @Test public void testSimpleObject() throws CommandException { + StubBundleContext context = new StubBundleContext(); + context.registerService(HeapDAO.class, dao, null); + cmd = new ObjectInfoCommand(context); + TestCommandContextFactory factory = new TestCommandContextFactory(); SimpleArguments args = new SimpleArguments(); args.addArgument("heapId", HEAP_ID); @@ -208,6 +218,10 @@ } public void testHeapNotFound() throws CommandException { + StubBundleContext context = new StubBundleContext(); + context.registerService(HeapDAO.class, dao, null); + cmd = new ObjectInfoCommand(context); + TestCommandContextFactory factory = new TestCommandContextFactory(); SimpleArguments args = new SimpleArguments(); args.addArgument("heapId", "fluff"); @@ -222,6 +236,10 @@ } public void testObjectNotFound() throws CommandException { + StubBundleContext context = new StubBundleContext(); + context.registerService(HeapDAO.class, dao, null); + cmd = new ObjectInfoCommand(context); + TestCommandContextFactory factory = new TestCommandContextFactory(); SimpleArguments args = new SimpleArguments(); args.addArgument("heapId", HEAP_ID); @@ -234,5 +252,23 @@ assertEquals("Object not found: fluff", ex.getMessage()); } } + + @Test + public void testNoHeapDAO() throws CommandException { + StubBundleContext context = new StubBundleContext(); + cmd = new ObjectInfoCommand(context); + + TestCommandContextFactory factory = new TestCommandContextFactory(); + SimpleArguments args = new SimpleArguments(); + args.addArgument("heapId", HEAP_ID); + args.addArgument("objectId", "foo"); + + try { + cmd.run(factory.createContext(args)); + fail(); + } catch (CommandException e) { + assertEquals(translator.localize(LocaleResources.HEAP_SERVICE_UNAVAILABLE), e.getMessage()); + } + } } diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/SaveHeapDumpToFileCommandTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -39,6 +39,7 @@ 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; @@ -47,23 +48,34 @@ import java.io.FileNotFoundException; import java.nio.charset.Charset; +import org.junit.Before; import org.junit.Test; 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.utils.OSGIUtils; +import com.redhat.thermostat.common.locale.Translate; import com.redhat.thermostat.test.TestCommandContextFactory; -import com.redhat.thermostat.vm.heap.analysis.command.internal.SaveHeapDumpToFileCommand; +import com.redhat.thermostat.testutils.StubBundleContext; import com.redhat.thermostat.vm.heap.analysis.command.internal.SaveHeapDumpToFileCommand.FileStreamCreator; +import com.redhat.thermostat.vm.heap.analysis.command.locale.LocaleResources; import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO; import com.redhat.thermostat.vm.heap.analysis.common.model.HeapInfo; public class SaveHeapDumpToFileCommandTest { + + private static final Translate translator = LocaleResources.createLocalizer(); + + private StubBundleContext context; + + @Before + public void setup() { + context = new StubBundleContext(); + } @Test public void verifyBasicInformation() { - Command command = new SaveHeapDumpToFileCommand(); + Command command = new SaveHeapDumpToFileCommand(context, mock(FileStreamCreator.class)); assertEquals("save-heap-dump-to-file", command.getName()); assertNotNull(command.getDescription()); assertNotNull(command.getUsage()); @@ -78,10 +90,10 @@ args.addArgument("vmId", "1"); args.addArgument("file", "heap-id-1"); - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(HeapDAO.class)).thenReturn(mock(HeapDAO.class)); - - Command command = new SaveHeapDumpToFileCommand(serviceProvider, mock(FileStreamCreator.class)); + HeapDAO heapDAO = mock(HeapDAO.class); + context.registerService(HeapDAO.class, heapDAO, null); + + Command command = new SaveHeapDumpToFileCommand(context, mock(FileStreamCreator.class)); command.run(factory.createContext(args)); } @@ -94,10 +106,10 @@ args.addArgument("vmId", "1"); args.addArgument("heapId", "heap-id-1"); - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(HeapDAO.class)).thenReturn(mock(HeapDAO.class)); + HeapDAO heapDAO = mock(HeapDAO.class); + context.registerService(HeapDAO.class, heapDAO, null); - Command command = new SaveHeapDumpToFileCommand(serviceProvider, mock(FileStreamCreator.class)); + Command command = new SaveHeapDumpToFileCommand(context, mock(FileStreamCreator.class)); command.run(factory.createContext(args)); } @@ -111,13 +123,10 @@ ByteArrayOutputStream heapDumpStream = new ByteArrayOutputStream(); HeapDAO heapDao = mock(HeapDAO.class); - HeapInfo info = mock(HeapInfo.class); when(heapDao.getHeapInfo(HEAP_ID)).thenReturn(info); when(heapDao.getHeapDumpData(info)).thenReturn(new ByteArrayInputStream(HEAP_CONTENT_BYTES)); - - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(HeapDAO.class)).thenReturn(heapDao); + context.registerService(HeapDAO.class, heapDao, null); TestCommandContextFactory factory = new TestCommandContextFactory(); @@ -128,11 +137,31 @@ FileStreamCreator creator = mock(FileStreamCreator.class); when(creator.createOutputStream(FILE_NAME)).thenReturn(heapDumpStream); - Command command = new SaveHeapDumpToFileCommand(serviceProvider, creator); + Command command = new SaveHeapDumpToFileCommand(context, creator); command.run(factory.createContext(args)); assertArrayEquals(HEAP_CONTENT_BYTES, heapDumpStream.toByteArray()); } + + @Test + public void testNoHeapDAO() throws CommandException { + TestCommandContextFactory factory = new TestCommandContextFactory(); + + SimpleArguments args = new SimpleArguments(); + args.addArgument("hostId", "host-id"); + args.addArgument("vmId", "1"); + args.addArgument("heapId", "heap-id-1"); + args.addArgument("file", "heap-id-1"); + + Command command = new SaveHeapDumpToFileCommand(context, mock(FileStreamCreator.class)); + + try { + command.run(factory.createContext(args)); + fail(); + } catch (CommandException e) { + assertEquals(translator.localize(LocaleResources.HEAP_SERVICE_UNAVAILABLE), e.getMessage()); + } + } } diff -r c6c9f2c7961c -r 3526e4183714 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 Fri Mar 22 09:24:14 2013 +0100 +++ b/vm-heap-analysis/command/src/test/java/com/redhat/thermostat/vm/heap/analysis/command/internal/ShowHeapHistogramCommandTest.java Fri Mar 22 12:01:35 2013 -0400 @@ -38,9 +38,9 @@ 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; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import org.junit.Test; @@ -48,8 +48,10 @@ 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.utils.OSGIUtils; +import com.redhat.thermostat.common.locale.Translate; import com.redhat.thermostat.test.TestCommandContextFactory; +import com.redhat.thermostat.testutils.StubBundleContext; +import com.redhat.thermostat.vm.heap.analysis.command.locale.LocaleResources; import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO; import com.redhat.thermostat.vm.heap.analysis.common.ObjectHistogram; import com.redhat.thermostat.vm.heap.analysis.common.model.HeapInfo; @@ -57,10 +59,13 @@ import com.sun.tools.hat.internal.model.JavaHeapObject; public class ShowHeapHistogramCommandTest { + + private static final Translate translator = LocaleResources.createLocalizer(); @Test public void verifyBasics() { - Command command = new ShowHeapHistogramCommand(); + StubBundleContext context = new StubBundleContext(); + Command command = new ShowHeapHistogramCommand(context); assertEquals("show-heap-histogram", command.getName()); assertNotNull(command.getDescription()); @@ -97,10 +102,10 @@ when(heapDao.getHeapInfo(HEAP_ID)).thenReturn(heapInfo); when(heapDao.getHistogram(heapInfo)).thenReturn(histo); - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(HeapDAO.class)).thenReturn(heapDao); + StubBundleContext context = new StubBundleContext(); + context.registerService(HeapDAO.class, heapDao, null); - Command command = new ShowHeapHistogramCommand(serviceProvider); + Command command = new ShowHeapHistogramCommand(context); TestCommandContextFactory factory = new TestCommandContextFactory(); SimpleArguments args = new SimpleArguments(); @@ -110,7 +115,6 @@ assertEquals("class1 2 8\n" + "verylongclassnameclass2 1 10\n", factory.getOutput()); - verify(serviceProvider).ungetService(HeapDAO.class, heapDao); } @Test @@ -122,10 +126,10 @@ when(heapDao.getHeapInfo(BAD_HEAP_ID)).thenReturn(null); when(heapDao.getHistogram(any(HeapInfo.class))).thenReturn(null); - OSGIUtils serviceProvider = mock(OSGIUtils.class); - when(serviceProvider.getServiceAllowNull(HeapDAO.class)).thenReturn(heapDao); + StubBundleContext context = new StubBundleContext(); + context.registerService(HeapDAO.class, heapDao, null); - Command command = new ShowHeapHistogramCommand(serviceProvider); + Command command = new ShowHeapHistogramCommand(context); TestCommandContextFactory factory = new TestCommandContextFactory(); SimpleArguments args = new SimpleArguments(); @@ -134,7 +138,25 @@ command.run(factory.createContext(args)); assertEquals("Heap ID not found: " + BAD_HEAP_ID + "\n", factory.getOutput()); - verify(serviceProvider).ungetService(HeapDAO.class, heapDao); + } + + @Test + public void testNoHeapDAO() throws CommandException { + final String HEAP_ID = "heap-id-1"; + StubBundleContext context = new StubBundleContext(); + + Command command = new ShowHeapHistogramCommand(context); + TestCommandContextFactory factory = new TestCommandContextFactory(); + + SimpleArguments args = new SimpleArguments(); + args.addArgument("heapId", HEAP_ID); + + try { + command.run(factory.createContext(args)); + fail(); + } catch (CommandException e) { + assertEquals(translator.localize(LocaleResources.HEAP_SERVICE_UNAVAILABLE), e.getMessage()); + } } }