# HG changeset patch # User Severin Gehwolf # Date 1362749315 -3600 # Node ID 3f0416e9db80dc9c9db760f3472cf647cfeb224a # Parent edaf77f17cbdd82cb16c0548cc97e21993a02b24 Don't use System.exit anywhere other than in Laucher. Reviewed-by: omajid Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-March/006029.html diff -r edaf77f17cbd -r 3f0416e9db80 agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/Activator.java --- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/Activator.java Tue Mar 26 11:17:08 2013 -0400 +++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/Activator.java Fri Mar 08 14:28:35 2013 +0100 @@ -40,8 +40,11 @@ import org.osgi.framework.BundleActivator; import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceReference; +import org.osgi.util.tracker.ServiceTracker; import com.redhat.thermostat.agent.cli.impl.db.StorageCommand; +import com.redhat.thermostat.common.ExitStatus; import com.redhat.thermostat.common.cli.CommandRegistry; import com.redhat.thermostat.common.cli.CommandRegistryImpl; @@ -49,19 +52,33 @@ private CommandRegistry reg; private AgentApplication agentApplication; + @SuppressWarnings("rawtypes") + private ServiceTracker exitStatusTracker; + @SuppressWarnings({ "rawtypes", "unchecked" }) @Override public void start(final BundleContext context) throws Exception { - reg = new CommandRegistryImpl(context); - agentApplication = new AgentApplication(context); - reg.registerCommands(Arrays.asList(new ServiceCommand(), - new StorageCommand(), agentApplication)); + exitStatusTracker = new ServiceTracker(context, ExitStatus.class, null) { + + @Override + public Object addingService(ServiceReference reference) { + ExitStatus exitStatus = (ExitStatus)context.getService(reference); + reg = new CommandRegistryImpl(context); + agentApplication = new AgentApplication(context, exitStatus); + reg.registerCommands(Arrays.asList(new ServiceCommand(context), + new StorageCommand(exitStatus), agentApplication)); + return exitStatus; + } + + }; + exitStatusTracker.open(); } @Override public void stop(BundleContext context) throws Exception { agentApplication.shutdown(); reg.unregisterCommands(); + exitStatusTracker.close(); } } diff -r edaf77f17cbd -r 3f0416e9db80 agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java --- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java Tue Mar 26 11:17:08 2013 -0400 +++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/AgentApplication.java Fri Mar 08 14:28:35 2013 +0100 @@ -55,7 +55,7 @@ import com.redhat.thermostat.agent.config.AgentStartupConfiguration; import com.redhat.thermostat.backend.BackendRegistry; import com.redhat.thermostat.backend.BackendService; -import com.redhat.thermostat.common.Constants; +import com.redhat.thermostat.common.ExitStatus; import com.redhat.thermostat.common.LaunchException; import com.redhat.thermostat.common.MultipleServiceTracker; import com.redhat.thermostat.common.MultipleServiceTracker.Action; @@ -84,19 +84,22 @@ private AgentStartupConfiguration configuration; private AgentOptionParser parser; private DbServiceFactory dbServiceFactory; + @SuppressWarnings("rawtypes") private ServiceTracker configServerTracker; private MultipleServiceTracker daoTracker; + private final ExitStatus exitStatus; private CountDownLatch shutdownLatch; - public AgentApplication(BundleContext bundleContext) { - this(bundleContext, new ConfigurationCreator(), new DbServiceFactory()); + public AgentApplication(BundleContext bundleContext, ExitStatus exitStatus) { + this(bundleContext, exitStatus, new ConfigurationCreator(), new DbServiceFactory()); } - AgentApplication(BundleContext bundleContext, ConfigurationCreator configurationCreator, DbServiceFactory dbServiceFactory) { + AgentApplication(BundleContext bundleContext, ExitStatus exitStatus, ConfigurationCreator configurationCreator, DbServiceFactory dbServiceFactory) { this.bundleContext = bundleContext; this.configurationCreator = configurationCreator; this.dbServiceFactory = dbServiceFactory; + this.exitStatus = exitStatus; } private void parseArguments(Arguments args) throws InvalidConfigurationException { @@ -104,6 +107,7 @@ parser.parse(); } + @SuppressWarnings({ "rawtypes", "unchecked" }) private void runAgent(CommandContext ctx) { long startTime = System.currentTimeMillis(); configuration.setStartTime(startTime); @@ -237,7 +241,7 @@ } - private Agent startAgent(final Logger logger, final Storage storage, + Agent startAgent(final Logger logger, final Storage storage, AgentInfoDAO agentInfoDAO, BackendInfoDAO backendInfoDAO) { BackendRegistry backendRegistry = null; try { @@ -245,7 +249,11 @@ } catch (Exception e) { logger.log(Level.SEVERE, "Could not get BackendRegistry instance.", e); - System.exit(Constants.EXIT_ERROR); + exitStatus.setExitStatus(ExitStatus.EXIT_ERROR); + shutdown(); + // Since this would throw NPE's down the line if we continue in this + // method, let's fail right and early :) + throw new RuntimeException(e); } final Agent agent = new Agent(backendRegistry, configuration, storage, agentInfoDAO, backendInfoDAO); @@ -259,7 +267,8 @@ logger.log(Level.SEVERE, "Agent could not start, probably because a configured backend could not be activated.", le); - System.exit(Constants.EXIT_ERROR); + exitStatus.setExitStatus(ExitStatus.EXIT_ERROR); + shutdown(); } logger.fine("Agent started."); @@ -267,7 +276,7 @@ logger.info("agent started."); return agent; } - + private void handleConnected(final ConfigurationServer configServer, final Logger logger, final CountDownLatch shutdownLatch) { Class[] deps = new Class[] { diff -r edaf77f17cbd -r 3f0416e9db80 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 Tue Mar 26 11:17:08 2013 -0400 +++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/ServiceCommand.java Fri Mar 08 14:28:35 2013 +0100 @@ -71,10 +71,6 @@ private BundleContext context; private Launcher launcher; - public ServiceCommand() { - this(FrameworkUtil.getBundle(ServiceCommand.class).getBundleContext()); - } - public ServiceCommand(BundleContext context) { this.context = context; listeners = new ArrayList<>(); diff -r edaf77f17cbd -r 3f0416e9db80 agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommand.java --- a/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommand.java Tue Mar 26 11:17:08 2013 -0400 +++ b/agent/cli/src/main/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommand.java Fri Mar 08 14:28:35 2013 +0100 @@ -39,11 +39,6 @@ import java.io.File; import java.io.IOException; -import org.osgi.framework.BundleContext; -import org.osgi.framework.FrameworkUtil; -import org.osgi.framework.ServiceReference; - -import com.redhat.thermostat.common.Constants; import com.redhat.thermostat.common.ExitStatus; import com.redhat.thermostat.common.cli.AbstractStateNotifyingCommand; import com.redhat.thermostat.common.cli.Arguments; @@ -60,17 +55,12 @@ private DBStartupConfiguration configuration; private DBOptionParser parser; - private BundleContext context; + private final ExitStatus exitStatus; private MongoProcessRunner runner; - public StorageCommand() { - this(FrameworkUtil.getBundle(StorageCommand.class).getBundleContext()); - } - - // For testing - StorageCommand(BundleContext context) { - this.context = context; + public StorageCommand(ExitStatus exitStatus) { + this.exitStatus = exitStatus; } private void parseArguments(Arguments args) throws InvalidConfigurationException { @@ -134,7 +124,7 @@ } catch (ApplicationException | InvalidConfigurationException | IOException e) { // something went wrong set status appropriately. This makes sure // that the JVM exits with this status. - setExitStatus(Constants.EXIT_ERROR); + exitStatus.setExitStatus(ExitStatus.EXIT_ERROR); // rethrow throw e; } @@ -149,7 +139,7 @@ } catch (ApplicationException | InvalidConfigurationException | InterruptedException | IOException e) { // something went wrong set status appropriately. This makes sure // that the JVM exits with this status. - setExitStatus(Constants.EXIT_ERROR); + exitStatus.setExitStatus(ExitStatus.EXIT_ERROR); throw e; } getNotifier().fireAction(ApplicationState.STOP); @@ -167,14 +157,6 @@ throw new InvalidConfigurationException("database directories do not exist..."); } } - - @SuppressWarnings({ "unchecked", "rawtypes" }) - private void setExitStatus(final int newStatus) { - ServiceReference exitStatusRef = context.getServiceReference(ExitStatus.class); - ExitStatus status = (ExitStatus) context.getService(exitStatusRef); - status.setExitStatus(newStatus); - context.ungetService(exitStatusRef); - } public DBStartupConfiguration getConfiguration() { return configuration; diff -r edaf77f17cbd -r 3f0416e9db80 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 Tue Mar 26 11:17:08 2013 -0400 +++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/ActivatorTest.java Fri Mar 08 14:28:35 2013 +0100 @@ -43,29 +43,19 @@ 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.agent.cli.impl.db.StorageCommand; +import com.redhat.thermostat.common.ExitStatus; import com.redhat.thermostat.common.cli.Command; import com.redhat.thermostat.testutils.StubBundleContext; -@RunWith(PowerMockRunner.class) -@PrepareForTest({FrameworkUtil.class}) public class ActivatorTest { @Test - public void verifyActivatorRegistersCommands() throws Exception { - - PowerMockito.mockStatic(FrameworkUtil.class); - Bundle mockBundle = mock(Bundle.class); - when(FrameworkUtil.getBundle(any(Class.class))).thenReturn(mockBundle); - + public void verifyActivatorRegistersCommands() throws Exception { StubBundleContext bundleContext = new StubBundleContext(); + ExitStatus exitStatus = mock(ExitStatus.class); + bundleContext.registerService(ExitStatus.class, exitStatus, null); Activator activator = new Activator(); @@ -77,7 +67,7 @@ activator.stop(bundleContext); - assertEquals(0, bundleContext.getAllServices().size()); + assertEquals(1, bundleContext.getAllServices().size()); } } diff -r edaf77f17cbd -r 3f0416e9db80 agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java --- a/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java Tue Mar 26 11:17:08 2013 -0400 +++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/AgentApplicationTest.java Fri Mar 08 14:28:35 2013 +0100 @@ -39,26 +39,39 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.powermock.api.mockito.PowerMockito.whenNew; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.logging.Logger; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import org.osgi.framework.BundleContext; +import org.osgi.framework.InvalidSyntaxException; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; -import com.redhat.thermostat.agent.cli.impl.AgentApplication; +import com.redhat.thermostat.agent.Agent; import com.redhat.thermostat.agent.cli.impl.AgentApplication.ConfigurationCreator; import com.redhat.thermostat.agent.command.ConfigurationServer; import com.redhat.thermostat.agent.config.AgentStartupConfiguration; +import com.redhat.thermostat.backend.BackendRegistry; +import com.redhat.thermostat.common.ExitStatus; +import com.redhat.thermostat.common.LaunchException; import com.redhat.thermostat.common.cli.Arguments; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; @@ -72,15 +85,19 @@ import com.redhat.thermostat.storage.dao.BackendInfoDAO; import com.redhat.thermostat.testutils.StubBundleContext; +@PrepareForTest({ AgentApplication.class }) +@RunWith(PowerMockRunner.class) public class AgentApplicationTest { // TODO: Test i18nized versions when they come. private StubBundleContext context; - private AgentApplication agent; private ConfigurationServer configServer; private DbService dbService; + private ConfigurationCreator configCreator; + private ExitStatus exitStatus; + private DbServiceFactory dbServiceFactory; @Before public void setUp() throws InvalidConfigurationException { @@ -90,7 +107,7 @@ AgentStartupConfiguration config = mock(AgentStartupConfiguration.class); when(config.getDBConnectionString()).thenReturn("test string; please ignore"); - ConfigurationCreator configCreator = mock(ConfigurationCreator.class); + configCreator = mock(ConfigurationCreator.class); when(configCreator.create()).thenReturn(config); Storage storage = mock(Storage.class); @@ -101,33 +118,96 @@ context.registerService(BackendInfoDAO.class.getName(), backendInfoDAO, null); configServer = mock(ConfigurationServer.class); context.registerService(ConfigurationServer.class.getName(), configServer, null); - DbServiceFactory dbServiceFactory = mock(DbServiceFactory.class); + dbServiceFactory = mock(DbServiceFactory.class); dbService = mock(DbService.class); when(dbServiceFactory.createDbService(anyString(), anyString(), anyString())).thenReturn(dbService); - agent = new AgentApplication(context, configCreator, dbServiceFactory); + exitStatus = mock(ExitStatus.class); } @After public void tearDown() { - agent = null; + context = null; + configServer = null; + dbService = null; + configCreator = null; + dbServiceFactory = null; + exitStatus = null; } @Test public void testName() { + AgentApplication agent = new AgentApplication(context, exitStatus, configCreator, dbServiceFactory); String name = agent.getName(); assertEquals("agent", name); } @Test public void testDescAndUsage() { + AgentApplication agent = new AgentApplication(context, exitStatus, configCreator, dbServiceFactory); assertNotNull(agent.getDescription()); assertNotNull(agent.getUsage()); } @Test public void testAgentStartup() throws CommandException, InterruptedException { + final AgentApplication agent = new AgentApplication(context, exitStatus, configCreator, dbServiceFactory); + final CountDownLatch latch = new CountDownLatch(1); + final CommandException[] ce = new CommandException[1]; final long timeoutMillis = 5000L; + + startAgentRunThread(timeoutMillis, agent, ce, latch); + + boolean ret = latch.await(timeoutMillis, TimeUnit.MILLISECONDS); + if (ce[0] != null) { + throw ce[0]; + } + if (!ret) { + fail("Timeout expired!"); + } + + } + + @SuppressWarnings("unchecked") + @Test + public void verifyBackendRegistryProblemsSetsExitStatus() throws Exception { + whenNew(BackendRegistry.class).withParameterTypes(BundleContext.class) + .withArguments(any(BundleContext.class)) + .thenThrow(InvalidSyntaxException.class); + final AgentApplication agent = new AgentApplication(context, + exitStatus, configCreator, dbServiceFactory); + try { + agent.startAgent(mock(Logger.class), null, null, null); + } catch (RuntimeException e) { + assertEquals(InvalidSyntaxException.class, e.getCause().getClass()); + } + verify(exitStatus).setExitStatus(ExitStatus.EXIT_ERROR); + } + + @Test + public void verifyAgentLaunchExceptionSetsExitStatus() throws Exception { + whenNew(BackendRegistry.class).withParameterTypes(BundleContext.class) + .withArguments(any(BundleContext.class)) + .thenReturn(mock(BackendRegistry.class)); + Agent mockAgent = mock(Agent.class); + whenNew(Agent.class).withParameterTypes(BackendRegistry.class, + AgentStartupConfiguration.class, Storage.class, + AgentInfoDAO.class, BackendInfoDAO.class).withArguments( + any(BackendRegistry.class), + any(AgentStartupConfiguration.class), any(Storage.class), + any(AgentInfoDAO.class), any(BackendInfoDAO.class)).thenReturn(mockAgent); + doThrow(LaunchException.class).when(mockAgent).start(); + final AgentApplication agent = new AgentApplication(context, + exitStatus, configCreator, dbServiceFactory); + try { + agent.startAgent(mock(Logger.class), null, null, null); + } catch (RuntimeException e) { + fail("Should not have thrown RuntimeException"); + } + verify(exitStatus).setExitStatus(ExitStatus.EXIT_ERROR); + } + + private void startAgentRunThread(final long timoutMillis, final AgentApplication agent, final CommandException[] ce, final CountDownLatch latch) { Arguments args = mock(Arguments.class); final CommandContext commandContext = mock(CommandContext.class); when(commandContext.getArguments()).thenReturn(args); @@ -147,9 +227,6 @@ }).when(dbService).connect(); - final CountDownLatch latch = new CountDownLatch(1); - - final CommandException[] ce = new CommandException[1]; // Run agent in a new thread so we can timeout on failure Thread t = new Thread(new Runnable() { @@ -174,14 +251,6 @@ }); t.start(); - boolean ret = latch.await(timeoutMillis, TimeUnit.MILLISECONDS); - if (ce[0] != null) { - throw ce[0]; - } - if (!ret) { - fail("Timeout expired!"); - } - } } diff -r edaf77f17cbd -r 3f0416e9db80 agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommandTest.java --- a/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommandTest.java Tue Mar 26 11:17:08 2013 -0400 +++ b/agent/cli/src/test/java/com/redhat/thermostat/agent/cli/impl/db/StorageCommandTest.java Fri Mar 08 14:28:35 2013 +0100 @@ -55,12 +55,9 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.osgi.framework.BundleContext; -import org.osgi.framework.ServiceReference; import com.redhat.thermostat.common.ActionEvent; import com.redhat.thermostat.common.ActionListener; -import com.redhat.thermostat.common.Constants; import com.redhat.thermostat.common.ExitStatus; import com.redhat.thermostat.common.cli.CommandContext; import com.redhat.thermostat.common.cli.CommandException; @@ -68,7 +65,6 @@ import com.redhat.thermostat.common.config.InvalidConfigurationException; import com.redhat.thermostat.common.tools.ApplicationException; import com.redhat.thermostat.common.tools.ApplicationState; -import com.redhat.thermostat.testutils.StubBundleContext; public class StorageCommandTest { @@ -77,11 +73,11 @@ private static final String DB = "storage/db"; private String tmpDir; - private BundleContext context; + private ExitStatus exitStatus; @Before public void setup() { - context = mock(BundleContext.class); + exitStatus = mock(ExitStatus.class); // need to create a dummy config file for the test try { Random random = new Random(); @@ -113,7 +109,7 @@ @After public void tearDown() { - context = null; + exitStatus = null; } @Test @@ -125,7 +121,7 @@ CommandContext ctx = mock(CommandContext.class); when(ctx.getArguments()).thenReturn(args); - StorageCommand service = new StorageCommand(context) { + StorageCommand service = new StorageCommand(exitStatus) { @Override MongoProcessRunner createRunner() { throw new AssertionError("dry run should never create an actual runner"); @@ -152,7 +148,7 @@ // TODO: stop not tested yet, but be sure it's not called from the code doThrow(new ApplicationException("mock exception")).when(runner).stopService(); - StorageCommand service = new StorageCommand(context) { + StorageCommand service = new StorageCommand(exitStatus) { @Override MongoProcessRunner createRunner() { return runner; @@ -233,8 +229,7 @@ @Test public void exceptionSetsExitStatusOnFailure() throws Exception { - context = new StubBundleContext(); - ExitStatus exitStatusImpl = new ExitStatus() { + this.exitStatus = new ExitStatus() { private int exitStatus = -1; @@ -248,8 +243,7 @@ return exitStatus; } }; - context.registerService(ExitStatus.class, exitStatusImpl, null); - assertEquals(-1, getExitStatusService(context).getExitStatus()); + assertEquals(-1, this.exitStatus.getExitStatus()); StorageCommand command = prepareService(false); final CountDownLatch latch = new CountDownLatch(1); final boolean[] result = new boolean[1]; @@ -273,13 +267,12 @@ latch.await(); // should have failed assertTrue(result[0]); - assertEquals(Constants.EXIT_ERROR, getExitStatusService(context).getExitStatus()); + assertEquals(ExitStatus.EXIT_ERROR, this.exitStatus.getExitStatus()); } @Test public void exitStatusRemainsUntouchedOnSuccess() throws Exception { - context = new StubBundleContext(); - ExitStatus exitStatusImpl = new ExitStatus() { + this.exitStatus = new ExitStatus() { private int exitStatus = -1; @@ -293,8 +286,6 @@ return exitStatus; } }; - context.registerService(ExitStatus.class, exitStatusImpl, null); - assertEquals(-1, getExitStatusService(context).getExitStatus()); StorageCommand command = prepareService(true); final CountDownLatch latch = new CountDownLatch(1); final boolean[] result = new boolean[1]; @@ -319,14 +310,7 @@ // should have worked assertTrue(result[0]); // this impl of ExitStatus has a default value of -1 - assertEquals(-1, getExitStatusService(context).getExitStatus()); - } - - @SuppressWarnings({ "rawtypes", "unchecked" }) - private ExitStatus getExitStatusService(BundleContext context) { - ServiceReference ref = context.getServiceReference(ExitStatus.class); - ExitStatus exitStatus = (ExitStatus)context.getService(ref); - return exitStatus; + assertEquals(-1, this.exitStatus.getExitStatus()); } private CommandContext prepareContext() { @@ -340,14 +324,14 @@ @Test public void testName() { - StorageCommand dbService = new StorageCommand(context); + StorageCommand dbService = new StorageCommand(exitStatus); String name = dbService.getName(); assertEquals("storage", name); } @Test public void testDescAndUsage() { - StorageCommand dbService = new StorageCommand(context); + StorageCommand dbService = new StorageCommand(exitStatus); assertNotNull(dbService.getDescription()); assertNotNull(dbService.getUsage()); } diff -r edaf77f17cbd -r 3f0416e9db80 common/core/src/main/java/com/redhat/thermostat/common/Constants.java --- a/common/core/src/main/java/com/redhat/thermostat/common/Constants.java Tue Mar 26 11:17:08 2013 -0400 +++ b/common/core/src/main/java/com/redhat/thermostat/common/Constants.java Fri Mar 08 14:28:35 2013 +0100 @@ -42,9 +42,6 @@ */ public class Constants { - public static final int EXIT_ERROR = 1; - public static final int EXIT_SUCCESS = 0; - public static final String GENERIC_SERVICE_CLASSNAME = "GenericClassName"; } diff -r edaf77f17cbd -r 3f0416e9db80 common/core/src/main/java/com/redhat/thermostat/common/ExitStatus.java --- a/common/core/src/main/java/com/redhat/thermostat/common/ExitStatus.java Tue Mar 26 11:17:08 2013 -0400 +++ b/common/core/src/main/java/com/redhat/thermostat/common/ExitStatus.java Fri Mar 08 14:28:35 2013 +0100 @@ -1,3 +1,38 @@ +/* + * Copyright 2012, 2013 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * . + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ package com.redhat.thermostat.common; import com.redhat.thermostat.annotations.Service; @@ -6,17 +41,30 @@ * Service which can be used for certain commands to set the exit status on * JVM termination. If the status is set multiple times, the last thread which * calls {@link #setExitStatus(int)} wins. + * */ @Service public interface ExitStatus { /** - * Sets the exit status, which will be used as status when the - * JVM terminates. + * JVM exit status indicating successful termination. + */ + public static final int EXIT_SUCCESS = 0; + + /** + * JVM exit status indicating a generic error. + */ + public static final int EXIT_ERROR = 1; + + /** + * Sets the exit status, which will be used as status when the JVM + * terminates. * * @param newExitStatus + * @throws IllegalArgumentException + * If newExitStatus < 0 or newExitStatus > 255 */ - void setExitStatus(int newExitStatus); + void setExitStatus(int newExitStatus) throws IllegalArgumentException; /** * Get the currently set exit status. diff -r edaf77f17cbd -r 3f0416e9db80 launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java Tue Mar 26 11:17:08 2013 -0400 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/Activator.java Fri Mar 08 14:28:35 2013 +0100 @@ -45,7 +45,6 @@ import org.osgi.util.tracker.ServiceTracker; import org.osgi.util.tracker.ServiceTrackerCustomizer; -import com.redhat.thermostat.common.Constants; import com.redhat.thermostat.common.ExitStatus; import com.redhat.thermostat.common.Launcher; import com.redhat.thermostat.common.cli.CommandContextFactory; @@ -94,7 +93,7 @@ new CommandContextFactory(context), bundleService); launcherReg = context.registerService(Launcher.class.getName(), launcher, null); bundleManReg = context.registerService(BundleManager.class, bundleService, null); - ExitStatus exitStatus = new ExitStatusImpl(Constants.EXIT_SUCCESS); + ExitStatus exitStatus = new ExitStatusImpl(ExitStatus.EXIT_SUCCESS); exitStatusReg = context.registerService(ExitStatus.class, exitStatus, null); return keyring; } diff -r edaf77f17cbd -r 3f0416e9db80 launcher/src/main/java/com/redhat/thermostat/launcher/internal/ExitStatusImpl.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/ExitStatusImpl.java Tue Mar 26 11:17:08 2013 -0400 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/ExitStatusImpl.java Fri Mar 08 14:28:35 2013 +0100 @@ -14,7 +14,12 @@ this.exitStatus = initialExitStatus; } - public synchronized void setExitStatus(int newExitStatus) { + public synchronized void setExitStatus(int newExitStatus) + throws IllegalArgumentException { + if (newExitStatus < 0 || newExitStatus > 255) { + throw new IllegalArgumentException( + "Status (" + newExitStatus + ") out ouf range. 0 <= status <= 255."); + } this.exitStatus = newExitStatus; } diff -r edaf77f17cbd -r 3f0416e9db80 launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java --- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java Tue Mar 26 11:17:08 2013 -0400 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java Fri Mar 08 14:28:35 2013 +0100 @@ -51,7 +51,6 @@ import com.redhat.thermostat.common.ActionListener; import com.redhat.thermostat.common.ActionNotifier; import com.redhat.thermostat.common.ApplicationService; -import com.redhat.thermostat.common.Constants; import com.redhat.thermostat.common.ExitStatus; import com.redhat.thermostat.common.Launcher; import com.redhat.thermostat.common.Version; @@ -169,7 +168,7 @@ } // default to success for exit status - int exitStatus = Constants.EXIT_SUCCESS; + int exitStatus = ExitStatus.EXIT_SUCCESS; if (context != null) { ServiceReference storageRef = context.getServiceReference(Storage.class); if (storageRef != null) { diff -r edaf77f17cbd -r 3f0416e9db80 launcher/src/test/java/com/redhat/thermostat/launcher/internal/ExitStatusImplTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/ExitStatusImplTest.java Fri Mar 08 14:28:35 2013 +0100 @@ -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.launcher.internal; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import org.junit.Test; + +import com.redhat.thermostat.common.ExitStatus; + +public class ExitStatusImplTest { + + @Test + public void initialValue() { + ExitStatus status = new ExitStatusImpl(0); + assertEquals(0, status.getExitStatus()); + } + + @Test + public void valueOutOfRange() { + ExitStatus status = new ExitStatusImpl(0); + try { + status.setExitStatus(-1); + fail("should have thrown IllegalArgumentExcpt"); + } catch (IllegalArgumentException e) { + // pass + assertEquals(0, status.getExitStatus()); + } + try { + status.setExitStatus(256); + fail("should have thrown exptn"); + } catch (IllegalArgumentException e) { + // pass + assertEquals(0, status.getExitStatus()); + } + try { + status.setExitStatus(150); + assertEquals(150, status.getExitStatus()); + } catch (IllegalArgumentException e) { + fail(); + } + } +} diff -r edaf77f17cbd -r 3f0416e9db80 launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java --- a/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java Tue Mar 26 11:17:08 2013 -0400 +++ b/launcher/src/test/java/com/redhat/thermostat/launcher/internal/LauncherImplTest.java Fri Mar 08 14:28:35 2013 +0100 @@ -70,7 +70,7 @@ import com.redhat.thermostat.common.ActionListener; import com.redhat.thermostat.common.ActionNotifier; import com.redhat.thermostat.common.ApplicationService; -import com.redhat.thermostat.common.Constants; +import com.redhat.thermostat.common.ExitStatus; import com.redhat.thermostat.common.Version; import com.redhat.thermostat.common.cli.AbstractStateNotifyingCommand; import com.redhat.thermostat.common.cli.Arguments; @@ -533,7 +533,7 @@ } catch (ExitException e) { // pass, by default launcher exits with an exit status // of 0. - assertEquals(Constants.EXIT_SUCCESS, e.getExitStatus()); + assertEquals(ExitStatus.EXIT_SUCCESS, e.getExitStatus()); } }