Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

<property name="lastUpdated" type="timestamp" not-null="true" />

<property name="value" column="value" access="property" length="1200" />
<property name="value" column="value" access="property" length="1200" not-null="true" />

<property name="updatedBy" column="updatedby" not-null="false" />

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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<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");
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<TrackedEntityAttributeValue> attributeValues =
trackedEntityAttributeValueService.getTrackedEntityAttributeValues(trackedEntity);
assertContainsOnly(
List.of("sYn3tkL3XKa", "sTGqP5JNy6E"),
attributeValues.stream().map(av -> av.getAttribute().getUid()).toList());
}

@Test
void shouldSetMinCharactersToSearchFromImportOrDefaultToZeroIfNotSpecified() {
List<TrackedEntityAttribute> trackedEntityAttributes =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading