You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ba...@apache.org on 2010/07/06 08:52:21 UTC

svn commit: r960817 - in /tomcat/taglibs/standard/trunk/impl/src/main/java/org/apache/taglibs/standard: resources/Resources.properties tag/common/core/SetSupport.java

Author: bayard
Date: Tue Jul  6 06:52:21 2010
New Revision: 960817

URL: http://svn.apache.org/viewvc?rev=960817&view=rev
Log:
Applying Jeremy Boynes' refactoring of SetSupport per #49548

"Patch primarily refactors the large "if" clause that sets
the result into smaller methods. It also clears up IDE warnings (at least from
IDEA).

One side effect was I18N of the exception message for "SET_BAD_DEFERRED_SCOPE"

I18N also needed access to the original value (which was not being retained)
and in conjunction with that I replaced the "scopeSpecified" indicator with a
check on whether the "scope" attribute is null.

I also changed the comment where the exception is thrown when target is null.
This can happen as target is a request-time attribute and the spec does define
this behaviour.

When setting a bean property, the for loop now exits after setting its property
rather than continuing to iterate through all of them. This should have no
impact unless a bean can have two PropertyDescriptors with the same name and I
don't believe it can.

Finally, all throws of JspException were replaced with JspTagException as these
are errors coming from a Tag Handler rather than the JSP Engine itself."


Modified:
    tomcat/taglibs/standard/trunk/impl/src/main/java/org/apache/taglibs/standard/resources/Resources.properties
    tomcat/taglibs/standard/trunk/impl/src/main/java/org/apache/taglibs/standard/tag/common/core/SetSupport.java

Modified: tomcat/taglibs/standard/trunk/impl/src/main/java/org/apache/taglibs/standard/resources/Resources.properties
URL: http://svn.apache.org/viewvc/tomcat/taglibs/standard/trunk/impl/src/main/java/org/apache/taglibs/standard/resources/Resources.properties?rev=960817&r1=960816&r2=960817&view=diff
==============================================================================
--- tomcat/taglibs/standard/trunk/impl/src/main/java/org/apache/taglibs/standard/resources/Resources.properties (original)
+++ tomcat/taglibs/standard/trunk/impl/src/main/java/org/apache/taglibs/standard/resources/Resources.properties Tue Jul  6 06:52:21 2010
@@ -77,6 +77,9 @@ SET_INVALID_PROPERTY=\
 SET_INVALID_TARGET=\
     Attempt to set the property of an invalid object in <set>.
 
+SET_BAD_DEFERRED_SCOPE=\
+    Invalid scope for ValueExpression in <set>. "page" scope is required but was "{0}"
+
 SET_NO_VALUE=\
     Need either non-whitespace body or "value" attribute in <set>
 

Modified: tomcat/taglibs/standard/trunk/impl/src/main/java/org/apache/taglibs/standard/tag/common/core/SetSupport.java
URL: http://svn.apache.org/viewvc/tomcat/taglibs/standard/trunk/impl/src/main/java/org/apache/taglibs/standard/tag/common/core/SetSupport.java?rev=960817&r1=960816&r2=960817&view=diff
==============================================================================
--- tomcat/taglibs/standard/trunk/impl/src/main/java/org/apache/taglibs/standard/tag/common/core/SetSupport.java (original)
+++ tomcat/taglibs/standard/trunk/impl/src/main/java/org/apache/taglibs/standard/tag/common/core/SetSupport.java Tue Jul  6 06:52:21 2010
@@ -61,8 +61,7 @@ public class SetSupport extends BodyTagS
     protected Object target;            // tag attribute
     protected String property;          // tag attribute
     private String var;                 // tag attribute
-    private int scope;                  // tag attribute
-    private boolean scopeSpecified;     // status
+    private String scope;               // tag attribute
 
     //*********************************************************************
     // Construction and initialization
@@ -79,9 +78,8 @@ public class SetSupport extends BodyTagS
 
     // resets local state
     private void init() {
-        value = target = property = var = null;
-        scopeSpecified = valueSpecified = false;
-        scope = PageContext.PAGE_SCOPE;
+        value = target = property = var = scope = null;
+        valueSpecified = false;
     }
 
     // Releases any resources we may have (or inherit)
