svn commit: r1331802 - in /ofbiz/trunk/framework/minilang: dtd/simple-methods-v2.xsd src/org/ofbiz/minilang/method/callops/AddError.java src/org/ofbiz/minilang/method/callops/CheckErrors.java

[adrianc]

Author: adrianc
Date: Sat Apr 28 18:48:42 2012
New Revision: 1331802

URL: http://svn.apache.org/viewvc?rev=1331802&view=rev
Log:
Overhauled Mini-language <check-errors> and <add-error> elements.

This commit fixes a bug in the <add-error> element. Some of the tests written in Mini-language fail now because errors were previously ignored, but now the error handling works properly.

To assist the developer community with fixing the tests, I added an INFO log message to track down the faulty test code. Run the tests, and then search the log file for "<add-error>".

Modified:
    ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd
    ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/AddError.java
    ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CheckErrors.java

Modified: ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd?rev=1331802&r1=1331801&r2=1331802&view=diff
==============================================================================
--- ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd (original)
+++ ofbiz/trunk/framework/minilang/dtd/simple-methods-v2.xsd Sat Apr 28 18:48:42 2012
@@ -3560,42 +3560,25 @@ under the License.
     <xs:element name="check-errors" substitutionGroup="ControlOperations">
         <xs:annotation>
             <xs:documentation>
-                The message lists from invoking are not checked until the check-errors tag is used.
-                The named list is checked and if it contains any messages they are put in the servlet
-                request object and the specified error code is returned to the control servlet.
+                Halts script execution if the error message list contains any messages.
             </xs:documentation>
         </xs:annotation>
         <xs:complexType>
-            <xs:sequence>
-                <xs:element minOccurs="0" ref="error-prefix"/>
-                <xs:element minOccurs="0" ref="error-suffix"/>
-                <xs:element minOccurs="0" ref="message-prefix"/>
-                <xs:element minOccurs="0" ref="message-suffix"/>
-            </xs:sequence>
-            <xs:attributeGroup ref="attlist.check-errors"/>
+            <xs:attribute type="xs:string" name="error-code">
+                <xs:annotation>
+                    <xs:documentation>
+                    The error code to return to the calling process. Defaults to "error".
+                    &lt;br/&gt;&lt;br/&gt;
+                    Optional. Attribute type: constant+expr
+                    </xs:documentation>
+                </xs:annotation>
+            </xs:attribute>
         </xs:complexType>
     </xs:element>
-    <xs:attributeGroup name="attlist.check-errors">
-        <xs:attribute type="xs:string" name="error-code" default="error">
-             <xs:annotation>
-                <xs:documentation>
-                    Defaults to "error".
-                </xs:documentation>
-            </xs:annotation>
-        </xs:attribute>
-        <xs:attribute type="xs:string" name="error-list-name" default="error_list">
-            <xs:annotation>
-                <xs:documentation>
-                    The name of the list in the method environment to check for error messages.
-                    Defaults to "error_list".
-                </xs:documentation>
-            </xs:annotation>
-        </xs:attribute>
-    </xs:attributeGroup>
     <xs:element name="add-error" substitutionGroup="ControlOperations">
         <xs:annotation>
             <xs:documentation>
-                Adds an error message with to the given error list from either an inline message or a message from a properties file.
+                Adds an error message to the error message list.
             </xs:documentation>
         </xs:annotation>
         <xs:complexType>
@@ -3603,19 +3586,8 @@ under the License.
                 <xs:element ref="fail-message"/>
                 <xs:element ref="fail-property"/>
             </xs:choice>
-            <xs:attributeGroup ref="attlist.add-error"/>
         </xs:complexType>
     </xs:element>
-    <xs:attributeGroup name="attlist.add-error">
-        <xs:attribute type="xs:string" name="error-list-name" default="error_list">
-            <xs:annotation>
-                <xs:documentation>
-                    The name of the list in the method environment to check for error messages.
-                    Defaults to "error_list".
-                </xs:documentation>
-            </xs:annotation>
-        </xs:attribute>
-    </xs:attributeGroup>
     <xs:element name="break" substitutionGroup="ControlOperations">
         <xs:annotation>
             <xs:documentation>
@@ -5305,22 +5277,21 @@ under the License.
     <xs:element name="fail-message">
         <xs:annotation>
             <xs:documentation>
-                Used to specify an inline message. Has one attribute called 'message'.
+                Used to specify an error message.
             </xs:documentation>
         </xs:annotation>
         <xs:complexType>
-            <xs:attributeGroup ref="attlist.fail-message"/>
+            <xs:attribute type="xs:string" name="message" use="required">
+                <xs:annotation>
+                    <xs:documentation>
+                        The error message text.
+                        &lt;br/&gt;&lt;br/&gt;
+                        Required. Attribute type: constant+expr
+                    </xs:documentation>
+                </xs:annotation>
+            </xs:attribute>
         </xs:complexType>
     </xs:element>
