You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by hl...@apache.org on 2008/08/01 00:01:31 UTC

svn commit: r681513 [1/3] - in /tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry: ./ binding/ engine/ util/

Author: hlship
Date: Thu Jul 31 15:01:31 2008
New Revision: 681513

URL: http://svn.apache.org/viewvc?rev=681513&view=rev
Log:
Refactor and simplify concurrency support.

Added:
    tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/util/ResourceLockManager.java
Modified:
    tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/ApplicationServlet.java
    tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/BaseComponentTemplateLoader.java
    tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/binding/ExpressionBinding.java
    tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/AbstractEngine.java
    tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/BaseEngine.java
    tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/DefaultSpecificationSource.java
    tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/DefaultTemplateSource.java
    tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/ExpressionCacheImpl.java
    tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/engine/ExpressionEvaluatorImpl.java

Modified: tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/ApplicationServlet.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/ApplicationServlet.java?rev=681513&r1=681512&r2=681513&view=diff
==============================================================================
--- tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/ApplicationServlet.java (original)
+++ tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/ApplicationServlet.java Thu Jul 31 15:01:31 2008
@@ -14,7 +14,6 @@
 
 package org.apache.tapestry;
 
-import edu.emory.mathcs.backport.java.util.concurrent.locks.ReentrantLock;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.tapestry.engine.BaseEngine;
@@ -39,141 +38,108 @@
 import java.util.Locale;
 
 /**
- *  Links a servlet container with a Tapestry application.  The servlet has some
- *  responsibilities related to bootstrapping the application (in terms of
- *  logging, reading the {@link ApplicationSpecification specification}, etc.).
- *  It is also responsible for creating or locating the {@link IEngine} and delegating
- *  incoming requests to it.
- * 
- *  <p>The servlet init parameter
- *  <code>org.apache.tapestry.specification-path</code>
- *  should be set to the complete resource path (within the classpath)
- *  to the application specification, i.e.,
- *  <code>/com/foo/bar/MyApp.application</code>. 
+ * Links a servlet container with a Tapestry application.  The servlet has some responsibilities related to
+ * bootstrapping the application (in terms of logging, reading the {@link ApplicationSpecification specification},
+ * etc.). It is also responsible for creating or locating the {@link IEngine} and delegating incoming requests to it.
+ * <p/>
+ * <p>The servlet init parameter <code>org.apache.tapestry.specification-path</code> should be set to the complete
+ * resource path (within the classpath) to the application specification, i.e., <code>/com/foo/bar/MyApp.application</code>.
+ * <p/>
+ * <p>In some servlet containers (notably <a href="www.bea.com"/>WebLogic</a>) it is necessary to invoke {@link
+ * HttpSession#setAttribute(String,Object)} in order to force a persistent value to be replicated to the other servers
+ * in the cluster.  Tapestry applications usually only have a single persistent value, the {@link IEngine engine}.  For
+ * persistence to work in such an environment, the JVM system property <code>org.apache.tapestry.store-engine</code>
+ * must be set to <code>true</code>.  This will force the application servlet to restore the engine into the {@link
+ * HttpSession} at the end of each request cycle.
+ * <p/>
+ * <p>As of release 1.0.1, it is no longer necessary for a {@link HttpSession} to be created on the first request cycle.
+ * Instead, the HttpSession is created as needed by the {@link IEngine} ... that is, when a visit object is created, or
+ * when persistent page state is required.  Otherwise, for sessionless requests, an {@link IEngine} from a {@link Pool}
+ * is used.  Additional work must be done so that the {@link IEngine} can change locale <em>without</em> forcing the
+ * creation of a session; this involves the servlet and the engine storing locale information in a {@link Cookie}.
  *
- *  <p>In some servlet containers (notably
- *  <a href="www.bea.com"/>WebLogic</a>)
- *  it is necessary to invoke {@link HttpSession#setAttribute(String,Object)}
- *  in order to force a persistent value to be replicated to the other
- *  servers in the cluster.  Tapestry applications usually only have a single
- *  persistent value, the {@link IEngine engine}.  For persistence to
- *  work in such an environment, the
- *  JVM system property <code>org.apache.tapestry.store-engine</code>
- *  must be set to <code>true</code>.  This will force the application
- *  servlet to restore the engine into the {@link HttpSession} at the
- *  end of each request cycle.
- * 
- *  <p>As of release 1.0.1, it is no longer necessary for a {@link HttpSession}
- *  to be created on the first request cycle.  Instead, the HttpSession is created
- *  as needed by the {@link IEngine} ... that is, when a visit object is created,
- *  or when persistent page state is required.  Otherwise, for sessionless requests,
- *  an {@link IEngine} from a {@link Pool} is used.  Additional work must be done
- *  so that the {@link IEngine} can change locale <em>without</em> forcing 
- *  the creation of a session; this involves the servlet and the engine storing
- *  locale information in a {@link Cookie}.
- * 
- *  @version $Id$
- *  @author Howard Lewis Ship
- * 
- **/
+ * @author Howard Lewis Ship
+ * @version $Id$
+ */
 
 public class ApplicationServlet extends HttpServlet
 {
     private static final Log LOG = LogFactory.getLog(ApplicationServlet.class);
 
-    /** @since 2.3 **/
+    /**
+     * @since 2.3 *
+     */
 
     private static final String APP_SPEC_PATH_PARAM =
-        "org.apache.tapestry.application-specification";
+            "org.apache.tapestry.application-specification";
 
     /**
-     *  Name of the cookie written to the client web browser to
-     *  identify the locale.
-     *
-     **/
+     * Name of the cookie written to the client web browser to identify the locale.
+     */
 
     private static final String LOCALE_COOKIE_NAME = "org.apache.tapestry.locale";
 
     /**
-     *  A {@link Pool} used to store {@link IEngine engine}s that are not currently
-     *  in use.  The key is on {@link Locale}.
-     *
-     **/
+     * A {@link Pool} used to store {@link IEngine engine}s that are not currently in use.  The key is on {@link
+     * Locale}.
+     */
 
     private Pool _enginePool = new Pool();
 
     /**
-     *  The application specification, which is read once and kept in memory
-     *  thereafter.
-     *
-     **/
+     * The application specification, which is read once and kept in memory thereafter.
+     */
 
     private IApplicationSpecification _specification;
 
     /**
-     * The name under which the {@link IEngine engine} is stored within the
-     * {@link HttpSession}.
-     *
-     **/
+     * The name under which the {@link IEngine engine} is stored within the {@link HttpSession}.
+     */
 
     private String _attributeName;
 
     /**
-     *  The resolved class name used to instantiate the engine.
-     * 
-     *  @since 3.0
-     * 
-     **/
+     * The resolved class name used to instantiate the engine.
+     *
+     * @since 3.0
+     */
 
     private String _engineClassName;
 
     /**
-     *  Used to search for configuration properties.
-     * 
-     *  
-     *  @since 3.0
-     * 
-     **/
+     * Used to search for configuration properties.
+     *
+     * @since 3.0
+     */
 
     private IPropertySource _propertySource;
 
     /**
-     * Global lock used to synchronize global thread access to commonly shared
-     * services / data structures.
-     */
-    private ReentrantLock _lock = new ReentrantLock();
-
-    /**
-     *  Invokes {@link #doService(HttpServletRequest, HttpServletResponse)}.
-     *
-     *  @since 1.0.6
+     * Invokes {@link #doService(HttpServletRequest, HttpServletResponse)}.
      *
-     **/
+     * @since 1.0.6
+     */
 
     public void doGet(HttpServletRequest request, HttpServletResponse response)
-        throws IOException, ServletException
+            throws IOException, ServletException
     {
         doService(request, response);
     }
 
     /**
-     *  @since 2.3
-     * 
-     **/
+     * @since 2.3
+     */
 
     private IResourceResolver _resolver;
 
     /**
-     * Handles the GET and POST requests. Performs the following:
-     * <ul>
-     * <li>Construct a {@link RequestContext}
-     * <li>Invoke {@link #getEngine(RequestContext)} to get or create the {@link IEngine}
-     * <li>Invoke {@link IEngine#service(RequestContext)} on the application
-     * </ul>
-     **/
+     * Handles the GET and POST requests. Performs the following: <ul> <li>Construct a {@link RequestContext} <li>Invoke
+     * {@link #getEngine(RequestContext)} to get or create the {@link IEngine} <li>Invoke {@link
+     * IEngine#service(RequestContext)} on the application </ul>
+     */
 
     protected void doService(HttpServletRequest request, HttpServletResponse response)
-        throws IOException, ServletException
+            throws IOException, ServletException
     {
         RequestContext context = null;
 
@@ -189,7 +155,7 @@
 
             if (engine == null)
                 throw new ServletException(
-                    Tapestry.getMessage("ApplicationServlet.could-not-locate-engine"));
+                        Tapestry.getMessage("ApplicationServlet.could-not-locate-engine"));
 
             boolean dirty = engine.service(context);
 
@@ -210,7 +176,7 @@
                 {
 
                     boolean forceStore =
-                        engine.isStateful() && (session.getAttribute(_attributeName) == null);
+                            engine.isStateful() && (session.getAttribute(_attributeName) == null);
 
                     if (forceStore || dirty)
                     {
@@ -239,9 +205,9 @@
             if (engine.isStateful())
             {
                 LOG.error(
-                    Tapestry.format(
-                        "ApplicationServlet.engine-stateful-without-session",
-                        engine));
+                        Tapestry.format(
+                                "ApplicationServlet.engine-stateful-without-session",
+                                engine));
                 return;
             }
 
@@ -285,18 +251,17 @@
     }
 
     /**
-     *  Invoked by {@link #doService(HttpServletRequest, HttpServletResponse)} to create
-     *  the {@link RequestContext} for this request cycle.  Some applications may need to
-     *  replace the default RequestContext with a subclass for particular behavior.
-     * 
-     *  @since 2.3
-     * 
-     **/
+     * Invoked by {@link #doService(HttpServletRequest, HttpServletResponse)} to create the {@link RequestContext} for
+     * this request cycle.  Some applications may need to replace the default RequestContext with a subclass for
+     * particular behavior.
+     *
+     * @since 2.3
+     */
 
     protected RequestContext createRequestContext(
-        HttpServletRequest request,
-        HttpServletResponse response)
-        throws IOException
+            HttpServletRequest request,
+            HttpServletResponse response)
+            throws IOException
     {
         return new RequestContext(this, request, response);
     }
@@ -312,22 +277,18 @@
     }
 
     /**
-     *  Invokes {@link #doService(HttpServletRequest, HttpServletResponse)}.
-     *
-     *
-     **/
+     * Invokes {@link #doService(HttpServletRequest, HttpServletResponse)}.
+     */
 
     public void doPost(HttpServletRequest request, HttpServletResponse response)
-        throws IOException, ServletException
+            throws IOException, ServletException
     {
         doService(request, response);
     }
 
     /**
-     *  Returns the application specification, which is read
-     *  by the {@link #init(ServletConfig)} method.
-     *
-     **/
+     * Returns the application specification, which is read by the {@link #init(ServletConfig)} method.
+     */
 
     public IApplicationSpecification getApplicationSpecification()
     {
@@ -335,25 +296,10 @@
     }
 
     /**
-     * Used internally to synchronize access to creation of shared services.
-     *
-     * @return The global shared lock.
+     * Retrieves the {@link IEngine engine} that will process this request.  This comes from one of the following
+     * places: <ul> <li>The {@link HttpSession}, if the there is one. <li>From the pool of available engines <li>Freshly
+     * created </ul>
      */
-    public ReentrantLock getLock()
-    {
-        return _lock;
-    }
-
-    /**
-     *  Retrieves the {@link IEngine engine} that will process this
-     *  request.  This comes from one of the following places:
-     *  <ul>
-     *  <li>The {@link HttpSession}, if the there is one.
-     *  <li>From the pool of available engines
-     *  <li>Freshly created
-     *  </ul>
-     *
-     **/
 
     protected IEngine getEngine(RequestContext context) throws ServletException
     {
@@ -396,12 +342,9 @@
     }
 
     /**
-     *  Determines the {@link Locale} for the incoming request.
-     *  This is determined from the locale cookie or, if not set,
-     *  from the request itself.  This may return null
-     *  if no locale is determined.
-     *
-     **/
+     * Determines the {@link Locale} for the incoming request. This is determined from the locale cookie or, if not set,
+     * from the request itself.  This may return null if no locale is determined.
+     */
 
     protected Locale getLocaleFromRequest(RequestContext context) throws ServletException
     {
@@ -414,15 +357,13 @@
     }
 
     /**
-     *  Reads the application specification when the servlet is
-     *  first initialized.  All {@link IEngine engine instances}
-     *  will have access to the specification via the servlet.
-     * 
-     *  @see #getApplicationSpecification()
-     *  @see #constructApplicationSpecification()
-     *  @see #createResourceResolver()
+     * Reads the application specification when the servlet is first initialized.  All {@link IEngine engine instances}
+     * will have access to the specification via the servlet.
      *
-     **/
+     * @see #getApplicationSpecification()
+     * @see #constructApplicationSpecification()
+     * @see #createResourceResolver()
+     */
 
     public void init(ServletConfig config) throws ServletException
     {
@@ -436,17 +377,15 @@
     }
 
     /**
-     *  Invoked from {@link #init(ServletConfig)} to create a resource resolver
-     *  for the servlet (which will utlimately be shared and used through the
-     *  application).
-     * 
-     *  <p>This implementation constructs a {@link DefaultResourceResolver}, subclasses
-     *  may provide a different implementation.
-     * 
-     *  @see #getResourceResolver()
-     *  @since 2.3
-     * 
-     **/
+     * Invoked from {@link #init(ServletConfig)} to create a resource resolver for the servlet (which will utlimately be
+     * shared and used through the application).
+     * <p/>
+     * <p>This implementation constructs a {@link DefaultResourceResolver}, subclasses may provide a different
+     * implementation.
+     *
+     * @see #getResourceResolver()
+     * @since 2.3
+     */
 
     protected IResourceResolver createResourceResolver() throws ServletException
     {
@@ -454,24 +393,18 @@
     }
 
     /**
-     *  Invoked from {@link #init(ServletConfig)} to read and construct
-     *  the {@link ApplicationSpecification} for this servlet.
-     *  Invokes {@link #getApplicationSpecificationPath()}, opens
-     *  the resource as a stream, then invokes
-     *  {@link #parseApplicationSpecification(IResourceLocation)}.
-     * 
-     *  <p>
-     *  This method exists to be overriden in
-     *  applications where the application specification cannot be
-     *  loaded from the classpath.  Alternately, a subclass
-     *  could override this method, invoke this implementation,
-     *  and then add additional data to it (for example, an application
-     *  where some of the pages are defined in an external source
-     *  such as a database).
-     *  
-     *  @since 2.2
-     * 
-     **/
+     * Invoked from {@link #init(ServletConfig)} to read and construct the {@link ApplicationSpecification} for this
+     * servlet. Invokes {@link #getApplicationSpecificationPath()}, opens the resource as a stream, then invokes {@link
+     * #parseApplicationSpecification(IResourceLocation)}.
+     * <p/>
+     * <p/>
+     * This method exists to be overriden in applications where the application specification cannot be loaded from the
+     * classpath.  Alternately, a subclass could override this method, invoke this implementation, and then add
+     * additional data to it (for example, an application where some of the pages are defined in an external source such
+     * as a database).
+     *
+     * @since 2.2
+     */
 
     protected IApplicationSpecification constructApplicationSpecification() throws ServletException
     {
@@ -492,24 +425,16 @@
     }
 
     /**
-     *  Gets the location of the application specification, if there is one.
-     *  
-     *  <ul>
-     *  <li>Invokes {@link #getApplicationSpecificationPath()} to get the
-     *  location of the application specification on the classpath.
-     *  <li>If that return null, search for the application specification:
-     *  <ul>
-     *  <li><i>name</i>.application in /WEB-INF/<i>name</i>/
-     *  <li><i>name</i>.application in /WEB-INF/
-     *  </ul>
-     *  </ul>
-     * 
-     *  <p>Returns the location of the application specification, or null
-     *  if not found.
-     * 
-     *  @since 3.0
-     * 
-     **/
+     * Gets the location of the application specification, if there is one.
+     * <p/>
+     * <ul> <li>Invokes {@link #getApplicationSpecificationPath()} to get the location of the application specification
+     * on the classpath. <li>If that return null, search for the application specification: <ul>
+     * <li><i>name</i>.application in /WEB-INF/<i>name</i>/ <li><i>name</i>.application in /WEB-INF/ </ul> </ul>
+     * <p/>
+     * <p>Returns the location of the application specification, or null if not found.
+     *
+     * @since 3.0
+     */
 
     protected IResourceLocation getApplicationSpecificationLocation() throws ServletException
     {
@@ -533,12 +458,10 @@
     }
 
     /**
-     *  Checks for the application specification relative to the specified
-     *  location.
-     * 
-     *  @since 3.0
-     * 
-     **/
+     * Checks for the application specification relative to the specified location.
+     *
+     * @since 3.0
+     */
 
     private IResourceLocation check(IResourceLocation location, String name)
     {
@@ -557,21 +480,19 @@
     }
 
     /**
-     *  Invoked from {@link #constructApplicationSpecification()} when
-     *  the application doesn't have an explicit specification.  A
-     *  simple specification is constructed and returned.  This is useful
-     *  for minimal applications and prototypes.
-     * 
-     *  @since 3.0
-     * 
-     **/
+     * Invoked from {@link #constructApplicationSpecification()} when the application doesn't have an explicit
+     * specification.  A simple specification is constructed and returned.  This is useful for minimal applications and
+     * prototypes.
+     *
+     * @since 3.0
+     */
 
     protected IApplicationSpecification constructStandinSpecification()
     {
         ApplicationSpecification result = new ApplicationSpecification();
 
         IResourceLocation virtualLocation =
-            new ContextResourceLocation(getServletContext(), "/WEB-INF/");
+                new ContextResourceLocation(getServletContext(), "/WEB-INF/");
 
         result.setSpecificationLocation(virtualLocation);
 
@@ -582,16 +503,14 @@
     }
 
     /**
-     *  Invoked from {@link #constructApplicationSpecification()} to
-     *  actually parse the stream (with content provided from the path)
-     *  and convert it into an {@link ApplicationSpecification}.
-     * 
-     *  @since 2.2
-     * 
-     **/
+     * Invoked from {@link #constructApplicationSpecification()} to actually parse the stream (with content provided
+     * from the path) and convert it into an {@link ApplicationSpecification}.
+     *
+     * @since 2.2
+     */
 
     protected IApplicationSpecification parseApplicationSpecification(IResourceLocation location)
-        throws ServletException
+            throws ServletException
     {
         try
         {
@@ -604,15 +523,14 @@
             show(ex);
 
             throw new ServletException(
-                Tapestry.format("ApplicationServlet.could-not-parse-spec", location),
-                ex);
+                    Tapestry.format("ApplicationServlet.could-not-parse-spec", location),
+                    ex);
         }
     }
 
     /**
-     *  Closes the stream, ignoring any exceptions.
-     * 
-     **/
+     * Closes the stream, ignoring any exceptions.
+     */
 
     protected void close(InputStream stream)
     {
@@ -628,18 +546,15 @@
     }
 
     /**
-     *  Reads the servlet init parameter
-     *  <code>org.apache.tapestry.application-specification</code>, which
-     *  is the location, on the classpath, of the application specification.
-     *
-     *  <p>
-     *  If the parameter is not set, this method returns null, and a search
-     *  for the application specification within the servlet context
-     *  will begin.
-     * 
-     *  @see #getApplicationSpecificationLocation()
-     * 
-     **/
+     * Reads the servlet init parameter <code>org.apache.tapestry.application-specification</code>, which is the
+     * location, on the classpath, of the application specification.
+     * <p/>
+     * <p/>
+     * If the parameter is not set, this method returns null, and a search for the application specification within the
+     * servlet context will begin.
+     *
+     * @see #getApplicationSpecificationLocation()
+     */
 
     protected String getApplicationSpecificationPath() throws ServletException
     {
@@ -647,17 +562,13 @@
     }
 
     /**
-     *  Invoked by {@link #getEngine(RequestContext)} to create
-     *  the {@link IEngine} instance specific to the
-     *  application, if not already in the
-     *  {@link HttpSession}.
-     *
-     *  <p>The {@link IEngine} instance returned is stored into the
-     *  {@link HttpSession}.
+     * Invoked by {@link #getEngine(RequestContext)} to create the {@link IEngine} instance specific to the application,
+     * if not already in the {@link HttpSession}.
+     * <p/>
+     * <p>The {@link IEngine} instance returned is stored into the {@link HttpSession}.
      *
-     *  @see #getEngineClassName()    
-     *
-     **/
+     * @see #getEngineClassName()
+     */
 
     protected IEngine createEngine(RequestContext context) throws ServletException
     {
@@ -684,19 +595,12 @@
     }
 
     /**
-     * 
-     *  Returns the name of the class to use when instantiating
-     *  an engine instance for this application.  
-     *  If the application specification
-     *  provides a value, this is returned.  Otherwise, a search for
-     *  the configuration property 
-     *  <code>org.apache.tapestry.engine-class</code>
-     *  occurs (see {@link #searchConfiguration(String)}).
-     * 
-     *  <p>If the search is still unsuccessful, then
-     *  {@link org.apache.tapestry.engine.BaseEngine} is used.
-     * 
-     **/
+     * Returns the name of the class to use when instantiating an engine instance for this application. If the
+     * application specification provides a value, this is returned.  Otherwise, a search for the configuration property
+     * <code>org.apache.tapestry.engine-class</code> occurs (see {@link #searchConfiguration(String)}).
+     * <p/>
+     * <p>If the search is still unsuccessful, then {@link org.apache.tapestry.engine.BaseEngine} is used.
+     */
 
     protected String getEngineClassName()
     {
@@ -715,48 +619,30 @@
     }
 
     /**
-     *  Searches for a configuration property in:
-     *  <ul>
-     *  <li>The servlet's initial parameters
-     *  <li>The servlet context's initial parameters
-     *  <li>JVM system properties
-     *  </ul>
-     * 
-     *  @see #createPropertySource()
-     *  @since 3.0
-     * 
-     **/
+     * Searches for a configuration property in: <ul> <li>The servlet's initial parameters <li>The servlet context's
+     * initial parameters <li>JVM system properties </ul>
+     *
+     * @see #createPropertySource()
+     * @since 3.0
+     */
 
     protected String searchConfiguration(String propertyName)
     {
-        if (_propertySource == null)
+        synchronized (this)
         {
-            _lock.lock();
-
-            try
-            {
-                if (_propertySource == null)
-                {
-                    _propertySource = createPropertySource();
-                }
-            } finally
-            {
-                _lock.unlock();
-            }
+            if (_propertySource == null)
+                _propertySource = createPropertySource();
         }
 
         return _propertySource.getPropertyValue(propertyName);
     }
 
     /**
-     *  Creates an instance of {@link IPropertySource} used for
-     *  searching of configuration values.  Subclasses may override
-     *  this method if they want to change the normal locations
-     *  that properties are searched for within.
-     * 
-     *  @since 3.0
-     * 
-     **/
+     * Creates an instance of {@link IPropertySource} used for searching of configuration values.  Subclasses may
+     * override this method if they want to change the normal locations that properties are searched for within.
+     *
+     * @since 3.0
+     */
 
     protected IPropertySource createPropertySource()
     {
@@ -770,16 +656,14 @@
     }
 
     /**
-     *  Invoked from the {@link IEngine engine}, just prior to starting to
-     *  render a response, when the locale has changed.  The servlet writes a
-     *  {@link Cookie} so that, on subsequent request cycles, an engine localized
-     *  to the selected locale is chosen.
+     * Invoked from the {@link IEngine engine}, just prior to starting to render a response, when the locale has
+     * changed.  The servlet writes a {@link Cookie} so that, on subsequent request cycles, an engine localized to the
+     * selected locale is chosen.
+     * <p/>
+     * <p>At this time, the cookie is <em>not</em> persistent.  That may change in subsequent releases.
      *
-     *  <p>At this time, the cookie is <em>not</em> persistent.  That may
-     *  change in subsequent releases.
-     *
-     *  @since 1.0.1
-     **/
+     * @since 1.0.1
+     */
 
     public void writeLocaleCookie(Locale locale, IEngine engine, RequestContext cycle)
     {
@@ -793,14 +677,11 @@
     }
 
     /**
-     *  Returns a resource resolver that can access classes and resources related
-     *  to the current web application context.  Relies on
-     *  {@link java.lang.Thread#getContextClassLoader()}, which is set by
-     *  most modern servlet containers.
-     * 
-     *  @since 2.3
+     * Returns a resource resolver that can access classes and resources related to the current web application context.
+     * Relies on {@link java.lang.Thread#getContextClassLoader()}, which is set by most modern servlet containers.
      *
-     **/
+     * @since 2.3
+     */
 
     public IResourceResolver getResourceResolver()
     {
@@ -809,6 +690,7 @@
 
     /**
      * Ensures that shared janitor thread is terminated.
+     *
      * @see javax.servlet.Servlet#destroy()
      * @since 3.0.3
      */

Modified: tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/BaseComponentTemplateLoader.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/BaseComponentTemplateLoader.java?rev=681513&r1=681512&r2=681513&view=diff
==============================================================================
--- tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/BaseComponentTemplateLoader.java (original)
+++ tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/BaseComponentTemplateLoader.java Thu Jul 31 15:01:31 2008
@@ -20,7 +20,6 @@
 import org.apache.tapestry.binding.ExpressionBinding;
 import org.apache.tapestry.binding.StaticBinding;
 import org.apache.tapestry.binding.StringBinding;
-import org.apache.tapestry.engine.ExpressionEvaluator;
 import org.apache.tapestry.engine.IPageLoader;
 import org.apache.tapestry.engine.IPageSource;
 import org.apache.tapestry.engine.ITemplateSource;
@@ -34,16 +33,14 @@
 import java.util.Set;
 
 /**
- *  Utility class instantiated by {@link org.apache.tapestry.BaseComponent} to
- *  process the component's {@link org.apache.tapestry.parse.ComponentTemplate template},
- *  which involves working through the nested structure of the template and hooking
- *  the various static template blocks and components together using
- *  {@link IComponent#addBody(IRender)} and 
- *  {@link org.apache.tapestry.BaseComponent#addOuter(IRender)}.
+ * Utility class instantiated by {@link org.apache.tapestry.BaseComponent} to process the component's {@link
+ * org.apache.tapestry.parse.ComponentTemplate template}, which involves working through the nested structure of the
+ * template and hooking the various static template blocks and components together using {@link
+ * IComponent#addBody(IRender)} and {@link org.apache.tapestry.BaseComponent#addOuter(IRender)}.
  *
- *  @author Howard Lewis Ship
- *  @version $Id$
- *  @since 3.0
+ * @author Howard Lewis Ship
+ * @version $Id$
+ * @since 3.0
  */
 
 public class BaseComponentTemplateLoader
@@ -58,12 +55,10 @@
     private IComponent[] _stack;
     private int _stackx = 0;
     private IComponent _activeComponent = null;
-    private ExpressionEvaluator _evaluator;
     private Set _seenIds = new HashSet();
 
     /**
-     *  A class used with invisible localizations.  Constructed
-     *  from a {@link TextToken}.
+     * A class used with invisible localizations.  Constructed from a {@link TextToken}.
      */
 
     private static class LocalizedStringRender implements IRender
@@ -130,11 +125,11 @@
     }
 
     public BaseComponentTemplateLoader(
-        IRequestCycle requestCycle,
-        IPageLoader pageLoader,
-        BaseComponent loadComponent,
-        ComponentTemplate template,
-        IPageSource pageSource)
+            IRequestCycle requestCycle,
+            IPageLoader pageLoader,
+            BaseComponent loadComponent,
+            ComponentTemplate template,
+            IPageSource pageSource)
     {
         _requestCycle = requestCycle;
         _pageLoader = pageLoader;
@@ -185,21 +180,20 @@
 
         if (_stackx != 0)
             throw new ApplicationRuntimeException(
-                Tapestry.getMessage("BaseComponent.unbalance-open-tags"),
-                _loadComponent,
-                null,
-                null);
+                    Tapestry.getMessage("BaseComponent.unbalance-open-tags"),
+                    _loadComponent,
+                    null,
+                    null);
 
         checkAllComponentsReferenced();
     }
 
     /**
-     *  Adds the token (which implements {@link IRender})
-     *  to the active component (using {@link IComponent#addBody(IRender)}),
-     *  or to this component {@link BaseComponent#addOuter(IRender)}.
-     * 
-     *  <p>
-     *  A check is made that the active component allows a body.
+     * Adds the token (which implements {@link IRender}) to the active component (using {@link
+     * IComponent#addBody(IRender)}), or to this component {@link BaseComponent#addOuter(IRender)}.
+     * <p/>
+     * <p/>
+     * A check is made that the active component allows a body.
      */
 
     private void process(TextToken token)
@@ -235,13 +229,13 @@
 
         if (_seenIds.contains(id))
             throw new ApplicationRuntimeException(
-                Tapestry.format(
-                    "BaseComponent.multiple-component-references",
-                    _loadComponent.getExtendedId(),
-                    id),
-                _loadComponent,
-                token.getLocation(),
-                null);
+                    Tapestry.format(
+                            "BaseComponent.multiple-component-references",
+                            _loadComponent.getExtendedId(),
+                            id),
+                    _loadComponent,
+                    token.getLocation(),
+                    null);
 
         _seenIds.add(id);
 
@@ -274,25 +268,25 @@
 
         if (cc != null)
             throw new ApplicationRuntimeException(
-                Tapestry.format(
-                    "BaseComponentTemplateLoader.dupe-component-id",
-                    id,
+                    Tapestry.format(
+                            "BaseComponentTemplateLoader.dupe-component-id",
+                            id,
+                            location,
+                            cc.getLocation()),
+                    _loadComponent,
                     location,
-                    cc.getLocation()),
-                _loadComponent,
-                location,
-                null);
+                    null);
     }
 
     private IComponent createImplicitComponent(String id, String componentType, ILocation location)
     {
         IComponent result =
-            _pageLoader.createImplicitComponent(
-                _requestCycle,
-                _loadComponent,
-                id,
-                componentType,
-                location);
+                _pageLoader.createImplicitComponent(
+                        _requestCycle,
+                        _loadComponent,
+                        id,
+                        componentType,
+                        location);
 
         return result;
     }
@@ -309,10 +303,10 @@
 
         if (_stackx <= 0)
             throw new ApplicationRuntimeException(
-                Tapestry.getMessage("BaseComponent.unbalanced-close-tags"),
-                _loadComponent,
-                token.getLocation(),
-                null);
+                    Tapestry.getMessage("BaseComponent.unbalanced-close-tags"),
+                    _loadComponent,
+                    token.getLocation(),
+                    null);
 
         // Null and forget the top element on the stack.
 
@@ -332,7 +326,7 @@
     }
 
     /**
-     *  Adds bindings based on attributes in the template.
+     * Adds bindings based on attributes in the template.
      */
 
     private void addTemplateBindings(IComponent component, OpenToken token)
@@ -356,32 +350,32 @@
                 if (type == AttributeType.OGNL_EXPRESSION)
                 {
                     addExpressionBinding(
-                        component,
-                        spec,
-                        name,
-                        attribute.getValue(),
-                        token.getLocation());
+                            component,
+                            spec,
+                            name,
+                            attribute.getValue(),
+                            token.getLocation());
                     continue;
                 }
 
                 if (type == AttributeType.LOCALIZATION_KEY)
                 {
                     addStringBinding(
-                        component,
-                        spec,
-                        name,
-                        attribute.getValue(),
-                        token.getLocation());
+                            component,
+                            spec,
+                            name,
+                            attribute.getValue(),
+                            token.getLocation());
                     continue;
                 }
 
                 if (type == AttributeType.LITERAL)
                     addStaticBinding(
-                        component,
-                        spec,
-                        name,
-                        attribute.getValue(),
-                        token.getLocation());
+                            component,
+                            spec,
+                            name,
+                            attribute.getValue(),
+                            token.getLocation());
             }
         }
 
