Skip to content

Commit

Permalink
feat: improved data integrity execution [DHIS2-16223] (2.39) (#15823)
Browse files Browse the repository at this point in the history
* feat: improved data integrity execution [DHIS2-16223] (2.40) (#15804)

* feat: improved data integrity execution [DHIS2-16223] (2.40)

* fix: compiles again

* fix: update YAML and schema to master

* fix: job creation in controller

* fix: revert table renames

* fix: test setup

* test: backport of tests

* fix: backport of JSON test models

* fix: exception in test setup

* fix: compile error missing constants

* fix: removed moved test
  • Loading branch information
jbee authored Dec 4, 2023
1 parent 77e2be1 commit 29f45b4
Show file tree
Hide file tree
Showing 166 changed files with 12,773 additions and 212 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,16 @@
*/
package org.hisp.dhis.dataintegrity;

import static java.util.Comparator.comparingLong;
import static java.util.stream.Collectors.joining;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import java.io.Serializable;
import java.util.Comparator;
import java.util.function.Function;
import java.util.stream.Stream;
import javax.annotation.Nonnull;
import lombok.AccessLevel;
import lombok.AllArgsConstructor;
import lombok.Builder;
Expand All @@ -50,23 +57,58 @@
@Builder
@AllArgsConstructor(access = AccessLevel.PRIVATE)
public final class DataIntegrityCheck implements Serializable {
@JsonProperty private final String name;

@JsonProperty private final String displayName;
public static final Comparator<DataIntegrityCheck> FAST_TO_SLOW =
comparingLong(DataIntegrityCheck::getExecutionTimeIndicator);

@JsonProperty private final String name;
@JsonProperty private final String displayName;
@JsonProperty private final String section;

@JsonProperty private final int sectionOrder;
@JsonProperty private final DataIntegritySeverity severity;

@JsonProperty private final String description;

@JsonProperty private final String introduction;

@JsonProperty private final String recommendation;

@JsonProperty private final String issuesIdType;
@JsonProperty private final boolean isSlow;
@JsonProperty private final boolean isProgrammatic;

private long executionTime;
private int executionCount;

public @JsonProperty Long getAverageExecutionTime() {
return executionTime <= 0L ? null : executionTime / executionCount;
}

@JsonIgnore
long getExecutionTimeIndicator() {
Long time = getAverageExecutionTime();
if (time != null) return time;
return isSlow ? Long.MAX_VALUE : 1000;
}

@JsonProperty
public String getCode() {
return getCodeFromName(name);
}

private final transient Function<DataIntegrityCheck, DataIntegritySummary> runSummaryCheck;

private final transient Function<DataIntegrityCheck, DataIntegrityDetails> runDetailsCheck;

public DataIntegrityCheck addExecution(long time) {
executionCount++;
executionTime += time;
return this;
}

/**
* Method that takes in a name of a {@link DataIntegrityCheck} and converts it to an acronym of
* its name using the first letter from each word e.g. my_data_integrity_check -> MDIC
*/
public static String getCodeFromName(@Nonnull String fullName) {
return Stream.of(fullName.split("_"))
.map(word -> String.valueOf(word.charAt(0)).toUpperCase())
.collect(joining());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public enum DataIntegrityCheckType {
PROGRAM_RULE_ACTIONS_WITHOUT_DATA_OBJECT,
PROGRAM_RULE_ACTIONS_WITHOUT_NOTIFICATION,
PROGRAM_RULE_ACTIONS_WITHOUT_SECTION,
PROGRAM_RULE_ACTIONS_WITHOUT_STAGE;
PROGRAM_RULE_ACTIONS_WITHOUT_STAGE_ID;

public String getName() {
return name().toLowerCase();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
public class DataIntegrityDetails implements Serializable {
@JsonUnwrapped private final DataIntegrityCheck check;

@JsonProperty private final Date startTime;

@JsonProperty private final Date finishedTime;

@JsonProperty private final String error;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.Collection;
import java.util.Map;
import java.util.Set;
import javax.annotation.Nonnull;
import org.hisp.dhis.scheduling.JobProgress;

/**
Expand All @@ -46,23 +47,39 @@ public interface DataIntegrityService {
* kept for backwards compatibility until new UI exists
*/
@Deprecated(since = "2.38", forRemoval = true)
@Nonnull
FlattenedDataIntegrityReport getReport(Set<String> checks, JobProgress progress);

/*
* New generic API
*/

default Collection<DataIntegrityCheck> getDataIntegrityChecks() {
default @Nonnull Collection<DataIntegrityCheck> getDataIntegrityChecks() {
return getDataIntegrityChecks(Set.of());
}

@Nonnull
Collection<DataIntegrityCheck> getDataIntegrityChecks(Set<String> checks);

Map<String, DataIntegritySummary> getSummaries(Set<String> checks, long timeout);
@Nonnull
Map<String, DataIntegritySummary> getSummaries(@Nonnull Set<String> checks, long timeout);

Map<String, DataIntegrityDetails> getDetails(Set<String> checks, long timeout);
@Nonnull
Map<String, DataIntegrityDetails> getDetails(@Nonnull Set<String> checks, long timeout);

void runSummaryChecks(Set<String> checks, JobProgress progress);
void runSummaryChecks(@Nonnull Set<String> checks, JobProgress progress);

void runDetailsChecks(Set<String> checks, JobProgress progress);
void runDetailsChecks(@Nonnull Set<String> checks, JobProgress progress);

@Nonnull
Set<String> getRunningSummaryChecks();

@Nonnull
Set<String> getRunningDetailsChecks();

@Nonnull
Set<String> getCompletedSummaryChecks();

@Nonnull
Set<String> getCompletedDetailsChecks();
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
public class DataIntegritySummary implements Serializable {
@JsonUnwrapped private final DataIntegrityCheck check;

@JsonProperty private final Date startTime;

@JsonProperty private final Date finishedTime;

@JsonProperty private final String error;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,8 @@ public FlattenedDataIntegrityReport(Map<String, DataIntegrityDetails> detailsByN
DataIntegrityCheckType.PROGRAM_RULE_ACTIONS_WITHOUT_SECTION.getName()));
this.programRuleActionsWithNoStageId =
mapOfRefsByDisplayNameOrUid(
detailsByName.get(DataIntegrityCheckType.PROGRAM_RULE_ACTIONS_WITHOUT_STAGE.getName()));
detailsByName.get(
DataIntegrityCheckType.PROGRAM_RULE_ACTIONS_WITHOUT_STAGE_ID.getName()));
}

private List<String> listOfDisplayNameOrUid(DataIntegrityDetails details) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.hisp.dhis.scheduling.parameters.AggregateDataExchangeJobParameters;
import org.hisp.dhis.scheduling.parameters.AnalyticsJobParameters;
import org.hisp.dhis.scheduling.parameters.ContinuousAnalyticsJobParameters;
import org.hisp.dhis.scheduling.parameters.DataIntegrityDetailsJobParameters;
import org.hisp.dhis.scheduling.parameters.DataIntegrityJobParameters;
import org.hisp.dhis.scheduling.parameters.DataSynchronizationJobParameters;
import org.hisp.dhis.scheduling.parameters.DisableInactiveUsersJobParameters;
Expand Down Expand Up @@ -341,6 +342,9 @@ public void setDelay(Integer delay) {
value = TrackerTrigramIndexJobParameters.class,
name = "TRACKER_SEARCH_OPTIMIZATION"),
@JsonSubTypes.Type(value = DataIntegrityJobParameters.class, name = "DATA_INTEGRITY"),
@JsonSubTypes.Type(
value = DataIntegrityDetailsJobParameters.class,
name = "DATA_INTEGRITY_DETAILS"),
@JsonSubTypes.Type(
value = AggregateDataExchangeJobParameters.class,
name = "AGGREGATE_DATA_EXCHANGE"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.hisp.dhis.scheduling.parameters.AggregateDataExchangeJobParameters;
import org.hisp.dhis.scheduling.parameters.AnalyticsJobParameters;
import org.hisp.dhis.scheduling.parameters.ContinuousAnalyticsJobParameters;
import org.hisp.dhis.scheduling.parameters.DataIntegrityDetailsJobParameters;
import org.hisp.dhis.scheduling.parameters.DataIntegrityJobParameters;
import org.hisp.dhis.scheduling.parameters.DataSynchronizationJobParameters;
import org.hisp.dhis.scheduling.parameters.DisableInactiveUsersJobParameters;
Expand Down Expand Up @@ -61,6 +62,11 @@ public enum JobType {
SchedulingType.CRON,
DataIntegrityJobParameters.class,
Map.of("checks", "/api/dataIntegrity")),
DATA_INTEGRITY_DETAILS(
true,
SchedulingType.CRON,
DataIntegrityDetailsJobParameters.class,
Map.of("checks", "/api/dataIntegrity")),
RESOURCE_TABLE(true),
ANALYTICS_TABLE(
true,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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.parameters;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty;
import java.util.Set;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
import org.hisp.dhis.common.DxfNamespaces;
import org.hisp.dhis.scheduling.JobParameters;

/**
* @author Jan Bernitt
*/
@Getter
@Setter
@NoArgsConstructor
@AllArgsConstructor
public class DataIntegrityDetailsJobParameters implements JobParameters {

@JsonProperty(required = false)
@JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0)
private Set<String> checks;
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty;
import java.util.Set;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
import org.hisp.dhis.common.DxfNamespaces;
import org.hisp.dhis.scheduling.JobParameters;
Expand All @@ -40,6 +42,8 @@
*/
@Getter
@Setter
@NoArgsConstructor
@AllArgsConstructor
public class DataIntegrityJobParameters implements JobParameters {
public enum DataIntegrityReportType {
REPORT,
Expand All @@ -49,9 +53,9 @@ public enum DataIntegrityReportType {

@JsonProperty(required = false)
@JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0)
private Set<String> checks;
private DataIntegrityReportType type;

@JsonProperty(required = false)
@JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0)
private DataIntegrityReportType type;
private Set<String> checks;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
* 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.dataintegrity;

import static java.util.stream.Collectors.toList;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.List;
import org.junit.jupiter.api.Test;

/**
* @author Jan Bernitt
*/
class DataIntegrityCheckTest {

@Test
void testSortingFastToSlow_NoExecutionTime() {
List<DataIntegrityCheck> checks =
List.of(
DataIntegrityCheck.builder().name("a").isSlow(true).build(),
DataIntegrityCheck.builder().name("b").isSlow(false).build(),
DataIntegrityCheck.builder().name("c").isSlow(true).build());
List<DataIntegrityCheck> actual =
checks.stream().sorted(DataIntegrityCheck.FAST_TO_SLOW).collect(toList());
assertEquals(
List.of("b", "a", "c"), actual.stream().map(DataIntegrityCheck::getName).collect(toList()));
}

@Test
void testSortingFastToSlow() {
List<DataIntegrityCheck> checks =
List.of(
DataIntegrityCheck.builder().name("a").isSlow(true).build().addExecution(12345),
DataIntegrityCheck.builder().name("b").isSlow(false).build().addExecution(20),
DataIntegrityCheck.builder().name("c").isSlow(true).build().addExecution(500));
List<DataIntegrityCheck> actual =
checks.stream().sorted(DataIntegrityCheck.FAST_TO_SLOW).collect(toList());
assertEquals(
List.of("b", "c", "a"), actual.stream().map(DataIntegrityCheck::getName).collect(toList()));
}

@Test
void testSortingFastToSlow_MixedExecutionTime() {
List<DataIntegrityCheck> checks =
List.of(
DataIntegrityCheck.builder().name("a").isSlow(true).build().addExecution(12345),
DataIntegrityCheck.builder().name("b").isSlow(false).build(),
DataIntegrityCheck.builder().name("c").isSlow(true).build().addExecution(500));
List<DataIntegrityCheck> actual =
checks.stream().sorted(DataIntegrityCheck.FAST_TO_SLOW).collect(toList());
assertEquals(
List.of("c", "b", "a"), actual.stream().map(DataIntegrityCheck::getName).collect(toList()));
}

@Test
void testAddExecution() {
DataIntegrityCheck check = DataIntegrityCheck.builder().name("a").build();

check.addExecution(1000).addExecution(1000); // sum is 2000, 2 times

assertEquals(1000L, check.getAverageExecutionTime());

check.addExecution(4000); // sum is now 6000, 3 times

assertEquals(2000L, check.getAverageExecutionTime());
}
}
Loading

0 comments on commit 29f45b4

Please sign in to comment.