Re: svn commit: r1070230 - /ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java

[Jacques Le Roux]

Reverted at r1070282  for the trunk,  r1070284  for R10.04

I must say I was a was too quick on this one. I only trusted the idea and did 
not notice the defaultEmptyOK variable, sorry.

Jacques

From: "David E Jones" <d...@me.com>

Please revert this change.

Changing things this way completely breaks the whole idea of the "defaultEmptyOK" variable. If someone wants this to behave differently then they can change it locally (patch or subclass). It should NOT be changed in the project.

The whole idea with these validations is that you get one error if a field is empty (if it is not allowed to be empty) and another different error if the formatting is incorrect.

With this change it is now IMPOSSIBLE to have a field checked by these methods 
that is allowed to be empty.

The current code is not broken. The problem is a lack of understanding of how 
these validation methods are designed.

Jacques: please revert this and also please understand and test before committing contributed patches. Don't trust the description alone, look at the code and test it!

-David


On Feb 13, 2011, at 4:55 AM, jler...@apache.org wrote:

Author: jleroux
Date: Sun Feb 13 12:55:45 2011
New Revision: 1070230

URL: http://svn.apache.org/viewvc?rev=1070230&view=rev
Log:

A patch from Stephen Rufle "Blank year in UtilValidate.isYear should return false" (https://issues.apache.org/jira/browse/OFBIZ-4171) - OFBIZ-4171

UtilValidate.isYear returns true for a blank year which the calling function UtilValidate.isDate(String, String, String) tries to parse. This causes an exception to be thrown ValidateMethod.java:96 :ERROR] [ValidateMethod.exec] Error in validation method isDateAfterToday of class org.ofbiz.base.util.UtilValidate: null

I found this error when trying to add a new credit card in the eCommerce 
checkout flow.
  1. Add a product to the cart
  2. login as any user I used "admin"
  3. Checkout Step "Shipping Address" (Step 1: Where shall we ship it?)
         * Click Next
  4. Checkout Step "Shipping Options" (Step 2: How shall we ship it?)
         * Choose "UPS Air"
         * Click Next
  5. Checkout Step "Payment Options" (Step 3: How shall you pay?)
  6. Create "Credit Card"
         * Fill Name
         * Card Type "Visa"
         * Card Number "4111111111111111"
         * Expiration Date Month drop-down 01
         * Expiration Date Year drop-down leave blank
         * Choose billing address
         * Click "Save" button

Should see "Error in validation method isDateAfterToday of class 
org.ofbiz.base.util.UtilValidate: null"

My fix is to change isYear, isMonth, and isDay to return false when a blank 
value is entered.
After I make my change the message is "The expiration date is before today"

Modified:
   ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java

Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java

URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java?rev=1070230&r1=1070229&r2=1070230&view=diff

==============================================================================
--- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java 
(original)
+++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/UtilValidate.java Sun 
Feb 13 12:55:45 2011
@@ -746,7 +746,7 @@ public class UtilValidate {
     *  to use 4-digit year numbers everywhere.
     */
    public static boolean isYear(String s) {
-        if (isEmpty(s)) return defaultEmptyOK;
+        if (isEmpty(s)) return false;

        if (!isNonnegativeInteger(s)) return false;
        return ((s.length() == 2) || (s.length() == 4));
@@ -771,13 +771,13 @@ public class UtilValidate {

    /** isMonth returns true if string s is a valid month number between 1 and 
12. */
    public static boolean isMonth(String s) {
-        if (isEmpty(s)) return defaultEmptyOK;
+        if (isEmpty(s)) return false;
        return isIntegerInRange(s, 1, 12);
    }

    /** isDay returns true if string s is a valid day number between 1 and 31. 
*/
    public static boolean isDay(String s) {
-        if (isEmpty(s)) return defaultEmptyOK;
+        if (isEmpty(s)) return false;
        return isIntegerInRange(s, 1, 31);
    }