fix: Do not load unloaded collections when building metadata audit entries [DHIS2-22235] (2.43)

dhis-2/dhis-support/dhis-support-artemis/src/main/java/org/hisp/dhis/artemis/audit/listener/AbstractHibernateListener.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004-2022, University of Oslo
+ * Copyright (c) 2004-2026, University of Oslo
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -41,6 +41,7 @@
 import java.util.Optional;
 import lombok.extern.slf4j.Slf4j;
 import org.hibernate.Hibernate;
+import org.hibernate.collection.spi.PersistentCollection;
 import org.hibernate.event.spi.EventSource;
 import org.hibernate.event.spi.PostDeleteEvent;
 import org.hibernate.event.spi.PostInsertEvent;
@@ -187,7 +188,7 @@ protected Object createAuditEntry(
       String pName = persister.getPropertyNames()[i];
       Property property = properties.get(pName);
 
-      if (shouldIgnoreProperty(property)) {
+      if (shouldIgnoreProperty(property) || isUnloadedCollection(value)) {
         continue;
       }
 
@@ -254,17 +255,11 @@ private void putValueToMap(Property property, Map<String, Object> objectMap, Obj
     if (value == null) return;
 
     if (property.isCollection() && (value instanceof Collection<?> collectionValue)) {
-
-      if (collectionValue.isEmpty()) return;
-
+      // a loaded empty collection is written as [], so a missing key only means "not loaded"
       Collection<IdentifiableObject> collection = (Collection<IdentifiableObject>) collectionValue;
 
       if (IdentifiableObject.class.isAssignableFrom(property.getItemKlass())) {
-        List<String> uids = IdentifiableObjectUtils.getUids(collection);
-
-        if (uids != null && !uids.isEmpty()) {
-          objectMap.put(property.getFieldName(), uids);
-        }
+        objectMap.put(property.getFieldName(), IdentifiableObjectUtils.getUids(collection));
       } else {
         handleNonIdentifiableCollection(property, value, objectMap);
       }
@@ -294,6 +289,14 @@ private boolean shouldInitializeProxy(Object value) {
     return true;
   }
 
+  /**
+   * Rule 2: a collection the request did not load is left out, as loading it here would read every
+   * element from the database (hundreds of thousands for large org unit sets).
+   */
+  private static boolean isUnloadedCollection(Object value) {
+    return value instanceof PersistentCollection collection && !collection.wasInitialized();
+  }
+
   private Object getId(Object object) {
     if (object instanceof IdentifiableObject obj) return obj.getUid();
     return object;

dhis-2/dhis-support/dhis-support-artemis/src/test/java/org/hisp/dhis/artemis/audit/listener/AuditHibernateListenerTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004-2022, University of Oslo
+ * Copyright (c) 2004-2026, University of Oslo
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -30,18 +30,27 @@
 package org.hisp.dhis.artemis.audit.listener;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
+import java.util.Set;
+import org.hibernate.collection.internal.PersistentSet;
 import org.hibernate.event.spi.EventSource;
 import org.hibernate.persister.entity.EntityPersister;
+import org.hibernate.proxy.HibernateProxy;
 import org.hisp.dhis.artemis.audit.AuditManager;
 import org.hisp.dhis.artemis.audit.configuration.AuditMatrix;
 import org.hisp.dhis.artemis.audit.legacy.AuditObjectFactory;
 import org.hisp.dhis.artemis.config.UsernameSupplier;
+import org.hisp.dhis.organisationunit.OrganisationUnit;
+import org.hisp.dhis.organisationunit.OrganisationUnitGroup;
 import org.hisp.dhis.schema.Property;
 import org.hisp.dhis.schema.Schema;
 import org.hisp.dhis.schema.SchemaService;
@@ -95,6 +104,110 @@ void testSavePassword() {
     assertEquals("userA", auditObjectMap.get("userName").toString());
   }
 
+  @Test
+  @SuppressWarnings("unchecked")
+  void testUnloadedCollectionIsNotLoadedOrIncluded() {
+    OrganisationUnitGroup group = new OrganisationUnitGroup("groupA");
+    // A collection that was not loaded by the request. It has no session, so any read of it
+    // (size, isEmpty, iteration) throws LazyInitializationException
+    PersistentSet members = new PersistentSet(null);
+
+    Object[] state = new Object[] {"groupA", members};
+    EventSource session = mock(EventSource.class);
+    EntityPersister persister = mock(EntityPersister.class);
+    when(persister.getPropertyNames()).thenReturn(new String[] {"name", "members"});
+    // Reading the property through an entity proxy returns the same unloaded collection
+    HibernateProxy proxy = mock(HibernateProxy.class);
+    lenient().when(persister.createProxy(1, session)).thenReturn(proxy);
+    lenient().when(persister.getPropertyValue(proxy, "members")).thenReturn(members);
+
+    mockSchema(
+        OrganisationUnitGroup.class,
+        createProperty("name", true),
+        createCollectionProperty("members", OrganisationUnit.class));
+
+    Map<String, Object> auditObjectMap =
+        (Map<String, Object>)
+            postInsertAuditListener.createAuditEntry(group, state, session, 1, persister);
+
+    assertFalse(members.wasInitialized());
+    assertFalse(auditObjectMap.containsKey("members"));
+    assertEquals("groupA", auditObjectMap.get("name"));
+  }
+
+  @Test
+  @SuppressWarnings("unchecked")
+  void testLoadedCollectionIsIncludedAsUids() {
+    OrganisationUnitGroup group = new OrganisationUnitGroup("groupA");
+    OrganisationUnit unit = new OrganisationUnit("unitA");
+    unit.setUid("ouabcdefghA");
+    PersistentSet members = new PersistentSet(null, Set.of(unit));
+
+    Object[] state = new Object[] {"groupA", members};
+    EventSource session = mock(EventSource.class);
+    EntityPersister persister = mock(EntityPersister.class);
+    when(persister.getPropertyNames()).thenReturn(new String[] {"name", "members"});
+
+    mockSchema(
+        OrganisationUnitGroup.class,
+        createProperty("name", true),
+        createCollectionProperty("members", OrganisationUnit.class));
+
+    Map<String, Object> auditObjectMap =
+        (Map<String, Object>)
+            postInsertAuditListener.createAuditEntry(group, state, session, 1, persister);
+
+    assertEquals(List.of("ouabcdefghA"), auditObjectMap.get("members"));
+  }
+
+  /**
+   * A collection that was loaded and is empty (e.g. every member removed) is written as an empty
+   * list, so that a missing key only ever means "not loaded".
+   */
+  @Test
+  @SuppressWarnings("unchecked")
+  void testLoadedEmptyCollectionIsIncludedAsEmptyList() {
+    OrganisationUnitGroup group = new OrganisationUnitGroup("groupA");
+    PersistentSet members = new PersistentSet(null, new HashSet<>());
+
+    Object[] state = new Object[] {"groupA", members};
+    EventSource session = mock(EventSource.class);
+    EntityPersister persister = mock(EntityPersister.class);
+    when(persister.getPropertyNames()).thenReturn(new String[] {"name", "members"});
+
+    mockSchema(
+        OrganisationUnitGroup.class,
+        createProperty("name", true),
+        createCollectionProperty("members", OrganisationUnit.class));
+
+    Map<String, Object> auditObjectMap =
+        (Map<String, Object>)
+            postInsertAuditListener.createAuditEntry(group, state, session, 1, persister);
+
+    assertEquals(List.of(), auditObjectMap.get("members"));
+  }
+
+  private void mockSchema(Class<?> klass, Property... properties) {
+    Map<String, Property> map = new HashMap<>();
+    for (Property property : properties) {
+      map.put(property.getFieldName(), property);
+    }
+    Schema schema = mock(Schema.class);
+    when(schema.getFieldNameMapProperties()).thenReturn(map);
+    when(schemaService.getSchema(klass)).thenReturn(schema);
+  }
+
+  private Property createCollectionProperty(String name, Class<?> itemKlass) {
+    Property property = new Property(Set.class);
+    property.setReadable(true);
+    property.setFieldName(name);
+    property.setOwner(true);
+    property.setEmbeddedObject(false);
+    property.setCollection(true);
+    property.setItemKlass(itemKlass);
+    return property;
+  }
+
   private User createUser(int id, String userName, String password) {
     User user = new User();
     user.setId(id);