You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by ad...@apache.org on 2012/04/28 21:25:49 UTC

svn commit: r1331811 - /ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java

Author: adrianc
Date: Sat Apr 28 19:25:48 2012
New Revision: 1331811

URL: http://svn.apache.org/viewvc?rev=1331811&view=rev
Log:
Reverting changes to the Mini-language <add-error> element. Too much code depends on the "error_list" name.

Modified:
    ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java

Modified: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java?rev=1331811&r1=1331810&r2=1331811&view=diff
==============================================================================
--- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java (original)
+++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java Sat Apr 28 19:25:48 2012
@@ -22,118 +22,78 @@ import java.util.List;
 
 import javolution.util.FastList;
 
-import org.ofbiz.base.util.Debug;
 import org.ofbiz.base.util.UtilProperties;
+import org.ofbiz.base.util.UtilValidate;
 import org.ofbiz.base.util.UtilXml;
-import org.ofbiz.base.util.string.FlexibleStringExpander;
 import org.ofbiz.minilang.MiniLangException;
-import org.ofbiz.minilang.MiniLangUtil;
-import org.ofbiz.minilang.MiniLangValidate;
 import org.ofbiz.minilang.SimpleMethod;
+import org.ofbiz.minilang.method.ContextAccessor;
 import org.ofbiz.minilang.method.MethodContext;
 import org.ofbiz.minilang.method.MethodOperation;
 import org.w3c.dom.Element;
 
 /**
- * Adds an error message to the error message list.
+ * Adds the fail-message or fail-property value to the error-list.
  */