@@ -389,33 +383,30 @@
         // there is no established binding for that parameter, 
         // add a static binding carrying the template tag  
         if (spec.getParameter(ITemplateSource.TEMPLATE_TAG_PARAMETER_NAME) != null
-            && component.getBinding(ITemplateSource.TEMPLATE_TAG_PARAMETER_NAME) == null)
+                && component.getBinding(ITemplateSource.TEMPLATE_TAG_PARAMETER_NAME) == null)
         {
             addStaticBinding(
-                component,
-                spec,
-                ITemplateSource.TEMPLATE_TAG_PARAMETER_NAME,
-                token.getTag(),
-                token.getLocation());
+                    component,
+                    spec,
+                    ITemplateSource.TEMPLATE_TAG_PARAMETER_NAME,
+                    token.getTag(),
+                    token.getLocation());
         }
 
     }
 
     /**
-     *  Adds an expression binding, checking for errors related
-     *  to reserved and informal parameters.
-     *
-     *  <p>It is an error to specify expression 
-     *  bindings in both the specification
-     *  and the template.
+     * Adds an expression binding, checking for errors related to reserved and informal parameters.
+     * <p/>
+     * <p>It is an error to specify expression bindings in both the specification and the template.
      */
 
     private void addExpressionBinding(
-        IComponent component,
-        IComponentSpecification spec,
-        String name,
-        String expression,
-        ILocation location)
+            IComponent component,
+            IComponentSpecification spec,
+            String name,
+            String expression,
+            ILocation location)
     {
 
         // If matches a formal parameter name, allow it to be set
@@ -427,68 +418,65 @@
         {
             if (component.getBinding(name) != null)
                 throw new ApplicationRuntimeException(
-                    Tapestry.format(
-                        "BaseComponent.dupe-template-expression",
-                        name,
-                        component.getExtendedId(),
-                        _loadComponent.getExtendedId()),
-                    component,
-                    location,
-                    null);
+                        Tapestry.format(
+                                "BaseComponent.dupe-template-expression",
+                                name,
+                                component.getExtendedId(),
+                                _loadComponent.getExtendedId()),
+                        component,
+                        location,
+                        null);
         }
         else
         {
             if (!spec.getAllowInformalParameters())
                 throw new ApplicationRuntimeException(
-                    Tapestry.format(
-                        "BaseComponent.template-expression-for-informal-parameter",
-                        name,
-                        component.getExtendedId(),
-                        _loadComponent.getExtendedId()),
-                    component,
-                    location,
-                    null);
+                        Tapestry.format(
+                                "BaseComponent.template-expression-for-informal-parameter",
+                                name,
+                                component.getExtendedId(),
+                                _loadComponent.getExtendedId()),
+                        component,
+                        location,
+                        null);
 
             // If the name is reserved (matches a formal parameter
             // or reserved name, caselessly), then skip it.
 
             if (spec.isReservedParameterName(name))
                 throw new ApplicationRuntimeException(
-                    Tapestry.format(
-                        "BaseComponent.template-expression-for-reserved-parameter",
-                        name,
-                        component.getExtendedId(),
-                        _loadComponent.getExtendedId()),
-                    component,
-                    location,
-                    null);
+                        Tapestry.format(
+                                "BaseComponent.template-expression-for-reserved-parameter",
+                                name,
+                                component.getExtendedId(),
+                                _loadComponent.getExtendedId()),
+                        component,
+                        location,
+                        null);
         }
 
         IBinding binding =
