diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/trackedentityattributevalue/hibernate/TrackedEntityAttributeValue.hbm.xml b/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/trackedentityattributevalue/hibernate/TrackedEntityAttributeValue.hbm.xml index 6086499741ca..2496d4d26354 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/trackedentityattributevalue/hibernate/TrackedEntityAttributeValue.hbm.xml +++ b/dhis-2/dhis-services/dhis-service-core/src/main/resources/org/hisp/dhis/trackedentityattributevalue/hibernate/TrackedEntityAttributeValue.hbm.xml @@ -21,7 +21,7 @@ - + diff --git a/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.44/V2_44_15__teav_value_not_null.sql b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.44/V2_44_15__teav_value_not_null.sql new file mode 100644 index 000000000000..5446206e2bc6 --- /dev/null +++ b/dhis-2/dhis-support/dhis-support-db-migration/src/main/resources/org/hisp/dhis/db/migration/2.44/V2_44_15__teav_value_not_null.sql @@ -0,0 +1,15 @@ +-- Enforce NOT NULL on trackedentityattributevalue.value. +-- +-- A tracked entity attribute value row is meaningful only because of its value, so a row with a +-- NULL value carries no information and is not a state the application produces: the tracker +-- importer deletes the row when a value is cleared (it never stores NULL), and the legacy +-- TrackedEntityAttributeValueService skips the save when the value is NULL. The dual-column +-- encryption storage that historically left `value` NULL (with the ciphertext in `encryptedvalue`) +-- was removed in 2.44 (V2_44_6__remove_confidential_from_tea.sql). The only remaining NULL rows are +-- therefore legacy/orphan data, which we remove before adding the constraint. Empty string ('') is +-- intentionally left allowed. +DELETE FROM trackedentityattributevalue +WHERE value IS NULL; + +ALTER TABLE trackedentityattributevalue +ALTER COLUMN value SET NOT NULL; diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/bundle/TrackedEntityAttributeTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/bundle/TrackedEntityAttributeTest.java index 1b008a2b756e..42175d8bdabd 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/bundle/TrackedEntityAttributeTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/bundle/TrackedEntityAttributeTest.java @@ -194,6 +194,77 @@ void shouldUpdateExistingAttributeValueWhenImportingWithNonUidIdScheme() throws assertEquals("updated value", updatedValue.getValue()); } + @Test + void shouldNotPersistAttributeValueWhenImportingEmptyValueForAttributeNotInDb() { + UID te = UID.generate(); + TrackerObjects trackerObjects = + TrackerObjects.builder() + .trackedEntities( + List.of( + org.hisp.dhis.tracker.imports.domain.TrackedEntity.builder() + .trackedEntity(te) + .trackedEntityType(MetadataIdentifier.ofUid("KrYIdvLxkMb")) + .orgUnit(MetadataIdentifier.ofUid("cNEZTkdAvmg")) + .attributes( + List.of( + Attribute.builder() + .attribute(MetadataIdentifier.ofUid("sYn3tkL3XKa")) + .value("123") + .build(), + Attribute.builder() + .attribute(MetadataIdentifier.ofUid("TsfP85GKsU5")) + .value("") + .build())) + .build())) + .build(); + + assertNoErrors( + trackerImportService.importTracker(TrackerImportParams.builder().build(), trackerObjects)); + clearSession(); + + TrackedEntity trackedEntity = manager.get(TrackedEntity.class, te.getValue()); + List attributeValues = + trackedEntityAttributeValueService.getTrackedEntityAttributeValues(trackedEntity); + assertContainsOnly( + List.of("sYn3tkL3XKa"), + attributeValues.stream().map(av -> av.getAttribute().getUid()).toList()); + } + + @Test + void shouldDeleteAttributeValueWhenImportingEmptyValueForExistingAttribute() throws IOException { + testSetup.importTrackerData("tracker/te_with_tea_data.json"); + clearSession(); + + TrackerObjects update = + TrackerObjects.builder() + .trackedEntities( + List.of( + org.hisp.dhis.tracker.imports.domain.TrackedEntity.builder() + .trackedEntity(UID.of("CLR1fvPj4ic")) + .trackedEntityType(MetadataIdentifier.ofUid("KrYIdvLxkMb")) + .orgUnit(MetadataIdentifier.ofUid("cNEZTkdAvmg")) + .attributes( + List.of( + Attribute.builder() + .attribute(MetadataIdentifier.ofUid("TsfP85GKsU5")) + .value("") + .build())) + .build())) + .build(); + TrackerImportParams params = + TrackerImportParams.builder().importStrategy(TrackerImportStrategy.UPDATE).build(); + + assertNoErrors(trackerImportService.importTracker(params, update)); + clearSession(); + + TrackedEntity trackedEntity = manager.get(TrackedEntity.class, "CLR1fvPj4ic"); + List attributeValues = + trackedEntityAttributeValueService.getTrackedEntityAttributeValues(trackedEntity); + assertContainsOnly( + List.of("sYn3tkL3XKa", "sTGqP5JNy6E"), + attributeValues.stream().map(av -> av.getAttribute().getUid()).toList()); + } + @Test void shouldSetMinCharactersToSearchFromImportOrDefaultToZeroIfNotSpecified() { List trackedEntityAttributes = diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/AbstractTrackerPersister.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/AbstractTrackerPersister.java index b9c8f645263a..3ca40dbed86d 100644 --- a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/AbstractTrackerPersister.java +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/AbstractTrackerPersister.java @@ -525,11 +525,16 @@ protected void handleTrackedEntityAttributeValues( String previousValue = isNew ? null : currentValue.getValue(); boolean valueChanged = isNew || !Objects.equals(previousValue, attribute.getValue()); - if (isDelete && !isNew) { - delete(preheat, currentValue, trackedEntity, user, changeLogs, batch); - // Leave the entry in the map: the DELETE is not flushed until the end of the run, so a - // later occurrence of the same TE+attribute in this run must still see it as existing - // (matching the pre-batch DB-read behaviour). + if (isDelete) { + if (!isNew) { + delete(preheat, currentValue, trackedEntity, user, changeLogs, batch); + + // Leave the entry in the map: the DELETE is not flushed until the end of + // the run, so a later occurrence of the same TE+attribute in this run must + // still see it as existing (matching the pre-batch DB-read behaviour). + } + + // If the value doesn't exist yet, deleting it is a no-op. } else if (valueChanged) { TrackedEntityAttributeValue persisted = saveOrUpdateAttributeValue(