-    <xs:attributeGroup name="attlist.fail-message">
-        <xs:attribute type="xs:string" name="message" use="required">
-            <xs:annotation>
-                <xs:documentation>
-                    Literal or flexible string using ${} syntax.
-                </xs:documentation>
-            </xs:annotation>
-        </xs:attribute>
-    </xs:attributeGroup>
     <xs:element name="fail-property">
         <xs:annotation>
             <xs:documentation>
@@ -5329,23 +5300,24 @@ under the License.
             </xs:documentation>
         </xs:annotation>
         <xs:complexType>
-            <xs:attributeGroup ref="attlist.fail-property"/>
+            <xs:attribute type="xs:string" name="resource" use="required">
+                <xs:annotation>
+                    <xs:documentation>
+                        The name of a properties file on the classpath.
+                        &lt;br/&gt;&lt;br/&gt;
+                        Required. Attribute type: constant
+                    </xs:documentation>
+                </xs:annotation>
+            </xs:attribute>
+            <xs:attribute type="xs:string" name="property" use="required">
+                <xs:annotation>
+                    <xs:documentation>
+                        The property key.
+                        &lt;br/&gt;&lt;br/&gt;
+                        Required. Attribute type: constant
+                    </xs:documentation>
+                </xs:annotation>
+            </xs:attribute>
         </xs:complexType>
     </xs:element>
-    <xs:attributeGroup name="attlist.fail-property">
-        <xs:attribute type="xs:string" name="resource" use="required">
-            <xs:annotation>
-                <xs:documentation>
-                    Name of a properties file on the classpath.
-                </xs:documentation>
-            </xs:annotation>
-        </xs:attribute>
-        <xs:attribute type="xs:string" name="property" use="required">
-            <xs:annotation>
-                <xs:documentation>
-                    Name of the property in the properties file qualifed by 'resource'.
-                </xs:documentation>
-            </xs:annotation>
-        </xs:attribute>
-    </xs:attributeGroup>
 </xs:schema>

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=1331802&r1=1331801&r2=1331802&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 18:48:42 2012
@@ -22,78 +22,118 @@ 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 the fail-message or fail-property value to the error-list.
+ * Adds an error message to the error message list.
  */
-public class AddError extends MethodOperation {
+public final class AddError extends MethodOperation {
 
-    ContextAccessor<List<Object>> errorListAcsr;
-    boolean isProperty = false;
-    String message = null;
-    String propertyResource = null;
+    // 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;
 
     public AddError(Element element, SimpleMethod simpleMethod) throws MiniLangException {
         super(element, simpleMethod);
-        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;
+        if (MiniLangValidate.validationOn()) {
+            MiniLangValidate.childElements(simpleMethod, element, "fail-message", "fail-property");
+            MiniLangValidate.requireAnyChildElement(simpleMethod, element, "fail-message", "fail-property");
         }
-    }
-
-    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 {
-                messages.add(methodContext.expandString(propMsg));
+        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 {
-            messages.add("Simple Method error occurred, but no message was found, sorry.");
+            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");
+            } else {
+                this.messageFse = FlexibleStringExpander.getInstance(null);
+                this.propertykey = null;
+                this.propertyResource = null;
+            }
         }
     }
 
     @Override
     public boolean exec(MethodContext methodContext) throws MiniLangException {
-        List<Object> messages = errorListAcsr.get(methodContext);
-        if (messages == null) {
-            messages = FastList.newInstance();
-            errorListAcsr.put(methodContext, messages);
+        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());
         }
-        this.addMessage(messages, methodContext.getLoader(), methodContext);
         return true;
     }
 
     @Override
     public String expandedString(MethodContext methodContext) {
-        // TODO: something more than a stub/dummy
-        return this.rawString();
+        return FlexibleStringExpander.expandString(toString(), methodContext.getEnvMap());
     }
 
     @Override
     public String rawString() {
-        // TODO: something more than the empty tag
+        return toString();
+    }
+
+    @Override
+    public String toString() {
         return "<add-error/>";
     }
 

Modified: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CheckErrors.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CheckErrors.java?rev=1331802&r1=1331801&r2=1331802&view=diff
==============================================================================
--- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CheckErrors.java (original)
+++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/callops/CheckErrors.java Sat Apr 28 18:48:42 2012
@@ -20,58 +20,58 @@ package org.ofbiz.minilang.method.callop
 
 import java.util.List;
 
-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;
 
 /**
- * An event operation that checks a message list and may introduce a return code and stop the event
+ * Halts script execution if the error message list contains any messages.
  */
