Skip to content

Commit

Permalink
Cleanup some stuff (#894)
Browse files Browse the repository at this point in the history
  • Loading branch information
rdehuyss committed Dec 13, 2023
1 parent d54569e commit c211a78
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ public void runOnJobProcessingFilters() {
}

public void runOnJobProcessingSucceededFilters() {
jobServerFilters().forEach(catchThrowable(jobServerFilter -> jobServerFilter.onProcessed(job)));
jobServerFilters().forEach(catchThrowable(jobServerFilter -> jobServerFilter.onProcessingSucceeded(job)));
}

Expand Down
10 changes: 0 additions & 10 deletions core/src/main/java/org/jobrunr/jobs/filters/JobServerFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,6 @@ public interface JobServerFilter extends JobFilter {
default void onProcessing(Job job) {
}

/**
* This hook is called when the Job processing succeeded (note that the job still has the <code>PROCESSING</code> state).
*
* @param job the job that has been processed successfully.
* @deprecated Please use {@link JobServerFilter#onProcessingSucceeded(Job)}
*/
@Deprecated
default void onProcessed(Job job) {
}

/**
* This hook is called when the Job processing succeeded (note that the job still has the <code>PROCESSING</code> state).
*
Expand Down
12 changes: 2 additions & 10 deletions core/src/main/java/org/jobrunr/jobs/filters/RetryFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import org.jobrunr.jobs.Job;
import org.jobrunr.jobs.states.FailedState;
import org.jobrunr.jobs.states.JobState;
import org.jobrunr.utils.JobUtils;

import static java.time.Instant.now;
import static org.jobrunr.jobs.states.StateName.FAILED_STATES;
Expand Down Expand Up @@ -79,14 +78,7 @@ private boolean isNotFailed(JobState newState) {
}

private int getMaxNumberOfRetries(Job job) {
if(job.getAmountOfRetries() != null) return job.getAmountOfRetries();
return getMaxNumberOfRetriesViaJobAnnotation(job);
}

@Deprecated // this will be removed in JobRunr 6
private int getMaxNumberOfRetriesViaJobAnnotation(Job job) {
return JobUtils.getJobAnnotation(job.getJobDetails())
.map(jobAnnotation -> jobAnnotation.retries() > org.jobrunr.jobs.annotations.Job.NBR_OF_RETRIES_NOT_PROVIDED ? jobAnnotation.retries() : null)
.orElse(this.numberOfRetries);
if (job.getAmountOfRetries() != null) return job.getAmountOfRetries();
return this.numberOfRetries;
}
}
11 changes: 1 addition & 10 deletions core/src/main/java/org/jobrunr/server/runner/MockJobContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,10 @@ public class MockJobContext {
private MockJobContext() {}

public static void setUpJobContextForJob(Job job) {
MockJobContext.setJobContext(new RunnerJobContext(job));
MockJobContext.setUpJobContext(new RunnerJobContext(job));
}

public static void setUpJobContext(JobContext jobContext) {
ThreadLocalJobContext.setJobContext(jobContext);
}

/**
* @param jobContext the JobContext to setup
* @deprecated use {@link MockJobContext#setUpJobContext(JobContext)}
*/
@Deprecated
public static void setJobContext(JobContext jobContext) {
ThreadLocalJobContext.setJobContext(jobContext);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,20 @@

import java.util.Map;

import static org.assertj.core.api.Assertions.*;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.jobrunr.jobs.JobDetailsTestBuilder.classThatDoesNotExistJobDetails;
import static org.jobrunr.jobs.JobDetailsTestBuilder.methodThatDoesNotExistJobDetails;
import static org.jobrunr.jobs.JobTestBuilder.*;
import static org.jobrunr.jobs.states.StateName.*;
import static org.jobrunr.jobs.JobTestBuilder.aFailedJob;
import static org.jobrunr.jobs.JobTestBuilder.aJobInProgress;
import static org.jobrunr.jobs.JobTestBuilder.anEnqueuedJob;
import static org.jobrunr.jobs.states.StateName.DELETED;
import static org.jobrunr.jobs.states.StateName.ENQUEUED;
import static org.jobrunr.jobs.states.StateName.FAILED;
import static org.jobrunr.jobs.states.StateName.PROCESSING;
import static org.jobrunr.jobs.states.StateName.SCHEDULED;
import static org.jobrunr.jobs.states.StateName.SUCCEEDED;

class JobPerformingFiltersTest {

Expand Down Expand Up @@ -63,8 +72,7 @@ void ifOtherFilterIsProvidedItIsUsed() {

assertThat(metadata)
.containsKey("onStateApplied")
.containsKey("onProcessing")
.containsKey("onProcessed");
.containsKey("onProcessing");
}

@Test
Expand Down
30 changes: 24 additions & 6 deletions core/src/test/java/org/jobrunr/jobs/filters/RetryFilterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,14 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.jobrunr.JobRunrException.problematicConfigurationException;
import static org.jobrunr.jobs.JobTestBuilder.*;
import static org.jobrunr.jobs.states.StateName.*;
import static org.jobrunr.jobs.JobTestBuilder.aCopyOf;
import static org.jobrunr.jobs.JobTestBuilder.aFailedJob;
import static org.jobrunr.jobs.JobTestBuilder.aFailedJobWithRetries;
import static org.jobrunr.jobs.JobTestBuilder.aJob;
import static org.jobrunr.jobs.JobTestBuilder.anEnqueuedJob;
import static org.jobrunr.jobs.states.StateName.ENQUEUED;
import static org.jobrunr.jobs.states.StateName.FAILED;
import static org.jobrunr.jobs.states.StateName.SCHEDULED;

class RetryFilterTest {

Expand All @@ -21,8 +27,9 @@ void setupRetryFilter() {
}

@Test
void skipsIfTestIsNotFailed() {
void skipsIfStateIsNotFailed() {
final Job job = anEnqueuedJob().build();
applyDefaultJobFilter(job);
int beforeVersion = job.getJobStates().size();

retryFilter.onStateElection(job, job.getJobState());
Expand All @@ -33,8 +40,9 @@ void skipsIfTestIsNotFailed() {
}

@Test
void retryFilterSchedulesJobAgainIfItIsFailed() {
void retryFilterSchedulesJobAgainIfStateIsFailed() {
final Job job = aFailedJob().build();
applyDefaultJobFilter(job);
int beforeVersion = job.getJobStates().size();

retryFilter.onStateElection(job, job.getJobState());
Expand All @@ -45,11 +53,12 @@ void retryFilterSchedulesJobAgainIfItIsFailed() {
}

@Test
void retryFilterSchedulesJobAgainIfItIsFailedButMaxNumberOfRetriesIsNotReached() {
void retryFilterSchedulesJobAgainIfStateIsFailedButMaxNumberOfRetriesIsNotReached() {
final Job job = aJob()
.<TestService>withJobDetails(ts -> ts.doWorkThatFails())
.withState(new FailedState("a message", new RuntimeException("boem")))
.build();
applyDefaultJobFilter(job);
int beforeVersion = job.getJobStates().size();

retryFilter.onStateElection(job, job.getJobState());
Expand All @@ -67,6 +76,7 @@ void retryFilterDoesNotScheduleJobAgainIfMaxNumberOfRetriesIsReached() {
.withState(new FailedState("a message", new RuntimeException("boom")))
.withState(new FailedState("firstRetry", new RuntimeException("boom")))
.build();
applyDefaultJobFilter(job);
int beforeVersion = job.getJobStates().size();

retryFilter.onStateElection(job, job.getJobState());
Expand All @@ -79,6 +89,7 @@ void retryFilterDoesNotScheduleJobAgainIfMaxNumberOfRetriesIsReached() {
@Test
void retryFilterDoesNotScheduleJobAgainIfTheExceptionIsProblematic() {
final Job job = aFailedJob().withState(new FailedState("a message", problematicConfigurationException("big problem"))).build();
applyDefaultJobFilter(job);
int beforeVersion = job.getJobStates().size();

retryFilter.onStateElection(job, job.getJobState());
Expand All @@ -91,6 +102,7 @@ void retryFilterDoesNotScheduleJobAgainIfTheExceptionIsProblematic() {
@Test
void retryFilterDoesNotScheduleJobAgainIfItHasFailed10Times() {
final Job job = aFailedJobWithRetries().build();
applyDefaultJobFilter(job);
int beforeVersion = job.getJobStates().size();

retryFilter.onStateElection(job, job.getJobState());
Expand All @@ -102,11 +114,11 @@ void retryFilterDoesNotScheduleJobAgainIfItHasFailed10Times() {

@Test
void retryFilterKeepsDefaultRetryFilterValueOf10IfRetriesOnJobAnnotationIsNotProvided() {

final Job job = aJob()
.<TestService>withJobDetails(ts -> ts.doWork())
.withState(new FailedState("a message", new RuntimeException("boom")))
.build();
applyDefaultJobFilter(job);
int beforeVersion = job.getJobStates().size();

retryFilter.onStateElection(job, job.getJobState());
Expand All @@ -123,6 +135,7 @@ void retryFilterKeepsDefaultGivenRetryFilterValueIfRetriesOnJobAnnotationIsNotPr
.<TestService>withJobDetails(ts -> ts.doWork())
.withState(new FailedState("a message", new RuntimeException("boom")))
.build();
applyDefaultJobFilter(job);
int beforeVersion = job.getJobStates().size();

retryFilter.onStateElection(job, job.getJobState());
Expand All @@ -141,6 +154,7 @@ void retryFilterUsesValueOfRetriesOnJobAnnotationIfProvided() {
.<TestService>withJobDetails(ts -> ts.doWorkThatFails())
.withState(new FailedState("a message", new RuntimeException("boom")))
.build();
applyDefaultJobFilter(job);
int beforeVersion = job.getJobStates().size();

// WHEN
Expand All @@ -165,4 +179,8 @@ void retryFilterUsesValueOfRetriesOnJobAnnotationIfProvided() {
assertThat(afterVersion).isEqualTo(beforeVersion);
assertThat(job.getState()).isEqualTo(FAILED);
}

void applyDefaultJobFilter(Job job) {
new DefaultJobFilter().onCreating(job);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,19 @@
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.awaitility.Awaitility.await;
import static org.awaitility.Durations.*;
import static org.awaitility.Durations.FIVE_SECONDS;
import static org.awaitility.Durations.ONE_MINUTE;
import static org.awaitility.Durations.TEN_SECONDS;
import static org.jobrunr.JobRunrAssertions.assertThat;
import static org.jobrunr.jobs.JobDetailsTestBuilder.classThatDoesNotExistJobDetails;
import static org.jobrunr.jobs.JobDetailsTestBuilder.methodThatDoesNotExistJobDetails;
import static org.jobrunr.jobs.JobTestBuilder.anEnqueuedJob;
import static org.jobrunr.jobs.states.StateName.*;
import static org.jobrunr.jobs.states.StateName.DELETED;
import static org.jobrunr.jobs.states.StateName.ENQUEUED;
import static org.jobrunr.jobs.states.StateName.FAILED;
import static org.jobrunr.jobs.states.StateName.PROCESSING;
import static org.jobrunr.jobs.states.StateName.SCHEDULED;
import static org.jobrunr.jobs.states.StateName.SUCCEEDED;
import static org.jobrunr.scheduling.JobBuilder.aJob;
import static org.jobrunr.scheduling.RecurringJobBuilder.aRecurringJob;
import static org.jobrunr.server.BackgroundJobServerConfiguration.usingStandardBackgroundJobServerConfiguration;
Expand Down Expand Up @@ -607,7 +614,6 @@ void jobRunrCallsJobFiltersUsingGlobalFilter() {
assertThat(logAllStateChangesFilter.onCreatingIsCalled(jobId)).isTrue();
assertThat(logAllStateChangesFilter.onCreatedIsCalled(jobId)).isTrue();
assertThat(logAllStateChangesFilter.onProcessingIsCalled(jobId)).isTrue();
assertThat(logAllStateChangesFilter.onProcessedIsCalled(jobId)).isTrue();
}

interface SomeJobInterface {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@
import org.jobrunr.jobs.filters.JobServerFilter;
import org.jobrunr.jobs.states.JobState;

import java.util.*;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;

public class LogAllStateChangesFilter implements ApplyStateFilter, JobClientFilter, JobServerFilter {

private final Map<String, Boolean> onCreatingIsCalled;
private final Map<String, Boolean> onCreatedIsCalled;
private final Map<UUID, Boolean> onProcessingIsCalled;
private final Map<UUID, Boolean> onProcessedIsCalled;
private final Map<UUID, Boolean> onProcessingSucceededIsCalled;
private final Map<UUID, Boolean> onProcessingFailedIsCalled;
private final Map<UUID, Boolean> onFailedAfterRetriesIsCalled;
Expand All @@ -25,7 +28,6 @@ public LogAllStateChangesFilter() {
onCreatingIsCalled = new HashMap<>();
onCreatedIsCalled = new HashMap<>();
onProcessingIsCalled = new HashMap<>();
onProcessedIsCalled = new HashMap<>();
onProcessingSucceededIsCalled = new HashMap<>();
onProcessingFailedIsCalled = new HashMap<>();
onFailedAfterRetriesIsCalled = new HashMap<>();
Expand Down Expand Up @@ -58,11 +60,6 @@ public void onProcessing(Job job) {
this.onProcessingIsCalled.put(job.getId(), true);
}

@Override
public void onProcessed(Job job) {
this.onProcessedIsCalled.put(job.getId(), true);
}

@Override
public void onProcessingSucceeded(Job job) {
this.onProcessingSucceededIsCalled.put(job.getId(), true);
Expand Down Expand Up @@ -98,18 +95,6 @@ public boolean onProcessingIsCalled(UUID jobId) {
return this.onProcessingIsCalled.getOrDefault(jobId, false);
}

public boolean onProcessedIsCalled(Job job) {
return onProcessedIsCalled(job.getId());
}

public boolean onProcessedIsCalled(JobId jobId) {
return onProcessedIsCalled(jobId.asUUID());
}

public boolean onProcessedIsCalled(UUID jobId) {
return this.onProcessedIsCalled.getOrDefault(jobId, false);
}

public boolean onProcessingSucceededIsCalled(Job job) {
return onProcessingSucceededIsCalled(job.getId());
}
Expand Down Expand Up @@ -162,7 +147,6 @@ public void clear() {
onCreatingIsCalled.clear();
onCreatedIsCalled.clear();
onProcessingIsCalled.clear();
onProcessedIsCalled.clear();
onProcessingSucceededIsCalled.clear();
onProcessingFailedIsCalled.clear();
onFailedAfterRetriesIsCalled.clear();
Expand Down
8 changes: 3 additions & 5 deletions core/src/testFixtures/java/org/jobrunr/stubs/TestService.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
import java.util.concurrent.TimeUnit;

import static org.assertj.core.api.Assertions.assertThat;
import static org.jobrunr.jobs.states.StateName.*;
import static org.jobrunr.jobs.states.StateName.ENQUEUED;
import static org.jobrunr.jobs.states.StateName.FAILED;
import static org.jobrunr.jobs.states.StateName.PROCESSING;

public class TestService implements TestServiceInterface {

Expand Down Expand Up @@ -443,10 +445,6 @@ public void onProcessing(org.jobrunr.jobs.Job job) {
job.getMetadata().put("onProcessing", "");
}

@Override
public void onProcessed(org.jobrunr.jobs.Job job) {
job.getMetadata().put("onProcessed", "");
}
}

public interface Command<T> {
Expand Down

0 comments on commit c211a78

Please sign in to comment.