fix: Prevent NPE in 'ou' ownership export [DHIS2-15467] (#14328) (#14344

)

* fix: Prevent NPE in 'ou' ownership export [DHIS2-15467]

* fix: Make constants private [DHIS2-15467]

* fix: Code formatting [DHIS2-15467]

* fix: More code formatting [DHIS2-15467]

dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcOwnershipAnalyticsTableManager.java

@@ -29,6 +29,7 @@
 
 import static java.util.Collections.emptyList;
 import static java.util.stream.Collectors.toList;
+import static org.apache.commons.lang3.StringUtils.SPACE;
 import static org.hisp.dhis.analytics.ColumnDataType.CHARACTER_11;
 import static org.hisp.dhis.analytics.ColumnDataType.DATE;
 import static org.hisp.dhis.analytics.ColumnNotNullConstraint.NOT_NULL;
@@ -212,20 +213,25 @@ private String getInputSql( Program program )
         // (The start date in the analytics table will be a far past date for
         // the first row for each TEI, or the previous row's end date plus one
         // day in subsequent rows for that TEI.)
+        //
+        // Rows in programownershiphistory that don't have organisationunitid
+        // will be filtered out.
 
         return sb.append( " from (" +
-            "select h.trackedentityinstanceid, '" + HISTORY_TABLE_ID
-            + "' as startdate, h.enddate as enddate, h.organisationunitid " +
+            "select h.trackedentityinstanceid, '" + HISTORY_TABLE_ID +
+            "' as startdate, h.enddate as enddate, h.organisationunitid " +
             "from programownershiphistory h " +
-            "where h.programid=" + program.getId() + " " +
+            "where h.programid=" + program.getId() + SPACE +
+            "and h.organisationunitid is not null " +
             "union " +
-            "select o.trackedentityinstanceid, '" + TEI_OWN_TABLE_ID
-            + "' as startdate, null as enddate, o.organisationunitid " +
+            "select o.trackedentityinstanceid, '" + TEI_OWN_TABLE_ID +
+            "' as startdate, null as enddate, o.organisationunitid " +
             "from trackedentityprogramowner o " +
-            "where o.programid=" + program.getId() + " " +
-            "and exists (select programid from programownershiphistory p " +
+            "where o.programid=" + program.getId() + SPACE +
+            "and exists (select 1 from programownershiphistory p " +
             "where o.trackedentityinstanceid = p.trackedentityinstanceid " +
-            "and p.programid=" + program.getId() + ")" +
+            "and p.programid=" + program.getId() + SPACE +
+            "and p.organisationunitid is not null)" +
             ") a " +
             "inner join trackedentityinstance tei on a.trackedentityinstanceid = tei.trackedentityinstanceid " +
             "inner join organisationunit ou on a.organisationunitid = ou.organisationunitid " +

dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcOwnershipWriter.java

@@ -38,6 +38,7 @@
 import java.util.GregorianCalendar;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 
 import lombok.AccessLevel;
 import lombok.RequiredArgsConstructor;
@@ -122,13 +123,17 @@ else if ( sameValue( OU ) || sameValue( ENDDATE ) )
     // -------------------------------------------------------------------------
 
     /**
-     * Process the first row for a TEI. For now just save it as the previous row
-     * and set the start date for far in the past.
+     * Process the first row for a TEI. Save it as the previous row and set the
+     * start date for far in the past. If the previous row does not have an
+     * "enddate", we enforce a default one.
      */
     private void startNewTei()
     {
         prevRow = newRow;
 
+        // Ensure a default "enddate" value.
+        prevRow.putIfAbsent( ENDDATE, FAR_FUTURE_DATE );
+
         prevRow.put( STARTDATE, FAR_PAST_DATE );
     }
 
@@ -206,6 +211,6 @@ private boolean hasNullValue( Map<String, Object> row, String colName )
      */
     private boolean sameValue( String colName )
     {
-        return prevRow.get( colName ).equals( newRow.get( colName ) );
+        return Objects.equals( prevRow.get( colName ), newRow.get( colName ) );
     }
 }

dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcOwnershipAnalyticsTableManagerTest.java

@@ -290,14 +290,14 @@ void testPopulateTable()
         assertEquals( "select tei.uid,a.startdate,a.enddate,ou.uid from (" +
             "select h.trackedentityinstanceid, '1001-01-01' as startdate, h.enddate as enddate, h.organisationunitid " +
             "from programownershiphistory h " +
-            "where h.programid=0 " +
+            "where h.programid=0 and h.organisationunitid is not null " +
             "union " +
             "select o.trackedentityinstanceid, '2002-02-02' as startdate, null as enddate, o.organisationunitid " +
             "from trackedentityprogramowner o " +
             "where o.programid=0 " +
-            "and exists (select programid from programownershiphistory p " +
+            "and exists (select 1 from programownershiphistory p " +
             "where o.trackedentityinstanceid = p.trackedentityinstanceid " +
-            "and p.programid=0)" +
+            "and p.programid=0 and p.organisationunitid is not null)" +
             ") a " +
             "inner join trackedentityinstance tei on a.trackedentityinstanceid = tei.trackedentityinstanceid " +
             "inner join organisationunit ou on a.organisationunitid = ou.organisationunitid " +

dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcOwnershipWriterTest.java

@@ -29,11 +29,13 @@
 
 import static java.util.Calendar.FEBRUARY;
 import static java.util.Calendar.JANUARY;
+import static org.apache.commons.lang3.reflect.FieldUtils.writeField;
 import static org.hisp.dhis.analytics.table.JdbcOwnershipWriter.ENDDATE;
 import static org.hisp.dhis.analytics.table.JdbcOwnershipWriter.OU;
 import static org.hisp.dhis.analytics.table.JdbcOwnershipWriter.STARTDATE;
 import static org.hisp.dhis.analytics.table.JdbcOwnershipWriter.TEIUID;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mockingDetails;
 import static org.mockito.Mockito.never;
@@ -134,7 +136,6 @@ void testWriteNoOwnershipChanges()
 
     @Test
     void testWriteOneOwnershipChange()
-        throws SQLException
     {
         writer.write( mapOf( TEIUID, teiA, OU, ouA, ENDDATE, date_2022_01 ) );
         writer.write( mapOf( TEIUID, teiA, OU, ouA, ENDDATE, date_2022_02 ) );
@@ -154,7 +155,6 @@ void testWriteOneOwnershipChange()
 
     @Test
     void testWriteTwoOwnershipChanges()
-        throws SQLException
     {
         writer.write( mapOf( TEIUID, teiA, OU, ouA, ENDDATE, date_2022_01 ) );
         writer.write( mapOf( TEIUID, teiA, OU, ouB, ENDDATE, date_2022_02 ) );
@@ -175,7 +175,6 @@ void testWriteTwoOwnershipChanges()
 
     @Test
     void testWriteThreeOwnershipChanges()
-        throws SQLException
     {
         writer.write( mapOf( TEIUID, teiA, OU, ouA, ENDDATE, date_2022_01 ) );
         writer.write( mapOf( TEIUID, teiA, OU, ouB, ENDDATE, date_2022_02 ) );
@@ -196,6 +195,19 @@ void testWriteThreeOwnershipChanges()
             getUpdateSql() );
     }
 
+    @Test
+    void testWriteWhenEndDateIsNull()
+        throws IllegalAccessException
+    {
+        JdbcOwnershipWriter writer = JdbcOwnershipWriter.getInstance( batchHandler );
+        Map<String, Object> prevRow = new HashMap<>();
+        writeField( writer, "prevRow", prevRow, true );
+
+        writer.write( mapOf( TEIUID, teiB, OU, ouB, ENDDATE, null ) );
+
+        assertNotNull( prevRow.get( ENDDATE ) );
+    }
+
     // -------------------------------------------------------------------------
     // Supportive methods
     // -------------------------------------------------------------------------