@@ -101,92 +99,19 @@ public class SetSupport extends BodyTagS
 
         // decide what to do with the result
         if (var != null) {
-
-            /*
-             * Store the result, letting an IllegalArgumentException
-             * propagate back if the scope is invalid (e.g., if an attempt
-             * is made to store something in the session without any
-             * HttpSession existing).
-             */
-            ELContext myELContext = pageContext.getELContext();
-            VariableMapper vm = myELContext.getVariableMapper();
-            if (result != null) {
-                //check for instanceof valueExpression
-                if (result instanceof ValueExpression) {
-                    if (scope!=PageContext.PAGE_SCOPE) {
-                        throw new JspException("Incorrect scope for ValueExpression.  PageScope is required.");
-                    }
-                    //set variable in var Mapper
-                    vm.setVariable(var, (ValueExpression)result);
-                } else {
-                    // make sure to remove it from the VariableMapper if we will be setting into page scope 
-                    if (scope == PageContext.PAGE_SCOPE && vm.resolveVariable(var) != null) {
-                        vm.setVariable(var, null);
-                    }
-                    pageContext.setAttribute(var, result, scope);
-                }
-            } else {
-                //make sure to remove it from the Var mapper
-                if (vm.resolveVariable(var)!=null) {
-                    vm.setVariable(var, null);
-                }
-                if (scopeSpecified)
-                    pageContext.removeAttribute(var, scope);
-                else
-                    pageContext.removeAttribute(var);
-            }
-
-        } else if (target != null) {
-
-            // save the result to target.property
-            if (target instanceof Map) {
-                // ... treating it as a Map entry
-                if (result == null)
-                    ((Map)target).remove(property);
-                else
-                    ((Map)target).put(property, result);
-            } else {
-                // ... treating it as a bean property
-                try {
-                    PropertyDescriptor pd[] = Introspector.getBeanInfo(target.getClass()).getPropertyDescriptors();
-                    boolean succeeded = false;
-                    for (int i = 0; i < pd.length; i++) {
-                        if (pd[i].getName().equals(property)) {
-                            Method m = pd[i].getWriteMethod();
-                            if (m == null) {
-                                throw new JspException(Resources.getMessage("SET_NO_SETTER_METHOD", property));
-                            }
-                            if (result != null) {  
-                                try {
-                                    m.invoke(target, new Object[] { convertToExpectedType(result, m.getParameterTypes()[0]) });
-                                } catch (ELException ex) {
-                                    throw new JspTagException(ex);
-                                }
-                            } else {
-                                m.invoke(target, new Object[] { null });
-                            }
-                            succeeded = true;
-                        }
-                    }
-                    if (!succeeded) {
-                        throw new JspTagException(Resources.getMessage("SET_INVALID_PROPERTY", property));
-                    }
-                } catch (IllegalAccessException ex) {
-                    throw new JspException(ex);
-                } catch (IntrospectionException ex) {
-                    throw new JspException(ex);
-                } catch (InvocationTargetException ex) {
-                    throw new JspException(ex);
-                }
-            }
+            exportToVariable(result);
+        } else if (target == null) {
+            // can happen if target evaluates to null
+            throw new JspTagException(Resources.getMessage("SET_INVALID_TARGET"));
+        } else if (target instanceof Map) {
+            exportToMapProperty(result);
         } else {
-            // should't ever occur because of validation in TLV and setters
-            throw new JspTagException();
+            exportToBeanProperty(result);
         }
 
         return EVAL_PAGE;
     }
-    
+
     Object getResult() {
         if (valueSpecified) {
             return value;
@@ -201,12 +126,114 @@ public class SetSupport extends BodyTagS
             }
         }
     }
