Mercurial > hg > release > thermostat-0.7
changeset 757:1d2dd10ff10d
Remove OSGiContext interface.
In a previous review I got the impression that using OSGiContext is bad. Since
it was used by GuiClientCommand and ShellCommand only I thought we might as
well remove it. Similar to an earlier patch I've used FrameworkUtil to get at a
BundleContext and used static mocking for ActivatorTest in client-heapdumper.
This should reduce things which we special-case due to a command requiring a
bundle context.
Reviewed-by: neugens
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2012-October/003983.html
| author | Severin Gehwolf <sgehwolf@redhat.com> |
|---|---|
| date | Tue, 30 Oct 2012 12:41:23 +0100 |
| parents | 998a0bc94088 |
| children | a1eaa2e0c7cf |
| files | client/heapdumper/core/pom.xml client/heapdumper/core/src/test/java/com/redhat/thermostat/client/heap/internal/ActivatorTest.java client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/GUIClientCommand.java client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/osgi/ThermostatActivator.java client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/GUIClientCommandTest.java common/core/src/main/java/com/redhat/thermostat/common/cli/CommandRegistryImpl.java common/core/src/main/java/com/redhat/thermostat/common/cli/OSGiContext.java tools/src/main/java/com/redhat/thermostat/tools/cli/ShellCommand.java tools/src/test/java/com/redhat/thermostat/tools/cli/ShellCommandTest.java |
| diffstat | 9 files changed, 52 insertions(+), 81 deletions(-) [+] |
line wrap: on
line diff
--- a/client/heapdumper/core/pom.xml Tue Oct 30 09:11:50 2012 -0400 +++ b/client/heapdumper/core/pom.xml Tue Oct 30 12:41:23 2012 +0100 @@ -83,6 +83,16 @@ <scope>test</scope> </dependency> <dependency> + <groupId>org.powermock</groupId> + <artifactId>powermock-api-mockito</artifactId> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.powermock</groupId> + <artifactId>powermock-module-junit4</artifactId> + <scope>test</scope> + </dependency> + <dependency> <groupId>org.osgi</groupId> <artifactId>org.osgi.core</artifactId> <scope>provided</scope>
--- a/client/heapdumper/core/src/test/java/com/redhat/thermostat/client/heap/internal/ActivatorTest.java Tue Oct 30 09:11:50 2012 -0400 +++ b/client/heapdumper/core/src/test/java/com/redhat/thermostat/client/heap/internal/ActivatorTest.java Tue Oct 30 12:41:23 2012 +0100 @@ -37,8 +37,16 @@ package com.redhat.thermostat.client.heap.internal; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.mock; import org.junit.Test; +import org.junit.runner.RunWith; +import org.osgi.framework.Bundle; +import org.osgi.framework.FrameworkUtil; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; import com.redhat.thermostat.client.heap.cli.DumpHeapCommand; import com.redhat.thermostat.client.heap.cli.FindObjectsCommand; @@ -49,12 +57,26 @@ import com.redhat.thermostat.client.heap.cli.ShowHeapHistogramCommand; import com.redhat.thermostat.common.cli.Command; import com.redhat.thermostat.test.StubBundleContext; +import com.redhat.thermostat.tools.cli.ShellCommand; +@RunWith(PowerMockRunner.class) +@PrepareForTest(FrameworkUtil.class) public class ActivatorTest { @Test public void testCommandsRegistered() throws Exception { + /* + * ServiceLoader pulls in all commands from bundles on the classpath. + * During maven builds the tools bundle is on the classpath which uses + * the no-arg constructor of ShellCommand. That in turn uses + * FrameworkUtil. We need to mock FrameworkUtil here in order to avoid + * that throwing NPEs. + */ + PowerMockito.mockStatic(FrameworkUtil.class); + Bundle mockBundle = mock(Bundle.class); + when(FrameworkUtil.getBundle(ShellCommand.class)).thenReturn(mockBundle); StubBundleContext ctx = new StubBundleContext(); + when(mockBundle.getBundleContext()).thenReturn(ctx); Activator activator = new Activator(); activator.start(ctx);
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/GUIClientCommand.java Tue Oct 30 09:11:50 2012 -0400 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/GUIClientCommand.java Tue Oct 30 12:41:23 2012 +0100 @@ -37,6 +37,7 @@ package com.redhat.thermostat.client.swing.internal; import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; import com.redhat.thermostat.client.osgi.service.ApplicationService; import com.redhat.thermostat.client.osgi.service.ContextAction; @@ -44,21 +45,20 @@ import com.redhat.thermostat.client.swing.internal.osgi.ContextActionServiceProvider; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; -import com.redhat.thermostat.common.cli.OSGiContext; import com.redhat.thermostat.common.cli.SimpleCommand; -public class GUIClientCommand extends SimpleCommand implements OSGiContext { +public class GUIClientCommand extends SimpleCommand { private BundleContext context; private Main clientMain; public GUIClientCommand(Main clientMain) { - this.clientMain = clientMain; + this(clientMain, FrameworkUtil.getBundle(GUIClientCommand.class).getBundleContext()); } - - @Override - public void setBundleContext(BundleContext context) { + + GUIClientCommand(Main clientMain, BundleContext context) { this.context = context; + this.clientMain = clientMain; } @Override
--- a/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/osgi/ThermostatActivator.java Tue Oct 30 09:11:50 2012 -0400 +++ b/client/swing/src/main/java/com/redhat/thermostat/client/swing/internal/osgi/ThermostatActivator.java Tue Oct 30 12:41:23 2012 +0100 @@ -136,7 +136,6 @@ Main main = new Main(keyring, uiFacadeFactory, new String[0]); GUIClientCommand cmd = new GUIClientCommand(main); - cmd.setBundleContext(context); cmdReg.registerCommands(Arrays.asList(cmd)); return super.addingService(reference);
--- a/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/GUIClientCommandTest.java Tue Oct 30 09:11:50 2012 -0400 +++ b/client/swing/src/test/java/com/redhat/thermostat/client/swing/internal/GUIClientCommandTest.java Tue Oct 30 12:41:23 2012 +0100 @@ -68,7 +68,8 @@ @Before public void setUp() { clientMain = mock(Main.class); - cmd = new GUIClientCommand(clientMain); + BundleContext ctxt = mock(BundleContext.class); + cmd = new GUIClientCommand(clientMain, ctxt); } @After @@ -85,7 +86,7 @@ CommandContext cmdCtx = mock(CommandContext.class); when(cmdCtx.getCommandContextFactory()).thenReturn(cmdCtxFactory); - cmd.setBundleContext(bCtx); + cmd = new GUIClientCommand(clientMain, bCtx); cmd.run(cmdCtx); verify(clientMain).run();
--- a/common/core/src/main/java/com/redhat/thermostat/common/cli/CommandRegistryImpl.java Tue Oct 30 09:11:50 2012 -0400 +++ b/common/core/src/main/java/com/redhat/thermostat/common/cli/CommandRegistryImpl.java Tue Oct 30 12:41:23 2012 +0100 @@ -63,9 +63,6 @@ @Override public void registerCommand(Command cmd) { - if (cmd instanceof OSGiContext) { - ((OSGiContext) cmd).setBundleContext(context); - } proxy.registerService(cmd, cmd.getName()); myRegisteredCommands.add(cmd); }
--- a/common/core/src/main/java/com/redhat/thermostat/common/cli/OSGiContext.java Tue Oct 30 09:11:50 2012 -0400 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,48 +0,0 @@ -/* - * Copyright 2012 Red Hat, Inc. - * - * This file is part of Thermostat. - * - * Thermostat is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published - * by the Free Software Foundation; either version 2, or (at your - * option) any later version. - * - * Thermostat is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with Thermostat; see the file COPYING. If not see - * <http://www.gnu.org/licenses/>. - * - * Linking this code with other modules is making a combined work - * based on this code. Thus, the terms and conditions of the GNU - * General Public License cover the whole combination. - * - * As a special exception, the copyright holders of this code give - * you permission to link this code with independent modules to - * produce an executable, regardless of the license terms of these - * independent modules, and to copy and distribute the resulting - * executable under terms of your choice, provided that you also - * meet, for each linked independent module, the terms and conditions - * of the license of that module. An independent module is a module - * which is not derived from or based on this code. If you modify - * this code, you may extend this exception to your version of the - * library, but you are not obligated to do so. If you do not wish - * to do so, delete this exception statement from your version. - */ - -package com.redhat.thermostat.common.cli; - -import org.osgi.framework.BundleContext; - -/** - * Interface that allows classes to carry a {@link BundleContext}. - */ -public interface OSGiContext { - - void setBundleContext(BundleContext context); - -}
--- a/tools/src/main/java/com/redhat/thermostat/tools/cli/ShellCommand.java Tue Oct 30 09:11:50 2012 -0400 +++ b/tools/src/main/java/com/redhat/thermostat/tools/cli/ShellCommand.java Tue Oct 30 12:41:23 2012 +0100 @@ -37,7 +37,6 @@ package com.redhat.thermostat.tools.cli; import java.io.IOException; -import java.io.OutputStreamWriter; import java.util.Arrays; import java.util.logging.Level; import java.util.logging.Logger; @@ -50,18 +49,18 @@ import jline.console.history.PersistentHistory; import org.osgi.framework.BundleContext; +import org.osgi.framework.FrameworkUtil; import org.osgi.framework.ServiceReference; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; -import com.redhat.thermostat.common.cli.OSGiContext; import com.redhat.thermostat.common.cli.SimpleCommand; import com.redhat.thermostat.common.config.ConfigUtils; import com.redhat.thermostat.common.config.InvalidConfigurationException; import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.launcher.Launcher; -public class ShellCommand extends SimpleCommand implements OSGiContext { +public class ShellCommand extends SimpleCommand { private static final Logger logger = LoggingUtils.getLogger(ShellCommand.class); @@ -88,16 +87,12 @@ } public ShellCommand() { - this(new HistoryProvider()); + this(FrameworkUtil.getBundle(ShellCommand.class).getBundleContext(), new HistoryProvider()); } - ShellCommand(HistoryProvider provider) { + ShellCommand(BundleContext context, HistoryProvider provider) { this.historyProvider = provider; - } - - @Override - public void setBundleContext(BundleContext context) { - bundleContext = context; + this.bundleContext = context; } @Override
--- a/tools/src/test/java/com/redhat/thermostat/tools/cli/ShellCommandTest.java Tue Oct 30 09:11:50 2012 -0400 +++ b/tools/src/test/java/com/redhat/thermostat/tools/cli/ShellCommandTest.java Tue Oct 30 12:41:23 2012 +0100 @@ -69,15 +69,18 @@ public class ShellCommandTest { private ShellCommand cmd; + private BundleContext bundleContext; @Before public void setUp() { - cmd = new ShellCommand(); + bundleContext = mock(BundleContext.class); + cmd = new ShellCommand(bundleContext, new HistoryProvider()); } @After public void tearDown() { cmd = null; + bundleContext = null; TerminalFactory.registerFlavor(Flavor.UNIX, UnixTerminal.class); TerminalFactory.reset(); } @@ -85,8 +88,6 @@ @Test public void testBasic() throws CommandException { ServiceReference ref = mock(ServiceReference.class); - BundleContext bundleContext = mock(BundleContext.class); - cmd.setBundleContext(bundleContext); when(bundleContext.getServiceReference(Launcher.class.getName())).thenReturn(ref); Launcher launcher = mock(Launcher.class); @@ -153,16 +154,13 @@ when(provider.get()).thenReturn(history); ServiceReference ref = mock(ServiceReference.class); - BundleContext bundleContext = mock(BundleContext.class); - cmd.setBundleContext(bundleContext); when(bundleContext.getServiceReference(Launcher.class.getName())).thenReturn(ref); Launcher launcher = mock(Launcher.class); when(bundleContext.getService(ref)).thenReturn(launcher); TestCommandContextFactory ctxFactory = new TestCommandContextFactory(bundleContext); - cmd = new ShellCommand(provider); - cmd.setBundleContext(bundleContext); + cmd = new ShellCommand(bundleContext, provider); // "\u001b[A" is the escape code for up-arrow. use xxd -p to generate ctxFactory.setInput("\u001b[A\nexit\n"); @@ -184,15 +182,12 @@ when(provider.get()).thenReturn(mockHistory); ServiceReference ref = mock(ServiceReference.class); - BundleContext bundleContext = mock(BundleContext.class); when(bundleContext.getServiceReference(Launcher.class.getName())).thenReturn(ref); Launcher launcher = mock(Launcher.class); when(bundleContext.getService(ref)).thenReturn(launcher); TestCommandContextFactory ctxFactory = new TestCommandContextFactory(bundleContext); - cmd.setBundleContext(bundleContext); - cmd = new ShellCommand(provider); - cmd.setBundleContext(bundleContext); + cmd = new ShellCommand(bundleContext, provider); ctxFactory.setInput("add-to-history\nexit\n"); Arguments args = new SimpleArguments();