-            new ExpressionBinding(
-                _pageSource.getResourceResolver(),
-                _loadComponent,
-                expression,
-                location);
+                new ExpressionBinding(
+                        _pageSource.getResourceResolver(),
+                        _loadComponent,
+                        expression,
+                        location);
 
         component.setBinding(name, binding);
     }
 
     /**
-      *  Adds an expression binding, checking for errors related
-      *  to reserved and informal parameters.
-      *
-      *  <p>It is an error to specify expression 
-      *  bindings in both the specification
-      *  and the template.
-      */
+     * Adds an expression binding, checking for errors related to reserved and informal parameters.
+     * <p/>
+     * <p>It is an error to specify expression bindings in both the specification and the template.
+     */
 
     private void addStringBinding(
-        IComponent component,
-        IComponentSpecification spec,
-        String name,
-        String localizationKey,
-        ILocation location)
+            IComponent component,
+            IComponentSpecification spec,
+            String name,
+            String localizationKey,
+            ILocation location)
     {
         // If matches a formal parameter name, allow it to be set
         // unless there's already a binding.
@@ -499,41 +487,41 @@
         {
             if (component.getBinding(name) != null)
                 throw new ApplicationRuntimeException(
-                    Tapestry.format(
-                        "BaseComponent.dupe-string",
-                        name,
-                        component.getExtendedId(),
-                        _loadComponent.getExtendedId()),
-                    component,
-                    location,
-                    null);
+                        Tapestry.format(
+                                "BaseComponent.dupe-string",
+                                name,
+                                component.getExtendedId(),
+                                _loadComponent.getExtendedId()),
+                        component,
+                        location,
+                        null);
         }
         else
         {
             if (!spec.getAllowInformalParameters())
                 throw new ApplicationRuntimeException(
-                    Tapestry.format(
-                        "BaseComponent.template-expression-for-informal-parameter",
-                        name,
-                        component.getExtendedId(),
-                        _loadComponent.getExtendedId()),
-                    component,
-                    location,
-                    null);
+                        Tapestry.format(
+                                "BaseComponent.template-expression-for-informal-parameter",
+                                name,
+                                component.getExtendedId(),
+                                _loadComponent.getExtendedId()),
+                        component,
+                        location,
+                        null);
 
             // If the name is reserved (matches a formal parameter
             // or reserved name, caselessly), then skip it.
 
             if (spec.isReservedParameterName(name))
                 throw new ApplicationRuntimeException(
-                    Tapestry.format(
-                        "BaseComponent.template-expression-for-reserved-parameter",
-                        name,
-                        component.getExtendedId(),
-                        _loadComponent.getExtendedId()),
-                    component,
-                    location,
-                    null);
+                        Tapestry.format(
+                                "BaseComponent.template-expression-for-reserved-parameter",
+                                name,
+                                component.getExtendedId(),
+                                _loadComponent.getExtendedId()),
+                        component,
+                        location,
+                        null);
         }
 
         IBinding binding = new StringBinding(_loadComponent, localizationKey, location);
