svn commit: r1558774 - /ofbiz/trunk/framework/service/src/org/ofbiz/service/job/JobManager.java

[adrianc]

Author: adrianc
Date: Thu Jan 16 12:48:44 2014
New Revision: 1558774

URL: http://svn.apache.org/r1558774
Log:
Small rewrite in JobManager.java to fix a bug where jobs could be duplicated under certain conditions. https://issues.apache.org/jira/browse/OFBIZ-5496

Modified:
    ofbiz/trunk/framework/service/src/org/ofbiz/service/job/JobManager.java

Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/job/JobManager.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/job/JobManager.java?rev=1558774&r1=1558773&r2=1558774&view=diff
==============================================================================
--- ofbiz/trunk/framework/service/src/org/ofbiz/service/job/JobManager.java (original)
+++ ofbiz/trunk/framework/service/src/org/ofbiz/service/job/JobManager.java Thu Jan 16 12:48:44 2014
@@ -21,6 +21,7 @@ package org.ofbiz.service.job;
 import java.io.IOException;
 import java.sql.Timestamp;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
@@ -154,11 +155,10 @@ public final class JobManager {
         // The rest of this method logs exceptions and does not throw them.
         // The idea is to keep the JobPoller working even when a database
         // connection is not available (possible on a saturated server).
-        List<Job> poll = new ArrayList<Job>(limit);
         DispatchContext dctx = getDispatcher().getDispatchContext();
         if (dctx == null) {
             Debug.logWarning("Unable to locate DispatchContext object; not running job!", module);
-            return poll;
+            return Collections.emptyList();
         }
         // basic query
         List<EntityExpr> expressions = UtilMisc.toList(EntityCondition.makeCondition("runTime", EntityOperator.LESS_THAN_EQUAL_TO, UtilDateTime.nowTimestamp()),
@@ -171,7 +171,7 @@ public final class JobManager {
             pools = getRunPools();
         } catch (GenericConfigException e) {
             Debug.logWarning(e, "Unable to get run pools - not running job: ", module);
-            return poll;
+            return Collections.emptyList();
         }
         List<EntityExpr> poolsExpr = UtilMisc.toList(EntityCondition.makeCondition("poolId", EntityOperator.EQUALS, null));
         if (!pools.isEmpty()) {
@@ -179,6 +179,7 @@ public final class JobManager {
                 poolsExpr.add(EntityCondition.makeCondition("poolId", EntityOperator.EQUALS, poolName));
             }
         }
+        List<Job> poll = new ArrayList<Job>(limit);
         // make the conditions
         EntityCondition baseCondition = EntityCondition.makeCondition(expressions);
         EntityCondition poolCondition = EntityCondition.makeCondition(poolsExpr, EntityOperator.OR);
@@ -188,7 +189,7 @@ public final class JobManager {
         try {
             beganTransaction = TransactionUtil.begin();
             if (!beganTransaction) {
-                Debug.logWarning("Unable to poll JobSandbox for jobs; transaction was not started by this process", module);
+                Debug.logWarning("Unable to poll JobSandbox for jobs; unable to begin transaction.", module);
                 return poll;
             }
             jobsIterator = delegator.find("JobSandbox", mainCondition, null, null, UtilMisc.toList("runTime"), null);
@@ -205,15 +206,16 @@ public final class JobManager {
                 }
                 jobValue = jobsIterator.next();
             }
+            TransactionUtil.commit(beganTransaction);
         } catch (Throwable t) {
-            poll.clear();
-            String errMsg =  "Exception thrown while polling JobSandbox: ";
-            Debug.logWarning(t, errMsg, module);
+            String errMsg = "Exception thrown while polling JobSandbox: ";
             try {
-                TransactionUtil.rollback(beganTransaction, errMsg + t.getMessage(), t);
+                TransactionUtil.rollback(beganTransaction, errMsg, t);
             } catch (GenericEntityException e) {
                 Debug.logWarning(e, "Exception thrown while rolling back transaction: ", module);
             }
+            Debug.logWarning(t, errMsg, module);
+            return Collections.emptyList();
         } finally {
             if (jobsIterator != null) {
                 try {
@@ -222,11 +224,6 @@ public final class JobManager {
                     Debug.logWarning(e, module);
                 }
             }
-            try {
-                TransactionUtil.commit(beganTransaction);
-            } catch (GenericTransactionException e) {
-                Debug.logWarning(e, "Transaction error trying to commit when polling and updating the JobSandbox: ", module);
-            }
         }
         if (poll.isEmpty()) {
             // No jobs to run, see if there are any jobs to purge
@@ -236,7 +233,7 @@ public final class JobManager {
                 cal.add(Calendar.DAY_OF_YEAR, -daysToKeep);
             } catch (GenericConfigException e) {
                 Debug.logWarning(e, "Unable to get purge job days: ", module);
-                return poll;
+                return Collections.emptyList();
             }
             Timestamp purgeTime = new Timestamp(cal.getTimeInMillis());
             List<EntityExpr> finExp = UtilMisc.toList(EntityCondition.makeCondition("finishDateTime", EntityOperator.NOT_EQUAL, null), EntityCondition.makeCondition("finishDateTime", EntityOperator.LESS_THAN, purgeTime));
@@ -248,8 +245,8 @@ public final class JobManager {
             try {
                 beganTransaction = TransactionUtil.begin();
                 if (!beganTransaction) {
-                    Debug.logWarning("Unable to poll JobSandbox for jobs; transaction was not started by this process", module);
-                    return poll;
+                    Debug.logWarning("Unable to poll JobSandbox for jobs; unable to begin transaction.", module);
+                    return Collections.emptyList();
                 }
                 jobsIterator = delegator.find("JobSandbox", mainCondition, null, null, UtilMisc.toList("jobId"), null);
                 GenericValue jobValue = jobsIterator.next();
@@ -260,15 +257,16 @@ public final class JobManager {
                     }
                     jobValue = jobsIterator.next();
                 }
+                TransactionUtil.commit(beganTransaction);
             } catch (Throwable t) {
-                poll.clear();
-                String errMsg =  "Exception thrown while polling JobSandbox: ";
-                Debug.logWarning(t, errMsg, module);
+                String errMsg = "Exception thrown while polling JobSandbox: ";
                 try {
-                    TransactionUtil.rollback(beganTransaction, errMsg + t.getMessage(), t);
+                    TransactionUtil.rollback(beganTransaction, errMsg, t);
                 } catch (GenericEntityException e) {
                     Debug.logWarning(e, "Exception thrown while rolling back transaction: ", module);
                 }
+                Debug.logWarning(t, errMsg, module);
+                return Collections.emptyList();
             } finally {
                 if (jobsIterator != null) {
                     try {
@@ -277,11 +275,6 @@ public final class JobManager {
                         Debug.logWarning(e, module);
                     }
                 }
-                try {
-                    TransactionUtil.commit(beganTransaction);
-                } catch (GenericTransactionException e) {
-                    Debug.logWarning(e, "Transaction error trying to commit when polling the JobSandbox: ", module);
-                }
             }
         }
         return poll;