VELOCITY-704 default all scope controls to off, except $foreach and do a few other tiny, scope-related performance boosts (merge from trunk, r778045)
git-svn-id: https://svn.apache.org/repos/asf/velocity/engine/branches/2.0_Exp@778057 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 168c18c..ac94596 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -75,9 +75,10 @@
Change the scoping behavior of Velocity, keeping it optional (everything global
scoped by default) but now providing an explicit namespace in content-containing
directives (like macros, #foreach, #parse, etc.) in which references that should
- stay local may be kept. This is accompanied by numerous related changes, including:
+ stay local may be kept. This is accompanied by numerous related changes, including:
<ul>
- <li>A Scope reference is now available within each content directive:
+ <li>A Scope reference can now be automatically made
+ available within each content directive:
<ul>
<li>$template in Template.merge and #parse content<li>
<li>$macro in #macro</li>
@@ -88,8 +89,9 @@
(where &lt;amacro&gt; is the name of a macro being called with a body)</li>
</ul>
</li>
- <li>Allowing the above to be suppressed by setting a velocity property like:
- <br/><code>foreach.provide.scope.control = false</code></li>
+ <li>For performance and compatibility these are all off by
+ default, *except* for $foreach. The others may be enabled by setting a velocity property like:
+ <br/><code>macro.provide.scope.control = true</code></li>
<li>When scopes of the same type are nested make the parent Scope available
through the child (e.g. $foreach.parent or $foreach.topmost).</li>
<li>When a Scope reference overrides an existing reference that is not a Scope,
diff --git a/src/java/org/apache/velocity/Template.java b/src/java/org/apache/velocity/Template.java
index c14c044..883d0b6 100644
--- a/src/java/org/apache/velocity/Template.java
+++ b/src/java/org/apache/velocity/Template.java
@@ -71,7 +71,7 @@
* the scope object into the context.
*/
private String scopeName = "template";
- private boolean provideScope = true;
+ private boolean provideScope = false;
private VelocityException errorCondition = null;
diff --git a/src/java/org/apache/velocity/app/event/implement/EscapeReference.java b/src/java/org/apache/velocity/app/event/implement/EscapeReference.java
index bd24a25..3bcfd3d 100644
--- a/src/java/org/apache/velocity/app/event/implement/EscapeReference.java
+++ b/src/java/org/apache/velocity/app/event/implement/EscapeReference.java
@@ -21,6 +21,8 @@
import org.apache.oro.text.perl.MalformedPerl5PatternException;
import org.apache.oro.text.perl.Perl5Util;
+//import java.util.regex.Pattern;
+//import java.util.regex.Matcher;
import org.apache.velocity.app.event.ReferenceInsertionEventHandler;
import org.apache.velocity.runtime.RuntimeServices;
import org.apache.velocity.util.RuntimeServicesAware;
@@ -57,12 +59,13 @@
*/
public abstract class EscapeReference implements ReferenceInsertionEventHandler,RuntimeServicesAware {
-
+//
private Perl5Util perl = new Perl5Util();
private RuntimeServices rs;
private String matchRegExp = null;
+ //private Pattern pattern = null;
/**
* Escape the given text. Override this in a subclass to do the actual
@@ -98,11 +101,13 @@
}
if (matchRegExp == null)
+ //if (pattern == null)
{
return escape(value);
}
else if (perl.match(matchRegExp,reference))
+ //else if (pattern.matcher(reference).matches())
{
return escape(value);
}
diff --git a/src/java/org/apache/velocity/runtime/RuntimeInstance.java b/src/java/org/apache/velocity/runtime/RuntimeInstance.java
index 1eddcc7..cdee29e 100644
--- a/src/java/org/apache/velocity/runtime/RuntimeInstance.java
+++ b/src/java/org/apache/velocity/runtime/RuntimeInstance.java
@@ -196,7 +196,7 @@
* Settings for provision of root scope for evaluate(...) calls.
*/
private String evaluateScopeName = "evaluate";
- private boolean provideEvaluateScope = true;
+ private boolean provideEvaluateScope = false;
/*
* Opaque reference to something specificed by the
diff --git a/src/java/org/apache/velocity/runtime/defaults/velocity.properties b/src/java/org/apache/velocity/runtime/defaults/velocity.properties
index 93449b3..352a8bc 100644
--- a/src/java/org/apache/velocity/runtime/defaults/velocity.properties
+++ b/src/java/org/apache/velocity/runtime/defaults/velocity.properties
@@ -75,12 +75,12 @@
# These are the properties that govern whether or not a Scope object
# is automatically provided for each of the given scopes to serve as a
# scope-safe reference namespace and "label" for #break calls. The default
-# for all of these is true. Note that <bodymacroname> should be replaced by
+# for most of these is false. Note that <bodymacroname> should be replaced by
# name of macros that take bodies for which you want to suppress the scope.
# ----------------------------------------------------------------------------
# template.provide.scope.control = false
# evaluate.provide.scope.control = false
-# foreach.provide.scope.control = false
+foreach.provide.scope.control = true
# macro.provide.scope.control = false
# define.provide.scope.control = false
# <bodymacroname>.provide.scope.control = false
diff --git a/src/java/org/apache/velocity/runtime/directive/Directive.java b/src/java/org/apache/velocity/runtime/directive/Directive.java
index 94a2bcb..67ed23c 100644
--- a/src/java/org/apache/velocity/runtime/directive/Directive.java
+++ b/src/java/org/apache/velocity/runtime/directive/Directive.java
@@ -46,7 +46,7 @@
{
private int line = 0;
private int column = 0;
- private boolean provideScope = true;
+ private boolean provideScope = false;
private String templateName;
/**
@@ -147,7 +147,7 @@
rsvc = rs;
String property = getScopeName()+'.'+RuntimeConstants.PROVIDE_SCOPE_CONTROL;
- this.provideScope = rsvc.getBoolean(property, true);
+ this.provideScope = rsvc.getBoolean(property, provideScope);
}
/**
@@ -193,10 +193,15 @@
{
String name = getScopeName();
Object previous = context.get(name);
- context.put(name, new Scope(this, previous));
+ context.put(name, makeScope(previous));
}
}
+ protected Scope makeScope(Object prev)
+ {
+ return new Scope(this, prev);
+ }
+
/**
* This cleans up any scope control for this directive after rendering,
* assuming the scope control was turned on.
@@ -208,13 +213,7 @@
String name = getScopeName();
Object obj = context.get(name);
- // the user can override the scope with a #set,
- // since that means they don't care about a replaced value
- // and obviously aren't too keen on their scope control,
- // and especially since #set is meant to be handled globally,
- // we'll assume they know what they're doing and not worry
- // about replacing anything superseded by this directive's scope
- if (obj instanceof Scope)
+ try
{
Scope scope = (Scope)obj;
if (scope.getParent() != null)
@@ -230,6 +229,15 @@
context.remove(name);
}
}
+ catch (ClassCastException cce)
+ {
+ // the user can override the scope with a #set,
+ // since that means they don't care about a replaced value
+ // and obviously aren't too keen on their scope control,
+ // and especially since #set is meant to be handled globally,
+ // we'll assume they know what they're doing and not worry
+ // about replacing anything superseded by this directive's scope
+ }
}
}
diff --git a/src/java/org/apache/velocity/runtime/directive/Scope.java b/src/java/org/apache/velocity/runtime/directive/Scope.java
index 21d4ce8..acb1e3c 100755
--- a/src/java/org/apache/velocity/runtime/directive/Scope.java
+++ b/src/java/org/apache/velocity/runtime/directive/Scope.java
@@ -33,23 +33,23 @@
public class Scope extends AbstractMap
{
private Map storage;
- protected final Object replaced;
- protected final Scope parent;
+ private Object replaced;
+ private Scope parent;
protected final Object owner;
public Scope(Object owner, Object previous)
{
this.owner = owner;
- if (previous instanceof Scope)
+ if (previous != null)
{
- this.parent = (Scope)previous;
- // keep easy access to the user's object
- this.replaced = this.parent.replaced;
- }
- else
- {
- this.parent = null;
- this.replaced = previous;
+ try
+ {
+ this.parent = (Scope)previous;
+ }
+ catch (ClassCastException cce)
+ {
+ this.replaced = previous;
+ }
}
}
@@ -137,6 +137,10 @@
*/
public Object getReplaced()
{
+ if (replaced == null && parent != null)
+ {
+ return parent.getReplaced();
+ }
return replaced;
}
diff --git a/src/java/org/apache/velocity/runtime/resource/ResourceCacheImpl.java b/src/java/org/apache/velocity/runtime/resource/ResourceCacheImpl.java
index d16ea81..9721ab6 100644
--- a/src/java/org/apache/velocity/runtime/resource/ResourceCacheImpl.java
+++ b/src/java/org/apache/velocity/runtime/resource/ResourceCacheImpl.java
@@ -28,7 +28,7 @@
import org.apache.commons.collections.map.LRUMap;
import org.apache.velocity.runtime.RuntimeConstants;
import org.apache.velocity.runtime.RuntimeServices;
-import org.apache.velocity.util.MapFactory;
+import java.util.concurrent.ConcurrentHashMap;
/**
* Default implementation of the resource cache for the default
@@ -49,7 +49,7 @@
/**
* Cache storage, assumed to be thread-safe.
*/
- protected Map cache = MapFactory.create(512, 0.5f, 30, false);
+ protected Map cache = new ConcurrentHashMap(512, 0.5f, 30);
/**
* Runtime services, generally initialized by the
diff --git a/src/test/org/apache/velocity/test/BreakDirectiveTestCase.java b/src/test/org/apache/velocity/test/BreakDirectiveTestCase.java
index 0757c2e..45c7136 100644
--- a/src/test/org/apache/velocity/test/BreakDirectiveTestCase.java
+++ b/src/test/org/apache/velocity/test/BreakDirectiveTestCase.java
@@ -19,6 +19,8 @@
* under the License.
*/
+import org.apache.velocity.app.VelocityEngine;
+
/**
* This class tests the break directive.
*/
@@ -29,6 +31,15 @@
super(name);
}
+ protected void setUpEngine(VelocityEngine engine)
+ {
+ engine.setProperty("a.provide.scope.control", "true");
+ engine.setProperty("define.provide.scope.control", "true");
+ engine.setProperty("evaluate.provide.scope.control", "true");
+ engine.setProperty("macro.provide.scope.control", "true");
+ engine.setProperty("template.provide.scope.control", "true");
+ }
+
public void testBadArgs()
{
context.put("foo","foo");
diff --git a/src/test/org/apache/velocity/test/EvaluateTestCase.java b/src/test/org/apache/velocity/test/EvaluateTestCase.java
index e0b5e19..cefd490 100644
--- a/src/test/org/apache/velocity/test/EvaluateTestCase.java
+++ b/src/test/org/apache/velocity/test/EvaluateTestCase.java
@@ -131,6 +131,7 @@
*/
public void testStopAndBreak()
{
+ engine.setProperty("evaluate.provide.scope.control", "true");
assertEvalEquals("t ", "t #stop t2 #evaluate('t3')");
assertEvalEquals("t ", "t #break t2 #evaluate('t3')");
assertEvalEquals("t t2 t3 ", "t t2 #evaluate('t3 #stop t4') t5");
diff --git a/src/test/org/apache/velocity/test/ScopeTestCase.java b/src/test/org/apache/velocity/test/ScopeTestCase.java
index 3976827..cd38cb3 100755
--- a/src/test/org/apache/velocity/test/ScopeTestCase.java
+++ b/src/test/org/apache/velocity/test/ScopeTestCase.java
@@ -21,6 +21,7 @@
import java.util.HashMap;
import org.apache.velocity.VelocityContext;
+import org.apache.velocity.app.VelocityEngine;
import org.apache.velocity.runtime.RuntimeConstants;
import org.apache.velocity.runtime.directive.Scope;
@@ -34,6 +35,17 @@
super(name);
}
+ protected void setUpEngine(VelocityEngine engine)
+ {
+ engine.setProperty("a.provide.scope.control", "true");
+ engine.setProperty("define.provide.scope.control", "true");
+ engine.setProperty("evaluate.provide.scope.control", "true");
+ engine.setProperty("foo.provide.scope.control", "true");
+ engine.setProperty("macro.provide.scope.control", "true");
+ engine.setProperty("template.provide.scope.control", "true");
+ engine.setProperty("vm.provide.scope.control", "true");
+ }
+
public void testRootTemplateMergeScope()
{
addTemplate("foo", "foo#break($template)bar");
diff --git a/xdocs/docs/developer-guide.xml b/xdocs/docs/developer-guide.xml
index 4cd7821..d900712 100644
--- a/xdocs/docs/developer-guide.xml
+++ b/xdocs/docs/developer-guide.xml
@@ -1591,10 +1591,12 @@
</p>
<p>
-<code>define.provide.scope.control = true</code><br/>
-Used to control the automatic provision of the $define scope control
-during #define() calls. The default is true.
-Set it to false if unused and you want a tiny performance boost.
+<code>define.provide.scope.control = false</code><br/>
+Used to turn on the automatic provision of the $define scope control
+during #define() calls. The default is false.
+Set it to true if you want a local, managed namespace
+you can put references in when within a #define block or if you want it for
+more advanced #break usage.
</p>
<p>
@@ -1602,10 +1604,12 @@
</p>
<p>
-<code>evaluate.provide.scope.control = true</code><br/>
-Used to control the automatic provision of the $evaluate scope
+<code>evaluate.provide.scope.control = false</code><br/>
+Used to turn on the automatic provision of the $evaluate scope
during #evaluate() or Velocity[Engine].evaluate(...) calls. The default
-is true. Set it to false if unused and you want a tiny performance boost.
+is false. Set it to true if you want a local, managed namespace
+you can put references in during an evaluation or if you want it for
+more advanced #break usage.
</p>
<p>
@@ -1615,7 +1619,8 @@
<p>
<code>foreach.provide.scope.control = true</code><br/>
Used to control the automatic provision of the $foreach scope
-during #foreach calls. The default is true.
+during #foreach calls. This gives access to the foreach status information
+(e.g. $foreach.index or $foreach.hasNext). The default is true.
Set it to false if unused and you want a tiny performance boost.
</p>
@@ -1653,10 +1658,11 @@
prevents runaway #parse() recursion.
</p>
<p>
-<code>template.provide.scope.control = true</code><br/>
-Used to control the automatic provision of the $template scope control
-during #parse calls (and template.merge(...) calls). The default is true.
-Set it to false if unused and you want a tiny performance boost.
+<code>template.provide.scope.control = false</code><br/>
+Used to turn on the automatic provision of the $template scope control
+during #parse calls and template.merge(...) calls. The default is false.
+Set it to true if you want a secure namespace for your template variables
+or more advanced #break control.
</p>
@@ -1829,17 +1835,18 @@
macro feature was introduced in Velocity 1.7.
</p>
<p>
-<code>macro.provide.scope.control = true</code><br/>
-Used to control the automatic provision of the $macro scope control
-during #macro calls. The default is true.
-Set it to false if unused and you want a tiny performance boost.
+<code>macro.provide.scope.control = false</code><br/>
+Used to turn on the automatic provision of the $macro scope control
+during #macro calls. The default is false.
+Set it to true if you need a local namespace in macros or more
+advanced #break controls.
</p>
<p>
-<code><somebodymacro>.provide.scope.control = true</code><br/>
-Used to control the automatic provision of the $<nameofthemacro> scope control
+<code><somebodymacro>.provide.scope.control = false</code><br/>
+Used to turn on the automatic provision of the $<nameofthemacro> scope control
during a call to a macro with a body (e.g. #@foo #set($foo.a=$b) ... $foo.a #end).
-The default is true. Set it to false if you heavily use that body macro without
-using the scope control and you want a tiny performance boost.
+The default is false. Set it to true if you happen to need a namespace just
+for your macro's body content or more advanced #break controls.
</p>
<p>