-public final class AddError extends MethodOperation {
+public class AddError extends MethodOperation {
 
-    // This method is needed only during the v1 to v2 transition
-    private static boolean autoCorrect(Element element) {
-        String errorListAttr = element.getAttribute("error-list-name");
-        if (errorListAttr.length() > 0) {
-            element.removeAttribute("error-list-name");
-            return true;
-        }
-        return false;
-    }
-    
-    private final FlexibleStringExpander messageFse;
-    private final String propertykey;
-    private final String propertyResource;
+    ContextAccessor<List<Object>> errorListAcsr;
+    boolean isProperty = false;
+    String message = null;
+    String propertyResource = null;
 
     public AddError(Element element, SimpleMethod simpleMethod) throws MiniLangException {
         super(element, simpleMethod);
-        if (MiniLangValidate.validationOn()) {
-            MiniLangValidate.childElements(simpleMethod, element, "fail-message", "fail-property");
-            MiniLangValidate.requireAnyChildElement(simpleMethod, element, "fail-message", "fail-property");
+        errorListAcsr = new ContextAccessor<List<Object>>(element.getAttribute("error-list-name"), "error_list");
+        Element failMessage = UtilXml.firstChildElement(element, "fail-message");
+        Element failProperty = UtilXml.firstChildElement(element, "fail-property");
+        if (failMessage != null) {
+            this.message = failMessage.getAttribute("message");
+            this.isProperty = false;
+        } else if (failProperty != null) {
+            this.propertyResource = failProperty.getAttribute("resource");
+            this.message = failProperty.getAttribute("property");
+            this.isProperty = true;
         }
-        boolean elementModified = autoCorrect(element);
-        if (elementModified && MiniLangUtil.autoCorrectOn()) {
-            MiniLangUtil.flagDocumentAsCorrected(element);
-        }
-        Element childElement = UtilXml.firstChildElement(element, "fail-message");
-        if (childElement != null) {
-            if (MiniLangValidate.validationOn()) {
-                MiniLangValidate.attributeNames(simpleMethod, childElement, "message");
-                MiniLangValidate.requiredAttributes(simpleMethod, childElement, "message");
-                MiniLangValidate.constantPlusExpressionAttributes(simpleMethod, childElement, "message");
-            }
-            this.messageFse = FlexibleStringExpander.getInstance(childElement.getAttribute("message"));
-            this.propertykey = null;
-            this.propertyResource = null;
-        } else {
-            childElement = UtilXml.firstChildElement(element, "fail-property");
-            if (childElement != null) {
-                if (MiniLangValidate.validationOn()) {
-                    MiniLangValidate.attributeNames(simpleMethod, childElement, "property", "resource");
-                    MiniLangValidate.requiredAttributes(simpleMethod, childElement, "property", "resource");
-                    MiniLangValidate.constantAttributes(simpleMethod, childElement, "property", "resource");
-                }
-                this.messageFse = FlexibleStringExpander.getInstance(null);
-                this.propertykey = childElement.getAttribute("property");
-                this.propertyResource = childElement.getAttribute("resource");
+    }
+
+    public void addMessage(List<Object> messages, ClassLoader loader, MethodContext methodContext) {
+        String message = methodContext.expandString(this.message);
+        String propertyResource = methodContext.expandString(this.propertyResource);
+        if (!isProperty && message != null) {
+            messages.add(message);
+        } else if (isProperty && propertyResource != null && message != null) {
+            String propMsg = UtilProperties.getMessage(propertyResource, message, methodContext.getEnvMap(), methodContext.getLocale());
+            if (UtilValidate.isEmpty(propMsg)) {
+                messages.add("Simple Method error occurred, but no message was found, sorry.");
             } else {
-                this.messageFse = FlexibleStringExpander.getInstance(null);
-                this.propertykey = null;
-                this.propertyResource = null;
+                messages.add(methodContext.expandString(propMsg));
             }
+        } else {
+            messages.add("Simple Method error occurred, but no message was found, sorry.");
         }
     }
 
     @Override
     public boolean exec(MethodContext methodContext) throws MiniLangException {
-        String message = null;
-        if (!this.messageFse.isEmpty()) {
-            message = this.messageFse.expandString(methodContext.getEnvMap());
-        } else if (this.propertyResource != null) {
-            message = UtilProperties.getMessage(this.propertyResource, this.propertykey, methodContext.getEnvMap(), methodContext.getLocale());
-        }
-        if (message != null) {
-            List<String> messages = null;
-            if (methodContext.getMethodType() == MethodContext.EVENT) {
-                messages = methodContext.getEnv(this.simpleMethod.getEventErrorMessageListName());
-                if (messages == null) {
-                    messages = FastList.newInstance();
-                    methodContext.putEnv(this.simpleMethod.getEventErrorMessageListName(), messages);
-                }
-            } else {
-                messages = methodContext.getEnv(this.simpleMethod.getServiceErrorMessageListName());
-                if (messages == null) {
-                    messages = FastList.newInstance();
-                    methodContext.putEnv(this.simpleMethod.getServiceErrorMessageListName(), messages);
-                }
-            }
-            messages.add(message);
-            // TODO: Remove this line after tests are fixed
-            Debug.logInfo("<add-error> message = " + message + ", location = " + this.simpleMethod.getLocationAndName() + ", line = " + this.getLineNumber(), this.getClass().getName());
+        List<Object> messages = errorListAcsr.get(methodContext);
+        if (messages == null) {
+            messages = FastList.newInstance();
+            errorListAcsr.put(methodContext, messages);
         }
+        this.addMessage(messages, methodContext.getLoader(), methodContext);
         return true;
     }
 
     @Override
     public String expandedString(MethodContext methodContext) {
-        return FlexibleStringExpander.expandString(toString(), methodContext.getEnvMap());
+        // TODO: something more than a stub/dummy
+        return this.rawString();
     }
 
     @Override
     public String rawString() {
-        return toString();
-    }
-
-    @Override
-    public String toString() {
+        // TODO: something more than the empty tag
         return "<add-error/>";
     }
 



Re: svn commit: r1331811 - /ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java

Posted by Scott Gray <sc...@hotwaxmedia.com>.
Awesome thanks for the quick response Adrian.  I'll admit to not knowing much about it but my understanding has always been that the method exits immediately if an internal error is generated.  

Regards
Scott

On 11/05/2012, at 10:27 PM, Adrian Crum wrote:

> Oops, I missed two lines while changing it back. Fixed in rev 1337103.
> 
> -Adrian
> 
> On 5/11/2012 10:56 AM, Adrian Crum wrote:
>> Fixed in revision 1337092. Thanks Scott!
>> 
>> The <add-error> + <check-errors> element combination and interaction was very confusing to me. In some places it is used as you described, and in other places the <add-error> element was used to simply create a list of messages. Plus, in the original code, the <check-errors> element completely ignores any errors generated internally by the script engine. In my previous commits I was trying to bring some consistency to the behavior, but that appears to have broken things, so I restored the original behavior.
>> 
>> -Adrian
>> 
>> On 5/11/2012 6:21 AM, Scott Gray wrote:
>>> Hi Adrian,
>>> 
>>> <add-error>  +<check-errors/>  doesn't seem to be working for minilang events anymore.  For example:
>>> https://demo-trunk.ofbiz.apache.org:8443/ecommerce/control/AnonContactus
>>> 
>>> Submitting that form while completely empty returns successfully even though the event has validation for the required fields.  I think add-error is adding to a differently list than what check-errors is looking at.
>>> 
>>> Thanks
>>> Scott
>>> 
>>> On 29/04/2012, at 7:25 AM, adrianc@apache.org wrote:
>>> 
>>>> Author: adrianc
>>>> Date: Sat Apr 28 19:25:48 2012
>>>> New Revision: 1331811
>>>> 
>>>> URL: http://svn.apache.org/viewvc?rev=1331811&view=rev
>>>> Log:
>>>> Reverting changes to the Mini-language<add-error>  element. Too much code depends on the "error_list" name.
>>>> 
>>>> Modified:
>>>>    ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java
>>>> 
>>>> Modified: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java?rev=1331811&r1=1331810&r2=1331811&view=diff
>>>> ============================================================================== 
>>>> --- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java (original)
>>>> +++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java Sat Apr 28 19:25:48 2012
>>>> @@ -22,118 +22,78 @@ import java.util.List;
>>>> 
>>>> import javolution.util.FastList;
>>>> 
>>>> -import org.ofbiz.base.util.Debug;
>>>> import org.ofbiz.base.util.UtilProperties;
>>>> +import org.ofbiz.base.util.UtilValidate;
>>>> import org.ofbiz.base.util.UtilXml;
>>>> -import org.ofbiz.base.util.string.FlexibleStringExpander;
>>>> import org.ofbiz.minilang.MiniLangException;
>>>> -import org.ofbiz.minilang.MiniLangUtil;
>>>> -import org.ofbiz.minilang.MiniLangValidate;
>>>> import org.ofbiz.minilang.SimpleMethod;
>>>> +import org.ofbiz.minilang.method.ContextAccessor;
>>>> import org.ofbiz.minilang.method.MethodContext;
>>>> import org.ofbiz.minilang.method.MethodOperation;
>>>> import org.w3c.dom.Element;
>>>> 
>>>> /**
>>>> - * Adds an error message to the error message list.
>>>> + * Adds the fail-message or fail-property value to the error-list.
>>>>  */
>>>> -public final class AddError extends MethodOperation {
>>>> +public class AddError extends MethodOperation {
>>>> 
>>>> -    // This method is needed only during the v1 to v2 transition
>>>> -    private static boolean autoCorrect(Element element) {
>>>> -        String errorListAttr = element.getAttribute("error-list-name");
>>>> -        if (errorListAttr.length()>  0) {
>>>> -            element.removeAttribute("error-list-name");
>>>> -            return true;
>>>> -        }
>>>> -        return false;
>>>> -    }
>>>> -
>>>> -    private final FlexibleStringExpander messageFse;
>>>> -    private final String propertykey;
>>>> -    private final String propertyResource;
>>>> +    ContextAccessor<List<Object>>  errorListAcsr;
>>>> +    boolean isProperty = false;
>>>> +    String message = null;
>>>> +    String propertyResource = null;
>>>> 
>>>>     public AddError(Element element, SimpleMethod simpleMethod) throws MiniLangException {
>>>>         super(element, simpleMethod);
>>>> -        if (MiniLangValidate.validationOn()) {
>>>> -            MiniLangValidate.childElements(simpleMethod, element, "fail-message", "fail-property");
>>>> -            MiniLangValidate.requireAnyChildElement(simpleMethod, element, "fail-message", "fail-property");
>>>> +        errorListAcsr = new ContextAccessor<List<Object>>(element.getAttribute("error-list-name"), "error_list");
>>>> +        Element failMessage = UtilXml.firstChildElement(element, "fail-message");
>>>> +        Element failProperty = UtilXml.firstChildElement(element, "fail-property");
>>>> +        if (failMessage != null) {
>>>> +            this.message = failMessage.getAttribute("message");
>>>> +            this.isProperty = false;
>>>> +        } else if (failProperty != null) {
>>>> +            this.propertyResource = failProperty.getAttribute("resource");
>>>> +            this.message = failProperty.getAttribute("property");
>>>> +            this.isProperty = true;
>>>>         }
>>>> -        boolean elementModified = autoCorrect(element);
>>>> -        if (elementModified&&  MiniLangUtil.autoCorrectOn()) {
>>>> -            MiniLangUtil.flagDocumentAsCorrected(element);
>>>> -        }
>>>> -        Element childElement = UtilXml.firstChildElement(element, "fail-message");
>>>> -        if (childElement != null) {
>>>> -            if (MiniLangValidate.validationOn()) {
>>>> -                MiniLangValidate.attributeNames(simpleMethod, childElement, "message");
>>>> -                MiniLangValidate.requiredAttributes(simpleMethod, childElement, "message");
>>>> -                MiniLangValidate.constantPlusExpressionAttributes(simpleMethod, childElement, "message");
>>>> -            }
>>>> -            this.messageFse = FlexibleStringExpander.getInstance(childElement.getAttribute("message"));
>>>> -            this.propertykey = null;
>>>> -            this.propertyResource = null;
>>>> -        } else {
>>>> -            childElement = UtilXml.firstChildElement(element, "fail-property");
>>>> -            if (childElement != null) {
>>>> -                if (MiniLangValidate.validationOn()) {
>>>> -                    MiniLangValidate.attributeNames(simpleMethod, childElement, "property", "resource");
>>>> -                    MiniLangValidate.requiredAttributes(simpleMethod, childElement, "property", "resource");
>>>> -                    MiniLangValidate.constantAttributes(simpleMethod, childElement, "property", "resource");
>>>> -                }
>>>> -                this.messageFse = FlexibleStringExpander.getInstance(null);
>>>> -                this.propertykey = childElement.getAttribute("property");
>>>> -                this.propertyResource = childElement.getAttribute("resource");
>>>> +    }
>>>> +
>>>> +    public void addMessage(List<Object>  messages, ClassLoader loader, MethodContext methodContext) {
>>>> +        String message = methodContext.expandString(this.message);
>>>> +        String propertyResource = methodContext.expandString(this.propertyResource);
>>>> +        if (!isProperty&&  message != null) {
>>>> +            messages.add(message);
>>>> +        } else if (isProperty&&  propertyResource != null&&  message != null) {
>>>> +            String propMsg = UtilProperties.getMessage(propertyResource, message, methodContext.getEnvMap(), methodContext.getLocale());
>>>> +            if (UtilValidate.isEmpty(propMsg)) {
>>>> +                messages.add("Simple Method error occurred, but no message was found, sorry.");
>>>>             } else {
>>>> -                this.messageFse = FlexibleStringExpander.getInstance(null);
>>>> -                this.propertykey = null;
>>>> -                this.propertyResource = null;
>>>> +                messages.add(methodContext.expandString(propMsg));
>>>>             }
>>>> +        } else {
>>>> +            messages.add("Simple Method error occurred, but no message was found, sorry.");
>>>>         }
>>>>     }
>>>> 
>>>>     @Override
>>>>     public boolean exec(MethodContext methodContext) throws MiniLangException {
>>>> -        String message = null;
>>>> -        if (!this.messageFse.isEmpty()) {
>>>> -            message = this.messageFse.expandString(methodContext.getEnvMap());
>>>> -        } else if (this.propertyResource != null) {
>>>> -            message = UtilProperties.getMessage(this.propertyResource, this.propertykey, methodContext.getEnvMap(), methodContext.getLocale());
>>>> -        }
>>>> -        if (message != null) {
>>>> -            List<String>  messages = null;
>>>> -            if (methodContext.getMethodType() == MethodContext.EVENT) {
>>>> -                messages = methodContext.getEnv(this.simpleMethod.getEventErrorMessageListName());
>>>> -                if (messages == null) {
>>>> -                    messages = FastList.newInstance();
>>>> -                    methodContext.putEnv(this.simpleMethod.getEventErrorMessageListName(), messages);
>>>> -                }
>>>> -            } else {
>>>> -                messages = methodContext.getEnv(this.simpleMethod.getServiceErrorMessageListName());
>>>> -                if (messages == null) {
>>>> -                    messages = FastList.newInstance();
>>>> -                    methodContext.putEnv(this.simpleMethod.getServiceErrorMessageListName(), messages);
>>>> -                }
>>>> -            }
>>>> -            messages.add(message);
>>>> -            // TODO: Remove this line after tests are fixed
>>>> -            Debug.logInfo("<add-error>  message = " + message + ", location = " + this.simpleMethod.getLocationAndName() + ", line = " + this.getLineNumber(), this.getClass().getName());
>>>> +        List<Object>  messages = errorListAcsr.get(methodContext);
>>>> +        if (messages == null) {
>>>> +            messages = FastList.newInstance();
>>>> +            errorListAcsr.put(methodContext, messages);
>>>>         }
>>>> +        this.addMessage(messages, methodContext.getLoader(), methodContext);
>>>>         return true;
>>>>     }
>>>> 
>>>>     @Override
>>>>     public String expandedString(MethodContext methodContext) {
>>>> -        return FlexibleStringExpander.expandString(toString(), methodContext.getEnvMap());
>>>> +        // TODO: something more than a stub/dummy
>>>> +        return this.rawString();
>>>>     }
>>>> 
>>>>     @Override
>>>>     public String rawString() {
>>>> -        return toString();
>>>> -    }
>>>> -
>>>> -    @Override
>>>> -    public String toString() {
>>>> +        // TODO: something more than the empty tag
>>>>         return "<add-error/>";
>>>>     }
>>>> 
>>>> 
>>>> 


Re: svn commit: r1331811 - /ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java

Posted by Adrian Crum <ad...@sandglass-software.com>.
Oops, I missed two lines while changing it back. Fixed in rev 1337103.

-Adrian

On 5/11/2012 10:56 AM, Adrian Crum wrote:
> Fixed in revision 1337092. Thanks Scott!
>
> The <add-error> + <check-errors> element combination and interaction 
> was very confusing to me. In some places it is used as you described, 
> and in other places the <add-error> element was used to simply create 
> a list of messages. Plus, in the original code, the <check-errors> 
> element completely ignores any errors generated internally by the 
> script engine. In my previous commits I was trying to bring some 
> consistency to the behavior, but that appears to have broken things, 
> so I restored the original behavior.
>
> -Adrian
>
> On 5/11/2012 6:21 AM, Scott Gray wrote:
>> Hi Adrian,
>>
>> <add-error>  +<check-errors/>  doesn't seem to be working for 
>> minilang events anymore.  For example:
>> https://demo-trunk.ofbiz.apache.org:8443/ecommerce/control/AnonContactus
>>
>> Submitting that form while completely empty returns successfully even 
>> though the event has validation for the required fields.  I think 
>> add-error is adding to a differently list than what check-errors is 
>> looking at.
>>
>> Thanks
>> Scott
>>
>> On 29/04/2012, at 7:25 AM, adrianc@apache.org wrote:
>>
>>> Author: adrianc
>>> Date: Sat Apr 28 19:25:48 2012
>>> New Revision: 1331811
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1331811&view=rev
>>> Log:
>>> Reverting changes to the Mini-language<add-error>  element. Too much 
>>> code depends on the "error_list" name.
>>>
>>> Modified:
>>>     
>>> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java
>>>
>>> Modified: 
>>> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java
>>> URL: 
>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java?rev=1331811&r1=1331810&r2=1331811&view=diff
>>> ============================================================================== 
>>>
>>> --- 
>>> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java 
>>> (original)
>>> +++ 
>>> ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java 
>>> Sat Apr 28 19:25:48 2012
>>> @@ -22,118 +22,78 @@ import java.util.List;
>>>
>>> import javolution.util.FastList;
>>>
>>> -import org.ofbiz.base.util.Debug;
>>> import org.ofbiz.base.util.UtilProperties;
>>> +import org.ofbiz.base.util.UtilValidate;
>>> import org.ofbiz.base.util.UtilXml;
>>> -import org.ofbiz.base.util.string.FlexibleStringExpander;
>>> import org.ofbiz.minilang.MiniLangException;
>>> -import org.ofbiz.minilang.MiniLangUtil;
>>> -import org.ofbiz.minilang.MiniLangValidate;
>>> import org.ofbiz.minilang.SimpleMethod;
>>> +import org.ofbiz.minilang.method.ContextAccessor;
>>> import org.ofbiz.minilang.method.MethodContext;
>>> import org.ofbiz.minilang.method.MethodOperation;
>>> import org.w3c.dom.Element;
>>>
>>> /**
>>> - * Adds an error message to the error message list.
>>> + * Adds the fail-message or fail-property value to the error-list.
>>>   */
>>> -public final class AddError extends MethodOperation {
>>> +public class AddError extends MethodOperation {
>>>
>>> -    // This method is needed only during the v1 to v2 transition
>>> -    private static boolean autoCorrect(Element element) {
>>> -        String errorListAttr = 
>>> element.getAttribute("error-list-name");
>>> -        if (errorListAttr.length()>  0) {
>>> -            element.removeAttribute("error-list-name");
>>> -            return true;
>>> -        }
>>> -        return false;
>>> -    }
>>> -
>>> -    private final FlexibleStringExpander messageFse;
>>> -    private final String propertykey;
>>> -    private final String propertyResource;
>>> +    ContextAccessor<List<Object>>  errorListAcsr;
>>> +    boolean isProperty = false;
>>> +    String message = null;
>>> +    String propertyResource = null;
>>>
>>>      public AddError(Element element, SimpleMethod simpleMethod) 
>>> throws MiniLangException {
>>>          super(element, simpleMethod);
>>> -        if (MiniLangValidate.validationOn()) {
>>> -            MiniLangValidate.childElements(simpleMethod, element, 
>>> "fail-message", "fail-property");
>>> -            MiniLangValidate.requireAnyChildElement(simpleMethod, 
>>> element, "fail-message", "fail-property");
>>> +        errorListAcsr = new 
>>> ContextAccessor<List<Object>>(element.getAttribute("error-list-name"), 
>>> "error_list");
>>> +        Element failMessage = UtilXml.firstChildElement(element, 
>>> "fail-message");
>>> +        Element failProperty = UtilXml.firstChildElement(element, 
>>> "fail-property");
>>> +        if (failMessage != null) {
>>> +            this.message = failMessage.getAttribute("message");
>>> +            this.isProperty = false;
>>> +        } else if (failProperty != null) {
>>> +            this.propertyResource = 
>>> failProperty.getAttribute("resource");
>>> +            this.message = failProperty.getAttribute("property");
>>> +            this.isProperty = true;
>>>          }
>>> -        boolean elementModified = autoCorrect(element);
>>> -        if (elementModified&&  MiniLangUtil.autoCorrectOn()) {
>>> -            MiniLangUtil.flagDocumentAsCorrected(element);
>>> -        }
>>> -        Element childElement = UtilXml.firstChildElement(element, 
>>> "fail-message");
>>> -        if (childElement != null) {
>>> -            if (MiniLangValidate.validationOn()) {
>>> -                MiniLangValidate.attributeNames(simpleMethod, 
>>> childElement, "message");
>>> -                MiniLangValidate.requiredAttributes(simpleMethod, 
>>> childElement, "message");
>>> -                
>>> MiniLangValidate.constantPlusExpressionAttributes(simpleMethod, 
>>> childElement, "message");
>>> -            }
>>> -            this.messageFse = 
>>> FlexibleStringExpander.getInstance(childElement.getAttribute("message"));
>>> -            this.propertykey = null;
>>> -            this.propertyResource = null;
>>> -        } else {
>>> -            childElement = UtilXml.firstChildElement(element, 
>>> "fail-property");
>>> -            if (childElement != null) {
>>> -                if (MiniLangValidate.validationOn()) {
>>> -                    MiniLangValidate.attributeNames(simpleMethod, 
>>> childElement, "property", "resource");
>>> -                    
>>> MiniLangValidate.requiredAttributes(simpleMethod, childElement, 
>>> "property", "resource");
>>> -                    
>>> MiniLangValidate.constantAttributes(simpleMethod, childElement, 
>>> "property", "resource");
>>> -                }
>>> -                this.messageFse = 
>>> FlexibleStringExpander.getInstance(null);
>>> -                this.propertykey = 
>>> childElement.getAttribute("property");
>>> -                this.propertyResource = 
>>> childElement.getAttribute("resource");
>>> +    }
>>> +
>>> +    public void addMessage(List<Object>  messages, ClassLoader 
>>> loader, MethodContext methodContext) {
>>> +        String message = methodContext.expandString(this.message);
>>> +        String propertyResource = 
>>> methodContext.expandString(this.propertyResource);
>>> +        if (!isProperty&&  message != null) {
>>> +            messages.add(message);
>>> +        } else if (isProperty&&  propertyResource != null&&  
>>> message != null) {
>>> +            String propMsg = 
>>> UtilProperties.getMessage(propertyResource, message, 
>>> methodContext.getEnvMap(), methodContext.getLocale());
>>> +            if (UtilValidate.isEmpty(propMsg)) {
>>> +                messages.add("Simple Method error occurred, but no 
>>> message was found, sorry.");
>>>              } else {
>>> -                this.messageFse = 
>>> FlexibleStringExpander.getInstance(null);
>>> -                this.propertykey = null;
>>> -                this.propertyResource = null;
>>> +                messages.add(methodContext.expandString(propMsg));
>>>              }
>>> +        } else {
>>> +            messages.add("Simple Method error occurred, but no 
>>> message was found, sorry.");
>>>          }
>>>      }
>>>
>>>      @Override
>>>      public boolean exec(MethodContext methodContext) throws 
>>> MiniLangException {
>>> -        String message = null;
>>> -        if (!this.messageFse.isEmpty()) {
>>> -            message = 
>>> this.messageFse.expandString(methodContext.getEnvMap());
>>> -        } else if (this.propertyResource != null) {
>>> -            message = 
>>> UtilProperties.getMessage(this.propertyResource, this.propertykey, 
>>> methodContext.getEnvMap(), methodContext.getLocale());
>>> -        }
>>> -        if (message != null) {
>>> -            List<String>  messages = null;
>>> -            if (methodContext.getMethodType() == 
>>> MethodContext.EVENT) {
>>> -                messages = 
>>> methodContext.getEnv(this.simpleMethod.getEventErrorMessageListName());
>>> -                if (messages == null) {
>>> -                    messages = FastList.newInstance();
>>> -                    
>>> methodContext.putEnv(this.simpleMethod.getEventErrorMessageListName(), 
>>> messages);
>>> -                }
>>> -            } else {
>>> -                messages = 
>>> methodContext.getEnv(this.simpleMethod.getServiceErrorMessageListName());
>>> -                if (messages == null) {
>>> -                    messages = FastList.newInstance();
>>> -                    
>>> methodContext.putEnv(this.simpleMethod.getServiceErrorMessageListName(), 
>>> messages);
>>> -                }
>>> -            }
>>> -            messages.add(message);
>>> -            // TODO: Remove this line after tests are fixed
>>> -            Debug.logInfo("<add-error>  message = " + message + ", 
>>> location = " + this.simpleMethod.getLocationAndName() + ", line = " 
>>> + this.getLineNumber(), this.getClass().getName());
>>> +        List<Object>  messages = errorListAcsr.get(methodContext);
>>> +        if (messages == null) {
>>> +            messages = FastList.newInstance();
>>> +            errorListAcsr.put(methodContext, messages);
>>>          }
>>> +        this.addMessage(messages, methodContext.getLoader(), 
>>> methodContext);
>>>          return true;
>>>      }
>>>
>>>      @Override
>>>      public String expandedString(MethodContext methodContext) {
>>> -        return FlexibleStringExpander.expandString(toString(), 
>>> methodContext.getEnvMap());
>>> +        // TODO: something more than a stub/dummy
>>> +        return this.rawString();
>>>      }
>>>
>>>      @Override
>>>      public String rawString() {
>>> -        return toString();
>>> -    }
>>> -
>>> -    @Override
>>> -    public String toString() {
>>> +        // TODO: something more than the empty tag
>>>          return "<add-error/>";
>>>      }
>>>
>>>
>>>

Re: svn commit: r1331811 - /ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java

Posted by Adrian Crum <ad...@sandglass-software.com>.
Fixed in revision 1337092. Thanks Scott!

The <add-error> + <check-errors> element combination and interaction was 
very confusing to me. In some places it is used as you described, and in 
other places the <add-error> element was used to simply create a list of 
messages. Plus, in the original code, the <check-errors> element 
completely ignores any errors generated internally by the script engine. 
In my previous commits I was trying to bring some consistency to the 
behavior, but that appears to have broken things, so I restored the 
original behavior.

-Adrian

On 5/11/2012 6:21 AM, Scott Gray wrote:
> Hi Adrian,
>
> <add-error>  +<check-errors/>  doesn't seem to be working for minilang events anymore.  For example:
> https://demo-trunk.ofbiz.apache.org:8443/ecommerce/control/AnonContactus
>
> Submitting that form while completely empty returns successfully even though the event has validation for the required fields.  I think add-error is adding to a differently list than what check-errors is looking at.
>
> Thanks
> Scott
>
> On 29/04/2012, at 7:25 AM, adrianc@apache.org wrote:
>
>> Author: adrianc
>> Date: Sat Apr 28 19:25:48 2012
>> New Revision: 1331811
>>
>> URL: http://svn.apache.org/viewvc?rev=1331811&view=rev
>> Log:
>> Reverting changes to the Mini-language<add-error>  element. Too much code depends on the "error_list" name.
>>
>> Modified:
>>     ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java
>>
>> Modified: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java?rev=1331811&r1=1331810&r2=1331811&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java (original)
>> +++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java Sat Apr 28 19:25:48 2012
>> @@ -22,118 +22,78 @@ import java.util.List;
>>
>> import javolution.util.FastList;
>>
>> -import org.ofbiz.base.util.Debug;
>> import org.ofbiz.base.util.UtilProperties;
>> +import org.ofbiz.base.util.UtilValidate;
>> import org.ofbiz.base.util.UtilXml;
>> -import org.ofbiz.base.util.string.FlexibleStringExpander;
>> import org.ofbiz.minilang.MiniLangException;
>> -import org.ofbiz.minilang.MiniLangUtil;
>> -import org.ofbiz.minilang.MiniLangValidate;
>> import org.ofbiz.minilang.SimpleMethod;
>> +import org.ofbiz.minilang.method.ContextAccessor;
>> import org.ofbiz.minilang.method.MethodContext;
>> import org.ofbiz.minilang.method.MethodOperation;
>> import org.w3c.dom.Element;
>>
>> /**
>> - * Adds an error message to the error message list.
>> + * Adds the fail-message or fail-property value to the error-list.
>>   */
>> -public final class AddError extends MethodOperation {
>> +public class AddError extends MethodOperation {
>>
>> -    // This method is needed only during the v1 to v2 transition
>> -    private static boolean autoCorrect(Element element) {
>> -        String errorListAttr = element.getAttribute("error-list-name");
>> -        if (errorListAttr.length()>  0) {
>> -            element.removeAttribute("error-list-name");
>> -            return true;
>> -        }
>> -        return false;
>> -    }
>> -
>> -    private final FlexibleStringExpander messageFse;
>> -    private final String propertykey;
>> -    private final String propertyResource;
>> +    ContextAccessor<List<Object>>  errorListAcsr;
>> +    boolean isProperty = false;
>> +    String message = null;
>> +    String propertyResource = null;
>>
>>      public AddError(Element element, SimpleMethod simpleMethod) throws MiniLangException {
>>          super(element, simpleMethod);
>> -        if (MiniLangValidate.validationOn()) {
>> -            MiniLangValidate.childElements(simpleMethod, element, "fail-message", "fail-property");
>> -            MiniLangValidate.requireAnyChildElement(simpleMethod, element, "fail-message", "fail-property");
>> +        errorListAcsr = new ContextAccessor<List<Object>>(element.getAttribute("error-list-name"), "error_list");
>> +        Element failMessage = UtilXml.firstChildElement(element, "fail-message");
>> +        Element failProperty = UtilXml.firstChildElement(element, "fail-property");
>> +        if (failMessage != null) {
>> +            this.message = failMessage.getAttribute("message");
>> +            this.isProperty = false;
>> +        } else if (failProperty != null) {
>> +            this.propertyResource = failProperty.getAttribute("resource");
>> +            this.message = failProperty.getAttribute("property");
>> +            this.isProperty = true;
>>          }
>> -        boolean elementModified = autoCorrect(element);
>> -        if (elementModified&&  MiniLangUtil.autoCorrectOn()) {
>> -            MiniLangUtil.flagDocumentAsCorrected(element);
>> -        }
>> -        Element childElement = UtilXml.firstChildElement(element, "fail-message");
>> -        if (childElement != null) {
>> -            if (MiniLangValidate.validationOn()) {
>> -                MiniLangValidate.attributeNames(simpleMethod, childElement, "message");
>> -                MiniLangValidate.requiredAttributes(simpleMethod, childElement, "message");
>> -                MiniLangValidate.constantPlusExpressionAttributes(simpleMethod, childElement, "message");
>> -            }
>> -            this.messageFse = FlexibleStringExpander.getInstance(childElement.getAttribute("message"));
>> -            this.propertykey = null;
>> -            this.propertyResource = null;
>> -        } else {
>> -            childElement = UtilXml.firstChildElement(element, "fail-property");
>> -            if (childElement != null) {
>> -                if (MiniLangValidate.validationOn()) {
>> -                    MiniLangValidate.attributeNames(simpleMethod, childElement, "property", "resource");
>> -                    MiniLangValidate.requiredAttributes(simpleMethod, childElement, "property", "resource");
>> -                    MiniLangValidate.constantAttributes(simpleMethod, childElement, "property", "resource");
>> -                }
>> -                this.messageFse = FlexibleStringExpander.getInstance(null);
>> -                this.propertykey = childElement.getAttribute("property");
>> -                this.propertyResource = childElement.getAttribute("resource");
>> +    }
>> +
>> +    public void addMessage(List<Object>  messages, ClassLoader loader, MethodContext methodContext) {
>> +        String message = methodContext.expandString(this.message);
>> +        String propertyResource = methodContext.expandString(this.propertyResource);
>> +        if (!isProperty&&  message != null) {
>> +            messages.add(message);
>> +        } else if (isProperty&&  propertyResource != null&&  message != null) {
>> +            String propMsg = UtilProperties.getMessage(propertyResource, message, methodContext.getEnvMap(), methodContext.getLocale());
>> +            if (UtilValidate.isEmpty(propMsg)) {
>> +                messages.add("Simple Method error occurred, but no message was found, sorry.");
>>              } else {
>> -                this.messageFse = FlexibleStringExpander.getInstance(null);
>> -                this.propertykey = null;
>> -                this.propertyResource = null;
>> +                messages.add(methodContext.expandString(propMsg));
>>              }
>> +        } else {
>> +            messages.add("Simple Method error occurred, but no message was found, sorry.");
>>          }
>>      }
>>
>>      @Override
>>      public boolean exec(MethodContext methodContext) throws MiniLangException {
>> -        String message = null;
>> -        if (!this.messageFse.isEmpty()) {
>> -            message = this.messageFse.expandString(methodContext.getEnvMap());
>> -        } else if (this.propertyResource != null) {
>> -            message = UtilProperties.getMessage(this.propertyResource, this.propertykey, methodContext.getEnvMap(), methodContext.getLocale());
>> -        }
>> -        if (message != null) {
>> -            List<String>  messages = null;
>> -            if (methodContext.getMethodType() == MethodContext.EVENT) {
>> -                messages = methodContext.getEnv(this.simpleMethod.getEventErrorMessageListName());
>> -                if (messages == null) {
>> -                    messages = FastList.newInstance();
>> -                    methodContext.putEnv(this.simpleMethod.getEventErrorMessageListName(), messages);
>> -                }
>> -            } else {
>> -                messages = methodContext.getEnv(this.simpleMethod.getServiceErrorMessageListName());
>> -                if (messages == null) {
>> -                    messages = FastList.newInstance();
>> -                    methodContext.putEnv(this.simpleMethod.getServiceErrorMessageListName(), messages);
>> -                }
>> -            }
>> -            messages.add(message);
>> -            // TODO: Remove this line after tests are fixed
>> -            Debug.logInfo("<add-error>  message = " + message + ", location = " + this.simpleMethod.getLocationAndName() + ", line = " + this.getLineNumber(), this.getClass().getName());
>> +        List<Object>  messages = errorListAcsr.get(methodContext);
>> +        if (messages == null) {
>> +            messages = FastList.newInstance();
>> +            errorListAcsr.put(methodContext, messages);
>>          }
>> +        this.addMessage(messages, methodContext.getLoader(), methodContext);
>>          return true;
>>      }
>>
>>      @Override
>>      public String expandedString(MethodContext methodContext) {
>> -        return FlexibleStringExpander.expandString(toString(), methodContext.getEnvMap());
>> +        // TODO: something more than a stub/dummy
>> +        return this.rawString();
>>      }
>>
>>      @Override
>>      public String rawString() {
>> -        return toString();
>> -    }
>> -
>> -    @Override
>> -    public String toString() {
>> +        // TODO: something more than the empty tag
>>          return "<add-error/>";
>>      }
>>
>>
>>

Re: svn commit: r1331811 - /ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java

Posted by Scott Gray <sc...@hotwaxmedia.com>.
Hi Adrian,

<add-error> + <check-errors/> doesn't seem to be working for minilang events anymore.  For example:
https://demo-trunk.ofbiz.apache.org:8443/ecommerce/control/AnonContactus

Submitting that form while completely empty returns successfully even though the event has validation for the required fields.  I think add-error is adding to a differently list than what check-errors is looking at.

Thanks
Scott

On 29/04/2012, at 7:25 AM, adrianc@apache.org wrote:

> Author: adrianc
> Date: Sat Apr 28 19:25:48 2012
> New Revision: 1331811
> 
> URL: http://svn.apache.org/viewvc?rev=1331811&view=rev
> Log:
> Reverting changes to the Mini-language <add-error> element. Too much code depends on the "error_list" name.
> 
> Modified:
>    ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java
> 
> Modified: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java?rev=1331811&r1=1331810&r2=1331811&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java (original)
> +++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java Sat Apr 28 19:25:48 2012
> @@ -22,118 +22,78 @@ import java.util.List;
> 
> import javolution.util.FastList;
> 
> -import org.ofbiz.base.util.Debug;
> import org.ofbiz.base.util.UtilProperties;
> +import org.ofbiz.base.util.UtilValidate;
> import org.ofbiz.base.util.UtilXml;
> -import org.ofbiz.base.util.string.FlexibleStringExpander;
> import org.ofbiz.minilang.MiniLangException;
> -import org.ofbiz.minilang.MiniLangUtil;
> -import org.ofbiz.minilang.MiniLangValidate;
> import org.ofbiz.minilang.SimpleMethod;
> +import org.ofbiz.minilang.method.ContextAccessor;
> import org.ofbiz.minilang.method.MethodContext;
> import org.ofbiz.minilang.method.MethodOperation;
> import org.w3c.dom.Element;
> 
> /**
> - * Adds an error message to the error message list.
> + * Adds the fail-message or fail-property value to the error-list.
>  */
> -public final class AddError extends MethodOperation {
> +public class AddError extends MethodOperation {
> 
> -    // This method is needed only during the v1 to v2 transition
> -    private static boolean autoCorrect(Element element) {
> -        String errorListAttr = element.getAttribute("error-list-name");
> -        if (errorListAttr.length() > 0) {
> -            element.removeAttribute("error-list-name");
> -            return true;
> -        }
> -        return false;
> -    }
> -    
> -    private final FlexibleStringExpander messageFse;
> -    private final String propertykey;
> -    private final String propertyResource;
> +    ContextAccessor<List<Object>> errorListAcsr;
> +    boolean isProperty = false;
> +    String message = null;
> +    String propertyResource = null;
> 
>     public AddError(Element element, SimpleMethod simpleMethod) throws MiniLangException {
>         super(element, simpleMethod);
> -        if (MiniLangValidate.validationOn()) {
> -            MiniLangValidate.childElements(simpleMethod, element, "fail-message", "fail-property");
> -            MiniLangValidate.requireAnyChildElement(simpleMethod, element, "fail-message", "fail-property");
> +        errorListAcsr = new ContextAccessor<List<Object>>(element.getAttribute("error-list-name"), "error_list");
> +        Element failMessage = UtilXml.firstChildElement(element, "fail-message");
> +        Element failProperty = UtilXml.firstChildElement(element, "fail-property");
> +        if (failMessage != null) {
> +            this.message = failMessage.getAttribute("message");
> +            this.isProperty = false;
> +        } else if (failProperty != null) {
> +            this.propertyResource = failProperty.getAttribute("resource");
> +            this.message = failProperty.getAttribute("property");
> +            this.isProperty = true;
>         }
> -        boolean elementModified = autoCorrect(element);
> -        if (elementModified && MiniLangUtil.autoCorrectOn()) {
> -            MiniLangUtil.flagDocumentAsCorrected(element);
> -        }
> -        Element childElement = UtilXml.firstChildElement(element, "fail-message");
> -        if (childElement != null) {
> -            if (MiniLangValidate.validationOn()) {
> -                MiniLangValidate.attributeNames(simpleMethod, childElement, "message");
> -                MiniLangValidate.requiredAttributes(simpleMethod, childElement, "message");
> -                MiniLangValidate.constantPlusExpressionAttributes(simpleMethod, childElement, "message");
> -            }
> -            this.messageFse = FlexibleStringExpander.getInstance(childElement.getAttribute("message"));
> -            this.propertykey = null;
> -            this.propertyResource = null;
> -        } else {
> -            childElement = UtilXml.firstChildElement(element, "fail-property");
> -            if (childElement != null) {
> -                if (MiniLangValidate.validationOn()) {
> -                    MiniLangValidate.attributeNames(simpleMethod, childElement, "property", "resource");
> -                    MiniLangValidate.requiredAttributes(simpleMethod, childElement, "property", "resource");
> -                    MiniLangValidate.constantAttributes(simpleMethod, childElement, "property", "resource");
> -                }
> -                this.messageFse = FlexibleStringExpander.getInstance(null);
> -                this.propertykey = childElement.getAttribute("property");
> -                this.propertyResource = childElement.getAttribute("resource");
> +    }
> +
> +    public void addMessage(List<Object> messages, ClassLoader loader, MethodContext methodContext) {
> +        String message = methodContext.expandString(this.message);
> +        String propertyResource = methodContext.expandString(this.propertyResource);
> +        if (!isProperty && message != null) {
> +            messages.add(message);
> +        } else if (isProperty && propertyResource != null && message != null) {
> +            String propMsg = UtilProperties.getMessage(propertyResource, message, methodContext.getEnvMap(), methodContext.getLocale());
> +            if (UtilValidate.isEmpty(propMsg)) {
> +                messages.add("Simple Method error occurred, but no message was found, sorry.");
>             } else {
> -                this.messageFse = FlexibleStringExpander.getInstance(null);
> -                this.propertykey = null;
> -                this.propertyResource = null;
> +                messages.add(methodContext.expandString(propMsg));
>             }
> +        } else {
> +            messages.add("Simple Method error occurred, but no message was found, sorry.");
>         }
>     }
> 
>     @Override
>     public boolean exec(MethodContext methodContext) throws MiniLangException {
> -        String message = null;
> -        if (!this.messageFse.isEmpty()) {
> -            message = this.messageFse.expandString(methodContext.getEnvMap());
> -        } else if (this.propertyResource != null) {
> -            message = UtilProperties.getMessage(this.propertyResource, this.propertykey, methodContext.getEnvMap(), methodContext.getLocale());
> -        }
> -        if (message != null) {
> -            List<String> messages = null;
> -            if (methodContext.getMethodType() == MethodContext.EVENT) {
> -                messages = methodContext.getEnv(this.simpleMethod.getEventErrorMessageListName());
> -                if (messages == null) {
> -                    messages = FastList.newInstance();
> -                    methodContext.putEnv(this.simpleMethod.getEventErrorMessageListName(), messages);
> -                }
> -            } else {
> -                messages = methodContext.getEnv(this.simpleMethod.getServiceErrorMessageListName());
> -                if (messages == null) {
> -                    messages = FastList.newInstance();
> -                    methodContext.putEnv(this.simpleMethod.getServiceErrorMessageListName(), messages);
> -                }
> -            }
> -            messages.add(message);
> -            // TODO: Remove this line after tests are fixed
> -            Debug.logInfo("<add-error> message = " + message + ", location = " + this.simpleMethod.getLocationAndName() + ", line = " + this.getLineNumber(), this.getClass().getName());
> +        List<Object> messages = errorListAcsr.get(methodContext);
> +        if (messages == null) {
> +            messages = FastList.newInstance();
> +            errorListAcsr.put(methodContext, messages);
>         }
> +        this.addMessage(messages, methodContext.getLoader(), methodContext);
>         return true;
>     }
> 
>     @Override
>     public String expandedString(MethodContext methodContext) {
> -        return FlexibleStringExpander.expandString(toString(), methodContext.getEnvMap());
> +        // TODO: something more than a stub/dummy
> +        return this.rawString();
>     }
> 
>     @Override
>     public String rawString() {
> -        return toString();
> -    }
> -
> -    @Override
> -    public String toString() {
> +        // TODO: something more than the empty tag
>         return "<add-error/>";
>     }
> 
> 
>