Make <propertyfile>'s default attribute work as expected.

PR: 3955
Submitted by:	Erik Hatcher <jakarta-ant@ehatchersolutions.com>


git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@269802 13f79535-47bb-0310-9956-ffa450edef68

docs/manual/OptionalTasks/propertyfile.html

@@ -56,7 +56,7 @@ are lost.  Work is being done to make this task a bit more "human friendly&
 <h3>Parameters specified as nested elements</h3>
 <h4><a name="entryElement">Entry</a></h4>
 <p>Use nested <code>&lt;entry&gt;</code>
-elements to specify actual modifcations to the property file itself</p>
+elements to specify actual modifcations to the property file itself.</p>
 <table border="1" cellpadding="2" cellspacing="0">
   <tr>
     <td valign="top"><b>Attribute</b></td>
@@ -71,8 +71,14 @@ elements to specify actual modifcations to the property file itself</p>
   <tr>
     <td valign="top">value</td>
     <td valign="top">Value to set (=), to add (+) or subtract (-)</td>
-    <td valign="top" align="center">Yes</td>
+    <td valign="top" align="center" rowspan="2">At least one must be specified</td>
   </tr>
+  <tr>
+    <td valign="top">default</td>
+    <td valign="top">Initial value to set for a property if it is not
+                     already defined in the property file.<br>
+                     For type date, two additional keywords are allowed: &quot;now&quot; or &quot;never&quot;.</td>
+    </tr>
   <tr>
     <td valign="top">type</td>
     <td valign="top">Regard the value as : int, date or string (default)</td>
@@ -84,13 +90,6 @@ elements to specify actual modifcations to the property file itself</p>
     </td>
     <td valign="top" align="center">No</td>
   </tr>
-  <tr>
-    <td valign="top">default</td>
-    <td valign="top">Initial value to set for a property if it is not
-                     already defined in the property file.<br>
-                     For type date, two additional keywords are allowed: &quot;now&quot; or &quot;never&quot;.</td>
-    <td valign="top" align="center">No</td>
-  </tr>
   <tr>
     <td valign="top">pattern</td>
     <td valign="top">For int and date type only. If present, Values will
@@ -98,6 +97,23 @@ elements to specify actual modifcations to the property file itself</p>
     <td valign="top" align="center">No</td>
   </tr>
 </table>
+<p>The rules used when setting a property value are shown below.&nbsp; The 
+operation occurs <b>after</b> these rules are considered.</p>
+
+<ul>
+  <li>If only value is specified, the property is set to it regardless of its 
+  previous value.</li>
+  <li>If only default is specified and the property previously existed in the 
+  property file, it is unchanged.</li>
+  <li>If only default is specified and the property did not exist in the 
+  property file, the property is set to default.</li>
+  <li>If value and default are both specified and the property previously 
+  existed in the property file, the property is set to value.</li>
+  <li>If value and default are both specified and the property did not exist in 
+  the property file, the property is set to default.</li>
+</ul>
+<p>&nbsp;</p>
+
 <h3>Examples</h3>
 
 <p>The following changes the my.properties file.  Assume my.properties look like:</p>
@@ -153,4 +169,4 @@ Concatenation of strings :</p>
 <p>Each time called, a &quot;.&quot; will be appended to &quot;progress&quot;
 </p>
 </body>
-</html>
+</html>

src/etc/testcases/taskdefs/propertyfile.xml → src/etc/testcases/taskdefs/optional/propertyfile.xml

@@ -18,6 +18,9 @@
         <entry  key="firstname" value="${firstname}" />
         <entry  key="lastname"  value="${lastname}" />
         <entry  key="email"     value="${email}" />
+        <entry  key="phone"     default="${phone}" />
+        <entry  key="age"       default="${age}" type="int"/>
+        <entry  key="date"      default="${date}" type="date"/>
       </propertyfile>
       
    </target>

src/main/org/apache/tools/ant/taskdefs/optional/PropertyFile.java

@@ -307,7 +307,7 @@ public class PropertyFile extends Task
         private String              m_key = null;
         private int                 m_type = Type.STRING_TYPE;
         private int                 m_operation = Operation.EQUALS_OPER;
-        private String              m_value ="1";
+        private String              m_value = "";
         private String              m_default = null;
         private String              m_pattern = null;
 
@@ -394,6 +394,21 @@ public class PropertyFile extends Task
             if (m_pattern == null) m_pattern = "yyyy/MM/dd HH:mm";
             DateFormat fmt = new SimpleDateFormat(m_pattern);
 