@@ -542,20 +530,18 @@
     }
 
     /**
-     *  Adds a static binding, checking for errors related
-     *  to reserved and informal parameters.
-     * 
-     *  <p>
-     *  Static bindings that conflict with bindings in the
-     *  specification are quietly ignored.
+     * Adds a static binding, checking for errors related to reserved and informal parameters.
+     * <p/>
+     * <p/>
+     * Static bindings that conflict with bindings in the specification are quietly ignored.
      */
 
     private void addStaticBinding(
-        IComponent component,
-        IComponentSpecification spec,
-        String name,
-        String staticValue,
-        ILocation location)
+            IComponent component,
+            IComponentSpecification spec,
+            String name,
+            String staticValue,
+            ILocation location)
     {
 
         if (component.getBinding(name) != null)
@@ -609,12 +595,12 @@
         int count = ids.size();
 
         String key =
-            (count == 1)
+                (count == 1)
                 ? "BaseComponent.missing-component-spec-single"
                 : "BaseComponent.missing-component-spec-multi";
 
         StringBuffer buffer =
-            new StringBuffer(Tapestry.format(key, _loadComponent.getExtendedId()));
+                new StringBuffer(Tapestry.format(key, _loadComponent.getExtendedId()));
 
         Iterator i = ids.iterator();
         int j = 1;
@@ -623,15 +609,14 @@
         {
             if (j == 1)
                 buffer.append(' ');
+            else if (j == count)
+            {
+                buffer.append(' ');
+                buffer.append(Tapestry.getMessage("BaseComponent.and"));
+                buffer.append(' ');
+            }
             else
-                if (j == count)
-                {
-                    buffer.append(' ');
-                    buffer.append(Tapestry.getMessage("BaseComponent.and"));
-                    buffer.append(' ');
-                }
-                else
-                    buffer.append(", ");
+                buffer.append(", ");
 
             buffer.append(i.next());
 
@@ -646,9 +631,9 @@
     protected ApplicationRuntimeException createBodylessComponentException(IComponent component)
     {
         return new ApplicationRuntimeException(
-            Tapestry.getMessage("BaseComponentTemplateLoader.bodyless-component"),
-            component,
-            null,
-            null);
+                Tapestry.getMessage("BaseComponentTemplateLoader.bodyless-component"),
+                component,
+                null,
+                null);
     }
 }

