diff --git a/dhis-2/dhis-support/dhis-support-artemis/src/main/java/org/hisp/dhis/artemis/audit/listener/AbstractHibernateListener.java b/dhis-2/dhis-support/dhis-support-artemis/src/main/java/org/hisp/dhis/artemis/audit/listener/AbstractHibernateListener.java index e05daa102ec7..241f04c25383 100644 --- a/dhis-2/dhis-support/dhis-support-artemis/src/main/java/org/hisp/dhis/artemis/audit/listener/AbstractHibernateListener.java +++ b/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 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 collection = (Collection) collectionValue; if (IdentifiableObject.class.isAssignableFrom(property.getItemKlass())) { - List 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; diff --git a/dhis-2/dhis-support/dhis-support-artemis/src/test/java/org/hisp/dhis/artemis/audit/listener/AuditHibernateListenerTest.java b/dhis-2/dhis-support/dhis-support-artemis/src/test/java/org/hisp/dhis/artemis/audit/listener/AuditHibernateListenerTest.java index cc1246f47bb8..5c5d35fb8dcd 100644 --- a/dhis-2/dhis-support/dhis-support-artemis/src/test/java/org/hisp/dhis/artemis/audit/listener/AuditHibernateListenerTest.java +++ b/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 auditObjectMap = + (Map) + 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 auditObjectMap = + (Map) + 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 auditObjectMap = + (Map) + postInsertAuditListener.createAuditEntry(group, state, session, 1, persister); + + assertEquals(List.of(), auditObjectMap.get("members")); + } + + private void mockSchema(Class klass, Property... properties) { + Map 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);