Feature 1638 force admin

src/edu/ucsb/nceas/metacat/properties/PropertiesWrapper.java

@@ -18,7 +18,9 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.Vector;
@@ -46,12 +48,41 @@ public class PropertiesWrapper {
     private Properties mainProperties = null;
 
     // default properties only; not including any site-specific properties
-    private Properties defaultProperties = null;
+    protected Properties defaultProperties = null;
 
     // Contains mappings of properties.key=METACAT_ENV_KEY, used to retrieve secret credentials
     // from environment variables
     private final Properties envSecretKeyMappings = new Properties();
 
+    /**
+     * TODO: Eliminate the need for this list! It is here for legacy reasons;
+     *       See GitHub Issue #1638: https://github.com/NCEAS/metacat/issues/1638
+     * Whenever setPropertyNoPersist() is called, the code checks to see if the passed propertyName
+     * parameter is listed in 'writeableDefaults'. If so, the property is added directly to
+     * 'defaultProperties' instead of to 'mainProperties'. It will therefore end up being written
+     * to the 'metacat.properties' file instead of the 'metacat-site.properties' file.
+     * This breaks the design intent of the Properties setup, and has been done as a temporary
+     * workaround preceding incremental improvements.
+     * DO NOT ADD ANY MORE KEYS TO THIS LIST! - IT WILL BE DELETED when the code is refactored to
+     * to handle upgrades across deployments for k8s and legacy installations. Basically, the
+     * metacat.properties file should never need to be writeable. If you find yourself wanting to
+     * write to it, there must be a better solution.
+     */
+    protected static final List<String> WRITEABLE_DEFAULTS = Arrays.asList(
+        PropertyService.SITE_PROPERTIES_DIR_PATH_KEY,
+        "configutil.propertiesConfigured",
+        "configutil.authConfigured",
+        "configutil.skinsConfigured",
+        "configutil.databaseConfigured",
+        "configutil.solrserverConfigured",
+        "configutil.dataoneConfigured",
+        "configutil.ezidConfigured",
+        "configutil.quotaConfigured",
+        "configutil.upgrade.status",
+        "configutil.upgrade.database.status",
+        "configutil.upgrade.java.status",
+        "configutil.upgrade.solr.status"
+    );
 
     /**
      * private constructor since this is a singleton
@@ -234,15 +265,24 @@ protected void setPropertyNoPersist(String propertyName, String newValue)
                 "Illegal argument - at least one of params was NULL! key = " + propertyName
                     + "; value = " + newValue);
         }
+        propertyName = propertyName.trim();
         if (null == mainProperties.getProperty(propertyName)) {
             // TODO: MB - can we get rid of this? Default java.util.Properties behavior is
             //  to add a new entry if it doesn't already exist, when setProperty() is called
             throw new PropertyNotFoundException(
                 "Property: " + propertyName + " could not be updated to: " + newValue
                     + " because it does not already exist in properties.");
         }
-        if (propertyName.trim().equals(PropertyService.SITE_PROPERTIES_DIR_PATH_KEY)) {
-            changeSitePropsPath(newValue);
+        if (WRITEABLE_DEFAULTS.contains(propertyName)) {
+            // TODO: Eliminate the need for setting default props! This is here for legacy reasons;
+            //       See GitHub Issue #1638: https://github.com/NCEAS/metacat/issues/1638
+            //       and see comment for WRITEABLE_DEFAULTS constant
+            if (propertyName.equals(PropertyService.SITE_PROPERTIES_DIR_PATH_KEY)) {
+                changeSitePropsPath(newValue);
+            }
+            defaultProperties.setProperty(propertyName, newValue);
+            // Don't re-use/copy the above code - it needs to go away. The default properties
+            // (i.e. those in the metacat.properties file) should never need to be set explicitly.
         } else {
             mainProperties.setProperty(propertyName, newValue);
         }
@@ -375,21 +415,12 @@ private void changeSitePropsPath(String newDir) throws GeneralPropertyException
                         + " to " + newSitePropsPath, e);
                 }
             }
+            sitePropertiesFilePath = newSitePropsPath;
 
-            defaultProperties.setProperty(PropertyService.SITE_PROPERTIES_DIR_PATH_KEY, newDir);
-
-            try (Writer output = new FileWriter(defaultPropertiesFilePath.toFile())) {
-                defaultProperties.store(output, null);
-            } catch (IOException e) {
-                logAndThrow("I/O exception trying to store default properties to: "
-                    + defaultPropertiesFilePath, e);
-            }
             logMetacat.debug(
                 "updated sitePropertiesFilePath from " + sitePropertiesFilePath + " to "
-                    + newSitePropsPath + " and wrote new directory location to DEFAULT properties: "
-                    + newDir);
-            sitePropertiesFilePath = newSitePropsPath;
-            PropertyService.syncToSettings();
+                    + newSitePropsPath
+                    + " and added new directory location to in-memory DEFAULT properties: " + newDir);
         }
     }
 
@@ -419,7 +450,7 @@ private void initialize() throws GeneralPropertyException {
             defaultProperties.load(Files.newBufferedReader(defaultPropertiesFilePath));
 
             logMetacat.debug("PropertiesWrapper.initialize(): finished "
-                + "loading metacat.properties into mainDefaultProperties");
+                + "loading metacat.properties into defaultProperties");
 
             // mainProperties comprises (1) the default properties loaded from metacat.properties...
             mainProperties = new Properties(defaultProperties);
@@ -574,14 +605,19 @@ private static void logAndThrow(String message, Exception e) throws GeneralPrope
     }
 
     /**
-     * store the properties to file. NOTE that this will persist only the properties that are: (a)
-     * not included in mainDefaultProperties, or (b) have changed from the values in
-     * mainDefaultProperties. From the <a
-     * href="https://docs.oracle.com/javase/8/docs/api/java/util/Properties.html">
+     * Store the properties to file. NOTE that this will persist only the properties that are:
+     * (a) not included in defaultProperties, or (b) have changed from the values in
+     * defaultProperties, by writing them to the metacat-site.properties file.
+     * From the <a href="https://docs.oracle.com/javase/8/docs/api/java/util/Properties.html">
      * java.util.Properties javadoc</a>: "Properties from the defaults table of this Properties
      * table (if any) are not written out by this method." Also, Properties class is thread-safe -
      * no need to synchronize.
      *
+     * As a side effect, the metacat.properties file is also written back to disk. This is needed
+     * for legacy reasons, and will be removed when the relevant code is refactored.
+     * See GitHub Issue #1638: https://github.com/NCEAS/metacat/issues/1638 and also
+     * @see PropertiesWrapper.WRITEABLE_DEFAULTS
+     *
      * @throws GeneralPropertyException if an I/O Exception is encountered
      */
     private void store() throws GeneralPropertyException {
@@ -590,6 +626,16 @@ private void store() throws GeneralPropertyException {
         } catch (IOException e) {
             logAndThrow("I/O exception trying to call mainProperties.store(): ", e);
         }
+        // TODO: Eliminate the need for storing default props! It is here for legacy reasons;
+        //       See GitHub Issue #1638: https://github.com/NCEAS/metacat/issues/1638
+        //       and see comment for WRITEABLE_DEFAULTS constant
+        try (Writer output = new FileWriter(defaultPropertiesFilePath.toFile())) {
+            defaultProperties.store(output, null);
+        } catch (IOException e) {
+            logAndThrow("I/O exception trying to call defaultProperties.store(): ", e);
+        }
+        // Don't re-use/copy the above code - it needs to go away. The metacat.properties file
+        // should never need to be written to.
     }
 
     private boolean isNotBlankPath(Path path) {

test/edu/ucsb/nceas/LeanTestUtils.java

@@ -36,7 +36,7 @@ public class LeanTestUtils {
 
     private static final Path DEFAULT_PROPS_FILE_PATH = Paths.get("lib/metacat.properties");
     private static final Path SITE_PROPS_FILE_PATH = Paths.get("test/test.properties");
-    private static boolean printDebug = false;
+    private static final boolean printDebug;
     private static Properties expectedProperties;
 
     static {
@@ -68,28 +68,53 @@ public static void debug(String debugMessage) {
     public static Properties getExpectedProperties() {
 
         if (expectedProperties == null) {
-            assertTrue("LeanTestUtils.getExpectedProperties(): default properties files not found",
-                doesPropertiesFileExist(DEFAULT_PROPS_FILE_PATH));
-            assertTrue("LeanTestUtils.getExpectedProperties(): site properties files not found",
-                doesPropertiesFileExist(SITE_PROPS_FILE_PATH));
-            Properties metacatProps = new Properties();
-            try {
-                metacatProps.load(Files.newBufferedReader(DEFAULT_PROPS_FILE_PATH));
-                expectedProperties = new Properties(metacatProps);
-                expectedProperties.load(Files.newBufferedReader(SITE_PROPS_FILE_PATH));
-            } catch (IOException e) {
-                fail("I/O exception trying to load properties from " + DEFAULT_PROPS_FILE_PATH
-                    + " and " + SITE_PROPS_FILE_PATH);
-            }
-            assertFalse("LeanTestUtils: expected properties EMPTY!", expectedProperties.isEmpty());
-            assertFalse("metacat.properties not loaded?",
-                expectedProperties.getProperty("application.metacatVersion").trim().isEmpty());
-            assertFalse("test.properties not loaded?",
-                        expectedProperties.getProperty("metacat.contextDir").trim().isEmpty());
+            expectedProperties = getLatestPropertiesFromDisk();
         }
         return expectedProperties;
     }
 
+    /**
+     * Get a Properties object containing the default properties from lib/metacat.properties,
+     * overlaid with the test properties from test/test.properties, that have been freshly loaded
+     * from the files on disk. These will therefore reflect any changes that have been persisted
+     * back to the properties during a test run, whereas a call to getExpectedProperties() will
+     * always return the same starting properties that were in effect the first time that method
+     * was called.
+     * These can therefore be used to test persistence of properties, or can be  used to retrieve
+     * test-specific properties such as the debug flag ("test.printdebug") or the metacat deployment
+     * location ("metacat.contextDir")
+     *
+     * @return  java.util.Properties object containing a freshly-loaded copy of the default
+     *          properties from lib/metacat.properties, overlaid with the test properties from
+     *          test/test.properties
+     */
+    public static Properties getLatestPropertiesFromDisk() {
+
+        Properties latestProperties = new Properties();
+        assertTrue(
+            "LeanTestUtils.getLatestPropertiesFromDisk(): default properties files not found",
+            doesPropertiesFileExist(DEFAULT_PROPS_FILE_PATH));
+        assertTrue(
+            "LeanTestUtils.getLatestPropertiesFromDisk(): site properties files not found",
+            doesPropertiesFileExist(SITE_PROPS_FILE_PATH));
+        Properties metacatProps = new Properties();
+        try {
+            metacatProps.load(Files.newBufferedReader(DEFAULT_PROPS_FILE_PATH));
+            latestProperties = new Properties(metacatProps);
+            latestProperties.load(Files.newBufferedReader(SITE_PROPS_FILE_PATH));
+        } catch (IOException e) {
+            fail("I/O exception trying to load properties from " + DEFAULT_PROPS_FILE_PATH + " and "
+                     + SITE_PROPS_FILE_PATH);
+        }
+        assertFalse("LeanTestUtils: latestProperties EMPTY!", latestProperties.isEmpty());
+        assertFalse("metacat.properties not loaded?",
+            latestProperties.getProperty("application.metacatVersion").trim().isEmpty());
+        assertFalse("test.properties not loaded?",
+            latestProperties.getProperty("metacat.contextDir").trim().isEmpty());
+
+        return latestProperties;
+    }
+
     /**
      * Lightweight initialization of the PropertyService without needing to have a servlet context
      * available.