svn commit: r696134 - in /ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar: TemporalExpression.java TemporalExpressions.java

[adrianc]

Author: adrianc
Date: Tue Sep 16 19:39:19 2008
New Revision: 696134

URL: http://svn.apache.org/viewvc?rev=696134&view=rev
Log:
Safety/sanity checks for the temporal expression Java code.

Yes, there is millisecond arithmetic in this code. No, I haven't changed my view on it - millisecond arithmetic is still Evil. In this case I'm just using it to get a java.util.Calendar instance in the neighborhood of where I need it.

Modified:
    ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpression.java
    ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java

Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpression.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpression.java?rev=696134&r1=696133&r2=696134&view=diff
==============================================================================
--- ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpression.java (original)
+++ ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpression.java Tue Sep 16 19:39:19 2008
@@ -96,6 +96,10 @@
         return set;
     }
 
+    protected boolean containsExpression(TemporalExpression expression) {
+        return false;
+    }
+
     protected Calendar setStartOfDay(Calendar cal) {
         cal.set(Calendar.HOUR_OF_DAY, 0);
         cal.set(Calendar.MINUTE, 0);

Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java?rev=696134&r1=696133&r2=696134&view=diff
==============================================================================
--- ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java (original)
+++ ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java Tue Sep 16 19:39:19 2008
@@ -56,6 +56,9 @@
                 throw new IllegalArgumentException("expressionSet argument cannot be null");
             }
             this.expressionSet = expressionSet;
+            if (containsExpression(this)) {
+                throw new IllegalArgumentException("recursive expression");
+            }
             if (this.expressionSet.size() > 0) {
                 TemporalExpression that = this.expressionSet.iterator().next();
                 if (this.compareTo(that) > 0) {
@@ -126,6 +129,15 @@
             }
             return finalSet;
         }
+
+        protected boolean containsExpression(TemporalExpression expression) {
+            for (TemporalExpression setItem : this.expressionSet) {
+                if (setItem.containsExpression(expression)) {
+                    return true;
+                }
+            }
+            return false;
+        }
     }
 
     /** This class represents a mathematical intersection of all of its
@@ -139,7 +151,15 @@
             if (expressionSet == null) {
                 throw new IllegalArgumentException("expressionSet argument cannot be null");
             }
+            for (TemporalExpression that : expressionSet) {
+                if (this == that) {
+                    throw new IllegalArgumentException("recursive expression");
+                }
+            }
             this.expressionSet = expressionSet;
+            if (containsExpression(this)) {
+                throw new IllegalArgumentException("recursive expression");
+            }
             if (this.expressionSet.size() > 0) {
                 TemporalExpression that = this.expressionSet.iterator().next();
                 if (this.compareTo(that) > 0) {
@@ -235,6 +255,15 @@
             }
             return finalSet;
         }
+
+        protected boolean containsExpression(TemporalExpression expression) {
+            for (TemporalExpression setItem : this.expressionSet) {
+                if (setItem.containsExpression(expression)) {
+                    return true;
+                }
+            }
+            return false;
+        }
     }
 
     /** This class represents a difference of two temporal expressions. */
@@ -246,11 +275,11 @@
             if (included == null) {
                 throw new IllegalArgumentException("included argument cannot be null");
             }
-            if (excluded == null) {
-                throw new IllegalArgumentException("excluded argument cannot be null");
-            }
             this.included = included;
             this.excluded = excluded;
+            if (containsExpression(this)) {
+                throw new IllegalArgumentException("recursive expression");
+            }
             if (this.compareTo(included) > 0) {
                 this.sequence = included.sequence;
                 this.subSequence = included.subSequence;
@@ -307,6 +336,10 @@
             }
             return finalSet;
         }
+
+        protected boolean containsExpression(TemporalExpression expression) {
+            return this.included.containsExpression(expression) || this.excluded.containsExpression(expression);
+        }
     }
 
     /** A temporal expression that represents a range of dates. */
