Fix for importing system properties into GlobalProperties.

Author: Craigacp

olcut-core/src/main/java/com/oracle/labs/mlrg/olcut/config/property/GlobalProperties.java

@@ -44,32 +44,56 @@
  */
 public class GlobalProperties extends ImmutableGlobalProperties {
 
+    /**
+     * Creates a GlobalProperties which only contains the built in properties.
+     */
     public GlobalProperties() {
         super();
     }
 
+    /**
+     * Copies the supplied GlobalProperties.
+     * @param globalProperties The properties to copy.
+     */
     public GlobalProperties(GlobalProperties globalProperties) {
         super(globalProperties);
     }
 
     /**
-     * Imports the system properties into GlobalProperties.
+     * Imports the system properties into this GlobalProperties.
      */
     public final void importSystemProperties() {
         Properties props = AccessController.doPrivileged((PrivilegedAction<Properties>) System::getProperties);
+        importProperties(props);
+    }
+
+    /**
+     * Imports the supplied properties.
+     *
+     * @param props The properties to import.
+     */
+    // Exposed for unit testing
+    void importProperties(Properties props) {
         for (Map.Entry<Object,Object> e : props.entrySet()) {
             // These two calls use .toString rather than a cast
-            // because sometimes Fusion inserts Integers into the system properties.
+            // because sometimes people insert Integers into the system properties.
             String param = e.getKey().toString();
             String value = e.getValue() == null ? "null" : e.getValue().toString();
-            setValue(param, value);
+            // Checks to see if the system property could be a valid global property,
+            // only insert it if it is.
+            // Bypasses setValue as that throws an exception if the key isn't well formed.
+            String testValue = "${" + param + "}";
+            Matcher m = GlobalProperty.globalSymbolPattern.matcher(testValue);
+            if (m.matches()) {
+                map.put(param, new GlobalProperty(value));
+            }
         }
     }
 
     /**
      * Adds a value to this GlobalProperties. Throws PropertyException if the
      * name does not conform to the {@link GlobalProperty#globalSymbolPattern}.
-     *
+     * <p>
      * It overwrites values if they already exist.
      * @param propertyName The name of the new global property.
      * @param value The value for the new global property.
@@ -82,7 +106,7 @@ public void setValue(String propertyName, String value) throws PropertyException
     /**
      * Adds a value to this GlobalProperties. Throws PropertyException if the
      * name does not conform to the {@link GlobalProperty#globalSymbolPattern}.
-     *
+     * <p>
      * It overwrites values if they already exist.
      * @param propertyName The name of the new global property.
      * @param value The value for the new global property.

olcut-core/src/main/java/com/oracle/labs/mlrg/olcut/config/property/ImmutableGlobalProperties.java

@@ -31,6 +31,7 @@
 import com.oracle.labs.mlrg.olcut.config.PropertyException;
 import com.oracle.labs.mlrg.olcut.util.Util;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
@@ -40,35 +41,58 @@
 
 /**
  * A collection of GlobalProperties which can't be mutated.
+ * <p>
+ * In addition to the set properties, it also includes
+ * preset properties. This set currently includes {@link #HOSTNAME}
+ * which lazily resolves to the system hostname.
  */
 public class ImmutableGlobalProperties implements Iterable<Map.Entry<String,GlobalProperty>> {
 
+    public static final String HOSTNAME = "gp.hostName";
+
     /**
      * A set of distinguished properties that we would like to have.
      */
-    private static Map<String, GlobalProperty> distinguished = new HashMap<>();
+    private static final Map<String, GlobalProperty> distinguished = new HashMap<>();
 
     protected final HashMap<String, GlobalProperty> map;
 
     static {
-        distinguished.put("gp.hostName", new LazyGlobalProperty(Util::getHostName));
+        distinguished.put(HOSTNAME, new LazyGlobalProperty(Util::getHostName));
     }
 
+    /**
+     * Creates an ImmutableGlobalProperties.
+     */
     public ImmutableGlobalProperties() {
         this.map = new HashMap<>();
     }
 
+    /**
+     * Creates an ImmutableGlobalProperties copy of the other properties.
+     * @param globalProperties The properties to copy.
+     */
     public ImmutableGlobalProperties(GlobalProperties globalProperties) {
         this.map = new HashMap<>();
         for(String key : globalProperties.map.keySet()) {
             map.put(key, new GlobalProperty(globalProperties.get(key)));
         }
     }
 
+    /**
+     * Creates an ImmutableGlobalProperties view of this map.
+     * @param map The map to wrap.
+     */
     private ImmutableGlobalProperties(HashMap<String, GlobalProperty> map) {
         this.map = map;
     }
 
+    /**
+     * Returns the GlobalProperty associated with the name, or
+     * null if there is no such property.
+     * @param propertyName The property name.
+     * @return The GlobalProperty if found, or null.
+     */
     public GlobalProperty get(String propertyName) {
         GlobalProperty gp = map.get(propertyName);
         if(gp == null) {
@@ -127,10 +151,20 @@ public String replaceGlobalProperties(String instanceName,
         }
     }
 
+    /**
+     * Returns a view over the property names.
+     * @return The set of property names.
+     */
     public Set<String> keySet() {
-        return map.keySet();
+        return Collections.unmodifiableSet(map.keySet());
     }
 
+    /**
+     * Returns an unmodifiable view of this GlobalProperties.
+     * <p>
+     * Changes in the underlying properties are reflected in this view.
+     * @return An unmodifiable view of the properties.
+     */
     public ImmutableGlobalProperties getImmutableProperties() {
         return new ImmutableGlobalProperties(map);
     }

olcut-core/src/test/java/com/oracle/labs/mlrg/olcut/config/GlobalPropertyTest.java

@@ -30,7 +30,6 @@
 
 import com.oracle.labs.mlrg.olcut.config.io.ConfigLoaderException;
 import com.oracle.labs.mlrg.olcut.util.Util;
-import java.io.IOException;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;

olcut-core/src/test/java/com/oracle/labs/mlrg/olcut/config/property/GlobalPropertiesTest.java

@@ -0,0 +1,25 @@
+package com.oracle.labs.mlrg.olcut.config.property;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.Properties;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+public class GlobalPropertiesTest {
+
+    @Test
+    public void strangeSystemProperties() {
+        Properties sysProps = new Properties();
+        sysProps.setProperty("this property has spaces spaces are bad","some-value");
+        sysProps.setProperty("this property has weird punctuation !@#$%^&&*","another-value");
+        sysProps.setProperty("this.property.conforms.to.the.global.props.regex","also-a-value");
+
+        GlobalProperties gp = new GlobalProperties();
+        gp.importProperties(sysProps);
+        assertNull(gp.get("this property has spaces spaces are bad"));
+        assertNull(gp.get("this property has weird punctuation !@#$%^&&*"));
+        assertEquals(new GlobalProperty("also-a-value"),gp.get("this.property.conforms.to.the.global.props.regex"));
+    }
+}