From cebd5bbdbbfe6e0ab3cf417f6c65cfb7322949ad Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Wed, 1 Nov 2023 12:46:30 +0100 Subject: [PATCH 01/10] fix: pickup manually run jobs that have been executed before [DHIS2-15027] --- .../org/hisp/dhis/scheduling/HibernateJobConfigurationStore.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/HibernateJobConfigurationStore.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/HibernateJobConfigurationStore.java index 5f39fee72d6b..c4a93d3f9425 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/HibernateJobConfigurationStore.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/HibernateJobConfigurationStore.java @@ -207,7 +207,6 @@ public Stream getDueJobConfigurations(boolean includeWaiting) where enabled = true and jobstatus = 'SCHEDULED' and (queueposition is null or queueposition = 0 or schedulingtype = 'ONCE_ASAP') - and (schedulingtype != 'ONCE_ASAP' or lastfinished is null) and (:waiting = true or not exists ( select 1 from jobconfiguration j2 where j2.jobtype = j1.jobtype From 8b78cf96361cbe50d20d2e88fed699bad366ae36 Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Thu, 2 Nov 2023 13:54:33 +0100 Subject: [PATCH 02/10] feat: persistent errors for job processing [DHIS2-15276] --- .../org/hisp/dhis/feedback/ErrorMessage.java | 36 +++--- .../org/hisp/dhis/feedback/ErrorReport.java | 115 ++++-------------- .../org/hisp/dhis/message/MessageService.java | 3 +- .../hisp/dhis/preheat/PreheatErrorReport.java | 2 +- .../dhis/scheduling/JobConfiguration.java | 4 + .../scheduling/JobConfigurationService.java | 6 +- .../scheduling/JobConfigurationStore.java | 6 +- .../org/hisp/dhis/scheduling/JobProgress.java | 103 +++++++++++++--- .../dhis/scheduling/JobSchedulerService.java | 4 + .../org/hisp/dhis/scheduling/JobType.java | 9 +- .../event/data/DefaultQueryItemLocator.java | 7 +- .../org/hisp/dhis/config/StartupConfig.java | 6 +- .../dhis/message/DefaultMessageService.java | 6 +- .../DefaultJobConfigurationService.java | 13 +- .../DefaultJobSchedulerLoopService.java | 39 +++++- .../DefaultJobSchedulerService.java | 19 ++- .../hisp/dhis/scheduling/HeartbeatJob.java | 2 +- .../HibernateJobConfigurationStore.java | 36 +++++- .../hisp/dhis/scheduling/JobScheduler.java | 7 ++ .../scheduling/JobSchedulerLoopService.java | 6 + .../dhis/scheduling/RecordingJobProgress.java | 24 +++- .../org/hisp/dhis/startup/SchedulerStart.java | 3 - .../hibernate/JobConfiguration.hbm.xml | 3 + .../DefaultMetadataWorkflowService.java | 4 +- ...2_41_32__job_configuation_errors_in_db.sql | 11 ++ .../metadata/MetadataImportJob.java | 13 ++ .../JobConfigurationController.java | 6 + 27 files changed, 307 insertions(+), 186 deletions(-) create mode 100644 dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.41/V2_41_32__job_configuation_errors_in_db.sql diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorMessage.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorMessage.java index 222e69cf96f3..180bb1dcba94 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorMessage.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorMessage.java @@ -30,41 +30,37 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import java.text.MessageFormat; +import java.util.List; +import java.util.stream.Stream; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; +import lombok.Getter; +import lombok.ToString; /** * @author Morten Olav Hansen */ +@Getter +@ToString public class ErrorMessage { - private final ErrorCode errorCode; - private final Object[] args; - - private final String message; + @JsonProperty private final ErrorCode errorCode; + @JsonProperty private final List args; + @JsonProperty private String message; public ErrorMessage(ErrorCode errorCode, Object... args) { this.errorCode = errorCode; - this.args = args; + this.args = Stream.of(args).map(Object::toString).toList(); this.message = MessageFormat.format(errorCode.getMessage(), this.args); } @JsonCreator public ErrorMessage( - @JsonProperty("message") String message, @JsonProperty("errorCode") ErrorCode errorCode) { + @Nonnull @JsonProperty("message") String message, + @Nonnull @JsonProperty("errorCode") ErrorCode errorCode, + @CheckForNull @JsonProperty("args") List args) { this.errorCode = errorCode; - this.args = null; + this.args = args == null ? List.of() : args; this.message = message; } - - public ErrorCode getErrorCode() { - return errorCode; - } - - public String getMessage() { - return message; - } - - @Override - public String toString() { - return String.format("[%s: '%s']", errorCode.name(), message); - } } diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorReport.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorReport.java index 32729ce25797..99b48f670998 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorReport.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorReport.java @@ -29,133 +29,66 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty; -import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlRootElement; -import com.google.common.base.MoreObjects; -import java.util.ArrayList; -import java.util.Arrays; import java.util.List; -import org.hisp.dhis.common.DxfNamespaces; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; +import lombok.Getter; +import lombok.Setter; +import lombok.ToString; +import lombok.experimental.Accessors; /** * @author Morten Olav Hansen */ -@JacksonXmlRootElement(localName = "errorReport", namespace = DxfNamespaces.DXF_2_0) +@ToString +@Getter +@Setter +@Accessors(chain = true) public class ErrorReport { - protected final ErrorMessage message; - protected final Class mainKlass; - - protected String mainId; - - protected Class errorKlass; - - protected String errorProperty; - - protected List errorProperties = new ArrayList<>(); - - protected Object value; + private final ErrorMessage message; + @JsonProperty private final Class mainKlass; + @JsonProperty private String mainId; + @JsonProperty private Class errorKlass; + @JsonProperty private String errorProperty; + @Nonnull @JsonProperty private List errorProperties; + @JsonProperty private Object value; public ErrorReport(Class mainKlass, ErrorCode errorCode, Object... args) { this.mainKlass = mainKlass; this.message = new ErrorMessage(errorCode, args); - this.errorProperties.addAll(Arrays.asList(args)); + this.errorProperties = List.of(args); } public ErrorReport(Class mainKlass, ErrorMessage message) { this.mainKlass = mainKlass; this.message = message; + this.errorProperties = message.getArgs(); } @JsonCreator public ErrorReport( @JsonProperty("message") String message, + @CheckForNull @JsonProperty("args") List args, @JsonProperty("mainKlass") Class mainKlass, @JsonProperty("errorCode") ErrorCode errorCode) { this.mainKlass = mainKlass; - this.message = new ErrorMessage(message, errorCode); + this.message = new ErrorMessage(message, errorCode, args); + this.errorProperties = args == null ? List.of() : args; } @JsonProperty - @JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0) public ErrorCode getErrorCode() { return message.getErrorCode(); } @JsonProperty - @JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0) public String getMessage() { return message.getMessage(); } @JsonProperty - @JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0) - public Class getMainKlass() { - return mainKlass; - } - - @JsonProperty - @JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0) - public String getMainId() { - return mainId; - } - - public ErrorReport setMainId(String mainId) { - this.mainId = mainId; - return this; - } - - @JsonProperty - @JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0) - public Class getErrorKlass() { - return errorKlass; - } - - public ErrorReport setErrorKlass(Class errorKlass) { - this.errorKlass = errorKlass; - return this; - } - - @JsonProperty - @JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0) - public String getErrorProperty() { - return errorProperty; - } - - public ErrorReport setErrorProperty(String errorProperty) { - this.errorProperty = errorProperty; - return this; - } - - @JsonProperty - @JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0) - public List getErrorProperties() { - return errorProperties; - } - - public void setErrorProperties(List errorProperties) { - this.errorProperties = errorProperties; - } - - @JsonProperty - @JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0) - public Object getValue() { - return value; - } - - public ErrorReport setValue(Object value) { - this.value = value; - return this; - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("message", getMessage()) - .add("errorCode", message.getErrorCode()) - .add("mainKlass", mainKlass) - .add("errorKlass", errorKlass) - .add("value", value) - .toString(); + public List getArgs() { + return message.getArgs(); } } diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageService.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageService.java index 4b9011600d9f..21dd74fd09a5 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageService.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageService.java @@ -30,6 +30,7 @@ import java.util.Collection; import java.util.List; import java.util.Set; +import javax.annotation.Nonnull; import org.hisp.dhis.dataset.CompleteDataSetRegistration; import org.hisp.dhis.fileresource.FileResource; import org.hisp.dhis.user.User; @@ -56,7 +57,7 @@ long sendValidationMessage( long sendMessage(MessageConversationParams params); - long sendSystemErrorNotification(String subject, Throwable t); + long sendSystemErrorNotification(String subject, @Nonnull Throwable t); void sendReply( MessageConversation conversation, diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/preheat/PreheatErrorReport.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/preheat/PreheatErrorReport.java index 23d53f6d34fc..39853cd9a2dd 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/preheat/PreheatErrorReport.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/preheat/PreheatErrorReport.java @@ -75,6 +75,6 @@ public PreheatIdentifier getPreheatIdentifier() { } public IdentifiableObject getObjectReference() { - return value != null ? (IdentifiableObject) value : null; + return getValue() != null ? (IdentifiableObject) getValue() : null; } } diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfiguration.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfiguration.java index 7eca33034291..2ae1d3a13a15 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfiguration.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfiguration.java @@ -166,6 +166,10 @@ public class JobConfiguration extends BaseIdentifiableObject implements Secondar @JsonProperty(access = JsonProperty.Access.READ_ONLY) private Integer queuePosition; + @CheckForNull + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + private String errorCodes; + public JobConfiguration() {} /** diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationService.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationService.java index 49aef39a283e..fd12a9b467d2 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationService.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationService.java @@ -55,11 +55,7 @@ String create(JobConfiguration config, MimeType contentType, InputStream content */ int createDefaultJobs(); - /** - * Make sure the {@link JobType#HEARTBEAT} entry exists as it is responsible for spawning the - * other system jobs when needed using {@link #createDefaultJobs()}. - */ - void createHeartbeatJob(); + void createDefaultJob(JobType type); /** * Updates all {@link JobConfiguration}s that are not {@link JobConfiguration#isEnabled()} to diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationStore.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationStore.java index 4994f62d395c..a0d90d73df58 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationStore.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationStore.java @@ -69,6 +69,9 @@ public interface JobConfigurationStore extends GenericDimensionalObjectStore args) { + // default implementation is a NOOP, we don't remember or handle the error + } + /* * Tracking API: */ @@ -563,15 +568,77 @@ enum FailurePolicy { @Getter final class Progress { - @JsonValue final Deque sequence; + @Nonnull @JsonProperty final Deque sequence; + + @Nonnull + @JsonProperty + @JsonInclude(JsonInclude.Include.NON_EMPTY) + private final Map>> errors; public Progress() { this.sequence = new ConcurrentLinkedDeque<>(); + this.errors = new ConcurrentHashMap<>(); } @JsonCreator - public Progress(Deque sequence) { + public Progress( + @Nonnull @JsonProperty("sequence") Deque sequence, + @CheckForNull @JsonProperty("errors") Map>> errors) { this.sequence = sequence; + this.errors = errors == null ? Map.of() : errors; + } + + public void addError(Error error) { + errors + .computeIfAbsent(error.getId(), key -> new ConcurrentHashMap<>()) + .computeIfAbsent(error.getCode(), key2 -> new ConcurrentLinkedQueue<>()) + .add(error); + } + + public boolean hasErrors() { + return !errors.isEmpty(); + } + + public Set getErrorCodes() { + return errors.values().stream() + .flatMap(e -> e.keySet().stream()) + .collect(toUnmodifiableSet()); + } + } + + @Getter + final class Error { + + @Nonnull @JsonProperty private final ErrorCode code; + + /** The object that has the error */ + @Nonnull @JsonProperty private final String id; + + /** The type of the object identified by #id that has the error */ + @Nonnull @JsonProperty private final String type; + + /** + * The row index in the payload of the import. This is the index in the list of objects of a + * single type. This means the same index occurs for each object type. For some imports this + * information is not available. + */ + @CheckForNull @JsonProperty private final Integer index; + + /** The arguments used in the {@link #code}'s {@link ErrorCode#getMessage()} template */ + @Nonnull @JsonProperty private final List args; + + @JsonCreator + public Error( + @Nonnull @JsonProperty("code") ErrorCode code, + @Nonnull @JsonProperty("id") String id, + @Nonnull @JsonProperty("type") String type, + @CheckForNull @JsonProperty("index") Integer index, + @Nonnull @JsonProperty("args") List args) { + this.code = code; + this.id = id; + this.type = type; + this.index = index; + this.args = args; } } @@ -641,7 +708,7 @@ public Process(String description) { this.stages = new ConcurrentLinkedDeque<>(); } - /** For recreation when de-serializing from string */ + /** For recreation when de-serializing from a JSON string */ @JsonCreator public Process( @JsonProperty("error") String error, diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobSchedulerService.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobSchedulerService.java index b04973be2ae1..43c276948491 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobSchedulerService.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobSchedulerService.java @@ -27,6 +27,7 @@ */ package org.hisp.dhis.scheduling; +import java.util.List; import java.util.Set; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; @@ -91,6 +92,9 @@ public interface JobSchedulerService { @CheckForNull Progress getProgress(@Nonnull String jobId); + @Nonnull + List getErrors(@Nonnull String jobId); + @CheckForNull Progress getRunningProgress(@Nonnull JobType type); diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobType.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobType.java index 6a48216c8bde..7357c130c46c 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobType.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobType.java @@ -112,7 +112,7 @@ public enum JobType { /* System Jobs */ - HEARTBEAT(every(20, "DHIS2rocks1", "Heartbeat")), + HOUSEKEEPING(every(20, "DHIS2rocks1", "Housekeeping")), DATA_SET_NOTIFICATION(daily2am("YvAwAmrqAtN", "Dataset notification")), CREDENTIALS_EXPIRY_ALERT(daily2am("sHMedQF7VYa", "Credentials expiry alert")), DATA_STATISTICS(daily2am("BFa3jDsbtdO", "Data statistics")), @@ -163,8 +163,8 @@ static Defaults dailyRandomBetween3and5(String uid, String name) { } } - private final Class jobParameters; - private final Defaults defaults; + @CheckForNull private final Class jobParameters; + @CheckForNull private final Defaults defaults; JobType() { this(null, null); @@ -178,7 +178,8 @@ static Defaults dailyRandomBetween3and5(String uid, String name) { this(null, defaults); } - JobType(Class jobParameters, Defaults defaults) { + JobType( + @CheckForNull Class jobParameters, @CheckForNull Defaults defaults) { this.jobParameters = jobParameters; this.defaults = defaults; } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultQueryItemLocator.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultQueryItemLocator.java index a2322bd9aa27..9835c2b4a6ad 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultQueryItemLocator.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/DefaultQueryItemLocator.java @@ -32,10 +32,7 @@ import static org.hisp.dhis.common.DimensionalObject.DIMENSION_IDENTIFIER_SEP; import static org.hisp.dhis.common.DimensionalObject.ITEM_SEP; -import java.util.Collections; -import java.util.Date; -import java.util.Objects; -import java.util.Optional; +import java.util.*; import java.util.function.Supplier; import java.util.stream.Stream; import lombok.RequiredArgsConstructor; @@ -275,7 +272,7 @@ private static RepeatableStageParams getRepeatableStageParams(String dimension) return repeatableStageParams; } catch (InvalidRepeatableStageParamsException e) { - ErrorMessage errorMessage = new ErrorMessage(dimension, ErrorCode.E1101); + ErrorMessage errorMessage = new ErrorMessage(dimension, ErrorCode.E1101, List.of(dimension)); throw new IllegalQueryException(errorMessage); } diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/config/StartupConfig.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/config/StartupConfig.java index f8cfa72e2cfb..b6f9f35dc659 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/config/StartupConfig.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/config/StartupConfig.java @@ -37,7 +37,6 @@ import org.hisp.dhis.organisationunit.OrganisationUnitService; import org.hisp.dhis.period.PeriodStore; import org.hisp.dhis.period.PeriodTypePopulator; -import org.hisp.dhis.scheduling.JobConfigurationService; import org.hisp.dhis.scheduling.JobScheduler; import org.hisp.dhis.setting.SystemSettingManager; import org.hisp.dhis.startup.ConfigurationPopulator; @@ -124,9 +123,8 @@ public DefaultAdminUserPopulator defaultAdminUserPopulator(UserService userServi } @Bean - public SchedulerStart schedulerStart( - JobScheduler scheduler, JobConfigurationService jobConfigurationService) { - SchedulerStart schedulerStart = new SchedulerStart(scheduler, jobConfigurationService); + public SchedulerStart schedulerStart(JobScheduler scheduler) { + SchedulerStart schedulerStart = new SchedulerStart(scheduler); schedulerStart.setRunlevel(15); schedulerStart.setSkipInTests(true); return schedulerStart; diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java index 9b9902ddbed5..33fb27fc4305 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java @@ -209,8 +209,10 @@ public long sendMessage(MessageConversationParams params) { } @Override - @Transactional - public long sendSystemErrorNotification(String subject, Throwable t) { + @Transactional // FIXME this should always require an existing TX (in the hopes that should then a + // exception occur in this method this will not cause a rollback if it is caught on + // the caller side inside the existing TX) + public long sendSystemErrorNotification(String subject, @Nonnull Throwable t) { String title = systemSettingManager.getStringSetting(SettingKey.APPLICATION_TITLE); String baseUrl = configurationProvider.getServerBaseUrl(); diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobConfigurationService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobConfigurationService.java index 45c3fb54417d..03ab931c1313 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobConfigurationService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobConfigurationService.java @@ -141,18 +141,9 @@ public int createDefaultJobs() { @Override @Transactional - public void createHeartbeatJob() { - JobConfiguration config = jobConfigurationStore.getByUid(JobType.HEARTBEAT.getDefaults().uid()); - if (config == null) { - createDefaultJob(JobType.HEARTBEAT); - } else if (config.getJobStatus() != JobStatus.SCHEDULED) { - config.setJobStatus(JobStatus.SCHEDULED); - jobConfigurationStore.update(config); - } - } - - private void createDefaultJob(JobType type) { + public void createDefaultJob(JobType type) { Defaults job = type.getDefaults(); + if (job == null) return; JobConfiguration config = new JobConfiguration(job.name(), type); config.setCronExpression(job.cronExpression()); config.setDelay(job.delay()); diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerLoopService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerLoopService.java index 1c8f9f4fd070..880733f64ddb 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerLoopService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerLoopService.java @@ -28,6 +28,7 @@ package org.hisp.dhis.scheduling; import static java.lang.String.format; +import static java.util.stream.Collectors.joining; import static org.hisp.dhis.eventhook.EventUtils.schedulerCompleted; import static org.hisp.dhis.eventhook.EventUtils.schedulerFailed; import static org.hisp.dhis.eventhook.EventUtils.schedulerStart; @@ -45,6 +46,7 @@ import lombok.extern.slf4j.Slf4j; import org.hisp.dhis.commons.util.DebugUtils; import org.hisp.dhis.eventhook.EventHookPublisher; +import org.hisp.dhis.feedback.ErrorCode; import org.hisp.dhis.feedback.NotFoundException; import org.hisp.dhis.leader.election.LeaderManager; import org.hisp.dhis.message.MessageService; @@ -83,6 +85,19 @@ public class DefaultJobSchedulerLoopService implements JobSchedulerLoopService { */ private final Map recordingsById = new ConcurrentHashMap<>(); + @Override + @Transactional + public void createHousekeepingJob() { + JobType.Defaults defaults = JobType.HOUSEKEEPING.getDefaults(); + if (defaults == null) return; + JobConfiguration config = jobConfigurationStore.getByUid(defaults.uid()); + if (config == null) { + jobConfigurationService.createDefaultJob(JobType.HOUSEKEEPING); + } else if (config.getJobStatus() != JobStatus.SCHEDULED) { + finishRunCancel(config.getUid()); + } + } + @Override @Transactional(readOnly = true) public int applyCancellation() { @@ -185,10 +200,18 @@ public void finishRunFail(@Nonnull String jobId, @CheckForNull Exception ex) { doSafely("fail", "log.error", () -> logError(message, ex)); doSafely("fail", "MDC.remove", () -> MDC.remove("sessionId")); doSafely("fail", "publishEvent", () -> events.publishEvent(schedulerFailed(job))); - doSafely( - "fail", - "sendSystemErrorNotification", - () -> messages.sendSystemErrorNotification(message, ex)); + Exception cause = ex; + if (cause == null) { + RecordingJobProgress progress = recordingsById.get(jobId); + if (progress != null) cause = progress.getCause(); + } + if (cause != null) { + Exception causeF = cause; + doSafely( + "fail", + "sendSystemErrorNotification", + () -> messages.sendSystemErrorNotification(message, causeF)); + } skipRestOfQueue(job); } } @@ -260,9 +283,13 @@ private void updateProgress(@Nonnull String jobId) { RecordingJobProgress job = recordingsById.get(jobId); if (job == null) return; try { - jobConfigurationStore.updateProgress(jobId, jsonMapper.writeValueAsString(job.getProgress())); + JobProgress.Progress progress = job.getProgress(); + String errorCodes = + progress.getErrorCodes().stream().map(ErrorCode::name).collect(joining(" ")); + jobConfigurationStore.updateProgress( + jobId, jsonMapper.writeValueAsString(progress), errorCodes); } catch (JsonProcessingException ex) { - jobConfigurationStore.updateProgress(jobId, null); + jobConfigurationStore.updateProgress(jobId, null, null); log.error("Failed to attach progress json", ex); } } diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerService.java index 0fa63bf64602..7a1ffe4436f9 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerService.java @@ -29,11 +29,12 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; -import java.util.Set; +import java.util.*; import javax.annotation.Nonnull; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.hisp.dhis.feedback.ConflictException; +import org.hisp.dhis.feedback.ErrorCode; import org.hisp.dhis.feedback.NotFoundException; import org.hisp.dhis.scheduling.JobProgress.Progress; import org.springframework.stereotype.Service; @@ -111,6 +112,22 @@ public Progress getProgress(@Nonnull String jobId) { return json == null ? null : mapToProgress(json); } + @Nonnull + @Override + @Transactional(readOnly = true) + public List getErrors(@Nonnull String jobId) { + String json = jobConfigurationStore.getErrors(jobId); + if (json == null) return List.of(); + Progress progress = mapToProgress("{\"sequence\":[],\"errors\":" + json + "}"); + if (progress == null) return List.of(); + Map>> map = progress.getErrors(); + return map.isEmpty() + ? List.of() + : map.values().stream() + .flatMap(errors -> errors.values().stream().flatMap(Collection::stream)) + .toList(); + } + @Override @Transactional(readOnly = true) public Progress getRunningProgress(@Nonnull JobType type) { diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/HeartbeatJob.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/HeartbeatJob.java index 1d19bd425a51..ec3830ada9ee 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/HeartbeatJob.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/HeartbeatJob.java @@ -54,7 +54,7 @@ public class HeartbeatJob implements Job { @Override public JobType getJobType() { - return JobType.HEARTBEAT; + return JobType.HOUSEKEEPING; } @Override diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/HibernateJobConfigurationStore.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/HibernateJobConfigurationStore.java index c4a93d3f9425..be63df14bf74 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/HibernateJobConfigurationStore.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/HibernateJobConfigurationStore.java @@ -97,12 +97,32 @@ public String getProgress(@Nonnull String jobId) { // language=SQL String sql = """ - select progress #>> '{}' from jobconfiguration + select + case jsonb_typeof(progress) + when 'object' then progress #>> '{}' + when 'array' then jsonb_build_object('sequence', progress) #>> '{}' + end + from jobconfiguration where uid = :id """; return getSingleResultOrNull(nativeQuery(sql).setParameter("id", jobId)); } + @Override + public String getErrors(@Nonnull String jobId) { + // language=SQL + String sql = + """ + select + case jsonb_typeof(progress) + when 'object' then progress #>> '{errors}' + end + from jobconfiguration + where uid = :id + """; + return getSingleResultOrNull(nativeQuery(sql).setParameter("id", jobId)); + } + @Override public Set getAllIds() { // language=SQL @@ -350,17 +370,23 @@ public boolean trySkip(@Nonnull String queue) { } @Override - public void updateProgress(@Nonnull String jobId, @CheckForNull String progressJson) { + public void updateProgress( + @Nonnull String jobId, @CheckForNull String progressJson, @CheckForNull String errorCodes) { // language=SQL String sql = """ update jobconfiguration set - progress = to_jsonb(:json), - lastalive = case when jobstatus = 'RUNNING' then now() else lastalive end + lastalive = case when jobstatus = 'RUNNING' then now() else lastalive end, + errorcodes = :errors, + progress = cast(:json as jsonb) where uid = :id """; - nativeQuery(sql).setParameter("id", jobId).setParameter("json", progressJson).executeUpdate(); + nativeQuery(sql) + .setParameter("id", jobId) + .setParameter("json", progressJson) + .setParameter("errors", errorCodes) + .executeUpdate(); } @Override diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/JobScheduler.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/JobScheduler.java index 0ce4060b804d..f51c97a62c58 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/JobScheduler.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/JobScheduler.java @@ -119,6 +119,13 @@ public void run() { .collect(groupingBy(JobConfiguration::getJobType)); // only attempt to start one per type per loop invocation readyByType.forEach((type, jobs) -> runIfDue(now, type, jobs)); + if (!readyByType.containsKey(JobType.HOUSEKEEPING)) { + try { + service.createHousekeepingJob(); + } catch (Exception ex) { + log.error("Unable to create house-keeping job: " + ex.getMessage()); + } + } } } catch (Exception ex) { log.error("Exceptions thrown in scheduler loop", ex); diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/JobSchedulerLoopService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/JobSchedulerLoopService.java index 161aabedcf5a..0bc2d30f6d19 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/JobSchedulerLoopService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/JobSchedulerLoopService.java @@ -44,6 +44,12 @@ */ public interface JobSchedulerLoopService { + /** + * Make sure the {@link JobType#HOUSEKEEPING} entry exists as it is responsible for spawning the + * other system jobs when needed using {@link JobConfigurationService#createDefaultJobs()}. + */ + void createHousekeepingJob(); + /** * @return true, if node is or become the leader, else false */ diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java index 9ede398504c7..01dbe261cd64 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java @@ -32,10 +32,14 @@ import java.time.Duration; import java.util.Deque; +import java.util.List; import java.util.concurrent.CancellationException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import javax.annotation.CheckForNull; +import lombok.Getter; import lombok.extern.slf4j.Slf4j; +import org.hisp.dhis.feedback.ErrorCode; import org.hisp.dhis.message.MessageService; import org.hisp.dhis.user.CurrentUserDetails; import org.hisp.dhis.user.CurrentUserUtil; @@ -62,7 +66,7 @@ public class RecordingJobProgress implements JobProgress { private final AtomicBoolean cancellationRequested = new AtomicBoolean(); private final AtomicBoolean abortAfterFailure = new AtomicBoolean(); private final AtomicBoolean skipCurrentStage = new AtomicBoolean(); - private final Progress progress = new Progress(); + @Getter private final Progress progress = new Progress(); private final AtomicReference incompleteProcess = new AtomicReference<>(); private final AtomicReference incompleteStage = new AtomicReference<>(); private final ThreadLocal incompleteItem = new ThreadLocal<>(); @@ -89,6 +93,15 @@ public RecordingJobProgress( messageService != null && configuration.getJobType().isUsingErrorNotification(); } + /** + * @return the exception that likely caused the job to abort + */ + @CheckForNull + public Exception getCause() { + Process process = progress.sequence.peekLast(); + return process == null ? null : process.getCause(); + } + public void requestCancellation() { if (cancellationRequested.compareAndSet(false, true)) { progress.sequence.forEach( @@ -99,10 +112,6 @@ public void requestCancellation() { } } - public Progress getProgress() { - return progress; - } - @Override public boolean isSuccessful() { autoComplete(); @@ -134,6 +143,11 @@ public boolean isSkipCurrentStage() { return skipCurrentStage.get() || isCancelled(); } + @Override + public void addError(ErrorCode code, String uid, String type, Integer index, List args) { + progress.addError(new Error(code, uid, type, index, args)); + } + @Override public void startingProcess(String description) { if (isCancelled()) { diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/startup/SchedulerStart.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/startup/SchedulerStart.java index 59e3d3b9d2f0..3564eeceb0ce 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/startup/SchedulerStart.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/startup/SchedulerStart.java @@ -28,7 +28,6 @@ package org.hisp.dhis.startup; import lombok.RequiredArgsConstructor; -import org.hisp.dhis.scheduling.JobConfigurationService; import org.hisp.dhis.scheduling.JobScheduler; import org.hisp.dhis.system.startup.AbstractStartupRoutine; @@ -42,11 +41,9 @@ public class SchedulerStart extends AbstractStartupRoutine { private final JobScheduler scheduler; - private final JobConfigurationService jobConfigurationService; @Override public void execute() throws Exception { - jobConfigurationService.createHeartbeatJob(); scheduler.start(); } } diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/scheduling/hibernate/JobConfiguration.hbm.xml b/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/scheduling/hibernate/JobConfiguration.hbm.xml index ff08e5ef8673..d27693fbf591 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/scheduling/hibernate/JobConfiguration.hbm.xml +++ b/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/scheduling/hibernate/JobConfiguration.hbm.xml @@ -67,5 +67,8 @@ + + + diff --git a/dhis-2/dhis-services/dhis-service-metadata-workflow/src/main/java/org/hisp/dhis/metadata/DefaultMetadataWorkflowService.java b/dhis-2/dhis-services/dhis-service-metadata-workflow/src/main/java/org/hisp/dhis/metadata/DefaultMetadataWorkflowService.java index 53316ce878a6..5bfe838e9709 100644 --- a/dhis-2/dhis-services/dhis-service-metadata-workflow/src/main/java/org/hisp/dhis/metadata/DefaultMetadataWorkflowService.java +++ b/dhis-2/dhis-services/dhis-service-metadata-workflow/src/main/java/org/hisp/dhis/metadata/DefaultMetadataWorkflowService.java @@ -27,12 +27,12 @@ */ package org.hisp.dhis.metadata; -import static java.util.Collections.singletonList; import static org.hisp.dhis.util.JsonUtils.jsonToObject; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import java.util.Date; +import java.util.List; import javax.annotation.PostConstruct; import javax.persistence.NoResultException; import lombok.RequiredArgsConstructor; @@ -264,7 +264,7 @@ private ImportReport createImportReportWithError( ObjectReport objectReport = new ObjectReport(objType, null); ErrorReport errorReport = new ErrorReport(MetadataProposal.class, errorCode, args); errorReport.setErrorProperty(property); - errorReport.setErrorProperties(singletonList(property)); + errorReport.setErrorProperties(List.of(property)); objectReport.addErrorReport(errorReport); TypeReport typeReport = new TypeReport(objType); typeReport.addObjectReport(objectReport); diff --git a/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.41/V2_41_32__job_configuation_errors_in_db.sql b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.41/V2_41_32__job_configuation_errors_in_db.sql new file mode 100644 index 000000000000..1790def06a8d --- /dev/null +++ b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.41/V2_41_32__job_configuation_errors_in_db.sql @@ -0,0 +1,11 @@ +-- adds error tracking to the job configuration table +-- this comes in two parts, the errors themself are added to the existing progress JSON object +-- the "index" of the errors, the list of error codes, from the full error description +-- is extracted and kept as a space seperated list in a new column "error_codes" +-- the column is null for row that existed before the feature and did not run since +-- the column is an empty string if there were no errors +-- the column is a list of error codes, like "E1100 E1105" if there where these error +-- list of error codes are always sorted by their number to allow combination searches using patterns + +alter table jobconfiguration + add column if not exists errorcodes text; \ No newline at end of file diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/metadata/MetadataImportJob.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/metadata/MetadataImportJob.java index ca4fa2bc080c..4a695b82878b 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/metadata/MetadataImportJob.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/metadata/MetadataImportJob.java @@ -100,6 +100,19 @@ public void execute(JobConfiguration config, JobProgress progress) { progress.failedProcess("Import failed, no summary available"); return; } + + if (report.hasErrorReports()) { + report.forEachErrorReport( + r -> { + progress.addError( + r.getErrorCode(), + r.getMainId(), + r.getMainKlass().getSimpleName(), + null, + r.getArgs()); + }); + } + notifier.addJobSummary(config, report, ImportReport.class); Stats count = report.getStats(); Consumer endProcess = diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/scheduling/JobConfigurationController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/scheduling/JobConfigurationController.java index 032b494cf97d..05a9f58d4860 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/scheduling/JobConfigurationController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/scheduling/JobConfigurationController.java @@ -39,6 +39,7 @@ import org.hisp.dhis.feedback.ObjectReport; import org.hisp.dhis.scheduling.JobConfiguration; import org.hisp.dhis.scheduling.JobConfigurationService; +import org.hisp.dhis.scheduling.JobProgress; import org.hisp.dhis.scheduling.JobProgress.Progress; import org.hisp.dhis.scheduling.JobSchedulerService; import org.hisp.dhis.schema.Property; @@ -117,6 +118,11 @@ public Progress getProgress(@PathVariable("uid") String uid) { return jobSchedulerService.getProgress(uid); } + @GetMapping("{uid}/errors") + public List getErrors(@PathVariable("uid") String uid) { + return jobSchedulerService.getErrors(uid); + } + @PreAuthorize("hasRole('ALL') or hasRole('F_PERFORM_MAINTENANCE')") @PostMapping("clean") @ResponseStatus(HttpStatus.NO_CONTENT) From 81ce0da02c561c1dd3f3c0912e156626f3c8f69f Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Mon, 6 Nov 2023 12:56:39 +0100 Subject: [PATCH 03/10] fix: message arguments from array [DHIS2-15276] --- .../java/org/hisp/dhis/feedback/ErrorMessage.java | 2 +- .../java/org/hisp/dhis/scheduling/JobProgress.java | 10 ++++++++++ .../dhis/scheduling/DefaultJobSchedulerService.java | 12 ++++++++---- .../scheduling/JobConfigurationController.java | 3 +++ 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorMessage.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorMessage.java index 180bb1dcba94..79d43505d910 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorMessage.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorMessage.java @@ -51,7 +51,7 @@ public class ErrorMessage { public ErrorMessage(ErrorCode errorCode, Object... args) { this.errorCode = errorCode; this.args = Stream.of(args).map(Object::toString).toList(); - this.message = MessageFormat.format(errorCode.getMessage(), this.args); + this.message = MessageFormat.format(errorCode.getMessage(), args); } @JsonCreator diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobProgress.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobProgress.java index c26101702e1a..932cbf92a6f7 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobProgress.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobProgress.java @@ -45,6 +45,7 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import lombok.*; +import lombok.experimental.Accessors; import org.hisp.dhis.feedback.ErrorCode; /** @@ -607,6 +608,7 @@ public Set getErrorCodes() { } @Getter + @Accessors(chain = true) final class Error { @Nonnull @JsonProperty private final ErrorCode code; @@ -627,6 +629,14 @@ final class Error { /** The arguments used in the {@link #code}'s {@link ErrorCode#getMessage()} template */ @Nonnull @JsonProperty private final List args; + /** + * The message as created from {@link #code} and {@link #args}. This is only set in service + * layer for the web API using the setter, it is not persisted. + */ + @Setter + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + private String message; + @JsonCreator public Error( @Nonnull @JsonProperty("code") ErrorCode code, diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerService.java index 7a1ffe4436f9..0fa7649de58d 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerService.java @@ -29,6 +29,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import java.text.MessageFormat; import java.util.*; import javax.annotation.Nonnull; import lombok.RequiredArgsConstructor; @@ -121,11 +122,14 @@ public List getErrors(@Nonnull String jobId) { Progress progress = mapToProgress("{\"sequence\":[],\"errors\":" + json + "}"); if (progress == null) return List.of(); Map>> map = progress.getErrors(); - return map.isEmpty() - ? List.of() - : map.values().stream() - .flatMap(errors -> errors.values().stream().flatMap(Collection::stream)) + if (map.isEmpty()) return List.of(); + List errors = + map.values().stream() + .flatMap(e -> e.values().stream().flatMap(Collection::stream)) .toList(); + errors.forEach( + e -> e.setMessage(MessageFormat.format(e.getCode().getMessage(), e.getArgs().toArray()))); + return errors; } @Override diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/scheduling/JobConfigurationController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/scheduling/JobConfigurationController.java index 05a9f58d4860..586eed1a290e 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/scheduling/JobConfigurationController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/scheduling/JobConfigurationController.java @@ -110,14 +110,17 @@ public ObjectReport executeNow(@PathVariable("uid") String uid) @PostMapping("{uid}/cancel") @ResponseStatus(HttpStatus.NO_CONTENT) public void cancelExecution(@PathVariable("uid") String uid) { + // TODO check auth jobSchedulerService.requestCancel(uid); } + @PreAuthorize("hasRole('ALL') or hasRole('F_PERFORM_MAINTENANCE')") @GetMapping("{uid}/progress") public Progress getProgress(@PathVariable("uid") String uid) { return jobSchedulerService.getProgress(uid); } + @PreAuthorize("hasRole('ALL') or hasRole('F_PERFORM_MAINTENANCE')") @GetMapping("{uid}/errors") public List getErrors(@PathVariable("uid") String uid) { return jobSchedulerService.getErrors(uid); From dc341960c677d4fd368241751442eee388af7a85 Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Mon, 6 Nov 2023 13:50:29 +0100 Subject: [PATCH 04/10] fix: NPE in error messages [DHIS2-15276] --- .../src/main/java/org/hisp/dhis/feedback/ErrorMessage.java | 5 ++++- .../src/main/java/org/hisp/dhis/feedback/ErrorReport.java | 7 ++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorMessage.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorMessage.java index 79d43505d910..88ac061d3a59 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorMessage.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorMessage.java @@ -50,7 +50,10 @@ public class ErrorMessage { public ErrorMessage(ErrorCode errorCode, Object... args) { this.errorCode = errorCode; - this.args = Stream.of(args).map(Object::toString).toList(); + this.args = + Stream.of(args) + .map(obj -> obj == null ? null : obj.toString()) + .toList(); // OBS! Must support null values! this.message = MessageFormat.format(errorCode.getMessage(), args); } diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorReport.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorReport.java index 99b48f670998..aaf4f74e5e18 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorReport.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorReport.java @@ -29,6 +29,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.Arrays; import java.util.List; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; @@ -54,13 +55,13 @@ public class ErrorReport { @Nonnull @JsonProperty private List errorProperties; @JsonProperty private Object value; - public ErrorReport(Class mainKlass, ErrorCode errorCode, Object... args) { + public ErrorReport(@Nonnull Class mainKlass, @Nonnull ErrorCode errorCode, Object... args) { this.mainKlass = mainKlass; this.message = new ErrorMessage(errorCode, args); - this.errorProperties = List.of(args); + this.errorProperties = Arrays.asList(args); // OBS! Must support null values! } - public ErrorReport(Class mainKlass, ErrorMessage message) { + public ErrorReport(@Nonnull Class mainKlass, @Nonnull ErrorMessage message) { this.mainKlass = mainKlass; this.message = message; this.errorProperties = message.getArgs(); From 65ad507ca7d21f8ba515e79587abcfbe6d7786ca Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Mon, 6 Nov 2023 15:47:05 +0100 Subject: [PATCH 05/10] fix: OpenAPI name conflict [DHIS2-15276] --- .../main/java/org/hisp/dhis/tracker/imports/report/Error.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/report/Error.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/report/Error.java index 1d6892504b35..4038def54b20 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/report/Error.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/report/Error.java @@ -31,9 +31,11 @@ import com.fasterxml.jackson.annotation.JsonProperty; import lombok.Builder; import lombok.Value; +import org.hisp.dhis.common.OpenApi; @Value @Builder +@OpenApi.Shared(name = "TrackerImportError") public class Error { private final String errorMessage; From 2d50a009aea2f03c20f0c4508bcbff13611c8937 Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Mon, 6 Nov 2023 16:05:46 +0100 Subject: [PATCH 06/10] fix: sonar issues [DHIS2-15276] --- .../DefaultJobConfigurationService.java | 3 ++- .../org/hisp/dhis/scheduling/JobScheduler.java | 14 +++++++++----- .../controller/metadata/MetadataImportJob.java | 15 +++++++-------- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobConfigurationService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobConfigurationService.java index 03ab931c1313..68b40d57e50f 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobConfigurationService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobConfigurationService.java @@ -131,7 +131,8 @@ public int createDefaultJobs() { int created = 0; Set jobIds = jobConfigurationStore.getAllIds(); for (JobType t : JobType.values()) { - if (t.getDefaults() != null && !jobIds.contains(t.getDefaults().uid())) { + Defaults defaults = t.getDefaults(); + if (defaults != null && !jobIds.contains(defaults.uid())) { createDefaultJob(t); created++; } diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/JobScheduler.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/JobScheduler.java index f51c97a62c58..5a81d5c5646d 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/JobScheduler.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/JobScheduler.java @@ -120,11 +120,7 @@ public void run() { // only attempt to start one per type per loop invocation readyByType.forEach((type, jobs) -> runIfDue(now, type, jobs)); if (!readyByType.containsKey(JobType.HOUSEKEEPING)) { - try { - service.createHousekeepingJob(); - } catch (Exception ex) { - log.error("Unable to create house-keeping job: " + ex.getMessage()); - } + createHousekeepingJob(); } } } catch (Exception ex) { @@ -133,6 +129,14 @@ public void run() { } } + private void createHousekeepingJob() { + try { + service.createHousekeepingJob(); + } catch (Exception ex) { + log.error("Unable to create house-keeping job: " + ex.getMessage()); + } + } + private void runIfDue(Instant now, JobType type, List jobs) { if (!type.isUsingContinuousExecution()) { runIfDue(now, jobs.get(0)); diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/metadata/MetadataImportJob.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/metadata/MetadataImportJob.java index 4a695b82878b..20c76b3ae303 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/metadata/MetadataImportJob.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/metadata/MetadataImportJob.java @@ -103,14 +103,13 @@ public void execute(JobConfiguration config, JobProgress progress) { if (report.hasErrorReports()) { report.forEachErrorReport( - r -> { - progress.addError( - r.getErrorCode(), - r.getMainId(), - r.getMainKlass().getSimpleName(), - null, - r.getArgs()); - }); + r -> + progress.addError( + r.getErrorCode(), + r.getMainId(), + r.getMainKlass().getSimpleName(), + null, + r.getArgs())); } notifier.addJobSummary(config, report, ImportReport.class); From 3d9e6bbce6e42906196afe710694e24884e79076 Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Tue, 7 Nov 2023 12:00:25 +0100 Subject: [PATCH 07/10] fix: send emails async to not fail TX in case email is not configured [DHIS2-15276] --- .../java/org/hisp/dhis/message/MessageService.java | 2 +- .../org/hisp/dhis/message/DefaultMessageService.java | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageService.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageService.java index 21dd74fd09a5..744e43378538 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageService.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageService.java @@ -57,7 +57,7 @@ long sendValidationMessage( long sendMessage(MessageConversationParams params); - long sendSystemErrorNotification(String subject, @Nonnull Throwable t); + void sendSystemErrorNotification(@Nonnull String subject, @Nonnull Throwable t); void sendReply( MessageConversation conversation, diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java index 33fb27fc4305..cde580ae6e26 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java @@ -60,6 +60,7 @@ import org.hisp.dhis.user.UserSettingService; import org.hisp.dhis.util.ObjectUtils; import org.joda.time.DateTime; +import org.springframework.scheduling.annotation.Async; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -209,10 +210,9 @@ public long sendMessage(MessageConversationParams params) { } @Override - @Transactional // FIXME this should always require an existing TX (in the hopes that should then a - // exception occur in this method this will not cause a rollback if it is caught on - // the caller side inside the existing TX) - public long sendSystemErrorNotification(String subject, @Nonnull Throwable t) { + @Async + @Transactional + public void sendSystemErrorNotification(@Nonnull String subject, @Nonnull Throwable t) { String title = systemSettingManager.getStringSetting(SettingKey.APPLICATION_TITLE); String baseUrl = configurationProvider.getServerBaseUrl(); @@ -234,7 +234,7 @@ public long sendSystemErrorNotification(String subject, @Nonnull Throwable t) { .withMessageType(MessageType.SYSTEM) .build(); - return sendMessage(params); + sendMessage(params); } @Override From 6b1a01b6bcc7d04b9e17d256a3d94582dc886a43 Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Tue, 7 Nov 2023 12:06:24 +0100 Subject: [PATCH 08/10] chore: rename asny method [DHIS2-15276] --- .../src/main/java/org/hisp/dhis/message/MessageService.java | 2 +- .../main/java/org/hisp/dhis/message/DefaultMessageService.java | 2 +- .../hisp/dhis/scheduling/DefaultJobSchedulerLoopService.java | 2 +- .../java/org/hisp/dhis/scheduling/RecordingJobProgress.java | 2 +- .../java/org/hisp/dhis/validation/scheduling/MonitoringJob.java | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageService.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageService.java index 744e43378538..7c1bfd41baaa 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageService.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/message/MessageService.java @@ -57,7 +57,7 @@ long sendValidationMessage( long sendMessage(MessageConversationParams params); - void sendSystemErrorNotification(@Nonnull String subject, @Nonnull Throwable t); + void asyncSendSystemErrorNotification(@Nonnull String subject, @Nonnull Throwable t); void sendReply( MessageConversation conversation, diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java index cde580ae6e26..2f5e784151ba 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/message/DefaultMessageService.java @@ -212,7 +212,7 @@ public long sendMessage(MessageConversationParams params) { @Override @Async @Transactional - public void sendSystemErrorNotification(@Nonnull String subject, @Nonnull Throwable t) { + public void asyncSendSystemErrorNotification(@Nonnull String subject, @Nonnull Throwable t) { String title = systemSettingManager.getStringSetting(SettingKey.APPLICATION_TITLE); String baseUrl = configurationProvider.getServerBaseUrl(); diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerLoopService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerLoopService.java index 880733f64ddb..27c2aedb29fa 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerLoopService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerLoopService.java @@ -210,7 +210,7 @@ public void finishRunFail(@Nonnull String jobId, @CheckForNull Exception ex) { doSafely( "fail", "sendSystemErrorNotification", - () -> messages.sendSystemErrorNotification(message, causeF)); + () -> messages.asyncSendSystemErrorNotification(message, causeF)); } skipRestOfQueue(job); } diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java index 01dbe261cd64..d15a4589d72d 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java @@ -398,7 +398,7 @@ private void sendErrorNotification(Node node, Exception cause) { if (usingErrorNotification) { String subject = node.getClass().getSimpleName() + " failed: " + node.getDescription(); try { - messageService.sendSystemErrorNotification(subject, cause); + messageService.asyncSendSystemErrorNotification(subject, cause); } catch (Exception ex) { log.debug("Failed to send error notification for failed job processing"); } diff --git a/dhis-2/dhis-services/dhis-service-reporting/src/main/java/org/hisp/dhis/validation/scheduling/MonitoringJob.java b/dhis-2/dhis-services/dhis-service-reporting/src/main/java/org/hisp/dhis/validation/scheduling/MonitoringJob.java index 988fb802beef..7b9cc8dc1b6d 100644 --- a/dhis-2/dhis-services/dhis-service-reporting/src/main/java/org/hisp/dhis/validation/scheduling/MonitoringJob.java +++ b/dhis-2/dhis-services/dhis-service-reporting/src/main/java/org/hisp/dhis/validation/scheduling/MonitoringJob.java @@ -106,7 +106,7 @@ public void execute(JobConfiguration config, JobProgress progress) { progress.completedProcess("Data validation done"); } catch (RuntimeException ex) { progress.failedProcess(ex); - messageService.sendSystemErrorNotification("Data validation failed", ex); + messageService.asyncSendSystemErrorNotification("Data validation failed", ex); throw ex; } } From 9f01e8c68ea5470cf51e3acc023b8efbb27dd34c Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Tue, 7 Nov 2023 13:33:14 +0100 Subject: [PATCH 09/10] fix: catch errors when adding errors, use empty string for undefined UID [DHIS2-15276] --- .../dhis/scheduling/RecordingJobProgress.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java index d15a4589d72d..2d0938b9ceb7 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java @@ -37,6 +37,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; import lombok.Getter; import lombok.extern.slf4j.Slf4j; import org.hisp.dhis.feedback.ErrorCode; @@ -144,8 +145,18 @@ public boolean isSkipCurrentStage() { } @Override - public void addError(ErrorCode code, String uid, String type, Integer index, List args) { - progress.addError(new Error(code, uid, type, index, args)); + public void addError( + @Nonnull ErrorCode code, + @CheckForNull String uid, + @Nonnull String type, + @CheckForNull Integer index, + @Nonnull List args) { + try { + // Note: we use empty string in case the UID is not known/defined yet to allow use in maps + progress.addError(new Error(code, uid == null ? "" : uid, type, index, args)); + } catch (Exception ex) { + log.error("Failed to add error: %s %s %s %d %s".formatted(code, uid, type, index, args), ex); + } } @Override From 2e10b52f11e92987f6890fe3106de32015aa402c Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Tue, 7 Nov 2023 13:45:37 +0100 Subject: [PATCH 10/10] fix: require auth to cancel jobs [DHIS2-15276] --- .../scheduling/JobConfigurationController.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/scheduling/JobConfigurationController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/scheduling/JobConfigurationController.java index 586eed1a290e..085f7f97c01a 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/scheduling/JobConfigurationController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/scheduling/JobConfigurationController.java @@ -35,6 +35,7 @@ import org.hisp.dhis.common.IdentifiableObjects; import org.hisp.dhis.common.OpenApi; import org.hisp.dhis.feedback.ConflictException; +import org.hisp.dhis.feedback.ForbiddenException; import org.hisp.dhis.feedback.NotFoundException; import org.hisp.dhis.feedback.ObjectReport; import org.hisp.dhis.scheduling.JobConfiguration; @@ -44,6 +45,8 @@ import org.hisp.dhis.scheduling.JobSchedulerService; import org.hisp.dhis.schema.Property; import org.hisp.dhis.schema.descriptors.JobConfigurationSchemaDescriptor; +import org.hisp.dhis.user.CurrentUser; +import org.hisp.dhis.user.User; import org.hisp.dhis.webapi.controller.AbstractCrudController; import org.hisp.dhis.webapi.webdomain.JobTypes; import org.springframework.http.HttpStatus; @@ -109,8 +112,15 @@ public ObjectReport executeNow(@PathVariable("uid") String uid) @PostMapping("{uid}/cancel") @ResponseStatus(HttpStatus.NO_CONTENT) - public void cancelExecution(@PathVariable("uid") String uid) { - // TODO check auth + public void cancelExecution(@PathVariable("uid") String uid, @CurrentUser User currentUser) + throws NotFoundException, ForbiddenException { + JobConfiguration obj = jobConfigurationService.getJobConfigurationByUid(uid); + if (obj == null) throw new NotFoundException(JobConfiguration.class, uid); + boolean canCancel = + currentUser.isSuper() + || currentUser.isAuthorized("F_PERFORM_MAINTENANCE") + || currentUser.getUid().equals(obj.getExecutedBy()); + if (!canCancel) throw new ForbiddenException(JobConfiguration.class, obj.getUid()); jobSchedulerService.requestCancel(uid); }