This is an automated email from the ASF dual-hosted git repository. mleila pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push: new b80e411bb8 Fixed: RuntimeData and RecurrenceInfo not cleaned after job purge (OFBIZ-12679) b80e411bb8 is described below commit b80e411bb827134e995bdaeb7857feb07c6beb28 Author: MLeila <mle...@apache.org> AuthorDate: Thu Dec 22 12:14:17 2022 +0100 Fixed: RuntimeData and RecurrenceInfo not cleaned after job purge (OFBIZ-12679) Add jobSandbox related runtimeDataId and recurrenceInfoId fields when searching for jobs to purge so that the PurgeJob can retrieve and clean job related entities (JobUtil.removeJob) Also add a test that validate the complete purge Thanks: Gaƫtan Chaboussie for report and corrective patch --- .../ofbiz/service/test/ServicePurgeTest.groovy | 38 ++++++++++++++++++++ .../org/apache/ofbiz/service/job/JobManager.java | 42 +++++++++++++--------- .../org/apache/ofbiz/service/job/JobServices.java | 25 ++----------- framework/service/testdef/servicetests.xml | 4 +++ 4 files changed, 70 insertions(+), 39 deletions(-) diff --git a/framework/service/src/main/groovy/org/apache/ofbiz/service/test/ServicePurgeTest.groovy b/framework/service/src/main/groovy/org/apache/ofbiz/service/test/ServicePurgeTest.groovy new file mode 100644 index 0000000000..1537fe2d4f --- /dev/null +++ b/framework/service/src/main/groovy/org/apache/ofbiz/service/test/ServicePurgeTest.groovy @@ -0,0 +1,38 @@ +package org.apache.ofbiz.service + +import org.apache.ofbiz.base.util.UtilDateTime +import org.apache.ofbiz.entity.GenericValue +import org.apache.ofbiz.entity.util.EntityQuery +import org.apache.ofbiz.service.config.ServiceConfigUtil +import org.apache.ofbiz.testtools.GroovyScriptTestCase + +class ServicePurgeTest extends GroovyScriptTestCase { + +// ./gradlew 'ofbiz -t component=service -t suitename=servicetests -t case=service-purge-test' --debug-jvm + + void testRuntimeDataIsCleanedAfterServicePurge() { + GenericValue sysUserLogin = delegator.findOne('UserLogin', true, 'userLoginId', 'system') + String jobId = delegator.getNextSeqId('JobSandbox') + + def createRuntimeResult = dispatcher.runSync('createRuntimeData', [ + runtimeInfo: 'This is a runtimeInfo', + userLogin : sysUserLogin + ]) + String runtimeDataId = createRuntimeResult.runtimeDataId + + dispatcher.runSync('createJobSandbox', [ + userLogin : sysUserLogin, + poolId : ServiceConfigUtil.getServiceEngine().getThreadPool().getSendToPool(), + jobId : jobId, + runtimeDataId : runtimeDataId, + statusId : 'SERVICE_FINISHED', + serviceName : 'sendMail', + finishDateTime: UtilDateTime.addDaysToTimestamp(UtilDateTime.nowTimestamp(), -10) + ]) + + dispatcher.runSync('purgeOldJobs', [userLogin: sysUserLogin]) + + assert EntityQuery.use(delegator).from('JobSandbox').where('jobId', jobId).queryCount() == 0 + assert EntityQuery.use(delegator).from('RuntimeData').where('runtimeDataId', runtimeDataId).queryCount() == 0 + } +} diff --git a/framework/service/src/main/java/org/apache/ofbiz/service/job/JobManager.java b/framework/service/src/main/java/org/apache/ofbiz/service/job/JobManager.java index 3016474812..ce82e0bcfd 100644 --- a/framework/service/src/main/java/org/apache/ofbiz/service/job/JobManager.java +++ b/framework/service/src/main/java/org/apache/ofbiz/service/job/JobManager.java @@ -253,16 +253,6 @@ public final class JobManager { Debug.logWarning(e, "Unable to get purge job days: ", MODULE); return Collections.emptyList(); } - List<EntityCondition> purgeCondition = UtilMisc.toList( - EntityCondition.makeCondition("runByInstanceId", INSTANCE_ID), - EntityCondition.makeCondition(UtilMisc.toList( - EntityCondition.makeCondition(UtilMisc.toList( - EntityCondition.makeCondition("finishDateTime", EntityOperator.NOT_EQUAL, null), - EntityCondition.makeCondition("finishDateTime", EntityOperator.LESS_THAN, purgeTime))), - EntityCondition.makeCondition(UtilMisc.toList( - EntityCondition.makeCondition("cancelDateTime", EntityOperator.NOT_EQUAL, null), - EntityCondition.makeCondition("cancelDateTime", EntityOperator.LESS_THAN, purgeTime)))), - EntityOperator.OR)); beganTransaction = false; try { beganTransaction = TransactionUtil.begin(); @@ -270,12 +260,8 @@ public final class JobManager { Debug.logWarning("Unable to poll JobSandbox for jobs; unable to begin transaction.", MODULE); return Collections.emptyList(); } - List<GenericValue> jobs = EntityQuery.use(delegator).from("JobSandbox") - .where(purgeCondition) - .select("jobId") - .maxRows(limit) - .queryList(); - jobs.forEach(jobValue -> poll.add(new PurgeJob(jobValue))); + getJobsToPurge(delegator, null, INSTANCE_ID, limit, purgeTime) + .forEach(jobValue -> poll.add(new PurgeJob(jobValue))); TransactionUtil.commit(beganTransaction); } catch (Throwable t) { String errMsg = "Exception thrown while polling JobSandbox: "; @@ -291,6 +277,30 @@ public final class JobManager { return poll; } + public static List<GenericValue> getJobsToPurge(Delegator delegator, String poolId, String instanceId, int limit, Timestamp purgeTime) + throws GenericEntityException { + List<EntityCondition> purgeCondition = UtilMisc.toList( + EntityCondition.makeCondition(UtilMisc.toList( + EntityCondition.makeCondition(UtilMisc.toList( + EntityCondition.makeCondition("finishDateTime", EntityOperator.NOT_EQUAL, null), + EntityCondition.makeCondition("finishDateTime", EntityOperator.LESS_THAN, purgeTime))), + EntityCondition.makeCondition(UtilMisc.toList( + EntityCondition.makeCondition("cancelDateTime", EntityOperator.NOT_EQUAL, null), + EntityCondition.makeCondition("cancelDateTime", EntityOperator.LESS_THAN, purgeTime)))), + EntityOperator.OR)); + if (UtilValidate.isNotEmpty(instanceId)) { + purgeCondition.add(EntityCondition.makeCondition("runByInstanceId", instanceId)); + } + if (UtilValidate.isNotEmpty(poolId)) { + purgeCondition.add(EntityCondition.makeCondition("poolId", poolId)); + } + return EntityQuery.use(delegator).from("JobSandbox") + .where(purgeCondition) + .select("jobId", "runtimeDataId", "recurrenceInfoId") + .maxRows(limit) + .queryList(); + } + public synchronized void reloadCrashedJobs() { assertIsRunning(); if (crashedJobsReloaded) { diff --git a/framework/service/src/main/java/org/apache/ofbiz/service/job/JobServices.java b/framework/service/src/main/java/org/apache/ofbiz/service/job/JobServices.java index 4a459e8ab8..3b82354a17 100644 --- a/framework/service/src/main/java/org/apache/ofbiz/service/job/JobServices.java +++ b/framework/service/src/main/java/org/apache/ofbiz/service/job/JobServices.java @@ -21,7 +21,6 @@ package org.apache.ofbiz.service.job; import java.sql.Timestamp; import java.time.Duration; import java.time.Instant; -import java.util.List; import java.util.Locale; import java.util.Map; import org.apache.ofbiz.base.config.GenericConfigException; @@ -32,8 +31,6 @@ import org.apache.ofbiz.base.util.UtilProperties; import org.apache.ofbiz.entity.Delegator; import org.apache.ofbiz.entity.GenericEntityException; import org.apache.ofbiz.entity.GenericValue; -import org.apache.ofbiz.entity.condition.EntityCondition; -import org.apache.ofbiz.entity.condition.EntityOperator; import org.apache.ofbiz.entity.util.EntityQuery; import org.apache.ofbiz.security.Security; import org.apache.ofbiz.service.DispatchContext; @@ -126,27 +123,9 @@ public class JobServices { } Delegator delegator = dctx.getDelegator(); Timestamp purgeTime = Timestamp.from(Instant.now().minus(Duration.ofDays(daysToKeep))); - - // create the conditions to query - List<EntityCondition> purgeCondition = UtilMisc.toList( - EntityCondition.makeCondition("poolId", sendPool), - EntityCondition.makeCondition(UtilMisc.toList( - EntityCondition.makeCondition(UtilMisc.toList( - EntityCondition.makeCondition("finishDateTime", EntityOperator.NOT_EQUAL, null), - EntityCondition.makeCondition("finishDateTime", EntityOperator.LESS_THAN, purgeTime))), - EntityCondition.makeCondition(UtilMisc.toList( - EntityCondition.makeCondition("cancelDateTime", EntityOperator.NOT_EQUAL, null), - EntityCondition.makeCondition("cancelDateTime", EntityOperator.LESS_THAN, purgeTime)))), - EntityOperator.OR)); - - EntityQuery jobQuery = EntityQuery.use(delegator).from("JobSandbox") - .where(purgeCondition) - .select("jobId"); - if (limit != null) { - jobQuery.maxRows(limit); - } try { - jobQuery.queryList().forEach(JobUtil::removeJob); + JobManager.getJobsToPurge(delegator, sendPool, null, limit, purgeTime) + .forEach(JobUtil::removeJob); } catch (GenericEntityException e) { Debug.logWarning(e, MODULE); } diff --git a/framework/service/testdef/servicetests.xml b/framework/service/testdef/servicetests.xml index 630085844e..1dad1a6031 100644 --- a/framework/service/testdef/servicetests.xml +++ b/framework/service/testdef/servicetests.xml @@ -83,4 +83,8 @@ under the License. <junit-test-suite class-name="org.apache.ofbiz.service.test.ServicePermissionTests"/> </test-case> + <test-case case-name="service-purge-test"> + <groovy-test-suite name="ServicePurgeTest" location="component://service/src/main/groovy/org/apache/ofbiz/service/test/ServicePurgeTest.groovy"/> + </test-case> + </test-suite>