Bug 40184

Summary:	[PATCH] Move property expansion from Property and MacroInstance to the PropertyHelper class
Product:	Ant	Reporter:	Stefano Marsili <efanomars>
Component:	Core	Assignee:	Ant Notifications List <notifications>
Status:	RESOLVED FIXED
Severity:	enhancement	CC:	mguillemot
Priority:	P2	Keywords:	PatchAvailable
Version:	1.7.0
Target Milestone:	1.8.0
Hardware:	PC
OS:	Linux
Attachments:	Modified Property, PropertyHelper Modified Property, MacroInstance and PropertyHelper

Description Stefano Marsili 2006-08-04 05:32:00 UTC

Why: 
  The property task currently does resolve and expand
  the properties read from a property file 
  (with <property file="..."/>) in an unsynchronised way,
  both accessing the Project and PropertyHelper interfaces.
  In a multithreaded situation this could lead to
  unexpected results. It would also be better to have all
  property handling code in the property helper.

Proposed solution:
  The handling of the properties is naturally moved from the
  Property task to the PropertyHelper. This allows easier 
  synchronisation and possibly code reuse. It also lets
  custom property helpers reimplement the exposed methods.

Stefano Marsili

Comment 1 Stefano Marsili 2006-08-04 05:38:04 UTC

Created attachment 18677 [details]
Modified Property, PropertyHelper

** Sources **
PropertyHelper.java:
 - added public setNewInheritedProperty(ns, n, v)
   like setNewProperty, adds a user property only if 
   there isn't one already defined
 - added addNewProperties(ns, props, prefix)
   resolves, expands, prefixes and adds a set of properties 
 - added addNewInheritedProperties(ns, props, prefix)
   resolves, expands, prefixes and adds a set of user properties 

Property.java:
 - removed resolveAllProperties(props), now in PropertyHelper
 - modified addProperties and addProperty to call the 
   corresponding PropertyHelper methods

** Tests **
No additional testcases added.

** Note **
Modified and patched against latest 1.70alpha.

I did this patch a bit quickly. Probably everything 
(interface, comments, implementation) can be improved.
Maybe the public methods added to the PropertyHelper 
should also be added to the Project (and called from 
the Property task)?
(I also noticed that replaceProperties is not synchronized,
I'm not sure but maybe properties could be set while 
replacing, leading to inconsistent results (not in the patch).)

Stefano Marsili

Comment 2 Stefano Marsili 2006-08-14 06:29:13 UTC

I realised that also MacroInstance's expansion of attributes 
(macrodef task) could be moved to the PropertyHelper so that
all evaluation of expressions is centralized (and may also be
reimplemented).

Thank you
Stefano Marsili

Comment 3 Stefano Marsili 2006-08-14 06:32:08 UTC

Created attachment 18713 [details]
Modified Property, MacroInstance and PropertyHelper

Correction, improvements, ideas are welcome.
Stefano Marsili

Comment 4 Stefan Bodewig 2009-09-01 05:04:40 UTC

Property#resolveAllProperties uses PropertyHelper by now (svn revision 578769 ) while the PropertyHelper logic has been modified strong enough to keep the MacroInstance code separate (we most likely do not want to use the same logic for macro attribute expansions as for property expansions).

Using FIXED; but WONTFIX or "INVALID by now" would be as good as resolution types.