Skip to content

Commit

Permalink
fix: runStage null to non-null or cancel via exception (#17846)
Browse files Browse the repository at this point in the history
  • Loading branch information
jbee committed Jun 20, 2024
1 parent 67977c0 commit d918acb
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,24 @@ default void endingProcess(boolean success) {
void startingStage(@Nonnull String description, int workItems, @Nonnull FailurePolicy onFailure)
throws CancellationException;

/**
* Should be called after a {@link #runStage(Callable)} or {@link #runStage(Object, Callable)} or
* on of the other variants in case the returned value may be null but never should be null in
* order to be able to continue the process.
*
* @param value a value returned by a {@code runStage} method that might be null
* @return the same value but only if it is non-null
* @param <T> type of the checked value
* @throws CancellationException in case the value is null, this is similar to starting the
* cancellation based exception that would occur by starting the next stage except that it
* also has a message indicating that a failed post condition was the cause
*/
@Nonnull
default <T> T nonNullStagePostCondition(@CheckForNull T value) throws CancellationException {
if (value == null) throw new CancellationException("Post-condition was null");
return value;
}

default void startingStage(@Nonnull String description, int workItems) {
startingStage(description, workItems, FailurePolicy.PARENT);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,7 @@ private void addError(
public void startingProcess(String description, Object... args) {
observer.run();

if (isCancelled()) {
throw cancellationException();
}
if (isCancelled()) throw cancellationException(false);
String message = format(description, args);
tracker.startingProcess(format(message, args));
incompleteProcess.set(null);
Expand All @@ -211,10 +209,13 @@ public void startingProcess(String description, Object... args) {
}

@Nonnull
private RuntimeException cancellationException() {
private RuntimeException cancellationException(boolean failedPostCondition) {
Exception cause = getCause();
if (skipRecording && cause instanceof RuntimeException rex) throw rex;
CancellationException ex = new CancellationException();
CancellationException ex =
failedPostCondition
? new CancellationException("Non-null post-condition failed")
: new CancellationException();
ex.initCause(cause);
return ex;
}
Expand Down Expand Up @@ -275,16 +276,22 @@ public void startingStage(
@Nonnull String description, int workItems, @Nonnull FailurePolicy onFailure) {
observer.run();

if (isCancelled()) {
throw cancellationException();
}
if (isCancelled()) throw cancellationException(false);
skipCurrentStage.set(false);
tracker.startingStage(description, workItems);
Stage stage =
addStageRecord(getOrAddLastIncompleteProcess(), description, workItems, onFailure);
logInfo(stage, "", description);
}

@Nonnull
@Override
public <T> T nonNullStagePostCondition(@CheckForNull T value) throws CancellationException {
observer.run();
if (value == null) throw cancellationException(true);
return value;
}

@Override
public void completedStage(String summary, Object... args) {
observer.run();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ public void execute(JobConfiguration jobConfig, JobProgress progress) {

progress.startingStage("Loading file resource");
FileResource data =
progress.runStage(() -> fileResourceService.getFileResource(jobConfig.getUid()));
progress.nonNullStagePostCondition(
progress.runStage(() -> fileResourceService.getFileResource(jobConfig.getUid())));

progress.startingStage("Loading file content");
try (InputStream input =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ public void execute(JobConfiguration jobId, JobProgress progress) {
ImportOptions options = (ImportOptions) jobId.getJobParameters();
progress.startingStage("Loading file resource");
FileResource data =
progress.runStage(() -> fileResourceService.getFileResource(jobId.getUid()));
progress.nonNullStagePostCondition(
progress.runStage(() -> fileResourceService.getFileResource(jobId.getUid())));
progress.startingStage("Loading file content");
try (InputStream input =
progress.runStage(() -> fileResourceService.getFileResourceContent(data))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.google.common.base.Enums;
import java.util.List;
import java.util.Map;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -99,14 +100,17 @@ public ImportReport importMetadata(
handleDeprecationIfEventReport(bundleParams);

progress.startingStage("Creating bundle");
ObjectBundle bundle = progress.runStage(() -> objectBundleService.create(bundleParams));
ObjectBundle bundle =
progress.nonNullStagePostCondition(
progress.runStage(() -> objectBundleService.create(bundleParams)));

progress.startingStage("Running postCreateBundle");
progress.runStage(() -> postCreateBundle(bundle, bundleParams));

progress.startingStage("Validating bundle");
ObjectBundleValidationReport validationReport =
progress.runStage(() -> objectBundleValidationService.validate(bundle));
progress.nonNullStagePostCondition(
progress.runStage(() -> objectBundleValidationService.validate(bundle)));
ImportReport report = new ImportReport();
report.setImportParams(params);
report.setStatus(Status.OK);
Expand Down Expand Up @@ -241,7 +245,7 @@ private boolean getBooleanWithDefault(

String value = String.valueOf(parameters.get(key).get(0));

return "true".equals(value.toLowerCase());
return "true".equalsIgnoreCase(value);
}

private <T extends Enum<T>> T getEnumWithDefault(
Expand Down Expand Up @@ -276,8 +280,8 @@ private void preCreateBundleObject(BaseIdentifiableObject object, ObjectBundlePa
object.setLastUpdatedBy(params.getUser());
}

private void postCreateBundle(ObjectBundle bundle, ObjectBundleParams params) {
if (bundle.getUser() == null) {
private void postCreateBundle(@CheckForNull ObjectBundle bundle, ObjectBundleParams params) {
if (bundle == null || bundle.getUser() == null) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,21 @@ public ImportReport importTracker(

jobProgress.startingStage("Running PreHeat");
TrackerBundle trackerBundle =
jobProgress.runStage(() -> trackerBundleService.create(params, trackerObjects, user));
jobProgress.nonNullStagePostCondition(
jobProgress.runStage(() -> trackerBundleService.create(params, trackerObjects, user)));

jobProgress.startingStage("Calculating Payload Size");
Map<TrackerType, Integer> bundleSize =
jobProgress.runStage(() -> calculatePayloadSize(trackerBundle));
jobProgress.nonNullStagePostCondition(
jobProgress.runStage(() -> calculatePayloadSize(trackerBundle)));

jobProgress.startingStage("Running PreProcess");
jobProgress.runStage(() -> trackerPreprocessService.preprocess(trackerBundle));

jobProgress.startingStage("Running Validation");
ValidationResult validationResult = jobProgress.runStage(() -> validateBundle(trackerBundle));
ValidationResult validationResult =
jobProgress.nonNullStagePostCondition(
jobProgress.runStage(() -> validateBundle(trackerBundle)));

ValidationReport validationReport = ValidationReport.fromResult(validationResult);

Expand All @@ -103,7 +107,8 @@ public ImportReport importTracker(

jobProgress.startingStage("Running Rule Engine Validation");
ValidationResult result =
jobProgress.runStage(() -> validationService.validateRuleEngine(trackerBundle));
jobProgress.nonNullStagePostCondition(
jobProgress.runStage(() -> validationService.validateRuleEngine(trackerBundle)));
trackerBundle.setTrackedEntities(result.getTrackedEntities());
trackerBundle.setEnrollments(result.getEnrollments());
trackerBundle.setEvents(result.getEvents());
Expand All @@ -118,7 +123,9 @@ public ImportReport importTracker(
}

jobProgress.startingStage("Commit Transaction");
PersistenceReport persistenceReport = jobProgress.runStage(() -> commit(params, trackerBundle));
PersistenceReport persistenceReport =
jobProgress.nonNullStagePostCondition(
jobProgress.runStage(() -> commit(params, trackerBundle)));

jobProgress.startingStage("PostCommit");
jobProgress.runStage(() -> trackerBundleService.postCommit(trackerBundle));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ public void execute(JobConfiguration config, JobProgress progress) {
MetadataImportParams params = (MetadataImportParams) config.getJobParameters();
progress.startingStage("Loading file resource");
FileResource data =
progress.runStage(() -> fileResourceService.getExistingFileResource(config.getUid()));
progress.nonNullStagePostCondition(
progress.runStage(() -> fileResourceService.getExistingFileResource(config.getUid())));
progress.startingStage("Loading file content");
try (InputStream input =
progress.runStage(() -> fileResourceService.getFileResourceContent(data))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ public void execute(JobConfiguration config, JobProgress progress) {
TrackerImportParams params = (TrackerImportParams) config.getJobParameters();
progress.startingStage("Loading file resource");
FileResource data =
progress.runStage(() -> fileResourceService.getExistingFileResource(config.getUid()));
progress.nonNullStagePostCondition(
progress.runStage(() -> fileResourceService.getExistingFileResource(config.getUid())));
progress.startingStage("Loading file content");
try (InputStream input =
progress.runStage(() -> fileResourceService.getFileResourceContent(data))) {
Expand Down

0 comments on commit d918acb

Please sign in to comment.