From 0120de6e5e509b4cde0e058504b4017025931f9e Mon Sep 17 00:00:00 2001 From: Enrico Date: Fri, 18 Oct 2024 12:01:24 +0200 Subject: [PATCH] chore: Clean up data values persistence [DHIS2-18223] --- .../imports/bundle/TrackerObjectsMapper.java | 19 -- .../bundle/persister/EventPersister.java | 175 ++++++++---------- .../tracker/imports/domain/DataValue.java | 5 - .../event/DataValuesValidatorTest.java | 49 ----- 4 files changed, 81 insertions(+), 167 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/TrackerObjectsMapper.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/TrackerObjectsMapper.java index f5a61780045f..6533430f2a2c 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/TrackerObjectsMapper.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/TrackerObjectsMapper.java @@ -38,9 +38,7 @@ import java.util.stream.Collectors; import javax.annotation.Nonnull; import org.hisp.dhis.category.CategoryOptionCombo; -import org.hisp.dhis.dataelement.DataElement; import org.hisp.dhis.event.EventStatus; -import org.hisp.dhis.eventdatavalue.EventDataValue; import org.hisp.dhis.note.Note; import org.hisp.dhis.organisationunit.OrganisationUnit; import org.hisp.dhis.program.Enrollment; @@ -54,7 +52,6 @@ import org.hisp.dhis.relationship.RelationshipType; import org.hisp.dhis.trackedentity.TrackedEntity; import org.hisp.dhis.trackedentity.TrackedEntityType; -import org.hisp.dhis.tracker.imports.domain.DataValue; import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat; import org.hisp.dhis.tracker.imports.util.RelationshipKeySupport; import org.hisp.dhis.user.User; @@ -240,22 +237,6 @@ private TrackerObjectsMapper() { assignedUser.ifPresent(dbEvent::setAssignedUser); } - // TODO(DHIS2-18223): Remove data value mapping and fix changelog logic - for (DataValue dataValue : event.getDataValues()) { - DataElement dataElement = preheat.getDataElement(dataValue.getDataElement()); - - EventDataValue eventDataValue = new EventDataValue(); - eventDataValue.setDataElement(dataElement.getUid()); - eventDataValue.setCreated(DateUtils.fromInstant(dataValue.getCreatedAt())); - eventDataValue.setCreatedByUserInfo(UserInfoSnapshot.from(user)); - eventDataValue.setValue(dataValue.getValue()); - eventDataValue.setLastUpdated(now); - eventDataValue.setProvidedElsewhere(dataValue.isProvidedElsewhere()); - eventDataValue.setLastUpdatedByUserInfo(UserInfoSnapshot.from(user)); - - dbEvent.getEventDataValues().add(eventDataValue); - } - if (isNotEmpty(event.getNotes())) { dbEvent .getNotes() diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/EventPersister.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/EventPersister.java index fcf9ae0fff1e..38fd271e1361 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/EventPersister.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/EventPersister.java @@ -27,10 +27,7 @@ */ package org.hisp.dhis.tracker.imports.bundle.persister; -import static com.google.common.base.Preconditions.checkNotNull; - import jakarta.persistence.EntityManager; -import java.time.Instant; import java.util.ArrayList; import java.util.Collections; import java.util.Date; @@ -41,8 +38,8 @@ import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; -import lombok.Builder; -import lombok.Data; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; import org.apache.commons.lang3.StringUtils; import org.hisp.dhis.changelog.ChangeLogType; import org.hisp.dhis.common.UID; @@ -50,6 +47,7 @@ import org.hisp.dhis.event.EventStatus; import org.hisp.dhis.eventdatavalue.EventDataValue; import org.hisp.dhis.program.Event; +import org.hisp.dhis.program.UserInfoSnapshot; import org.hisp.dhis.reservedvalue.ReservedValueService; import org.hisp.dhis.tracker.TrackerType; import org.hisp.dhis.tracker.export.event.EventChangeLogService; @@ -62,7 +60,6 @@ import org.hisp.dhis.tracker.imports.job.TrackerNotificationDataBundle; import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat; import org.hisp.dhis.user.UserDetails; -import org.hisp.dhis.util.DateUtils; import org.springframework.stereotype.Component; /** @@ -172,63 +169,94 @@ private void handleDataValues( payloadDataValues.forEach( dv -> { DataElement dataElement = preheat.getDataElement(dv.getDataElement()); - checkNotNull( - dataElement, - "Data element should never be NULL here if validation is enforced before commit."); // EventDataValue.dataElement contains a UID EventDataValue eventDataValue = dataValueDBMap.get(dataElement.getUid()); - ValuesHolder valuesHolder = getAuditAndDateParameters(eventDataValue, dv); + if (isNewDataValue(eventDataValue, dv)) { + eventDataValue = new EventDataValue(); + eventDataValue.setCreated(new Date()); + eventDataValue.setCreatedByUserInfo(UserInfoSnapshot.from(user)); + eventDataValue.setDataElement(dataElement.getUid()); + eventDataValue.setStoredBy(user.getUsername()); + + eventDataValue.setLastUpdated(new Date()); + eventDataValue.setLastUpdatedByUserInfo(UserInfoSnapshot.from(user)); + + eventDataValue.setValue(dv.getValue()); + eventDataValue.setProvidedElsewhere(dv.isProvidedElsewhere()); - eventDataValue = valuesHolder.getEventDataValue(); + if (dataElement.isFileType()) { + assignFileResource(entityManager, preheat, event.getUid(), eventDataValue.getValue()); + } - eventDataValue.setDataElement(dataElement.getUid()); - eventDataValue.setStoredBy(dv.getStoredBy()); + event.getEventDataValues().add(eventDataValue); + logTrackedEntityDataValueHistory( + user.getUsername(), + dataElement, + event, + new Date(), + eventDataValue, + ChangeLogType.CREATE); + } else if (isUpdate(eventDataValue, dv)) { + eventDataValue.setLastUpdated(new Date()); + eventDataValue.setLastUpdatedByUserInfo(UserInfoSnapshot.from(user)); - if (StringUtils.isEmpty(dv.getValue())) { if (dataElement.isFileType()) { unassignFileResource( entityManager, preheat, event.getUid(), eventDataValue.getValue()); + assignFileResource(entityManager, preheat, event.getUid(), dv.getValue()); } - event.getEventDataValues().remove(eventDataValue); - } else { + logTrackedEntityDataValueHistory( + user.getUsername(), + dataElement, + event, + new Date(), + eventDataValue, + ChangeLogType.UPDATE); + eventDataValue.setProvidedElsewhere(dv.isProvidedElsewhere()); eventDataValue.setValue(dv.getValue()); - + } else if (isDeletion(eventDataValue, dv)) { if (dataElement.isFileType()) { - assignFileResource(entityManager, preheat, event.getUid(), eventDataValue.getValue()); + unassignFileResource( + entityManager, preheat, event.getUid(), eventDataValue.getValue()); } - + eventDataValue.setLastUpdated(new Date()); + eventDataValue.setLastUpdatedByUserInfo(UserInfoSnapshot.from(user)); + + logTrackedEntityDataValueHistory( + user.getUsername(), + dataElement, + event, + new Date(), + eventDataValue, + ChangeLogType.DELETE); + eventDataValue.setValue(dv.getValue()); + eventDataValue.setProvidedElsewhere(dv.isProvidedElsewhere()); event.getEventDataValues().remove(eventDataValue); - event.getEventDataValues().add(eventDataValue); } - - logTrackedEntityDataValueHistory( - user.getUsername(), dataElement, event, new Date(), valuesHolder); }); } - private Date getFromOrNewDate(DataValue dv, Function dateGetter) { - return Optional.of(dv).map(dateGetter).map(DateUtils::fromInstant).orElseGet(Date::new); - } - private void logTrackedEntityDataValueHistory( - String userName, DataElement de, Event event, Date created, ValuesHolder valuesHolder) { - ChangeLogType changeLogType = valuesHolder.getChangeLogType(); - - if (changeLogType != null) { - TrackedEntityDataValueChangeLog valueAudit = new TrackedEntityDataValueChangeLog(); - valueAudit.setEvent(event); - valueAudit.setValue(valuesHolder.getValue()); - valueAudit.setAuditType(changeLogType); - valueAudit.setDataElement(de); - valueAudit.setModifiedBy(userName); - valueAudit.setProvidedElsewhere(valuesHolder.isProvidedElseWhere()); - valueAudit.setCreated(created); - - eventChangeLogService.addTrackedEntityDataValueChangeLog(valueAudit); - } + String userName, + DataElement de, + Event event, + Date created, + EventDataValue eventDataValue, + ChangeLogType changeLogType) { + + TrackedEntityDataValueChangeLog valueAudit = new TrackedEntityDataValueChangeLog(); + valueAudit.setEvent(event); + valueAudit.setValue(eventDataValue.getValue()); + valueAudit.setAuditType(changeLogType); + valueAudit.setDataElement(de); + valueAudit.setModifiedBy(userName); + valueAudit.setProvidedElsewhere(eventDataValue.getProvidedElsewhere()); + valueAudit.setCreated(created); + + eventChangeLogService.addTrackedEntityDataValueChangeLog(valueAudit); } @Override @@ -245,61 +273,20 @@ protected String getUpdatedTrackedEntity(Event entity) { .orElse(null); } - private boolean isNewDataValue(EventDataValue eventDataValue, DataValue dv) { - return eventDataValue == null - || (eventDataValue.getCreated() == null && StringUtils.isNotBlank(dv.getValue())); - } - - private boolean isDeletion(EventDataValue eventDataValue, DataValue dv) { - return StringUtils.isNotBlank(eventDataValue.getValue()) && StringUtils.isBlank(dv.getValue()); + private boolean isNewDataValue( + @CheckForNull EventDataValue eventDataValue, @Nonnull DataValue dv) { + return eventDataValue == null && !StringUtils.isBlank(dv.getValue()); } - private boolean isUpdate(EventDataValue eventDataValue, DataValue dv) { - return !StringUtils.equals(dv.getValue(), eventDataValue.getValue()); + private boolean isDeletion(@CheckForNull EventDataValue eventDataValue, @Nonnull DataValue dv) { + return eventDataValue != null + && StringUtils.isNotBlank(eventDataValue.getValue()) + && StringUtils.isBlank(dv.getValue()); } - private ValuesHolder getAuditAndDateParameters(EventDataValue eventDataValue, DataValue dv) { - String persistedValue; - - ChangeLogType changeLogType = null; - - if (isNewDataValue(eventDataValue, dv)) { - eventDataValue = new EventDataValue(); - eventDataValue.setCreated(getFromOrNewDate(dv, DataValue::getCreatedAt)); - eventDataValue.setLastUpdated(getFromOrNewDate(dv, DataValue::getUpdatedAt)); - persistedValue = dv.getValue(); - changeLogType = ChangeLogType.CREATE; - } else { - persistedValue = eventDataValue.getValue(); - - if (isUpdate(eventDataValue, dv)) { - changeLogType = ChangeLogType.UPDATE; - eventDataValue.setLastUpdated(getFromOrNewDate(dv, DataValue::getUpdatedAt)); - } - - if (isDeletion(eventDataValue, dv)) { - changeLogType = ChangeLogType.DELETE; - eventDataValue.setLastUpdated(getFromOrNewDate(dv, DataValue::getUpdatedAt)); - } - } - - return ValuesHolder.builder() - .value(persistedValue) - .providedElseWhere(dv.isProvidedElsewhere()) - .changeLogType(changeLogType) - .eventDataValue(eventDataValue) - .build(); - } - - @Data - @Builder - static class ValuesHolder { - private final String value; - - private final boolean providedElseWhere; - - private final ChangeLogType changeLogType; - - private final EventDataValue eventDataValue; + private boolean isUpdate(@CheckForNull EventDataValue eventDataValue, @Nonnull DataValue dv) { + return eventDataValue != null + && !StringUtils.isBlank(dv.getValue()) + && !StringUtils.equals(dv.getValue(), eventDataValue.getValue()); } } diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/domain/DataValue.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/domain/DataValue.java index e62a4c6116aa..12f88b58d538 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/domain/DataValue.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/domain/DataValue.java @@ -29,7 +29,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import java.io.Serializable; -import java.time.Instant; import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; @@ -44,10 +43,6 @@ @AllArgsConstructor // TODO(DHIS2-18222) Remove unused fields public class DataValue implements Serializable { - @JsonProperty private Instant createdAt; // remove - - @JsonProperty private Instant updatedAt; // remove - @JsonProperty private String storedBy; @JsonProperty private boolean providedElsewhere; diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/DataValuesValidatorTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/DataValuesValidatorTest.java index 46de27f8af1e..3f90906df9a8 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/DataValuesValidatorTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/DataValuesValidatorTest.java @@ -58,7 +58,6 @@ import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat; import org.hisp.dhis.tracker.imports.validation.Reporter; import org.hisp.dhis.tracker.imports.validation.ValidationCode; -import org.hisp.dhis.util.DateUtils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -148,52 +147,6 @@ void successValidationWhenDataElementIsValid() { assertIsEmpty(reporter.getErrors()); } - @Test - void successValidationWhenCreatedAtIsNull() { - DataElement dataElement = dataElement(); - when(preheat.getDataElement(MetadataIdentifier.ofUid(dataElementUid))).thenReturn(dataElement); - - ProgramStage programStage = programStage(dataElement); - when(preheat.getProgramStage(MetadataIdentifier.ofUid(programStageUid))) - .thenReturn(programStage); - - DataValue validDataValue = dataValue(); - validDataValue.setCreatedAt(null); - Event event = - Event.builder() - .programStage(idSchemes.toMetadataIdentifier(programStage)) - .status(EventStatus.ACTIVE) - .dataValues(Set.of(validDataValue)) - .build(); - - validator.validate(reporter, bundle, event); - - assertIsEmpty(reporter.getErrors()); - } - - @Test - void failValidationWhenUpdatedAtIsNull() { - DataElement dataElement = dataElement(); - when(preheat.getDataElement(MetadataIdentifier.ofUid(dataElementUid))).thenReturn(dataElement); - - ProgramStage programStage = programStage(dataElement); - when(preheat.getProgramStage(MetadataIdentifier.ofUid(programStageUid))) - .thenReturn(programStage); - - DataValue validDataValue = dataValue(); - validDataValue.setUpdatedAt(null); - Event event = - Event.builder() - .programStage(idSchemes.toMetadataIdentifier(programStage)) - .status(EventStatus.ACTIVE) - .dataValues(Set.of(validDataValue)) - .build(); - - validator.validate(reporter, bundle, event); - - assertIsEmpty(reporter.getErrors()); - } - @Test void failValidationWhenDataElementIsInvalid() { DataElement dataElement = dataElement(); @@ -1128,8 +1081,6 @@ private DataValue dataValue(String value) { private DataValue dataValue() { DataValue dataValue = new DataValue(); - dataValue.setCreatedAt(DateUtils.instantFromDateAsString("2020-10-10")); - dataValue.setUpdatedAt(DateUtils.instantFromDateAsString("2020-10-10")); dataValue.setValue("text"); dataValue.setDataElement(MetadataIdentifier.ofUid(dataElementUid)); return dataValue;