feat: tracker import errors to job tracking error forward [DHIS2-1527…

…6] (#15723) * feat: tracker import errors to job tracking error forward [DHIS2-15276] * fix: handle nulls in args * fix: sonar warnings * fix: use JSON as storage format for tracker imports [DHIS2-15276] * fix: test setup [DHIS2-15276] * test: tracker import and input include [DHIS2-15276]
dhis2 · Nov 20, 2023 · 7fd3140 · 7fd3140
1 parent b551613
commit 7fd3140
Show file tree

Hide file tree

Showing 28 changed files with 291 additions and 185 deletions.
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
@@ -47,6 +47,7 @@
 import lombok.*;
 import lombok.experimental.Accessors;
 import org.hisp.dhis.feedback.ErrorCode;
+import org.hisp.dhis.tracker.imports.validation.ValidationCode;
 
 /**
  *
@@ -147,6 +148,14 @@ default void addError(
     addError(code, uid, type, List.of(args));
   }
 
+  default void addError(
+      @Nonnull ValidationCode code,
+      @CheckForNull String uid,
+      @Nonnull String type,
+      String... args) {
+    addError(code, uid, type, List.of(args));
+  }
+
   default void addError(
       @Nonnull ErrorCode code,
       @CheckForNull String uid,
@@ -156,6 +165,15 @@ default void addError(
     // default is to not collect errors
   }
 
+  default void addError(
+      @Nonnull ValidationCode code,
+      @CheckForNull String uid,
+      @Nonnull String type,
+      @Nonnull List<String> args) {
+    // is overridden by a tracker that collects errors
+    // default is to not collect errors
+  }
+
   /*
    * Tracking API:
    */
@@ -580,7 +598,7 @@ final class Progress {
     @Nonnull
     @JsonProperty
     @JsonInclude(JsonInclude.Include.NON_EMPTY)
-    private final Map<String, Map<ErrorCode, Queue<Error>>> errors;
+    private final Map<String, Map<String, Queue<Error>>> errors;
 
     public Progress() {
       this.sequence = new ConcurrentLinkedDeque<>();
@@ -590,7 +608,7 @@ public Progress() {
     @JsonCreator
     public Progress(
         @Nonnull @JsonProperty("sequence") Deque<Process> sequence,
-        @CheckForNull @JsonProperty("errors") Map<String, Map<ErrorCode, Queue<Error>>> errors) {
+        @CheckForNull @JsonProperty("errors") Map<String, Map<String, Queue<Error>>> errors) {
       this.sequence = sequence;
       this.errors = errors == null ? Map.of() : errors;
     }
@@ -609,7 +627,7 @@ public boolean hasErrors() {
       return !errors.isEmpty();
     }
 
-    public Set<ErrorCode> getErrorCodes() {
+    public Set<String> getErrorCodes() {
       return errors.values().stream()
           .flatMap(e -> e.keySet().stream())
           .collect(toUnmodifiableSet());
@@ -620,7 +638,7 @@ public Set<ErrorCode> getErrorCodes() {
   @Accessors(chain = true)
   final class Error {
 
-    @Nonnull @JsonProperty private final ErrorCode code;
+    @Nonnull @JsonProperty private final String code;
 
     /** The object that has the error */
     @Nonnull @JsonProperty private final String id;
@@ -641,7 +659,7 @@ final class Error {
 
     @JsonCreator
     public Error(
-        @Nonnull @JsonProperty("code") ErrorCode code,
+        @Nonnull @JsonProperty("code") String code,
         @Nonnull @JsonProperty("id") String id,
         @Nonnull @JsonProperty("type") String type,
         @Nonnull @JsonProperty("args") List<String> args) {

diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobRunErrorsParams.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobRunErrorsParams.java
@@ -72,4 +72,6 @@ public class JobRunErrorsParams {
 
   /** The {@link JobType} with errors to select, any match combined */
   @CheckForNull private List<JobType> type;
+
+  boolean includeInput = false;
 }
diff --git a/...er/imports/validation/ValidationCode.java → ...er/imports/validation/ValidationCode.java b/...er/imports/validation/ValidationCode.java → ...er/imports/validation/ValidationCode.java
diff --git a/...s-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobConfigurationService.java b/...s-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobConfigurationService.java
@@ -27,6 +27,8 @@
  */
 package org.hisp.dhis.scheduling;
 
+import static org.hisp.dhis.jsontree.JsonBuilder.createArray;
+import static org.hisp.dhis.scheduling.JobType.TRACKER_IMPORT_JOB;
 import static org.hisp.dhis.scheduling.JobType.values;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
@@ -40,6 +42,7 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
+import java.nio.charset.StandardCharsets;
 import java.text.MessageFormat;
 import java.time.Duration;
 import java.time.Instant;
@@ -49,7 +52,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import javax.annotation.Nonnull;
@@ -72,6 +74,7 @@
 import org.hisp.dhis.schema.Property;
 import org.hisp.dhis.setting.SettingKey;
 import org.hisp.dhis.setting.SystemSettingManager;
+import org.hisp.dhis.tracker.imports.validation.ValidationCode;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Propagation;
 import org.springframework.transaction.annotation.Transactional;
@@ -256,48 +259,69 @@ public List<JobConfiguration> getStaleConfigurations(int staleForSeconds) {
   @Override
   @Transactional(readOnly = true)
   public List<JsonObject> findJobRunErrors(@Nonnull JobRunErrorsParams params) {
-    Function<String, JsonObject> toObject =
-        json -> {
-          JsonObject obj = JsonMixed.of(json);
-          List<JsonNode> flatErrors = new ArrayList<>();
-          JsonObject errors = obj.getObject("errors");
-          errors
-              .node()
-              .members()
-              .forEach(
-                  byObject ->
-                      byObject
-                          .getValue()
-                          .members()
-                          .forEach(
-                              byCode ->
-                                  byCode
-                                      .getValue()
-                                      .elements()
-                                      .forEach(
-                                          error -> {
-                                            ErrorCode code =
-                                                ErrorCode.valueOf(
-                                                    JsonMixed.of(error).getString("code").string());
-                                            Object[] args =
-                                                JsonMixed.of(error)
-                                                    .getArray("args")
-                                                    .stringValues()
-                                                    .toArray(new String[0]);
-                                            String msg =
-                                                MessageFormat.format(code.getMessage(), args);
-                                            flatErrors.add(
-                                                error
-                                                    .extract()
-                                                    .addMembers(e -> e.addString("message", msg)));
-                                          })));
-          return JsonMixed.of(
-              errors
-                  .node()
-                  .replaceWith(
-                      JsonBuilder.createArray(arr -> flatErrors.forEach(arr::addElement))));
-        };
-    return jobConfigurationStore.findJobRunErrors(params).map(toObject).toList();
+    return jobConfigurationStore
+        .findJobRunErrors(params)
+        .map(json -> errorEntryWithMessages(json, params))
+        .toList();
+  }
+
+  private JsonObject errorEntryWithMessages(String json, JobRunErrorsParams params) {
+    JsonObject entry = JsonMixed.of(json);
+    List<JsonNode> flatErrors = new ArrayList<>();
+    JobType type = entry.getString("type").parsed(JobType::valueOf);
+    String fileResourceId = entry.getString("file").string();
+    JsonObject errors = entry.getObject("errors");
+    String errorCodeNamespace =
+        type == TRACKER_IMPORT_JOB
+            ? ValidationCode.class.getSimpleName()
+            : ErrorCode.class.getSimpleName();
+    errors
+        .node()
+        .members()
+        .forEach(
+            byObject ->
+                byObject
+                    .getValue()
+                    .members()
+                    .forEach(
+                        byCode ->
+                            byCode
+                                .getValue()
+                                .elements()
+                                .forEach(error -> flatErrors.add(errorWithMessage(type, error)))));
+
+    return JsonMixed.of(
+        errors
+            .node()
+            .replaceWith(createArray(arr -> flatErrors.forEach(arr::addElement)))
+            .addMembers(
+                obj ->
+                    obj.addString("codes", errorCodeNamespace)
+                        .addMember("input", getJobInput(params, fileResourceId))));
+  }
+
+  private JsonNode getJobInput(JobRunErrorsParams params, String fileResourceId) {
+    if (!params.isIncludeInput()) return JsonNode.of("null");
+    try {
+      byte[] bytes =
+          fileResourceService.copyFileResourceContent(
+              fileResourceService.getFileResource(fileResourceId));
+      return JsonNode.of(new String(bytes, StandardCharsets.UTF_8));
+    } catch (IOException ex) {
+      log.warn("Could not copy file content to error info for file: " + fileResourceId, ex);
+      return JsonNode.of("\"" + ex.getMessage() + "\"");
+    }
+  }
+
+  private static JsonNode errorWithMessage(JobType type, JsonNode error) {
+    String codeName = JsonMixed.of(error).getString("code").string();
+    String template =
+        type == TRACKER_IMPORT_JOB
+            ? ValidationCode.valueOf(codeName).getMessage()
+            : ErrorCode.valueOf(codeName).getMessage();
+    Object[] args = JsonMixed.of(error).getArray("args").stringValues().toArray(new String[0]);
+    String msg = MessageFormat.format(template, args);
+    return error.extract().addMembers(e -> e.addString("message", msg));
   }
 
   @Override

diff --git a/...s-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerLoopService.java b/...s-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerLoopService.java
@@ -46,7 +46,6 @@
 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;
@@ -284,8 +283,7 @@ private void updateProgress(@Nonnull String jobId) {
     if (job == null) return;
     try {
       JobProgress.Progress progress = job.getProgress();
-      String errorCodes =
-          progress.getErrorCodes().stream().map(ErrorCode::name).sorted().collect(joining(" "));
+      String errorCodes = progress.getErrorCodes().stream().sorted().collect(joining(" "));
       jobConfigurationStore.updateProgress(
           jobId, jsonMapper.writeValueAsString(progress), errorCodes);
     } catch (JsonProcessingException ex) {

diff --git a/.../dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerService.java b/.../dhis-service-core/src/main/java/org/hisp/dhis/scheduling/DefaultJobSchedulerService.java
@@ -29,13 +29,11 @@
 
 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;
 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;
@@ -127,15 +125,11 @@ public List<JobProgress.Error> getErrors(@Nonnull String jobId) {
     if (json == null) return List.of();
     Progress progress = mapToProgress("{\"sequence\":[],\"errors\":" + json + "}");
     if (progress == null) return List.of();
-    Map<String, Map<ErrorCode, Queue<JobProgress.Error>>> map = progress.getErrors();
+    Map<String, Map<String, Queue<JobProgress.Error>>> map = progress.getErrors();
     if (map.isEmpty()) return List.of();
-    List<JobProgress.Error> 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;
+    return map.values().stream()
+        .flatMap(e -> e.values().stream().flatMap(Collection::stream))
+        .toList();
   }
 
   @Override

diff --git a/...s-service-core/src/main/java/org/hisp/dhis/scheduling/HibernateJobConfigurationStore.java b/...s-service-core/src/main/java/org/hisp/dhis/scheduling/HibernateJobConfigurationStore.java
@@ -261,6 +261,7 @@ select jsonb_build_object(
     'created', c.created,
     'executed', c.lastexecuted,
     'finished', c.lastfinished,
+    'file', fr.uid,
     'filesize', fr.contentlength,
     'filetype', fr.contenttype,
     'errors', c.progress -> 'errors') #>> '{}'

diff --git a/...rvices/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java b/...rvices/dhis-service-core/src/main/java/org/hisp/dhis/scheduling/RecordingJobProgress.java
@@ -42,6 +42,7 @@
 import lombok.extern.slf4j.Slf4j;
 import org.hisp.dhis.feedback.ErrorCode;
 import org.hisp.dhis.message.MessageService;
+import org.hisp.dhis.tracker.imports.validation.ValidationCode;
 import org.hisp.dhis.user.CurrentUserDetails;
 import org.hisp.dhis.user.CurrentUserUtil;
 
@@ -150,6 +151,23 @@ public void addError(
       @CheckForNull String uid,
       @Nonnull String type,
       @Nonnull List<String> args) {
+    addError(code.name(), uid, type, args);
+  }
+
+  @Override
+  public void addError(
+      @Nonnull ValidationCode code,
+      @CheckForNull String uid,
+      @Nonnull String type,
+      @Nonnull List<String> args) {
+    addError(code.name(), uid, type, args);
+  }
+
+  private void addError(
+      @Nonnull String code,
+      @CheckForNull String uid,
+      @Nonnull String type,
+      @Nonnull List<String> 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, args));

diff --git a/...rvices/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/report/Error.java b/...rvices/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/report/Error.java
@@ -29,51 +29,36 @@
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.List;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
 import lombok.Builder;
+import lombok.EqualsAndHashCode;
 import lombok.Value;
 import org.hisp.dhis.common.OpenApi;
 
 @Value
 @Builder
 @OpenApi.Shared(name = "TrackerImportError")
 public class Error {
-  private final String errorMessage;
 
-  private final String errorCode;
-
-  private final String trackerType;
-
-  private final String uid;
+  @Nonnull @JsonProperty String message;
+  @Nonnull @JsonProperty String errorCode;
+  @Nonnull @JsonProperty String trackerType;
+  @Nonnull @JsonProperty String uid;
+  @EqualsAndHashCode.Exclude @Nonnull @JsonProperty List<String> args;
 
   @JsonCreator
   public Error(
-      @JsonProperty("message") String errorMessage,
-      @JsonProperty("errorCode") String errorCode,
-      @JsonProperty("trackerType") String trackerType,
-      @JsonProperty("uid") String uid) {
-    this.errorMessage = errorMessage;
+      @Nonnull @JsonProperty("message") String message,
+      @Nonnull @JsonProperty("errorCode") String errorCode,
+      @Nonnull @JsonProperty("trackerType") String trackerType,
+      @Nonnull @JsonProperty("uid") String uid,
+      @CheckForNull @JsonProperty("args") List<String> args) {
+    this.message = message;
     this.errorCode = errorCode;
     this.trackerType = trackerType;
     this.uid = uid;
-  }
-
-  @JsonProperty
-  public String getErrorCode() {
-    return errorCode;
-  }
-
-  @JsonProperty
-  public String getMessage() {
-    return errorMessage;
-  }
-
-  @JsonProperty
-  public String getTrackerType() {
-    return trackerType;
-  }
-
-  @JsonProperty
-  public String getUid() {
-    return uid;
+    this.args = args == null ? List.of() : args;
   }
 }