fix: Remove user field from tracker preheat [DHIS2-18001]

dhis2 · Sep 26, 2024 · cfd9ef5 · cfd9ef5
1 parent 0f99aef
commit cfd9ef5
Show file tree

Hide file tree

Showing 10 changed files with 54 additions and 34 deletions.
diff --git a/...rc/main/java/org/hisp/dhis/tracker/imports/bundle/persister/AbstractTrackerPersister.java b/...rc/main/java/org/hisp/dhis/tracker/imports/bundle/persister/AbstractTrackerPersister.java
@@ -66,6 +66,7 @@
 import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat;
 import org.hisp.dhis.tracker.imports.report.Entity;
 import org.hisp.dhis.tracker.imports.report.TrackerTypeReport;
+import org.hisp.dhis.user.CurrentUserUtil;
 
 /**
  * @author Luciano Fiandesio
@@ -377,7 +378,10 @@ private void delete(
             : entityManager.merge(trackedEntityAttributeValue));
 
     logTrackedEntityAttributeValueHistory(
-        preheat.getUsername(), trackedEntityAttributeValue, trackedEntity, ChangeLogType.DELETE);
+        CurrentUserUtil.getCurrentUsername(),
+        trackedEntityAttributeValue,
+        trackedEntity,
+        ChangeLogType.DELETE);
   }
 
   private void saveOrUpdate(
@@ -409,7 +413,10 @@ private void saveOrUpdate(
     }
 
     logTrackedEntityAttributeValueHistory(
-        preheat.getUsername(), trackedEntityAttributeValue, trackedEntity, changeLogType);
+        CurrentUserUtil.getCurrentUsername(),
+        trackedEntityAttributeValue,
+        trackedEntity,
+        changeLogType);
   }
 
   private static boolean isFileResource(TrackedEntityAttributeValue trackedEntityAttributeValue) {

diff --git a/...-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/EventPersister.java b/...-tracker/src/main/java/org/hisp/dhis/tracker/imports/bundle/persister/EventPersister.java
@@ -61,6 +61,7 @@
 import org.hisp.dhis.tracker.imports.job.NotificationTrigger;
 import org.hisp.dhis.tracker.imports.job.TrackerNotificationDataBundle;
 import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat;
+import org.hisp.dhis.user.CurrentUserUtil;
 import org.hisp.dhis.util.DateUtils;
 import org.springframework.stereotype.Component;
 
@@ -211,7 +212,7 @@ private void handleDataValues(
           }
 
           logTrackedEntityDataValueHistory(
-              preheat.getUsername(), dataElement, event, new Date(), valuesHolder);
+              CurrentUserUtil.getCurrentUsername(), dataElement, event, new Date(), valuesHolder);
         });
   }
 

diff --git a/.../main/java/org/hisp/dhis/tracker/imports/converter/EnrollmentTrackerConverterService.java b/.../main/java/org/hisp/dhis/tracker/imports/converter/EnrollmentTrackerConverterService.java
@@ -44,6 +44,7 @@
 import org.hisp.dhis.trackedentity.TrackedEntity;
 import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat;
 import org.hisp.dhis.tracker.imports.validation.validator.TrackerImporterAssertErrors;
+import org.hisp.dhis.user.CurrentUserUtil;
 import org.hisp.dhis.util.DateUtils;
 import org.hisp.dhis.util.ObjectUtils;
 import org.springframework.stereotype.Service;
@@ -155,7 +156,7 @@ private Enrollment from(
     if (previousStatus != dbEnrollment.getStatus()) {
       if (dbEnrollment.isCompleted()) {
         dbEnrollment.setCompletedDate(now);
-        dbEnrollment.setCompletedBy(preheat.getUsername());
+        dbEnrollment.setCompletedBy(CurrentUserUtil.getCurrentUsername());
       } else if (EnrollmentStatus.CANCELLED == dbEnrollment.getStatus()) {
         dbEnrollment.setCompletedDate(now);
       }

diff --git a/...r/src/main/java/org/hisp/dhis/tracker/imports/converter/EventTrackerConverterService.java b/...r/src/main/java/org/hisp/dhis/tracker/imports/converter/EventTrackerConverterService.java
@@ -55,6 +55,7 @@
 import org.hisp.dhis.tracker.imports.domain.MetadataIdentifier;
 import org.hisp.dhis.tracker.imports.domain.User;
 import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat;
+import org.hisp.dhis.user.CurrentUserUtil;
 import org.hisp.dhis.util.DateUtils;
 import org.springframework.stereotype.Service;
 
@@ -228,7 +229,7 @@ private Event from(
 
     if (currentStatus != previousStatus && currentStatus == EventStatus.COMPLETED) {
       result.setCompletedDate(now);
-      result.setCompletedBy(preheat.getUsername());
+      result.setCompletedBy(CurrentUserUtil.getCurrentUsername());
     }
 
     if (currentStatus != EventStatus.COMPLETED) {

diff --git a/...-tracker/src/main/java/org/hisp/dhis/tracker/imports/converter/NotesConverterService.java b/...-tracker/src/main/java/org/hisp/dhis/tracker/imports/converter/NotesConverterService.java
@@ -29,18 +29,25 @@
 
 import java.util.List;
 import java.util.stream.Collectors;
+import lombok.RequiredArgsConstructor;
 import org.hisp.dhis.note.Note;
+import org.hisp.dhis.tracker.imports.TrackerUserService;
 import org.hisp.dhis.tracker.imports.domain.User;
 import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat;
 import org.hisp.dhis.util.DateUtils;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 
 /**
  * @author Luciano Fiandesio
  */
 @Service
