From 117b7b4ca92f16837422d6b621fad010ee463550 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Wed, 17 Sep 2025 12:52:29 -0700 Subject: [PATCH 01/11] Plate Events, Plate Data Events --- .../assay/AbstractAssayTsvDataHandler.java | 11 +- .../api/audit/AbstractAuditTypeProvider.java | 29 +-- .../org/labkey/api/audit/AuditTypeEvent.java | 12 +- .../org/labkey/api/data/triggers/Trigger.java | 3 - assay/src/org/labkey/assay/AssayModule.java | 3 + .../src/org/labkey/assay/PlateController.java | 6 +- .../data/generator/PlateSetDataGenerator.java | 2 +- .../plate/AssayPlateMetadataServiceImpl.java | 2 +- .../src/org/labkey/assay/plate/PlateImpl.java | 32 ++- .../org/labkey/assay/plate/PlateManager.java | 213 +++++++++++++----- .../labkey/assay/plate/PlateManagerTest.java | 6 +- .../assay/plate/audit/PlateAuditEvent.java | 142 ++++++++++++ .../assay/plate/audit/PlateAuditProvider.java | 204 +++++++++++++++++ .../assay/plate/data/PlateMapExcelWriter.java | 2 +- .../assay/plate/data/PlateTriggerFactory.java | 78 +++++++ .../org/labkey/assay/plate/data/WellData.java | 2 +- .../assay/plate/data/WellTriggerFactory.java | 6 +- .../labkey/assay/plate/model/PlateBean.java | 27 ++- .../labkey/assay/plate/query/PlateTable.java | 40 +++- .../labkey/assay/plate/query/WellTable.java | 61 ++++- 20 files changed, 754 insertions(+), 127 deletions(-) create mode 100644 assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java create mode 100644 assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java create mode 100644 assay/src/org/labkey/assay/plate/data/PlateTriggerFactory.java diff --git a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java index 072e536c44e..a12dfcda858 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java +++ b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java @@ -598,14 +598,11 @@ protected void insertRowData( OntologyManager.UpdateableTableImportHelper importHelper = new SimpleAssayDataImportHelper(data, protocol, provider); if (provider.isPlateMetadataEnabled(protocol)) { - if (context.getReRunId() != null) + // check if we are merging the re-imported data + if (context != null && context.getReRunId() != null && context.getReImportOption() == MERGE_DATA) { - // check if we are merging the re-imported data - if (context.getReImportOption() == MERGE_DATA) - { - DataIteratorBuilder mergedData = AssayPlateMetadataService.get().mergeReRunData(container, user, context, fileData, provider, protocol, data); - fileData = DataIteratorUtil.wrapMap(mergedData.getDataIterator(new DataIteratorContext()), false); - } + DataIteratorBuilder mergedData = AssayPlateMetadataService.get().mergeReRunData(container, user, context, fileData, provider, protocol, data); + fileData = DataIteratorUtil.wrapMap(mergedData.getDataIterator(new DataIteratorContext()), false); } importHelper = AssayPlateMetadataService.get().getImportHelper(container, user, run, data, protocol, provider, context); diff --git a/api/src/org/labkey/api/audit/AbstractAuditTypeProvider.java b/api/src/org/labkey/api/audit/AbstractAuditTypeProvider.java index e5fc84d182f..c9a804f1c33 100644 --- a/api/src/org/labkey/api/audit/AbstractAuditTypeProvider.java +++ b/api/src/org/labkey/api/audit/AbstractAuditTypeProvider.java @@ -222,20 +222,11 @@ private void ensureProperties(User user, Domain domain) } } - // #26311 We want to trigger a save if the scale has changed + // Issue 26311: We want to trigger a save if the scale has changed // CONSIDER: check for other differences here as well. private boolean differ(PropertyDescriptor pd, DomainProperty dp, Container c) { - return dp.getScale() != pd.getScale() - || !dp.getRangeURI().equals(pd.getRangeURI()) -// || !dp.getLabel().equals(pd.getLabel()) -// || dp.isRequired() != pd.isRequired() -// || dp.isHidden() != pd.isHidden() -// || dp.isMvEnabled() != pd.isMvEnabled() -// || dp.getDefaultValueTypeEnum() != pd.getDefaultValueTypeEnum() - ; - - + return dp.getScale() != pd.getScale() || !dp.getRangeURI().equals(pd.getRangeURI()); } private void copyTo(DomainProperty dp, PropertyDescriptor pd, Container c) @@ -321,7 +312,7 @@ protected void appendValueMapColumns(AbstractTableInfo table, String eventName, MutableColumnInfo oldCol = table.getMutableColumn(FieldKey.fromString(OLD_RECORD_PROP_NAME)); MutableColumnInfo newCol = table.getMutableColumn(FieldKey.fromString(NEW_RECORD_PROP_NAME)); - if(oldCol != null) + if (oldCol != null) { var added = table.addColumn(new AliasedColumn(table, "OldValues", oldCol)); added.setDisplayColumnFactory(DataMapColumn::new); @@ -330,7 +321,7 @@ protected void appendValueMapColumns(AbstractTableInfo table, String eventName, oldCol.setHidden(true); } - if(newCol != null) + if (newCol != null) { var added = table.addColumn(new AliasedColumn(table, "NewValues", newCol)); added.setDisplayColumnFactory(DataMapColumn::new); @@ -390,16 +381,16 @@ public static String encodeForDataMap(Map properties) entry.getKey().equals(ExperimentService.ALIASCOLUMNALIAS)) continue; Object value = entry.getValue(); - if (value instanceof Time) + if (value instanceof Time time) { - String formatted = DateUtil.formatIsoLongTime((Time)value); + String formatted = DateUtil.formatIsoLongTime(time); stringMap.put(entry.getKey(), formatted); } - else if (value instanceof Date) + else if (value instanceof Date date) { - // issue: 35002 - normalize Date values to avoid Timestamp/Date toString differences - // issue: 36472 - use iso format to show date-time values - String formatted = DateUtil.toISO((Date)value); + // Issue 35002 - normalize Date values to avoid Timestamp/Date toString differences + // Issue 36472 - use iso format to show date-time values + String formatted = DateUtil.toISO(date); stringMap.put(entry.getKey(), formatted); } else diff --git a/api/src/org/labkey/api/audit/AuditTypeEvent.java b/api/src/org/labkey/api/audit/AuditTypeEvent.java index 68f5b313908..af57a884112 100644 --- a/api/src/org/labkey/api/audit/AuditTypeEvent.java +++ b/api/src/org/labkey/api/audit/AuditTypeEvent.java @@ -30,8 +30,6 @@ /** * Bean object to capture audit log entries. Will be used to populate the database tables via get/set methods that * align with column names in the corresponding provisioned table. - * User: klum - * Date: 7/12/13 */ public class AuditTypeEvent { @@ -53,7 +51,7 @@ public class AuditTypeEvent private User _createdBy; private Date _modified; private User _modifiedBy; - private String userComment; + private String _userComment; private Long _transactionId; public AuditTypeEvent(@NotNull String eventType, @NotNull Container container, @Nullable String comment) @@ -69,7 +67,9 @@ public AuditTypeEvent(@NotNull String eventType, @NotNull Container container, @ } /** Important for reflection-based instantiation */ - public AuditTypeEvent(){} + public AuditTypeEvent() + { + } public long getRowId() { @@ -173,12 +173,12 @@ public void setModifiedBy(User modifiedBy) public void setUserComment(String userComment) { - this.userComment = userComment; + _userComment = userComment; } public String getUserComment() { - return this.userComment; + return _userComment; } public Long getTransactionId() diff --git a/api/src/org/labkey/api/data/triggers/Trigger.java b/api/src/org/labkey/api/data/triggers/Trigger.java index d72b4bfdef3..238d1332b23 100644 --- a/api/src/org/labkey/api/data/triggers/Trigger.java +++ b/api/src/org/labkey/api/data/triggers/Trigger.java @@ -32,9 +32,6 @@ import java.util.stream.Collectors; /** - * User: kevink - * Date: 12/21/15 - * * Trigger scripts are invoked before insert/update/delete on many LabKey tables. * The Trigger is created by a TriggerFactory added to AbstractTableInfo. */ diff --git a/assay/src/org/labkey/assay/AssayModule.java b/assay/src/org/labkey/assay/AssayModule.java index 1900856f75c..73486e937f4 100644 --- a/assay/src/org/labkey/assay/AssayModule.java +++ b/assay/src/org/labkey/assay/AssayModule.java @@ -34,6 +34,7 @@ import org.labkey.api.assay.plate.PlateService; import org.labkey.api.assay.plate.PlateUtils; import org.labkey.api.assay.plate.PositionImpl; +import org.labkey.api.audit.AuditLogService; import org.labkey.api.cache.CacheManager; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; @@ -87,6 +88,7 @@ import org.labkey.assay.plate.PlateReplicateStatsDomainKind; import org.labkey.assay.plate.PlateSetDocumentProvider; import org.labkey.assay.plate.TsvPlateLayoutHandler; +import org.labkey.assay.plate.audit.PlateAuditProvider; import org.labkey.assay.plate.query.PlateSchema; import org.labkey.assay.plate.query.PlateSchemaTest; import org.labkey.assay.plate.query.PlateTypeTable; @@ -203,6 +205,7 @@ protected void startupAfterSpringConfig(ModuleContext moduleContext) return result; }); PlateManager.get().registerLsidHandlers(); + AuditLogService.get().registerAuditType(new PlateAuditProvider()); SearchService ss = SearchService.get(); // ASSAY_CATEGORY diff --git a/assay/src/org/labkey/assay/PlateController.java b/assay/src/org/labkey/assay/PlateController.java index aa71855a304..de4374282d8 100644 --- a/assay/src/org/labkey/assay/PlateController.java +++ b/assay/src/org/labkey/assay/PlateController.java @@ -278,9 +278,9 @@ public void validateCommand(NameForm target, Errors errors) @Override public boolean handlePost(NameForm form, BindException errors) throws Exception { - Plate template = PlateService.get().getPlate(getContainer(), form.getPlateId()); - if (template != null && template.getRowId() != null) - PlateService.get().deletePlate(getContainer(), getUser(), template.getRowId()); + Plate plate = PlateService.get().getPlate(getContainer(), form.getPlateId()); + if (plate != null && plate.getRowId() != null) + PlateService.get().deletePlate(getContainer(), getUser(), plate.getRowId()); return true; } diff --git a/assay/src/org/labkey/assay/data/generator/PlateSetDataGenerator.java b/assay/src/org/labkey/assay/data/generator/PlateSetDataGenerator.java index 2a8bd136a7e..b5439d8c12e 100644 --- a/assay/src/org/labkey/assay/data/generator/PlateSetDataGenerator.java +++ b/assay/src/org/labkey/assay/data/generator/PlateSetDataGenerator.java @@ -267,7 +267,7 @@ private PlateSet createPlateSet(PlateSetType plateSetType, @Nullable PlateSet pa rowMap.put(WellTable.WELL_LOCATION, position.getDescription()); rowMap.put(WellTable.Column.Type.name(), WellGroup.Type.SAMPLE.name()); - rowMap.put(WellTable.Column.SampleID.name(), ids.get(sampleIdx++)); + rowMap.put(WellTable.Column.SampleId.name(), ids.get(sampleIdx++)); for (String name : wellProperties) { diff --git a/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java b/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java index 02505afad06..f0edf931167 100644 --- a/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java +++ b/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java @@ -1507,7 +1507,7 @@ public String format(FieldKey fieldKey) } @Override - public UserSchema getPlateSchema(QuerySchema querySchema, Set contextualRoles) + public @NotNull UserSchema getPlateSchema(QuerySchema querySchema, Set contextualRoles) { return new PlateSchema(querySchema, contextualRoles); } diff --git a/assay/src/org/labkey/assay/plate/PlateImpl.java b/assay/src/org/labkey/assay/plate/PlateImpl.java index b0071a0c912..1d45c39ae6e 100644 --- a/assay/src/org/labkey/assay/plate/PlateImpl.java +++ b/assay/src/org/labkey/assay/plate/PlateImpl.java @@ -92,6 +92,7 @@ public class PlateImpl extends PropertySetImpl implements Plate, Cloneable private WellImpl[][] _wells = null; private Map _wellMap; private Integer _metadataDomainId; + private transient Long _sourcePlateRowId; // no-param constructor for reflection public PlateImpl() @@ -162,7 +163,8 @@ public static PlateImpl from(PlateBean bean) // entity fields Container container = ContainerManager.getForId(bean.getContainerId()); - plate.setContainer(container); + if (container != null) + plate.setContainer(container); plate.setCreated(bean.getCreated()); plate.setCreatedBy(bean.getCreatedBy()); plate.setModified(bean.getModified()); @@ -436,10 +438,16 @@ public void setCreated(Date created) @JsonProperty("createdBy") public JSONObject getCreatedBy() { - User user = UserManager.getUser(_createdBy); + User user = getCreatedByUser(); return user != null ? user.getUserProps() : null; } + @JsonIgnore + public User getCreatedByUser() + { + return UserManager.getUser(_createdBy); + } + public void setCreatedBy(int createdBy) { _createdBy = createdBy; @@ -458,10 +466,16 @@ public void setModified(Date modified) @JsonProperty("modifiedBy") public JSONObject getModifiedBy() { - User user = UserManager.getUser(_modifiedBy); + User user = getModifiedByUser(); return user != null ? user.getUserProps() : null; } + @JsonIgnore + public User getModifiedByUser() + { + return UserManager.getUser(_modifiedBy); + } + public void setModifiedBy(int modifiedBy) { _modifiedBy = modifiedBy; @@ -762,6 +776,18 @@ public boolean isNew() return _rowId == null || _rowId <= 0; } + @JsonIgnore + public Long getSourcePlateRowId() + { + return _sourcePlateRowId; + } + + @JsonIgnore + public void setSourcePlateRowId(Long sourcePlateRowId) + { + _sourcePlateRowId = sourcePlateRowId; + } + public static class TestCase extends Assert { private PlateSetImpl _plateSet; diff --git a/assay/src/org/labkey/assay/plate/PlateManager.java b/assay/src/org/labkey/assay/plate/PlateManager.java index 0ec539f3d0d..f03915c7bca 100644 --- a/assay/src/org/labkey/assay/plate/PlateManager.java +++ b/assay/src/org/labkey/assay/plate/PlateManager.java @@ -45,6 +45,7 @@ import org.labkey.api.assay.plate.Well; import org.labkey.api.assay.plate.WellCustomField; import org.labkey.api.assay.plate.WellGroup; +import org.labkey.api.audit.AuditLogService; import org.labkey.api.collections.ArrayListMap; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; @@ -100,9 +101,11 @@ import org.labkey.api.exp.property.DomainProperty; import org.labkey.api.exp.property.DomainUtil; import org.labkey.api.exp.property.PropertyService; +import org.labkey.api.gwt.client.AuditBehaviorType; import org.labkey.api.gwt.client.model.GWTDomain; import org.labkey.api.gwt.client.model.GWTPropertyDescriptor; import org.labkey.api.qc.DataState; +import org.labkey.api.query.AbstractQueryUpdateService; import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.FieldKey; import org.labkey.api.query.QueryService; @@ -132,6 +135,8 @@ import org.labkey.assay.AssayManager; import org.labkey.assay.PlateController; import org.labkey.assay.TsvAssayProvider; +import org.labkey.assay.plate.audit.PlateAuditEvent; +import org.labkey.assay.plate.audit.PlateAuditProvider; import org.labkey.assay.plate.data.PlateMapExcelWriter; import org.labkey.assay.plate.data.WellData; import org.labkey.assay.plate.layout.LayoutEngine; @@ -181,6 +186,7 @@ import static java.util.Collections.unmodifiableList; import static org.labkey.api.assay.plate.PlateSet.MAX_PLATES; import static org.labkey.api.assay.plate.WellGroup.Type.SAMPLE; +import static org.labkey.api.dataiterator.DetailedAuditLogDataIterator.AuditConfigs.AuditBehavior; import static org.labkey.api.util.IntegerUtils.asInteger; import static org.labkey.api.util.IntegerUtils.asLong; import static org.labkey.assay.plate.query.WellTable.WELL_LOCATION; @@ -302,6 +308,18 @@ public List getWellGroupTypes() @Nullable Long plateSetId, @Nullable List> data ) throws Exception + { + return createAndSavePlate(container, user, plate, plateSetId, data, false); + } + + private @NotNull Plate createAndSavePlate( + @NotNull Container container, + @NotNull User user, + @NotNull Plate plate, + @Nullable Long plateSetId, + @Nullable List> data, + boolean skipAudit + ) throws Exception { if (!plate.isNew()) throw new ValidationException(String.format("Failed to create plate. The provided plate already exists with rowId (%d).", plate.getRowId())); @@ -311,6 +329,8 @@ public List getWellGroupTypes() try (DbScope.Transaction tx = ensureTransaction()) { + ensureTransactionAuditId(tx, container, user, QueryService.AuditAction.INSERT); + PlateSet plateSet = null; if (plateSetId != null) @@ -325,13 +345,16 @@ public List getWellGroupTypes() ((PlateImpl) plate).setPlateSet(plateSet); } - long plateRowId = save(container, user, plate, data); + long plateRowId = save(container, user, plate, data, true); plate = getPlate(container, plateRowId); if (plate == null) throw new IllegalStateException("Unexpected failure. Failed to retrieve plate after save (pre-commit)."); deriveCustomFieldsFromWellData(container, user, plate, data, plateSet); + if (!skipAudit) + addPlateCreatedAuditEvents(container, user, tx, List.of(plate), null); + tx.commit(); // re-fetch the plate to get updated well data @@ -370,11 +393,11 @@ private List getDefaultFieldsForPlateSet(@NotNull Plate plate, fields.add(new PlateCustomField(WellTable.Column.Type.fieldKey())); fields.add(new PlateCustomField(WellTable.Column.WellGroup.fieldKey())); fields.add(new PlateCustomField(WellTable.Column.ReplicateGroup.fieldKey())); - fields.add(new PlateCustomField(WellTable.Column.SampleID.fieldKey())); + fields.add(new PlateCustomField(WellTable.Column.SampleId.fieldKey())); } } else if (plateSet.isPrimary()) - fields.add(new PlateCustomField(WellTable.Column.SampleID.fieldKey())); + fields.add(new PlateCustomField(WellTable.Column.SampleId.fieldKey())); else if (plateSet.isTemplate()) fields = templateFields; @@ -469,7 +492,7 @@ public List getMetadataColumns(@NotNull PlateSet plateSet, Container c for (PlateCustomField field : plate.getCustomFields()) { - if (WellTable.Column.SampleID.fieldKey().equals(field.getFieldKey())) + if (WellTable.Column.SampleId.fieldKey().equals(field.getFieldKey())) continue; FieldKey lookupFk = displayColumns.get(field.getPropertyURI()); if (lookupFk != null) @@ -905,13 +928,13 @@ public boolean isDuplicatePlateTemplateName(Container container, String name) if (StringUtils.trimToNull(name) == null) return false; - SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Name"), name); - filter.addCondition(FieldKey.fromParts("Template"), true); + SimpleFilter filter = new SimpleFilter(PlateTable.Column.Name.fieldKey(), name); + filter.addCondition(PlateTable.Column.Template.fieldKey(), true); ContainerFilter cf = getPlateLookupContainerFilter(container, User.getAdminServiceUser()); - filter.addCondition(cf.createFilterClause(AssayDbSchema.getInstance().getSchema(), FieldKey.fromParts("Container"))); + filter.addCondition(cf.createFilterClause(AssayDbSchema.getInstance().getSchema(), PlateTable.Column.Container.fieldKey())); - return new TableSelector(AssayDbSchema.getInstance().getTableInfoPlate(), Set.of("RowId"), filter, null).exists(); + return new TableSelector(AssayDbSchema.getInstance().getTableInfoPlate(), Set.of(PlateTable.Column.RowId.name()), filter, null).exists(); } private @NotNull ContainerFilter getPlateLookupContainerFilter(Container container, User user) @@ -986,13 +1009,13 @@ private void setProperties(Container container, PropertySetImpl propertySet) @Override public long save(Container container, User user, Plate plate) throws Exception { - return save(container, user, plate, null); + return save(container, user, plate, null, false); } - private long save(Container container, User user, Plate plate, @Nullable List> wellData) throws Exception + private long save(Container container, User user, Plate plate, @Nullable List> wellData, boolean skipAudit) throws Exception { if (plate instanceof PlateImpl plateTemplate) - return savePlateImpl(container, user, plateTemplate, false, wellData); + return savePlateImpl(container, user, plateTemplate, false, wellData, skipAudit); throw new IllegalArgumentException("Only plate instances created by the plate service can be saved."); } @@ -1078,7 +1101,7 @@ protected Plate populatePlate(PlateBean bean) private WellImpl[] getWells(Plate plate) { - SimpleFilter plateFilter = new SimpleFilter(FieldKey.fromParts("PlateId"), plate.getRowId()); + SimpleFilter plateFilter = new SimpleFilter(WellTable.Column.PlateId.fieldKey(), plate.getRowId()); Sort sort = new Sort("Col,Row"); return new TableSelector(AssayDbSchema.getInstance().getTableInfoWell(), plateFilter, sort).getArray(WellImpl.class); } @@ -1100,7 +1123,7 @@ else if (type == Well.class) return new Lsid(nameSpace, "Folder-" + container.getRowId(), GUID.makeGUID()); } - /* package private */ DbScope.Transaction ensureTransaction(Lock... locks) + public DbScope.Transaction ensureTransaction(Lock... locks) { return AssayDbSchema.getInstance().getSchema().getScope().ensureTransaction(locks); } @@ -1112,7 +1135,7 @@ public long savePlateImpl(Container container, User user, @NotNull PlateImpl pla private long savePlateImpl(Container container, User user, @NotNull PlateImpl plate, boolean isCopy) throws Exception { - return savePlateImpl(container, user, plate, isCopy, null); + return savePlateImpl(container, user, plate, isCopy, null, false); } private long savePlateImpl( @@ -1120,13 +1143,16 @@ private long savePlateImpl( User user, @NotNull PlateImpl plate, boolean isCopy, - @Nullable List> wellData + @Nullable List> wellData, + boolean skipAudit ) throws Exception { boolean updateExisting = plate.getRowId() != null; try (DbScope.Transaction transaction = ensureTransaction()) { + ensureTransactionAuditId(transaction, container, user, updateExisting ? QueryService.AuditAction.UPDATE : QueryService.AuditAction.INSERT); + Long plateId = plate.getRowId(); if (!updateExisting && plate.getPlateSet() == null) @@ -1138,7 +1164,7 @@ private long savePlateImpl( plate.setPlateSet(createPlateSet(container, user, plateSet, null, null)); } - Map plateRow = ObjectFactory.Registry.getFactory(PlateBean.class).toMap(PlateBean.from(plate), new ArrayListMap<>()); + Map plateRow = ObjectFactory.Registry.getFactory(PlateBean.class).toMap(PlateBean.from(plate, false), new ArrayListMap<>()); QueryUpdateService qus = getPlateUpdateService(container, user); BatchValidationException errors = new BatchValidationException(); @@ -1158,12 +1184,12 @@ private long savePlateImpl( if (errors.hasErrors()) throw errors; Map row = insertedRows.get(0); - plateId = MapUtils.getLong(row,"RowId"); + plateId = MapUtils.getLong(row,PlateTable.Column.RowId.name()); plate.setRowId(plateId); - plate.setLsid((String) row.get("Lsid")); - plate.setName((String) row.get("Name")); - plate.setPlateId((String) row.get("PlateId")); - plate.setBarcode((String) row.get("Barcode")); + plate.setLsid((String) row.get(PlateTable.Column.Lsid.name())); + plate.setName((String) row.get(PlateTable.Column.Name.name())); + plate.setPlateId((String) row.get(PlateTable.Column.PlateId.name())); + plate.setBarcode((String) row.get(PlateTable.Column.Barcode.name())); } savePropertyBag(container, user, plate.getLSID(), plate.getProperties(), updateExisting); @@ -1207,7 +1233,7 @@ private long savePlateImpl( if (wellGroupErrors.hasErrors()) throw wellGroupErrors; - wellGroupInstanceLsid = (String)insertedRows.get(0).get("Lsid"); + wellGroupInstanceLsid = (String) insertedRows.get(0).get(WellTable.Column.Lsid.name()); wellgroup = ObjectFactory.Registry.getFactory(WellGroupImpl.class).fromMap(wellgroup, insertedRows.get(0)); savePropertyBag(container, user, wellGroupInstanceLsid, wellgroup.getProperties(), false); } @@ -1250,8 +1276,9 @@ private long savePlateImpl( } } + Map configParameters = Map.of(AuditBehavior, AuditBehaviorType.DETAILED); BatchValidationException wellErrors = new BatchValidationException(); - insertedRows = wellQus.insertRows(user, container, wellRows, wellErrors, null, extraScriptContext); + insertedRows = wellQus.insertRows(user, container, wellRows, wellErrors, configParameters, extraScriptContext); if (wellErrors.hasErrors()) throw wellErrors; } @@ -1292,6 +1319,16 @@ private long savePlateImpl( if (plate.getPlateSet() != null) indexPlateSet(SearchService.get().defaultTask().getQueue(container, SearchService.PRIORITY.modified), plate.getPlateSet()); }, DbScope.CommitTaskOption.POSTCOMMIT); + + if (!skipAudit && !updateExisting) + { + var auditPlate = getPlate(container, plateRowId); + if (auditPlate == null) + throw new IllegalStateException("Unable to audit plate after save. Plate not found."); + + addPlateCreatedAuditEvents(container, user, transaction, List.of(auditPlate), null); + } + transaction.commit(); return plateId; @@ -1427,19 +1464,19 @@ private void savePropertyBag( @Override public void deletePlate(Container container, User user, long rowId) throws Exception { - Map key = Collections.singletonMap("RowId", rowId); + Map key = Collections.singletonMap(PlateTable.Column.RowId.name(), rowId); QueryUpdateService qus = getPlateUpdateService(container, user); qus.deleteRows(user, container, Collections.singletonList(key), null, null); } - // Called by the Plate Query Update Service after deleting a plate + // Called by the Plate Query Update Service after deleting a plate (post-commit) public void afterPlateDelete(Container container, Plate plate) { clearCache(container, plate); deindexPlates(List.of(Lsid.parse(plate.getLSID()))); } - // Called by the Plate Query Update Service prior to deleting a plate + // Called by the Plate Query Update Service before deleting a plate public void beforePlateDelete(Container container, Integer plateId) { assert requireActiveTransaction(); @@ -1819,8 +1856,13 @@ private void copyWellData(User user, @NotNull Plate source, @NotNull Plate copy, var container = source.getContainer(); var wellTable = getWellTable(container, user); - var sourceWellData = new TableSelector(wellTable, Set.of("RowId", "LSID", "SampleId"), new SimpleFilter(FieldKey.fromParts("PlateId"), source.getRowId()), new Sort("RowId")).getMapArray(); - var copyWellData = new TableSelector(wellTable, Set.of("RowId", "LSID"), new SimpleFilter(FieldKey.fromParts("PlateId"), copy.getRowId()), new Sort("RowId")).getMapArray(); + var lsidColumn = WellTable.Column.Lsid.name(); + var lsidFieldKey = WellTable.Column.Lsid.fieldKey(); + var rowIdColumn = WellTable.Column.RowId.name(); + var sampleIdColumn = WellTable.Column.SampleId.name(); + + var sourceWellData = new TableSelector(wellTable, Set.of(rowIdColumn, lsidColumn, sampleIdColumn), new SimpleFilter(WellTable.Column.PlateId.fieldKey(), source.getRowId()), new Sort(rowIdColumn)).getMapArray(); + var copyWellData = new TableSelector(wellTable, Set.of(rowIdColumn, lsidColumn), new SimpleFilter(WellTable.Column.PlateId.fieldKey(), copy.getRowId()), new Sort(rowIdColumn)).getMapArray(); if (sourceWellData.length != copyWellData.length) { @@ -1828,8 +1870,8 @@ private void copyWellData(User user, @NotNull Plate source, @NotNull Plate copy, throw new ValidationException(String.format(msg, source.getName(), sourceWellData.length, copyWellData.length)); } - var sourceWellLSIDS = Arrays.stream(sourceWellData).map(data -> data.get("LSID")).toList(); - var sourceFilter = new SimpleFilter(FieldKey.fromParts("LSID"), sourceWellLSIDS, CompareType.IN); + var sourceWellLsids = Arrays.stream(sourceWellData).map(data -> data.get(lsidColumn)).toList(); + var sourceFilter = new SimpleFilter(lsidFieldKey, sourceWellLsids, CompareType.IN); final List wellMetadataFields; final Map> sourceMetaData; @@ -1838,13 +1880,13 @@ private void copyWellData(User user, @NotNull Plate source, @NotNull Plate copy, if (metadataTable != null) { wellMetadataFields = metadataTable.getColumns() - .stream().filter(c -> !FieldKey.fromParts("LSID").equals(c.getFieldKey())) + .stream().filter(c -> !lsidFieldKey.equals(c.getFieldKey())) .toList(); var metaDataRows = new TableSelector(metadataTable, sourceFilter, null).getMapCollection(); // note that row map keys here are column.getAlias() sourceMetaData = new CaseInsensitiveHashMap<>(); for (var row : metaDataRows) - sourceMetaData.put((String) row.get("LSID"), row); + sourceMetaData.put((String) row.get(lsidColumn), row); } else { @@ -1857,12 +1899,12 @@ private void copyWellData(User user, @NotNull Plate source, @NotNull Plate copy, for (int i = 0; i < sourceWellData.length; i++) { var sourceRow = sourceWellData[i]; - String sourceWellLSID = (String) sourceRow.get("LSID"); + String sourceWellLSID = (String) sourceRow.get(lsidColumn); var copyRow = copyWellData[i]; var updateCopyRow = new CaseInsensitiveHashMap<>(); - if (copySample && sourceRow.get("SampleId") != null) - updateCopyRow.put("SampleId", sourceRow.get("SampleId")); + if (copySample && sourceRow.get(sampleIdColumn) != null) + updateCopyRow.put(sampleIdColumn, sourceRow.get(sampleIdColumn)); if (sourceMetaData.containsKey(sourceWellLSID)) { @@ -1879,7 +1921,7 @@ private void copyWellData(User user, @NotNull Plate source, @NotNull Plate copy, if (!updateCopyRow.isEmpty()) { - updateCopyRow.put("RowId", copyRow.get("RowId")); + updateCopyRow.put(rowIdColumn, copyRow.get(rowIdColumn)); newWellData.add(updateCopyRow); } } @@ -1888,8 +1930,9 @@ private void copyWellData(User user, @NotNull Plate source, @NotNull Plate copy, return; var errors = new BatchValidationException(); + Map configParameters = Map.of(AuditBehavior, AuditBehaviorType.DETAILED); Map extraScriptContext = CaseInsensitiveHashMap.of(PLATE_COPY_FLAG, true); - getWellUpdateService(container, user).updateRows(user, container, newWellData, null, errors, null, extraScriptContext); + getWellUpdateService(container, user).updateRows(user, container, newWellData, null, errors, configParameters, extraScriptContext); if (errors.hasErrors()) throw errors; } @@ -1952,8 +1995,10 @@ else if (isDuplicatePlateName(container, user, name, destinationPlateSet)) throw new ValidationException(String.format("Failed to copy plate. A plate already exists with the name \"%s\".", name)); } - try (DbScope.Transaction tx = ExperimentService.get().ensureTransaction()) + try (DbScope.Transaction tx = ensureTransaction()) { + ensureTransactionAuditId(tx, container, user, QueryService.AuditAction.INSERT); + // Copy the plate PlateImpl newPlate = new PlateImpl(container, name, null, sourcePlate.getAssayType(), sourcePlate.getPlateType()); List newFields = new ArrayList<>(sourcePlate.getCustomFields()); @@ -1961,7 +2006,7 @@ else if (isDuplicatePlateName(container, user, name, destinationPlateSet)) if (copyAsTemplate) { newPlate.setTemplate(true); - newFields.removeIf((f) -> WellTable.Column.SampleID.fieldKey().equals(f.getFieldKey())); + newFields.removeIf((f) -> WellTable.Column.SampleId.fieldKey().equals(f.getFieldKey())); } else newPlate.setPlateSet(destinationPlateSet); @@ -1973,7 +2018,7 @@ else if (isDuplicatePlateName(container, user, name, destinationPlateSet)) copyWellGroups(sourcePlate, newPlate); // Save the plate - long plateId = savePlateImpl(container, user, newPlate, true); + long plateId = savePlateImpl(container, user, newPlate, true, null, true); newPlate = (PlateImpl) getPlate(container, plateId); if (newPlate == null) throw new IllegalStateException("Unexpected failure. Failed to retrieve plate after save (pre-commit)."); @@ -1983,6 +2028,11 @@ else if (isDuplicatePlateName(container, user, name, destinationPlateSet)) copySamples = true; copyWellData(user, sourcePlate, newPlate, !newPlate.isTemplate() && copySamples); + // Specify the source plate for auditing + newPlate.setSourcePlateRowId(sourcePlate.getRowId()); + String auditComment = String.format("Copied from %s \"%s\".", sourcePlate.isTemplate() ? "plate template" : "plate", sourcePlate.getName()); + addPlateCreatedAuditEvents(container, user, tx, List.of(newPlate), auditComment); + tx.commit(); return newPlate; @@ -2420,7 +2470,7 @@ public Container getPlateMetadataDomainContainer(Container container) fields.add(new PlateCustomField(wellTable.getColumn(WellTable.Column.Type.fieldKey()))); fields.add(new PlateCustomField(wellTable.getColumn(WellTable.Column.WellGroup.fieldKey()))); fields.add(new PlateCustomField(wellTable.getColumn(WellTable.Column.ReplicateGroup.fieldKey()))); - fields.add(new PlateCustomField(wellTable.getColumn(WellTable.Column.SampleID.fieldKey()))); + fields.add(new PlateCustomField(wellTable.getColumn(WellTable.Column.SampleId.fieldKey()))); } Domain metadataDomain = getPlateMetadataDomain(container, user); @@ -2563,7 +2613,7 @@ private List sortCustomFields(List fields) order.put(WellTable.Column.Type.fieldKey(), 0); order.put(WellTable.Column.WellGroup.fieldKey(), 1); order.put(WellTable.Column.ReplicateGroup.fieldKey(), 2); - order.put(WellTable.Column.SampleID.fieldKey(), 3); + order.put(WellTable.Column.SampleId.fieldKey(), 3); Comparator nameComparator = Comparator.comparing((k) -> k.getName().toLowerCase(), Comparator.nullsLast(String::compareTo)); fields.sort((f1, f2) -> { @@ -2599,7 +2649,7 @@ else if (f2.isBuiltIn()) Map customFieldMap = new HashMap<>(); for (WellCustomField customField : fields) customFieldMap.put(FieldKey.fromParts(customField.getName()), customField); - SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("rowId"), wellId); + SimpleFilter filter = new SimpleFilter(WellTable.Column.RowId.fieldKey(), wellId); TableInfo wellTable = getWellTable(plate.getContainer(), user); Map columnMap = QueryService.get().getColumns(wellTable, customFieldMap.keySet()); @@ -2758,7 +2808,8 @@ private List addPlatesToPlateSet( User user, long plateSetId, boolean plateSetIsTemplate, - @NotNull List plates + @NotNull List plates, + @Nullable String additionalAuditComment ) throws Exception { if (plates.isEmpty()) @@ -2766,6 +2817,8 @@ private List addPlatesToPlateSet( try (DbScope.Transaction tx = ensureTransaction()) { + ensureTransactionAuditId(tx, container, user, QueryService.AuditAction.INSERT); + pausePlateIndexing(); tx.addCommitTask(this::resumePlateIndexing, DbScope.CommitTaskOption.POSTCOMMIT, DbScope.CommitTaskOption.POSTROLLBACK); @@ -2778,9 +2831,11 @@ private List addPlatesToPlateSet( plateImpl.setTemplate(plateSetIsTemplate); // TODO: Write a cheaper plate create/save for multiple plates - platesAdded.add(createAndSavePlate(container, user, plateImpl, plateSetId, plate.data)); + platesAdded.add(createAndSavePlate(container, user, plateImpl, plateSetId, plate.data, true)); } + addPlateCreatedAuditEvents(container, user, tx, platesAdded, additionalAuditComment); + tx.commit(); return platesAdded; @@ -2823,6 +2878,7 @@ public PlateSetImpl createPlateSet( try (DbScope.Transaction tx = ensureTransaction()) { + ensureTransactionAuditId(tx, container, user, QueryService.AuditAction.INSERT); BatchValidationException errors = new BatchValidationException(); QueryUpdateService qus = getPlateSetUpdateService(container, user); @@ -2831,12 +2887,12 @@ public PlateSetImpl createPlateSet( if (errors.hasErrors()) throw errors; - Integer plateSetId = asInteger(rows.get(0).get("RowId")); + Integer plateSetId = asInteger(rows.get(0).get(PlateSetTable.Column.RowId.name())); savePlateSetHeritage(plateSetId, plateSet.getType(), parentPlateSet); if (plates != null) - addPlatesToPlateSet(container, user, plateSetId, plateSet.isTemplate(), plates); + addPlatesToPlateSet(container, user, plateSetId, plateSet.isTemplate(), plates, String.format("Added during creation of plate set \"%s\".", plateSet.getName())); plateSet = (PlateSetImpl) getPlateSet(container, plateSetId); tx.commit(); @@ -2884,7 +2940,7 @@ public PlateSet createOrAddToPlateSet(Container container, User user, CreatePlat return createPlateSet(container, user, targetPlateSet, plates, options.getParentPlateSetId()); // Update an existing plate set - addPlatesToPlateSet(container, user, targetPlateSet.getRowId(), targetPlateSet.isTemplate(), plates); + addPlatesToPlateSet(container, user, targetPlateSet.getRowId(), targetPlateSet.isTemplate(), plates, null); return getPlateSet(container, targetPlateSet.getRowId()); } @@ -2902,6 +2958,7 @@ private PlateSet replatePlateSet( try (DbScope.Transaction tx = ensureTransaction()) { + ensureTransactionAuditId(tx, container, user, QueryService.AuditAction.INSERT); PlateSet newPlateSet = createPlateSet(container, user, targetPlateSet, null, parentId); for (Plate plate : parentPlateSet.getPlates()) @@ -3492,7 +3549,7 @@ Pair>> getWellSampleData( int colIdx = iterateByColumn ? outerIdx : innerIdx; wellSampleDataForPlate.add(CaseInsensitiveHashMap.of( - WellTable.Column.SampleID.name(), sampleIds.get(sampleIdsCounter), + WellTable.Column.SampleId.name(), sampleIds.get(sampleIdsCounter), WellTable.Column.Type.name(), SAMPLE.name(), WELL_LOCATION, createPosition(c, rowIdx, colIdx).getDescription() )); @@ -3698,7 +3755,7 @@ public List getInstrumentInstructions(long plateSetId, List private List getPlateExportFieldKeys(Plate plate, boolean isMapView) { List fieldKeys = new ArrayList<>(); - fieldKeys.add(FieldKey.fromParts(WellTable.Column.SampleID.name(), "Name")); + fieldKeys.add(FieldKey.fromParts(WellTable.Column.SampleId.name(), "Name")); if (isMapView) { @@ -3716,7 +3773,7 @@ private List getPlateExportFieldKeys(Plate plate, boolean isMapView) if (isMapView) { Set excludedColumns = Set.of( - WellTable.Column.SampleID.fieldKey(), + WellTable.Column.SampleId.fieldKey(), WellTable.Column.Type.fieldKey(), WellTable.Column.WellGroup.fieldKey(), WellTable.Column.ReplicateGroup.fieldKey() @@ -3756,7 +3813,7 @@ private List getPlateDisplayColumns(QueryView queryView) // Filter on isQueryColumn, so we don't get the details or update columns return dataRegion.getDisplayColumns().stream() .filter(DisplayColumn::isQueryColumn) - .filter(col -> !col.getName().equalsIgnoreCase(WellTable.Column.SampleID.name())) + .filter(col -> !col.getName().equalsIgnoreCase(WellTable.Column.SampleId.name())) .toList(); } @@ -3776,7 +3833,7 @@ public List exportPlateData(Container c, User user, ContainerFil QueryView plateQueryView = getPlateQueryView(c, user, cf, plate, false); List displayColumns = getPlateDisplayColumns(plateQueryView); PlateFileBytes plateFileBytes = new PlateFileBytes(plate.getName(), new ByteArrayOutputStream()); - FieldKey sampleIdNameFieldKey = FieldKey.fromParts(WellTable.Column.SampleID.name(), "Name"); + FieldKey sampleIdNameFieldKey = FieldKey.fromParts(WellTable.Column.SampleId.name(), "Name"); try (TSVGridWriter writer = new TSVGridWriter(plateQueryView::getResults, displayColumns, Collections.singletonMap(sampleIdNameFieldKey.toString(), "Sample ID"))) { @@ -3827,10 +3884,10 @@ public List getWellData(Container container, User user, long plateRowI columns.add(WellTable.Column.WellGroup.name()); columns.add(WellTable.Column.ReplicateGroup.name()); if (includeSamples) - columns.add(WellTable.Column.SampleID.name()); + columns.add(WellTable.Column.SampleId.name()); var wellTable = getWellTable(container, user, getPlateLookupContainerFilter(container, user)); - var filter = new SimpleFilter(FieldKey.fromParts(WellTable.Column.PlateId.name()), plateRowId); + var filter = new SimpleFilter(WellTable.Column.PlateId.fieldKey(), plateRowId); var wellDatas = new TableSelector(wellTable, columns, filter, new Sort(WellTable.Column.RowId.name())).getArrayList(WellData.class); if (includeMetadata) @@ -3848,7 +3905,7 @@ private List getWellMetadata(Container container, User user, List>(); var ignoredKeys = CaseInsensitiveHashSet.of("_row", WellTable.Column.Lsid.name()); @@ -4095,7 +4152,7 @@ private void validateWells(Plate plate) throws ValidationException "Well %s must specify a \"%s\" when a \"%s\" is specified on plate \"%s\".", position.getDescription(), WellTable.Column.Type.name(), - WellTable.Column.SampleID.name(), + WellTable.Column.SampleId.name(), plate.getName() )); } @@ -4244,7 +4301,7 @@ private String getSampleGroupLabKeySql(@NotNull Long plateSetRowId, boolean incl columnNames.add(WellTable.Column.Type.name()); columnNames.add(WellTable.Column.WellGroup.name()); if (includeSampleId) - columnNames.add(WellTable.Column.SampleID.name()); + columnNames.add(WellTable.Column.SampleId.name()); String columns = columnNames.stream().map(LabKeySql::quoteIdentifier).collect(Collectors.joining(", ")); String wellTypes = StringUtils.join( @@ -4471,6 +4528,8 @@ public record ReformatResult( { try (DbScope.Transaction tx = ensureTransaction()) { + ensureTransactionAuditId(tx, container, user, QueryService.AuditAction.INSERT); + if (!existingPlates.isEmpty()) { QueryUpdateService qus = requiredUpdateService(getWellTable(container, user)); @@ -4487,7 +4546,7 @@ public record ReformatResult( plateSetRowId = targetPlateSet.getRowId(); plateSetName = targetPlateSet.getName(); - newPlates = addPlatesToPlateSet(container, user, plateSetRowId, targetPlateSet.isTemplate(), plateData); + newPlates = addPlatesToPlateSet(container, user, plateSetRowId, targetPlateSet.isTemplate(), plateData, String.format("Added via reformat to plate set \"%s\".", plateSetName)); tx.commit(); } @@ -4527,7 +4586,7 @@ public record ReformatResult( for (Map row : plateData.data) { - Long sampleId = MapUtils.getLong(row, WellTable.Column.SampleID.name()); + Long sampleId = MapUtils.getLong(row, WellTable.Column.SampleId.name()); if (sampleId != null) { wellsFilled++; @@ -4575,7 +4634,7 @@ public record ReformatResult( for (Map row : plate.data) { - Long sampleId = MapUtils.getLong(row,WellTable.Column.SampleID.name()); + Long sampleId = MapUtils.getLong(row,WellTable.Column.SampleId.name()); if (sampleId != null) { wellsFilled++; @@ -5045,4 +5104,36 @@ public void run() } } } + + private void addPlateCreatedAuditEvents(Container container, User user, DbScope.Transaction tx, Collection plates, @Nullable String additionalComment) + { + if (plates.isEmpty()) + return; + + List auditEvents = new ArrayList<>(); + for (Plate plate : plates) + auditEvents.add(PlateAuditProvider.EventFactory.plateCreated(container, tx.getAuditId(), (PlateImpl) plate, additionalComment)); + + AuditLogService.get().addEvents(user, auditEvents, true); + } + + public void addPlateDeletedAuditEvents(Container container, User user, DbScope.Transaction tx, Collection plates) + { + if (plates.isEmpty()) + return; + + List auditEvents = new ArrayList<>(); + for (Plate plate : plates) + auditEvents.add(PlateAuditProvider.EventFactory.plateDeleted(container, tx.getAuditId(), (PlateImpl) plate)); + + AuditLogService.get().addEvents(user, auditEvents, true); + } + + public void ensureTransactionAuditId(DbScope.Transaction tx, Container container, User user, QueryService.AuditAction auditAction) + { + if (tx.getAuditId() != null) + return; + + AbstractQueryUpdateService.addTransactionAuditEvent(tx, user, AbstractQueryUpdateService.createTransactionAuditEvent(container, auditAction)); + } } diff --git a/assay/src/org/labkey/assay/plate/PlateManagerTest.java b/assay/src/org/labkey/assay/plate/PlateManagerTest.java index 0aed33b910a..800098da2df 100644 --- a/assay/src/org/labkey/assay/plate/PlateManagerTest.java +++ b/assay/src/org/labkey/assay/plate/PlateManagerTest.java @@ -1465,7 +1465,7 @@ public void testReformatArrayFromTemplate() throws Exception int t = 0; while (r.next()) { - var sampleId = r.getInt(FieldKey.fromParts(WellTable.Column.SampleID.name())); + var sampleId = r.getInt(FieldKey.fromParts(WellTable.Column.SampleId.name())); var wellPosition = r.getString(FieldKey.fromParts("position")); switch (wellPosition) @@ -1500,7 +1500,7 @@ public void testReformatArrayFromTemplate() throws Exception int t = 0; while (r.next()) { - var sampleId = r.getInt(FieldKey.fromParts(WellTable.Column.SampleID.name())); + var sampleId = r.getInt(FieldKey.fromParts(WellTable.Column.SampleId.name())); var wellPosition = r.getString(FieldKey.fromParts("position")); switch (wellPosition) @@ -1534,7 +1534,7 @@ public void testReformatArrayFromTemplate() throws Exception { for (Map well : data.data()) { - if (asLongElseNull(well.get(WellTable.Column.SampleID.name())) instanceof Long num) + if (asLongElseNull(well.get(WellTable.Column.SampleId.name())) instanceof Long num) sampleIds.add(num); } } diff --git a/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java b/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java new file mode 100644 index 00000000000..127043ede7b --- /dev/null +++ b/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java @@ -0,0 +1,142 @@ +package org.labkey.assay.plate.audit; + +import org.labkey.api.audit.AbstractAuditTypeProvider; +import org.labkey.api.audit.DetailedAuditTypeEvent; +import org.labkey.api.collections.CaseInsensitiveHashMap; +import org.labkey.api.collections.CaseInsensitiveHashSet; +import org.labkey.api.data.Container; +import org.labkey.api.data.ObjectFactory; +import org.labkey.assay.plate.PlateImpl; +import org.labkey.assay.plate.model.PlateBean; +import org.labkey.assay.plate.query.PlateTable; + +import java.util.Map; +import java.util.Set; + +import static org.labkey.assay.plate.audit.PlateAuditProvider.EVENT_NAME; + +public class PlateAuditEvent extends DetailedAuditTypeEvent +{ + private String _plateEventType; + private Long _plateRowId; + private String _plateName; + private Long _plateSetRowId; + private Long _plateTypeRowId; + private Long _sourcePlateRowId; + private boolean _template; + + public PlateAuditEvent() + { + super(); + } + + protected PlateAuditEvent( + PlateAuditProvider.PlateEventType eventType, + Container container, + PlateImpl plate, + Long transactionAuditId + ) + { + super(EVENT_NAME, container, eventType.getComment()); + setPlateEventType(eventType.name()); + setPlateRowId(plate.getRowId()); + setPlateName(plate.getName()); + setPlateSetRowId(plate.getPlateSetId()); + setPlateTypeRowId(plate.getPlateType().getRowId()); + setSourcePlateRowId(plate.getSourcePlateRowId()); + setTemplate(plate.isTemplate()); + setTransactionId(transactionAuditId); + } + + public String getPlateEventType() + { + return _plateEventType; + } + + public void setPlateEventType(String plateEventType) + { + _plateEventType = plateEventType; + } + + public Long getPlateRowId() + { + return _plateRowId; + } + + public void setPlateRowId(Long plateRowId) + { + _plateRowId = plateRowId; + } + + public String getPlateName() + { + return _plateName; + } + + public void setPlateName(String plateName) + { + _plateName = plateName; + } + + public Long getPlateSetRowId() + { + return _plateSetRowId; + } + + public void setPlateSetRowId(Long plateSetRowId) + { + _plateSetRowId = plateSetRowId; + } + + public Long getPlateTypeRowId() + { + return _plateTypeRowId; + } + + public void setPlateTypeRowId(Long plateTypeRowId) + { + _plateTypeRowId = plateTypeRowId; + } + + public Long getSourcePlateRowId() + { + return _sourcePlateRowId; + } + + public void setSourcePlateRowId(Long sourcePlateRowId) + { + _sourcePlateRowId = sourcePlateRowId; + } + + public boolean isTemplate() + { + return _template; + } + + public void setTemplate(boolean template) + { + _template = template; + } + + private static final Set EXCLUDED_PROPERTIES = CaseInsensitiveHashSet.of("ContainerId", PlateTable.Column.DataFileId.name(), "EntityId"); + + public void setNewRecordMap(Container container, PlateImpl plate) + { + Map plateRow = ObjectFactory.Registry.getFactory(PlateBean.class) + .toMap(PlateBean.from(plate, true), new CaseInsensitiveHashMap<>()); + EXCLUDED_PROPERTIES.forEach(plateRow::remove); + + var newRecordMap = AbstractAuditTypeProvider.encodeForDataMap(plateRow); + setNewRecordMap(newRecordMap, container); + } + + public void setOldRecordMap(Container container, PlateImpl plate) + { + Map plateRow = ObjectFactory.Registry.getFactory(PlateBean.class) + .toMap(PlateBean.from(plate, true), new CaseInsensitiveHashMap<>()); + EXCLUDED_PROPERTIES.forEach(plateRow::remove); + + var oldRecordMap = AbstractAuditTypeProvider.encodeForDataMap(plateRow); + setOldRecordMap(oldRecordMap, container); + } +} diff --git a/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java b/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java new file mode 100644 index 00000000000..adbe7018fcc --- /dev/null +++ b/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java @@ -0,0 +1,204 @@ +package org.labkey.assay.plate.audit; + +import org.jetbrains.annotations.Nullable; +import org.labkey.api.audit.AbstractAuditTypeProvider; +import org.labkey.api.audit.AuditTypeEvent; +import org.labkey.api.audit.query.AbstractAuditDomainKind; +import org.labkey.api.audit.query.DefaultAuditTypeTable; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.TableInfo; +import org.labkey.api.exp.PropertyDescriptor; +import org.labkey.api.exp.PropertyType; +import org.labkey.api.query.FieldKey; +import org.labkey.api.query.UserSchema; +import org.labkey.assay.plate.PlateImpl; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +public class PlateAuditProvider extends AbstractAuditTypeProvider +{ + public static final String EVENT_NAME = "PlateEvent"; + + enum Column + { + PlateEventType, + PlateName, + PlateRowId, + PlateSetRowId, + PlateTypeRowId, + SourcePlateRowId, + Template; + + private final FieldKey _fieldKey = FieldKey.fromParts(name()); + + public FieldKey fieldKey() + { + return _fieldKey; + } + } + + static final List defaultVisibleColumns = new ArrayList<>(); + + static + { + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CREATED)); + defaultVisibleColumns.add(Column.PlateRowId.fieldKey()); + defaultVisibleColumns.add(Column.PlateTypeRowId.fieldKey()); + defaultVisibleColumns.add(Column.PlateSetRowId.fieldKey()); + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_COMMENT)); + defaultVisibleColumns.add(Column.SourcePlateRowId.fieldKey()); + defaultVisibleColumns.add(Column.Template.fieldKey()); + } + + @Override + public List getDefaultVisibleColumns() + { + return defaultVisibleColumns; + } + + @Override + public TableInfo createTableInfo(UserSchema userSchema, ContainerFilter cf) + { + DefaultAuditTypeTable table = new DefaultAuditTypeTable(this, createStorageTableInfo(), userSchema, cf, getDefaultVisibleColumns()); + appendValueMapColumns(table, getEventName()); + + return table; + } + + public PlateAuditProvider() + { + super(new PlateAuditDomainKind()); + } + + @Override + public String getEventName() + { + return EVENT_NAME; + } + + @Override + public String getLabel() + { + return "Plate events"; + } + + @Override + public String getDescription() + { + return "Events related to plates"; + } + + @Override + public Class getEventClass() + { + return (Class) PlateAuditEvent.class; + } + + public enum PlateEventType + { + CREATE_PLATE("Plate was created.", "Created"), + DELETE_PLATE("Plate was deleted.", "Deleted"); + + private final String _actionLabel; + private final String _comment; + + PlateEventType(String comment, String actionLabel) + { + _comment = comment; + _actionLabel = actionLabel; + } + + public String getActionLabel() + { + return _actionLabel; + } + + public String getComment() + { + return _comment; + } + } + + public static class PlateAuditDomainKind extends AbstractAuditDomainKind + { + private static final String NAME = "PlateAuditDomain"; + private static final String NAMESPACE_PREFIX = "Audit-" + NAME; + + private final Set fields; + + public PlateAuditDomainKind() + { + super(EVENT_NAME); + + Set _fields = new LinkedHashSet<>(); + + // PlateAuditEvent fields + _fields.add(createPropertyDescriptor(Column.PlateEventType.name(), PropertyType.STRING)); + _fields.add(createPropertyDescriptor(Column.PlateName.name(), PropertyType.STRING)); + _fields.add(createPropertyDescriptor(Column.PlateRowId.name(), PropertyType.INTEGER)); + _fields.add(createPropertyDescriptor(Column.PlateSetRowId.name(), PropertyType.INTEGER)); + _fields.add(createPropertyDescriptor(Column.PlateTypeRowId.name(), PropertyType.INTEGER)); + _fields.add(createPropertyDescriptor(Column.SourcePlateRowId.name(), PropertyType.INTEGER)); + _fields.add(createPropertyDescriptor(Column.Template.name(), PropertyType.BOOLEAN)); + + // AbstractAuditTypeProvider fields + _fields.add(createPropertyDescriptor(COLUMN_NAME_COMMENT, PropertyType.STRING)); + _fields.add(createPropertyDescriptor(COLUMN_NAME_TRANSACTION_ID, PropertyType.BIGINT)); + _fields.add(createPropertyDescriptor(COLUMN_NAME_USER_COMMENT, PropertyType.STRING)); + _fields.add(createOldDataMapPropertyDescriptor()); + _fields.add(createNewDataMapPropertyDescriptor()); + + fields = Collections.unmodifiableSet(_fields); + } + + @Override + protected String getNamespacePrefix() + { + return NAMESPACE_PREFIX; + } + + @Override + public Set getProperties() + { + return fields; + } + + @Override + public String getKindName() + { + return NAME; + } + } + + public static class EventFactory + { + public static PlateAuditEvent plateCreated( + Container container, + Long transactionAuditId, + PlateImpl plate, + @Nullable String additionalComment + ) + { + var event = new PlateAuditEvent(PlateEventType.CREATE_PLATE, container, plate, transactionAuditId); + event.setNewRecordMap(container, plate); + + if (additionalComment != null) + event.setComment(event.getComment() + " " + additionalComment); + + return event; + } + + public static PlateAuditEvent plateDeleted(Container container, Long transactionAuditId, PlateImpl plate) + { + var event = new PlateAuditEvent(PlateEventType.DELETE_PLATE, container, plate, transactionAuditId); + event.setOldRecordMap(container, plate); + + return event; + } + } +} diff --git a/assay/src/org/labkey/assay/plate/data/PlateMapExcelWriter.java b/assay/src/org/labkey/assay/plate/data/PlateMapExcelWriter.java index 3803002938b..dbfd19cba34 100644 --- a/assay/src/org/labkey/assay/plate/data/PlateMapExcelWriter.java +++ b/assay/src/org/labkey/assay/plate/data/PlateMapExcelWriter.java @@ -37,7 +37,7 @@ public class PlateMapExcelWriter extends ExcelWriter { private static final Logger logger = LogHelper.getLogger(PlateMapExcelWriter.class, "Plate map export"); - private static final Set excludedFields = CaseInsensitiveHashSet.of(Column.SampleID.name(), Column.Type.name(), Column.WellGroup.name()); + private static final Set excludedFields = CaseInsensitiveHashSet.of(Column.SampleId.name(), Column.Type.name(), Column.WellGroup.name()); private final Plate _plate; private final QueryView _queryView; diff --git a/assay/src/org/labkey/assay/plate/data/PlateTriggerFactory.java b/assay/src/org/labkey/assay/plate/data/PlateTriggerFactory.java new file mode 100644 index 00000000000..337bd7fbb05 --- /dev/null +++ b/assay/src/org/labkey/assay/plate/data/PlateTriggerFactory.java @@ -0,0 +1,78 @@ +package org.labkey.assay.plate.data; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.labkey.api.assay.plate.Plate; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.DbScope; +import org.labkey.api.data.TableInfo; +import org.labkey.api.data.triggers.Trigger; +import org.labkey.api.data.triggers.TriggerFactory; +import org.labkey.api.query.BatchValidationException; +import org.labkey.api.query.ValidationException; +import org.labkey.api.security.User; +import org.labkey.api.util.GUID; +import org.labkey.assay.plate.PlateManager; +import org.labkey.assay.plate.query.PlateTable; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.labkey.api.util.IntegerUtils.asInteger; + +public final class PlateTriggerFactory implements TriggerFactory +{ + @Override + public @NotNull Collection createTrigger(@Nullable Container c, TableInfo table, Map extraContext) + { + return List.of( + new AuditPlateDeleteTrigger() + ); + } + + @SuppressWarnings("InnerClassMayBeStatic") + private class AuditPlateDeleteTrigger implements Trigger + { + private Map> _platesDeleted = null; + + @Override + public void beforeDelete(TableInfo table, Container c, User user, @Nullable Map oldRow, ValidationException errors, Map extraContext) + { + if (oldRow == null || errors.hasErrors()) + return; + + Integer plateId = asInteger(oldRow.get(PlateTable.Column.RowId.name())); + Plate plate = PlateManager.get().getPlate(c, plateId); + if (plate == null) + return; + + if (_platesDeleted == null) + _platesDeleted = new HashMap<>(); + _platesDeleted.putIfAbsent(c.getEntityId(), new ArrayList<>()); + _platesDeleted.get(c.getEntityId()).add(plate); + } + + @Override + public void complete(TableInfo table, Container c, User user, TableInfo.TriggerType event, BatchValidationException errors, Map extraContext) + { + DbScope.Transaction tx = DbScope.getLabKeyScope().getCurrentTransaction(); + assert tx != null; + + if (errors.hasErrors() || _platesDeleted == null || _platesDeleted.isEmpty()) + return; + + for (var entry : _platesDeleted.entrySet()) + { + var container = ContainerManager.getForId(entry.getKey()); + if (container == null) + continue; + + PlateManager.get().addPlateDeletedAuditEvents(container, user, tx, entry.getValue()); + } + } + } +} diff --git a/assay/src/org/labkey/assay/plate/data/WellData.java b/assay/src/org/labkey/assay/plate/data/WellData.java index f189e9de730..2824ba00cef 100644 --- a/assay/src/org/labkey/assay/plate/data/WellData.java +++ b/assay/src/org/labkey/assay/plate/data/WellData.java @@ -45,7 +45,7 @@ public Map getData(boolean includeWellId) if (_position != null) data.put(WellTable.WELL_LOCATION, _position); if (_sampleId != null) - data.put(WellTable.Column.SampleID.name(), _sampleId); + data.put(WellTable.Column.SampleId.name(), _sampleId); if (_type != null) data.put(WellTable.Column.Type.name(), _type.name()); if (_wellGroup != null) diff --git a/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java b/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java index 2b14f82e04b..8b82386f413 100644 --- a/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java +++ b/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java @@ -61,7 +61,7 @@ private void addTypeSample( return; // The "SampleID" is not being modified - if (newRow.get(WellTable.Column.SampleID.name()) == null) + if (newRow.get(WellTable.Column.SampleId.name()) == null) return; // A "Type" is being explicitly provided @@ -136,7 +136,7 @@ private void addWellId(@Nullable Map newRow) if ( newRow != null && newRow.containsKey(WellTable.Column.RowId.name()) && - newRow.getOrDefault(WellTable.Column.SampleID.name(), null) != null + newRow.getOrDefault(WellTable.Column.SampleId.name(), null) != null ) { Long wellRowId = MapUtils.getLong(newRow,WellTable.Column.RowId.name()); @@ -285,7 +285,7 @@ private boolean hasReplicateChange(Container container, User user, @NotNull Long private boolean hasSampleChange(@Nullable Map row) { - return row != null && row.containsKey(WellTable.Column.SampleID.name()); + return row != null && row.containsKey(WellTable.Column.SampleId.name()); } private boolean hasTypeGroupReplicateChange(@Nullable Map row) diff --git a/assay/src/org/labkey/assay/plate/model/PlateBean.java b/assay/src/org/labkey/assay/plate/model/PlateBean.java index df3d3ab577a..472cb7ce7da 100644 --- a/assay/src/org/labkey/assay/plate/model/PlateBean.java +++ b/assay/src/org/labkey/assay/plate/model/PlateBean.java @@ -21,7 +21,7 @@ public class PlateBean extends Entity private String _description; private String _barcode; - public static PlateBean from(PlateImpl plate) + public static PlateBean from(PlateImpl plate, boolean includeEntityProperties) { PlateBean bean = new PlateBean(); @@ -38,6 +38,31 @@ public static PlateBean from(PlateImpl plate) bean.setDescription(plate.getDescription()); bean.setBarcode(plate.getBarcode()); + if (includeEntityProperties) + { + if (plate.getCreated() != null) + bean.setCreated(plate.getCreated()); + + var createdBy = plate.getCreatedByUser(); + if (createdBy != null) + bean.setCreatedBy(createdBy.getUserId()); + + if (plate.getModified() != null) + bean.setModified(plate.getModified()); + + var modifiedBy = plate.getModifiedByUser(); + if (modifiedBy != null) + bean.setModifiedBy(modifiedBy.getUserId()); + + var container = plate.getContainer(); + if (container != null) + bean.setContainerId(container.getId()); + + var entityId = plate.getEntityId(); + if (entityId != null) + bean.setEntityId(entityId); + } + return bean; } diff --git a/assay/src/org/labkey/assay/plate/query/PlateTable.java b/assay/src/org/labkey/assay/plate/query/PlateTable.java index c44d7218481..225114eb401 100644 --- a/assay/src/org/labkey/assay/plate/query/PlateTable.java +++ b/assay/src/org/labkey/assay/plate/query/PlateTable.java @@ -54,6 +54,7 @@ import org.labkey.api.query.FieldKey; import org.labkey.api.query.InvalidKeyException; import org.labkey.api.query.PropertyForeignKey; +import org.labkey.api.query.QueryService; import org.labkey.api.query.QueryUpdateService; import org.labkey.api.query.QueryUpdateServiceException; import org.labkey.api.query.SimpleUserSchema; @@ -65,6 +66,7 @@ import org.labkey.api.security.permissions.Permission; import org.labkey.api.util.GUID; import org.labkey.assay.plate.PlateManager; +import org.labkey.assay.plate.data.PlateTriggerFactory; import org.labkey.assay.query.AssayDbSchema; import java.sql.SQLException; @@ -93,6 +95,7 @@ public enum Column Container, Created, CreatedBy, + DataFileId, Description, Lsid, Modified, @@ -134,6 +137,7 @@ public PlateTable(PlateSchema schema, @Nullable ContainerFilter cf, boolean allo super(schema, AssayDbSchema.getInstance().getTableInfoPlate(), cf); _allowInsert = allowInsert; setTitleColumn(Column.Name.name()); + addTriggerFactory(new PlateTriggerFactory()); } @Override @@ -279,9 +283,9 @@ public DataIteratorBuilder createImportDIB(User user, Container container, DataI (Supplier) () -> PlateManager.get().getLsid(Plate.class, container)); // generate the data file id if not provided - if (!nameMap.containsKey("dataFileId")) + if (!nameMap.containsKey(Column.DataFileId.name())) { - lsidGenerator.addColumn(plateTable.getColumn("dataFileId"), + lsidGenerator.addColumn(plateTable.getColumn(Column.DataFileId.name()), (Supplier) () -> GUID.makeGUID()); } @@ -300,7 +304,7 @@ public DataIteratorBuilder createImportDIB(User user, Container container, DataI nameExpressionTranslator.addColumn(nameCol, (Supplier) () -> null); } - // Add generated barcode column for use in BarcodeDataIterator + // Add a generated barcode column for use in BarcodeDataIterator String barcodeGeneratedName = "barcodeGenerated"; ColumnInfo genIdCol = new BaseColumnInfo(FieldKey.fromParts(barcodeGeneratedName), JdbcType.VARCHAR); nameExpressionTranslator.addTextSequenceColumn(genIdCol, genIdCol.getDbSequenceContainer(ContainerManager.getRoot()), PLATE_BARCODE_SEQUENCE, null, 100); @@ -372,9 +376,27 @@ protected Map updateRow(User user, Container container, Map> deleteRows(User user, Container container, List> keys, @Nullable Map configParameters, @Nullable Map extraScriptContext) throws InvalidKeyException, BatchValidationException, QueryUpdateServiceException, SQLException + { + List> result; + + try (DbScope.Transaction tx = PlateManager.get().ensureTransaction()) + { + PlateManager.get().ensureTransactionAuditId(tx, container, user, QueryService.AuditAction.DELETE); + result = super.deleteRows(user, container, keys, configParameters, extraScriptContext); + tx.commit(); + } + + return result; + } + @Override protected Map deleteRow(User user, Container container, Map oldRowMap) throws QueryUpdateServiceException, SQLException, InvalidKeyException { + DbScope.Transaction tx = DbScope.getLabKeyScope().getCurrentTransaction(); + assert tx != null; + Integer plateId = asInteger(oldRowMap.get(Column.RowId.name())); Plate plate = PlateManager.get().getPlate(container, plateId); if (plate == null) @@ -384,16 +406,12 @@ protected Map deleteRow(User user, Container container, Map 0) throw new QueryUpdateServiceException(String.format("%s is used by %d runs and cannot be deleted", plate.isTemplate() ? "Plate template" : "Plate", runsInUse)); - try (DbScope.Transaction transaction = AssayDbSchema.getInstance().getScope().ensureTransaction()) - { - PlateManager.get().beforePlateDelete(container, plateId); - Map returnMap = super.deleteRow(user, container, oldRowMap); + PlateManager.get().beforePlateDelete(container, plateId); + Map result = super.deleteRow(user, container, oldRowMap); - transaction.addCommitTask(() -> PlateManager.get().afterPlateDelete(container, plate), DbScope.CommitTaskOption.POSTCOMMIT); - transaction.commit(); + tx.addCommitTask(() -> PlateManager.get().afterPlateDelete(container, plate), DbScope.CommitTaskOption.POSTCOMMIT); - return returnMap; - } + return result; } private void preventUpdates(Map newRow, Map oldRow, Column... columns) throws QueryUpdateServiceException diff --git a/assay/src/org/labkey/assay/plate/query/WellTable.java b/assay/src/org/labkey/assay/plate/query/WellTable.java index 6a28ffc02c4..626c1898ac8 100644 --- a/assay/src/org/labkey/assay/plate/query/WellTable.java +++ b/assay/src/org/labkey/assay/plate/query/WellTable.java @@ -1,5 +1,6 @@ package org.labkey.assay.plate.query; +import org.apache.commons.beanutils.ConversionException; import org.apache.logging.log4j.Level; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -13,11 +14,14 @@ import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.FieldKeyRowMap; import org.labkey.api.data.JdbcType; import org.labkey.api.data.MutableColumnInfo; import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Table; import org.labkey.api.data.TableInfo; +import org.labkey.api.data.TableSelector; import org.labkey.api.dataiterator.DataIteratorBuilder; import org.labkey.api.dataiterator.DataIteratorContext; import org.labkey.api.dataiterator.DetailedAuditLogDataIterator; @@ -84,14 +88,16 @@ public enum Column ReplicateGroup, Row, RowId, - SampleID, + SampleId, Type, Value, WellGroup; + private final FieldKey _fieldKey = FieldKey.fromParts(name()); + public FieldKey fieldKey() { - return FieldKey.fromParts(name()); + return _fieldKey; } } @@ -273,7 +279,7 @@ public MutableColumnInfo wrapColumn(ColumnInfo col) var columnInfo = super.wrapColumn(col); // workaround for sample lookup not resolving correctly - if (columnInfo.getName().equalsIgnoreCase(Column.SampleID.name())) + if (columnInfo.getName().equalsIgnoreCase(Column.SampleId.name())) { columnInfo.setFk(QueryForeignKey.from(getUserSchema(), getContainerFilter()) .schema(ExpSchema.SCHEMA_NAME, getContainer()) @@ -383,6 +389,12 @@ public boolean hasPermission(@NotNull UserPrincipal user, @NotNull Class getExtraDetailedUpdateAuditFields() + { + return CaseInsensitiveHashSet.of(Column.Position.name(), Column.PlateId.name()); + } + protected static class WellUpdateService extends DefaultQueryUpdateService { private final TableInfo _provisionedTable; @@ -455,6 +467,48 @@ public List> insertRows( return super._insertRowsUsingDIB(user, container, rows, getDataIteratorContext(errors, InsertOption.INSERT, configParameters), extraScriptContext); } + @Override + protected Map getRow(User user, Container container, Map keys) throws InvalidKeyException, QueryUpdateServiceException, SQLException + { + return getRow(user, container, keys, false); + } + + @Override + protected Map getRow(User user, Container container, Map keys, boolean allowCrossContainer) throws InvalidKeyException, SQLException + { + aliasColumns(_columnMapping, keys); + + Long rowId = (Long) JdbcType.BIGINT.convert(keys.get(Column.RowId.name())); + if (null == rowId) + throw new InvalidKeyException(String.format("Value must be supplied for key field '%s'", Column.RowId.name()), keys); + + return _select(container, rowId, allowCrossContainer); + } + + @Override + protected Map _select(Container container, Object[] keys) throws ConversionException + { + throw new IllegalStateException("Should not be called"); + } + + private Map _select(Container container, Long rowId, boolean allowCrossContainer) throws SQLException + { + if (rowId == null) + return null; + + SimpleFilter filter = new SimpleFilter(Column.RowId.fieldKey(), rowId); + if (!allowCrossContainer) + filter.addCondition(Column.Container.fieldKey(), container.getEntityId()); + + try (var results = new TableSelector(getQueryTable(), filter, null).getResults()) + { + if (results.next()) + return FieldKeyRowMap.toNameMap(results.getFieldKeyRowMap()); + } + + return null; + } + @Override protected Map updateRow( User user, @@ -464,6 +518,7 @@ protected Map updateRow( @Nullable Map configParameters ) throws InvalidKeyException, ValidationException, QueryUpdateServiceException, SQLException { + // TODO: Stop doing this check for each row. Instead perform once per plate that is a part of the update. // enforce no updates if the plate has been imported in an assay run if (oldRow.containsKey(Column.PlateId.name())) { From 6513ea31fe02a6c3e040653e7fd9bc2198bc593e Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Wed, 17 Sep 2025 13:13:28 -0700 Subject: [PATCH 02/11] ValidateRunImportedPlateTrigger --- .../assay/plate/data/WellTriggerFactory.java | 39 +++++++++++++++++++ .../labkey/assay/plate/query/WellTable.java | 26 ------------- 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java b/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java index 8b82386f413..007a4efdb67 100644 --- a/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java +++ b/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java @@ -5,6 +5,7 @@ import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.labkey.api.assay.plate.Plate; import org.labkey.api.assay.plate.PlateSet; import org.labkey.api.assay.plate.WellGroup; import org.labkey.api.collections.LongHashMap; @@ -38,12 +39,50 @@ public final class WellTriggerFactory implements TriggerFactory public @NotNull Collection createTrigger(@Nullable Container c, TableInfo table, Map extraContext) { return List.of( + new ValidateRunImportedPlateTrigger(), new EnsureSampleWellTypeTrigger(), new ValidatePrimaryPlateSetUniqueSamplesTrigger(), new ComputeWellGroupsTrigger() ); } + @SuppressWarnings("InnerClassMayBeStatic") + private class ValidateRunImportedPlateTrigger implements Trigger + { + private final Set plateRowIds = new LongHashSet(); + + @Override + public void beforeUpdate( + TableInfo table, + Container c, + User user, + @Nullable Map newRow, + @Nullable Map oldRow, + ValidationException errors, + Map extraContext + ) throws ValidationException + { + if (oldRow == null || errors.hasErrors() || !oldRow.containsKey(WellTable.Column.PlateId.name())) + return; + + Long plateRowId = asLong(oldRow.get(WellTable.Column.PlateId.name())); + if (plateRowId == null) + return; + + if (plateRowIds.contains(plateRowId)) + return; + + plateRowIds.add(plateRowId); + Plate plate = PlateManager.get().getPlate(c, plateRowId); + if (plate == null) + return; + + int runsInUse = PlateManager.get().getRunCountUsingPlate(c, user, plate); + if (runsInUse > 0) + throw new ValidationException(String.format("This %s is used by %d runs and its wells cannot be modified.", plate.isTemplate() ? "Plate template" : "Plate", runsInUse)); + } + } + // When no "Type" is given but "SampleId" is populated, provide 'Sample' as the type private class EnsureSampleWellTypeTrigger implements Trigger { diff --git a/assay/src/org/labkey/assay/plate/query/WellTable.java b/assay/src/org/labkey/assay/plate/query/WellTable.java index 626c1898ac8..100372f1e7a 100644 --- a/assay/src/org/labkey/assay/plate/query/WellTable.java +++ b/assay/src/org/labkey/assay/plate/query/WellTable.java @@ -5,7 +5,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.assay.plate.AssayPlateMetadataService; -import org.labkey.api.assay.plate.Plate; import org.labkey.api.assay.plate.PositionImpl; import org.labkey.api.assay.plate.Well; import org.labkey.api.assay.plate.WellGroup; @@ -67,7 +66,6 @@ import java.util.Set; import java.util.function.Supplier; -import static org.labkey.api.util.IntegerUtils.asInteger; import static org.labkey.api.query.ExprColumn.STR_TABLE_ALIAS; public class WellTable extends SimpleUserSchema.SimpleTable @@ -509,30 +507,6 @@ private Map _select(Container container, Long rowId, boolean all return null; } - @Override - protected Map updateRow( - User user, - Container container, - Map row, - @NotNull Map oldRow, - @Nullable Map configParameters - ) throws InvalidKeyException, ValidationException, QueryUpdateServiceException, SQLException - { - // TODO: Stop doing this check for each row. Instead perform once per plate that is a part of the update. - // enforce no updates if the plate has been imported in an assay run - if (oldRow.containsKey(Column.PlateId.name())) - { - Plate plate = PlateManager.get().getPlate(container, asInteger(oldRow.get(Column.PlateId.name()))); - if (plate != null) - { - int runsInUse = PlateManager.get().getRunCountUsingPlate(container, user, plate); - if (runsInUse > 0) - throw new QueryUpdateServiceException(String.format("This %s is used by %d runs and its wells cannot be modified.", plate.isTemplate() ? "Plate template" : "Plate", runsInUse)); - } - } - return super.updateRow(user, container, row, oldRow, configParameters); - } - @Override protected Map _update( User user, From 000ee4363b1ad0f32fd883ade96ceda7dcaa2566 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Wed, 17 Sep 2025 15:50:44 -0700 Subject: [PATCH 03/11] Plate import event --- .../plate/AssayPlateMetadataServiceImpl.java | 25 +++++++++++------ .../org/labkey/assay/plate/PlateManager.java | 24 ++++++++++------- .../assay/plate/audit/PlateAuditEvent.java | 22 +++++++++++++++ .../assay/plate/audit/PlateAuditProvider.java | 27 +++++++++++++++---- 4 files changed, 75 insertions(+), 23 deletions(-) diff --git a/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java b/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java index f0edf931167..ca5a97004bc 100644 --- a/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java +++ b/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java @@ -164,7 +164,7 @@ public DataIteratorBuilder mergePlateMetadata( @Override public Map apply(Map row) { - // ensure the result data includes a wellLocation field with values like : A1, F12, etc + // ensure the result data includes a wellLocation field with position value (e.g., A1, F12, etc.) Object wellLocation = PropertyService.get().getDomainPropertyValueFromRow(wellLocationProperty, row); if (wellLocation == null) throw new RuntimeValidationException("Imported data must contain a WellLocation column to support plate metadata integration."); @@ -179,11 +179,14 @@ public Map apply(Map row) if (plateIdentifier == null) throw new RuntimeValidationException("Unable to resolve plate identifier for results row (" + rowCounter + ")."); - Plate plate = PlateService.get().getPlate(cf, plateSetId, plateIdentifier); - if (plate == null) - throw new RuntimeValidationException("Unable to resolve the plate \"" + plateIdentifier + "\" for the results row (" + rowCounter + ")."); + plateIdentifierMap.computeIfAbsent(plateIdentifier, k -> { + Plate plate = PlateService.get().getPlate(cf, plateSetId, plateIdentifier); + if (plate == null) + throw new RuntimeValidationException("Unable to resolve the plate \"" + plateIdentifier + "\" for the results row (" + rowCounter + ")."); - plateIdentifierMap.putIfAbsent(plateIdentifier, new Pair<>(plate, new HashMap<>())); + return Pair.of(plate, new HashMap<>()); + }); + Plate plate = plateIdentifierMap.get(plateIdentifier).first; // if the plate identifier is the plate name, we need to make sure it resolves during importRows // so replace it with the plateId (which will be unique) @@ -205,12 +208,14 @@ public Map apply(Map row) } if (!wellSamples.isEmpty()) + { // stash away any samples associated with the plate ExperimentService.get().getExpMaterials(wellSamples).forEach(s -> sampleMap.put(s.getRowId(), s)); + } } PositionImpl well = new PositionImpl(null, String.valueOf(wellLocation)); - // need to adjust the column value to be 0 based to match the template locations + // need to adjust the column value to be 0-based to match the template locations well.setColumn(well.getColumn() - 1); if (!positionToWell.containsKey(well)) @@ -386,7 +391,7 @@ public DataIteratorBuilder mergeReRunData( throw new ExperimentException(String.format("Unable to query the assay results for protocol : %s", protocol.getName())); // The plate identifier is either a row ID or plate ID on incoming data, need to match that when merging existing data. - FieldKey plateFieldKey = FieldKey.fromParts(AssayResultDomainKind.Column.Plate.name()); + FieldKey plateFieldKey = AssayResultDomainKind.Column.Plate.fieldKey(); // Note that in the case where there is a transform script on the assay design, the LK data parsing might not have // found any rows, and we might be deferring to the transform script to do that parsing. This block of code should // be able to proceed in that case by just passing through all run results to the transform script for the run being replaced. @@ -1650,11 +1655,12 @@ public void afterBatchInsert(int rowCount) AssayProtocolSchema schema = _provider.createProtocolSchema(_user, _container, _protocol, null); TableInfo resultsTable = schema.createDataTable(null, false); + boolean isReimport = isExistingRun(); // Re-select any hits that were present in the previous run, this works in conjunction with the code in // mergeReRunData where previous hits are removed for any data unchanged by the new incoming data. At this // point any remaining hits should represent selections we plan to move forward to the new run - if (isExistingRun()) + if (isReimport) { ExpRun prevRun = ExperimentService.get().getExpRun(_context.getReRunId()); if (prevRun != null) @@ -1677,6 +1683,9 @@ public void afterBatchInsert(int rowCount) AssayPlateMetadataService.get().applyHitSelectionCriteria(_container, _user, _protocol, resultsTable, List.of(_run.getRowId())); + // TODO: Check on wiring up of "reimport" column + PlateManager.get().addPlateImportAuditEvents(_container, _user, tx, _plateIdentifierMap.values().stream().toList(), _run, isReimport); + tx.commit(); } catch (Throwable e) diff --git a/assay/src/org/labkey/assay/plate/PlateManager.java b/assay/src/org/labkey/assay/plate/PlateManager.java index f03915c7bca..5cd48fe8dd9 100644 --- a/assay/src/org/labkey/assay/plate/PlateManager.java +++ b/assay/src/org/labkey/assay/plate/PlateManager.java @@ -178,6 +178,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.Lock; +import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -5105,28 +5106,31 @@ public void run() } } - private void addPlateCreatedAuditEvents(Container container, User user, DbScope.Transaction tx, Collection plates, @Nullable String additionalComment) + private void addPlateAuditEvents(User user, Collection plates, Function eventFactory) { if (plates.isEmpty()) return; - List auditEvents = new ArrayList<>(); + List auditEvents = new ArrayList<>(plates.size()); for (Plate plate : plates) - auditEvents.add(PlateAuditProvider.EventFactory.plateCreated(container, tx.getAuditId(), (PlateImpl) plate, additionalComment)); + auditEvents.add(eventFactory.apply((PlateImpl) plate)); AuditLogService.get().addEvents(user, auditEvents, true); } - public void addPlateDeletedAuditEvents(Container container, User user, DbScope.Transaction tx, Collection plates) + private void addPlateCreatedAuditEvents(Container container, User user, DbScope.Transaction tx, Collection plates, @Nullable String additionalComment) { - if (plates.isEmpty()) - return; + addPlateAuditEvents(user, plates, plate -> PlateAuditProvider.EventFactory.plateCreated(container, tx.getAuditId(), plate, additionalComment)); + } - List auditEvents = new ArrayList<>(); - for (Plate plate : plates) - auditEvents.add(PlateAuditProvider.EventFactory.plateDeleted(container, tx.getAuditId(), (PlateImpl) plate)); + public void addPlateDeletedAuditEvents(Container container, User user, DbScope.Transaction tx, Collection plates) + { + addPlateAuditEvents(user, plates, plate -> PlateAuditProvider.EventFactory.plateDeleted(container, tx.getAuditId(), plate)); + } - AuditLogService.get().addEvents(user, auditEvents, true); + public void addPlateImportAuditEvents(Container container, User user, DbScope.Transaction tx, Collection plates, ExpRun run, boolean isReimport) + { + addPlateAuditEvents(user, plates, plate -> PlateAuditProvider.EventFactory.plateImported(container, tx.getAuditId(), plate, run, isReimport)); } public void ensureTransactionAuditId(DbScope.Transaction tx, Container container, User user, QueryService.AuditAction auditAction) diff --git a/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java b/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java index 127043ede7b..eac64c73793 100644 --- a/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java +++ b/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java @@ -23,6 +23,8 @@ public class PlateAuditEvent extends DetailedAuditTypeEvent private Long _plateSetRowId; private Long _plateTypeRowId; private Long _sourcePlateRowId; + private Long _importRunId; + private Boolean _reimport; private boolean _template; public PlateAuditEvent() @@ -118,6 +120,26 @@ public void setTemplate(boolean template) _template = template; } + public Long getImportRunId() + { + return _importRunId; + } + + public void setImportRunId(Long importRunId) + { + _importRunId = importRunId; + } + + public Boolean isReimport() + { + return _reimport; + } + + public void setReimport(Boolean reimport) + { + _reimport = reimport; + } + private static final Set EXCLUDED_PROPERTIES = CaseInsensitiveHashSet.of("ContainerId", PlateTable.Column.DataFileId.name(), "EntityId"); public void setNewRecordMap(Container container, PlateImpl plate) diff --git a/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java b/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java index adbe7018fcc..d7b829904e7 100644 --- a/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java +++ b/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java @@ -10,6 +10,7 @@ import org.labkey.api.data.TableInfo; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.PropertyType; +import org.labkey.api.exp.api.ExpRun; import org.labkey.api.query.FieldKey; import org.labkey.api.query.UserSchema; import org.labkey.assay.plate.PlateImpl; @@ -31,6 +32,8 @@ enum Column PlateRowId, PlateSetRowId, PlateTypeRowId, + Reimport, + ImportRunId, SourcePlateRowId, Template; @@ -53,6 +56,8 @@ public FieldKey fieldKey() defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_COMMENT)); defaultVisibleColumns.add(Column.SourcePlateRowId.fieldKey()); defaultVisibleColumns.add(Column.Template.fieldKey()); + defaultVisibleColumns.add(Column.ImportRunId.fieldKey()); + defaultVisibleColumns.add(Column.Reimport.fieldKey()); } @Override @@ -102,7 +107,8 @@ public Class getEventClass() public enum PlateEventType { CREATE_PLATE("Plate was created.", "Created"), - DELETE_PLATE("Plate was deleted.", "Deleted"); + DELETE_PLATE("Plate was deleted.", "Deleted"), + PLATE_IMPORT("Plate was imported into an assay run.", "Imported"); private final String _actionLabel; private final String _comment; @@ -140,10 +146,12 @@ public PlateAuditDomainKind() // PlateAuditEvent fields _fields.add(createPropertyDescriptor(Column.PlateEventType.name(), PropertyType.STRING)); _fields.add(createPropertyDescriptor(Column.PlateName.name(), PropertyType.STRING)); - _fields.add(createPropertyDescriptor(Column.PlateRowId.name(), PropertyType.INTEGER)); - _fields.add(createPropertyDescriptor(Column.PlateSetRowId.name(), PropertyType.INTEGER)); - _fields.add(createPropertyDescriptor(Column.PlateTypeRowId.name(), PropertyType.INTEGER)); - _fields.add(createPropertyDescriptor(Column.SourcePlateRowId.name(), PropertyType.INTEGER)); + _fields.add(createPropertyDescriptor(Column.PlateRowId.name(), PropertyType.BIGINT)); + _fields.add(createPropertyDescriptor(Column.PlateSetRowId.name(), PropertyType.BIGINT)); + _fields.add(createPropertyDescriptor(Column.PlateTypeRowId.name(), PropertyType.BIGINT)); + _fields.add(createPropertyDescriptor(Column.SourcePlateRowId.name(), PropertyType.BIGINT)); + _fields.add(createPropertyDescriptor(Column.Reimport.name(), PropertyType.BOOLEAN)); + _fields.add(createPropertyDescriptor(Column.ImportRunId.name(), PropertyType.BIGINT)); _fields.add(createPropertyDescriptor(Column.Template.name(), PropertyType.BOOLEAN)); // AbstractAuditTypeProvider fields @@ -200,5 +208,14 @@ public static PlateAuditEvent plateDeleted(Container container, Long transaction return event; } + + public static PlateAuditEvent plateImported(Container container, Long transactionAuditId, PlateImpl plate, ExpRun run, boolean isReimport) + { + var event = new PlateAuditEvent(PlateEventType.PLATE_IMPORT, container, plate, transactionAuditId); + event.setImportRunId(run.getRowId()); + event.setReimport(isReimport); + + return event; + } } } From 61c28401854ba1352a5266dba02920c60c24efcc Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Thu, 18 Sep 2025 15:55:24 -0700 Subject: [PATCH 04/11] Wire up the reimport field --- .../labkey/assay/plate/AssayPlateMetadataServiceImpl.java | 6 ++---- assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java b/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java index ca5a97004bc..a3a8832bf04 100644 --- a/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java +++ b/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java @@ -1529,7 +1529,6 @@ private static class PlateMetadataImportHelper extends SimpleAssayDataImportHelp private final ExpProtocol _protocol; private final AssayProvider _provider; private final AssayRunUploadContext _context; - private DomainProperty _stateProp; public PlateMetadataImportHelper( ExpData data, @@ -1561,7 +1560,7 @@ public void bindAdditionalParameters(Map map, ParameterMapStatem Domain runDomain = _provider.getRunDomain(_protocol); Domain resultDomain = _provider.getResultsDomain(_protocol); - _stateProp = AssayPlateMetadataServiceImpl.getAssayStateProp(resultDomain); + DomainProperty stateProp = AssayPlateMetadataServiceImpl.getAssayStateProp(resultDomain); DomainProperty plateSetProperty = runDomain.getPropertyByName(AssayPlateMetadataService.PLATE_SET_COLUMN_NAME); DomainProperty plateProperty = resultDomain.getPropertyByName(AssayResultDomainKind.Column.Plate.name()); DomainProperty wellLocationProperty = resultDomain.getPropertyByName(AssayResultDomainKind.Column.WellLocation.name()); @@ -1634,7 +1633,7 @@ public void bindAdditionalParameters(Map map, ParameterMapStatem // Validate any data state values on the row. No hit selection / data state processing is done on import // because at this time transform script hit selection is not supported nor is there any intersection // in the re-import case yet. - validateRowDataStates(_container, map, _stateProp); + validateRowDataStates(_container, map, stateProp); } /** @@ -1683,7 +1682,6 @@ public void afterBatchInsert(int rowCount) AssayPlateMetadataService.get().applyHitSelectionCriteria(_container, _user, _protocol, resultsTable, List.of(_run.getRowId())); - // TODO: Check on wiring up of "reimport" column PlateManager.get().addPlateImportAuditEvents(_container, _user, tx, _plateIdentifierMap.values().stream().toList(), _run, isReimport); tx.commit(); diff --git a/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java b/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java index eac64c73793..06af01e9ef3 100644 --- a/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java +++ b/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java @@ -130,7 +130,7 @@ public void setImportRunId(Long importRunId) _importRunId = importRunId; } - public Boolean isReimport() + public Boolean getReimport() { return _reimport; } From d51ed6a32301c10aa6794f2e691185377204d7e3 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Fri, 19 Sep 2025 11:44:21 -0700 Subject: [PATCH 05/11] Plate set audit events --- assay/src/org/labkey/assay/AssayModule.java | 2 + .../data/generator/PlateSetDataGenerator.java | 2 +- .../plate/AssayPlateMetadataServiceImpl.java | 2 +- .../src/org/labkey/assay/plate/PlateImpl.java | 2 +- .../org/labkey/assay/plate/PlateManager.java | 59 +++-- .../labkey/assay/plate/PlateManagerTest.java | 2 +- .../org/labkey/assay/plate/PlateSetImpl.java | 13 + .../assay/plate/audit/PlateAuditProvider.java | 11 +- .../assay/plate/audit/PlateSetAuditEvent.java | 144 +++++++++++ .../plate/audit/PlateSetAuditProvider.java | 233 ++++++++++++++++++ 10 files changed, 441 insertions(+), 29 deletions(-) create mode 100644 assay/src/org/labkey/assay/plate/audit/PlateSetAuditEvent.java create mode 100644 assay/src/org/labkey/assay/plate/audit/PlateSetAuditProvider.java diff --git a/assay/src/org/labkey/assay/AssayModule.java b/assay/src/org/labkey/assay/AssayModule.java index 73486e937f4..df2a12ed944 100644 --- a/assay/src/org/labkey/assay/AssayModule.java +++ b/assay/src/org/labkey/assay/AssayModule.java @@ -89,6 +89,7 @@ import org.labkey.assay.plate.PlateSetDocumentProvider; import org.labkey.assay.plate.TsvPlateLayoutHandler; import org.labkey.assay.plate.audit.PlateAuditProvider; +import org.labkey.assay.plate.audit.PlateSetAuditProvider; import org.labkey.assay.plate.query.PlateSchema; import org.labkey.assay.plate.query.PlateSchemaTest; import org.labkey.assay.plate.query.PlateTypeTable; @@ -205,6 +206,7 @@ protected void startupAfterSpringConfig(ModuleContext moduleContext) return result; }); PlateManager.get().registerLsidHandlers(); + AuditLogService.get().registerAuditType(new PlateSetAuditProvider()); AuditLogService.get().registerAuditType(new PlateAuditProvider()); SearchService ss = SearchService.get(); diff --git a/assay/src/org/labkey/assay/data/generator/PlateSetDataGenerator.java b/assay/src/org/labkey/assay/data/generator/PlateSetDataGenerator.java index b5439d8c12e..3684c7308c8 100644 --- a/assay/src/org/labkey/assay/data/generator/PlateSetDataGenerator.java +++ b/assay/src/org/labkey/assay/data/generator/PlateSetDataGenerator.java @@ -284,7 +284,7 @@ private PlateSet createPlateSet(PlateSetType plateSetType, @Nullable PlateSet pa _plateSetsCreated++; CPUTimer timer = new CPUTimer("Plate"); timer.start(); - PlateSet result = PlateManager.get().createPlateSet(getContainer(), getUser(), plateSet, plates, parentPlateSet != null ? parentPlateSet.getRowId() : null); + PlateSet result = PlateManager.get().createPlateSet(getContainer(), getUser(), plateSet, plates, parentPlateSet != null ? parentPlateSet.getRowId() : null, null); timer.stop(); _plateTimings.add(Double.valueOf((double) timer.getTotalMilliseconds() / plates.size())); diff --git a/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java b/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java index a3a8832bf04..43ddec4b5ba 100644 --- a/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java +++ b/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java @@ -1718,7 +1718,7 @@ public void testGridAnnotations() throws Exception new PlateManager.PlateData(null, plateType.getRowId(), null, null, Collections.emptyList()) ); - PlateSet plateSet = PlateManager.get().createPlateSet(container, user, new PlateSetImpl(), plates, null); + PlateSet plateSet = PlateManager.get().createPlateSet(container, user, new PlateSetImpl(), plates, null, null); List plateSetPlates = PlateManager.get().getPlatesForPlateSet(plateSet); assertEquals("Expected two plates to be created.", 2, plateSetPlates.size()); Plate plate = plateSetPlates.get(0); diff --git a/assay/src/org/labkey/assay/plate/PlateImpl.java b/assay/src/org/labkey/assay/plate/PlateImpl.java index 1d45c39ae6e..5a255cf82ca 100644 --- a/assay/src/org/labkey/assay/plate/PlateImpl.java +++ b/assay/src/org/labkey/assay/plate/PlateImpl.java @@ -796,7 +796,7 @@ public static class TestCase extends Assert public void setup() throws Exception { PlateSetImpl plateSet = new PlateSetImpl(); - _plateSet = PlateManager.get().createPlateSet(JunitUtil.getTestContainer(), TestContext.get().getUser(), plateSet, null, null); + _plateSet = PlateManager.get().createPlateSet(JunitUtil.getTestContainer(), TestContext.get().getUser(), plateSet, null, null, null); } @Test diff --git a/assay/src/org/labkey/assay/plate/PlateManager.java b/assay/src/org/labkey/assay/plate/PlateManager.java index 5cd48fe8dd9..855f5b2acf7 100644 --- a/assay/src/org/labkey/assay/plate/PlateManager.java +++ b/assay/src/org/labkey/assay/plate/PlateManager.java @@ -137,6 +137,8 @@ import org.labkey.assay.TsvAssayProvider; import org.labkey.assay.plate.audit.PlateAuditEvent; import org.labkey.assay.plate.audit.PlateAuditProvider; +import org.labkey.assay.plate.audit.PlateSetAuditEvent; +import org.labkey.assay.plate.audit.PlateSetAuditProvider; import org.labkey.assay.plate.data.PlateMapExcelWriter; import org.labkey.assay.plate.data.WellData; import org.labkey.assay.plate.layout.LayoutEngine; @@ -1162,7 +1164,7 @@ private long savePlateImpl( PlateSetImpl plateSet = new PlateSetImpl(); plateSet.setTemplate(plate.isTemplate()); - plate.setPlateSet(createPlateSet(container, user, plateSet, null, null)); + plate.setPlateSet(createPlateSet(container, user, plateSet, null, null, null)); } Map plateRow = ObjectFactory.Registry.getFactory(PlateBean.class).toMap(PlateBean.from(plate, false), new ArrayListMap<>()); @@ -2298,25 +2300,28 @@ public void indexPlates(SearchService.TaskIndexingQueue queue, @Nullable Date mo public void indexPlateSet(Container container, Long plateSetRowId) { PlateSet plateSet = getPlateSet(container, plateSetRowId); - if (plateSet == null) return; - indexPlateSet(SearchService.get().defaultTask().getQueue(container, SearchService.PRIORITY.modified), plateSet); + indexPlateSet(plateSet); + } + + private void indexPlateSet(@NotNull PlateSet plateSet) + { + indexPlateSet(SearchService.get().defaultTask().getQueue(plateSet.getContainer(), SearchService.PRIORITY.modified), plateSet); } private void indexPlateSet(SearchService.TaskIndexingQueue queue, @NotNull PlateSet plateSet) { - WebdavResource resource = PlateSetDocumentProvider.createDocument(plateSet); - queue.addResource(resource); + queue.addResource(PlateSetDocumentProvider.createDocument(plateSet)); } public void indexPlateSets(SearchService.TaskIndexingQueue queue, @Nullable Date modifiedSince) { - for (PlateSet plateset : getPlateSets(queue.getContainer())) + for (PlateSet plateSet : getPlateSets(queue.getContainer())) { - if (modifiedSince == null || modifiedSince.before(((PlateSetImpl) plateset).getModified())) - indexPlateSet(queue, plateset); + if (modifiedSince == null || modifiedSince.before(((PlateSetImpl) plateSet).getModified())) + indexPlateSet(queue, plateSet); } } @@ -2848,7 +2853,8 @@ public PlateSetImpl createPlateSet( User user, @NotNull PlateSetImpl plateSet, @Nullable List plates, - @Nullable Long parentPlateSetId + @Nullable Long parentPlateSetId, + @Nullable String additionalAuditComment ) throws Exception { if (!container.hasPermission(user, InsertPermission.class)) @@ -2895,14 +2901,25 @@ public PlateSetImpl createPlateSet( if (plates != null) addPlatesToPlateSet(container, user, plateSetId, plateSet.isTemplate(), plates, String.format("Added during creation of plate set \"%s\".", plateSet.getName())); - plateSet = (PlateSetImpl) getPlateSet(container, plateSetId); - tx.commit(); - } + PlateSetImpl newPlateSet = (PlateSetImpl) requirePlateSet(container, plateSetId, "Failed to create plate set."); - if (plateSet != null) - indexPlateSet(SearchService.get().defaultTask().getQueue(container, SearchService.PRIORITY.modified), plateSet); + // Set transient parent plate set property for auditing + if (parentPlateSet != null) + newPlateSet.setParentPlateSetId(parentPlateSet.getRowId()); - return plateSet; + // Audit plate set creation + { + int plateCount = plates != null ? plates.size() : 0; + additionalAuditComment = String.format("%s Initially contains %d %s.", StringUtils.trimToEmpty(additionalAuditComment), plateCount, plateCount == 1 ? "plate" : "plates"); + PlateSetAuditEvent auditEvent = PlateSetAuditProvider.EventFactory.plateSetCreated(container, tx.getAuditId(), newPlateSet, additionalAuditComment); + AuditLogService.get().addEvent(user, auditEvent); + } + + tx.addCommitTask(() -> indexPlateSet(newPlateSet), DbScope.CommitTaskOption.POSTCOMMIT); + tx.commit(); + + return newPlateSet; + } } public PlateSet createOrAddToPlateSet(Container container, User user, CreatePlateSetOptions options) throws Exception @@ -2938,7 +2955,7 @@ public PlateSet createOrAddToPlateSet(Container container, User user, CreatePlat // Create a new plate set if (targetPlateSet.isNew()) - return createPlateSet(container, user, targetPlateSet, plates, options.getParentPlateSetId()); + return createPlateSet(container, user, targetPlateSet, plates, options.getParentPlateSetId(), null); // Update an existing plate set addPlatesToPlateSet(container, user, targetPlateSet.getRowId(), targetPlateSet.isTemplate(), plates, null); @@ -2954,13 +2971,12 @@ private PlateSet replatePlateSet( ) throws Exception { PlateSetImpl parentPlateSet = (PlateSetImpl) requirePlateSet(container, sourcePlateSetRowId, null); - Long parentId = parentPlateSet.isStandalone() ? null : parentPlateSet.getRowId(); try (DbScope.Transaction tx = ensureTransaction()) { ensureTransactionAuditId(tx, container, user, QueryService.AuditAction.INSERT); - PlateSet newPlateSet = createPlateSet(container, user, targetPlateSet, null, parentId); + PlateSet newPlateSet = createPlateSet(container, user, targetPlateSet, null, parentId, String.format("Re-plated from plate set \"%s\".", parentPlateSet.getName())); for (Plate plate : parentPlateSet.getPlates()) copyPlate(container, user, plate.getRowId(), false, newPlateSet.getRowId(), null, null, true); @@ -3024,6 +3040,8 @@ public void archive(Container container, User user, @Nullable List plateSe try (DbScope.Transaction tx = ensureTransaction()) { + ensureTransactionAuditId(tx, container, user, QueryService.AuditAction.UPDATE); + if (archivingPlates) { archive(container, user, AssayDbSchema.getInstance().getTableInfoPlate(), "plates", plateIds, archive); @@ -3034,6 +3052,9 @@ public void archive(Container container, User user, @Nullable List plateSe { archive(container, user, AssayDbSchema.getInstance().getTableInfoPlateSet(), "plate sets", plateSetIds, archive); tx.addCommitTask(() -> clearPlateSetCache(container, plateSetIds), DbScope.CommitTaskOption.POSTCOMMIT); + + List auditEvents = PlateSetAuditProvider.EventFactory.plateSetsArchived(container, tx.getAuditId(), plateSetIds, archive); + AuditLogService.get().addEvents(user, auditEvents, true); } tx.commit(); @@ -4520,7 +4541,7 @@ public record ReformatResult( PlateSet parentPlateSet = resolveParentPlateSet(container, user, options, sourcePlateSet); Long parentPlateSetId = parentPlateSet != null ? parentPlateSet.getRowId() : null; - PlateSet newPlateSet = createPlateSet(container, user, targetPlateSet, plateData, parentPlateSetId); + PlateSet newPlateSet = createPlateSet(container, user, targetPlateSet, plateData, parentPlateSetId, "Created via reformat."); plateSetRowId = newPlateSet.getRowId(); plateSetName = newPlateSet.getName(); newPlates = newPlateSet.getPlates(); diff --git a/assay/src/org/labkey/assay/plate/PlateManagerTest.java b/assay/src/org/labkey/assay/plate/PlateManagerTest.java index 800098da2df..df01525c075 100644 --- a/assay/src/org/labkey/assay/plate/PlateManagerTest.java +++ b/assay/src/org/labkey/assay/plate/PlateManagerTest.java @@ -2008,7 +2008,7 @@ private static PlateSetImpl createPlateSet( @Nullable Long parentPlateSetId ) throws Exception { - return PlateManager.get().createPlateSet(container, user, plateSet, plates, parentPlateSetId); + return PlateManager.get().createPlateSet(container, user, plateSet, plates, parentPlateSetId, null); } private void assertCreatePlateSetThrows( diff --git a/assay/src/org/labkey/assay/plate/PlateSetImpl.java b/assay/src/org/labkey/assay/plate/PlateSetImpl.java index 646207a4635..e17b57a09a7 100644 --- a/assay/src/org/labkey/assay/plate/PlateSetImpl.java +++ b/assay/src/org/labkey/assay/plate/PlateSetImpl.java @@ -30,6 +30,7 @@ public class PlateSetImpl extends Entity implements PlateSet private String _description; private String _name; private String _plateSetId; + private transient Long _parentPlateSetId; private Long _primaryPlateSetId; private Long _rootPlateSetId; private Long _rowId; @@ -196,6 +197,18 @@ public void setDescription(String description) _description = description; } + @JsonIgnore + public Long getParentPlateSetId() + { + return _parentPlateSetId; + } + + @JsonIgnore + public void setParentPlateSetId(Long parentPlateSetId) + { + _parentPlateSetId = parentPlateSetId; + } + public Long getPrimaryPlateSetId() { return _primaryPlateSetId; diff --git a/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java b/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java index d7b829904e7..20c1240eacd 100644 --- a/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java +++ b/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java @@ -60,6 +60,11 @@ public FieldKey fieldKey() defaultVisibleColumns.add(Column.Reimport.fieldKey()); } + public PlateAuditProvider() + { + super(new PlateAuditDomainKind()); + } + @Override public List getDefaultVisibleColumns() { @@ -75,11 +80,6 @@ public TableInfo createTableInfo(UserSchema userSchema, ContainerFilter cf) return table; } - public PlateAuditProvider() - { - super(new PlateAuditDomainKind()); - } - @Override public String getEventName() { @@ -134,7 +134,6 @@ public static class PlateAuditDomainKind extends AbstractAuditDomainKind { private static final String NAME = "PlateAuditDomain"; private static final String NAMESPACE_PREFIX = "Audit-" + NAME; - private final Set fields; public PlateAuditDomainKind() diff --git a/assay/src/org/labkey/assay/plate/audit/PlateSetAuditEvent.java b/assay/src/org/labkey/assay/plate/audit/PlateSetAuditEvent.java new file mode 100644 index 00000000000..4398a8bef3b --- /dev/null +++ b/assay/src/org/labkey/assay/plate/audit/PlateSetAuditEvent.java @@ -0,0 +1,144 @@ +package org.labkey.assay.plate.audit; + +import org.labkey.api.audit.AbstractAuditTypeProvider; +import org.labkey.api.audit.DetailedAuditTypeEvent; +import org.labkey.api.collections.CaseInsensitiveHashMap; +import org.labkey.api.collections.CaseInsensitiveHashSet; +import org.labkey.api.data.Container; +import org.labkey.api.data.ObjectFactory; +import org.labkey.assay.plate.PlateSetImpl; + +import java.util.Map; +import java.util.Set; + +import static org.labkey.assay.plate.audit.PlateSetAuditProvider.EVENT_NAME; + +public class PlateSetAuditEvent extends DetailedAuditTypeEvent +{ + private Boolean _archived; + private String _plateSetEventType; + private String _plateSetName; + private Long _plateSetRowId; + private String _plateSetType; + private Long _parentPlateSetRowId; + private Long _primaryPlateSetRowId; + private Long _rootPlateSetRowId; + + public PlateSetAuditEvent() + { + super(); + } + + public PlateSetAuditEvent( + PlateSetAuditProvider.PlateSetEventType eventType, + Container container, + PlateSetImpl plateSet, + Long transactionAuditId + ) + { + super(EVENT_NAME, container, eventType.getComment()); + setArchived(plateSet.isArchived()); + setPlateSetEventType(eventType.name()); + setPlateSetName(plateSet.getName()); + setPlateSetRowId(plateSet.getRowId()); + setPlateSetType(plateSet.getType().name()); + setPrimaryPlateSetRowId(plateSet.getPrimaryPlateSetId()); + setParentPlateSetRowId(plateSet.getParentPlateSetId()); + setRootPlateSetRowId(plateSet.getRootPlateSetId()); + setTransactionId(transactionAuditId); + } + + public Boolean getArchived() + { + return _archived; + } + + public void setArchived(Boolean archived) + { + _archived = archived; + } + + public String getPlateSetEventType() + { + return _plateSetEventType; + } + + public void setPlateSetEventType(String plateSetEventType) + { + _plateSetEventType = plateSetEventType; + } + + public String getPlateSetName() + { + return _plateSetName; + } + + public void setPlateSetName(String plateSetName) + { + _plateSetName = plateSetName; + } + + public Long getPlateSetRowId() + { + return _plateSetRowId; + } + + public void setPlateSetRowId(Long plateSetRowId) + { + _plateSetRowId = plateSetRowId; + } + + public String getPlateSetType() + { + return _plateSetType; + } + + public void setPlateSetType(String plateSetType) + { + _plateSetType = plateSetType; + } + + public Long getParentPlateSetRowId() + { + return _parentPlateSetRowId; + } + + public void setParentPlateSetRowId(Long parentPlateSetRowId) + { + _parentPlateSetRowId = parentPlateSetRowId; + } + + public Long getPrimaryPlateSetRowId() + { + return _primaryPlateSetRowId; + } + + public void setPrimaryPlateSetRowId(Long primaryPlateSetRowId) + { + _primaryPlateSetRowId = primaryPlateSetRowId; + } + + public Long getRootPlateSetRowId() + { + return _rootPlateSetRowId; + } + + public void setRootPlateSetRowId(Long rootPlateSetRowId) + { + _rootPlateSetRowId = rootPlateSetRowId; + } + + private static final Set EXCLUDED_PROPERTIES = CaseInsensitiveHashSet.of( + "Assay", "Container", "ContainerId", "ContainerName", "ExpObject", + "Folder", "Full", "LSIDNamespacePrefix", "New", "ParentPlateSetId", "PlateCount", + "Plates", "Primary", "QueryRowReference", "Standalone"); + + public void setNewRecordMap(Container container, PlateSetImpl plateSet) + { + Map plateSetRow = ObjectFactory.Registry.getFactory(PlateSetImpl.class).toMap(plateSet, new CaseInsensitiveHashMap<>()); + EXCLUDED_PROPERTIES.forEach(plateSetRow::remove); + + var newRecordMap = AbstractAuditTypeProvider.encodeForDataMap(plateSetRow); + setNewRecordMap(newRecordMap, container); + } +} diff --git a/assay/src/org/labkey/assay/plate/audit/PlateSetAuditProvider.java b/assay/src/org/labkey/assay/plate/audit/PlateSetAuditProvider.java new file mode 100644 index 00000000000..91e6df612ef --- /dev/null +++ b/assay/src/org/labkey/assay/plate/audit/PlateSetAuditProvider.java @@ -0,0 +1,233 @@ +package org.labkey.assay.plate.audit; + +import org.jetbrains.annotations.Nullable; +import org.labkey.api.audit.AbstractAuditTypeProvider; +import org.labkey.api.audit.AuditTypeEvent; +import org.labkey.api.audit.query.AbstractAuditDomainKind; +import org.labkey.api.audit.query.DefaultAuditTypeTable; +import org.labkey.api.collections.CaseInsensitiveHashMap; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.TableInfo; +import org.labkey.api.exp.PropertyDescriptor; +import org.labkey.api.exp.PropertyType; +import org.labkey.api.query.FieldKey; +import org.labkey.api.query.UserSchema; +import org.labkey.api.query.ValidationException; +import org.labkey.assay.plate.PlateManager; +import org.labkey.assay.plate.PlateSetImpl; +import org.labkey.assay.plate.query.PlateTable; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +public class PlateSetAuditProvider extends AbstractAuditTypeProvider +{ + public static final String EVENT_NAME = "PlateSetEvent"; + + enum Column + { + Archived, + PlateSetEventType, + PlateSetName, + PlateSetRowId, + PlateSetType, + ParentPlateSetRowId, + PrimaryPlateSetRowId, + RootPlateSetRowId; + + private final FieldKey _fieldKey = FieldKey.fromParts(name()); + + public FieldKey fieldKey() + { + return _fieldKey; + } + } + + static final List defaultVisibleColumns = new ArrayList<>(); + + static + { + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CREATED)); + defaultVisibleColumns.add(Column.PlateSetRowId.fieldKey()); + defaultVisibleColumns.add(Column.PlateSetType.fieldKey()); + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_COMMENT)); + defaultVisibleColumns.add(Column.ParentPlateSetRowId.fieldKey()); + defaultVisibleColumns.add(Column.PrimaryPlateSetRowId.fieldKey()); + defaultVisibleColumns.add(Column.RootPlateSetRowId.fieldKey()); + defaultVisibleColumns.add(Column.Archived.fieldKey()); + } + + public PlateSetAuditProvider() + { + super(new PlateSetAuditDomainKind()); + } + + @Override + public List getDefaultVisibleColumns() + { + return defaultVisibleColumns; + } + + @Override + public TableInfo createTableInfo(UserSchema userSchema, ContainerFilter cf) + { + DefaultAuditTypeTable table = new DefaultAuditTypeTable(this, createStorageTableInfo(), userSchema, cf, getDefaultVisibleColumns()); + appendValueMapColumns(table, getEventName()); + + return table; + } + + @Override + public String getEventName() + { + return EVENT_NAME; + } + + @Override + public String getLabel() + { + return "Plate set events"; + } + + @Override + public String getDescription() + { + return "Events related to plate sets"; + } + + @Override + public Class getEventClass() + { + return (Class) PlateSetAuditEvent.class; + } + + public enum PlateSetEventType + { + ARCHIVE_PLATE_SET("Plate set was archived.", "Archived"), + CREATE_PLATE_SET("Plate set was created.", "Created"), + RESTORE_PLATE_SET("Plate set was restored from the archive.", "Restored"); + + private final String _actionLabel; + private final String _comment; + + PlateSetEventType(String comment, String actionLabel) + { + _comment = comment; + _actionLabel = actionLabel; + } + + public String getActionLabel() + { + return _actionLabel; + } + + public String getComment() + { + return _comment; + } + } + + public static class PlateSetAuditDomainKind extends AbstractAuditDomainKind + { + private static final String NAME = "PlateSetAuditDomain"; + private static final String NAMESPACE_PREFIX = "Audit-" + NAME; + private final Set fields; + + public PlateSetAuditDomainKind() + { + super(EVENT_NAME); + + Set _fields = new LinkedHashSet<>(); + + // PlateSetAuditEvent fields + _fields.add(createPropertyDescriptor(Column.PlateSetEventType.name(), PropertyType.STRING)); + _fields.add(createPropertyDescriptor(Column.PlateSetName.name(), PropertyType.STRING)); + _fields.add(createPropertyDescriptor(Column.PlateSetRowId.name(), PropertyType.BIGINT)); + _fields.add(createPropertyDescriptor(Column.PlateSetType.name(), PropertyType.STRING)); + _fields.add(createPropertyDescriptor(Column.ParentPlateSetRowId.name(), PropertyType.BIGINT)); + _fields.add(createPropertyDescriptor(Column.PrimaryPlateSetRowId.name(), PropertyType.BIGINT)); + _fields.add(createPropertyDescriptor(Column.RootPlateSetRowId.name(), PropertyType.BIGINT)); + _fields.add(createPropertyDescriptor(Column.Archived.name(), PropertyType.BOOLEAN)); + + // AbstractAuditTypeProvider fields + _fields.add(createPropertyDescriptor(COLUMN_NAME_COMMENT, PropertyType.STRING)); + _fields.add(createPropertyDescriptor(COLUMN_NAME_TRANSACTION_ID, PropertyType.BIGINT)); + _fields.add(createPropertyDescriptor(COLUMN_NAME_USER_COMMENT, PropertyType.STRING)); + _fields.add(createOldDataMapPropertyDescriptor()); + _fields.add(createNewDataMapPropertyDescriptor()); + + fields = Collections.unmodifiableSet(_fields); + } + + @Override + protected String getNamespacePrefix() + { + return NAMESPACE_PREFIX; + } + + @Override + public Set getProperties() + { + return fields; + } + + @Override + public String getKindName() + { + return NAME; + } + } + + public static class EventFactory + { + public static PlateSetAuditEvent plateSetCreated( + Container container, + Long transactionAuditId, + PlateSetImpl plateSet, + @Nullable String additionalComment + ) + { + var event = new PlateSetAuditEvent(PlateSetEventType.CREATE_PLATE_SET, container, plateSet, transactionAuditId); + event.setNewRecordMap(container, plateSet); + + if (additionalComment != null) + event.setComment(event.getComment() + " " + additionalComment); + + return event; + } + + public static List plateSetsArchived( + Container container, + Long transactionAuditId, + List plateSetIds, + boolean archive + ) throws ValidationException + { + if (plateSetIds.isEmpty()) + return Collections.emptyList(); + + var events = new ArrayList(plateSetIds.size()); + var eventType = archive ? PlateSetEventType.ARCHIVE_PLATE_SET : PlateSetEventType.RESTORE_PLATE_SET; + + for (var plateSetId : plateSetIds) + { + var plateSet = (PlateSetImpl) PlateManager.get().getPlateSet(ContainerFilter.getUnsafeEverythingFilter(), plateSetId); + if (plateSet == null) + throw new ValidationException(String.format("Failed to audit archive change for plate set %d. Plate set not found.", plateSetId)); + + plateSet.setArchived(archive); + + var event = new PlateSetAuditEvent(eventType, container, plateSet, transactionAuditId); + event.setOldRecordMap(AbstractAuditTypeProvider.encodeForDataMap(CaseInsensitiveHashMap.of(PlateTable.Column.Archived.name(), String.valueOf(!archive)))); + event.setNewRecordMap(AbstractAuditTypeProvider.encodeForDataMap(CaseInsensitiveHashMap.of(PlateTable.Column.Archived.name(), String.valueOf(archive)))); + events.add(event); + } + + return events; + } + } +} From ad347d40bf25823735fb31908c37848e1441e5ad Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Mon, 22 Sep 2025 14:43:28 -0700 Subject: [PATCH 06/11] nits --- api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java | 1 - 1 file changed, 1 deletion(-) diff --git a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java index a12dfcda858..cd6972adf59 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java +++ b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java @@ -22,7 +22,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.json.JSONArray; -import org.labkey.api.action.ApiUsageException; import org.labkey.api.assay.plate.AssayPlateMetadataService; import org.labkey.api.assay.sample.AssaySampleLookupContext; import org.labkey.api.collections.CaseInsensitiveHashMap; From b515e0cea80998beebbfcd2e81b1bf5eab21ecaf Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Wed, 24 Sep 2025 13:05:37 -0700 Subject: [PATCH 07/11] Review feedback --- .../src/org/labkey/assay/PlateController.java | 1 - .../data/generator/PlateSetDataGenerator.java | 2 +- .../org/labkey/assay/plate/PlateManager.java | 36 ++++++++++--------- .../labkey/assay/plate/PlateManagerTest.java | 6 ++-- .../assay/plate/audit/PlateAuditEvent.java | 19 +++++----- .../assay/plate/data/PlateMapExcelWriter.java | 2 +- .../org/labkey/assay/plate/data/WellData.java | 2 +- .../assay/plate/data/WellTriggerFactory.java | 6 ++-- .../labkey/assay/plate/query/WellTable.java | 11 ++++-- 9 files changed, 46 insertions(+), 39 deletions(-) diff --git a/assay/src/org/labkey/assay/PlateController.java b/assay/src/org/labkey/assay/PlateController.java index de4374282d8..7bffbc4aeac 100644 --- a/assay/src/org/labkey/assay/PlateController.java +++ b/assay/src/org/labkey/assay/PlateController.java @@ -15,7 +15,6 @@ */ package org.labkey.assay; -import org.apache.commons.lang3.ArrayUtils; import org.apache.logging.log4j.Logger; import org.json.JSONArray; import org.json.JSONObject; diff --git a/assay/src/org/labkey/assay/data/generator/PlateSetDataGenerator.java b/assay/src/org/labkey/assay/data/generator/PlateSetDataGenerator.java index 3684c7308c8..50cafb7d961 100644 --- a/assay/src/org/labkey/assay/data/generator/PlateSetDataGenerator.java +++ b/assay/src/org/labkey/assay/data/generator/PlateSetDataGenerator.java @@ -267,7 +267,7 @@ private PlateSet createPlateSet(PlateSetType plateSetType, @Nullable PlateSet pa rowMap.put(WellTable.WELL_LOCATION, position.getDescription()); rowMap.put(WellTable.Column.Type.name(), WellGroup.Type.SAMPLE.name()); - rowMap.put(WellTable.Column.SampleId.name(), ids.get(sampleIdx++)); + rowMap.put(WellTable.Column.SampleID.name(), ids.get(sampleIdx++)); for (String name : wellProperties) { diff --git a/assay/src/org/labkey/assay/plate/PlateManager.java b/assay/src/org/labkey/assay/plate/PlateManager.java index 855f5b2acf7..41c5ce9f565 100644 --- a/assay/src/org/labkey/assay/plate/PlateManager.java +++ b/assay/src/org/labkey/assay/plate/PlateManager.java @@ -348,6 +348,8 @@ public List getWellGroupTypes() ((PlateImpl) plate).setPlateSet(plateSet); } + // Intentionally passing skipAudit=true, and not the passed in value for skipAudit, + // as this method does its own creation of audit events. long plateRowId = save(container, user, plate, data, true); plate = getPlate(container, plateRowId); if (plate == null) @@ -396,11 +398,11 @@ private List getDefaultFieldsForPlateSet(@NotNull Plate plate, fields.add(new PlateCustomField(WellTable.Column.Type.fieldKey())); fields.add(new PlateCustomField(WellTable.Column.WellGroup.fieldKey())); fields.add(new PlateCustomField(WellTable.Column.ReplicateGroup.fieldKey())); - fields.add(new PlateCustomField(WellTable.Column.SampleId.fieldKey())); + fields.add(new PlateCustomField(WellTable.Column.SampleID.fieldKey())); } } else if (plateSet.isPrimary()) - fields.add(new PlateCustomField(WellTable.Column.SampleId.fieldKey())); + fields.add(new PlateCustomField(WellTable.Column.SampleID.fieldKey())); else if (plateSet.isTemplate()) fields = templateFields; @@ -495,7 +497,7 @@ public List getMetadataColumns(@NotNull PlateSet plateSet, Container c for (PlateCustomField field : plate.getCustomFields()) { - if (WellTable.Column.SampleId.fieldKey().equals(field.getFieldKey())) + if (WellTable.Column.SampleID.fieldKey().equals(field.getFieldKey())) continue; FieldKey lookupFk = displayColumns.get(field.getPropertyURI()); if (lookupFk != null) @@ -1862,7 +1864,7 @@ private void copyWellData(User user, @NotNull Plate source, @NotNull Plate copy, var lsidColumn = WellTable.Column.Lsid.name(); var lsidFieldKey = WellTable.Column.Lsid.fieldKey(); var rowIdColumn = WellTable.Column.RowId.name(); - var sampleIdColumn = WellTable.Column.SampleId.name(); + var sampleIdColumn = WellTable.Column.SampleID.name(); var sourceWellData = new TableSelector(wellTable, Set.of(rowIdColumn, lsidColumn, sampleIdColumn), new SimpleFilter(WellTable.Column.PlateId.fieldKey(), source.getRowId()), new Sort(rowIdColumn)).getMapArray(); var copyWellData = new TableSelector(wellTable, Set.of(rowIdColumn, lsidColumn), new SimpleFilter(WellTable.Column.PlateId.fieldKey(), copy.getRowId()), new Sort(rowIdColumn)).getMapArray(); @@ -2009,7 +2011,7 @@ else if (isDuplicatePlateName(container, user, name, destinationPlateSet)) if (copyAsTemplate) { newPlate.setTemplate(true); - newFields.removeIf((f) -> WellTable.Column.SampleId.fieldKey().equals(f.getFieldKey())); + newFields.removeIf((f) -> WellTable.Column.SampleID.fieldKey().equals(f.getFieldKey())); } else newPlate.setPlateSet(destinationPlateSet); @@ -2476,7 +2478,7 @@ public Container getPlateMetadataDomainContainer(Container container) fields.add(new PlateCustomField(wellTable.getColumn(WellTable.Column.Type.fieldKey()))); fields.add(new PlateCustomField(wellTable.getColumn(WellTable.Column.WellGroup.fieldKey()))); fields.add(new PlateCustomField(wellTable.getColumn(WellTable.Column.ReplicateGroup.fieldKey()))); - fields.add(new PlateCustomField(wellTable.getColumn(WellTable.Column.SampleId.fieldKey()))); + fields.add(new PlateCustomField(wellTable.getColumn(WellTable.Column.SampleID.fieldKey()))); } Domain metadataDomain = getPlateMetadataDomain(container, user); @@ -2619,7 +2621,7 @@ private List sortCustomFields(List fields) order.put(WellTable.Column.Type.fieldKey(), 0); order.put(WellTable.Column.WellGroup.fieldKey(), 1); order.put(WellTable.Column.ReplicateGroup.fieldKey(), 2); - order.put(WellTable.Column.SampleId.fieldKey(), 3); + order.put(WellTable.Column.SampleID.fieldKey(), 3); Comparator nameComparator = Comparator.comparing((k) -> k.getName().toLowerCase(), Comparator.nullsLast(String::compareTo)); fields.sort((f1, f2) -> { @@ -3571,7 +3573,7 @@ Pair>> getWellSampleData( int colIdx = iterateByColumn ? outerIdx : innerIdx; wellSampleDataForPlate.add(CaseInsensitiveHashMap.of( - WellTable.Column.SampleId.name(), sampleIds.get(sampleIdsCounter), + WellTable.Column.SampleID.name(), sampleIds.get(sampleIdsCounter), WellTable.Column.Type.name(), SAMPLE.name(), WELL_LOCATION, createPosition(c, rowIdx, colIdx).getDescription() )); @@ -3777,7 +3779,7 @@ public List getInstrumentInstructions(long plateSetId, List private List getPlateExportFieldKeys(Plate plate, boolean isMapView) { List fieldKeys = new ArrayList<>(); - fieldKeys.add(FieldKey.fromParts(WellTable.Column.SampleId.name(), "Name")); + fieldKeys.add(FieldKey.fromParts(WellTable.Column.SampleID.name(), "Name")); if (isMapView) { @@ -3795,7 +3797,7 @@ private List getPlateExportFieldKeys(Plate plate, boolean isMapView) if (isMapView) { Set excludedColumns = Set.of( - WellTable.Column.SampleId.fieldKey(), + WellTable.Column.SampleID.fieldKey(), WellTable.Column.Type.fieldKey(), WellTable.Column.WellGroup.fieldKey(), WellTable.Column.ReplicateGroup.fieldKey() @@ -3835,7 +3837,7 @@ private List getPlateDisplayColumns(QueryView queryView) // Filter on isQueryColumn, so we don't get the details or update columns return dataRegion.getDisplayColumns().stream() .filter(DisplayColumn::isQueryColumn) - .filter(col -> !col.getName().equalsIgnoreCase(WellTable.Column.SampleId.name())) + .filter(col -> !col.getName().equalsIgnoreCase(WellTable.Column.SampleID.name())) .toList(); } @@ -3855,7 +3857,7 @@ public List exportPlateData(Container c, User user, ContainerFil QueryView plateQueryView = getPlateQueryView(c, user, cf, plate, false); List displayColumns = getPlateDisplayColumns(plateQueryView); PlateFileBytes plateFileBytes = new PlateFileBytes(plate.getName(), new ByteArrayOutputStream()); - FieldKey sampleIdNameFieldKey = FieldKey.fromParts(WellTable.Column.SampleId.name(), "Name"); + FieldKey sampleIdNameFieldKey = FieldKey.fromParts(WellTable.Column.SampleID.name(), "Name"); try (TSVGridWriter writer = new TSVGridWriter(plateQueryView::getResults, displayColumns, Collections.singletonMap(sampleIdNameFieldKey.toString(), "Sample ID"))) { @@ -3906,7 +3908,7 @@ public List getWellData(Container container, User user, long plateRowI columns.add(WellTable.Column.WellGroup.name()); columns.add(WellTable.Column.ReplicateGroup.name()); if (includeSamples) - columns.add(WellTable.Column.SampleId.name()); + columns.add(WellTable.Column.SampleID.name()); var wellTable = getWellTable(container, user, getPlateLookupContainerFilter(container, user)); var filter = new SimpleFilter(WellTable.Column.PlateId.fieldKey(), plateRowId); @@ -4174,7 +4176,7 @@ private void validateWells(Plate plate) throws ValidationException "Well %s must specify a \"%s\" when a \"%s\" is specified on plate \"%s\".", position.getDescription(), WellTable.Column.Type.name(), - WellTable.Column.SampleId.name(), + WellTable.Column.SampleID.name(), plate.getName() )); } @@ -4323,7 +4325,7 @@ private String getSampleGroupLabKeySql(@NotNull Long plateSetRowId, boolean incl columnNames.add(WellTable.Column.Type.name()); columnNames.add(WellTable.Column.WellGroup.name()); if (includeSampleId) - columnNames.add(WellTable.Column.SampleId.name()); + columnNames.add(WellTable.Column.SampleID.name()); String columns = columnNames.stream().map(LabKeySql::quoteIdentifier).collect(Collectors.joining(", ")); String wellTypes = StringUtils.join( @@ -4608,7 +4610,7 @@ public record ReformatResult( for (Map row : plateData.data) { - Long sampleId = MapUtils.getLong(row, WellTable.Column.SampleId.name()); + Long sampleId = MapUtils.getLong(row, WellTable.Column.SampleID.name()); if (sampleId != null) { wellsFilled++; @@ -4656,7 +4658,7 @@ public record ReformatResult( for (Map row : plate.data) { - Long sampleId = MapUtils.getLong(row,WellTable.Column.SampleId.name()); + Long sampleId = MapUtils.getLong(row,WellTable.Column.SampleID.name()); if (sampleId != null) { wellsFilled++; diff --git a/assay/src/org/labkey/assay/plate/PlateManagerTest.java b/assay/src/org/labkey/assay/plate/PlateManagerTest.java index df01525c075..55be8d02938 100644 --- a/assay/src/org/labkey/assay/plate/PlateManagerTest.java +++ b/assay/src/org/labkey/assay/plate/PlateManagerTest.java @@ -1465,7 +1465,7 @@ public void testReformatArrayFromTemplate() throws Exception int t = 0; while (r.next()) { - var sampleId = r.getInt(FieldKey.fromParts(WellTable.Column.SampleId.name())); + var sampleId = r.getInt(FieldKey.fromParts(WellTable.Column.SampleID.name())); var wellPosition = r.getString(FieldKey.fromParts("position")); switch (wellPosition) @@ -1500,7 +1500,7 @@ public void testReformatArrayFromTemplate() throws Exception int t = 0; while (r.next()) { - var sampleId = r.getInt(FieldKey.fromParts(WellTable.Column.SampleId.name())); + var sampleId = r.getInt(FieldKey.fromParts(WellTable.Column.SampleID.name())); var wellPosition = r.getString(FieldKey.fromParts("position")); switch (wellPosition) @@ -1534,7 +1534,7 @@ public void testReformatArrayFromTemplate() throws Exception { for (Map well : data.data()) { - if (asLongElseNull(well.get(WellTable.Column.SampleId.name())) instanceof Long num) + if (asLongElseNull(well.get(WellTable.Column.SampleID.name())) instanceof Long num) sampleIds.add(num); } } diff --git a/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java b/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java index 06af01e9ef3..5402646ef9f 100644 --- a/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java +++ b/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java @@ -140,25 +140,24 @@ public void setReimport(Boolean reimport) _reimport = reimport; } - private static final Set EXCLUDED_PROPERTIES = CaseInsensitiveHashSet.of("ContainerId", PlateTable.Column.DataFileId.name(), "EntityId"); - public void setNewRecordMap(Container container, PlateImpl plate) { - Map plateRow = ObjectFactory.Registry.getFactory(PlateBean.class) - .toMap(PlateBean.from(plate, true), new CaseInsensitiveHashMap<>()); - EXCLUDED_PROPERTIES.forEach(plateRow::remove); - - var newRecordMap = AbstractAuditTypeProvider.encodeForDataMap(plateRow); - setNewRecordMap(newRecordMap, container); + setNewRecordMap(encodeRecordMap(plate), container); } public void setOldRecordMap(Container container, PlateImpl plate) + { + setOldRecordMap(encodeRecordMap(plate), container); + } + + private static final Set EXCLUDED_PROPERTIES = CaseInsensitiveHashSet.of("ContainerId", PlateTable.Column.DataFileId.name(), "EntityId"); + + private static String encodeRecordMap(PlateImpl plate) { Map plateRow = ObjectFactory.Registry.getFactory(PlateBean.class) .toMap(PlateBean.from(plate, true), new CaseInsensitiveHashMap<>()); EXCLUDED_PROPERTIES.forEach(plateRow::remove); - var oldRecordMap = AbstractAuditTypeProvider.encodeForDataMap(plateRow); - setOldRecordMap(oldRecordMap, container); + return AbstractAuditTypeProvider.encodeForDataMap(plateRow); } } diff --git a/assay/src/org/labkey/assay/plate/data/PlateMapExcelWriter.java b/assay/src/org/labkey/assay/plate/data/PlateMapExcelWriter.java index dbfd19cba34..3803002938b 100644 --- a/assay/src/org/labkey/assay/plate/data/PlateMapExcelWriter.java +++ b/assay/src/org/labkey/assay/plate/data/PlateMapExcelWriter.java @@ -37,7 +37,7 @@ public class PlateMapExcelWriter extends ExcelWriter { private static final Logger logger = LogHelper.getLogger(PlateMapExcelWriter.class, "Plate map export"); - private static final Set excludedFields = CaseInsensitiveHashSet.of(Column.SampleId.name(), Column.Type.name(), Column.WellGroup.name()); + private static final Set excludedFields = CaseInsensitiveHashSet.of(Column.SampleID.name(), Column.Type.name(), Column.WellGroup.name()); private final Plate _plate; private final QueryView _queryView; diff --git a/assay/src/org/labkey/assay/plate/data/WellData.java b/assay/src/org/labkey/assay/plate/data/WellData.java index 2824ba00cef..f189e9de730 100644 --- a/assay/src/org/labkey/assay/plate/data/WellData.java +++ b/assay/src/org/labkey/assay/plate/data/WellData.java @@ -45,7 +45,7 @@ public Map getData(boolean includeWellId) if (_position != null) data.put(WellTable.WELL_LOCATION, _position); if (_sampleId != null) - data.put(WellTable.Column.SampleId.name(), _sampleId); + data.put(WellTable.Column.SampleID.name(), _sampleId); if (_type != null) data.put(WellTable.Column.Type.name(), _type.name()); if (_wellGroup != null) diff --git a/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java b/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java index 007a4efdb67..8a67ffdf7d7 100644 --- a/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java +++ b/assay/src/org/labkey/assay/plate/data/WellTriggerFactory.java @@ -100,7 +100,7 @@ private void addTypeSample( return; // The "SampleID" is not being modified - if (newRow.get(WellTable.Column.SampleId.name()) == null) + if (newRow.get(WellTable.Column.SampleID.name()) == null) return; // A "Type" is being explicitly provided @@ -175,7 +175,7 @@ private void addWellId(@Nullable Map newRow) if ( newRow != null && newRow.containsKey(WellTable.Column.RowId.name()) && - newRow.getOrDefault(WellTable.Column.SampleId.name(), null) != null + newRow.getOrDefault(WellTable.Column.SampleID.name(), null) != null ) { Long wellRowId = MapUtils.getLong(newRow,WellTable.Column.RowId.name()); @@ -324,7 +324,7 @@ private boolean hasReplicateChange(Container container, User user, @NotNull Long private boolean hasSampleChange(@Nullable Map row) { - return row != null && row.containsKey(WellTable.Column.SampleId.name()); + return row != null && row.containsKey(WellTable.Column.SampleID.name()); } private boolean hasTypeGroupReplicateChange(@Nullable Map row) diff --git a/assay/src/org/labkey/assay/plate/query/WellTable.java b/assay/src/org/labkey/assay/plate/query/WellTable.java index 100372f1e7a..8782c5a259c 100644 --- a/assay/src/org/labkey/assay/plate/query/WellTable.java +++ b/assay/src/org/labkey/assay/plate/query/WellTable.java @@ -36,6 +36,7 @@ import org.labkey.api.exp.property.DomainProperty; import org.labkey.api.exp.query.ExpMaterialTable; import org.labkey.api.exp.query.ExpSchema; +import org.labkey.api.gwt.client.AuditBehaviorType; import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.DefaultQueryUpdateService; import org.labkey.api.query.ExprColumn; @@ -86,7 +87,7 @@ public enum Column ReplicateGroup, Row, RowId, - SampleId, + SampleID, Type, Value, WellGroup; @@ -277,7 +278,7 @@ public MutableColumnInfo wrapColumn(ColumnInfo col) var columnInfo = super.wrapColumn(col); // workaround for sample lookup not resolving correctly - if (columnInfo.getName().equalsIgnoreCase(Column.SampleId.name())) + if (columnInfo.getName().equalsIgnoreCase(Column.SampleID.name())) { columnInfo.setFk(QueryForeignKey.from(getUserSchema(), getContainerFilter()) .schema(ExpSchema.SCHEMA_NAME, getContainer()) @@ -393,6 +394,12 @@ public boolean hasPermission(@NotNull UserPrincipal user, @NotNull Class Date: Wed, 24 Sep 2025 15:10:06 -0700 Subject: [PATCH 08/11] Fixups --- .../org/labkey/assay/plate/PlateManager.java | 14 ++++++----- .../assay/plate/audit/PlateAuditProvider.java | 1 - .../plate/audit/PlateSetAuditProvider.java | 1 - .../assay/plate/query/PlateSchemaTest.java | 25 +++++++++++-------- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/assay/src/org/labkey/assay/plate/PlateManager.java b/assay/src/org/labkey/assay/plate/PlateManager.java index 41c5ce9f565..f32e1ef4699 100644 --- a/assay/src/org/labkey/assay/plate/PlateManager.java +++ b/assay/src/org/labkey/assay/plate/PlateManager.java @@ -125,6 +125,7 @@ import org.labkey.api.util.GUID; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; +import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.util.UnexpectedException; import org.labkey.api.view.ActionURL; import org.labkey.api.view.HttpView; @@ -2900,20 +2901,21 @@ public PlateSetImpl createPlateSet( savePlateSetHeritage(plateSetId, plateSet.getType(), parentPlateSet); - if (plates != null) - addPlatesToPlateSet(container, user, plateSetId, plateSet.isTemplate(), plates, String.format("Added during creation of plate set \"%s\".", plateSet.getName())); - PlateSetImpl newPlateSet = (PlateSetImpl) requirePlateSet(container, plateSetId, "Failed to create plate set."); + if (plates != null) + addPlatesToPlateSet(container, user, plateSetId, newPlateSet.isTemplate(), plates, String.format("Added during creation of plate set \"%s\".", newPlateSet.getName())); + // Set transient parent plate set property for auditing if (parentPlateSet != null) newPlateSet.setParentPlateSetId(parentPlateSet.getRowId()); // Audit plate set creation { - int plateCount = plates != null ? plates.size() : 0; - additionalAuditComment = String.format("%s Initially contains %d %s.", StringUtils.trimToEmpty(additionalAuditComment), plateCount, plateCount == 1 ? "plate" : "plates"); - PlateSetAuditEvent auditEvent = PlateSetAuditProvider.EventFactory.plateSetCreated(container, tx.getAuditId(), newPlateSet, additionalAuditComment); + // Example comment: "Plate set was created. Created via reformat. Initially contains 5 plates." + int plateCount = plates == null ? 0 : plates.size(); + String comment = StringUtilsLabKey.joinNonBlank(" ", StringUtils.trimToEmpty(additionalAuditComment), String.format("Initially contains %s.", StringUtilsLabKey.pluralize(plateCount, "plate"))); + PlateSetAuditEvent auditEvent = PlateSetAuditProvider.EventFactory.plateSetCreated(container, tx.getAuditId(), newPlateSet, comment); AuditLogService.get().addEvent(user, auditEvent); } diff --git a/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java b/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java index 20c1240eacd..f340e5bf385 100644 --- a/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java +++ b/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java @@ -154,7 +154,6 @@ public PlateAuditDomainKind() _fields.add(createPropertyDescriptor(Column.Template.name(), PropertyType.BOOLEAN)); // AbstractAuditTypeProvider fields - _fields.add(createPropertyDescriptor(COLUMN_NAME_COMMENT, PropertyType.STRING)); _fields.add(createPropertyDescriptor(COLUMN_NAME_TRANSACTION_ID, PropertyType.BIGINT)); _fields.add(createPropertyDescriptor(COLUMN_NAME_USER_COMMENT, PropertyType.STRING)); _fields.add(createOldDataMapPropertyDescriptor()); diff --git a/assay/src/org/labkey/assay/plate/audit/PlateSetAuditProvider.java b/assay/src/org/labkey/assay/plate/audit/PlateSetAuditProvider.java index 91e6df612ef..b9e1ccc175a 100644 --- a/assay/src/org/labkey/assay/plate/audit/PlateSetAuditProvider.java +++ b/assay/src/org/labkey/assay/plate/audit/PlateSetAuditProvider.java @@ -154,7 +154,6 @@ public PlateSetAuditDomainKind() _fields.add(createPropertyDescriptor(Column.Archived.name(), PropertyType.BOOLEAN)); // AbstractAuditTypeProvider fields - _fields.add(createPropertyDescriptor(COLUMN_NAME_COMMENT, PropertyType.STRING)); _fields.add(createPropertyDescriptor(COLUMN_NAME_TRANSACTION_ID, PropertyType.BIGINT)); _fields.add(createPropertyDescriptor(COLUMN_NAME_USER_COMMENT, PropertyType.STRING)); _fields.add(createOldDataMapPropertyDescriptor()); diff --git a/assay/src/org/labkey/assay/plate/query/PlateSchemaTest.java b/assay/src/org/labkey/assay/plate/query/PlateSchemaTest.java index 1403cc3b6a5..e658eb1a96e 100644 --- a/assay/src/org/labkey/assay/plate/query/PlateSchemaTest.java +++ b/assay/src/org/labkey/assay/plate/query/PlateSchemaTest.java @@ -23,7 +23,7 @@ import org.labkey.assay.plate.PlateImpl; import org.labkey.assay.plate.PlateManager; -import java.util.Arrays; +import java.util.List; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -139,17 +139,22 @@ private void verifyTablePermissions(String tableName, boolean allowInsert, boole private @NotNull Plate updatePlate(CaseInsensitiveHashMap row) throws Exception { - var errors = new BatchValidationException(); - var plateRows = PlateManager.get().getPlateTable(container, user) - .getUpdateService() - .updateRows(user, container, Arrays.asList(row), null, errors, null, null); + try (var tx = PlateManager.get().ensureTransaction()) + { + var errors = new BatchValidationException(); + var plateRows = PlateManager.get().getPlateTable(container, user) + .getUpdateService() + .updateRows(user, container, List.of(row), null, errors, null, null); + + tx.commit(); - assertFalse("Expected no errors", errors.hasErrors()); - assertEquals("Expected a single row", 1, plateRows.size()); + assertFalse("Expected no errors", errors.hasErrors()); + assertEquals("Expected a single row", 1, plateRows.size()); - var plateRow = plateRows.get(0); - var plateRowId = (int) plateRow.get(PlateTable.Column.RowId.name()); + var plateRow = plateRows.get(0); + var plateRowId = (int) plateRow.get(PlateTable.Column.RowId.name()); - return getPlate(plateRowId); + return getPlate(plateRowId); + } } } From 97754bf8f1d8f8d6275620880bc4a21bebab5c28 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Thu, 25 Sep 2025 09:33:41 -0700 Subject: [PATCH 09/11] Issue 54017: revise replicate group validation message --- assay/src/org/labkey/assay/plate/PlateManager.java | 4 ++-- assay/src/org/labkey/assay/plate/PlateManagerTest.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/assay/src/org/labkey/assay/plate/PlateManager.java b/assay/src/org/labkey/assay/plate/PlateManager.java index f32e1ef4699..17c46f5999f 100644 --- a/assay/src/org/labkey/assay/plate/PlateManager.java +++ b/assay/src/org/labkey/assay/plate/PlateManager.java @@ -4307,7 +4307,7 @@ private void validatePlateSetReplicateGroups(Container container, User user, @No continue; if (groups.contains(groupName)) - throw new ValidationException(String.format("Replicate group \"%s\" contains mismatched well data. Ensure all data aligns for the replicates declared in these wells.", groupName)); + throw new ValidationException(String.format("Replicate group \"%s\" contains mismatched well data. Ensure the same data is recorded for each well in this replicate group across all plates in the plate set.", groupName)); groups.add(groupName); } @@ -4317,7 +4317,7 @@ private void validatePlateSetReplicateGroups(Container container, User user, @No throw UnexpectedException.wrap(e); } - // Fallback to more generic message if we did not resolve a specific mismatch + // Fallback to a more generic message if we did not resolve a specific mismatch throw new ValidationException(String.format("Plate set (%d) contains mismatched replicate well data.", plateSetRowId)); } diff --git a/assay/src/org/labkey/assay/plate/PlateManagerTest.java b/assay/src/org/labkey/assay/plate/PlateManagerTest.java index 55be8d02938..b0df652dbb9 100644 --- a/assay/src/org/labkey/assay/plate/PlateManagerTest.java +++ b/assay/src/org/labkey/assay/plate/PlateManagerTest.java @@ -1608,7 +1608,7 @@ public void testReplicateWellValidation() throws Exception } // Act / Assert - var expectedMessage = String.format("Replicate group \"%s\" contains mismatched well data. Ensure all data aligns for the replicates declared in these wells.", commonWellValues.get("replicateGroup")); + var expectedMessage = String.format("Replicate group \"%s\" contains mismatched well data. Ensure the same data is recorded for each well in this replicate group across all plates in the plate set.", commonWellValues.get("replicateGroup")); assertCreatePlateThrows(expectedMessage, PLATE_TYPE_96_WELLS, plateName, null, sourcePlateData); // Fixup rows by making all rows the same and resubmit @@ -1690,7 +1690,7 @@ public void testReplicateCrossPlateValidation() throws Exception // Act / Assert // Expect group "R1" to fail validation as it currently contains different samples across plates 1 and 2. - var expectedMessage = "Replicate group \"R1\" contains mismatched well data. Ensure all data aligns for the replicates declared in these wells."; + var expectedMessage = "Replicate group \"R1\" contains mismatched well data. Ensure the same data is recorded for each well in this replicate group across all plates in the plate set."; assertCreatePlateSetThrows(expectedMessage, plateSetImpl, plateData, null); // Fixup rows by making all rows the same and resubmit From b60843f6444f7f96e65d3ba3abca5fd2c6037801 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Fri, 26 Sep 2025 13:03:55 -0700 Subject: [PATCH 10/11] Recognize source plate is more cases --- .../org/labkey/assay/plate/PlateManager.java | 21 +++++++++++++++++-- .../assay/plate/audit/PlateAuditEvent.java | 2 +- .../assay/plate/audit/PlateAuditProvider.java | 8 +++---- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/assay/src/org/labkey/assay/plate/PlateManager.java b/assay/src/org/labkey/assay/plate/PlateManager.java index 17c46f5999f..81f3b709ea4 100644 --- a/assay/src/org/labkey/assay/plate/PlateManager.java +++ b/assay/src/org/labkey/assay/plate/PlateManager.java @@ -2840,7 +2840,10 @@ private List addPlatesToPlateSet( plateImpl.setTemplate(plateSetIsTemplate); // TODO: Write a cheaper plate create/save for multiple plates - platesAdded.add(createAndSavePlate(container, user, plateImpl, plateSetId, plate.data, true)); + var newPlate = (PlateImpl) createAndSavePlate(container, user, plateImpl, plateSetId, plate.data, true); + if (plate.templateId != null) + newPlate.setSourcePlateRowId(plate.templateId); + platesAdded.add(newPlate); } addPlateCreatedAuditEvents(container, user, tx, platesAdded, additionalAuditComment); @@ -5065,10 +5068,24 @@ private record HydratedResult(List plateData, @Nullable Integer plate { List> targetWellData = new ArrayList<>(); + Long templateId = null; if (wellLayout.getTargetTemplateId() != null) + { + templateId = wellLayout.getTargetTemplateId(); hydrateFromPlateTemplate(context, wellLayout, targetWellData); + } else + { + List sourcedWells = Arrays.stream(wellLayout.getWells()).filter(well -> well != null && well.sourcePlateId() > 0).toList(); + if (!sourcedWells.isEmpty()) + { + Long sourcePlateId = sourcedWells.get(0).sourcePlateId(); + if (sourcedWells.stream().allMatch(w -> sourcePlateId.equals(w.sourcePlateId()))) + templateId = sourcePlateId; + } + hydrateFromPlate(context, wellLayout, targetWellData); + } if (context.operation().produceEmptyPlates() || !targetWellData.isEmpty()) { @@ -5084,7 +5101,7 @@ private record HydratedResult(List plateData, @Nullable Integer plate } } - plates.add(new PlateData(name, wellLayout.getPlateType().getRowId(), null, barcode, targetWellData)); + plates.add(new PlateData(name, wellLayout.getPlateType().getRowId(), templateId, barcode, targetWellData)); } } diff --git a/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java b/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java index 5402646ef9f..5bfafa4e892 100644 --- a/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java +++ b/assay/src/org/labkey/assay/plate/audit/PlateAuditEvent.java @@ -39,7 +39,7 @@ protected PlateAuditEvent( Long transactionAuditId ) { - super(EVENT_NAME, container, eventType.getComment()); + super(EVENT_NAME, container, eventType.getComment(plate.isTemplate())); setPlateEventType(eventType.name()); setPlateRowId(plate.getRowId()); setPlateName(plate.getName()); diff --git a/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java b/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java index f340e5bf385..9c9697f226b 100644 --- a/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java +++ b/assay/src/org/labkey/assay/plate/audit/PlateAuditProvider.java @@ -106,8 +106,8 @@ public Class getEventClass() public enum PlateEventType { - CREATE_PLATE("Plate was created.", "Created"), - DELETE_PLATE("Plate was deleted.", "Deleted"), + CREATE_PLATE("%s was created.", "Created"), + DELETE_PLATE("%s was deleted.", "Deleted"), PLATE_IMPORT("Plate was imported into an assay run.", "Imported"); private final String _actionLabel; @@ -124,9 +124,9 @@ public String getActionLabel() return _actionLabel; } - public String getComment() + public String getComment(boolean isTemplate) { - return _comment; + return String.format(_comment, isTemplate ? "Plate template" : "Plate"); } } From 2e6126385dc940ce0349a12842a3712b236bc55d Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Mon, 29 Sep 2025 11:25:25 -0700 Subject: [PATCH 11/11] Bump @labkey/components --- assay/package-lock.json | 8 ++++---- assay/package.json | 2 +- core/package-lock.json | 8 ++++---- core/package.json | 2 +- experiment/package-lock.json | 8 ++++---- experiment/package.json | 2 +- pipeline/package-lock.json | 8 ++++---- pipeline/package.json | 2 +- 8 files changed, 20 insertions(+), 20 deletions(-) diff --git a/assay/package-lock.json b/assay/package-lock.json index a7ffbe4b3e3..a5f60a6ed81 100644 --- a/assay/package-lock.json +++ b/assay/package-lock.json @@ -8,7 +8,7 @@ "name": "assay", "version": "0.0.0", "dependencies": { - "@labkey/components": "6.61.1" + "@labkey/components": "6.62.7" }, "devDependencies": { "@labkey/build": "8.6.0", @@ -2458,9 +2458,9 @@ } }, "node_modules/@labkey/components": { - "version": "6.61.1", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-6.61.1.tgz", - "integrity": "sha512-5CxK+Pje33tYW31qNGS35Zf5hISUHk7aAByRqwIa/UqvtT0grCu/Qo6jGkSNzmK6S+JKvQDDVE+G90wc5+DTZw==", + "version": "6.62.7", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-6.62.7.tgz", + "integrity": "sha512-B6keJL4Wx0OD0PvJM4Ct9aOqp8rubCPTsF/f8N/0CDIGBikKBLBmwZej1JIDqk4MxaOzNKJqqwwUAplu/YaMaw==", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/assay/package.json b/assay/package.json index 6361f9f4aa8..a934c08608a 100644 --- a/assay/package.json +++ b/assay/package.json @@ -12,7 +12,7 @@ "clean": "rimraf resources/web/assay/gen && rimraf resources/views/gen && rimraf resources/web/gen" }, "dependencies": { - "@labkey/components": "6.61.1" + "@labkey/components": "6.62.7" }, "devDependencies": { "@labkey/build": "8.6.0", diff --git a/core/package-lock.json b/core/package-lock.json index bf493117b2e..eca0602edcf 100644 --- a/core/package-lock.json +++ b/core/package-lock.json @@ -8,7 +8,7 @@ "name": "labkey-core", "version": "0.0.0", "dependencies": { - "@labkey/components": "6.61.1", + "@labkey/components": "6.62.7", "@labkey/themes": "1.4.2" }, "devDependencies": { @@ -3504,9 +3504,9 @@ } }, "node_modules/@labkey/components": { - "version": "6.61.1", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-6.61.1.tgz", - "integrity": "sha512-5CxK+Pje33tYW31qNGS35Zf5hISUHk7aAByRqwIa/UqvtT0grCu/Qo6jGkSNzmK6S+JKvQDDVE+G90wc5+DTZw==", + "version": "6.62.7", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-6.62.7.tgz", + "integrity": "sha512-B6keJL4Wx0OD0PvJM4Ct9aOqp8rubCPTsF/f8N/0CDIGBikKBLBmwZej1JIDqk4MxaOzNKJqqwwUAplu/YaMaw==", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/core/package.json b/core/package.json index 030785dcfff..662e5f581ab 100644 --- a/core/package.json +++ b/core/package.json @@ -53,7 +53,7 @@ } }, "dependencies": { - "@labkey/components": "6.61.1", + "@labkey/components": "6.62.7", "@labkey/themes": "1.4.2" }, "devDependencies": { diff --git a/experiment/package-lock.json b/experiment/package-lock.json index 6a96832d5c3..bb7072786ae 100644 --- a/experiment/package-lock.json +++ b/experiment/package-lock.json @@ -8,7 +8,7 @@ "name": "experiment", "version": "0.0.0", "dependencies": { - "@labkey/components": "6.61.1" + "@labkey/components": "6.62.7" }, "devDependencies": { "@labkey/build": "8.6.0", @@ -3247,9 +3247,9 @@ } }, "node_modules/@labkey/components": { - "version": "6.61.1", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-6.61.1.tgz", - "integrity": "sha512-5CxK+Pje33tYW31qNGS35Zf5hISUHk7aAByRqwIa/UqvtT0grCu/Qo6jGkSNzmK6S+JKvQDDVE+G90wc5+DTZw==", + "version": "6.62.7", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-6.62.7.tgz", + "integrity": "sha512-B6keJL4Wx0OD0PvJM4Ct9aOqp8rubCPTsF/f8N/0CDIGBikKBLBmwZej1JIDqk4MxaOzNKJqqwwUAplu/YaMaw==", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/experiment/package.json b/experiment/package.json index 24ccc9004d3..c0949b22169 100644 --- a/experiment/package.json +++ b/experiment/package.json @@ -13,7 +13,7 @@ "test-integration": "cross-env NODE_ENV=test jest --ci --runInBand -c test/js/jest.config.integration.js" }, "dependencies": { - "@labkey/components": "6.61.1" + "@labkey/components": "6.62.7" }, "devDependencies": { "@labkey/build": "8.6.0", diff --git a/pipeline/package-lock.json b/pipeline/package-lock.json index 01bd8f62760..a09543d58d0 100644 --- a/pipeline/package-lock.json +++ b/pipeline/package-lock.json @@ -8,7 +8,7 @@ "name": "pipeline", "version": "0.0.0", "dependencies": { - "@labkey/components": "6.61.1" + "@labkey/components": "6.62.7" }, "devDependencies": { "@labkey/build": "8.6.0", @@ -2716,9 +2716,9 @@ } }, "node_modules/@labkey/components": { - "version": "6.61.1", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-6.61.1.tgz", - "integrity": "sha512-5CxK+Pje33tYW31qNGS35Zf5hISUHk7aAByRqwIa/UqvtT0grCu/Qo6jGkSNzmK6S+JKvQDDVE+G90wc5+DTZw==", + "version": "6.62.7", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-6.62.7.tgz", + "integrity": "sha512-B6keJL4Wx0OD0PvJM4Ct9aOqp8rubCPTsF/f8N/0CDIGBikKBLBmwZej1JIDqk4MxaOzNKJqqwwUAplu/YaMaw==", "license": "SEE LICENSE IN LICENSE.txt", "dependencies": { "@hello-pangea/dnd": "18.0.1", diff --git a/pipeline/package.json b/pipeline/package.json index 29f84bf9fff..891a9500a3f 100644 --- a/pipeline/package.json +++ b/pipeline/package.json @@ -14,7 +14,7 @@ "build-prod": "npm run clean && cross-env NODE_ENV=production PROD_SOURCE_MAP=source-map webpack --config node_modules/@labkey/build/webpack/prod.config.js --color --progress --profile" }, "dependencies": { - "@labkey/components": "6.61.1" + "@labkey/components": "6.62.7" }, "devDependencies": { "@labkey/build": "8.6.0",