From 53119485eade4dd5e6ea01045c8eed670ad466bc Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Sat, 3 Oct 2026 13:47:18 -0700 Subject: [PATCH] GH Issue 1640: Keep provided values aligned across detailed audit batches Rows past the first 1,001 of an import or update were audited with an earlier row's provided amount. Also fixes a missing comma that left the delta provided keys out of the timeline's excluded fields. --- api/src/org/labkey/api/ApiModule.java | 2 + .../api/audit/SampleTimelineAuditEvent.java | 2 +- .../DetailedAuditLogDataIterator.java | 57 +++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index d484ce6e7ee..bcc8c8ab6f1 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -91,6 +91,7 @@ import org.labkey.api.data.dialect.StandardDialectStringHandler; import org.labkey.api.dataiterator.CachingDataIterator; import org.labkey.api.dataiterator.DataIteratorUtil; +import org.labkey.api.dataiterator.DetailedAuditLogDataIterator; import org.labkey.api.dataiterator.DiskCachingDataIterator; import org.labkey.api.dataiterator.ExistingRecordDataIterator; import org.labkey.api.dataiterator.GenerateUniqueDataIterator; @@ -437,6 +438,7 @@ public void registerServlets(ServletContext servletCtx) DateUtil.TestCase.class, DbScope.DialectTestCase.class, DeltaTrackingMap.TestCase.class, + DetailedAuditLogDataIterator.TestCase.class, DetailsURL.TestCase.class, DiskCachingDataIterator.DiskTestCase.class, EmailTemplate.TestCase.class, diff --git a/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java b/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java index 2ca84b2770c..7d10297bc9f 100644 --- a/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java +++ b/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java @@ -44,7 +44,7 @@ public class SampleTimelineAuditEvent extends DetailedAuditTypeEvent public static final Set EXCLUDED_DETAIL_FIELDS = Set.of( AvailableAliquotVolume.name(), AvailableAliquotCount.name(), AliquotCount.name(), AliquotVolume.name(), AliquotUnit.name(), PROVIDED_DATA_PREFIX + StoredAmount.name(), PROVIDED_DATA_PREFIX + Units.name(), - DELTA_PROVIDED_DATA_PREFIX + StoredAmount.name() + DELTA_PROVIDED_DATA_PREFIX + Units.name()); + DELTA_PROVIDED_DATA_PREFIX + StoredAmount.name(), DELTA_PROVIDED_DATA_PREFIX + Units.name()); public enum SampleTimelineEventType { diff --git a/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java b/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java index 48141d6240d..490775a91fd 100644 --- a/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java @@ -17,7 +17,10 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.junit.Assert; +import org.junit.Test; import org.labkey.api.audit.AuditHandler; +import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.TableInfo; @@ -28,8 +31,11 @@ import org.labkey.api.security.User; import java.io.IOException; +import java.lang.reflect.Proxy; import java.util.ArrayList; +import java.util.List; import java.util.Map; +import java.util.Set; import java.util.function.Function; import static org.labkey.api.gwt.client.AuditBehaviorType.DETAILED; @@ -105,6 +111,7 @@ public boolean next() throws BatchValidationException if (!_updatedRows.isEmpty()) _auditHandler.addAuditEvent(_user, _container, _table, DETAILED, _userComment, _auditAction, _updatedRows, _existingRows, _providedValues, _useTransactionAuditCache); _updatedRows.clear(); + _providedValues.clear(); if (null != _existingRows) _existingRows.clear(); } @@ -172,4 +179,54 @@ public boolean supportsGetExistingRecord() { return _data.supportsGetExistingRecord(); } + + public static class TestCase extends Assert + { + @Test + public void testProvidedValuesAlignAcrossBatches() throws Exception + { + int rowCount = 2500; + List> rows = new ArrayList<>(); + for (int i = 0; i < rowCount; i++) + rows.add(CaseInsensitiveHashMap.of("name", "S-" + i)); + + List batchSizes = new ArrayList<>(); + List auditedNames = new ArrayList<>(); + AuditHandler recorder = (AuditHandler) Proxy.newProxyInstance(getClass().getClassLoader(), new Class[]{AuditHandler.class}, (_, method, args) -> { + if (method.getName().equals("addAuditEvent") && args.length == 10) + { + @SuppressWarnings("unchecked") List> batchRows = (List>) args[6]; + @SuppressWarnings("unchecked") List> provided = (List>) args[8]; + assertEquals("provided values must pair one-to-one with the batch's rows", batchRows.size(), provided.size()); + batchSizes.add(batchRows.size()); + for (int i = 0; i < batchRows.size(); i++) + { + assertEquals(batchRows.get(i).get("name"), provided.get(i).get("providedName")); + auditedNames.add((String) batchRows.get(i).get("name")); + } + } + return null; + }); + TableInfo table = (TableInfo) Proxy.newProxyInstance(getClass().getClassLoader(), new Class[]{TableInfo.class}, (_, method, _) -> switch (method.getName()) + { + case "supportsAuditTracking" -> true; + case "getEffectiveAuditBehavior" -> DETAILED; + case "getAuditHandler" -> recorder; + default -> null; + }); + + DataIteratorBuilder source = _ -> new ListofMapsDataIterator(Set.of("name"), rows); + DataIteratorBuilder audited = getDataIteratorBuilder(table, source, QueryUpdateService.InsertOption.INSERT, null, null, row -> Map.of("providedName", row.get("name"))); + try (DataIterator it = audited.getDataIterator(new DataIteratorContext())) + { + assertTrue(it instanceof DetailedAuditLogDataIterator); + while (it.next()) + { + } + } + + assertTrue("expected more than one audit batch", batchSizes.size() > 1); + assertEquals(rows.stream().map(row -> row.get("name")).toList(), auditedNames); + } + } }