From b06aed6911a895afb0238830d310e2532c3eb4b9 Mon Sep 17 00:00:00 2001 From: Peter Donald Date: Thu, 6 Dec 2001 09:11:17 +0000 Subject: [PATCH] Update to remove warnings generated by antcall due to last immutability tightening. Did this by adding a protected constructor to property. Made sure that operations internal to Project could not generate these warnings by adding a new setPropertyInternal method that didn't do any checks. Also made the getUserProperties and getProperties methods return copies of maps rather than direct references. Updated WHATSNEW to warn about these issues. Added some unit tests to verify that property immutability is enforced - where we actually do enforce it ;) Submitted by: "Erik Hatcher" git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@270078 13f79535-47bb-0310-9956-ffa450edef68 --- WHATSNEW | 4 +- src/etc/testcases/core/immutable.xml | 63 ++++++++++ src/main/org/apache/tools/ant/Project.java | 46 +++++-- .../org/apache/tools/ant/taskdefs/Ant.java | 5 +- .../apache/tools/ant/taskdefs/Property.java | 24 ++-- .../org/apache/tools/ant/ImmutableTest.java | 115 ++++++++++++++++++ 6 files changed, 238 insertions(+), 19 deletions(-) create mode 100644 src/etc/testcases/core/immutable.xml create mode 100644 src/testcases/org/apache/tools/ant/ImmutableTest.java diff --git a/WHATSNEW b/WHATSNEW index 0e2f8e67c..85db4b4dd 100644 --- a/WHATSNEW +++ b/WHATSNEW @@ -17,7 +17,9 @@ Changes that could break older environments: instead. * Some loopholes in the immutability rule have been closed. It is no longer - possible to overwrite a property using tasks like or . + possible to overwrite a property using tasks like , , + , , or . In some exceptional cases it will + generate a warning if you attempt to subvert property immutability. * Taskwriters please note: Whenever tasks had any overloaded set* methods, Ant's introspection mechanism would select the last overloaded method diff --git a/src/etc/testcases/core/immutable.xml b/src/etc/testcases/core/immutable.xml new file mode 100644 index 000000000..e32b46924 --- /dev/null +++ b/src/etc/testcases/core/immutable.xml @@ -0,0 +1,63 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/main/org/apache/tools/ant/Project.java b/src/main/org/apache/tools/ant/Project.java index 2e9658f77..9b7ad16ff 100644 --- a/src/main/org/apache/tools/ant/Project.java +++ b/src/main/org/apache/tools/ant/Project.java @@ -159,7 +159,7 @@ public class Project { public Project() { fileUtils = FileUtils.newFileUtils(); } - + /** * Initialise the project. * @@ -353,6 +353,18 @@ public class Project { userProperties.put(name, value); properties.put(name, value); } + + /** + * Allows Project and subclasses to set a property unless its + * already defined as a user property. There are a few cases + * internally to Project that need to do this currently. + */ + private void setPropertyInternal(String name, String value) { + if (null != userProperties.get(name)) { + return; + } + properties.put(name, value); + } /** * query a property. @@ -377,19 +389,37 @@ public class Project { } /** - * get the property hashtable + * get a copy of the property hashtable * @return the hashtable containing all properties, user included */ public Hashtable getProperties() { - return properties; + Hashtable propertiesCopy = new Hashtable(); + + Enumeration e = properties.keys(); + while (e.hasMoreElements()) { + Object name = e.nextElement(); + Object value = properties.get(name); + propertiesCopy.put(name, value); + } + + return propertiesCopy; } /** - * get the user property hashtable + * get a copy of the user property hashtable * @return the hashtable user properties only */ public Hashtable getUserProperties() { - return userProperties; + Hashtable propertiesCopy = new Hashtable(); + + Enumeration e = userProperties.keys(); + while (e.hasMoreElements()) { + Object name = e.nextElement(); + Object value = properties.get(name); + propertiesCopy.put(name, value); + } + + return propertiesCopy; } /** @@ -486,7 +516,7 @@ public class Project { if (!baseDir.isDirectory()) throw new BuildException("Basedir " + baseDir.getAbsolutePath() + " is not a directory"); this.baseDir = baseDir; - setProperty( "basedir", this.baseDir.getPath()); + setPropertyInternal( "basedir", this.baseDir.getPath()); String msg = "Project base dir set to: " + this.baseDir; log(msg, MSG_VERBOSE); } @@ -521,7 +551,7 @@ public class Project { * @throws BuildException if this Java version is not supported */ public void setJavaVersionProperty() throws BuildException { - setProperty("ant.java.version", javaVersion); + setPropertyInternal("ant.java.version", javaVersion); // sanity check if (javaVersion == JAVA_1_0) { @@ -543,7 +573,7 @@ public class Project { while (e.hasMoreElements()) { Object name = e.nextElement(); String value = systemP.get(name).toString(); - this.setProperty(name.toString(), value); + this.setPropertyInternal(name.toString(), value); } } diff --git a/src/main/org/apache/tools/ant/taskdefs/Ant.java b/src/main/org/apache/tools/ant/taskdefs/Ant.java index c9c008c72..15c12d56e 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Ant.java +++ b/src/main/org/apache/tools/ant/taskdefs/Ant.java @@ -330,8 +330,9 @@ public class Ant extends Task { if (newProject == null) { reinit(); } - Property p=(Property)newProject.createTask("property"); - p.setUserProperty(true); + Property p = new Property(true); + p.setProject(newProject); + p.setTaskName("property"); properties.addElement( p ); return p; } diff --git a/src/main/org/apache/tools/ant/taskdefs/Property.java b/src/main/org/apache/tools/ant/taskdefs/Property.java index d13308890..6af2cbb92 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Property.java +++ b/src/main/org/apache/tools/ant/taskdefs/Property.java @@ -85,10 +85,18 @@ public class Property extends Task { protected String resource; protected Path classpath; protected String env; - protected Reference ref = null; - - protected boolean userProperty=false; // set read-only properties + protected Reference ref; + protected boolean userProperty; // set read-only properties + + public Property() { + super(); + } + + protected Property(boolean userProperty) { + this.userProperty = userProperty; + } + public void setName(String name) { this.name = name; } @@ -161,10 +169,11 @@ public class Property extends Task { } /** - * @deprecated - */ + * @deprecated This was never a supported feature and has been deprecated without replacement + */ public void setUserProperty(boolean userProperty) { - this.userProperty = userProperty; + log("DEPRECATED: Ignoring request to set user property in Property task.", + Project.MSG_WARN); } public String toString() { @@ -287,12 +296,11 @@ public class Property extends Task { protected void addProperty(String n, String v) { if( userProperty ) { - log("DEPRECATED - Setting user properties through the Property task has been deprecated."); if (project.getUserProperty(n) == null) { project.setUserProperty(n, v); } else { log("Override ignored for " + n, Project.MSG_VERBOSE); - } + } } else { project.setNewProperty(n, v); } diff --git a/src/testcases/org/apache/tools/ant/ImmutableTest.java b/src/testcases/org/apache/tools/ant/ImmutableTest.java new file mode 100644 index 000000000..58b336f9e --- /dev/null +++ b/src/testcases/org/apache/tools/ant/ImmutableTest.java @@ -0,0 +1,115 @@ +/* + * The Apache Software License, Version 1.1 + * + * Copyright (c) 2000 The Apache Software Foundation. All rights + * reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * 3. The end-user documentation included with the redistribution, if + * any, must include the following acknowlegement: + * "This product includes software developed by the + * Apache Software Foundation (http://www.apache.org/)." + * Alternately, this acknowlegement may appear in the software itself, + * if and wherever such third-party acknowlegements normally appear. + * + * 4. The names "The Jakarta Project", "Ant", and "Apache Software + * Foundation" must not be used to endorse or promote products derived + * from this software without prior written permission. For written + * permission, please contact apache@apache.org. + * + * 5. Products derived from this software may not be called "Apache" + * nor may "Apache" appear in their names without prior written + * permission of the Apache Group. + * + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR + * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + */ + +package org.apache.tools.ant; + +import org.apache.tools.ant.BuildFileTest; + +/** + * @author Erik Hatcher + */ +public class ImmutableTest extends BuildFileTest { + + public ImmutableTest(String name) { + super(name); + } + + public void setUp() { + configureProject("src/etc/testcases/core/immutable.xml"); + } + + // override allowed on + public void test1() { + executeTarget("test1"); + assertEquals("override", project.getProperty("test")); + } + + // ensure 's new prefix attribute is working + public void test2() { + executeTarget("test2"); + assertNotNull(project.getProperty("DSTAMP")); + assertNotNull(project.getProperty("start.DSTAMP")); + } + + // ensure follows the immutability rule + public void test3() { + executeTarget("test3"); + assertEquals("original", project.getProperty("DSTAMP")); + } + + // ensure follows the immutability rule + public void test4() { + executeTarget("test4"); + assertEquals("original", project.getProperty("test")); + } + // ensure follows the immutability rule + public void test5() { + executeTarget("test5"); + assertEquals("original", project.getProperty("test")); + } + + // ensure follows the immutability rule + public void test6() { + executeTarget("test6"); + assertEquals("original", project.getProperty("test1")); + assertEquals("original", project.getProperty("test2")); + } + + // ensure follows the immutability rule + public void test7() { + executeTarget("test7"); + assertEquals("original", project.getProperty("test")); + } +} +