perf: AttributeValueService is no more an AbstractPersister dependenc…

…y [DHIS2-11988] (dhis2#9069)

* perf: AttributeValueService is no more an AbstractPersister dependency [DHIS2-11988]

* perf: minor changes from code review [DHIS2-11988]

* style: formatting [DHIS2-11988]

dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/TrackerType.java

@@ -27,25 +27,34 @@
  */
 package org.hisp.dhis.tracker;
 
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
 /**
  * @author Morten Olav Hansen <mortenoh@gmail.com>
  */
+@Getter
+@RequiredArgsConstructor
 public enum TrackerType
 {
-    TRACKED_ENTITY( "trackedEntity" ),
-    ENROLLMENT( "enrollment" ),
-    EVENT( "event" ),
-    RELATIONSHIP( "relationship" );
+    TRACKED_ENTITY( "trackedEntity", 1 ),
+    ENROLLMENT( "enrollment", 2 ),
+    EVENT( "event", 3 ),
+    RELATIONSHIP( "relationship", 4 );
 
-    private String name;
+    private final String name;
 
-    TrackerType( String name )
-    {
-        this.name = name;
-    }
+    private final Integer priority;
 
-    public String getName()
+    public static List<TrackerType> getOrderedByPriority()
     {
-        return name;
+        return Arrays.stream( values() )
+            .sorted( Comparator.comparing( TrackerType::getPriority ) )
+            .collect( Collectors.toList() );
     }
 }

dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/bundle/DefaultTrackerBundleService.java

@@ -150,7 +150,7 @@ public TrackerBundleReport commit( TrackerBundle bundle )
 
         Session session = sessionFactory.getCurrentSession();
 
-        Stream.of( TrackerType.values() )
+        TrackerType.getOrderedByPriority()
             .forEach( t -> bundleReport.getTypeReportMap().put( t, COMMIT_MAPPER.get( t )
                 .apply( session, bundle ) ) );
 

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

@@ -37,6 +37,8 @@
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
+import lombok.AccessLevel;
+import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 
 import org.apache.commons.lang3.StringUtils;
@@ -51,7 +53,6 @@
 import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValue;
 import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueAudit;
 import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueAuditService;
-import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueService;
 import org.hisp.dhis.tracker.AtomicMode;
 import org.hisp.dhis.tracker.FlushMode;
 import org.hisp.dhis.tracker.TrackerType;
@@ -67,24 +68,14 @@
  * @author Luciano Fiandesio
  */
 @Slf4j
+@RequiredArgsConstructor( access = AccessLevel.PROTECTED )
 public abstract class AbstractTrackerPersister<T extends TrackerDto, V extends BaseIdentifiableObject>
     implements TrackerPersister<T, V>
 {
     protected final ReservedValueService reservedValueService;
 
-    protected final TrackedEntityAttributeValueService attributeValueService;
-
     protected final TrackedEntityAttributeValueAuditService trackedEntityAttributeValueAuditService;
 
-    protected AbstractTrackerPersister( ReservedValueService reservedValueService,
-        TrackedEntityAttributeValueAuditService trackedEntityAttributeValueAuditService,
-        TrackedEntityAttributeValueService attributeValueService )
-    {
-        this.reservedValueService = reservedValueService;
-        this.attributeValueService = attributeValueService;
-        this.trackedEntityAttributeValueAuditService = trackedEntityAttributeValueAuditService;
-    }
-
     /**
      * Template method that can be used by classes extending this class to
      * execute the persistence flow of Tracker entities
@@ -344,18 +335,14 @@ private void assignFileResource( Session session, TrackerPreheat preheat, String
     protected void handleTrackedEntityAttributeValues( Session session, TrackerPreheat preheat,
         List<Attribute> payloadAttributes, TrackedEntityInstance trackedEntityInstance )
     {
-        // TODO: Do not use attributeValueService.
-        // We should have the right version of attribute values present in the
-        // TEI
-        // at any moment
-        Map<String, TrackedEntityAttributeValue> attributeValueDBMap = attributeValueService
-            .getTrackedEntityAttributeValues( trackedEntityInstance )
+        Map<String, TrackedEntityAttributeValue> attributeValueByUid = trackedEntityInstance
+            .getTrackedEntityAttributeValues()
             .stream()
             .collect( Collectors.toMap( teav -> teav.getAttribute().getUid(), Function.identity() ) );
 
         for ( Attribute at : payloadAttributes )
         {
-            boolean isNew = false;
+            boolean isNewAttribute = false;
             AuditType auditType;
 
             TrackedEntityAttribute attribute = preheat.get( TrackedEntityAttribute.class, at.getAttribute() );
@@ -364,15 +351,16 @@ protected void handleTrackedEntityAttributeValues( Session session, TrackerPrehe
                 "Attribute " + at.getAttribute()
                     + " should never be NULL here if validation is enforced before commit." );
 
-            TrackedEntityAttributeValue attributeValue = attributeValueDBMap.get( at.getAttribute() );
+            TrackedEntityAttributeValue attributeValue = attributeValueByUid.get( at.getAttribute() );
 
             if ( attributeValue == null )
             {
                 attributeValue = new TrackedEntityAttributeValue();
                 attributeValue.setAttribute( attribute );
                 attributeValue.setEntityInstance( trackedEntityInstance );
 
-                isNew = true;
+                isNewAttribute = true;
+
             }
 
             attributeValue
@@ -382,15 +370,25 @@ protected void handleTrackedEntityAttributeValues( Session session, TrackerPrehe
             // We cannot use attributeValue.getValue() because it uses
             // encryption logic
             // So we need to use at.getValue()
-            if ( StringUtils.isEmpty( at.getValue() ) )
+            if ( StringUtils.isEmpty( at.getValue() ) ) // if it's a DELETE
+                                                        // operation
             {
-                if ( attribute.getValueType() == ValueType.FILE_RESOURCE )
+                // DELETE on a new attribute doesn't make sense, so in this case
+                // we don't need to do anything
+                if ( !isNewAttribute )
                 {
-                    unassignFileResource( session, preheat, attributeValueDBMap.get( at.getAttribute() ).getValue() );
-                }
+                    if ( attribute.getValueType() == ValueType.FILE_RESOURCE )
+                    {
+                        unassignFileResource( session, preheat,
+                            attributeValueByUid.get( at.getAttribute() ).getValue() );
+                    }
 
-                session.remove( attributeValue );
-                auditType = AuditType.DELETE;
+                    session.remove( attributeValue );
+                    auditType = AuditType.DELETE;
+
+                    logTrackedEntityAttributeValueHistory( preheat.getUsername(), attributeValue,
+                        trackedEntityInstance, auditType );
+                }
             }
             else
             {
@@ -399,12 +397,21 @@ protected void handleTrackedEntityAttributeValues( Session session, TrackerPrehe
                     assignFileResource( session, preheat, attributeValue.getValue() );
                 }
 
-                saveOrUpdate( session, isNew, attributeValue );
-                auditType = isNew ? AuditType.CREATE : AuditType.UPDATE;
-            }
+                saveOrUpdate( session, isNewAttribute, attributeValue );
+
+                // let's put newly stored TEAV in the tracked entity instance
+                // that will go in the preheat
+                // but only if its a new attribute
+                if ( isNewAttribute )
+                {
+                    trackedEntityInstance.getTrackedEntityAttributeValues().add( attributeValue );
+                }
 
-            logTrackedEntityAttributeValueHistory( preheat.getUsername(), attributeValue,
-                trackedEntityInstance, auditType );
+                auditType = isNewAttribute ? AuditType.CREATE : AuditType.UPDATE;
+
+                logTrackedEntityAttributeValueHistory( preheat.getUsername(), attributeValue,
+                    trackedEntityInstance, auditType );
+            }
 
             handleReservedValue( attributeValue );
         }

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

@@ -37,7 +37,6 @@
 import org.hisp.dhis.reservedvalue.ReservedValueService;
 import org.hisp.dhis.trackedentity.TrackedEntityProgramOwnerService;
 import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueAuditService;
-import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueService;
 import org.hisp.dhis.trackedentitycomment.TrackedEntityComment;
 import org.hisp.dhis.trackedentitycomment.TrackedEntityCommentService;
 import org.hisp.dhis.tracker.TrackerIdScheme;
@@ -69,10 +68,9 @@ public EnrollmentPersister( ReservedValueService reservedValueService,
         TrackedEntityCommentService trackedEntityCommentService,
         TrackerSideEffectConverterService sideEffectConverterService,
         TrackedEntityProgramOwnerService trackedEntityProgramOwnerService,
-        TrackedEntityAttributeValueAuditService trackedEntityAttributeValueAuditService,
-        TrackedEntityAttributeValueService attributeValueService )
+        TrackedEntityAttributeValueAuditService trackedEntityAttributeValueAuditService )
     {
-        super( reservedValueService, trackedEntityAttributeValueAuditService, attributeValueService );
+        super( reservedValueService, trackedEntityAttributeValueAuditService );
 
         this.enrollmentConverter = enrollmentConverter;
         this.trackedEntityCommentService = trackedEntityCommentService;
@@ -85,7 +83,7 @@ protected void updateAttributes( Session session, TrackerPreheat preheat,
         Enrollment enrollment, ProgramInstance programInstance )
     {
         handleTrackedEntityAttributeValues( session, preheat, enrollment.getAttributes(),
-            programInstance.getEntityInstance() );
+            preheat.getTrackedEntity( TrackerIdScheme.UID, programInstance.getEntityInstance().getUid() ) );
     }
 
     @Override

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

@@ -46,7 +46,6 @@
 import org.hisp.dhis.program.ProgramStageInstance;
 import org.hisp.dhis.reservedvalue.ReservedValueService;
 import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueAuditService;
-import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueService;
 import org.hisp.dhis.trackedentitycomment.TrackedEntityComment;
 import org.hisp.dhis.trackedentitycomment.TrackedEntityCommentService;
 import org.hisp.dhis.tracker.TrackerIdScheme;
@@ -78,10 +77,9 @@ public EventPersister( ReservedValueService reservedValueService,
         TrackerConverterService<Event, ProgramStageInstance> eventConverter,
         TrackedEntityCommentService trackedEntityCommentService,
         TrackerSideEffectConverterService sideEffectConverterService,
-        TrackedEntityAttributeValueAuditService trackedEntityAttributeValueAuditService,
-        TrackedEntityAttributeValueService attributeValueService )
+        TrackedEntityAttributeValueAuditService trackedEntityAttributeValueAuditService )
     {
-        super( reservedValueService, trackedEntityAttributeValueAuditService, attributeValueService );
+        super( reservedValueService, trackedEntityAttributeValueAuditService );
         this.eventConverter = eventConverter;
         this.trackedEntityCommentService = trackedEntityCommentService;
         this.sideEffectConverterService = sideEffectConverterService;

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

@@ -30,7 +30,6 @@
 import org.hibernate.Session;
 import org.hisp.dhis.reservedvalue.ReservedValueService;
 import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueAuditService;
-import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueService;
 import org.hisp.dhis.tracker.TrackerIdScheme;
 import org.hisp.dhis.tracker.TrackerType;
 import org.hisp.dhis.tracker.bundle.TrackerBundle;
@@ -51,11 +50,10 @@ public class RelationshipPersister
 
     public RelationshipPersister( ReservedValueService reservedValueService,
         TrackerConverterService<Relationship, org.hisp.dhis.relationship.Relationship> relationshipConverter,
-        TrackedEntityAttributeValueAuditService trackedEntityAttributeValueAuditService,
-        TrackedEntityAttributeValueService attributeValueService )
+        TrackedEntityAttributeValueAuditService trackedEntityAttributeValueAuditService )
 
     {
-        super( reservedValueService, trackedEntityAttributeValueAuditService, attributeValueService );
+        super( reservedValueService, trackedEntityAttributeValueAuditService );
         this.relationshipConverter = relationshipConverter;
     }
 

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

@@ -36,7 +36,6 @@
 import org.hisp.dhis.reservedvalue.ReservedValueService;
 import org.hisp.dhis.trackedentity.TrackedEntityInstance;
 import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueAuditService;
-import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueService;
 import org.hisp.dhis.tracker.TrackerIdScheme;
 import org.hisp.dhis.tracker.TrackerType;
 import org.hisp.dhis.tracker.bundle.TrackerBundle;
@@ -57,10 +56,9 @@ public class TrackedEntityPersister extends AbstractTrackerPersister<TrackedEnti
 
     public TrackedEntityPersister( ReservedValueService reservedValueService,
         TrackerConverterService<TrackedEntity, TrackedEntityInstance> teConverter,
-        TrackedEntityAttributeValueAuditService trackedEntityAttributeValueAuditService,
-        TrackedEntityAttributeValueService attributeValueService )
+        TrackedEntityAttributeValueAuditService trackedEntityAttributeValueAuditService )
     {
-        super( reservedValueService, trackedEntityAttributeValueAuditService, attributeValueService );
+        super( reservedValueService, trackedEntityAttributeValueAuditService );
         this.teConverter = teConverter;
     }