changeset 1345:37efb2ef077a

Avoid side effects of CommonPathsImpl instantiation reviewed-by: omajid review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-November/008792.html
author Jon VanAlten <jon.vanalten@redhat.com>
date Mon, 18 Nov 2013 08:24:25 -0700
parents f940c4226608
children bd5ddefd7e9f
files config/src/main/java/com/redhat/thermostat/shared/config/DirectoryStructureCreator.java config/src/main/java/com/redhat/thermostat/shared/config/internal/Activator.java config/src/main/java/com/redhat/thermostat/shared/config/internal/CommonPathsImpl.java config/src/test/java/com/redhat/thermostat/shared/config/DirectoryStructureCreatorTest.java web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java
diffstat 5 files changed, 366 insertions(+), 35 deletions(-) [+]
line wrap: on
line diff
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/config/src/main/java/com/redhat/thermostat/shared/config/DirectoryStructureCreator.java	Mon Nov 18 08:24:25 2013 -0700
@@ -0,0 +1,83 @@
+/*
+ * Copyright 2013 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <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.shared.config;
+
+import java.io.File;
+
+import com.redhat.thermostat.shared.locale.Translate;
+import com.redhat.thermostat.shared.locale.internal.LocaleResources;
+
+public class DirectoryStructureCreator {
+
+    private static final Translate<LocaleResources> t = LocaleResources.createLocalizer();
+    private final CommonPaths paths;
+
+    public DirectoryStructureCreator(CommonPaths paths) {
+        this.paths = paths;
+    }
+
+    public void createPaths() {
+        makeDir(paths.getSystemThermostatHome());
+        makeDir(paths.getSystemPluginRoot());
+        makeDir(paths.getSystemLibRoot());
+        makeDir(paths.getSystemNativeLibsRoot());
+        makeDir(paths.getSystemBinRoot());
+        makeDir(paths.getSystemConfigurationDirectory());
+        makeDir(paths.getUserThermostatHome());
+        makeDir(paths.getUserConfigurationDirectory());
+        makeDir(paths.getUserRuntimeDataDirectory());
+        makeDir(paths.getUserLogDirectory());
+        makeDir(paths.getUserCacheDirectory());
+        makeDir(paths.getUserPersistentDataDirectory());
+        makeDir(paths.getUserPluginRoot());
+        makeDir(paths.getUserStorageDirectory());
+
+    }
+
+    private static void makeDir(File dir) {
+        boolean exists = dir.exists();
+        if (!exists) {
+            exists = dir.mkdirs();
+        }
+        if (!exists) {
+            throw new InvalidConfigurationException("Directory could not be created: " + dir.getAbsolutePath());
+        }
+        if (!dir.isDirectory()) {
+            throw new InvalidConfigurationException(t.localize(LocaleResources.GENERAL_NOT_A_DIR, dir.getAbsolutePath()));
+        }
+    }
+}
\ No newline at end of file
--- a/config/src/main/java/com/redhat/thermostat/shared/config/internal/Activator.java	Mon Nov 18 08:22:15 2013 -0700
+++ b/config/src/main/java/com/redhat/thermostat/shared/config/internal/Activator.java	Mon Nov 18 08:24:25 2013 -0700
@@ -40,6 +40,7 @@
 import org.osgi.framework.BundleContext;
 
 import com.redhat.thermostat.shared.config.CommonPaths;
+import com.redhat.thermostat.shared.config.DirectoryStructureCreator;
 import com.redhat.thermostat.shared.config.SSLConfiguration;
 import com.redhat.thermostat.shared.config.NativeLibraryResolver;
 
@@ -48,6 +49,8 @@
     @Override
     public void start(BundleContext context) throws Exception {
         CommonPaths paths = new CommonPathsImpl();
+        DirectoryStructureCreator pathsCreator = new DirectoryStructureCreator(paths);
+        pathsCreator.createPaths();
         // Prevents IllegalStateException when external dependencies attempt to resolve native libraries.
         NativeLibraryResolver.setCommonPaths(paths);
         context.registerService(CommonPaths.class.getName(), paths, null);
--- a/config/src/main/java/com/redhat/thermostat/shared/config/internal/CommonPathsImpl.java	Mon Nov 18 08:22:15 2013 -0700
+++ b/config/src/main/java/com/redhat/thermostat/shared/config/internal/CommonPathsImpl.java	Mon Nov 18 08:24:25 2013 -0700
@@ -98,11 +98,11 @@
     private static File defaultSystemUserPrefix;
 
     public CommonPathsImpl() throws InvalidConfigurationException {
-        this(makeDir(null, "/"));
+        this(new File("/"));
     }
 
     CommonPathsImpl(String altTestingPrefix) {
-        this(makeDir(null, altTestingPrefix));
+        this(new File(altTestingPrefix));
     }
 
     private CommonPathsImpl(File defaultPrefix) {
@@ -153,27 +153,27 @@
 
     @Override
     public File getSystemPluginRoot() throws InvalidConfigurationException {
-        return makeDir(systemHome, "plugins");
+        return new File(systemHome, "plugins");
     }
 
     @Override
     public File getSystemLibRoot() throws InvalidConfigurationException {
-        return makeDir(systemHome, "libs");
+        return new File(systemHome, "libs");
     }
 
     @Override
     public File getSystemBinRoot() throws InvalidConfigurationException {
-        return makeDir(systemHome, "bin");
+        return new File(systemHome, "bin");
     }
 
     @Override
     public File getSystemNativeLibsRoot() throws InvalidConfigurationException {
-        return makeDir(getSystemLibRoot(), "native");
+        return new File(getSystemLibRoot(), "native");
     }
 
     @Override
     public File getSystemConfigurationDirectory() throws InvalidConfigurationException {
-        return makeDir(getSystemThermostatHome(), "etc");
+        return new File(getSystemThermostatHome(), "etc");
     }
 
     @Override
@@ -216,7 +216,7 @@
 
     @Override
     public File getUserStorageDirectory() throws InvalidConfigurationException {
-        return makeDir(getUserPersistentDataDirectory(), "db");
+        return new File(getUserPersistentDataDirectory(), "db");
     }
 
     @Override
@@ -292,21 +292,6 @@
 
     }
 
-    private static File makeDir(File parent, String name) {
-        File dir = new File(parent, name);
-        boolean exists = dir.exists();
-        if (!exists) {
-            exists = dir.mkdirs();
-        }
-        if (!exists) {
-            throw new InvalidConfigurationException("Directory could not be created: " + dir.getAbsolutePath());
-        }
-        if (!dir.isDirectory()) {
-            throw new InvalidConfigurationException(t.localize(LocaleResources.GENERAL_NOT_A_DIR, dir.getAbsolutePath()));
-        }
-        return dir;
-    }
-
     /*
      * We need two different implementations because the paths are different. We
      * can't get clean paths by simply changing the prefix.
@@ -330,7 +315,7 @@
             if (userHome == null) {
                 userHome = System.getProperty("user.home") + File.separatorChar + THERMOSTAT_USER_DIR;
             }
-            this.userHome = makeDir(null, userHome);
+            this.userHome = new File(userHome);
         }
 
         public File getSystemRoot() throws InvalidConfigurationException {
@@ -339,23 +324,23 @@
 
 
         public File getUserConfigurationDirectory() throws InvalidConfigurationException {
-            return makeDir(getSystemRoot(), "etc");
+            return new File(getSystemRoot(), "etc");
         }
 
         public File getUserPersistentDataDirectory() throws InvalidConfigurationException {
-            return makeDir(getSystemRoot(), "data");
+            return new File(getSystemRoot(), "data");
         }
 
         public File getUserRuntimeDataDirectory() throws InvalidConfigurationException {
-            return makeDir(getSystemRoot(), "run");
+            return new File(getSystemRoot(), "run");
         }
 
         public File getUserLogDirectory() throws InvalidConfigurationException {
-            return makeDir(getSystemRoot(), "logs");
+            return new File(getSystemRoot(), "logs");
         }
 
         public File getUserCacheDirectory() throws InvalidConfigurationException {
-            return makeDir(getSystemRoot(), "cache");
+            return new File(getSystemRoot(), "cache");
         }
     }
 
@@ -372,7 +357,7 @@
             if (userHome == null) {
                 this.prefix = defaultSystemUserPrefix;
             } else {
-                this.prefix = makeDir(null, userHome);
+                this.prefix = new File(userHome);
             }
         }
 
@@ -382,23 +367,23 @@
 
 
         public File getUserConfigurationDirectory() throws InvalidConfigurationException {
-            return makeDir(getSystemRoot(), "etc/thermostat");
+            return new File(getSystemRoot(), "etc/thermostat");
         }
 
         public File getUserPersistentDataDirectory() throws InvalidConfigurationException {
-            return makeDir(getSystemRoot(), "var/lib/thermostat");
+            return new File(getSystemRoot(), "var/lib/thermostat");
         }
 
         public File getUserRuntimeDataDirectory() throws InvalidConfigurationException {
-            return makeDir(getSystemRoot(), "var/run/thermostat");
+            return new File(getSystemRoot(), "var/run/thermostat");
         }
 
         public File getUserLogDirectory() throws InvalidConfigurationException {
-            return makeDir(getSystemRoot(), "var/log/thermostat");
+            return new File(getSystemRoot(), "var/log/thermostat");
         }
 
         public File getUserCacheDirectory() throws InvalidConfigurationException {
-            return makeDir(getSystemRoot(), "var/cache/thermostat");
+            return new File(getSystemRoot(), "var/cache/thermostat");
         }
     }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/config/src/test/java/com/redhat/thermostat/shared/config/DirectoryStructureCreatorTest.java	Mon Nov 18 08:24:25 2013 -0700
@@ -0,0 +1,258 @@
+/*
+ * Copyright 2013 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <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.shared.config;
+
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+
+import org.junit.Test;
+
+public class DirectoryStructureCreatorTest {
+
+    private File systemThermostatHome, systemLibRoot, systemNativeLibsRoot, systemBinRoot,
+                systemPluginRoot, systemConfigurationDirectory, userThermostatHome,
+                userConfigurationDirectory, userRuntimeDataDirectory,
+                userLogDirectory, userCacheDirectory, userPersistentDataDirectory,
+                userPluginRoot, userStorageDirectory;
+
+
+    private class TestCommonPaths implements CommonPaths {
+
+        private TestCommonPaths(File tempRootDir) {
+            systemThermostatHome = new File(tempRootDir, "systemThermostatHome");
+            systemThermostatHome.deleteOnExit();
+            systemLibRoot = new File(tempRootDir, "systemLibRoot");
+            systemLibRoot.deleteOnExit();
+            systemNativeLibsRoot = new File(tempRootDir, "systemNativeLibsRoot");
+            systemNativeLibsRoot.deleteOnExit();
+            systemBinRoot = new File(tempRootDir, "systemBinRoot");
+            systemBinRoot.deleteOnExit();
+            systemPluginRoot = new File(tempRootDir, "systemPluginRoot");
+            systemPluginRoot.deleteOnExit();
+            systemConfigurationDirectory = new File(tempRootDir, "systemConfigurationDir");
+            systemConfigurationDirectory.deleteOnExit();
+            userThermostatHome = new File(tempRootDir, "userThermostatHome");
+            userThermostatHome.deleteOnExit();
+            userConfigurationDirectory = new File(tempRootDir, "userConfigurationDirectory");
+            userConfigurationDirectory.deleteOnExit();
+            userRuntimeDataDirectory = new File(tempRootDir, "userRuntimeDataDirectory");
+            userRuntimeDataDirectory.deleteOnExit();
+            userLogDirectory = new File(tempRootDir, "userLogDirectory");
+            userLogDirectory.deleteOnExit();
+            userCacheDirectory = new File(tempRootDir, "userCacheDirectory");
+            userCacheDirectory.deleteOnExit();
+            userPersistentDataDirectory = new File(tempRootDir, "userPersistentDataDirectory");
+            userPersistentDataDirectory.deleteOnExit();
+            userPluginRoot = new File(tempRootDir, "userPluginRoot");
+            userPluginRoot.deleteOnExit();
+            userStorageDirectory = new File(tempRootDir, "userStorageDirectory");
+            userStorageDirectory.deleteOnExit();
+        }
+
+        @Override
+        public File getSystemThermostatHome()
+                throws InvalidConfigurationException {
+            return systemThermostatHome;
+        }
+
+        @Override
+        public File getUserThermostatHome()
+                throws InvalidConfigurationException {
+            return userThermostatHome;
+        }
+
+        @Override
+        public File getSystemPluginRoot() throws InvalidConfigurationException {
+            return systemPluginRoot;
+        }
+
+        @Override
+        public File getSystemLibRoot() throws InvalidConfigurationException {
+            return systemLibRoot;
+        }
+
+        @Override
+        public File getSystemNativeLibsRoot()
+                throws InvalidConfigurationException {
+            return systemNativeLibsRoot;
+        }
+
+        @Override
+        public File getSystemBinRoot()
+                throws InvalidConfigurationException {
+            return systemBinRoot;
+        }
+
+        @Override
+        public File getSystemConfigurationDirectory()
+                throws InvalidConfigurationException {
+            return systemConfigurationDirectory;
+        }
+
+        @Override
+        public File getUserConfigurationDirectory()
+                throws InvalidConfigurationException {
+            return userConfigurationDirectory;
+        }
+
+        @Override
+        public File getUserPersistentDataDirectory()
+                throws InvalidConfigurationException {
+            return userPersistentDataDirectory;
+        }
+
+        @Override
+        public File getUserRuntimeDataDirectory()
+                throws InvalidConfigurationException {
+            return userRuntimeDataDirectory;
+        }
+
+        @Override
+        public File getUserLogDirectory() throws InvalidConfigurationException {
+            return userLogDirectory;
+        }
+
+        @Override
+        public File getUserCacheDirectory()
+                throws InvalidConfigurationException {
+            return userCacheDirectory;
+        }
+
+        @Override
+        public File getUserPluginRoot() throws InvalidConfigurationException {
+            return userPluginRoot;
+        }
+
+        @Override
+        public File getUserStorageDirectory()
+                throws InvalidConfigurationException {
+            return userStorageDirectory;
+        }
+
+        @Override
+        public File getSystemStorageConfigurationFile()
+                throws InvalidConfigurationException {
+            return null; // Only directories need to be created
+        }
+
+        @Override
+        public File getUserStorageConfigurationFile()
+                throws InvalidConfigurationException {
+            return null; // Only directories need to be created
+        }
+
+        @Override
+        public File getUserStorageLogFile()
+                throws InvalidConfigurationException {
+            return null; // Only directories need to be created
+        }
+
+        @Override
+        public File getUserStoragePidFile()
+                throws InvalidConfigurationException {
+            return null; // Only directories need to be created
+        }
+
+        @Override
+        public File getSystemAgentConfigurationFile()
+                throws InvalidConfigurationException {
+            return null; // Only directories need to be created
+        }
+
+        @Override
+        public File getUserAgentConfigurationFile()
+                throws InvalidConfigurationException {
+            return null; // Only directories need to be created
+        }
+
+        @Override
+        public File getSystemAgentAuthConfigFile()
+                throws InvalidConfigurationException {
+            return null; // Only directories need to be created
+        }
+
+        @Override
+        public File getUserAgentAuthConfigFile()
+                throws InvalidConfigurationException {
+            return null; // Only directories need to be created
+        }
+
+        @Override
+        public File getUserClientConfigurationFile()
+                throws InvalidConfigurationException {
+            return null; // Only directories need to be created
+        }
+
+        @Override
+        public File getUserHistoryFile() throws InvalidConfigurationException {
+            return null; // Only directories need to be created
+        }
+        
+    }
+
+    @Test
+    public void createsAllDirectories() throws IOException {
+        File testRoot = Files.createTempDirectory("dirStructTest").toFile();
+        testRoot.deleteOnExit();
+        CommonPaths paths = new TestCommonPaths(testRoot);
+        DirectoryStructureCreator creator = new DirectoryStructureCreator(paths);
+        creator.createPaths();
+
+        ensureDirCreated(systemThermostatHome);
+        ensureDirCreated(systemLibRoot);
+        ensureDirCreated(systemNativeLibsRoot);
+        ensureDirCreated(systemBinRoot);
+        ensureDirCreated(systemPluginRoot);
+        ensureDirCreated(systemConfigurationDirectory);
+        ensureDirCreated(userThermostatHome);
+        ensureDirCreated(userConfigurationDirectory);
+        ensureDirCreated(userRuntimeDataDirectory);
+        ensureDirCreated(userLogDirectory);
+        ensureDirCreated(userCacheDirectory);
+        ensureDirCreated(userPersistentDataDirectory);
+        ensureDirCreated(userPluginRoot);
+        ensureDirCreated(userStorageDirectory);
+    }
+
+    private void ensureDirCreated(File dir) {
+        assertTrue(dir.exists());
+        assertTrue(dir.isDirectory());
+    }
+}
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java	Mon Nov 18 08:22:15 2013 -0700
+++ b/web/server/src/main/java/com/redhat/thermostat/web/server/WebStorageEndPoint.java	Mon Nov 18 08:24:25 2013 -0700
@@ -71,6 +71,7 @@
 import com.google.gson.GsonBuilder;
 import com.redhat.thermostat.common.utils.LoggingUtils;
 import com.redhat.thermostat.shared.config.CommonPaths;
+import com.redhat.thermostat.shared.config.DirectoryStructureCreator;
 import com.redhat.thermostat.shared.config.InvalidConfigurationException;
 import com.redhat.thermostat.shared.config.internal.CommonPathsImpl;
 import com.redhat.thermostat.storage.core.Categories;
@@ -250,6 +251,7 @@
             // this throws config exception if neither the property
             // nor the env var is set
             paths = new CommonPathsImpl();
+            new DirectoryStructureCreator(paths).createPaths();
             return true;
         } catch (InvalidConfigurationException e) {
             return false;