From 3525b1e0fea414cfafb372c7c5f1a66b0a6b1e41 Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 14 Aug 2025 21:37:55 -0700 Subject: [PATCH 01/14] LK R&D LKSM - Update default audit level --- .../api/gwt/client/AuditBehaviorType.java | 26 ++++++++++++--- .../api/audit/AbstractAuditHandler.java | 4 +-- .../labkey/api/data/AbstractTableInfo.java | 20 +++++++++-- .../org/labkey/api/data/SchemaTableInfo.java | 15 +++++++-- api/src/org/labkey/api/data/TableInfo.java | 33 ++++++++++--------- .../DetailedAuditLogDataIterator.java | 4 +-- .../ExistingRecordDataIterator.java | 2 +- .../org/labkey/api/exp/query/ExpSchema.java | 10 +++--- .../api/query/AbstractQueryImportAction.java | 2 +- .../org/labkey/api/query/FilteredTable.java | 5 ++- .../labkey/api/assay/AssayResultTable.java | 9 +++++ .../api/ExpDataClassDataTableImpl.java | 13 ++------ .../experiment/api/ExpMaterialTableImpl.java | 7 ++++ .../experiment/api/ExperimentServiceImpl.java | 10 ++---- .../experiment/api/SampleTypeServiceImpl.java | 2 +- .../org/labkey/query/QueryServiceImpl.java | 7 ++-- .../query/controllers/QueryController.java | 2 +- .../labkey/study/model/DatasetDefinition.java | 4 +-- 18 files changed, 112 insertions(+), 63 deletions(-) diff --git a/api/gwtsrc/org/labkey/api/gwt/client/AuditBehaviorType.java b/api/gwtsrc/org/labkey/api/gwt/client/AuditBehaviorType.java index 6fcdec69095..e369374e6d1 100644 --- a/api/gwtsrc/org/labkey/api/gwt/client/AuditBehaviorType.java +++ b/api/gwtsrc/org/labkey/api/gwt/client/AuditBehaviorType.java @@ -15,25 +15,43 @@ */ package org.labkey.api.gwt.client; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + /** * User: klum * Date: 10/17/12 */ public enum AuditBehaviorType { - NONE("None"), - DETAILED("Detailed"), - SUMMARY("Summary"); + NONE("None", 1), + DETAILED("Detailed", 3), + SUMMARY("Summary", 2); private final String _label; + private final int _priority; - AuditBehaviorType(String label) + AuditBehaviorType(String label, int priority) { _label = label; + _priority = priority; } public String getLabel() { return _label; } + + public int getPriority() + { + return _priority; + } + + public static AuditBehaviorType getEffectiveAuditLevel(@Nullable AuditBehaviorType apiOverride, @NotNull AuditBehaviorType tableAuditBehaviorType) + { + if (apiOverride == null || apiOverride._priority < tableAuditBehaviorType._priority) + return tableAuditBehaviorType; + + return apiOverride; + } } diff --git a/api/src/org/labkey/api/audit/AbstractAuditHandler.java b/api/src/org/labkey/api/audit/AbstractAuditHandler.java index d34c1bc0a16..f2d24b1ea74 100644 --- a/api/src/org/labkey/api/audit/AbstractAuditHandler.java +++ b/api/src/org/labkey/api/audit/AbstractAuditHandler.java @@ -26,7 +26,7 @@ public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryS if (table.supportsAuditTracking()) { AuditConfigurable auditConfigurable = (AuditConfigurable) table; - AuditBehaviorType auditType = auditBehaviorType == null ? auditConfigurable.getAuditBehavior() : auditBehaviorType; + AuditBehaviorType auditType = auditBehaviorType == null ? auditConfigurable.getDefaultAuditBehavior() : auditBehaviorType; // if (auditType == SUMMARY) { @@ -68,7 +68,7 @@ public void addAuditEvent(User user, Container c, TableInfo table, @Nullable Aud if (table.supportsAuditTracking()) { AuditConfigurable auditConfigurable = (AuditConfigurable)table; - auditType = auditConfigurable.getAuditBehavior(auditType); + auditType = auditConfigurable.getEffectiveAuditBehavior(auditType); // Truncate audit event doesn't accept any params if (action == QueryService.AuditAction.TRUNCATE) diff --git a/api/src/org/labkey/api/data/AbstractTableInfo.java b/api/src/org/labkey/api/data/AbstractTableInfo.java index 7b70be7085b..08463289182 100644 --- a/api/src/org/labkey/api/data/AbstractTableInfo.java +++ b/api/src/org/labkey/api/data/AbstractTableInfo.java @@ -276,10 +276,16 @@ public AbstractTableInfo(DbSchema schema, String name) _schema = schema; _columnMap = constructColumnMap(); // This is just an empty map, not populating any columns yet setName(name); + _auditBehaviorType = getDefaultAuditBehaviorType(); addTriggerFactory(new ScriptTriggerFactory()); MemTracker.getInstance().put(this); } + @NotNull + protected AuditBehaviorType getDefaultAuditBehaviorType() + { + return AuditBehaviorType.NONE; + } public void afterConstruct() { @@ -1487,7 +1493,16 @@ else if (!calculatedFieldKeys.isEmpty()) if (xmlTable.getAuditLogging() != null) { AuditType.Enum auditBehavior = xmlTable.getAuditLogging(); - setAuditBehavior(AuditBehaviorType.valueOf(auditBehavior.toString())); + try + { + AuditBehaviorType auditBehaviorType = AuditBehaviorType.valueOf(auditBehavior.toString()); + setAuditBehavior(auditBehaviorType); + } + catch (IllegalArgumentException ignore) + { + + } + } _warnings.addAll(warnings); @@ -2001,7 +2016,8 @@ public void setAuditBehavior(AuditBehaviorType type) } @Override - public AuditBehaviorType getAuditBehavior() + @NotNull + public AuditBehaviorType getDefaultAuditBehavior() { return _auditBehaviorType; } diff --git a/api/src/org/labkey/api/data/SchemaTableInfo.java b/api/src/org/labkey/api/data/SchemaTableInfo.java index 8f45396f2f3..64bb539d3ac 100644 --- a/api/src/org/labkey/api/data/SchemaTableInfo.java +++ b/api/src/org/labkey/api/data/SchemaTableInfo.java @@ -527,14 +527,15 @@ public boolean supportsAuditTracking() } @Override - public void setAuditBehavior(AuditBehaviorType type) + public void setAuditBehavior(@NotNull AuditBehaviorType type) { _auditBehaviorType = type; _xmlAuditBehaviorType = type; } @Override - public AuditBehaviorType getAuditBehavior() + @NotNull + public AuditBehaviorType getDefaultAuditBehavior() { return _auditBehaviorType; } @@ -629,7 +630,15 @@ public void loadTablePropertiesFromXml(TableType xmlTable, boolean dontSetName) if (xmlTable.getAuditLogging() != null) { AuditType.Enum auditBehavior = xmlTable.getAuditLogging(); - setAuditBehavior(AuditBehaviorType.valueOf(auditBehavior.toString())); + try + { + AuditBehaviorType auditBehaviorType = AuditBehaviorType.valueOf(auditBehavior.toString()); + setAuditBehavior(auditBehaviorType); + } + catch (IllegalArgumentException ignore) + { + + } } // Stash so we can overlay ColumnInfo properties later diff --git a/api/src/org/labkey/api/data/TableInfo.java b/api/src/org/labkey/api/data/TableInfo.java index 69523006ed4..1671fc34dc4 100644 --- a/api/src/org/labkey/api/data/TableInfo.java +++ b/api/src/org/labkey/api/data/TableInfo.java @@ -161,7 +161,7 @@ public void addAuditEvent(User user, Container c, TableInfo table, @Nullable Aud default AuditHandler getAuditHandler(@Nullable AuditBehaviorType auditBehaviorOverride) { - if (!supportsAuditTracking() || auditBehaviorOverride == AuditBehaviorType.NONE || (auditBehaviorOverride == null && getAuditBehavior() == AuditBehaviorType.NONE)) + if (!supportsAuditTracking() || getEffectiveAuditBehavior(auditBehaviorOverride) == AuditBehaviorType.NONE) return new _DoNothingAuditHandler(); return QueryService.get().getDefaultAuditHandler(); } @@ -662,43 +662,46 @@ void fireRowTrigger(Container c, User user, TriggerType type, boolean before, in Set getAllInvolvedColumns(Collection selectColumns); /** see interface AuditConfigurable */ - default AuditBehaviorType getAuditBehavior() + @NotNull + default AuditBehaviorType getDefaultAuditBehavior() { return AuditBehaviorType.NONE; } + @NotNull + default AuditBehaviorType getEffectiveAuditBehavior() + { + return getXmlAuditBehaviorType() == null ? getDefaultAuditBehavior() : getXmlAuditBehaviorType(); + } + /** * Retrieves the audit behavior for this table taking into account, in order of precedence: - * - the setting from the XML file (always returned if there is a value set) * - the override value provided + * - the setting from the XML file (always returned if there is a value set) * - the value supplied by this table's implementation * - * @param overrideValue value used to override the behavior type provided by the table implementation + * @param apiOverride value used to override the behavior type provided by the table implementation * @return audit behavior for this table */ - default AuditBehaviorType getAuditBehavior(@Nullable AuditBehaviorType overrideValue) + @NotNull + default AuditBehaviorType getEffectiveAuditBehavior(@Nullable AuditBehaviorType apiOverride) { - AuditBehaviorType type = getXmlAuditBehaviorType(); - if (type != null) - return type; - if (overrideValue != null) - return overrideValue; - return getAuditBehavior(); + return AuditBehaviorType.getEffectiveAuditLevel(apiOverride, getEffectiveAuditBehavior()); } - default AuditBehaviorType getAuditBehavior(@Nullable String overrideValue) + default AuditBehaviorType getEffectiveAuditBehavior(@Nullable String apiOverrideValue) { - if (overrideValue != null) + if (apiOverrideValue != null) { try { - return getAuditBehavior(AuditBehaviorType.valueOf(overrideValue)); + return getEffectiveAuditBehavior(AuditBehaviorType.valueOf(apiOverrideValue)); } catch (IllegalArgumentException ignored) { } } - return getAuditBehavior(); + return getEffectiveAuditBehavior(); } /* Can be used to distinguish AuditBehaviorType.NONE vs absent xml audit config */ diff --git a/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java b/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java index 2c9375f0b33..5cbeb80fdb7 100644 --- a/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java @@ -70,7 +70,7 @@ protected DetailedAuditLogDataIterator(DataIterator data, DataIteratorContext co _auditAction = auditAction; _auditHandler = table.getAuditHandler(DETAILED); - assert DETAILED == table.getAuditBehavior() || DETAILED == context.getConfigParameter(AuditConfigs.AuditBehavior); + assert DETAILED == table.getEffectiveAuditBehavior((AuditBehaviorType) context.getConfigParameter(AuditConfigs.AuditBehavior)); assert !context.getInsertOption().mergeRows || _data.supportsGetExistingRecord(); assert !context.getConfigParameterBoolean(QueryUpdateService.ConfigParameters.BulkLoad); @@ -129,7 +129,7 @@ public static DataIteratorBuilder getDataIteratorBuilder(TableInfo queryTable, @ { AuditBehaviorType auditType = AuditBehaviorType.NONE; if (queryTable.supportsAuditTracking()) - auditType = queryTable.getAuditBehavior((AuditBehaviorType) context.getConfigParameter(AuditConfigs.AuditBehavior)); + auditType = queryTable.getEffectiveAuditBehavior((AuditBehaviorType) context.getConfigParameter(AuditConfigs.AuditBehavior)); // Detailed auditing and not set to bulk load in ETL if (auditType == DETAILED && !context.getConfigParameterBoolean(QueryUpdateService.ConfigParameters.BulkLoad)) diff --git a/api/src/org/labkey/api/dataiterator/ExistingRecordDataIterator.java b/api/src/org/labkey/api/dataiterator/ExistingRecordDataIterator.java index e2a74d3b152..08230feea24 100644 --- a/api/src/org/labkey/api/dataiterator/ExistingRecordDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/ExistingRecordDataIterator.java @@ -228,7 +228,7 @@ public static DataIteratorBuilder createBuilder(DataIteratorBuilder dib, TableIn { AuditBehaviorType auditType = AuditBehaviorType.NONE; if (target.supportsAuditTracking()) - auditType = target.getAuditBehavior((AuditBehaviorType) context.getConfigParameter(DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior)); + auditType = target.getEffectiveAuditBehavior((AuditBehaviorType) context.getConfigParameter(DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior)); boolean detailed = auditType == DETAILED; if (useGetRows) return new ExistingDataIteratorsGetRows(new CachingDataIterator(di), target, keys, sharedKeys, context, detailed); diff --git a/api/src/org/labkey/api/exp/query/ExpSchema.java b/api/src/org/labkey/api/exp/query/ExpSchema.java index 2348e8c7ff3..f302e8ab126 100644 --- a/api/src/org/labkey/api/exp/query/ExpSchema.java +++ b/api/src/org/labkey/api/exp/query/ExpSchema.java @@ -440,16 +440,14 @@ public Set getPermittedOps() */ public enum DataClassCategoryType { - registry(null, null), - media(null, null), - sources(AuditBehaviorType.DETAILED, ADDITIONAL_SOURCES_AUDIT_FIELDS); + registry(null), + media(null), + sources(ADDITIONAL_SOURCES_AUDIT_FIELDS); - public final AuditBehaviorType defaultBehavior; public final Set additionalAuditFields; - DataClassCategoryType(@Nullable AuditBehaviorType defaultBehavior, @Nullable Set addlAuditFields) + DataClassCategoryType(@Nullable Set addlAuditFields) { - this.defaultBehavior = defaultBehavior; this.additionalAuditFields = addlAuditFields; } diff --git a/api/src/org/labkey/api/query/AbstractQueryImportAction.java b/api/src/org/labkey/api/query/AbstractQueryImportAction.java index f2add325252..e54fd778555 100644 --- a/api/src/org/labkey/api/query/AbstractQueryImportAction.java +++ b/api/src/org/labkey/api/query/AbstractQueryImportAction.java @@ -437,7 +437,7 @@ public final ApiResponse _execute(FORM form, BindException errors) throws Except // Check first if the audit behavior has been defined for the table either in code or through XML. // If not defined there, check for the audit behavior defined in the action form (getAuditBehaviorType()). - AuditBehaviorType behaviorType = (_target != null) ? _target.getAuditBehavior(getAuditBehaviorType()) : getAuditBehaviorType(); + AuditBehaviorType behaviorType = (_target != null) ? _target.getEffectiveAuditBehavior(getAuditBehaviorType()) : getAuditBehaviorType(); try { diff --git a/api/src/org/labkey/api/query/FilteredTable.java b/api/src/org/labkey/api/query/FilteredTable.java index 82c09333aa6..26bdf19b19f 100644 --- a/api/src/org/labkey/api/query/FilteredTable.java +++ b/api/src/org/labkey/api/query/FilteredTable.java @@ -126,7 +126,10 @@ public FilteredTable(@NotNull TableInfo table, @NotNull SchemaType userSchema, @ _importTemplates = _rootTable.getRawImportTemplates(); _buttonBarConfig = BUTTON_BAR_NOT_SET; // lazy copy button bar when asked if (_rootTable.supportsAuditTracking()) - _auditBehaviorType = _rootTable.getAuditBehavior(); + { + _auditBehaviorType = _rootTable.getDefaultAuditBehavior(); + _xmlAuditBehaviorType = _rootTable.getXmlAuditBehaviorType(); + } // We used to copy the titleColumn from table, but this forced all ColumnInfos to load. Now, delegate // to _rootTable lazily, allowing overrides. diff --git a/assay/api-src/org/labkey/api/assay/AssayResultTable.java b/assay/api-src/org/labkey/api/assay/AssayResultTable.java index eac7ab03e07..ebddccc195d 100644 --- a/assay/api-src/org/labkey/api/assay/AssayResultTable.java +++ b/assay/api-src/org/labkey/api/assay/AssayResultTable.java @@ -50,6 +50,7 @@ import org.labkey.api.exp.property.DomainProperty; import org.labkey.api.exp.property.DomainUtil; import org.labkey.api.exp.query.ExpSchema; +import org.labkey.api.gwt.client.AuditBehaviorType; import org.labkey.api.query.AliasedColumn; import org.labkey.api.query.ExprColumn; import org.labkey.api.query.FieldKey; @@ -682,4 +683,12 @@ public boolean supportTableRules() { return super.supportTableRules(); } + + @Override + @NotNull + protected AuditBehaviorType getDefaultAuditBehaviorType() + { + return AuditBehaviorType.DETAILED; + } + } diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index f48ea42e2c2..75ed376919c 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -200,17 +200,10 @@ public Domain getDomain(boolean forUpdate) } @Override - public AuditBehaviorType getAuditBehavior() + @NotNull + protected AuditBehaviorType getDefaultAuditBehaviorType() { - // if there is xml config, use xml config - if (_auditBehaviorType == AuditBehaviorType.NONE && getXmlAuditBehaviorType() == null) - { - ExpSchema.DataClassCategoryType categoryType = ExpSchema.DataClassCategoryType.fromString(_dataClass.getCategory()); - if (categoryType != null && categoryType.defaultBehavior != null) - return categoryType.defaultBehavior; - } - - return _auditBehaviorType; + return AuditBehaviorType.DETAILED; } @Override diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index babcdbd03b0..9368839b2e1 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -1665,6 +1665,13 @@ public DataIteratorBuilder persistRows(DataIteratorBuilder data, DataIteratorCon } } + @Override + @NotNull + protected AuditBehaviorType getDefaultAuditBehaviorType() + { + return AuditBehaviorType.DETAILED; + } + static final Set excludeFromDetailedAuditField; static { diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 264c280b741..5790f84367a 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9561,10 +9561,10 @@ public Map moveDataClassObjects(Collection d // create summary audit entries for the source container only. The message is pretty generic, so having it // in both source and target doesn't help much. - addDataClassSummaryAuditEvent(user, sourceContainer, dataClassTable, updateCount, userComment); + QueryService.get().getDefaultAuditHandler().addSummaryAuditEvent(user, sourceContainer, dataClassTable, QueryService.AuditAction.UPDATE, updateCount, AuditBehaviorType.SUMMARY, userComment); // create new detailed events for each data object that was moved - AuditBehaviorType dcAuditBehavior = dataClassTable.getAuditBehavior(auditBehavior); + AuditBehaviorType dcAuditBehavior = dataClassTable.getEffectiveAuditBehavior(auditBehavior); if (dcAuditBehavior == AuditBehaviorType.DETAILED) { List> oldRows = new ArrayList<>(); @@ -9599,12 +9599,6 @@ public Map moveDataClassObjects(Collection d return updateCounts; } - private void addDataClassSummaryAuditEvent(User user, Container container, TableInfo dataClassTable, int rowCount, @Nullable String auditUserComment) - { - QueryService queryService = QueryService.get(); - queryService.getDefaultAuditHandler().addSummaryAuditEvent(user, container, dataClassTable, QueryService.AuditAction.UPDATE, rowCount, AuditBehaviorType.SUMMARY, auditUserComment); - } - private void moveDataClassObjectAttachments(ExpDataClass dataClass, Collection classObjects, Container targetContainer, User user) { List attachmentDomainProps = dataClass.getDomain() diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index 7dc15f24429..dbea9bbb73d 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -1894,7 +1894,7 @@ public Map moveSamples(Collection sample int auditEventCount = auditProvider.moveEvents(targetContainer, sampleIds); updateCounts.compute("sampleAuditEvents", (k, c) -> c == null ? auditEventCount : c + auditEventCount ); - AuditBehaviorType stAuditBehavior = samplesTable.getAuditBehavior(auditBehavior); + AuditBehaviorType stAuditBehavior = samplesTable.getEffectiveAuditBehavior(auditBehavior); // create new events for each sample that was moved. if (stAuditBehavior == AuditBehaviorType.DETAILED) { diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index eb16501c35a..110cd0dc18c 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -53,7 +53,6 @@ import org.labkey.api.data.ConvertHelper; import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbSchemaType; -import org.labkey.api.data.DbScope; import org.labkey.api.data.DisplayColumn; import org.labkey.api.data.Filter; import org.labkey.api.data.ForeignKey; @@ -3202,9 +3201,9 @@ public List getQueryUpdateAuditRecords(User user, Contai { if (table.supportsAuditTracking()) { - AuditBehaviorType auditBehavior = table.getAuditBehavior(); + AuditBehaviorType auditBehavior = table.getEffectiveAuditBehavior(); - if (auditBehavior != null && auditBehavior != AuditBehaviorType.NONE) + if (auditBehavior != AuditBehaviorType.NONE) { return new ActionURL(QueryController.AuditHistoryAction.class, c). addParameter(QueryParam.schemaName, table.getPublicSchemaName()). @@ -3220,7 +3219,7 @@ public DetailsURL getAuditDetailsURL(User user, Container c, TableInfo table) if (table.supportsAuditTracking()) { AuditConfigurable auditConfigurable = (AuditConfigurable)table; - if (auditConfigurable.getAuditBehavior() != AuditBehaviorType.NONE) + if (auditConfigurable.getEffectiveAuditBehavior() != AuditBehaviorType.NONE) { FieldKey rowPk = auditConfigurable.getAuditRowPk(); diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index 8c1ae5569e2..c56ee43c32d 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -4618,7 +4618,7 @@ protected JSONObject executeJson(JSONObject json, CommandType commandType, boole // Check first if the audit behavior has been defined for the table either in code or through XML. // If not defined there, check for the audit behavior defined in the action form (json). - AuditBehaviorType behaviorType = table.getAuditBehavior(json.optString("auditBehavior", null)); + AuditBehaviorType behaviorType = table.getEffectiveAuditBehavior(json.optString("auditBehavior", null)); if (behaviorType != null) { configParameters.put(DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior, behaviorType); diff --git a/study/src/org/labkey/study/model/DatasetDefinition.java b/study/src/org/labkey/study/model/DatasetDefinition.java index 08a39bc4114..9e058748eb7 100644 --- a/study/src/org/labkey/study/model/DatasetDefinition.java +++ b/study/src/org/labkey/study/model/DatasetDefinition.java @@ -1742,7 +1742,7 @@ public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryS @Override public void addAuditEvent(User user, Container c, TableInfo table, @Nullable AuditBehaviorType auditTypeOverride, @Nullable String userComment, QueryService.AuditAction action, @Nullable List> rows, @Nullable List> existingRows) { - switch (table.getAuditBehavior(auditTypeOverride)) + switch (table.getEffectiveAuditBehavior(auditTypeOverride)) { case NONE,SUMMARY -> {} case DETAILED -> @@ -1846,7 +1846,7 @@ public void addAuditEvent(User user, Container c, AuditBehaviorType requiredAudi { TableInfo table = _dataset.getTableInfo(user); - if (table != null && table.getAuditBehavior((AuditBehaviorType)null) != requiredAuditType) + if (table != null && table.getEffectiveAuditBehavior((AuditBehaviorType)null) != requiredAuditType) return; DatasetAuditProvider.DatasetAuditEvent event = new DatasetAuditProvider.DatasetAuditEvent(c, comment, _dataset.getDatasetId()); From c411e5ac62bed04fd3c2e0abc7112cf7693f9d1f Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 15 Aug 2025 10:41:52 -0700 Subject: [PATCH 02/14] LK R&D LKSM - fix build --- api/src/org/labkey/api/audit/AbstractAuditHandler.java | 3 ++- api/src/org/labkey/api/data/AbstractTableInfo.java | 2 +- api/src/org/labkey/api/data/SchemaTableInfo.java | 2 +- api/src/org/labkey/api/data/TableInfo.java | 4 ++-- api/src/org/labkey/api/query/FilteredTable.java | 2 +- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/audit/AbstractAuditHandler.java b/api/src/org/labkey/api/audit/AbstractAuditHandler.java index f2d24b1ea74..b3c06f947d0 100644 --- a/api/src/org/labkey/api/audit/AbstractAuditHandler.java +++ b/api/src/org/labkey/api/audit/AbstractAuditHandler.java @@ -26,7 +26,8 @@ public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryS if (table.supportsAuditTracking()) { AuditConfigurable auditConfigurable = (AuditConfigurable) table; - AuditBehaviorType auditType = auditBehaviorType == null ? auditConfigurable.getDefaultAuditBehavior() : auditBehaviorType; // + // don't use auditConfigurable.getAuditBehavior(auditBehaviorType) to allow extra summary independent of table config + AuditBehaviorType auditType = auditBehaviorType == null ? auditConfigurable.getAuditBehavior() : auditBehaviorType; if (auditType == SUMMARY) { diff --git a/api/src/org/labkey/api/data/AbstractTableInfo.java b/api/src/org/labkey/api/data/AbstractTableInfo.java index 08463289182..f8d3d95b93e 100644 --- a/api/src/org/labkey/api/data/AbstractTableInfo.java +++ b/api/src/org/labkey/api/data/AbstractTableInfo.java @@ -2017,7 +2017,7 @@ public void setAuditBehavior(AuditBehaviorType type) @Override @NotNull - public AuditBehaviorType getDefaultAuditBehavior() + public AuditBehaviorType getAuditBehavior() { return _auditBehaviorType; } diff --git a/api/src/org/labkey/api/data/SchemaTableInfo.java b/api/src/org/labkey/api/data/SchemaTableInfo.java index 64bb539d3ac..e316ebe78db 100644 --- a/api/src/org/labkey/api/data/SchemaTableInfo.java +++ b/api/src/org/labkey/api/data/SchemaTableInfo.java @@ -535,7 +535,7 @@ public void setAuditBehavior(@NotNull AuditBehaviorType type) @Override @NotNull - public AuditBehaviorType getDefaultAuditBehavior() + public AuditBehaviorType getAuditBehavior() { return _auditBehaviorType; } diff --git a/api/src/org/labkey/api/data/TableInfo.java b/api/src/org/labkey/api/data/TableInfo.java index 1671fc34dc4..68d523abcff 100644 --- a/api/src/org/labkey/api/data/TableInfo.java +++ b/api/src/org/labkey/api/data/TableInfo.java @@ -663,7 +663,7 @@ void fireRowTrigger(Container c, User user, TriggerType type, boolean before, in /** see interface AuditConfigurable */ @NotNull - default AuditBehaviorType getDefaultAuditBehavior() + default AuditBehaviorType getAuditBehavior() { return AuditBehaviorType.NONE; } @@ -671,7 +671,7 @@ default AuditBehaviorType getDefaultAuditBehavior() @NotNull default AuditBehaviorType getEffectiveAuditBehavior() { - return getXmlAuditBehaviorType() == null ? getDefaultAuditBehavior() : getXmlAuditBehaviorType(); + return getXmlAuditBehaviorType() == null ? getAuditBehavior() : getXmlAuditBehaviorType(); } /** diff --git a/api/src/org/labkey/api/query/FilteredTable.java b/api/src/org/labkey/api/query/FilteredTable.java index 26bdf19b19f..28ffb9ce404 100644 --- a/api/src/org/labkey/api/query/FilteredTable.java +++ b/api/src/org/labkey/api/query/FilteredTable.java @@ -127,7 +127,7 @@ public FilteredTable(@NotNull TableInfo table, @NotNull SchemaType userSchema, @ _buttonBarConfig = BUTTON_BAR_NOT_SET; // lazy copy button bar when asked if (_rootTable.supportsAuditTracking()) { - _auditBehaviorType = _rootTable.getDefaultAuditBehavior(); + _auditBehaviorType = _rootTable.getAuditBehavior(); _xmlAuditBehaviorType = _rootTable.getXmlAuditBehaviorType(); } From 5af5af22785871a4f3fafc78baa986d6c560eeb8 Mon Sep 17 00:00:00 2001 From: XingY Date: Sun, 17 Aug 2025 22:23:07 -0700 Subject: [PATCH 03/14] Allow bypassing audit log for inventory move events --- .../labkey/api/dataiterator/DetailedAuditLogDataIterator.java | 2 +- api/src/org/labkey/api/query/QueryUpdateService.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java b/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java index 5cbeb80fdb7..570c093676a 100644 --- a/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java @@ -132,7 +132,7 @@ public static DataIteratorBuilder getDataIteratorBuilder(TableInfo queryTable, @ auditType = queryTable.getEffectiveAuditBehavior((AuditBehaviorType) context.getConfigParameter(AuditConfigs.AuditBehavior)); // Detailed auditing and not set to bulk load in ETL - if (auditType == DETAILED && !context.getConfigParameterBoolean(QueryUpdateService.ConfigParameters.BulkLoad)) + if (auditType == DETAILED && !context.getConfigParameterBoolean(QueryUpdateService.ConfigParameters.BulkLoad) && !context.getConfigParameterBoolean(QueryUpdateService.ConfigParameters.ByPassAudit)) { DataIterator it = builder.getDataIterator(context); DataIterator in = DataIteratorUtil.wrapMap(it, true); diff --git a/api/src/org/labkey/api/query/QueryUpdateService.java b/api/src/org/labkey/api/query/QueryUpdateService.java index 1aea04e48fa..3bca51ae314 100644 --- a/api/src/org/labkey/api/query/QueryUpdateService.java +++ b/api/src/org/labkey/api/query/QueryUpdateService.java @@ -112,7 +112,8 @@ enum ConfigParameters SkipInsertOptionValidation, // (Bool) Skip assert(supportsInsertOption(context.getInsertOption())) for special scenarios (e.g., folder import uses merge action that's otherwise not supported for a table), PreferPKOverObjectUriAsKey, // (Bool) Prefer getPkColumnNames instead of getObjectURIColumnName to use as keys SkipReselectRows, // (Bool) If true, skip qus.getRows and use raw returned rows. Applicable for CommandType.insert/insertWithKeys/update/updateChangingKeys - TargetContainer + TargetContainer, + ByPassAudit // (Bool) If true, skip DetailedAuditLogDataIterator } From 6c5708d30234d8306d19fedce05a8df0264c605f Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 19 Aug 2025 16:53:36 -0700 Subject: [PATCH 04/14] LK R&D LKSM - Update default audit level --- .../DetailedAuditLogDataIterator.java | 1 + .../api/assay/AssayResultUpdateService.java | 109 +++++------------- 2 files changed, 32 insertions(+), 78 deletions(-) diff --git a/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java b/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java index 570c093676a..70678f012b6 100644 --- a/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java @@ -73,6 +73,7 @@ protected DetailedAuditLogDataIterator(DataIterator data, DataIteratorContext co assert DETAILED == table.getEffectiveAuditBehavior((AuditBehaviorType) context.getConfigParameter(AuditConfigs.AuditBehavior)); assert !context.getInsertOption().mergeRows || _data.supportsGetExistingRecord(); assert !context.getConfigParameterBoolean(QueryUpdateService.ConfigParameters.BulkLoad); + assert !context.getConfigParameterBoolean(QueryUpdateService.ConfigParameters.ByPassAudit); _existingRows = _data.supportsGetExistingRecord() ? new ArrayList<>() : null; } diff --git a/assay/api-src/org/labkey/api/assay/AssayResultUpdateService.java b/assay/api-src/org/labkey/api/assay/AssayResultUpdateService.java index 547ca23a36e..635f21fc3b8 100644 --- a/assay/api-src/org/labkey/api/assay/AssayResultUpdateService.java +++ b/assay/api-src/org/labkey/api/assay/AssayResultUpdateService.java @@ -16,8 +16,6 @@ package org.labkey.api.assay; import jakarta.servlet.http.HttpServletRequest; -import org.apache.commons.beanutils.ConversionException; -import org.apache.commons.beanutils.ConvertUtils; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -68,7 +66,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import static org.labkey.api.dataiterator.DetailedAuditLogDataIterator.AuditConfigs.AuditUserComment; @@ -77,6 +74,8 @@ public class AssayResultUpdateService extends DefaultQueryUpdateService private final AssaySampleLookupContext _assaySampleLookupContext; private final AssayProtocolSchema _schema; + private Map dataChangeCount; + public AssayResultUpdateService(AssayProtocolSchema schema, FilteredTable table) { super(table, table.getRealTable(), createMVMapping(schema.getProvider().getResultsDomain(schema.getProtocol()))); @@ -87,6 +86,27 @@ public AssayResultUpdateService(AssayProtocolSchema schema, FilteredTable table) _schema = schema; } + private void addRunAuditSummary(User user, @Nullable Map configParameters, String verb) + { + for (Long runId: dataChangeCount.keySet()) + { + String userComment = configParameters == null ? null : (String) configParameters.get(AuditUserComment); + var run = ExperimentService.get().getExpRun(runId); + int deletedCount = dataChangeCount.get(runId); + + ExperimentService.get().auditRunEvent(user, run.getProtocol(), run, null, deletedCount + " data row" + (deletedCount > 1 ? "s have" : " has") + " been " + verb + " in " + run.getProtocol().getName() + ".", userComment); + + } + dataChangeCount = null; + } + + private void incrementAuditRowCount(ExpRun run) + { + long runId = run.getRowId(); + dataChangeCount.putIfAbsent(runId, 0); + dataChangeCount.put(runId, dataChangeCount.get(runId) + 1); + } + @Override public List> updateRows( User user, @@ -98,6 +118,7 @@ public List> updateRows( Map extraScriptContext ) throws InvalidKeyException, BatchValidationException, QueryUpdateServiceException, SQLException { + dataChangeCount = new HashMap<>(); // handle transform scripts rows = transform(container, user, rows, oldKeys); var result = super.updateRows(user, container, rows, oldKeys, errors, configParameters, extraScriptContext); @@ -107,6 +128,8 @@ public List> updateRows( if (errors.hasErrors()) throw errors; + addRunAuditSummary(user, configParameters, "edited"); + return result; } @@ -263,70 +286,12 @@ protected Map updateRow( convertTypes(user, container, row, getDbTable(), assayResultsRunDir); Map result = super.updateRow(user, container, row, oldRow, configParameters); - Map updatedValues = getRow(user, container, oldRow); - - StringBuilder sb = new StringBuilder("Data row, id " + oldRow.get("RowId") + ", edited in " + run.getProtocol().getName() + "."); - for (Map.Entry entry : result.entrySet()) - { - // Also check for properties - TableInfo table = getQueryTable(); - ColumnInfo col = table.getColumn(entry.getKey()); - - if (col != null) - { - Object oldValue = col.getValue(originalRow); - Object newValue = col.getValue(updatedValues); - boolean hasValueChanged = !Objects.equals(oldValue, newValue); - - if (hasValueChanged) - _assaySampleLookupContext.trackSampleLookupChange(container, user, table, col, run); - - TableInfo fkTableInfo = col.getFkTableInfo(); - // Don't follow the lookup for specimen IDs, since their FK is very special and based on target study, etc - if (hasValueChanged && fkTableInfo != null && !AbstractAssayProvider.SPECIMENID_PROPERTY_NAME.equalsIgnoreCase(col.getName())) - { - // Do type conversion in case there's a mismatch in the lookup source and target columns - ColumnInfo fkTablePkCol = fkTableInfo.getPkColumns().get(0); - newValue = lookupDisplayValue(newValue, fkTableInfo, fkTablePkCol); - oldValue = lookupDisplayValue(oldValue, fkTableInfo, fkTablePkCol); - } - appendPropertyIfChanged(sb, col.getLabel(), oldValue, newValue); - } - } - String userComment = configParameters == null ? null : (String) configParameters.get(AuditUserComment); - ExperimentService.get().auditRunEvent(user, run.getProtocol(), run, null, sb.toString(), userComment); + incrementAuditRowCount(run); return result; } - private Object lookupDisplayValue(Object o, @NotNull TableInfo fkTableInfo, ColumnInfo fkTablePkCol) - { - if (o == null) - return null; - - if (fkTablePkCol == null) - return o; - - if (!fkTablePkCol.getJavaClass().isAssignableFrom(o.getClass())) - { - try - { - o = ConvertUtils.convert(o.toString(), fkTablePkCol.getJavaClass()); - Map newLookupTarget = new TableSelector(fkTableInfo).getMap(o); - if (newLookupTarget != null) - { - o = newLookupTarget.get(fkTableInfo.getTitleColumn()); - } - } - catch (ConversionException ex) - { - // ok - just use the value as is - } - } - - return o; - } private ExpRun getRun(Map row, User user, Class perm) throws InvalidKeyException { @@ -369,8 +334,7 @@ protected Map deleteRow( Map result = super.deleteRow(user, container, oldRowMap); - String userComment = configParameters == null ? null : (String) configParameters.get(AuditUserComment); - ExperimentService.get().auditRunEvent(user, run.getProtocol(), run, null, "Deleted data row, id " + oldRowMap.get("RowId"), userComment); + incrementAuditRowCount(run); if (null != dataObjectMap) { @@ -397,6 +361,7 @@ protected Map deleteRow( @Override public List> deleteRows(User user, Container container, List> keys, @Nullable Map configParameters, @Nullable Map extraScriptContext) throws InvalidKeyException, BatchValidationException, QueryUpdateServiceException, SQLException { + dataChangeCount = new HashMap<>(); var result = super.deleteRows(user, container, keys, configParameters, extraScriptContext); BatchValidationException errors = new BatchValidationException(); @@ -405,21 +370,9 @@ public List> deleteRows(User user, Container container, List if (errors.hasErrors()) throw errors; - return result; - } + addRunAuditSummary(user, configParameters, "deleted"); - private void appendPropertyIfChanged(StringBuilder sb, String label, Object oldValue, Object newValue) - { - if (Objects.equals(oldValue, newValue)) - return; - - sb.append(" "); - sb.append(label); - sb.append(" changed from "); - sb.append(oldValue == null ? "blank" : "'" + oldValue + "'"); - sb.append(" to "); - sb.append(newValue == null ? "blank" : "'" + newValue + "'"); - sb.append("."); + return result; } /** From 9ba473b5e257e6b7b2d84cc0fb94f0242fed5e21 Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 20 Aug 2025 11:14:12 -0700 Subject: [PATCH 05/14] metric --- .../experiment/api/ExperimentServiceImpl.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index d1a49c51b1c..c046309f26f 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9138,9 +9138,38 @@ public Map> getDomainMetrics() metrics.put("nameexpression", getNameExpressionMetrics()); metrics.put("parentalias", getParentAliasMetrics()); metrics.put("importTemplates", getImportTemplatesMetrics()); + metrics.put("auditLevelOverride", getAuditLevelOverrideMetrics()); return metrics; } + private Map getAuditLevelOverrideMetrics() + { + DbSchema dbSchema = CoreSchema.getInstance().getSchema(); + SQLFragment sql = new SQLFragment("SELECT \"schema\", metadata FROM query.querydef WHERE metadata LIKE '%%'"); + Map[] results = new SqlSelector(dbSchema, sql).getMapArray(); + Map counts = new HashMap<>(); + final String auditNone = "none"; + final String auditSummary = "summary"; + for (Map result : results) + { + String schema = (String) result.get("schema"); + String metadata = (String) result.get("metadata"); + metadata = metadata.toLowerCase(); + if (schema.toLowerCase().startsWith("assay.general.")) + schema = "assay"; + if (schema.equals("assay") || schema.equals("exp.data") || schema.equals("samples")) + { + if (!metadata.contains(auditNone) && !metadata.contains(auditSummary)) + continue; + long count = counts.get(schema) == null ? 0L : counts.get(schema); + counts.put(schema, ++count); + } + } + return Map.of("SampleType", counts.getOrDefault("samples", 0L), + "DataClass", counts.getOrDefault("exp.data", 0L), + "AssayDesign", counts.getOrDefault("assay", 0L)); + } + private Map getImportTemplatesMetrics() { DbSchema dbSchema = CoreSchema.getInstance().getSchema(); From 1342be19ed12b1b811cf2d79532771db2ea5d1bf Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 20 Aug 2025 19:11:58 -0700 Subject: [PATCH 06/14] Selenium test --- .../org/labkey/api/audit/AbstractAuditHandler.java | 4 +++- api/src/org/labkey/api/data/AbstractTableInfo.java | 12 +++--------- api/src/org/labkey/api/data/SchemaTableInfo.java | 6 +++--- api/src/org/labkey/api/data/TableInfo.java | 4 ++-- api/src/org/labkey/api/query/FilteredTable.java | 2 +- .../org/labkey/api/assay/AssayResultTable.java | 2 +- .../experiment/api/ExpDataClassDataTableImpl.java | 2 +- .../labkey/experiment/api/ExpMaterialTableImpl.java | 2 +- 8 files changed, 15 insertions(+), 19 deletions(-) diff --git a/api/src/org/labkey/api/audit/AbstractAuditHandler.java b/api/src/org/labkey/api/audit/AbstractAuditHandler.java index b3c06f947d0..6579b290501 100644 --- a/api/src/org/labkey/api/audit/AbstractAuditHandler.java +++ b/api/src/org/labkey/api/audit/AbstractAuditHandler.java @@ -27,7 +27,7 @@ public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryS { AuditConfigurable auditConfigurable = (AuditConfigurable) table; // don't use auditConfigurable.getAuditBehavior(auditBehaviorType) to allow extra summary independent of table config - AuditBehaviorType auditType = auditBehaviorType == null ? auditConfigurable.getAuditBehavior() : auditBehaviorType; + AuditBehaviorType auditType = auditBehaviorType == null ? auditConfigurable.getDefaultAuditBehavior() : auditBehaviorType; if (auditType == SUMMARY) { @@ -100,6 +100,8 @@ public void addAuditEvent(User user, Container c, TableInfo table, @Nullable Aud AuditTypeEvent event = createSummaryAuditRecord(user, c, auditConfigurable, action, userComment, rows.size(), rows.get(0)); AuditLogService.get().addEvent(user, event); + + return; } case DETAILED: { diff --git a/api/src/org/labkey/api/data/AbstractTableInfo.java b/api/src/org/labkey/api/data/AbstractTableInfo.java index f8d3d95b93e..965d408eefe 100644 --- a/api/src/org/labkey/api/data/AbstractTableInfo.java +++ b/api/src/org/labkey/api/data/AbstractTableInfo.java @@ -276,13 +276,14 @@ public AbstractTableInfo(DbSchema schema, String name) _schema = schema; _columnMap = constructColumnMap(); // This is just an empty map, not populating any columns yet setName(name); - _auditBehaviorType = getDefaultAuditBehaviorType(); + _auditBehaviorType = getDefaultAuditBehavior(); addTriggerFactory(new ScriptTriggerFactory()); MemTracker.getInstance().put(this); } @NotNull - protected AuditBehaviorType getDefaultAuditBehaviorType() + @Override + public AuditBehaviorType getDefaultAuditBehavior() { return AuditBehaviorType.NONE; } @@ -2015,13 +2016,6 @@ public void setAuditBehavior(AuditBehaviorType type) _xmlAuditBehaviorType = type; } - @Override - @NotNull - public AuditBehaviorType getAuditBehavior() - { - return _auditBehaviorType; - } - @Override public AuditBehaviorType getXmlAuditBehaviorType() { diff --git a/api/src/org/labkey/api/data/SchemaTableInfo.java b/api/src/org/labkey/api/data/SchemaTableInfo.java index e316ebe78db..98f835f92c9 100644 --- a/api/src/org/labkey/api/data/SchemaTableInfo.java +++ b/api/src/org/labkey/api/data/SchemaTableInfo.java @@ -533,9 +533,9 @@ public void setAuditBehavior(@NotNull AuditBehaviorType type) _xmlAuditBehaviorType = type; } - @Override - @NotNull - public AuditBehaviorType getAuditBehavior() + @Override + @NotNull + public AuditBehaviorType getDefaultAuditBehavior() { return _auditBehaviorType; } diff --git a/api/src/org/labkey/api/data/TableInfo.java b/api/src/org/labkey/api/data/TableInfo.java index 68d523abcff..1671fc34dc4 100644 --- a/api/src/org/labkey/api/data/TableInfo.java +++ b/api/src/org/labkey/api/data/TableInfo.java @@ -663,7 +663,7 @@ void fireRowTrigger(Container c, User user, TriggerType type, boolean before, in /** see interface AuditConfigurable */ @NotNull - default AuditBehaviorType getAuditBehavior() + default AuditBehaviorType getDefaultAuditBehavior() { return AuditBehaviorType.NONE; } @@ -671,7 +671,7 @@ default AuditBehaviorType getAuditBehavior() @NotNull default AuditBehaviorType getEffectiveAuditBehavior() { - return getXmlAuditBehaviorType() == null ? getAuditBehavior() : getXmlAuditBehaviorType(); + return getXmlAuditBehaviorType() == null ? getDefaultAuditBehavior() : getXmlAuditBehaviorType(); } /** diff --git a/api/src/org/labkey/api/query/FilteredTable.java b/api/src/org/labkey/api/query/FilteredTable.java index 28ffb9ce404..26bdf19b19f 100644 --- a/api/src/org/labkey/api/query/FilteredTable.java +++ b/api/src/org/labkey/api/query/FilteredTable.java @@ -127,7 +127,7 @@ public FilteredTable(@NotNull TableInfo table, @NotNull SchemaType userSchema, @ _buttonBarConfig = BUTTON_BAR_NOT_SET; // lazy copy button bar when asked if (_rootTable.supportsAuditTracking()) { - _auditBehaviorType = _rootTable.getAuditBehavior(); + _auditBehaviorType = _rootTable.getDefaultAuditBehavior(); _xmlAuditBehaviorType = _rootTable.getXmlAuditBehaviorType(); } diff --git a/assay/api-src/org/labkey/api/assay/AssayResultTable.java b/assay/api-src/org/labkey/api/assay/AssayResultTable.java index ebddccc195d..5e4dc295054 100644 --- a/assay/api-src/org/labkey/api/assay/AssayResultTable.java +++ b/assay/api-src/org/labkey/api/assay/AssayResultTable.java @@ -686,7 +686,7 @@ public boolean supportTableRules() @Override @NotNull - protected AuditBehaviorType getDefaultAuditBehaviorType() + public AuditBehaviorType getDefaultAuditBehavior() { return AuditBehaviorType.DETAILED; } diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 3bea86e44ec..6ed1a8eeec0 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -201,7 +201,7 @@ public Domain getDomain(boolean forUpdate) @Override @NotNull - protected AuditBehaviorType getDefaultAuditBehaviorType() + public AuditBehaviorType getDefaultAuditBehavior() { return AuditBehaviorType.DETAILED; } diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index 9368839b2e1..852fcb69578 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -1667,7 +1667,7 @@ public DataIteratorBuilder persistRows(DataIteratorBuilder data, DataIteratorCon @Override @NotNull - protected AuditBehaviorType getDefaultAuditBehaviorType() + public AuditBehaviorType getDefaultAuditBehavior() { return AuditBehaviorType.DETAILED; } From fe39216b1c0d0baebb87484cfa3f9d5e4a9532bd Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 22 Aug 2025 15:07:32 -0700 Subject: [PATCH 07/14] Add selenium tests --- .../api/audit/AbstractAuditHandler.java | 13 ++++++++++++- .../org/labkey/api/audit/AuditHandler.java | 5 +++++ .../api/query/AbstractQueryImportAction.java | 6 ++++-- .../api/query/AbstractQueryUpdateService.java | 9 ++++++--- .../api/query/QueryImportPipelineJob.java | 8 ++++---- .../experiment/api/ExperimentServiceImpl.java | 2 +- .../experiment/api/SampleTypeServiceImpl.java | 19 ++++++++++++++----- 7 files changed, 46 insertions(+), 16 deletions(-) diff --git a/api/src/org/labkey/api/audit/AbstractAuditHandler.java b/api/src/org/labkey/api/audit/AbstractAuditHandler.java index 6579b290501..7bc52938425 100644 --- a/api/src/org/labkey/api/audit/AbstractAuditHandler.java +++ b/api/src/org/labkey/api/audit/AbstractAuditHandler.java @@ -27,7 +27,7 @@ public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryS { AuditConfigurable auditConfigurable = (AuditConfigurable) table; // don't use auditConfigurable.getAuditBehavior(auditBehaviorType) to allow extra summary independent of table config - AuditBehaviorType auditType = auditBehaviorType == null ? auditConfigurable.getDefaultAuditBehavior() : auditBehaviorType; + AuditBehaviorType auditType = auditConfigurable.getEffectiveAuditBehavior(auditBehaviorType); if (auditType == SUMMARY) { @@ -38,6 +38,17 @@ public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryS } } + @Override + public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryService.AuditAction action, Integer dataRowCount, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String userComment, boolean skipAuditLevelCheck) + { + if (table.supportsAuditTracking()) + { + AuditConfigurable auditConfigurable = (AuditConfigurable) table; + AuditTypeEvent event = createSummaryAuditRecord(user, c, auditConfigurable, action, userComment, dataRowCount, null); + AuditLogService.get().addEvent(user, event); + } + } + /** * Create a detailed audit record object so it can be recorded in the audit tables * @param user making change diff --git a/api/src/org/labkey/api/audit/AuditHandler.java b/api/src/org/labkey/api/audit/AuditHandler.java index 6767eccbdcd..857c0c953f9 100644 --- a/api/src/org/labkey/api/audit/AuditHandler.java +++ b/api/src/org/labkey/api/audit/AuditHandler.java @@ -33,6 +33,11 @@ public interface AuditHandler { void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryService.AuditAction action, Integer dataRowCount, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String userComment); + default void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryService.AuditAction action, Integer dataRowCount, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String userComment, boolean skipAuditLevelCheck) + { + addSummaryAuditEvent(user, c, table, action, dataRowCount, auditBehaviorType, userComment); + } + void addAuditEvent(User user, Container c, TableInfo table, @Nullable AuditBehaviorType auditType, @Nullable String userComment, QueryService.AuditAction action, @Nullable List> rows, @Nullable List> existingRows, boolean useTransactionAuditCache); diff --git a/api/src/org/labkey/api/query/AbstractQueryImportAction.java b/api/src/org/labkey/api/query/AbstractQueryImportAction.java index e54fd778555..60ad82ff709 100644 --- a/api/src/org/labkey/api/query/AbstractQueryImportAction.java +++ b/api/src/org/labkey/api/query/AbstractQueryImportAction.java @@ -435,6 +435,8 @@ public final ApiResponse _execute(FORM form, BindException errors) throws Except _useAsync = Boolean.valueOf(getParam(Params.useAsync)); // useAsync will save import file to pipeline root as well as run import in a background job + boolean isCrossTypeImport = getOptionParamsMap().getOrDefault(AbstractQueryImportAction.Params.crossTypeImport, false); + // Check first if the audit behavior has been defined for the table either in code or through XML. // If not defined there, check for the audit behavior defined in the action form (getAuditBehaviorType()). AuditBehaviorType behaviorType = (_target != null) ? _target.getEffectiveAuditBehavior(getAuditBehaviorType()) : getAuditBehaviorType(); @@ -630,8 +632,8 @@ else if (!dataFileDir.exists()) configureLoader(loader); TransactionAuditProvider.TransactionAuditEvent auditEvent = null; - if (behaviorType != null && behaviorType != AuditBehaviorType.NONE) - auditEvent = createTransactionAuditEvent(getContainer(), QueryService.AuditAction.INSERT); + if (isCrossTypeImport || (behaviorType != null && behaviorType != AuditBehaviorType.NONE)) + auditEvent = createTransactionAuditEvent(getContainer(), _insertOption.auditAction); int rowCount = importData(loader, file, originalName, ve, behaviorType, auditEvent, _auditUserComment); diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 72533a62da8..43d5bf1752f 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -393,9 +393,12 @@ protected int _importRowsUsingDIB(User user, Container container, DataIteratorBu return 0; else { - AuditBehaviorType auditType = (AuditBehaviorType) context.getConfigParameter(DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior); - String auditUserComment = (String) context.getConfigParameter(DetailedAuditLogDataIterator.AuditConfigs.AuditUserComment); - getQueryTable().getAuditHandler(auditType).addSummaryAuditEvent(user, container, getQueryTable(), context.getInsertOption().auditAction, count, auditType, auditUserComment); + if (!context.isCrossTypeImport() && !context.isCrossFolderImport()) // audit handled at table level + { + AuditBehaviorType auditType = (AuditBehaviorType) context.getConfigParameter(DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior); + String auditUserComment = (String) context.getConfigParameter(DetailedAuditLogDataIterator.AuditConfigs.AuditUserComment); + getQueryTable().getAuditHandler(auditType).addSummaryAuditEvent(user, container, getQueryTable(), context.getInsertOption().auditAction, count, auditType, auditUserComment); + } return count; } } diff --git a/api/src/org/labkey/api/query/QueryImportPipelineJob.java b/api/src/org/labkey/api/query/QueryImportPipelineJob.java index 59e1c734440..81557cda7be 100644 --- a/api/src/org/labkey/api/query/QueryImportPipelineJob.java +++ b/api/src/org/labkey/api/query/QueryImportPipelineJob.java @@ -296,12 +296,12 @@ public void run() AbstractQueryImportAction.configureLoader(loader, target, _importContextBuilder.getRenamedColumns(), _importContextBuilder.allowLineageColumns(), _importContextBuilder.getLineageImportAliases()); - TransactionAuditProvider.TransactionAuditEvent auditEvent = null; + DataIteratorContext diContext = createDataIteratorContext(ve, getContainer()); - if (_importContextBuilder.getAuditBehaviorType() != null && _importContextBuilder.getAuditBehaviorType() != AuditBehaviorType.NONE) - auditEvent = createTransactionAuditEvent(getContainer(), QueryService.AuditAction.INSERT); + TransactionAuditProvider.TransactionAuditEvent auditEvent = null; + if (diContext.isCrossTypeImport() || (_importContextBuilder.getAuditBehaviorType() != null && _importContextBuilder.getAuditBehaviorType() != AuditBehaviorType.NONE)) + auditEvent = createTransactionAuditEvent(getContainer(), diContext.getInsertOption().auditAction); - DataIteratorContext diContext = createDataIteratorContext(ve, getContainer()); int importedCount = AbstractQueryImportAction.importData(loader, target, updateService, diContext, auditEvent, getInfo().getUser(), getInfo().getContainer()); if (ve.hasErrors()) diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index c046309f26f..580d36f8daa 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9594,7 +9594,7 @@ public Map moveDataClassObjects(Collection d // create summary audit entries for the source container only. The message is pretty generic, so having it // in both source and target doesn't help much. - QueryService.get().getDefaultAuditHandler().addSummaryAuditEvent(user, sourceContainer, dataClassTable, QueryService.AuditAction.UPDATE, updateCount, AuditBehaviorType.SUMMARY, userComment); + QueryService.get().getDefaultAuditHandler().addSummaryAuditEvent(user, sourceContainer, dataClassTable, QueryService.AuditAction.UPDATE, updateCount, AuditBehaviorType.SUMMARY, userComment, true); // create new detailed events for each data object that was moved AuditBehaviorType dcAuditBehavior = dataClassTable.getEffectiveAuditBehavior(auditBehavior); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index dbea9bbb73d..4bca8285eb7 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -1160,13 +1160,13 @@ public String getCommentDetailed(QueryService.AuditAction action, boolean isUpda @Override public DetailedAuditTypeEvent createDetailedAuditRecord(User user, Container c, AuditConfigurable tInfo, QueryService.AuditAction action, @Nullable String userComment, @Nullable Map row, Map existingRow) { - return createAuditRecord(c, getCommentDetailed(action, !existingRow.isEmpty()), userComment, action, row, existingRow); + return createAuditRecord(c, tInfo, getCommentDetailed(action, !existingRow.isEmpty()), userComment, action, row, existingRow); } @Override protected AuditTypeEvent createSummaryAuditRecord(User user, Container c, AuditConfigurable tInfo, QueryService.AuditAction action, @Nullable String userComment, int rowCount, @Nullable Map row) { - return createAuditRecord(c, String.format(action.getCommentSummary(), rowCount), userComment, row); + return createAuditRecord(c, tInfo, String.format(action.getCommentSummary(), rowCount), userComment, row); } @Override @@ -1183,9 +1183,9 @@ protected void addDetailedModifiedFields(Map originalRow, Map row) + private SampleTimelineAuditEvent createAuditRecord(Container c, AuditConfigurable tInfo, String comment, String userComment, @Nullable Map row) { - return createAuditRecord(c, comment, userComment, null, row, null); + return createAuditRecord(c, tInfo, comment, userComment, null, row, null); } private boolean isInputFieldKey(String fieldKey) @@ -1195,7 +1195,7 @@ private boolean isInputFieldKey(String fieldKey) slash==ExpMaterial.MATERIAL_INPUT_PARENT.length() && StringUtils.startsWithIgnoreCase(fieldKey,ExpMaterial.MATERIAL_INPUT_PARENT); } - private SampleTimelineAuditEvent createAuditRecord(Container c, String comment, String userComment, @Nullable QueryService.AuditAction action, @Nullable Map row, @Nullable Map existingRow) + private SampleTimelineAuditEvent createAuditRecord(Container c, AuditConfigurable tInfo, String comment, String userComment, @Nullable QueryService.AuditAction action, @Nullable Map row, @Nullable Map existingRow) { SampleTimelineAuditEvent event = new SampleTimelineAuditEvent(c, comment); event.setUserComment(userComment); @@ -1242,6 +1242,15 @@ else if (event.getSampleLsid() != null) // NOTE: to avoid a diff in the audit log make sure row("rowid") is correct! (not the unused generated value) row.put(ROW_ID,staticsRow.get(ROW_ID)); } + else if (tInfo != null) + { + ExpSampleType sampleType = SampleTypeService.get().getSampleType(c, tInfo.getUserSchema().getUser(), tInfo.getName()); + if (sampleType != null) + { + event.setSampleType(sampleType.getName()); + event.setSampleTypeId(sampleType.getRowId()); + } + } if (action != null) { From 0fe4beaa8e38d658b827ddd75febd2f43269dabe Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 25 Aug 2025 10:57:30 -0700 Subject: [PATCH 08/14] Fix EHR --- api/src/org/labkey/api/audit/AbstractAuditHandler.java | 1 - api/src/org/labkey/api/query/QueryUpdateService.java | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/audit/AbstractAuditHandler.java b/api/src/org/labkey/api/audit/AbstractAuditHandler.java index 7bc52938425..a90f5595120 100644 --- a/api/src/org/labkey/api/audit/AbstractAuditHandler.java +++ b/api/src/org/labkey/api/audit/AbstractAuditHandler.java @@ -26,7 +26,6 @@ public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryS if (table.supportsAuditTracking()) { AuditConfigurable auditConfigurable = (AuditConfigurable) table; - // don't use auditConfigurable.getAuditBehavior(auditBehaviorType) to allow extra summary independent of table config AuditBehaviorType auditType = auditConfigurable.getEffectiveAuditBehavior(auditBehaviorType); if (auditType == SUMMARY) diff --git a/api/src/org/labkey/api/query/QueryUpdateService.java b/api/src/org/labkey/api/query/QueryUpdateService.java index 3bca51ae314..02c63825781 100644 --- a/api/src/org/labkey/api/query/QueryUpdateService.java +++ b/api/src/org/labkey/api/query/QueryUpdateService.java @@ -113,7 +113,7 @@ enum ConfigParameters PreferPKOverObjectUriAsKey, // (Bool) Prefer getPkColumnNames instead of getObjectURIColumnName to use as keys SkipReselectRows, // (Bool) If true, skip qus.getRows and use raw returned rows. Applicable for CommandType.insert/insertWithKeys/update/updateChangingKeys TargetContainer, - ByPassAudit // (Bool) If true, skip DetailedAuditLogDataIterator + ByPassAudit // (Bool) If true, skip DetailedAuditLogDataIterator. For internal use only, don't expose for API. } From c5c17238424a990a44aac07f66717346dde852d6 Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 25 Aug 2025 14:08:16 -0700 Subject: [PATCH 09/14] Fix lineage sync --- .../api/assay/AssayResultUpdateService.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/assay/api-src/org/labkey/api/assay/AssayResultUpdateService.java b/assay/api-src/org/labkey/api/assay/AssayResultUpdateService.java index 635f21fc3b8..da7a104f505 100644 --- a/assay/api-src/org/labkey/api/assay/AssayResultUpdateService.java +++ b/assay/api-src/org/labkey/api/assay/AssayResultUpdateService.java @@ -66,6 +66,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import static org.labkey.api.dataiterator.DetailedAuditLogDataIterator.AuditConfigs.AuditUserComment; @@ -286,6 +287,23 @@ protected Map updateRow( convertTypes(user, container, row, getDbTable(), assayResultsRunDir); Map result = super.updateRow(user, container, row, oldRow, configParameters); + Map updatedValues = getRow(user, container, oldRow); + + TableInfo table = getQueryTable(); + for (Map.Entry entry : result.entrySet()) + { + ColumnInfo col = table.getColumn(entry.getKey()); + + if (col != null) + { + Object oldValue = col.getValue(originalRow); + Object newValue = col.getValue(updatedValues); + boolean hasValueChanged = !Objects.equals(oldValue, newValue); + + if (hasValueChanged) + _assaySampleLookupContext.trackSampleLookupChange(container, user, table, col, run); + } + } incrementAuditRowCount(run); From f8fe2e0eca2e9626ea31bf80ec5631cff88ae2e2 Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 25 Aug 2025 14:29:11 -0700 Subject: [PATCH 10/14] Update jest integration tests --- .../test/integration/MoveSamplesAction.ispec.ts | 14 +++++++++----- .../test/integration/MoveSourcesAction.ispec.ts | 13 ++++++++----- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/experiment/src/client/test/integration/MoveSamplesAction.ispec.ts b/experiment/src/client/test/integration/MoveSamplesAction.ispec.ts index 7a2ee57bc98..4b35a760cd3 100644 --- a/experiment/src/client/test/integration/MoveSamplesAction.ispec.ts +++ b/experiment/src/client/test/integration/MoveSamplesAction.ispec.ts @@ -478,13 +478,14 @@ describe('Move Samples', () => { schemaName: 'samples', queryName: SAMPLE_TYPE_NAME_1, rows: [{ rowId: sampleRowId }], + auditBehavior: "NONE", // sample default audit level is DETAILS, api override have no effect }, {...topFolderOptions, ...editorUserOptions}).expect(200); // Assert const {updateCounts} = response.body; expect(updateCounts.samples).toBe(1); expect(updateCounts.sampleAliases).toBe(0); - expect(updateCounts.sampleAuditEvents).toBe(0); + expect(updateCounts.sampleAuditEvents).toBe(1); const sampleExistsInTop = await sampleExists(server, sampleRowId, topFolderOptions, editorUserOptions, SAMPLE_TYPE_NAME_1); expect(sampleExistsInTop).toBe(false); @@ -496,7 +497,9 @@ describe('Move Samples', () => { const sampleEventsInTop = await getSampleTimelineAuditLogs(sampleRowId, topFolderOptions); expect(sampleEventsInTop).toHaveLength(0); const sampleEventsInSub1 = await getSampleTimelineAuditLogs(sampleRowId, subfolder1Options); - expect(sampleEventsInSub1).toHaveLength(0); + expect(sampleEventsInSub1).toHaveLength(2); + expect(caseInsensitive(sampleEventsInSub1[0], 'Comment')).toEqual("Sample folder was updated."); + expect(caseInsensitive(sampleEventsInSub1[1], 'Comment')).toEqual("Sample was registered."); }); it('success, move sample from parent project to subfolder, detailed audit logging', async () => { @@ -567,7 +570,7 @@ describe('Move Samples', () => { schemaName: 'samples', queryName: SAMPLE_TYPE_NAME_1, rows: [{ rowId: sampleRowId }], - auditBehavior: 'SUMMARY', + auditBehavior: 'SUMMARY', // sample default audit level is DETAILS, api override of 'SUMMARY' have no effect auditUserComment: userComment, }, {...topFolderOptions, ...editorUserOptions}).expect(200); @@ -587,8 +590,9 @@ describe('Move Samples', () => { const sampleEventsInTop = await getSampleTimelineAuditLogs(sampleRowId, topFolderOptions); expect(sampleEventsInTop).toHaveLength(0); const sampleEventsInSub1 = await getSampleTimelineAuditLogs(sampleRowId, subfolder1Options); - expect(sampleEventsInSub1).toHaveLength(1); - expect(caseInsensitive(sampleEventsInSub1[0], 'Comment')).toEqual("Sample was registered."); + expect(sampleEventsInSub1).toHaveLength(2); + expect(caseInsensitive(sampleEventsInSub1[0], 'Comment')).toEqual("Sample folder was updated."); + expect(caseInsensitive(sampleEventsInSub1[1], 'Comment')).toEqual("Sample was registered."); }); it('success, move sample from subfolder to parent project', async () => { diff --git a/experiment/src/client/test/integration/MoveSourcesAction.ispec.ts b/experiment/src/client/test/integration/MoveSourcesAction.ispec.ts index 356aeb8f4c9..300b89f33dc 100644 --- a/experiment/src/client/test/integration/MoveSourcesAction.ispec.ts +++ b/experiment/src/client/test/integration/MoveSourcesAction.ispec.ts @@ -469,7 +469,7 @@ describe('Move Sources', () => { expect(success).toBe(true); expect(updateCounts.sources).toBe(1); expect(updateCounts.sourceAliases).toBe(0); - expect(updateCounts.sourceAuditEvents).toBe(0); + expect(updateCounts.sourceAuditEvents).toBe(1); const existsInTop = await _sourceExists(sourceRowId, topFolderOptions); expect(existsInTop).toBe(false); @@ -481,7 +481,9 @@ describe('Move Sources', () => { const eventsInTop = await getDetailedQueryUpdateAuditLogs(sourceRowId, topFolderOptions); expect(eventsInTop).toHaveLength(0); const eventsInSub1 = await getDetailedQueryUpdateAuditLogs(sourceRowId, subfolder1Options); - expect(eventsInSub1).toHaveLength(0); + expect(eventsInSub1).toHaveLength(2); + expect(caseInsensitive(eventsInSub1[0], 'Comment')).toEqual("A row was updated."); + expect(caseInsensitive(eventsInSub1[1], 'Comment')).toEqual("A row was inserted."); }); it('success, move from parent project to subfolder, detailed audit logging', async () => { @@ -554,7 +556,7 @@ describe('Move Sources', () => { schemaName: 'exp.data', queryName: SOURCE_TYPE_NAME_1, rows: [{ RowId: sourceRowId }], - auditBehavior: 'SUMMARY', + auditBehavior: 'SUMMARY', // sample default audit level is DETAILS, api override have no effect auditUserComment: userComment, }, { ...topFolderOptions, ...editorUserOptions }).expect(200); @@ -575,8 +577,9 @@ describe('Move Sources', () => { const eventsInTop = await getDetailedQueryUpdateAuditLogs(sourceRowId, topFolderOptions); expect(eventsInTop).toHaveLength(0); const eventsInSub1 = await getDetailedQueryUpdateAuditLogs(sourceRowId, subfolder1Options); - expect(eventsInSub1).toHaveLength(1); - expect(caseInsensitive(eventsInSub1[0], 'Comment')).toEqual("A row was inserted."); + expect(eventsInSub1).toHaveLength(2); + expect(caseInsensitive(eventsInSub1[0], 'Comment')).toEqual("A row was updated."); + expect(caseInsensitive(eventsInSub1[1], 'Comment')).toEqual("A row was inserted."); }); it('success, move from subfolder to parent project', async () => { From d459f55fef7d74ec3b984d1734666ec7344a3ba8 Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 25 Aug 2025 20:07:06 -0700 Subject: [PATCH 11/14] add selenium test for ETL bulk load --- .../api/audit/AbstractAuditHandler.java | 19 +++++++------------ .../api/query/AbstractQueryUpdateService.java | 8 +++++++- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/api/src/org/labkey/api/audit/AbstractAuditHandler.java b/api/src/org/labkey/api/audit/AbstractAuditHandler.java index a90f5595120..d0d89d05b1f 100644 --- a/api/src/org/labkey/api/audit/AbstractAuditHandler.java +++ b/api/src/org/labkey/api/audit/AbstractAuditHandler.java @@ -22,13 +22,19 @@ public abstract class AbstractAuditHandler implements AuditHandler @Override public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryService.AuditAction action, Integer dataRowCount, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String userComment) + { + addSummaryAuditEvent(user, c, table, action, dataRowCount, auditBehaviorType, userComment, false); + } + + @Override + public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryService.AuditAction action, Integer dataRowCount, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String userComment, boolean skipAuditLevelCheck) { if (table.supportsAuditTracking()) { AuditConfigurable auditConfigurable = (AuditConfigurable) table; AuditBehaviorType auditType = auditConfigurable.getEffectiveAuditBehavior(auditBehaviorType); - if (auditType == SUMMARY) + if (auditType == SUMMARY || skipAuditLevelCheck) { AuditTypeEvent event = createSummaryAuditRecord(user, c, auditConfigurable, action, userComment, dataRowCount, null); @@ -37,17 +43,6 @@ public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryS } } - @Override - public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryService.AuditAction action, Integer dataRowCount, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String userComment, boolean skipAuditLevelCheck) - { - if (table.supportsAuditTracking()) - { - AuditConfigurable auditConfigurable = (AuditConfigurable) table; - AuditTypeEvent event = createSummaryAuditRecord(user, c, auditConfigurable, action, userComment, dataRowCount, null); - AuditLogService.get().addEvent(user, event); - } - } - /** * Create a detailed audit record object so it can be recorded in the audit tables * @param user making change diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 43d5bf1752f..7cd62f644b0 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -397,7 +397,13 @@ protected int _importRowsUsingDIB(User user, Container container, DataIteratorBu { AuditBehaviorType auditType = (AuditBehaviorType) context.getConfigParameter(DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior); String auditUserComment = (String) context.getConfigParameter(DetailedAuditLogDataIterator.AuditConfigs.AuditUserComment); - getQueryTable().getAuditHandler(auditType).addSummaryAuditEvent(user, container, getQueryTable(), context.getInsertOption().auditAction, count, auditType, auditUserComment); + boolean skipAuditLevelCheck = false; + if (context.getConfigParameterBoolean(QueryUpdateService.ConfigParameters.BulkLoad)) + { + if (getQueryTable().getEffectiveAuditBehavior(auditType) == AuditBehaviorType.DETAILED) // allow ETL to demote audit level for bulkLoad + skipAuditLevelCheck = true; + } + getQueryTable().getAuditHandler(auditType).addSummaryAuditEvent(user, container, getQueryTable(), context.getInsertOption().auditAction, count, auditType, auditUserComment, skipAuditLevelCheck); } return count; } From a0400eebe1015c735f4c8a171d1ac7c2e4ac8c34 Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 26 Aug 2025 12:45:33 -0700 Subject: [PATCH 12/14] code review changes --- api/src/org/labkey/api/data/AbstractTableInfo.java | 6 ++---- api/src/org/labkey/api/data/SchemaTableInfo.java | 6 +++++- .../labkey/api/assay/AssayResultUpdateService.java | 5 +++-- .../experiment/api/SampleTypeServiceImpl.java | 13 +++++++++---- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/api/src/org/labkey/api/data/AbstractTableInfo.java b/api/src/org/labkey/api/data/AbstractTableInfo.java index 965d408eefe..1335a44c86e 100644 --- a/api/src/org/labkey/api/data/AbstractTableInfo.java +++ b/api/src/org/labkey/api/data/AbstractTableInfo.java @@ -276,7 +276,6 @@ public AbstractTableInfo(DbSchema schema, String name) _schema = schema; _columnMap = constructColumnMap(); // This is just an empty map, not populating any columns yet setName(name); - _auditBehaviorType = getDefaultAuditBehavior(); addTriggerFactory(new ScriptTriggerFactory()); MemTracker.getInstance().put(this); } @@ -285,7 +284,7 @@ public AbstractTableInfo(DbSchema schema, String name) @Override public AuditBehaviorType getDefaultAuditBehavior() { - return AuditBehaviorType.NONE; + return _auditBehaviorType; } public void afterConstruct() @@ -1473,7 +1472,6 @@ else if (xmlColumn.isSetWrappedColumnName() && isNotBlank(xmlColumn.getWrappedCo catch (QueryParseException qpe) { warnings.add(qpe.setFieldName(xmlColumn.getColumnName())); - continue; } } @@ -1501,7 +1499,7 @@ else if (!calculatedFieldKeys.isEmpty()) } catch (IllegalArgumentException ignore) { - + warnings.add(new QueryParseWarning("Invalid AuditLogging: " + auditBehavior,null,0,0)); } } diff --git a/api/src/org/labkey/api/data/SchemaTableInfo.java b/api/src/org/labkey/api/data/SchemaTableInfo.java index 98f835f92c9..d4863150d8d 100644 --- a/api/src/org/labkey/api/data/SchemaTableInfo.java +++ b/api/src/org/labkey/api/data/SchemaTableInfo.java @@ -16,6 +16,7 @@ package org.labkey.api.data; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.cache.CacheManager; @@ -51,6 +52,7 @@ import org.labkey.api.util.StringExpression; import org.labkey.api.util.StringExpressionFactory; import org.labkey.api.util.URLHelper; +import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.ActionURL; import org.labkey.api.view.ViewContext; import org.labkey.data.xml.AuditType; @@ -112,6 +114,8 @@ public class SchemaTableInfo implements TableInfo, UpdateableTableInfo, AuditCon protected boolean _autoLoadMetaData = true; // TODO: Remove this? DatasetSchemaTableInfo is the only user of this. + private static final Logger LOG = LogHelper.getLogger(SchemaTableInfo.class, "TableInfo construction"); + public SchemaTableInfo(DbSchema parentSchema, DatabaseTableType tableType, String tableName, String metaDataName, SQLFragment selectName) { this(parentSchema, tableType, tableName, metaDataName, selectName, null); @@ -637,7 +641,7 @@ public void loadTablePropertiesFromXml(TableType xmlTable, boolean dontSetName) } catch (IllegalArgumentException ignore) { - + LOG.warn("Invalid AuditLogging: " + auditBehavior); } } diff --git a/assay/api-src/org/labkey/api/assay/AssayResultUpdateService.java b/assay/api-src/org/labkey/api/assay/AssayResultUpdateService.java index da7a104f505..379feab34f1 100644 --- a/assay/api-src/org/labkey/api/assay/AssayResultUpdateService.java +++ b/assay/api-src/org/labkey/api/assay/AssayResultUpdateService.java @@ -89,9 +89,10 @@ public AssayResultUpdateService(AssayProtocolSchema schema, FilteredTable table) private void addRunAuditSummary(User user, @Nullable Map configParameters, String verb) { + String userComment = configParameters == null ? null : (String) configParameters.get(AuditUserComment); + for (Long runId: dataChangeCount.keySet()) { - String userComment = configParameters == null ? null : (String) configParameters.get(AuditUserComment); var run = ExperimentService.get().getExpRun(runId); int deletedCount = dataChangeCount.get(runId); @@ -119,7 +120,7 @@ public List> updateRows( Map extraScriptContext ) throws InvalidKeyException, BatchValidationException, QueryUpdateServiceException, SQLException { - dataChangeCount = new HashMap<>(); + dataChangeCount = new LinkedHashMap<>(); // handle transform scripts rows = transform(container, user, rows, oldKeys); var result = super.updateRows(user, container, rows, oldKeys, errors, configParameters, extraScriptContext); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index 053a2149a2b..0d874408023 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -103,6 +103,7 @@ import org.labkey.api.query.QueryService; import org.labkey.api.query.SchemaKey; import org.labkey.api.query.SimpleValidationError; +import org.labkey.api.query.UserSchema; import org.labkey.api.query.ValidationException; import org.labkey.api.search.SearchService; import org.labkey.api.security.User; @@ -1245,11 +1246,15 @@ else if (event.getSampleLsid() != null) } else if (tInfo != null) { - ExpSampleType sampleType = SampleTypeService.get().getSampleType(c, tInfo.getUserSchema().getUser(), tInfo.getName()); - if (sampleType != null) + UserSchema schema = tInfo.getUserSchema(); + if (schema != null) { - event.setSampleType(sampleType.getName()); - event.setSampleTypeId(sampleType.getRowId()); + ExpSampleType sampleType = getSampleType(c, schema.getUser(), tInfo.getName()); + if (sampleType != null) + { + event.setSampleType(sampleType.getName()); + event.setSampleTypeId(sampleType.getRowId()); + } } } From 6c64e78c471b7b51a3d0bf4c9c92bd1cbe9a1fbb Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 27 Aug 2025 15:54:44 -0700 Subject: [PATCH 13/14] Add transaction audit event from update/insert done from LKS --- .../labkey/api/query/UserSchemaAction.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/api/src/org/labkey/api/query/UserSchemaAction.java b/api/src/org/labkey/api/query/UserSchemaAction.java index 9bde5af49d1..28c6f462382 100644 --- a/api/src/org/labkey/api/query/UserSchemaAction.java +++ b/api/src/org/labkey/api/query/UserSchemaAction.java @@ -20,6 +20,7 @@ import org.labkey.api.action.NullSafeBindException; import org.labkey.api.action.SpringActionController; import org.labkey.api.attachments.SpringAttachmentFile; +import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveMapWrapper; import org.labkey.api.data.ActionButton; @@ -29,6 +30,7 @@ import org.labkey.api.data.DbScope; import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.TableInfo; +import org.labkey.api.gwt.client.AuditBehaviorType; import org.labkey.api.security.permissions.InsertPermission; import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.util.ButtonBuilder; @@ -249,7 +251,24 @@ else if (table.getPkColumnNames().size() > 1) { try (DbScope.Transaction transaction = dbschema.getScope().ensureTransaction()) { + TransactionAuditProvider.TransactionAuditEvent auditEvent = null; + QueryService.AuditAction auditAction = insert ? QueryService.AuditAction.INSERT : QueryService.AuditAction.UPDATE; + + // transaction audit BatchValidationException batchErrors = new BatchValidationException(); + + AuditBehaviorType auditBehaviorType = table.getEffectiveAuditBehavior(); + if (auditBehaviorType != AuditBehaviorType.NONE) + { + if (transaction.getAuditEvent() != null) + auditEvent = transaction.getAuditEvent(); + else + { + auditEvent = AbstractQueryUpdateService.createTransactionAuditEvent(getContainer(), auditAction); + AbstractQueryUpdateService.addTransactionAuditEvent(transaction, getUser(), auditEvent); + } + } + if (insert) { ret = qus.insertRows(form.getUser(), form.getContainer(), rows, batchErrors, null, null); @@ -280,7 +299,11 @@ else if (table.getPkColumnNames().size() > 1) batchErrors.addToErrors(errors); if (!errors.hasErrors()) + { + auditEvent.addComment(auditAction, ret.size()); transaction.commit(); // Only commit if there were no errors + } + } catch (SQLException x) { From b4f1bf67d1c2720fd1d7e9f6533a51d57c9e6b50 Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 28 Aug 2025 13:06:07 -0700 Subject: [PATCH 14/14] Fix NPE --- api/src/org/labkey/api/query/UserSchemaAction.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/query/UserSchemaAction.java b/api/src/org/labkey/api/query/UserSchemaAction.java index 28c6f462382..12f94df02c1 100644 --- a/api/src/org/labkey/api/query/UserSchemaAction.java +++ b/api/src/org/labkey/api/query/UserSchemaAction.java @@ -300,7 +300,8 @@ else if (table.getPkColumnNames().size() > 1) if (!errors.hasErrors()) { - auditEvent.addComment(auditAction, ret.size()); + if (auditEvent != null) + auditEvent.addComment(auditAction, ret.size()); transaction.commit(); // Only commit if there were no errors }