fix: Do not save null values for TEAV [DHIS2-21599] (2.42)

dhis-2/dhis-services/dhis-service-administration/src/main/resources/data-integrity-checks.yaml

@@ -44,6 +44,7 @@ checks:
   - programs/tracker_geometry_invalid_srid.yaml
   - programs/program_stages_no_programs.yaml
   - programs/single_events_null_occurred_date.yaml
+  - programs/tracked_entity_attribute_null_value.yaml
   - programs/tracker_associate_is_deprecated.yaml
   - programs/programs_inconsistent_tracked_entity_type.yaml
   - program_indicators/program_indicators_without_expression.yaml

dhis-2/dhis-services/dhis-service-administration/src/main/resources/data-integrity-checks/programs/single_events_null_occurred_date.yaml

@@ -28,7 +28,7 @@
 name: single_events_null_occurred_date
 description: Single events without an occurred date
 section: Programs
-section_order: 3
+section_order: 6
 summary_sql: >-
   WITH rs as (
   SELECT e.uid

dhis-2/dhis-services/dhis-service-administration/src/main/resources/data-integrity-checks/programs/tracked_entity_attribute_null_value.yaml

@@ -0,0 +1,102 @@
+# Copyright (c) 2004-2025, 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.
+#
+---
+name: tracked_entity_attribute_null_value
+description: Tracked entity attribute value with no plain text value
+section: Programs
+section_order: 7
+summary_sql: >-
+  WITH rs as (
+  SELECT teav.trackedentityid, teav.trackedentityattributeid
+  FROM trackedentityattributevalue teav
+  WHERE teav.value IS NULL OR teav.value = ''
+  )
+  SELECT COUNT(*) as value,
+  100.0 * COUNT(*) / NULLIF( (SELECT COUNT(*) from trackedentityattributevalue),0) as percent
+  from rs;
+details_sql: >-
+  SELECT
+    te.uid as uid,
+    te.uid || ' / ' || tea.name as name,
+    CASE
+      WHEN teav.encryptedvalue IS NOT NULL
+        THEN 'Attribute "' || tea.name || '" (' || tea.uid ||
+             ') has an encrypted value but no plain text value'
+      ELSE 'Attribute "' || tea.name || '" (' || tea.uid ||
+           ') has no value'
+    END AS comment
+  FROM trackedentityattributevalue teav
+  INNER JOIN trackedentity te ON te.trackedentityid = teav.trackedentityid
+  INNER JOIN trackedentityattribute tea ON tea.trackedentityattributeid = teav.trackedentityattributeid
+  WHERE teav.value IS NULL OR teav.value = '';
+severity: SEVERE
+introduction: >
+  A tracked entity attribute value should always have a plain text value stored in the `value` column.
+  Some values only have an `encryptedvalue` and an empty or null `value`. This happened for attributes
+  that were marked as confidential, whose values were stored encrypted only.
+
+  Support for marking tracked entity attributes as confidential will be removed in version 2.44, and
+  such attributes are automatically migrated to `skipAnalytics = true`. That migration cannot recover the
+  plain text of values that were stored encrypted only, so any tracked entity attribute value that has an
+  encrypted value but no plain text value must be resolved before upgrading. This check must pass in order
+  to be able to upgrade to v44 and later.
+details_id_type: tracker
+recommendation: >-
+  For each flagged tracked entity attribute value you need to either restore a plain text value or delete
+  the value. How you proceed depends on whether an encrypted value is present (see the comment for each row).
+
+  Case 1 - the value has an encrypted value but no plain text value (previously confidential):
+  The plain text cannot be recovered by DHIS2 itself. You have two options.
+
+  Option A - recover the plain text value. This requires access to the encryption key that was configured
+  in `dhis.conf` (the `encryption.password` property) at the time the value was stored, and decrypting the
+  `encryptedvalue` outside of DHIS2. If you need the historical values preserved, please reach out on the
+  DHIS2 Community of Practice forum at https://community.dhis2.org for assistance before upgrading.
+
+  Option B - delete the encrypted-only values if you do not need them:
+
+  DELETE FROM trackedentityattributevalue
+  WHERE encryptedvalue IS NOT NULL
+  AND (value IS NULL OR value = '');
+
+  Case 2 - the value has neither a plain text value nor an encrypted value:
+  These are empty rows that carry no information and can be deleted:
+
+  DELETE FROM trackedentityattributevalue
+  WHERE encryptedvalue IS NULL
+  AND (value IS NULL OR value = '');
+
+  As is always the case with direct database manipulation, you should ensure that you have a backup of your
+  database before running these commands, ensure that DHIS2 is not running, and perform the change in a test
+  environment first.
+
+  After applying the fix, restart DHIS2 and re-run the integrity check to ensure that no tracked entity
+  attribute values are flagged.
+
+  If you are unsure how to proceed or encounter issues, please feel free to reach out on the DHIS2 Community
+  of Practice forum at https://community.dhis2.org.
+is_slow: true

dhis-2/dhis-services/dhis-service-administration/src/test/java/org/hisp/dhis/dataintegrity/DataIntegrityYamlReaderTest.java

@@ -93,7 +93,7 @@ void testReadDataIntegrityYaml() {
 
     List<DataIntegrityCheck> checks = new ArrayList<>();
     readYaml(checks, "data-integrity-checks.yaml", "data-integrity-checks", CLASS_PATH);
-    assertEquals(95, checks.size());
+    assertEquals(96, checks.size());
 
     // Names should be unique
     List<String> allNames = checks.stream().map(DataIntegrityCheck::getName).toList();

dhis-2/dhis-services/dhis-service-core/src/main/resources/i18n_global.properties

@@ -2124,6 +2124,7 @@ data_integrity.program_rules_no_action.name=Program rules without actions
 data_integrity.program_rules_no_expression.name=Program rules with no expression.
 data_integrity.program_rules_no_priority.name=Program rules without priorities
 data_integrity.single_events_null_occurred_date.name=Single events without an occurred date
+data_integrity.tracked_entity_attribute_null_value.name=Tracked entity attribute value with no plain text value
 data_integrity.tracker_associate_is_deprecated.name=TRACKER_ASSOCIATE value type is deprecated
 data_integrity.validation_rules_missing_value_strategy_null.name=Validation rule expressions without missing value strategies
 data_integrity.dashboards_not_viewed_one_year.name=Dashboards which have not been actively viewed in the last 12 months

dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/AbstractTrackerPersister.java

@@ -408,8 +408,16 @@ protected void handleTrackedEntityAttributeValues(
           String previousValue = isNew ? null : currentValue.getPlainValue();
           boolean valueChanged = isNew || !Objects.equals(previousValue, attribute.getValue());
 
-          if (isDelete && !isNew) {
-            delete(entityManager, preheat, currentValue, trackedEntity, user, changeLogs);
+          if (isDelete) {
+            if (!isNew) {
+              delete(entityManager, preheat, currentValue, trackedEntity, user, changeLogs);
+
+              // 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) {
             saveOrUpdateAttributeValue(
                 entityManager,

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/bundle/TrackedEntityAttributeTest.java

@@ -29,20 +29,29 @@
  */
 package org.hisp.dhis.tracker.imports.bundle;
 
+import static org.hisp.dhis.test.utils.Assertions.assertContainsOnly;
+import static org.hisp.dhis.tracker.Assertions.assertNoErrors;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 import java.io.IOException;
 import java.util.List;
 import org.hisp.dhis.common.IdentifiableObjectManager;
+import org.hisp.dhis.common.UID;
 import org.hisp.dhis.organisationunit.OrganisationUnit;
 import org.hisp.dhis.test.integration.PostgresIntegrationTestBase;
 import org.hisp.dhis.trackedentity.TrackedEntity;
 import org.hisp.dhis.trackedentity.TrackedEntityAttribute;
 import org.hisp.dhis.trackedentity.TrackedEntityType;
 import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValue;
 import org.hisp.dhis.tracker.TestSetup;
+import org.hisp.dhis.tracker.TrackerIdSchemeParam;
 import org.hisp.dhis.tracker.TrackerIdSchemeParams;
+import org.hisp.dhis.tracker.imports.TrackerImportParams;
+import org.hisp.dhis.tracker.imports.TrackerImportService;
+import org.hisp.dhis.tracker.imports.TrackerImportStrategy;
+import org.hisp.dhis.tracker.imports.domain.Attribute;
+import org.hisp.dhis.tracker.imports.domain.MetadataIdentifier;
 import org.hisp.dhis.tracker.imports.domain.TrackerObjects;
 import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat;
 import org.hisp.dhis.tracker.imports.preheat.TrackerPreheatService;
@@ -64,6 +73,8 @@ class TrackedEntityAttributeTest extends PostgresIntegrationTestBase {
 
   @Autowired private TrackerPreheatService trackerPreheatService;
 
+  @Autowired private TrackerImportService trackerImportService;
+
   @Autowired private TrackedEntityAttributeValueService trackedEntityAttributeValueService;
 
   @Autowired private IdentifiableObjectManager manager;
@@ -111,6 +122,118 @@ void shouldSetStoredByToAuthenticatedUserForTrackedEntityAttributeValue() throws
     TrackedEntity trackedEntity = manager.getAll(TrackedEntity.class).get(0);
     List<TrackedEntityAttributeValue> attributeValues =
         trackedEntityAttributeValueService.getTrackedEntityAttributeValues(trackedEntity);
+
     attributeValues.forEach(av -> assertEquals(importUser.getUsername(), av.getStoredBy()));
   }
+
+  @Test
+  void shouldUpdateExistingAttributeValueWhenImportingWithNonUidIdScheme() throws IOException {
+    testSetup.importTrackerData("tracker/te_with_tea_data.json");
+
+    // The persister must recognize the attribute value as existing (and route it to an UPDATE
+    // instead of a duplicate INSERT, which would violate the composite PK) regardless of the
+    // idScheme the import resolves attributes with.
+    TrackerObjects update =
+        TrackerObjects.builder()
+            .trackedEntities(
+                List.of(
+                    org.hisp.dhis.tracker.imports.domain.TrackedEntity.builder()
+                        .trackedEntity(UID.of("CLR1fvPj4ic"))
+                        .trackedEntityType(MetadataIdentifier.ofName("Person"))
+                        .orgUnit(MetadataIdentifier.ofName("Country"))
+                        .attributes(
+                            List.of(
+                                Attribute.builder()
+                                    .attribute(MetadataIdentifier.ofName("Attribute_Text"))
+                                    .value("updated value")
+                                    .build()))
+                        .build()))
+            .build();
+    TrackerImportParams params =
+        TrackerImportParams.builder()
+            .importStrategy(TrackerImportStrategy.UPDATE)
+            .idSchemes(TrackerIdSchemeParams.builder().idScheme(TrackerIdSchemeParam.NAME).build())
+            .build();
+
+    assertNoErrors(trackerImportService.importTracker(params, update));
+
+    TrackedEntity trackedEntity = manager.getAll(TrackedEntity.class).get(0);
+    List<TrackedEntityAttributeValue> attributeValues =
+        trackedEntityAttributeValueService.getTrackedEntityAttributeValues(trackedEntity);
+    assertEquals(3, attributeValues.size());
+    TrackedEntityAttributeValue updatedValue =
+        attributeValues.stream()
+            .filter(av -> "TsfP85GKsU5".equals(av.getAttribute().getUid()))
+            .findFirst()
+            .orElseThrow();
+    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));
+
+    TrackedEntity trackedEntity = manager.get(TrackedEntity.class, te.getValue());
+    List<TrackedEntityAttributeValue> 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");
+
+    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));
+
+    TrackedEntity trackedEntity = manager.get(TrackedEntity.class, "CLR1fvPj4ic");
+    List<TrackedEntityAttributeValue> attributeValues =
+        trackedEntityAttributeValueService.getTrackedEntityAttributeValues(trackedEntity);
+    assertContainsOnly(
+        List.of("sYn3tkL3XKa", "sTGqP5JNy6E"),
+        attributeValues.stream().map(av -> av.getAttribute().getUid()).toList());
+  }
 }