adri...@apache.org wrote:
> Author: adrianc
> Date: Sun Jan 31 03:24:58 2010
> New Revision: 904962
> 
> URL: http://svn.apache.org/viewvc?rev=904962&view=rev
> Log:
> Based on advice from Adam Heath, made FlexibleMapAccessor and UelUtil methods 
> syntactically correct. Methods that read Maps take read-only Map arguments, 
> and methods that write Maps take writable Map arguments.
> 
> 
> Modified:
>     
> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/collections/FlexibleMapAccessor.java
>     ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/UelUtil.java
>     
> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ContextAccessor.java
> 
> Modified: 
> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/collections/FlexibleMapAccessor.java
> URL: 
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/collections/FlexibleMapAccessor.java?rev=904962&r1=904961&r2=904962&view=diff
> ==============================================================================
> --- 
> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/collections/FlexibleMapAccessor.java
>  (original)
> +++ 
> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/collections/FlexibleMapAccessor.java
>  Sun Jan 31 03:24:58 2010
> @@ -170,7 +170,7 @@
>       * @param base the Map to remove from
>       * @return the object removed
>       */
> -    public T remove(Map<String, ? extends Object> base) {
> +    public T remove(Map<String, Object> base) {

Heh, not quite correct.  You are allowed to remove items from a map
with ? extends.

If you recall, ? extends means that the exact class of the value is
unknown.  Since you don't know what the concrete class is, it's not
possible to store any new values into the map, as you might break the
generics contract.

However, removing an item does not break the generics contract.  So,
this change is not correct.

>          if (this.isEmpty()) {
>              return null;
>          }
> @@ -179,8 +179,7 @@
>              return null;
>          }
>          try {
> -            Map<String, Object> writableMap = UtilGenerics.cast(base);
> -            UelUtil.removeValue(writableMap, getExpression(base));
> +            UelUtil.removeValue(base, getExpression(base));
>          } catch (Exception e) {
>              Debug.logError("UEL exception while removing value: " + e + ", 
> original = " + this.original, module);
>          }
> 
> Modified: 
> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/UelUtil.java
> URL: 
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/UelUtil.java?rev=904962&r1=904961&r2=904962&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/UelUtil.java 
> (original)
> +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/UelUtil.java 
> Sun Jan 31 03:24:58 2010
> @@ -82,7 +82,7 @@
>       */
>      @SuppressWarnings("unchecked")
>      public static Object evaluate(Map<String, ? extends Object> context, 
> String expression, Class expectedType) {
> -        ELContext elContext = new BasicContext(context);
> +        ELContext elContext = new ReadOnlyContext(context);
>          ValueExpression ve = exprFactory.createValueExpression(elContext, 
> expression, expectedType);
>          return ve.getValue(elContext);
>      }
> @@ -122,8 +122,29 @@
>      protected static class BasicContext extends ELContext {
>          protected final Map<String, Object> variables;
>          protected final VariableMapper variableMapper;
> -        public BasicContext(Map<String, ? extends Object> context) {
> +        public BasicContext(Map<String, Object> context) {
>              this.variableMapper = new BasicVariableMapper(this);
> +            this.variables = context;
> +        }
> +        @Override
> +        public ELResolver getELResolver() {
> +            return defaultResolver;
> +        }
> +        @Override
> +        public FunctionMapper getFunctionMapper() {
> +            return UelFunctions.getFunctionMapper();
> +        }
> +        @Override
> +        public VariableMapper getVariableMapper() {
> +            return this.variableMapper;
> +        }
> +    }
> +
> +    protected static class ReadOnlyContext extends ELContext {
> +        protected final Map<String, ? extends Object> variables;
> +        protected final VariableMapper variableMapper;
> +        public ReadOnlyContext(Map<String, ? extends Object> context) {
> +            this.variableMapper = new ReadOnlyVariableMapper(this);
>              this.variables = UtilGenerics.cast(context);
>          }
>          @Override
> @@ -138,6 +159,24 @@
>          public VariableMapper getVariableMapper() {
>              return this.variableMapper;
>          }
> +        protected static class ReadOnlyVariableMapper extends VariableMapper 
> {
> +            protected final ReadOnlyContext elContext;
> +            protected ReadOnlyVariableMapper(ReadOnlyContext elContext) {
> +                this.elContext = elContext;
> +            }
> +            @Override
> +            public ValueExpression resolveVariable(String variable) {
> +                Object obj = UelUtil.resolveVariable(variable, 
> this.elContext.variables, null);
> +                if (obj != null) {
> +                    return new ReadOnlyExpression(obj);
> +                }
> +                return null;
> +            }
> +            @Override
> +            public ValueExpression setVariable(String variable, 
> ValueExpression expression) {
> +                throw new PropertyNotWritableException();
> +            }
> +        }
>      }
>  
>      protected static class BasicVariableMapper extends VariableMapper {
> @@ -449,7 +488,7 @@
>          return result;
>      }
>  
> -    public static Object resolveVariable(String variable, Map<String, 
> Object> variables, Locale locale) {
> +    public static Object resolveVariable(String variable, Map<String, ? 
> extends Object> variables, Locale locale) {
>          Object obj = null;
>          String createObjectType = null;
>          String name = variable;
> 
> Modified: 
> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ContextAccessor.java
> URL: 
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ContextAccessor.java?rev=904962&r1=904961&r2=904962&view=diff
> ==============================================================================
> --- 
> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ContextAccessor.java
>  (original)
> +++ 
> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ContextAccessor.java
>  Sun Jan 31 03:24:58 2010
> @@ -91,7 +91,7 @@
>      }
>  
>      /** Based on name remove from Map or from List in Map */
> -    public T remove(Map<String, ? extends Object> theMap, MethodContext 
> methodContext) {
> +    public T remove(Map<String, Object> theMap, MethodContext methodContext) 
> {
>          return fma.remove(theMap);
>      }

Same explanation as above.

Reply via email to