You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2022/05/26 12:55:56 UTC
[GitHub] [sling-org-apache-sling-event] joerghoh commented on a diff in pull request #6: [SLING-8582] provide some test coverage for Eventing's JobHandler
joerghoh commented on code in PR #6:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/6#discussion_r882640284
##########
src/main/java/org/apache/sling/event/impl/jobs/JobHandler.java:
##########
@@ -78,91 +78,77 @@ public boolean startProcessing(final Queue queue) {
* Reschedule the job
* Update the retry count and remove the started time.
* @return <code>true</code> if rescheduling was successful, <code>false</code> otherwise.
- */
+ */
public boolean reschedule() {
- final ResourceResolver resolver = this.configuration.createResourceResolver();
- try {
- final Resource jobResource = resolver.getResource(job.getResourcePath());
- if ( jobResource != null ) {
- final ModifiableValueMap mvm = jobResource.adaptTo(ModifiableValueMap.class);
- mvm.put(Job.PROPERTY_JOB_RETRY_COUNT, job.getProperty(Job.PROPERTY_JOB_RETRY_COUNT, Integer.class));
- if ( job.getProperty(Job.PROPERTY_RESULT_MESSAGE) != null ) {
- mvm.put(Job.PROPERTY_RESULT_MESSAGE, job.getProperty(Job.PROPERTY_RESULT_MESSAGE));
- }
- mvm.remove(Job.PROPERTY_JOB_STARTED_TIME);
- mvm.put(JobImpl.PROPERTY_JOB_QUEUED, Calendar.getInstance());
- try {
- resolver.commit();
- return true;
- } catch ( final PersistenceException pe ) {
- this.configuration.getMainLogger().debug("Unable to update reschedule properties for job " + job.getId(), pe);
- }
+ return withJobResource((jobResource,mvm) -> {
+ mvm.put(Job.PROPERTY_JOB_RETRY_COUNT, job.getProperty(Job.PROPERTY_JOB_RETRY_COUNT, Integer.class));
+ if ( job.getProperty(Job.PROPERTY_RESULT_MESSAGE) != null ) {
+ mvm.put(Job.PROPERTY_RESULT_MESSAGE, job.getProperty(Job.PROPERTY_RESULT_MESSAGE));
}
- } finally {
- resolver.close();
- }
-
- return false;
+ mvm.remove(Job.PROPERTY_JOB_STARTED_TIME);
+ mvm.put(JobImpl.PROPERTY_JOB_QUEUED, Calendar.getInstance());
+ try {
+ jobResource.getResourceResolver().commit();
+ return true;
+ } catch ( final PersistenceException pe ) {
+ this.configuration.getMainLogger().debug("Unable to update reschedule properties for job " + job.getId(), pe);
+ }
+ return false;
+ });
}
/**
* Finish a job.
* @param state The state of the processing
* @param keepJobInHistory whether to keep the job in the job history.
* @param duration the duration of the processing.
- */
- public void finished(final Job.JobState state,
- final boolean keepJobInHistory,
- final Long duration) {
+ */
+ public void finished(final Job.JobState state, final boolean keepJobInHistory, final Long duration) {
final boolean isSuccess = (state == Job.JobState.SUCCEEDED);
- final ResourceResolver resolver = this.configuration.createResourceResolver();
- try {
- final Resource jobResource = resolver.getResource(job.getResourcePath());
- if ( jobResource != null ) {
- try {
- String newPath = null;
- if ( keepJobInHistory ) {
- final ValueMap vm = ResourceHelper.getValueMap(jobResource);
- newPath = this.configuration.getStoragePath(job.getTopic(), job.getId(), isSuccess);
- final Map<String, Object> props = new HashMap<>(vm);
- props.put(JobImpl.PROPERTY_FINISHED_STATE, state.name());
- if ( isSuccess ) {
- // we set the finish date to start date + duration
- final Date finishDate = new Date();
- finishDate.setTime(job.getProcessingStarted().getTime().getTime() + duration);
- final Calendar finishCal = Calendar.getInstance();
- finishCal.setTime(finishDate);
- props.put(JobImpl.PROPERTY_FINISHED_DATE, finishCal);
- } else {
- // current time is good enough
- props.put(JobImpl.PROPERTY_FINISHED_DATE, Calendar.getInstance());
- }
- if ( job.getProperty(Job.PROPERTY_RESULT_MESSAGE) != null ) {
- props.put(Job.PROPERTY_RESULT_MESSAGE, job.getProperty(Job.PROPERTY_RESULT_MESSAGE));
- }
- ResourceHelper.getOrCreateResource(resolver, newPath, props);
+ withJobResource((jobResource,mvm) -> {
+ try {
+ ResourceResolver rr = jobResource.getResourceResolver();
+ String newPath = null;
+ if (keepJobInHistory) {
+ newPath = this.configuration.getStoragePath(job.getTopic(), job.getId(), isSuccess);
+ final Map<String, Object> props = new HashMap<>(mvm);
+ props.put(JobImpl.PROPERTY_FINISHED_STATE, state.name());
+ if (isSuccess) {
+ // we set the finish date to start date + duration
+ final Date finishDate = new Date();
+ finishDate.setTime(job.getProcessingStarted().getTime().getTime() + duration);
+ final Calendar finishCal = Calendar.getInstance();
+ finishCal.setTime(finishDate);
+ props.put(JobImpl.PROPERTY_FINISHED_DATE, finishCal);
+ } else {
+ // current time is good enough
+ props.put(JobImpl.PROPERTY_FINISHED_DATE, Calendar.getInstance());
+ }
+ if (job.getProperty(Job.PROPERTY_RESULT_MESSAGE) != null) {
+ props.put(Job.PROPERTY_RESULT_MESSAGE, job.getProperty(Job.PROPERTY_RESULT_MESSAGE));
}
- resolver.delete(jobResource);
- resolver.commit();
+ ResourceHelper.getOrCreateResource(rr, newPath, props);
+ }
+ rr.delete(jobResource);
+ rr.commit();
- if ( keepJobInHistory && configuration.getMainLogger().isDebugEnabled() ) {
- if ( isSuccess ) {
- configuration.getMainLogger().debug("Kept successful job {} at {}", Utility.toString(job), newPath);
- } else {
- configuration.getMainLogger().debug("Moved cancelled job {} to {}", Utility.toString(job), newPath);
- }
+ if (keepJobInHistory && configuration.getMainLogger().isDebugEnabled()) {
+ if (isSuccess) {
+ configuration.getMainLogger().debug("Kept successful job {} at {}", Utility.toString(job),
+ newPath);
+ } else {
+ configuration.getMainLogger().debug("Moved cancelled job {} to {}", Utility.toString(job),
+ newPath);
}
- } catch ( final PersistenceException pe ) {
- this.configuration.getMainLogger().warn("Unable to finish job " + job.getId(), pe);
- } catch (final InstantiationException ie) {
- // something happened with the resource in the meantime
- this.configuration.getMainLogger().debug("Unable to instantiate job", ie);
}
+ } catch (final PersistenceException pe) {
+ this.configuration.getMainLogger().warn("Unable to finish job " + job.getId(), pe);
}
- } finally {
- resolver.close();
- }
+ return false;
Review Comment:
```persistJobProperties``` and ```reschedule``` return a boolean, so it has to be a Function, if I want to use the same code in all cases.
I will add statements to the Lambdas to indicate that this value is actually ignored.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org