From adbe377b20d2157e6653dd9fca4ef875c9cfc2c8 Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Thu, 9 Nov 2023 18:27:03 +0100 Subject: [PATCH 1/6] feat: query API for job configurations with errors [DHIS2-15276] --- .../JobConfigurationErrorParams.java | 64 +++++++++++++++++++ .../scheduling/JobConfigurationService.java | 8 +++ .../scheduling/JobConfigurationStore.java | 7 ++ .../DefaultJobConfigurationService.java | 8 +++ .../DefaultJobSchedulerLoopService.java | 2 +- .../HibernateJobConfigurationStore.java | 42 ++++++++++++ .../JobConfigurationController.java | 15 ++--- 7 files changed, 137 insertions(+), 9 deletions(-) create mode 100644 dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationErrorParams.java diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationErrorParams.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationErrorParams.java new file mode 100644 index 000000000000..98f93a2cb91b --- /dev/null +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationErrorParams.java @@ -0,0 +1,64 @@ +/* + * Copyright (c) 2004-2023, University of Oslo + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * Neither the name of the HISP project nor the names of its contributors may + * be used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR + * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package org.hisp.dhis.scheduling; + +import java.util.Date; +import java.util.List; +import javax.annotation.CheckForNull; +import lombok.Data; +import org.hisp.dhis.common.UID; +import org.hisp.dhis.feedback.ErrorCode; + +/** + * Query params when searching for {@link JobConfiguration}s with errors. + * + *

A match has to satisfy all filters (AND logic) but only one of the given codes or object + * {@link UID} (OR logic). + * + *

If any of the criteria is not defined it has no filter effect. + * + * @author Jan Bernitt + */ +@Data +public class JobConfigurationErrorParams { + + /** The user that ran the job */ + @CheckForNull private UID user; + + /** The earliest date the job ran that should be included */ + @CheckForNull private Date start; + + /** The latest date the job ran that should be included */ + @CheckForNull private Date end; + + /** The codes to select, any match combined */ + @CheckForNull private List code; + + /** The object with errors to select, any match combined */ + @CheckForNull private List object; +} 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 fd12a9b467d2..5f7fc9c2130e 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 @@ -30,6 +30,7 @@ import java.io.InputStream; import java.util.List; import java.util.Map; +import javax.annotation.Nonnull; import org.hisp.dhis.feedback.ConflictException; import org.hisp.dhis.schema.Property; import org.springframework.util.MimeType; @@ -160,6 +161,13 @@ String create(JobConfiguration config, MimeType contentType, InputStream content */ List getStaleConfigurations(int staleForSeconds); + /** + * @param params query parameters (criteria) to find + * @return all job configurations that match the query parameters + */ + @Nonnull + List findJobConfigurations(@Nonnull JobConfigurationErrorParams params); + /** * Get a map of parameter classes with appropriate properties This can be used for a frontend app * or for other appropriate applications which needs information about the jobs in the system. 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 a0d90d73df58..8333e8fc568f 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 @@ -115,6 +115,13 @@ public interface JobConfigurationStore extends GenericDimensionalObjectStore getDueJobConfigurations(boolean includeWaiting); + /** + * @param params query parameters (criteria) to find + * @return all job configurations that match the query parameters + */ + @Nonnull + List findJobConfigurations(@Nonnull JobConfigurationErrorParams params); + /** * @return A list of all job types that are currently in {@link JobStatus#RUNNING} state. */ 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 68b40d57e50f..87ceb24156ca 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 @@ -50,6 +50,7 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import javax.annotation.Nonnull; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.apache.commons.beanutils.PropertyUtils; @@ -247,6 +248,13 @@ public List getStaleConfigurations(int staleForSeconds) { return jobConfigurationStore.getStaleConfigurations(staleForSeconds); } + @Nonnull + @Override + @Transactional(readOnly = true) + public List findJobConfigurations(@Nonnull JobConfigurationErrorParams params) { + return jobConfigurationStore.findJobConfigurations(params); + } + @Override @Transactional(readOnly = true) public Map> getJobParametersSchema() { 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 27c2aedb29fa..ceb9d77c38c2 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 @@ -285,7 +285,7 @@ private void updateProgress(@Nonnull String jobId) { try { JobProgress.Progress progress = job.getProgress(); String errorCodes = - progress.getErrorCodes().stream().map(ErrorCode::name).collect(joining(" ")); + progress.getErrorCodes().stream().map(ErrorCode::name).sorted().collect(joining(" ")); jobConfigurationStore.updateProgress( jobId, jsonMapper.writeValueAsString(progress), errorCodes); } catch (JsonProcessingException ex) { 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 be63df14bf74..591c211599cf 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 @@ -30,6 +30,8 @@ import static java.lang.Math.max; import static java.util.stream.Collectors.toSet; +import com.vladmihalcea.hibernate.type.array.StringArrayType; +import java.util.Date; import java.util.List; import java.util.Set; import java.util.function.Function; @@ -39,7 +41,9 @@ import lombok.extern.slf4j.Slf4j; import org.hibernate.SessionFactory; import org.hibernate.query.NativeQuery; +import org.hisp.dhis.common.UID; import org.hisp.dhis.common.hibernate.HibernateIdentifiableObjectStore; +import org.hisp.dhis.feedback.ErrorCode; import org.hisp.dhis.security.acl.AclService; import org.hisp.dhis.user.CurrentUserService; import org.springframework.context.ApplicationEventPublisher; @@ -241,6 +245,44 @@ public Stream getDueJobConfigurations(boolean includeWaiting) .stream(); } + @Nonnull + @Override + public List findJobConfigurations(@Nonnull JobConfigurationErrorParams params) { + // language=SQL + String sql = + """ + select * from jobconfiguration + where errorcodes is not null and errorcodes != '' + and (:skipUser or executedby = :user) + and (:skipStart or lastexecuted >= :start) + and (:skipEnd or lastexecuted <= :end) + and (:skipObjects or jsonb_exists_any(progress -> 'errors', :objects )) + and (:skipCodes or string_to_array(errorcodes, ' ') && :codes) + order by lastexecuted desc; + """; + List objectList = params.getObject(); + List errors = + objectList == null ? List.of() : objectList.stream().map(UID::getValue).toList(); + List codeList = params.getCode(); + List codes = + codeList == null ? List.of() : codeList.stream().map(ErrorCode::name).toList(); + Date start = params.getStart(); + Date end = params.getEnd(); + return getSession() + .createNativeQuery(sql, JobConfiguration.class) + .setParameter("skipUser", params.getUser() == null) + .setParameter("user", params.getUser()) + .setParameter("skipStart", start == null) + .setParameter("start", start == null ? new Date() : start) + .setParameter("skipEnd", end == null) + .setParameter("end", end == null ? new Date() : end) + .setParameter("skipObjects", errors.isEmpty()) + .setParameter("objects", errors.toArray(String[]::new), StringArrayType.INSTANCE) + .setParameter("skipCodes", codes.isEmpty()) + .setParameter("codes", codes.toArray(String[]::new), StringArrayType.INSTANCE) + .list(); + } + @Override public boolean tryExecuteNow(@Nonnull String jobId) { // language=SQL 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 085f7f97c01a..58fe98b51117 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 @@ -34,15 +34,9 @@ import lombok.RequiredArgsConstructor; 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; -import org.hisp.dhis.scheduling.JobConfigurationService; -import org.hisp.dhis.scheduling.JobProgress; +import org.hisp.dhis.feedback.*; +import org.hisp.dhis.scheduling.*; import org.hisp.dhis.scheduling.JobProgress.Progress; -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; @@ -74,6 +68,11 @@ public class JobConfigurationController extends AbstractCrudController findJobConfigurations(JobConfigurationErrorParams params) { + return jobConfigurationService.findJobConfigurations(params); + } + @GetMapping("/due") public List getDueJobConfigurations( @RequestParam int seconds, From 5b1ea122fa3059b028b3a928bbdece49c25d8d5a Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Mon, 13 Nov 2023 09:57:41 +0100 Subject: [PATCH 2/6] fix: maven setup, flat job errors [DHIS2-15276] --- dhis-2/dhis-api/pom.xml | 4 ++ .../scheduling/JobConfigurationService.java | 3 +- .../scheduling/JobConfigurationStore.java | 2 +- .../org/hisp/dhis/scheduling/JobProgress.java | 14 +++-- ...rParams.java => JobsWithErrorsParams.java} | 6 +-- .../dhis-services/dhis-service-core/pom.xml | 5 ++ .../DefaultJobConfigurationService.java | 21 +++++++- .../HibernateJobConfigurationStore.java | 51 ++++++++++++------- .../dhis-support/dhis-support-commons/pom.xml | 4 ++ .../config/JacksonObjectMapperConfig.java | 2 + .../jackson/config/JsonValueSerializer.java | 23 +++++++++ .../JobConfigurationController.java | 5 +- dhis-2/dhis-web-embedded-jetty/pom.xml | 5 -- 13 files changed, 109 insertions(+), 36 deletions(-) rename dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/{JobConfigurationErrorParams.java => JobsWithErrorsParams.java} (95%) create mode 100644 dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/jackson/config/JsonValueSerializer.java diff --git a/dhis-2/dhis-api/pom.xml b/dhis-2/dhis-api/pom.xml index 397c0d4399e9..7de32b7e5330 100644 --- a/dhis-2/dhis-api/pom.xml +++ b/dhis-2/dhis-api/pom.xml @@ -97,6 +97,10 @@ org.hisp.dhis.rules rule-engine + + org.hisp.dhis + json-tree + org.springframework.security spring-security-core 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 5f7fc9c2130e..da4a12d35221 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 @@ -32,6 +32,7 @@ import java.util.Map; import javax.annotation.Nonnull; import org.hisp.dhis.feedback.ConflictException; +import org.hisp.dhis.jsontree.JsonObject; import org.hisp.dhis.schema.Property; import org.springframework.util.MimeType; @@ -166,7 +167,7 @@ String create(JobConfiguration config, MimeType contentType, InputStream content * @return all job configurations that match the query parameters */ @Nonnull - List findJobConfigurations(@Nonnull JobConfigurationErrorParams params); + List findJobsWithErrors(@Nonnull JobsWithErrorsParams params); /** * Get a map of parameter classes with appropriate properties This can be used for a frontend app 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 8333e8fc568f..1132120a946d 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 @@ -120,7 +120,7 @@ public interface JobConfigurationStore extends GenericDimensionalObjectStore findJobConfigurations(@Nonnull JobConfigurationErrorParams params); + Stream findJobsWithErrors(@Nonnull JobsWithErrorsParams params); /** * @return A list of all job types that are currently in {@link JobStatus#RUNNING} state. 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 932cbf92a6f7..360fe7b3d33e 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 @@ -147,7 +147,8 @@ default void addError(ErrorCode code, String uid, String type, Integer index, St } default void addError(ErrorCode code, String uid, String type, Integer index, List args) { - // default implementation is a NOOP, we don't remember or handle the error + // is overridden by a tracker that collects errors + // default is to not collect errors } /* @@ -590,10 +591,12 @@ public Progress( } public void addError(Error error) { - errors - .computeIfAbsent(error.getId(), key -> new ConcurrentHashMap<>()) - .computeIfAbsent(error.getCode(), key2 -> new ConcurrentLinkedQueue<>()) - .add(error); + Queue sameObjectAndCode = errors + .computeIfAbsent(error.getId(), key -> new ConcurrentHashMap<>()) + .computeIfAbsent(error.getCode(), key2 -> new ConcurrentLinkedQueue<>()); + if (sameObjectAndCode.stream().noneMatch(e -> e.args.equals(error.args))) { + sameObjectAndCode.add(error); + } } public boolean hasErrors() { @@ -625,6 +628,7 @@ final class Error { * information is not available. */ @CheckForNull @JsonProperty private final Integer index; + //TODO make a list/array to collect or have a count /** The arguments used in the {@link #code}'s {@link ErrorCode#getMessage()} template */ @Nonnull @JsonProperty private final List args; diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationErrorParams.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobsWithErrorsParams.java similarity index 95% rename from dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationErrorParams.java rename to dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobsWithErrorsParams.java index 98f93a2cb91b..9a2c11f5b2dd 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobConfigurationErrorParams.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobsWithErrorsParams.java @@ -45,16 +45,16 @@ * @author Jan Bernitt */ @Data -public class JobConfigurationErrorParams { +public class JobsWithErrorsParams { /** The user that ran the job */ @CheckForNull private UID user; /** The earliest date the job ran that should be included */ - @CheckForNull private Date start; + @CheckForNull private Date from; /** The latest date the job ran that should be included */ - @CheckForNull private Date end; + @CheckForNull private Date to; /** The codes to select, any match combined */ @CheckForNull private List code; diff --git a/dhis-2/dhis-services/dhis-service-core/pom.xml b/dhis-2/dhis-services/dhis-service-core/pom.xml index 67b693b9aacb..4110907e6117 100644 --- a/dhis-2/dhis-services/dhis-service-core/pom.xml +++ b/dhis-2/dhis-services/dhis-service-core/pom.xml @@ -117,6 +117,11 @@ org.hibernate hibernate-core + + com.vladmihalcea + hibernate-types-52 + ${hibernate-types.version} + com.fasterxml.jackson.core jackson-core 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 87ceb24156ca..0d83fda79da3 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 @@ -27,6 +27,7 @@ */ package org.hisp.dhis.scheduling; +import static java.util.stream.Collectors.joining; import static org.hisp.dhis.scheduling.JobType.values; import com.fasterxml.jackson.annotation.JsonProperty; @@ -40,6 +41,7 @@ import java.lang.reflect.Field; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; +import java.text.MessageFormat; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; @@ -48,6 +50,7 @@ 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; @@ -61,9 +64,11 @@ import org.hisp.dhis.common.NameableObject; import org.hisp.dhis.commons.util.TextUtils; import org.hisp.dhis.feedback.ConflictException; +import org.hisp.dhis.feedback.ErrorCode; import org.hisp.dhis.fileresource.FileResource; import org.hisp.dhis.fileresource.FileResourceDomain; import org.hisp.dhis.fileresource.FileResourceService; +import org.hisp.dhis.jsontree.*; import org.hisp.dhis.scheduling.JobType.Defaults; import org.hisp.dhis.schema.Property; import org.hisp.dhis.setting.SettingKey; @@ -251,8 +256,20 @@ public List getStaleConfigurations(int staleForSeconds) { @Nonnull @Override @Transactional(readOnly = true) - public List findJobConfigurations(@Nonnull JobConfigurationErrorParams params) { - return jobConfigurationStore.findJobConfigurations(params); + public List findJobsWithErrors(@Nonnull JobsWithErrorsParams params) { + Function toObject = json -> { + JsonObject obj = JsonMixed.of(json); + List 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().addMember("message", "\""+msg.replace("\"", "\\\"")+"\"")); + }))); + return JsonMixed.of(errors.node().replaceWith(JsonBuilder.createArray(arr -> flatErrors.forEach(arr::addElement)))); + }; + return jobConfigurationStore.findJobsWithErrors(params).map(toObject).toList(); } @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 591c211599cf..96e92c425d7c 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 @@ -41,6 +41,7 @@ import lombok.extern.slf4j.Slf4j; import org.hibernate.SessionFactory; import org.hibernate.query.NativeQuery; +import org.hibernate.query.Query; import org.hisp.dhis.common.UID; import org.hisp.dhis.common.hibernate.HibernateIdentifiableObjectStore; import org.hisp.dhis.feedback.ErrorCode; @@ -247,18 +248,27 @@ public Stream getDueJobConfigurations(boolean includeWaiting) @Nonnull @Override - public List findJobConfigurations(@Nonnull JobConfigurationErrorParams params) { + public Stream findJobsWithErrors(@Nonnull JobsWithErrorsParams params) { // language=SQL String sql = """ - select * from jobconfiguration - where errorcodes is not null and errorcodes != '' - and (:skipUser or executedby = :user) - and (:skipStart or lastexecuted >= :start) - and (:skipEnd or lastexecuted <= :end) - and (:skipObjects or jsonb_exists_any(progress -> 'errors', :objects )) - and (:skipCodes or string_to_array(errorcodes, ' ') && :codes) - order by lastexecuted desc; + select jsonb_build_object( + 'id', c.uid, + 'user', c.executedby, + 'created', c.created, + 'executed', c.lastexecuted, + 'finished', c.lastfinished, + 'filesize', fr.contentlength, + 'filetype', fr.contenttype, + 'errors', c.progress -> 'errors') #>> '{}' + from jobconfiguration c left join fileresource fr on c.uid = fr.uid + where c.errorcodes is not null and c.errorcodes != '' + and (:skipUser or c.executedby = :user) + and (:skipStart or c.lastexecuted >= :start) + and (:skipEnd or c.lastexecuted <= :end) + and (:skipObjects or jsonb_exists_any(c.progress -> 'errors', :objects )) + and (:skipCodes or string_to_array(c.errorcodes, ' ') && :codes) + order by c.lastexecuted desc; """; List objectList = params.getObject(); List errors = @@ -266,12 +276,12 @@ public List findJobConfigurations(@Nonnull JobConfigurationErr List codeList = params.getCode(); List codes = codeList == null ? List.of() : codeList.stream().map(ErrorCode::name).toList(); - Date start = params.getStart(); - Date end = params.getEnd(); - return getSession() - .createNativeQuery(sql, JobConfiguration.class) - .setParameter("skipUser", params.getUser() == null) - .setParameter("user", params.getUser()) + Date start = params.getFrom(); + Date end = params.getTo(); + UID user = params.getUser(); + return getResultStream(nativeQuery(sql) + .setParameter("skipUser", user == null) + .setParameter("user", user == null ? "" : user.getValue()) .setParameter("skipStart", start == null) .setParameter("start", start == null ? new Date() : start) .setParameter("skipEnd", end == null) @@ -280,7 +290,7 @@ public List findJobConfigurations(@Nonnull JobConfigurationErr .setParameter("objects", errors.toArray(String[]::new), StringArrayType.INSTANCE) .setParameter("skipCodes", codes.isEmpty()) .setParameter("codes", codes.toArray(String[]::new), StringArrayType.INSTANCE) - .list(); + , Object::toString); } @Override @@ -507,8 +517,15 @@ private static String getSingleResultOrNull(NativeQuery query) { } @SuppressWarnings("unchecked") - private static Set getResultSet(NativeQuery query, Function mapper) { + private static Set getResultSet(Query query, Function mapper) { Stream stream = (Stream) query.stream(); return stream.map(mapper).collect(toSet()); } + + @SuppressWarnings("unchecked") + private static Stream getResultStream(Query query, Function mapper) { + Stream stream = (Stream) query.stream(); + return stream.map(mapper); + } + } diff --git a/dhis-2/dhis-support/dhis-support-commons/pom.xml b/dhis-2/dhis-support/dhis-support-commons/pom.xml index f10142563aa8..ceac056ad74f 100644 --- a/dhis-2/dhis-support/dhis-support-commons/pom.xml +++ b/dhis-2/dhis-support/dhis-support-commons/pom.xml @@ -24,6 +24,10 @@ org.hisp.dhis dhis-api + + org.hisp.dhis + json-tree + org.apache.commons commons-lang3 diff --git a/dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/jackson/config/JacksonObjectMapperConfig.java b/dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/jackson/config/JacksonObjectMapperConfig.java index 9cb1ed4fe58d..7a9027a9330d 100644 --- a/dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/jackson/config/JacksonObjectMapperConfig.java +++ b/dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/jackson/config/JacksonObjectMapperConfig.java @@ -49,6 +49,7 @@ import org.hisp.dhis.commons.jackson.config.geometry.JtsXmlModule; import org.hisp.dhis.dataexchange.aggregate.Api; import org.hisp.dhis.dataexchange.aggregate.ApiSerializer; +import org.hisp.dhis.jsontree.JsonValue; import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.GeometryFactory; import org.locationtech.jts.geom.PrecisionModel; @@ -153,6 +154,7 @@ private static ObjectMapper configureMapper( module.addSerializer(Date.class, new WriteDateStdSerializer()); module.addSerializer(JsonPointer.class, new JsonPointerStdSerializer()); module.addSerializer(Api.class, new ApiSerializer()); + module.addSerializer(JsonValue.class, new JsonValueSerializer()); // Registering a custom Instant serializer/deserializer for DTOs JavaTimeModule javaTimeModule = new JavaTimeModule(); diff --git a/dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/jackson/config/JsonValueSerializer.java b/dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/jackson/config/JsonValueSerializer.java new file mode 100644 index 000000000000..d674fac465e1 --- /dev/null +++ b/dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/jackson/config/JsonValueSerializer.java @@ -0,0 +1,23 @@ +package org.hisp.dhis.commons.jackson.config; + +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.databind.JsonSerializer; +import com.fasterxml.jackson.databind.SerializerProvider; +import org.hisp.dhis.jsontree.JsonValue; + +import java.io.IOException; + +/** + * @author Jan Bernitt + */ +public class JsonValueSerializer extends JsonSerializer { + + @Override + public void serialize(JsonValue obj, JsonGenerator generator, SerializerProvider provider) throws IOException { + if (obj == null) { + generator.writeNull(); + } else { + generator.writeRawValue(obj.node().getDeclaration()); + } + } +} 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 58fe98b51117..20eccaf2659d 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.*; +import org.hisp.dhis.jsontree.JsonObject; import org.hisp.dhis.scheduling.*; import org.hisp.dhis.scheduling.JobProgress.Progress; import org.hisp.dhis.schema.Property; @@ -69,8 +70,8 @@ public class JobConfigurationController extends AbstractCrudController findJobConfigurations(JobConfigurationErrorParams params) { - return jobConfigurationService.findJobConfigurations(params); + public List findJobConfigurationErrors(JobsWithErrorsParams params) { + return jobConfigurationService.findJobsWithErrors(params); } @GetMapping("/due") diff --git a/dhis-2/dhis-web-embedded-jetty/pom.xml b/dhis-2/dhis-web-embedded-jetty/pom.xml index 247ccfd08fca..66fcb3563c24 100644 --- a/dhis-2/dhis-web-embedded-jetty/pom.xml +++ b/dhis-2/dhis-web-embedded-jetty/pom.xml @@ -34,11 +34,6 @@ dhis-service-dxf2 jar - - org.hisp.dhis - dhis-service-core - jar - org.hisp.dhis dhis-service-administration From fbc7e08131212253885e64a1af7f70f983dc25b3 Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Tue, 14 Nov 2023 09:54:24 +0100 Subject: [PATCH 3/6] fix: new json-tree version - consistent API [DHIS2-15276] --- .../scheduling/JobConfigurationService.java | 2 +- .../scheduling/JobConfigurationStore.java | 2 +- .../org/hisp/dhis/scheduling/JobProgress.java | 6 +- ...orsParams.java => JobRunErrorsParams.java} | 15 ++++- .../DefaultJobConfigurationService.java | 58 ++++++++++++++----- .../HibernateJobConfigurationStore.java | 39 ++++++++----- .../jackson/config/JsonValueSerializer.java | 45 +++++++++++--- .../JobConfigurationController.java | 22 ++++++- dhis-2/pom.xml | 2 +- 9 files changed, 143 insertions(+), 48 deletions(-) rename dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/{JobsWithErrorsParams.java => JobRunErrorsParams.java} (85%) 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 da4a12d35221..0e62820ab32d 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 @@ -167,7 +167,7 @@ String create(JobConfiguration config, MimeType contentType, InputStream content * @return all job configurations that match the query parameters */ @Nonnull - List findJobsWithErrors(@Nonnull JobsWithErrorsParams params); + List findJobRunErrors(@Nonnull JobRunErrorsParams params); /** * Get a map of parameter classes with appropriate properties This can be used for a frontend app 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 1132120a946d..40402cdd79db 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 @@ -120,7 +120,7 @@ public interface JobConfigurationStore extends GenericDimensionalObjectStore findJobsWithErrors(@Nonnull JobsWithErrorsParams params); + Stream findJobRunErrors(@Nonnull JobRunErrorsParams params); /** * @return A list of all job types that are currently in {@link JobStatus#RUNNING} state. 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 360fe7b3d33e..d0adcfc0d334 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 @@ -591,7 +591,8 @@ public Progress( } public void addError(Error error) { - Queue sameObjectAndCode = errors + Queue sameObjectAndCode = + errors .computeIfAbsent(error.getId(), key -> new ConcurrentHashMap<>()) .computeIfAbsent(error.getCode(), key2 -> new ConcurrentLinkedQueue<>()); if (sameObjectAndCode.stream().noneMatch(e -> e.args.equals(error.args))) { @@ -628,7 +629,8 @@ final class Error { * information is not available. */ @CheckForNull @JsonProperty private final Integer index; - //TODO make a list/array to collect or have a count + + // TODO make a list/array to collect or have a count /** The arguments used in the {@link #code}'s {@link ErrorCode#getMessage()} template */ @Nonnull @JsonProperty private final List args; diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobsWithErrorsParams.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobRunErrorsParams.java similarity index 85% rename from dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobsWithErrorsParams.java rename to dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobRunErrorsParams.java index 9a2c11f5b2dd..f7d0f28c74e1 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobsWithErrorsParams.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/scheduling/JobRunErrorsParams.java @@ -31,8 +31,11 @@ import java.util.List; import javax.annotation.CheckForNull; import lombok.Data; +import lombok.experimental.Accessors; +import org.hisp.dhis.common.OpenApi; import org.hisp.dhis.common.UID; import org.hisp.dhis.feedback.ErrorCode; +import org.hisp.dhis.user.User; /** * Query params when searching for {@link JobConfiguration}s with errors. @@ -45,10 +48,15 @@ * @author Jan Bernitt */ @Data -public class JobsWithErrorsParams { +@Accessors(chain = true) +public class JobRunErrorsParams { + + @OpenApi.Ignore @CheckForNull private UID job; /** The user that ran the job */ - @CheckForNull private UID user; + @OpenApi.Property({UID.class, User.class}) + @CheckForNull + private UID user; /** The earliest date the job ran that should be included */ @CheckForNull private Date from; @@ -61,4 +69,7 @@ public class JobsWithErrorsParams { /** The object with errors to select, any match combined */ @CheckForNull private List object; + + /** The {@link JobType} with errors to select, any match combined */ + @CheckForNull private List type; } 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 0d83fda79da3..28adb9f5c462 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 @@ -27,7 +27,6 @@ */ package org.hisp.dhis.scheduling; -import static java.util.stream.Collectors.joining; import static org.hisp.dhis.scheduling.JobType.values; import com.fasterxml.jackson.annotation.JsonProperty; @@ -256,20 +255,49 @@ public List getStaleConfigurations(int staleForSeconds) { @Nonnull @Override @Transactional(readOnly = true) - public List findJobsWithErrors(@Nonnull JobsWithErrorsParams params) { - Function toObject = json -> { - JsonObject obj = JsonMixed.of(json); - List 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().addMember("message", "\""+msg.replace("\"", "\\\"")+"\"")); - }))); - return JsonMixed.of(errors.node().replaceWith(JsonBuilder.createArray(arr -> flatErrors.forEach(arr::addElement)))); - }; - return jobConfigurationStore.findJobsWithErrors(params).map(toObject).toList(); + public List findJobRunErrors(@Nonnull JobRunErrorsParams params) { + Function toObject = + json -> { + JsonObject obj = JsonMixed.of(json); + List 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(); } @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 96e92c425d7c..5a2e4b7f294f 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 @@ -248,12 +248,13 @@ public Stream getDueJobConfigurations(boolean includeWaiting) @Nonnull @Override - public Stream findJobsWithErrors(@Nonnull JobsWithErrorsParams params) { + public Stream findJobRunErrors(@Nonnull JobRunErrorsParams params) { // language=SQL String sql = """ select jsonb_build_object( 'id', c.uid, + 'type', c.jobType, 'user', c.executedby, 'created', c.created, 'executed', c.lastexecuted, @@ -263,11 +264,13 @@ select jsonb_build_object( 'errors', c.progress -> 'errors') #>> '{}' from jobconfiguration c left join fileresource fr on c.uid = fr.uid where c.errorcodes is not null and c.errorcodes != '' + and (:skipUid or c.uid = :uid) and (:skipUser or c.executedby = :user) and (:skipStart or c.lastexecuted >= :start) and (:skipEnd or c.lastexecuted <= :end) and (:skipObjects or jsonb_exists_any(c.progress -> 'errors', :objects )) and (:skipCodes or string_to_array(c.errorcodes, ' ') && :codes) + and (:skipTypes or c.jobtype = any (:types)) order by c.lastexecuted desc; """; List objectList = params.getObject(); @@ -276,21 +279,30 @@ select jsonb_build_object( List codeList = params.getCode(); List codes = codeList == null ? List.of() : codeList.stream().map(ErrorCode::name).toList(); + List typeList = params.getType(); + List types = + typeList == null ? List.of() : typeList.stream().map(JobType::name).toList(); Date start = params.getFrom(); Date end = params.getTo(); UID user = params.getUser(); - return getResultStream(nativeQuery(sql) - .setParameter("skipUser", user == null) - .setParameter("user", user == null ? "" : user.getValue()) - .setParameter("skipStart", start == null) - .setParameter("start", start == null ? new Date() : start) - .setParameter("skipEnd", end == null) - .setParameter("end", end == null ? new Date() : end) - .setParameter("skipObjects", errors.isEmpty()) - .setParameter("objects", errors.toArray(String[]::new), StringArrayType.INSTANCE) - .setParameter("skipCodes", codes.isEmpty()) - .setParameter("codes", codes.toArray(String[]::new), StringArrayType.INSTANCE) - , Object::toString); + UID job = params.getJob(); + return getResultStream( + nativeQuery(sql) + .setParameter("skipUid", job == null) + .setParameter("uid", job == null ? "" : job.getValue()) + .setParameter("skipUser", user == null) + .setParameter("user", user == null ? "" : user.getValue()) + .setParameter("skipStart", start == null) + .setParameter("start", start == null ? new Date() : start) + .setParameter("skipEnd", end == null) + .setParameter("end", end == null ? new Date() : end) + .setParameter("skipObjects", errors.isEmpty()) + .setParameter("objects", errors.toArray(String[]::new), StringArrayType.INSTANCE) + .setParameter("skipCodes", codes.isEmpty()) + .setParameter("codes", codes.toArray(String[]::new), StringArrayType.INSTANCE) + .setParameter("skipTypes", types.isEmpty()) + .setParameter("types", types.toArray(String[]::new), StringArrayType.INSTANCE), + Object::toString); } @Override @@ -527,5 +539,4 @@ private static Stream getResultStream(Query query, Function Stream stream = (Stream) query.stream(); return stream.map(mapper); } - } diff --git a/dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/jackson/config/JsonValueSerializer.java b/dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/jackson/config/JsonValueSerializer.java index d674fac465e1..cbd08c9b9ccc 100644 --- a/dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/jackson/config/JsonValueSerializer.java +++ b/dhis-2/dhis-support/dhis-support-commons/src/main/java/org/hisp/dhis/commons/jackson/config/JsonValueSerializer.java @@ -1,23 +1,50 @@ +/* + * Copyright (c) 2004-2023, University of Oslo + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * Neither the name of the HISP project nor the names of its contributors may + * be used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR + * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ package org.hisp.dhis.commons.jackson.config; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.databind.JsonSerializer; import com.fasterxml.jackson.databind.SerializerProvider; -import org.hisp.dhis.jsontree.JsonValue; - import java.io.IOException; +import org.hisp.dhis.jsontree.JsonValue; /** * @author Jan Bernitt */ public class JsonValueSerializer extends JsonSerializer { - @Override - public void serialize(JsonValue obj, JsonGenerator generator, SerializerProvider provider) throws IOException { - if (obj == null) { - generator.writeNull(); - } else { - generator.writeRawValue(obj.node().getDeclaration()); - } + @Override + public void serialize(JsonValue obj, JsonGenerator generator, SerializerProvider provider) + throws IOException { + if (obj == null) { + generator.writeNull(); + } else { + generator.writeRawValue(obj.node().getDeclaration()); } + } } 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 20eccaf2659d..11aae6ce13d6 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 @@ -34,7 +34,9 @@ import lombok.RequiredArgsConstructor; import org.hisp.dhis.common.IdentifiableObjects; import org.hisp.dhis.common.OpenApi; +import org.hisp.dhis.common.UID; import org.hisp.dhis.feedback.*; +import org.hisp.dhis.jsontree.JsonMixed; import org.hisp.dhis.jsontree.JsonObject; import org.hisp.dhis.scheduling.*; import org.hisp.dhis.scheduling.JobProgress.Progress; @@ -70,8 +72,22 @@ public class JobConfigurationController extends AbstractCrudController findJobConfigurationErrors(JobsWithErrorsParams params) { - return jobConfigurationService.findJobsWithErrors(params); + public List getJobRunErrors(JobRunErrorsParams params) { + return jobConfigurationService.findJobRunErrors(params); + } + + @GetMapping("{uid}/errors") + public JsonObject getJobRunErrors( + @PathVariable("uid") @OpenApi.Param({UID.class, JobConfiguration.class}) UID uid) + throws NotFoundException { + List errors = + jobConfigurationService.findJobRunErrors(new JobRunErrorsParams().setJob(uid)); + if (errors.isEmpty()) { + JobConfiguration obj = jobConfigurationService.getJobConfigurationByUid(uid.getValue()); + if (obj == null) throw new NotFoundException(JobConfiguration.class, uid.getValue()); + return JsonMixed.of("{}"); + } + return errors.get(0); } @GetMapping("/due") @@ -131,7 +147,7 @@ public Progress getProgress(@PathVariable("uid") String uid) { } @PreAuthorize("hasRole('ALL') or hasRole('F_PERFORM_MAINTENANCE')") - @GetMapping("{uid}/errors") + @GetMapping("{uid}/progress/errors") public List getErrors(@PathVariable("uid") String uid) { return jobSchedulerService.getErrors(uid); } diff --git a/dhis-2/pom.xml b/dhis-2/pom.xml index ac33d71dda78..feba456c5748 100644 --- a/dhis-2/pom.xml +++ b/dhis-2/pom.xml @@ -91,7 +91,7 @@ 1.4.1 2.0.0 - 0.10.1 + 0.10.3 5.8.8 From bc8c6db50df79041e40b4672e24e58e35bcba83f Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Tue, 14 Nov 2023 12:47:14 +0100 Subject: [PATCH 4/6] chore: remove unused index field [DHIS2-15276] --- .../org/hisp/dhis/scheduling/JobProgress.java | 22 +++++++------------ .../dhis/scheduling/RecordingJobProgress.java | 5 ++--- .../metadata/MetadataImportJob.java | 1 - 3 files changed, 10 insertions(+), 18 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 index d0adcfc0d334..ee0f14d50f0f 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 @@ -142,11 +142,16 @@ default boolean isSkipCurrentStage() { Error reporting API: */ - default void addError(ErrorCode code, String uid, String type, Integer index, String... args) { - addError(code, uid, type, index, List.of(args)); + default void addError( + @Nonnull ErrorCode code, @CheckForNull String uid, @Nonnull String type, String... args) { + addError(code, uid, type, List.of(args)); } - default void addError(ErrorCode code, String uid, String type, Integer index, List args) { + default void addError( + @Nonnull ErrorCode code, + @CheckForNull String uid, + @Nonnull String type, + @Nonnull List args) { // is overridden by a tracker that collects errors // default is to not collect errors } @@ -623,15 +628,6 @@ final class Error { /** 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; - - // TODO make a list/array to collect or have a count - /** The arguments used in the {@link #code}'s {@link ErrorCode#getMessage()} template */ @Nonnull @JsonProperty private final List args; @@ -648,12 +644,10 @@ 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; } } 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 2d0938b9ceb7..f3367f15185a 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 @@ -149,13 +149,12 @@ 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)); + progress.addError(new Error(code, uid == null ? "" : uid, type, args)); } catch (Exception ex) { - log.error("Failed to add error: %s %s %s %d %s".formatted(code, uid, type, index, args), ex); + log.error("Failed to add error: %s %s %s %s".formatted(code, uid, type, args), ex); } } 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 20c76b3ae303..6eec6a407665 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 @@ -108,7 +108,6 @@ public void execute(JobConfiguration config, JobProgress progress) { r.getErrorCode(), r.getMainId(), r.getMainKlass().getSimpleName(), - null, r.getArgs())); } From e06fb4e5c8f35f57b3d1b7e7bac810b1d95fd919 Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Wed, 15 Nov 2023 13:11:44 +0100 Subject: [PATCH 5/6] test: adds integration tests for new API [DHIS2-15276] --- .../webapi/DhisControllerIntegrationTest.java | 12 ++ ...bConfigurationRunErrorsControllerTest.java | 138 ++++++++++++++++++ 2 files changed, 150 insertions(+) create mode 100644 dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/JobConfigurationRunErrorsControllerTest.java diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/DhisControllerIntegrationTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/DhisControllerIntegrationTest.java index 512e4518a0cb..534feb02ea57 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/DhisControllerIntegrationTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/DhisControllerIntegrationTest.java @@ -27,6 +27,8 @@ */ package org.hisp.dhis.webapi; +import java.time.Duration; +import java.util.function.BooleanSupplier; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.core.config.Configurator; import org.hisp.dhis.IntegrationTest; @@ -107,4 +109,14 @@ protected void integrationTestBefore() throws Exception { Configurator.setRootLevel(Level.INFO); } } + + protected static boolean await(Duration timeout, BooleanSupplier test) + throws InterruptedException { + while (!timeout.isNegative() && !test.getAsBoolean()) { + Thread.sleep(20); + timeout = timeout.minusMillis(20); + } + if (!timeout.isNegative()) return true; + return test.getAsBoolean(); + } } diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/JobConfigurationRunErrorsControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/JobConfigurationRunErrorsControllerTest.java new file mode 100644 index 000000000000..f05808e3eee2 --- /dev/null +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/JobConfigurationRunErrorsControllerTest.java @@ -0,0 +1,138 @@ +/* + * Copyright (c) 2004-2023, University of Oslo + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * Neither the name of the HISP project nor the names of its contributors may + * be used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR + * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package org.hisp.dhis.webapi.controller; + +import static java.time.Duration.ofSeconds; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.function.BooleanSupplier; +import org.hisp.dhis.jsontree.JsonArray; +import org.hisp.dhis.jsontree.JsonMixed; +import org.hisp.dhis.jsontree.JsonNodeType; +import org.hisp.dhis.jsontree.JsonObject; +import org.hisp.dhis.scheduling.JobStatus; +import org.hisp.dhis.web.HttpStatus; +import org.hisp.dhis.webapi.DhisControllerIntegrationTest; +import org.hisp.dhis.webapi.json.domain.JsonJobConfiguration; +import org.hisp.dhis.webapi.json.domain.JsonWebMessage; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Tests the job run error result API. + * + * @author Jan Bernitt + */ +public class JobConfigurationRunErrorsControllerTest extends DhisControllerIntegrationTest { + + private String jobId; + + @BeforeEach + void setUp() throws InterruptedException { + jobId = createAndRunImportWithErrors(); + } + + @Test + void testGetJobRunErrors_List() { + JsonArray list = GET("/jobConfigurations/errors").content(); + + assertEquals(1, list.size()); + JsonObject job = list.getObject(0); + assertEquals(jobId, job.getString("id").string()); + assertEquals(1, job.getArray("errors").size()); + } + + @Test + void testGetJobRunErrors_ListFilterUser() { + JsonArray list = + GET("/jobConfigurations/errors?user={user}", getCurrentUser().getUid()).content(); + assertEquals(1, list.size()); + assertEquals(0, GET("/jobConfigurations/errors?user=abcde123456").content().size()); + } + + @Test + void testGetJobRunErrors_ListFilterFrom() { + assertEquals(1, GET("/jobConfigurations/errors?from=2023-01-01").content().size()); + assertEquals(0, GET("/jobConfigurations/errors?from=2033-01-01").content().size()); + } + + @Test + void testGetJobRunErrors_ListFilterTo() { + assertEquals(1, GET("/jobConfigurations/errors?to=2033-01-01").content().size()); + assertEquals(0, GET("/jobConfigurations/errors?to=2023-01-01").content().size()); + } + + @Test + void testGetJobRunErrors_ListFilterCode() { + assertEquals(1, GET("/jobConfigurations/errors?code=E4000").content().size()); + assertEquals(0, GET("/jobConfigurations/errors?code=E5000").content().size()); + } + + @Test + void testGetJobRunErrors_ListFilterType() { + assertEquals(1, GET("/jobConfigurations/errors?type=METADATA_IMPORT").content().size()); + assertEquals(0, GET("/jobConfigurations/errors?type=DATA_INTEGRITY").content().size()); + } + + @Test + void testGetJobRunErrors_Object() { + JsonObject job = GET("/jobConfigurations/{uid}/errors", jobId).content(); + assertEquals(jobId, job.getString("id").string()); + assertEquals("METADATA_IMPORT", job.getString("type").string()); + assertTrue(job.has("created", "executed", "finished", "user", "filesize", "filetype")); + assertEquals(1, job.getArray("errors").size()); + } + + @Test + void testGetJobRunErrors_ObjectProgressErrors() { + JsonArray errors = GET("/jobConfigurations/{uid}/progress/errors", jobId).content(); + assertEquals(JsonNodeType.ARRAY, errors.node().getType()); + assertEquals(1, errors.size()); + } + + private String createAndRunImportWithErrors() throws InterruptedException { + JsonWebMessage message = + POST( + "/metadata?async=true", + "{'organisationUnits':[{'name':'My Unit', 'shortName':'OU1'}]}") + .content(HttpStatus.OK) + .as(JsonWebMessage.class); + String jobId = message.getString("response.id").string(); + + BooleanSupplier jobCompleted = + () -> isDone(GET("/jobConfigurations/{id}/gist?fields=id,jobStatus", jobId).content()); + assertTrue(await(ofSeconds(10), jobCompleted), "import did not run"); + return jobId; + } + + private static boolean isDone(JsonMixed config) { + JsonJobConfiguration c = config.as(JsonJobConfiguration.class); + return c.getJobStatus() == JobStatus.COMPLETED || c.getJobStatus() == JobStatus.DISABLED; + } +} From bda40068fc509e4c56c0356723b511e5aee9683f Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Wed, 15 Nov 2023 13:36:12 +0100 Subject: [PATCH 6/6] fix: sonar warnings --- .../controller/JobConfigurationRunErrorsControllerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/JobConfigurationRunErrorsControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/JobConfigurationRunErrorsControllerTest.java index f05808e3eee2..0a4971250222 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/JobConfigurationRunErrorsControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/JobConfigurationRunErrorsControllerTest.java @@ -49,7 +49,7 @@ * * @author Jan Bernitt */ -public class JobConfigurationRunErrorsControllerTest extends DhisControllerIntegrationTest { +class JobConfigurationRunErrorsControllerTest extends DhisControllerIntegrationTest { private String jobId;