@@ -648,33 +681,19 @@
 
         public boolean includesDate(Calendar cal) {
             int dom = cal.get(Calendar.DAY_OF_MONTH);
-            int end = this.end;
-            if (cal.getActualMaximum(Calendar.DAY_OF_MONTH) < end) {
-                end = cal.getActualMaximum(Calendar.DAY_OF_MONTH);
-            }
-            return dom >= this.start && dom <= end;
+            return dom >= this.start && dom <= this.end;
         }
 
         public Calendar first(Calendar cal) {
-            int month = cal.get(Calendar.MONTH);
-            Calendar first = (Calendar) cal.clone();
-            if (first.get(Calendar.DAY_OF_MONTH) > first.getActualMaximum(Calendar.DAY_OF_MONTH)) {
-                first.set(Calendar.DAY_OF_MONTH, first.getActualMaximum(Calendar.DAY_OF_MONTH));
-            }
-            setStartOfDay(first);
+            Calendar first = setStartOfDay((Calendar) cal.clone());
             while (!includesDate(first)) {
                 first.add(Calendar.DAY_OF_MONTH, 1);
-                if (first.get(Calendar.MONTH) != month) {
-                    first.set(Calendar.MONTH, month);
-                    first.set(Calendar.DAY_OF_MONTH, 1);
-                }
             }
             return first;
         }
 
         public Calendar next(Calendar cal) {
-            Calendar next = (Calendar) cal.clone();
-            setStartOfDay(next);
+            Calendar next = setStartOfDay((Calendar) cal.clone());
             next.add(Calendar.DAY_OF_MONTH, 1);
             while (!includesDate(next)) {
                 next.add(Calendar.DAY_OF_MONTH, 1);
@@ -737,9 +756,7 @@
 
         public Calendar first(Calendar cal) {
             int month = cal.get(Calendar.MONTH);
-            Calendar first = (Calendar) cal.clone();
-            alignDayOfWeek(first);
-            setStartOfDay(first);
+            Calendar first = setStartOfDay(alignDayOfWeek((Calendar) cal.clone()));
             if (first.before(cal)) {
                 first.set(Calendar.DAY_OF_MONTH, 1);
                 if (first.get(Calendar.MONTH) == month) {
@@ -752,9 +769,7 @@
 
         public Calendar next(Calendar cal) {
             int month = cal.get(Calendar.MONTH);
-            Calendar next = (Calendar) cal.clone();
-            alignDayOfWeek(next);
-            setStartOfDay(next);
+            Calendar next = setStartOfDay(alignDayOfWeek((Calendar) cal.clone()));
             if (next.before(cal) || next.equals(cal)) {
                 next.set(Calendar.DAY_OF_MONTH, 1);
                 if (next.get(Calendar.MONTH) == month) {
@@ -765,7 +780,7 @@
             return next;
         }
 
-        protected void alignDayOfWeek(Calendar cal) {
+        protected Calendar alignDayOfWeek(Calendar cal) {
             cal.set(Calendar.DAY_OF_MONTH, 1);
             if (this.occurrence > 0) {
                 while (cal.get(Calendar.DAY_OF_WEEK) != this.dayOfWeek) {
@@ -780,6 +795,7 @@
                 }
                 cal.add(Calendar.DAY_OF_MONTH, (this.occurrence + 1) * 7);
             }
+            return cal;
         }
     }
 
@@ -840,8 +856,7 @@
         }
 
         public Calendar first(Calendar cal) {
-            Calendar first = (Calendar) cal.clone();
-            first.setTime(this.start);
+            Calendar first = prepareCal(cal);
             while (first.before(cal)) {
                 first.add(this.freqType, this.freqCount);
             }
@@ -849,12 +864,41 @@
         }
 
         public Calendar next(Calendar cal) {
-            Calendar next = (Calendar) cal.clone();
-            next.setTime(this.start);
-            while (next.before(cal) || next.equals(cal)) {
+            Calendar next = first(cal);
+            if (next.equals(cal)) {
                 next.add(this.freqType, this.freqCount);
             }
             return next;
         }
+
+        protected Calendar prepareCal(Calendar cal) {
+            // Performs a "sane" skip forward in time - avoids time consuming loops
+            // like incrementing every second from Jan 1 2000 until today
+            Calendar skip = (Calendar) cal.clone();
+            skip.setTime(this.start);
+            long deltaMillis = cal.getTimeInMillis() - this.start.getTime();
+            if (deltaMillis < 1000) {
+                return skip;
+            }
+            long divisor = deltaMillis;
+            if (this.freqType == Calendar.DAY_OF_MONTH) {
+                divisor = 86400000;
+            } else if (this.freqType == Calendar.HOUR) {
+                divisor = 3600000;
+            } else if (this.freqType == Calendar.MINUTE) {
+                divisor = 60000;
+            } else if (this.freqType == Calendar.SECOND) {
+                divisor = 1000;
+            } else {
+                return skip;
+            }
+            float units = deltaMillis / divisor;
+            units = (units / this.freqCount) * this.freqCount;
+            skip.add(this.freqType, (int)units);
+            while (skip.after(cal)) {
+                skip.add(this.freqType, -this.freqCount);
+            }
+            return skip;
+        }
     }
 }