Merge pull request #156 from acidmoose/flags-properties-2

Flags through properties file gets no special treatment.
Cloudname · May 7, 2015 · 444caba · 444caba
2 parents 9ebaf5e + 503f185
commit 444caba
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 13 deletions.
diff --git a/flags/src/main/java/org/cloudname/flags/Flags.java b/flags/src/main/java/org/cloudname/flags/Flags.java
@@ -362,13 +362,16 @@ public Flags parse(String[] args) {
                     props.load(stream);
                     for (Enumeration<?> keys = props.propertyNames(); keys.hasMoreElements();) {
                         String flagName = (String) keys.nextElement();
-                        if (!options.containsKey(flagName) || optionSet.hasArgument(flagName)) {
-                            //Properties contains something not in options or is already set by commandline argument
+                        if (optionSet.hasArgument(flagName)) {
+                            //Properties contains something already set by commandline argument
                             //Command line argument takes precedence over properties file
                             continue;
                         }
                         newArgs.add("--" + flagName);
-                        newArgs.add(props.getProperty(flagName));
+                        String value = props.getProperty(flagName);
+                        if (value != null && ! value.isEmpty()) {
+                            newArgs.add(value);
+                        }
                     }
 
                     stream.close();

diff --git a/flags/src/test/java/org/cloudname/flags/FlagsPropertiesFile.java b/flags/src/test/java/org/cloudname/flags/FlagsPropertiesFile.java
@@ -9,4 +9,7 @@ public class FlagsPropertiesFile {
 
     @Flag(name="comments", description = "should not be set")
     static String comments;
+
+    @Flag(name="booleanvalue")
+    static boolean booleanValue = false;
 }
diff --git a/flags/src/test/java/org/cloudname/flags/FlagsTest.java b/flags/src/test/java/org/cloudname/flags/FlagsTest.java
@@ -24,6 +24,9 @@
  */
 public class FlagsTest {
 
+    @Rule
+    public ExpectedException exception = ExpectedException.none();
+
     /**
      * Test all supported field types.
      */
@@ -301,21 +304,40 @@ public void testLoadingFromPropertiesFile() throws Exception {
         File propertiesFile = File.createTempFile("test", "properties");
         propertiesFile.setWritable(true);
         FileOutputStream fio = new FileOutputStream(propertiesFile);
-        String properties = "integer=1\n\n" + "#comments=not included\n" +
-            "not-in-options=abc\n" + "name=myName\n" +
-            "help\n" + "version\n";
+        String properties = "integer=1\n\n"
+                + "#comments=not included\n"
+                + "name=myName\n"
+                + "booleanvalue\n"
+                + "help\n";
         fio.write(properties.getBytes());
         fio.close();
         Flags flags = new Flags()
             .loadOpts(FlagsPropertiesFile.class)
             .parse(new String[]{"--properties-file", propertiesFile.getAbsolutePath()});
-        assertFalse("Help seems to have been called. It should not have been.", flags.helpFlagged());
-        assertFalse("Version seems to have been called. It should not have been.", flags.versionFlagged());
         assertEquals("myName", FlagsPropertiesFile.name);
+        assertTrue(FlagsPropertiesFile.booleanValue);
+        assertTrue(flags.helpFlagged());
         assertEquals(1, FlagsPropertiesFile.integer);
         Assert.assertNull(FlagsPropertiesFile.comments);
     }
 
+    /**
+     * Test that --properties-file does not accept keys which are not defined as flags
+     */
+    @Test
+    public void testLoadingUndefinedFlagFromPropertiesFile() throws Exception {
+        exception.expect(joptsimple.OptionException.class);
+        File propertiesFile = File.createTempFile("test", "properties");
+        propertiesFile.setWritable(true);
+        FileOutputStream fio = new FileOutputStream(propertiesFile);
+        String properties = "not-in-options=abc\n";
+        fio.write(properties.getBytes());
+        fio.close();
+        new Flags()
+                .loadOpts(FlagsPropertiesFile.class)
+                .parse(new String[]{"--properties-file", propertiesFile.getAbsolutePath()});
+    }
+
     /**
      * Test that properties are loaded from two files using two --properties-files options
      */
@@ -340,8 +362,6 @@ public void testLoadingFromMultiplePropertyFiles() throws Exception {
             .parse(new String[]{"--properties-file", propertiesFile1.getAbsolutePath(),
                                 "--properties-file", propertiesFile2.getAbsolutePath()
             });
-        assertFalse("Help seems to have been called. It should not have been.", flags.helpFlagged());
-        assertFalse("Version seems to have been called. It should not have been.", flags.versionFlagged());
         assertEquals("myName", FlagsPropertiesFile.name);
         assertEquals(1, FlagsPropertiesFile.integer);
         Assert.assertNull(FlagsPropertiesFile.comments);
@@ -366,13 +386,11 @@ public void testLoadingFromMultiplePropertyFiles2() throws Exception {
             "help\n" + "version\n";
         fio2.write(properties2.getBytes());
         fio2.close();
-        Flags flags = new Flags()
+        new Flags()
             .loadOpts(FlagsPropertiesFile.class)
             .parse(new String[]{"--properties-file", propertiesFile1.getAbsolutePath() + ';'
                 +propertiesFile2.getAbsolutePath()
             });
-        assertFalse("Help seems to have been called. It should not have been.", flags.helpFlagged());
-        assertFalse("Version seems to have been called. It should not have been.", flags.versionFlagged());
         assertEquals("myName", FlagsPropertiesFile.name);
         assertEquals(1, FlagsPropertiesFile.integer);
         Assert.assertNull(FlagsPropertiesFile.comments);