+@RequiredArgsConstructor
 public class NotesConverterService
     implements TrackerConverterService<org.hisp.dhis.tracker.imports.domain.Note, Note> {
+
+  @Autowired private final TrackerUserService userService;
+
   @Override
   public org.hisp.dhis.tracker.imports.domain.Note to(Note note) {
     org.hisp.dhis.tracker.imports.domain.Note trackerNote =
@@ -71,7 +78,7 @@ public Note from(TrackerPreheat preheat, org.hisp.dhis.tracker.imports.domain.No
     trackerNote.setAutoFields();
     trackerNote.setNoteText(note.getValue());
 
-    trackerNote.setLastUpdatedBy(preheat.getUser());
+    trackerNote.setLastUpdatedBy(userService.getCurrentUser());
     trackerNote.setCreator(note.getStoredBy());
     return trackerNote;
   }

diff --git a/...ker/src/main/java/org/hisp/dhis/tracker/imports/preheat/DefaultTrackerPreheatService.java b/...ker/src/main/java/org/hisp/dhis/tracker/imports/preheat/DefaultTrackerPreheatService.java
@@ -27,8 +27,6 @@
  */
 package org.hisp.dhis.tracker.imports.preheat;
 
-import static com.google.common.base.Preconditions.checkNotNull;
-
 import java.beans.Introspector;
 import java.util.List;
 import javax.annotation.Nonnull;
@@ -76,9 +74,6 @@ public TrackerPreheat preheat(
       TrackerObjects trackerObjects, TrackerIdSchemeParams idSchemeParams, User user) {
     TrackerPreheat preheat = new TrackerPreheat();
     preheat.setIdSchemes(idSchemeParams);
-    preheat.setUser(user);
-
-    checkNotNull(preheat.getUser(), "TrackerPreheat is missing the user object.");
 
     for (String supplier : preheatSuppliers) {
       final String beanName = Introspector.decapitalize(supplier);

diff --git a/...s-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheat.java b/...s-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheat.java
@@ -82,9 +82,6 @@
  * @author Morten Olav Hansen <mortenoh@gmail.com>
  */
 public class TrackerPreheat {
-  /** User to use for import job (important for threaded imports). */
-  @Getter @Setter private User user;
-
   @Getter @Setter private UserInfoSnapshot userInfo;
 
   /**
@@ -286,10 +283,6 @@ public MetadataIdentifier getCategoryOptionComboIdentifier(
 
   public TrackerPreheat() {}
 
-  public String getUsername() {
-    return User.username(user);
-  }
-
   /**
    * Put a default metadata value (i.e. CategoryOption "default") into the preheat.
    *

diff --git a/.../java/org/hisp/dhis/tracker/imports/preheat/supplier/strategy/AbstractSchemaStrategy.java b/.../java/org/hisp/dhis/tracker/imports/preheat/supplier/strategy/AbstractSchemaStrategy.java
@@ -48,8 +48,7 @@
 import org.hisp.dhis.tracker.imports.preheat.cache.PreheatCacheService;
 import org.hisp.dhis.tracker.imports.preheat.mappers.CopyMapper;
 import org.hisp.dhis.tracker.imports.preheat.mappers.PreheatMapper;
-import org.hisp.dhis.user.User;
-import org.hisp.dhis.user.UserDetails;
+import org.hisp.dhis.user.CurrentUserUtil;
 import org.mapstruct.factory.Mappers;
 
 /**
@@ -109,7 +108,7 @@ protected void queryForIdentifiableObjects(
                 UID.of(idSchemeParam.getAttributeUid()),
                 ids);
       } else {
-        objects = cacheAwareFetch(preheat.getUser(), schema, idSchemeParam, ids, mapper);
+        objects = cacheAwareFetch(schema, idSchemeParam, ids, mapper);
       }
 
       preheat.put(idSchemeParam, objects);
@@ -122,7 +121,6 @@ private String buildCacheKey(Schema schema) {
 
   @SuppressWarnings({"unchecked", "rawtypes"})
   private List<IdentifiableObject> cacheAwareFetch(
-      User user,
       Schema schema,
       TrackerIdSchemeParam idSchemeParam,
       List<String> ids,
@@ -151,8 +149,7 @@ private List<IdentifiableObject> cacheAwareFetch(
         // cache
         objects =
             map(
-                (List<IdentifiableObject>)
-                    queryService.query(buildQuery(schema, user, idScheme, ids)),
+                (List<IdentifiableObject>) queryService.query(buildQuery(schema, idScheme, ids)),
                 mapper);
 
         // put objects in query based on given scheme. If the key
@@ -171,8 +168,7 @@ private List<IdentifiableObject> cacheAwareFetch(
     } else {
       objects =
           map(
-              (List<IdentifiableObject>)
-                  queryService.query(buildQuery(schema, user, idScheme, ids)),
+              (List<IdentifiableObject>) queryService.query(buildQuery(schema, idScheme, ids)),
               mapper);
     }
 
@@ -193,9 +189,9 @@ private List<IdentifiableObject> map(
     }
   }
 
-  private Query buildQuery(Schema schema, User user, TrackerIdScheme idScheme, List<String> ids) {
+  private Query buildQuery(Schema schema, TrackerIdScheme idScheme, List<String> ids) {
     Query query = Query.from(schema);
-    query.setCurrentUserDetails(user == null ? null : UserDetails.fromUser(user));
+    query.setCurrentUserDetails(CurrentUserUtil.getCurrentUserDetails());
     query.add(generateRestrictionFromIdentifiers(idScheme, ids));
     query.setDefaults(Defaults.INCLUDE);
 

diff --git a/...c/test/java/org/hisp/dhis/tracker/imports/converter/EventTrackerConverterServiceTest.java b/...c/test/java/org/hisp/dhis/tracker/imports/converter/EventTrackerConverterServiceTest.java
@@ -51,10 +51,12 @@
 import org.hisp.dhis.program.UserInfoSnapshot;
 import org.hisp.dhis.test.TestBase;
 import org.hisp.dhis.trackedentity.TrackedEntity;
+import org.hisp.dhis.tracker.imports.TrackerUserService;
 import org.hisp.dhis.tracker.imports.domain.DataValue;
 import org.hisp.dhis.tracker.imports.domain.MetadataIdentifier;
 import org.hisp.dhis.tracker.imports.domain.User;
 import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat;
+import org.hisp.dhis.user.UserDetails;
 import org.hisp.dhis.util.DateUtils;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -80,7 +82,10 @@ class EventTrackerConverterServiceTest extends TestBase {
 
   private static final Date today = new Date();
 
-  private final NotesConverterService notesConverterService = new NotesConverterService();
+  @Mock public TrackerUserService trackerUserService;
+
+  private final NotesConverterService notesConverterService =
+      new NotesConverterService(trackerUserService);
 
   private RuleEngineConverterService<org.hisp.dhis.tracker.imports.domain.Event, Event> converter;
 
@@ -100,6 +105,7 @@ class EventTrackerConverterServiceTest extends TestBase {
   void setUpTest() {
     converter = new EventTrackerConverterService(notesConverterService);
     user = makeUser("U");
+    injectSecurityContext(UserDetails.fromUser(user));
     programStage = createProgramStage('A', 1);
     programStage.setUid(PROGRAM_STAGE_UID);
     programStage.setProgram(program);
@@ -165,7 +171,6 @@ void shouldConvertFromEventWithCompletedDataWhenStatusIsCompleted() {
     dataElement.setUid(CodeGenerator.generateUid());
     when(preheat.getDataElement(MetadataIdentifier.ofUid(dataElement.getUid())))
         .thenReturn(dataElement);
-    when(preheat.getUsername()).thenReturn(USERNAME);
 
     org.hisp.dhis.tracker.imports.domain.Event event =
         event(dataValue(MetadataIdentifier.ofUid(dataElement.getUid()), "value"));
@@ -233,7 +238,6 @@ void shouldConvertFromExistingEventWithCompletedDataWhenStatusIsCompleted() {
     String eventUid = CodeGenerator.generateUid();
     when(preheat.getDataElement(MetadataIdentifier.ofUid(dataElement.getUid())))
         .thenReturn(dataElement);
-    when(preheat.getUsername()).thenReturn(USERNAME);
     dbEvent.setStatus(EventStatus.ACTIVE);
     when(preheat.getEvent(eventUid)).thenReturn(dbEvent);
 

diff --git a/...cker/src/test/java/org/hisp/dhis/tracker/imports/converter/NotesConverterServiceTest.java b/...cker/src/test/java/org/hisp/dhis/tracker/imports/converter/NotesConverterServiceTest.java
@@ -32,21 +32,28 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasSize;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.*;
 
 import java.util.List;
 import java.util.stream.Collectors;
 import org.hisp.dhis.note.Note;
 import org.hisp.dhis.test.TestBase;
 import org.hisp.dhis.test.random.BeanRandomizer;
+import org.hisp.dhis.tracker.imports.TrackerUserService;
 import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat;
 import org.hisp.dhis.user.User;
+import org.hisp.dhis.user.UserDetails;
 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;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
 
 /**
  * @author Luciano Fiandesio
  */
+@ExtendWith(MockitoExtension.class)
 class NotesConverterServiceTest extends TestBase {
 
   private static final String CURRENT_USER = "usernamea";
@@ -55,18 +62,24 @@ class NotesConverterServiceTest extends TestBase {
 
   private TrackerPreheat preheat;
 
+  private User user;
+
+  @Mock private TrackerUserService trackerUserService;
+
   private final BeanRandomizer rnd = BeanRandomizer.create();
 
   @BeforeEach
   void setUp() {
-    this.notesConverterService = new NotesConverterService();
-    User user = makeUser("A");
+    this.notesConverterService = new NotesConverterService(trackerUserService);
+    user = makeUser("A");
+    UserDetails userDetails = UserDetails.fromUser(user);
+    injectSecurityContext(userDetails);
     this.preheat = new TrackerPreheat();
-    preheat.setUser(user);
   }
 
   @Test
   void verifyConvertTrackerNoteToNote() {
+    when(trackerUserService.getCurrentUser()).thenReturn(user);
     org.hisp.dhis.tracker.imports.domain.Note trackerNote =
         rnd.nextObject(org.hisp.dhis.tracker.imports.domain.Note.class);
     final Note note = notesConverterService.from(preheat, trackerNote);
@@ -75,6 +88,7 @@ void verifyConvertTrackerNoteToNote() {
 
   @Test
   void verifyConvertTrackerNoteToNoteWithNoStoredByDefined() {
+    when(trackerUserService.getCurrentUser()).thenReturn(user);
     org.hisp.dhis.tracker.imports.domain.Note trackerNote =
         rnd.nextObject(org.hisp.dhis.tracker.imports.domain.Note.class);
     trackerNote.setStoredBy(null);
@@ -84,6 +98,7 @@ void verifyConvertTrackerNoteToNoteWithNoStoredByDefined() {
 
   @Test
   void verifyConvertTrackerNotesToNotes() {
+    when(trackerUserService.getCurrentUser()).thenReturn(user);
     List<org.hisp.dhis.tracker.imports.domain.Note> trackerNotes =
         rnd.objects(org.hisp.dhis.tracker.imports.domain.Note.class, 10)
             .collect(Collectors.toList());