-public class CheckErrors extends MethodOperation {
+public final class CheckErrors extends MethodOperation {
 
-    String errorCode;
-    ContextAccessor<List<Object>> errorListAcsr;
-    FlexibleMessage errorPrefix;
-    FlexibleMessage errorSuffix;
-    FlexibleMessage messagePrefix;
-    FlexibleMessage messageSuffix;
+    // 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 errorCodeFse;
 
     public CheckErrors(Element element, SimpleMethod simpleMethod) throws MiniLangException {
         super(element, simpleMethod);
-        errorCode = element.getAttribute("error-code");
-        if (UtilValidate.isEmpty(errorCode))
-            errorCode = "error";
-        errorListAcsr = new ContextAccessor<List<Object>>(element.getAttribute("error-list-name"), "error_list");
-        errorPrefix = new FlexibleMessage(UtilXml.firstChildElement(element, "error-prefix"), "check.error.prefix");
-        errorSuffix = new FlexibleMessage(UtilXml.firstChildElement(element, "error-suffix"), "check.error.suffix");
-        messagePrefix = new FlexibleMessage(UtilXml.firstChildElement(element, "message-prefix"), "check.message.prefix");
-        messageSuffix = new FlexibleMessage(UtilXml.firstChildElement(element, "message-suffix"), "check.message.suffix");
+        if (MiniLangValidate.validationOn()) {
+            MiniLangValidate.attributeNames(simpleMethod, element, "error-code");
+            MiniLangValidate.constantPlusExpressionAttributes(simpleMethod, element, "error-code");
+            MiniLangValidate.noChildElements(simpleMethod, element);
+        }
+        boolean elementModified = autoCorrect(element);
+        if (elementModified && MiniLangUtil.autoCorrectOn()) {
+            MiniLangUtil.flagDocumentAsCorrected(element);
+        }
+        this.errorCodeFse = FlexibleStringExpander.getInstance(element.getAttribute("error-code"));
     }
 
     @Override
     public boolean exec(MethodContext methodContext) throws MiniLangException {
-        List<Object> messages = errorListAcsr.get(methodContext);
-        if (UtilValidate.isNotEmpty(messages)) {
-            String errorCode = methodContext.expandString(this.errorCode);
-            if (methodContext.getMethodType() == MethodContext.EVENT) {
-                /*
-                 * The OLD way, now puts formatting control in the template... String errMsg = errorPrefix.getMessage(methodContext.getLoader(), methodContext) + ServiceUtil.makeMessageList(messages,
-                 * messagePrefix.getMessage(methodContext.getLoader(), methodContext), messageSuffix.getMessage(methodContext.getLoader(), methodContext)) +
-                 * errorSuffix.getMessage(methodContext.getLoader(), methodContext); methodContext.putEnv(simpleMethod.getEventErrorMessageName(), errMsg);
-                 */
-                methodContext.putEnv(simpleMethod.getEventErrorMessageListName(), messages);
-                methodContext.putEnv(simpleMethod.getEventResponseCodeName(), errorCode);
+        if (methodContext.getMethodType() == MethodContext.EVENT) {
+            List<Object> messages = methodContext.getEnv(this.simpleMethod.getEventErrorMessageListName());
+            if (messages != null && messages.size() > 0) {
+                methodContext.putEnv(this.simpleMethod.getEventResponseCodeName(), getErrorCode(methodContext));
                 return false;
-            } else if (methodContext.getMethodType() == MethodContext.SERVICE) {
-                methodContext.putEnv(simpleMethod.getServiceErrorMessageListName(), messages);
-                methodContext.putEnv(simpleMethod.getServiceResponseMessageName(), errorCode);
-                return false;
-            } else {
+            }
+        } else {
+            List<Object> messages = methodContext.getEnv(this.simpleMethod.getServiceErrorMessageListName());
+            if (messages != null && messages.size() > 0) {
+                methodContext.putEnv(this.simpleMethod.getServiceResponseMessageName(), getErrorCode(methodContext));
                 return false;
             }
         }
@@ -80,14 +80,30 @@ public class CheckErrors extends MethodO
 
     @Override
     public String expandedString(MethodContext methodContext) {
-        // TODO: something more than a stub/dummy
-        return this.rawString();
+        return FlexibleStringExpander.expandString(toString(), methodContext.getEnvMap());
+    }
+
+    private String getErrorCode(MethodContext methodContext) {
+        String errorCode = this.errorCodeFse.expandString(methodContext.getEnvMap());
+        if (errorCode.length() == 0) {
+            errorCode = this.simpleMethod.getDefaultErrorCode();
+        }
+        return errorCode;
     }
 
     @Override
     public String rawString() {
-        // TODO: something more than the empty tag
-        return "<check-errors/>";
+        return toString();
+    }
+
+    @Override
+    public String toString() {
+        StringBuilder sb = new StringBuilder("<check-errors ");
+        if (!this.errorCodeFse.isEmpty()) {
+            sb.append("error-code=\"").append(this.errorCodeFse).append("\" ");
+        }
+        sb.append("/>");
+        return sb.toString();
     }
 
     public static final class CheckErrorsFactory implements Factory<CheckErrors> {