-    
+
+    /**
+     * Export the result into a scoped variable.
+     *
+     * @param result the value to export
+     * @throws JspTagException if there was a problem exporting the result
+     */
+    void exportToVariable(Object result) throws JspTagException {
+        /*
+        * Store the result, letting an IllegalArgumentException
+        * propagate back if the scope is invalid (e.g., if an attempt
+        * is made to store something in the session without any
+        * HttpSession existing).
+        */
+        int scopeValue = Util.getScope(scope);
+        ELContext myELContext = pageContext.getELContext();
+        VariableMapper vm = myELContext.getVariableMapper();
+        if (result != null) {
+            // if the result is a ValueExpression we just export to the mapper
+            if (result instanceof ValueExpression) {
+                if (scopeValue != PageContext.PAGE_SCOPE) {
+                    throw new JspTagException(Resources.getMessage("SET_BAD_DEFERRED_SCOPE", scope));
+                }
+                vm.setVariable(var, (ValueExpression)result);
+            } else {
+                // make sure to remove it from the VariableMapper if we will be setting into page scope
+                if (scopeValue == PageContext.PAGE_SCOPE && vm.resolveVariable(var) != null) {
+                    vm.setVariable(var, null);
+                }
+                pageContext.setAttribute(var, result, scopeValue);
+            }
+        } else {
+            //make sure to remove it from the Var mapper
+            if (vm.resolveVariable(var)!=null) {
+                vm.setVariable(var, null);
+            }
+            if (scope != null) {
+                pageContext.removeAttribute(var, Util.getScope(scope));
+            } else {
+                pageContext.removeAttribute(var);
+            }
+        }
+    }
+
+    /**
+     * Export the result into a Map.
+     *
+     * @param result the value to export
+     */
+    void exportToMapProperty(Object result) {
+        @SuppressWarnings("unchecked")
+        Map<Object, Object> map = (Map<Object, Object>) target;
+        if (result == null) {
+            map.remove(property);
+        } else {
+            map.put(property, result);
+        }
+    }
+
     /**
-     * Convert an object to an expected type according to the conversion
+     * Export the result into a bean property.
+     *
+     * @param result the value to export
+     * @throws JspTagException if there was a problem exporting the result
+     */
+    void exportToBeanProperty(Object result) throws JspTagException {
+        PropertyDescriptor[] descriptors;
+        try {
+            descriptors = Introspector.getBeanInfo(target.getClass()).getPropertyDescriptors();
+        } catch (IntrospectionException ex) {
+            throw new JspTagException(ex);
+        }
+
+        for (PropertyDescriptor pd : descriptors) {
+            if (pd.getName().equals(property)) {
+                Method m = pd.getWriteMethod();
+                if (m == null) {
+                    throw new JspTagException(Resources.getMessage("SET_NO_SETTER_METHOD", property));
+                }
+                try {
+                    m.invoke(target, convertToExpectedType(result, m));
+                } catch (ELException ex) {
+                    throw new JspTagException(ex);
+                } catch (IllegalAccessException ex) {
+                    throw new JspTagException(ex);
+                } catch (InvocationTargetException ex) {
+                    throw new JspTagException(ex);
+                }
+                return;
+            }
+        }
+        throw new JspTagException(Resources.getMessage("SET_INVALID_PROPERTY", property));
+    }
+
+    /**
+     * Convert an object to an expected type of the method parameter according to the conversion
      * rules of the Expression Language.
+     *
+     * @param value the value to convert
+     * @param m the setter method
+     * @return value converted to an instance of the expected type; will be null if value was null
+     * @throws javax.el.ELException if there was a problem coercing the value
      */
-    private Object convertToExpectedType(final Object value, Class expectedType) throws ELException {
+    private Object convertToExpectedType(final Object value, Method m) throws ELException {
+        if (value == null) {
+            return null;
+        }
+        Class<?> expectedType = m.getParameterTypes()[0];
         JspFactory jspFactory = JspFactory.getDefaultFactory();
         ExpressionFactory expressionFactory = jspFactory.getJspApplicationContext(pageContext.getServletContext()).getExpressionFactory();
         return expressionFactory.coerceToType(value, expectedType);
@@ -215,14 +242,23 @@ public class SetSupport extends BodyTagS
     //*********************************************************************
     // Accessor methods
 
-    // for tag attribute
+    /**
+     * Name of the exported scoped variable to hold the value specified in the action.
+     * The type of the scoped variable is whatever type the value expression evaluates to.
+     *
+     * @param var name of the exported scoped variable
+     */
     public void setVar(String var) {
         this.var = var;
     }
 
-    // for tag attribute
+    /**
+     * Scope for var.
+     * Values are verified by TLV.
+     *
+     * @param scope the variable scope
+     */
     public void setScope(String scope) {
-        this.scope = Util.getScope(scope);
-        this.scopeSpecified = true;
+        this.scope = scope;
     }
 }



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org