From 0ff804102b2ed498639296cb7aa6fd21935af40c Mon Sep 17 00:00:00 2001 From: Enrico Date: Mon, 29 Jun 2026 13:13:00 +0200 Subject: [PATCH 1/3] fix: Do not save null values for TEAV [DHIS2-21599] --- .../TrackedEntityAttributeValue.hbm.xml | 2 +- .../2.44/V2_44_15__teav_value_not_null.sql | 15 ++++ .../bundle/TrackedEntityAttributeTest.java | 71 +++++++++++++++++++ .../persister/AbstractTrackerPersister.java | 2 + 4 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 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 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..13844dd63f39 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 @@ -530,6 +530,8 @@ protected void handleTrackedEntityAttributeValues( // 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). + } else if (isDelete && isNew) { + // If user is trying to delete a value not present in DB: do nothing. } else if (valueChanged) { TrackedEntityAttributeValue persisted = saveOrUpdateAttributeValue( From 424d3c0248b0b352964d5e80fc87d0b92e444e60 Mon Sep 17 00:00:00 2001 From: Enrico Colasante Date: Tue, 30 Jun 2026 11:25:50 +0200 Subject: [PATCH 2/3] Update dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/AbstractTrackerPersister.java Co-authored-by: marc --- .../bundle/persister/AbstractTrackerPersister.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) 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 13844dd63f39..2c0522cf8cd1 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,7 +525,17 @@ protected void handleTrackedEntityAttributeValues( String previousValue = isNew ? null : currentValue.getValue(); boolean valueChanged = isNew || !Objects.equals(previousValue, attribute.getValue()); - if (isDelete && !isNew) { +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. +} 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 From df184a54f36c0df0882d87b2571e39c4071517ec Mon Sep 17 00:00:00 2001 From: Enrico Date: Tue, 30 Jun 2026 11:29:35 +0200 Subject: [PATCH 3/3] Fix formatting --- .../persister/AbstractTrackerPersister.java | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) 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 2c0522cf8cd1..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,23 +525,16 @@ protected void handleTrackedEntityAttributeValues( String previousValue = isNew ? null : currentValue.getValue(); boolean valueChanged = isNew || !Objects.equals(previousValue, attribute.getValue()); -if (isDelete) { - if (!isNew) { - delete(preheat, currentValue, trackedEntity, user, changeLogs, batch); + 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). - } + // 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. -} - 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). - } else if (isDelete && isNew) { - // If user is trying to delete a value not present in DB: do nothing. + // If the value doesn't exist yet, deleting it is a no-op. } else if (valueChanged) { TrackedEntityAttributeValue persisted = saveOrUpdateAttributeValue(