# HG changeset patch # User Severin Gehwolf # Date 1368813116 -7200 # Node ID d586a5747cacd056c2a50e36ecca00a6cc926ad1 # Parent 83452b440c23984784415507df7112da7d089b57 Set THERMOSTAT_HOME via context listener in the web app. Reviewed-by: vanaltj Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-May/006668.html diff -r 83452b440c23 -r d586a5747cac integration-tests/src/test/java/com/redhat/thermostat/itest/WebAppTest.java --- a/integration-tests/src/test/java/com/redhat/thermostat/itest/WebAppTest.java Fri May 24 21:24:23 2013 -0400 +++ b/integration-tests/src/test/java/com/redhat/thermostat/itest/WebAppTest.java Fri May 17 19:51:56 2013 +0200 @@ -107,9 +107,6 @@ storage.expectClose(); assertNoExceptions(storage.getCurrentStandardOutContents(), storage.getCurrentStandardErrContents()); - // Setup required for JAAS login module. FIXME: We should set this via - // a context listener. - System.setProperty("THERMOSTAT_HOME", getThermostatHome()); backupUsers = Files.createTempFile("itest-backup-thermostat-users", ""); backupRoles = Files.createTempFile("itest-backup-thermostat-roles", ""); backupRoles.toFile().deleteOnExit(); diff -r 83452b440c23 -r d586a5747cac web/server/src/main/java/com/redhat/thermostat/web/server/PropertySettingServletContextListener.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/PropertySettingServletContextListener.java Fri May 17 19:51:56 2013 +0200 @@ -0,0 +1,63 @@ +/* + * 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.web.server; + +import javax.servlet.ServletContextEvent; +import javax.servlet.ServletContextListener; + +/** + * Sets the THERMOSTAT_HOME system property as + * configured via said context-param in web.xml. + * + */ +public class PropertySettingServletContextListener implements + ServletContextListener { + + private static final String PROPERTY_NAME = "THERMOSTAT_HOME"; + + @Override + public void contextInitialized(ServletContextEvent sce) { + String thermostatHome = sce.getServletContext().getInitParameter(PROPERTY_NAME); + System.setProperty(PROPERTY_NAME, thermostatHome); + } + + @Override + public void contextDestroyed(ServletContextEvent sce) { + System.clearProperty(PROPERTY_NAME); + } + +} diff -r 83452b440c23 -r d586a5747cac web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java --- a/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Fri May 24 21:24:23 2013 -0400 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java Fri May 17 19:51:56 2013 +0200 @@ -41,7 +41,6 @@ import java.io.InputStream; import java.io.OutputStream; import java.io.Writer; -import java.security.Principal; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -120,30 +119,10 @@ public void init(ServletConfig config) throws ServletException { super.init(config); logger.log(Level.INFO, "Initializing web service"); - if (!isThermostatHomeSet()) { - // This is the webapp and our entry point into thermostat's web - // service. The launcher did not run and hence THERMOSTAT_HOME is - // not set and we need to do this ourselves. - String thermostatHome = config.getInitParameter("THERMOSTAT_HOME"); - if (thermostatHome == null) { - String msg = "THERMOSTAT_HOME config parameter not set!"; - logger.log(Level.SEVERE, msg); - throw new RuntimeException(msg); - } - File thermostatHomeFile = new File(thermostatHome); - if (!thermostatHomeFile.canRead()) { - // This is bad news. If we can't at least read THERMOSTAT_HOME - // we are bound to fail in some weird ways at some later point. - String msg = "THERMOSTAT_HOME = " - + thermostatHome - + " is not readable or does not exist!"; - logger.log(Level.SEVERE, msg); - throw new RuntimeException(msg); - } - logger.log(Level.INFO, "Setting THERMOSTAT_HOME for webapp to " - + thermostatHome); - System.setProperty("THERMOSTAT_HOME", thermostatHome); - } + + // check if thermostat home is set and readable + checkThermostatHome(); + gson = new GsonBuilder().registerTypeHierarchyAdapter(Pojo.class, new ThermostatGSONConverter()).create(); categoryIds = new HashMap<>(); categories = new HashMap<>(); @@ -223,6 +202,37 @@ return false; } } + + private void checkThermostatHome() { + if (!isThermostatHomeSet()) { + String msg = "THERMOSTAT_HOME context parameter not set!"; + logger.log(Level.SEVERE, msg); + throw new RuntimeException(msg); + } + File thermostatHomeFile = getThermostatHome(); + if (!thermostatHomeFile.canRead()) { + // This is bad news. If we can't at least read THERMOSTAT_HOME + // we are bound to fail in some weird ways at some later point. + String msg = "THERMOSTAT_HOME = " + + thermostatHomeFile.getAbsolutePath() + + " is not readable or does not exist!"; + logger.log(Level.SEVERE, msg); + throw new RuntimeException(msg); + } + logger.log(Level.FINEST, "THERMOSTAT_HOME == " + + thermostatHomeFile.getAbsolutePath()); + } + + private File getThermostatHome() { + try { + Configuration config = new Configuration(); + return new File(config.getThermostatHome()); + } catch (InvalidConfigurationException e) { + // we should have just checked if this throws any exception + logger.log(Level.SEVERE, "Illegal configuration!", e); + return null; + } + } @WebStoragePathHandler( path = "ping" ) private void ping(HttpServletRequest req, HttpServletResponse resp) { diff -r 83452b440c23 -r d586a5747cac web/server/src/test/java/com/redhat/thermostat/web/server/PropertySettingServletContextListenerTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/web/server/src/test/java/com/redhat/thermostat/web/server/PropertySettingServletContextListenerTest.java Fri May 17 19:51:56 2013 +0200 @@ -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.web.server; + +import javax.servlet.ServletContext; +import javax.servlet.ServletContextEvent; + +import org.junit.After; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.Matchers.any; + +public class PropertySettingServletContextListenerTest { + + @After + public void tearDown() { + System.clearProperty("THERMOSTAT_HOME"); + } + + @Test + public void verifySetsProperty() { + PropertySettingServletContextListener listener = new PropertySettingServletContextListener(); + ServletContextEvent event = mock(ServletContextEvent.class); + ServletContext mockContext = mock(ServletContext.class); + when(event.getServletContext()).thenReturn(mockContext); + String fakeHome = "testing"; + when(mockContext.getInitParameter(any(String.class))).thenReturn(fakeHome); + listener.contextInitialized(event); + assertEquals(fakeHome, System.getProperty("THERMOSTAT_HOME")); + } + + @Test + public void verifyClearsProperty() { + System.setProperty("THERMOSTAT_HOME", "somevalue"); + PropertySettingServletContextListener listener = new PropertySettingServletContextListener(); + listener.contextDestroyed(null); + assertNull(System.getProperty("THERMOSTAT_HOME")); + } +} diff -r 83452b440c23 -r d586a5747cac web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java --- a/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java Fri May 24 21:24:23 2013 -0400 +++ b/web/server/src/test/java/com/redhat/thermostat/web/server/WebStorageEndpointTest.java Fri May 17 19:51:56 2013 +0200 @@ -47,6 +47,7 @@ import static org.mockito.Mockito.when; import java.io.ByteArrayInputStream; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; @@ -165,9 +166,12 @@ @Before public void setUp() throws Exception { - // Set thermostat home to something so we don't set - // it in WebStorageEndPoint.init() - System.setProperty("THERMOSTAT_HOME", "does not matter"); + // Set thermostat home to something existing and readable + File fakeHome = new File(getClass().getResource("/broken_test_roles.properties").getFile()); + // fakeHome does not need to be a real THERMOSTAT_HOME, but needs to + // be readable and must exist. + assertTrue(fakeHome.canRead()); + System.setProperty("THERMOSTAT_HOME", fakeHome.getAbsolutePath()); mockStorage = mock(Storage.class); StorageWrapper.setStorage(mockStorage); diff -r 83452b440c23 -r d586a5747cac web/war/src/main/webapp/WEB-INF/web.xml --- a/web/war/src/main/webapp/WEB-INF/web.xml Fri May 24 21:24:23 2013 -0400 +++ b/web/war/src/main/webapp/WEB-INF/web.xml Fri May 17 19:51:56 2013 +0200 @@ -12,11 +12,6 @@ com.redhat.thermostat.storage.mongodb.MongoStorageProvider - - THERMOSTAT_HOME - ${project.build.directory}/../../../distribution/target/ - - storage.endpoint mongodb://127.0.0.1:27518 @@ -72,5 +67,15 @@ thermostat-realm + + + + THERMOSTAT_HOME + ${project.build.directory}/../../../distribution/target/ + + + + com.redhat.thermostat.web.server.PropertySettingServletContextListener +