This was trickier than I thought it would be to fix because the code for the default logic is in three places for each type supported (int, String, date). But I've fixed the particular bug reported including the other two types, and hopefully have not introduced any new ones. Thanks to Levi's test case I was quickly able to add what I needed there to identify the bug and verify it had been fixed with my changes. Given that the code is a bit convoluted, more test cases should probably be added to verify that its all working properly.
Levi - I adjusted your PropertyFileTest code to use the 'optional'
subdirectory for the build and properties files. I'm guessing from your
comments that you are going to refactor this task even further.
This is a very valuable task, and one that I feel should be moved to a
built-in task eventually.
Let me know if there are any questions/issues with this submission.
Erik
cvs diff -u docs/manual/OptionalTasks/propertyfile.html
src/main/org/apache/tools/ant/taskdefs/optional/PropertyFile.java
Index: docs/manual/OptionalTasks/propertyfile.html
===================================================================
RCS file:
/home/cvspublic/jakarta-ant/docs/manual/OptionalTasks/propertyfile.html,v
retrieving revision 1.2
diff -u -r1.2 propertyfile.html
--- docs/manual/OptionalTasks/propertyfile.html 2001/02/13 12:31:56 1.2
+++ docs/manual/OptionalTasks/propertyfile.html 2001/10/04 21:15:09
@@ -56,7 +56,7 @@
<h3>Parameters specified as nested elements</h3>
<h4><a name="entryElement">Entry</a></h4>
<p>Use nested <code><entry></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,9 +71,15 @@
<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:
"now" or "never".</td>
+ </tr>
+ <tr>
<td valign="top">type</td>
<td valign="top">Regard the value as : int, date or string (default)</td>
<td valign="top" align="center">No</td>
@@ -85,19 +91,29 @@
<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:
"now" or "never".</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
be parsed and formatted accordingly.</td>
<td valign="top" align="center">No</td>
</tr>
</table>
+<p>The rules used when setting a property value are shown below. 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> </p>
+
<h3>Examples</h3>
<p>The following changes the my.properties file. Assume my.properties look
like:</p>
@@ -153,4 +169,4 @@
<p>Each time called, a "." will be appended to "progress"
</p>
</body>
-</html>
+</html>
\ No newline at end of file
Index: src/main/org/apache/tools/ant/taskdefs/optional/PropertyFile.java
===================================================================
RCS file:
/home/cvspublic/jakarta-ant/src/main/org/apache/tools/ant/taskdefs/optional/PropertyFile.java,v
retrieving revision 1.5
diff -u -r1.5 PropertyFile.java
--- src/main/org/apache/tools/ant/taskdefs/optional/PropertyFile.java
2001/02/04 10:40:07 1.5
+++ src/main/org/apache/tools/ant/taskdefs/optional/PropertyFile.java
2001/10/04 21:15:10
@@ -307,7 +307,7 @@
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 @@
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());
@@ -420,38 +435,23 @@
}
}
-
- // 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 @@
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 @@
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 @@
}
m_value = newValue;
}
-
+
/**
* Check if parameter combinations can be supported
*/
@@ -564,8 +569,8 @@
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");
<?xml version="1.0"?>
<project name="propertyfile-test" default="main" basedir=".">
<property file="propertyfile.build.properties"/>
<target name="main">
<fail>
This file is for testing purposes only...
@see PropertyFileTest.java for more info.
</fail>
</target>
<target name="update-existing-properties">
<propertyfile file="${test.propertyfile}"
comment="unit test for the property file task...">
<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>
</project>
PropertyFileTest.java
Description: Binary data