Modified: tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/binding/ExpressionBinding.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/binding/ExpressionBinding.java?rev=681513&r1=681512&r2=681513&view=diff
==============================================================================
--- tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/binding/ExpressionBinding.java (original)
+++ tapestry/tapestry3/trunk/tapestry-framework/src/org/apache/tapestry/binding/ExpressionBinding.java Thu Jul 31 15:01:31 2008
@@ -29,118 +29,87 @@
 import java.util.Map;
 
 /**
- *  Implements a dynamic binding, based on getting and fetching
- *  values using JavaBeans property access.  This is built
- *  upon the <a href="http://www.ognl.org">OGNL</a> library.
+ * Implements a dynamic binding, based on getting and fetching values using JavaBeans property access.  This is built
+ * upon the <a href="http://www.ognl.org">OGNL</a> library.
+ * <p/>
+ * <p><b>Optimization of the Expression</b>
+ * <p/>
+ * <p>There's a lot of room for optimization here because we can count on some portions of the expression to be
+ * effectively static.  Note that we type the root object as {@link IComponent}.  We have some expectations that certain
+ * properties of the root (and properties reachable from the root) will be constant for the lifetime of the binding. For
+ * example, components never change thier page or container.  This means that certain property prefixes can be
+ * optimized:
+ * <p/>
+ * <ul> <li>page <li>container <li>components.<i>name</i> </ul>
+ * <p/>
+ * <p>This means that once an ExpressionBinding has been triggered, the {@link #toString()} method may return different
+ * values for the root component and the expression than was originally set.
+ * <p/>
+ * <p><b>Identifying Invariants</b>
+ * <p/>
+ * <p>Most expressions are fully dynamic; they must be resolved each time they are accessed.  This can be somewhat
+ * inefficient. Tapestry can identify certain paths as invariant:
+ * <p/>
+ * <ul> <li>A component within the page hierarchy <li>An {@link org.apache.tapestry.IAsset} from then assets map
+ * (property <code>assets</code>) <li>A {@link org.apache.tapestry.IActionListener} from the listener map (property
+ * <code>listeners</code>) <li>A bean with a {@link org.apache.tapestry.spec.BeanLifecycle#PAGE} lifecycle (property
+ * <code>beans</code>) <li>A binding (property <code>bindings</code>) </ul>
+ * <p/>
+ * <p/>
+ * These optimizations have some inherent dangers; they assume that the components have not overidden the specified
+ * properties; the last one (concerning helper beans) assumes that the component does inherit from {@link
+ * org.apache.tapestry.AbstractComponent}. If this becomes a problem in the future, it may be necessary to have the
+ * component itself involved in these determinations.
  *
- *  <p><b>Optimization of the Expression</b>
- *
- *  <p>There's a lot of room for optimization here because we can
- *  count on some portions of the expression to be
- *  effectively static.  Note that we type the root object as
- *  {@link IComponent}.  We have some expectations that
- *  certain properties of the root (and properties reachable from the root)
- *  will be constant for the lifetime of the binding.  For example, 
- *  components never change thier page or container.  This means
- *  that certain property prefixes can be optimized:
- *
- *  <ul>
- *  <li>page
- *  <li>container
- *  <li>components.<i>name</i>
- *  </ul>
- *
- *  <p>This means that once an ExpressionBinding has been triggered, 
- *  the {@link #toString()} method may return different values for the root
- *  component and the expression than was originally set.
- *
- *  <p><b>Identifying Invariants</b>
- *
- *  <p>Most expressions are fully dynamic; they must be
- *  resolved each time they are accessed.  This can be somewhat inefficient.
- *  Tapestry can identify certain paths as invariant:
- *
- *  <ul>
- *  <li>A component within the page hierarchy 
- *  <li>An {@link org.apache.tapestry.IAsset} from then assets map (property <code>assets</code>)
- *  <li>A {@link org.apache.tapestry.IActionListener}
- *  from the listener map (property <code>listeners</code>)
- *  <li>A bean with a {@link org.apache.tapestry.spec.BeanLifecycle#PAGE}
- *  lifecycle (property <code>beans</code>)
- *  <li>A binding (property <code>bindings</code>)
- *  </ul>
- *
- *  <p>
- *  These optimizations have some inherent dangers; they assume that
- *  the components have not overidden the specified properties;
- *  the last one (concerning helper beans) assumes that the
- *  component does inherit from {@link org.apache.tapestry.AbstractComponent}.
- *  If this becomes a problem in the future, it may be necessary to
- *  have the component itself involved in these determinations.
- *
- *  @author Howard Lewis Ship
- *  @version $Id$
- *  @since 2.2
- *
- **/
+ * @author Howard Lewis Ship
+ * @version $Id$
+ * @since 2.2
+ */
 
 public class ExpressionBinding extends AbstractBinding
 {
     /**
-     *  The root object against which the nested property name is evaluated.
-     *
-     **/
+     * The root object against which the nested property name is evaluated.
+     */
 
     private IComponent _root;
 
     /**
-     *  The OGNL expression, as a string.
-     *
-     **/
+     * The OGNL expression, as a string.
+     */
 
     private String _expression;
 
     /**
-     *  If true, then the binding is invariant, and cachedValue
-     *  is the ultimate value.
-     *
-     **/
+     * If true, then the binding is invariant, and cachedValue is the ultimate value.
+     */
 
     private boolean _invariant = false;
 
     /**
-     *  Stores the cached value for the binding, if invariant
-     *  is true.
-     *
-     **/
+     * Stores the cached value for the binding, if invariant is true.
+     */
 
     private Object _cachedValue;
 
     /**
-     *   Parsed OGNL expression.
-     *
-     **/
+     * Parsed OGNL expression.
+     */
 
     private Node _parsedExpression;
 
     /**
-     *  Flag set once the binding has initialized.
-     *  _cachedValue, _invariant and _final value
-     *  for _expression
-     *  are not valid until after initialization.
-     *
-     *
-     **/
+     * Flag set once the binding has initialized. _cachedValue, _invariant and _final value for _expression are not
+     * valid until after initialization.
+     */
 
     private boolean _initialized;
 
     private IResourceResolver _resolver;
 
     /**
-     *  The OGNL context for this binding.  It is retained
-     *  for the lifespan of the binding once created.
-     *
-     **/
+     * The OGNL context for this binding.  It is retained for the lifespan of the binding once created.
+     */
 
     private Map _context;
 
@@ -156,17 +125,19 @@
     private ExpressionAccessor _accessor;
 
     /**
-     * Used to detect previous failed attempts at writing values when compiling expressions so
-     * that as many expressions as possible can be fully compiled into their java byte form when
-     * all objects in the expression are available.
+     * Used to detect previous failed attempts at writing values when compiling expressions so that as many expressions
+     * as possible can be fully compiled into their java byte form when all objects in the expression are available.
      */
     private boolean _writeFailed;
 
     /**
-     *  Creates a {@link ExpressionBinding} from the root object
-     *  and an OGNL expression.
-     *
-     **/
+     * This is current set to false, to prevent any attempts at bytecode compilation of OGNL expressions.
+     */
+    private static final boolean EXPRESSION_EVALUATION_ENABLED = false;
+
+    /**
+     * Creates a {@link ExpressionBinding} from the root object and an OGNL expression.
+     */
 
     public ExpressionBinding(
             IResourceResolver resolver,
@@ -193,12 +164,10 @@
     }
 
     /**
-     *  Gets the value of the property path, with the assistance of a 
-     *  OGNL.
-     *
-     *  @throws BindingException if an exception is thrown accessing the property.
+     * Gets the value of the property path, with the assistance of a OGNL.
      *
-     **/
+     * @throws BindingException if an exception is thrown accessing the property.
+     */
 
     public Object getObject()
     {
@@ -214,7 +183,7 @@
     {
         try
         {
-            if (false && _evaluator.isCompileEnabled() && _accessor == null && !_writeFailed)
+            if (EXPRESSION_EVALUATION_ENABLED && _evaluator.isCompileEnabled() && _accessor == null && !_writeFailed)
             {
                 _evaluator.compileExpression(_root, _parsedExpression, _expression);
                 _accessor = _parsedExpression.getAccessor();
@@ -238,14 +207,10 @@
     }
 
     /**
-     *  Creates an OGNL context used to get or set a value.
-     *  We may extend this in the future to set additional
-     *  context variables (such as page, request cycle and engine).
-     *  An optional type converter will be added to the OGNL context
-     *  if it is specified as an application extension with the name
-     *  {@link Tapestry#OGNL_TYPE_CONVERTER}.
-     *
-     **/
+     * Creates an OGNL context used to get or set a value. We may extend this in the future to set additional context
+     * variables (such as page, request cycle and engine). An optional type converter will be added to the OGNL context
+     * if it is specified as an application extension with the name {@link Tapestry#OGNL_TYPE_CONVERTER}.
+     */
 
     private Map getOgnlContext()
     {
@@ -273,11 +238,8 @@
     }
 
     /**
-     *  Returns true if the binding is expected to always 
-     *  return the same value.
-     *
-     *
-     **/
+     * Returns true if the binding is expected to always return the same value.
+     */
 
     public boolean isInvariant()
     {
@@ -307,10 +269,8 @@
     }
 
     /**
-     *  Sets up the helper object, but also optimizes the property path
-     *  and determines if the binding is invarant.
-     *
-     **/
+     * Sets up the helper object, but also optimizes the property path and determines if the binding is invarant.
+     */
 
     private void initialize()
     {
@@ -341,7 +301,6 @@
             throw new BindingException(ex.getMessage(), this, ex);
         }
 
-
         // Split the expression into individual property names.
         // We then optimize what we can from the expression.  This will
         // shorten the expression and, in some cases, eliminate
@@ -375,13 +334,11 @@
     }
 
     /**
-     *  Looks for common prefixes on the expression (provided pre-split) that
-     *  are recognized as references to other components.
+     * Looks for common prefixes on the expression (provided pre-split) that are recognized as references to other
+     * components.
      *
-     *  @return the number of leading elements of the split expression that
-     *  have been removed.
-     *
-     **/
+     * @return the number of leading elements of the split expression that have been removed.
+     */
 
     private int optimizeRootObject(String[] split)
     {
@@ -437,10 +394,8 @@
     }
 
     /**
-     *  Reassembles the remainder of the split property path
-     *  from the start point.
-     *
-     **/
+     * Reassembles the remainder of the split property path from the start point.
+     */
 
     private String reassemble(int start, String[] split)
     {
@@ -466,9 +421,8 @@
     }
 
     /**
-     *  Checks to see if the binding can be converted to an invariant.
-     *
-     **/
+     * Checks to see if the binding can be converted to an invariant.
+     */
 
     private void checkForInvariant(int start, String[] split)
     {
@@ -547,12 +501,12 @@
     }
 
     /**
-     *  Updates the property for the binding to the given value.  
+     * Updates the property for the binding to the given value.
      *
-     *  @throws BindingException if the property can't be updated (typically
-     *  due to an security problem, or a missing mutator method).
-     *  @throws BindingException if the binding is invariant.
-     **/
+     * @throws BindingException if the property can't be updated (typically due to an security problem, or a missing
+     *                          mutator method).
+     * @throws BindingException if the binding is invariant.
+     */
 
     public void setObject(Object value)
     {
@@ -566,7 +520,8 @@
             if (_accessor != null)
             {
                 _evaluator.write(_root, _accessor, value);
-            } else if (false && _evaluator.isCompileEnabled() && _accessor == null)
+            }
+            else if (EXPRESSION_EVALUATION_ENABLED && _evaluator.isCompileEnabled() && _accessor == null)
             {
                 //_evaluator.compileExpression(_root, _parsedExpression, _expression);
                 //_accessor = _parsedExpression.getAccessor();
@@ -578,7 +533,8 @@
                     {
                         _evaluator.compileExpression(_root, _parsedExpression, _expression);
                         _accessor = _parsedExpression.getAccessor();
-                    } catch (Throwable t)
+                    }
+                    catch (Throwable t)
                     {
                         // ignore re-read failures as they aren't supposed to be happening now anyways
                         // and a more user friendly version will be available if someone actually calls
@@ -589,7 +545,8 @@
                             _writeFailed = true;
                     }
                 }
-            } else
+            }
+            else
             {
                 _evaluator.writeCompiled(_root, _parsedExpression, value);
             }
@@ -608,12 +565,10 @@
     }
 
     /**
-     *  Returns the a String representing the property path.  This includes
-     *  the {@link IComponent#getExtendedId() extended id} of the root component
-     *  and the property path ... once the binding is used, these may change
-     *  due to optimization of the property path.
-     *
-     **/
+     * Returns the a String representing the property path.  This includes the {@link IComponent#getExtendedId()
+     * extended id} of the root component and the property path ... once the binding is used, these may change due to
+     * optimization of the property path.
+     */
 
     public String toString()
     {