+            // special case
+            if (m_default != null &&
+                NOW_VALUE_.equals(m_default.toLowerCase()) &&
+                (m_operation == Operation.INCREMENT_OPER ||
+                 m_operation == Operation.DECREMENT_OPER) ) {
+                oldValue = null;
+            }
+
+            if (oldValue != null) {
+                try {
+                    value.setTime(fmt.parse(oldValue));
+                }
+                catch (ParseException pe)  { /* swollow */ }
+            }
+
             if (m_value != null) {
                 if (NOW_VALUE_.equals(m_value.toLowerCase())) {
                     value.setTime(new Date());
@@ -421,37 +436,22 @@ public class PropertyFile extends Task
                 }
             }
 
-            // special case
-            if (m_default != null &&
-                NOW_VALUE_.equals(m_default.toLowerCase()) &&
-                (m_operation == Operation.INCREMENT_OPER ||
-                 m_operation == Operation.DECREMENT_OPER) ) {
-                oldValue = null;
-            }
-
-            if (oldValue != null) {
-                try {
-                    newValue.setTime(fmt.parse(oldValue));
+            if (m_default != null && oldValue == null) {
+                if (NOW_VALUE_.equals(m_default.toLowerCase())) {
+                    value.setTime(new Date());
                 }
-                catch (ParseException pe)  { /* swollow */ }
-            }
-            else {
-                if (m_default != null) {
-                    if (NOW_VALUE_.equals(m_default.toLowerCase())) {
-                        newValue.setTime(new Date());
-                    }
-                    else if (NULL_VALUE_.equals(m_default.toLowerCase())) {
-                        newValue = null;
-                    }
-                    else {
-                        try {
-                            newValue.setTime(fmt.parse(m_default));
-                        }
-                        catch (ParseException pe)  { /* swollow */ }
+                else if (NULL_VALUE_.equals(m_default.toLowerCase())) {
+                    value = null;
+                }
+                else {
+                    try {
+                        value.setTime(fmt.parse(m_default));
                     }
+                    catch (ParseException pe)  { /* swollow */ }
                 }
             }
 
+
             if (m_operation == Operation.EQUALS_OPER) {
                 newValue = value;
             }
@@ -491,23 +491,23 @@ public class PropertyFile extends Task
             DecimalFormat fmt = (m_pattern != null) ? new DecimalFormat(m_pattern)
                                                     : new DecimalFormat();
 
-            if (m_value != null) {
+            if (oldValue != null) {
                 try {
-                    value = fmt.parse(m_value).intValue();
+                    value = fmt.parse(oldValue).intValue();
                 }
                 catch (NumberFormatException nfe) { /* swollow */ }
                 catch (ParseException pe)  { /* swollow */ }
             }
-            if (oldValue != null) {
+            if (m_value != null) {
                 try {
-                    newValue = fmt.parse(oldValue).intValue();
+                    value = fmt.parse(m_value).intValue();
                 }
                 catch (NumberFormatException nfe) { /* swollow */ }
                 catch (ParseException pe)  { /* swollow */ }
             }
-            else if (m_default != null) {
+            if (m_default != null && oldValue == null) {
                 try {
-                    newValue = fmt.parse(m_default).intValue();
+                    value = fmt.parse(m_default).intValue();
                 }
                 catch (NumberFormatException nfe) { /* swollow */ }
                 catch (ParseException pe)  { /* swollow */ }
@@ -537,16 +537,21 @@ public class PropertyFile extends Task
             String value = "";
             String newValue  = "";
 
+            // the order of events is, of course, very important here
+            // default initially to the old value
+            if (oldValue != null) {
+                value = oldValue;
+            }
+            // but if a value is specified, use it
             if (m_value != null) {
                 value = m_value;
             }
-            if (oldValue != null) {
-                newValue = oldValue;
-            }
-            else if (m_default != null) {
-                newValue = m_default;
+            // even if value is specified, ignore it and set to the default
+            // value if it is specified and there is no previous value
+            if (m_default != null && oldValue == null) {
+                value = m_default;
             }
-
+            
             if (m_operation == Operation.EQUALS_OPER) {
                 newValue = value;
             }
@@ -555,7 +560,7 @@ public class PropertyFile extends Task
             }
             m_value = newValue;
         }
-
+        
         /**
          * Check if parameter combinations can be supported
          */
@@ -564,8 +569,8 @@ public class PropertyFile extends Task
                 m_operation == Operation.DECREMENT_OPER) {
                 throw new BuildException("- is not suported for string properties (key:" + m_key + ")");
             }
-            if (m_value == null) {
-                throw new BuildException("value is mandatory (key:" + m_key + ")");
+            if (m_value == null && m_default == null) {
+                throw new BuildException("value and/or default must be specified (key:" + m_key + ")");
             }
             if (m_key == null) {
                 throw new BuildException("key is mandatory");

src/testcases/org/apache/tools/ant/taskdefs/optional/PropertyFileTest.java

@@ -101,27 +101,33 @@ public class PropertyFileTest extends TaskdefsTest {
      *  update properties that are already defined-
      */
     public void testUpdatesExistingProperties() throws Exception {
-        FileInputStream beforeUpdateFile = new FileInputStream(testPropsFilePath);
-        Properties beforeUpdate = new Properties();
-        beforeUpdate.load(beforeUpdateFile);
+        Properties beforeUpdate = getTestProperties();
         assertEquals(FNAME, beforeUpdate.getProperty(FNAME_KEY));
         assertEquals(LNAME, beforeUpdate.getProperty(LNAME_KEY));
         assertEquals(EMAIL, beforeUpdate.getProperty(EMAIL_KEY));
-        beforeUpdateFile.close();
+        assertEquals(null, beforeUpdate.getProperty(PHONE_KEY));
+        assertEquals(null, beforeUpdate.getProperty(AGE_KEY));
+        assertEquals(null, beforeUpdate.getProperty(DATE_KEY));
       
         // ask ant to update the properties...
         executeTarget("update-existing-properties");
       
-        FileInputStream afterUpdateFile = new FileInputStream(testPropsFilePath);
-        Properties afterUpdate = new Properties();
-        afterUpdate.load(afterUpdateFile);
+        Properties afterUpdate = getTestProperties();
         assertEquals(NEW_FNAME, afterUpdate.getProperty(FNAME_KEY));
         assertEquals(NEW_LNAME, afterUpdate.getProperty(LNAME_KEY));
         assertEquals(NEW_EMAIL, afterUpdate.getProperty(EMAIL_KEY));
-        afterUpdateFile.close();
+        assertEquals(NEW_PHONE, afterUpdate.getProperty(PHONE_KEY));
+        assertEquals(NEW_AGE, afterUpdate.getProperty(AGE_KEY));
+        assertEquals(NEW_DATE, afterUpdate.getProperty(DATE_KEY));
     }
 
-
+    private Properties getTestProperties() throws Exception {
+        Properties testProps = new Properties();
+        FileInputStream propsFile = new FileInputStream(testPropsFilePath);
+        testProps.load(propsFile);
+        propsFile.close();
+        return testProps;
+    }
 
 
     private void initTestPropFile() throws Exception {
@@ -142,6 +148,9 @@ public class PropertyFileTest extends TaskdefsTest {
         buildProps.put(FNAME_KEY, NEW_FNAME);
         buildProps.put(LNAME_KEY, NEW_LNAME);
         buildProps.put(EMAIL_KEY, NEW_EMAIL);
+        buildProps.put(PHONE_KEY, NEW_PHONE);
+        buildProps.put(AGE_KEY, NEW_AGE);
+        buildProps.put(DATE_KEY, NEW_DATE);
       
         FileOutputStream fos = new FileOutputStream(buildPropsFilePath);
         buildProps.save(fos, null);
@@ -153,7 +162,7 @@ public class PropertyFileTest extends TaskdefsTest {
         File tempFile = new File(testPropsFilePath);
         tempFile.delete();
         tempFile = null;
-      
+
         tempFile = new File(buildPropsFilePath);
         tempFile.delete();
         tempFile = null;
@@ -162,13 +171,13 @@ public class PropertyFileTest extends TaskdefsTest {
 
 
     private static final String 
-        projectFilePath     = "src/etc/testcases/taskdefs/propertyfile.xml",
+        projectFilePath     = "src/etc/testcases/taskdefs/optional/propertyfile.xml",
       
         testPropertyFile    = "propertyfile.test.properties",
         testPropertyFileKey = "test.propertyfile",
-        testPropsFilePath   = "src/etc/testcases/taskdefs/" + testPropertyFile,
+        testPropsFilePath   = "src/etc/testcases/taskdefs/optional/" + testPropertyFile,
       
-        buildPropsFilePath  = "src/etc/testcases/taskdefs/propertyfile.build.properties",
+        buildPropsFilePath  = "src/etc/testcases/taskdefs/optional/propertyfile.build.properties",
       
         FNAME     = "Bruce",
         NEW_FNAME = "Clark",
@@ -180,6 +189,15 @@ public class PropertyFileTest extends TaskdefsTest {
       
         EMAIL     = "incredible@hulk.com",
         NEW_EMAIL = "kc@superman.com",
-        EMAIL_KEY = "email";
+        EMAIL_KEY = "email",
+   
+        NEW_PHONE = "(520) 555-1212",
+        PHONE_KEY = "phone",
+      
+        NEW_AGE = "30",
+        AGE_KEY = "age",
+      
+        NEW_DATE = "2001/01/01 12:45",
+        DATE_KEY = "date";
 }