From 8c8c0638f9c7a16f0f9ad0ca8db4e964094cbbd8 Mon Sep 17 00:00:00 2001 From: labkey-nicka Date: Thu, 23 Apr 2026 13:39:10 -0700 Subject: [PATCH] Compound: support moving SMILES properties --- .../org/labkey/api/exp/OntologyManager.java | 69 --------------- .../labkey/api/exp/api/ExperimentService.java | 14 ++- .../api/ExpDataClassDataTableImpl.java | 2 +- .../experiment/api/ExperimentServiceImpl.java | 87 ++++++++++--------- .../controllers/exp/ExperimentController.java | 2 +- 5 files changed, 59 insertions(+), 115 deletions(-) diff --git a/api/src/org/labkey/api/exp/OntologyManager.java b/api/src/org/labkey/api/exp/OntologyManager.java index 4676d7ad469..469b025eecc 100644 --- a/api/src/org/labkey/api/exp/OntologyManager.java +++ b/api/src/org/labkey/api/exp/OntologyManager.java @@ -2561,13 +2561,11 @@ public static Pair getURICacheKey(DomainDescriptor dd) return getCacheKey(dd.getDomainURI(), dd.getContainer()); } - public static Pair getCacheKey(PropertyDescriptor pd) { return getCacheKey(pd.getPropertyURI(), pd.getContainer()); } - public static Pair getCacheKey(String uri, Container c) { Container proj = c.getProject(); @@ -2661,7 +2659,6 @@ public static PropertyDescriptor insertOrUpdatePropertyDescriptor(PropertyDescri } } - static final String parameters = "propertyuri,name,description,rangeuri,concepturi,label," + "format,container,project,lookupcontainer,lookupschema,lookupquery,defaultvaluetype,hidden," + "mvenabled,importaliases,url,urltarget,shownininsertview,showninupdateview,shownindetailsview,measure,dimension,scale," + @@ -2704,71 +2701,6 @@ else if ("propertyuri".equals(p)) return new ParameterMapStatement(t.getSchema().getScope(), conn, sql, null); } - static ParameterMapStatement getUpdateStmt(Connection conn, User user, TableInfo t) throws SQLException - { - user = null==user ? User.guest : user; - SQLFragment sql = new SQLFragment("UPDATE exp.propertydescriptor SET "); - ColumnInfo c; - String comma = ""; - for (var p : parametersArray) - { - if (null == (c = t.getColumn(p))) - continue; - sql.append(comma).append(p).append("=?"); - comma = ", "; - sql.add(new Parameter(p, c.getJdbcType())); - } - sql.append(", modifiedby=" + user.getUserId() + ", modified={fn now()}"); - sql.append("\nWHERE propertyid=?"); - sql.add(new Parameter("propertyid", JdbcType.INTEGER)); - return new ParameterMapStatement(t.getSchema().getScope(), conn, sql, null); - } - - - public static void insertPropertyDescriptors(User user, List pds) throws SQLException - { - if (null == pds || pds.isEmpty()) - return; - TableInfo t = getTinfoPropertyDescriptor(); - try (Connection conn = t.getSchema().getScope().getConnection(); - ParameterMapStatement stmt = getInsertStmt(conn, user, t, false)) - { - ObjectFactory f = ObjectFactory.Registry.getFactory(PropertyDescriptor.class); - Map m = null; - for (PropertyDescriptor pd : pds) - { - m = f.toMap(pd, m); - stmt.clearParameters(); - stmt.putAll(m); - stmt.addBatch(); - } - stmt.executeBatch(); - } - } - - - public static void updatePropertyDescriptors(User user, List pds) throws SQLException - { - if (null == pds || pds.isEmpty()) - return; - TableInfo t = getTinfoPropertyDescriptor(); - try (Connection conn = t.getSchema().getScope().getConnection(); - ParameterMapStatement stmt = getUpdateStmt(conn, user, t)) - { - ObjectFactory f = ObjectFactory.Registry.getFactory(PropertyDescriptor.class); - Map m = null; - for (PropertyDescriptor pd : pds) - { - m = f.toMap(pd, m); - stmt.clearParameters(); - stmt.putAll(m); - stmt.addBatch(); - } - stmt.executeBatch(); - } - } - - public static PropertyDescriptor insertPropertyDescriptor(PropertyDescriptor pd) throws ChangePropertyDescriptorException { assert pd.getPropertyId() == 0; @@ -2779,7 +2711,6 @@ public static PropertyDescriptor insertPropertyDescriptor(PropertyDescriptor pd) return pd; } - //todo: we automatically update a pd to the last one in? public static PropertyDescriptor updatePropertyDescriptor(PropertyDescriptor pd) { diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index be904efff7c..5b7fd515268 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -21,6 +21,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.assay.AssayProvider; +import org.labkey.api.attachments.AttachmentParent; import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.collections.CaseInsensitiveHashSet; import org.labkey.api.data.Container; @@ -246,6 +247,15 @@ ExpRun createRunForProvenanceRecording(Container container, User user, @Nullable ExpData getExpData(ExpDataClass dataClass, long rowId); + /** + * Build an {@link org.labkey.api.attachments.AttachmentParent} that points at an ExpData's + * attachment storage. Useful for callers outside the experiment module (for example, module + * triggers) that need to move, read, or delete ExpData attachments without referencing + * experiment-internal classes. + */ + @NotNull + AttachmentParent getDataClassAttachmentParent(@NotNull Container container, @NotNull String dataLsid); + /** * Get a Data with name at a specific time. */ @@ -1150,10 +1160,6 @@ List getExpProtocolsWithParameterValue( int aliasMapRowContainerUpdate(TableInfo aliasMapTable, List dataIds, Container targetContainer); - Map moveDataClassObjects(Collection dataObjects, @NotNull Container sourceContainer, @NotNull Container targetContainer, @NotNull User user, @Nullable String userComment, @Nullable AuditBehaviorType auditBehavior) throws ExperimentException, BatchValidationException; - - int moveAuditEvents(Container targetContainer, List runLsids); - /** * From a list of barcodes, find material lsids * @param uniqueIds A list of barcodes diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 2b79effb966..d6df5d425c5 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1189,7 +1189,7 @@ public Map moveRows(User user, Container container, Container ta try { - Map response = ExperimentService.get().moveDataClassObjects(containerObjects.get(c), c, targetContainer, user, auditUserComment, auditType); + Map response = ExperimentServiceImpl.get().moveDataClassObjects(containerObjects.get(c), c, targetContainer, user, errors, auditUserComment, auditType); incrementCounts(allContainerResponse, response); } catch (ExperimentException e) diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 427cf6f2f42..383e42b212d 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -1044,13 +1044,13 @@ public void indexDeleted() // Clear the last indexed value on all tables that back a search document for (TableInfo indexedTable : List.of(getTinfoSampleType(), getTinfoDataClass())) { - new SqlExecutor(ExperimentService.get().getSchema()).execute("UPDATE " + indexedTable + + new SqlExecutor(getExpSchema()).execute("UPDATE " + indexedTable + " SET LastIndexed = NULL WHERE LastIndexed IS NOT NULL"); } for (TableInfo indexedTable : List.of(getTinfoMaterialIndexed(), getTinfoDataIndexed())) { - new SqlExecutor(ExperimentService.get().getSchema()).execute("TRUNCATE TABLE " + indexedTable); + new SqlExecutor(getExpSchema()).execute("TRUNCATE TABLE " + indexedTable); } } @@ -1898,6 +1898,12 @@ public ExpDataImpl getExpData(ExpDataClass dataClass, long rowId) return data == null ? null : new ExpDataImpl(data); } + @Override + public @NotNull AttachmentParent getDataClassAttachmentParent(@NotNull Container container, @NotNull String dataLsid) + { + return new ExpDataClassAttachmentParent(container, Lsid.parse(dataLsid)); + } + @Override @Nullable public ExpData getEffectiveData(@NotNull ExpDataClass dataClass, String name, @NotNull Date effectiveDate, @NotNull Container container, @Nullable ContainerFilter cf) @@ -4626,7 +4632,7 @@ private static void _lockDomainsAndProvisionedTables(AssayService assayService, { if (null == assayService) return; - DbSchema expSchema = ExperimentService.get().getSchema(); + DbSchema expSchema = getExpSchema(); for (var expProtocol : expProtocols) { AssayProvider provider = assayService.getProvider(expProtocol); @@ -8043,9 +8049,9 @@ else if (domain.getPropertyByName(propertyName) != null) // issue 25275 DefaultValueService.get().setDefaultValues(domain.getContainer(), defaultValues); if (options != null && options.getExcludedContainerIds() != null && !options.getExcludedContainerIds().isEmpty()) - ExperimentService.get().ensureDataTypeContainerExclusions(DataTypeForExclusion.DataClass, options.getExcludedContainerIds(), impl.getRowId(), u); + ensureDataTypeContainerExclusions(DataTypeForExclusion.DataClass, options.getExcludedContainerIds(), impl.getRowId(), u); else - ExperimentService.get().ensureDataTypeContainerExclusionsNonAdmin(DataTypeForExclusion.DataClass, impl.getRowId(), c, u); + ensureDataTypeContainerExclusionsNonAdmin(DataTypeForExclusion.DataClass, impl.getRowId(), c, u); tx.addCommitTask(() -> clearDataClassCache(c), DbScope.CommitTaskOption.IMMEDIATE, POSTCOMMIT, POSTROLLBACK); tx.addCommitTask(() -> indexDataClass(getDataClass(c, bean.getName()), SearchService.get().defaultTask().getQueue(c, SearchService.PRIORITY.modified)), POSTCOMMIT); @@ -8134,7 +8140,7 @@ public ValidationException updateDataClass(@NotNull Container c, @NotNull User u if (options != null && options.getExcludedContainerIds() != null) { - Pair, Collection> exclusionChanges = ExperimentService.get().ensureDataTypeContainerExclusions(DataTypeForExclusion.DataClass, options.getExcludedContainerIds(), dataClass.getRowId(), u); + Pair, Collection> exclusionChanges = ensureDataTypeContainerExclusions(DataTypeForExclusion.DataClass, options.getExcludedContainerIds(), dataClass.getRowId(), u); oldProps.put("ContainerExclusions", exclusionChanges.first); newProps.put("ContainerExclusions", exclusionChanges.second); } @@ -8688,9 +8694,7 @@ public List getExpMaterialsForRun(long runId) */ public Collection ensureAliases(User user, Set aliasNames) { - final ExperimentService svc = ExperimentService.get(); - - TableInfo aliasTable = svc.getTinfoAlias(); + TableInfo aliasTable = getTinfoAlias(); SimpleFilter filter = new SimpleFilter(); filter.addCondition(FieldKey.fromParts("name"), aliasNames, IN); Map existingAliases = new HashMap<>(); @@ -9029,13 +9033,11 @@ public void ensureDataTypeContainerExclusionsNonAdmin(@NotNull DataTypeForExclus // exclude the containers this user does not have permission to if (folderIds.removeAll(userFolderIds)) { - ExperimentService.get().ensureDataTypeContainerExclusions(dataType, folderIds, dataTypeId, user); + ensureDataTypeContainerExclusions(dataType, folderIds, dataTypeId, user); } - } } - private void addAuditEventForDataTypeContainerUpdate(DataTypeForExclusion type, String containerId, User user) { Container container = ContainerManager.getForId(containerId); @@ -9311,7 +9313,7 @@ private Pair getParentAliasMetrics(TableInfo tableInfo, String alias .append(" WHERE ") .append(aliasField) .append(" IS NOT NULL"); - List aliases = new SqlSelector(ExperimentService.get().getSchema(), sql).getArrayList(String.class); + List aliases = new SqlSelector(getExpSchema(), sql).getArrayList(String.class); Long requiredSampleParentCount = 0L; Long requiredDataParentCount = 0L; @@ -9368,7 +9370,7 @@ private Map getNameExpressionMetrics() .append(" WHERE ") .append(nameExpressionType.nameExpressionCol) .append(" IS NOT NULL"); - List nameExpressionStrs = new SqlSelector(ExperimentService.get().getSchema(), sql).getArrayList(String.class); + List nameExpressionStrs = new SqlSelector(getExpSchema(), sql).getArrayList(String.class); Map substitutionMetrics = new HashMap<>(); for (NameGenerator.SubstitutionValue substitutionValue : NameGenerator.SubstitutionValue.values()) @@ -9558,22 +9560,22 @@ public String getInvalidRequiredImportAliasUpdate(String dataTypeLsid, boolean i return null; } - private static @Nullable TableInfo getTableInfo(String schemaName) + private @Nullable TableInfo getTableInfo(String schemaName) { // 'samples' | 'exp.data' | 'assay' if (SamplesSchema.SCHEMA_NAME.equalsIgnoreCase(schemaName)) - return ExperimentService.get().getTinfoMaterial(); + return getTinfoMaterial(); else if ("exp.data".equalsIgnoreCase(schemaName)) - return ExperimentService.get().getTinfoData(); + return getTinfoData(); else if (AssaySchema.NAME.equalsIgnoreCase(schemaName)) - return ExperimentService.get().getTinfoExperimentRun(); + return getTinfoExperimentRun(); else return null; } - public static Pair getCurrentAndCrossFolderDataCount(Collection rowIds, String dataType, Container container) + public Pair getCurrentAndCrossFolderDataCount(Collection rowIds, String dataType, Container container) { - DbSchema expSchema = DbSchema.get("exp", DbSchemaType.Module); + DbSchema expSchema = getExpSchema(); SqlDialect dialect = expSchema.getSqlDialect(); TableInfo tableInfo = getTableInfo(dataType); @@ -9636,15 +9638,22 @@ public int aliasMapRowContainerUpdate(TableInfo aliasMapTable, List dataId return new SqlExecutor(aliasMapTable.getSchema()).execute(aliasMapUpdate); } - @Override - public Map moveDataClassObjects(Collection dataObjects, @NotNull Container sourceContainer, @NotNull Container targetContainer, @NotNull User user, @Nullable String userComment, @Nullable AuditBehaviorType auditBehavior) throws ExperimentException, BatchValidationException + public Map moveDataClassObjects( + Collection dataObjects, + @NotNull Container sourceContainer, + @NotNull Container targetContainer, + @NotNull User user, + BatchValidationException errors, + @Nullable String userComment, + @Nullable AuditBehaviorType auditBehavior + ) throws ExperimentException, BatchValidationException { if (dataObjects == null || dataObjects.isEmpty()) throw new IllegalArgumentException("No sources provided to move operation."); Map> dataClassesMap = new HashMap<>(); dataObjects.forEach(dataObject -> - dataClassesMap.computeIfAbsent(dataObject.getDataClass(user), t -> new ArrayList<>()).add(dataObject)); + dataClassesMap.computeIfAbsent(dataObject.getDataClass(user), _ -> new ArrayList<>()).add(dataObject)); Map updateCounts = new HashMap<>(); updateCounts.put("sources", 0); @@ -9668,6 +9677,12 @@ public Map moveDataClassObjects(Collection d DataClassUserSchema schema = new DataClassUserSchema(dataClass.getContainer(), user); TableInfo dataClassTable = schema.getTable(dataClass.getName()); + // Before trigger per batch + Map extraContext = Map.of("dataClass", dataClass, "targetContainer", targetContainer, "classObjects", classObjects, "dataIds", dataIds); + dataClassTable.fireBatchTrigger(sourceContainer, user, TableInfo.TriggerType.MOVE, null, true, errors, extraContext); + if (errors.hasErrors()) + throw errors; + // update exp.data.container int updateCount = Table.updateContainer(getTinfoData(), "rowId", dataIds, targetContainer, user, true); updateCounts.put("sources", updateCounts.get("sources") + updateCount); @@ -9681,10 +9696,7 @@ public Map moveDataClassObjects(Collection d // update core.document.container for any files attached to the data objects that are moving moveDataClassObjectAttachments(dataClass, classObjects, targetContainer, user); - // LKB registry data class objects can have related junction list rows that need to be updated as well. - // Since those tables already wire up trigger scripts, we'll use that mechanism here as well for the move event. - BatchValidationException errors = new BatchValidationException(); - Map extraContext = Map.of("targetContainer", targetContainer, "classObjects", classObjects, "dataIds", dataIds); + // After trigger per batch dataClassTable.fireBatchTrigger(sourceContainer, user, TableInfo.TriggerType.MOVE, null, false, errors, extraContext); if (errors.hasErrors()) throw errors; @@ -9746,8 +9758,8 @@ private void moveDataClassObjectAttachments(ExpDataClass dataClass, Collection moveDerivationRuns(Collection da runIdData.computeIfAbsent(dataObject.getRunId(), t -> new HashSet<>()).add(dataObject); }); // find the set of runs associated with data objects that are moving - List runs = ExperimentService.get().getExpRuns(runIdData.keySet()); List toUpdate = new ArrayList<>(); List toSplit = new ArrayList<>(); - for (ExpRun run : runs) + for (ExpRun run : getExpRuns(runIdData.keySet())) { Set outputIds = run.getDataOutputs().stream().map(ExpData::getRowId).collect(Collectors.toSet()); Set movingIds = runIdData.get(run.getRowId()).stream().map(ExpData::getRowId).collect(Collectors.toSet()); @@ -9801,7 +9812,7 @@ public int moveExperimentRuns(List runs, Container targetContainer, User runsTable.getSchema().getSqlDialect().appendInClauseSql(materialUpdate, runRowIds); int updateCount = new SqlExecutor(runsTable.getSchema()).execute(materialUpdate); - ExperimentService.get().updateExpObjectContainers(getTinfoExperimentRun(), runRowIds, targetContainer); + updateExpObjectContainers(getTinfoExperimentRun(), runRowIds, targetContainer); // LKB media have object properties associated with the protocol applications of the run // and object properties associated with the material and data inputs for those protocol applications @@ -9825,7 +9836,6 @@ public int moveExperimentRuns(List runs, Container targetContainer, User private int splitExperimentRuns(List runs, Map> movingData, Container targetContainer, User user) throws ExperimentException, BatchValidationException { final ViewBackgroundInfo targetInfo = new ViewBackgroundInfo(targetContainer, user, null); - ExperimentServiceImpl expService = (ExperimentServiceImpl) ExperimentService.get(); int runCount = 0; for (ExpRun run : runs) { @@ -9858,7 +9868,7 @@ private int splitExperimentRuns(List runs, Map> movin try { // create a new derivation run for the data that are moving - expService.derive(run.getMaterialInputs(), run.getDataInputs(), Collections.emptyMap(), movingOutputsMap, targetInfo, LOG); + derive(run.getMaterialInputs(), run.getDataInputs(), Collections.emptyMap(), movingOutputsMap, targetInfo, LOG); } catch (ValidationException e) { @@ -9893,8 +9903,6 @@ public Map moveAssayRuns(@NotNull List assayR List runLsids = assayRuns.stream().map(ExpRun::getLSID).toList(); - ExperimentService expService = ExperimentService.get(); - AbstractAssayProvider.AssayMoveData assayMoveData = new AbstractAssayProvider.AssayMoveData(new HashMap<>(), new HashMap<>()); try (DbScope.Transaction transaction = ensureTransaction()) { @@ -9917,7 +9925,7 @@ public Map moveAssayRuns(@NotNull List assayR { provider.moveRuns(runs, targetContainer, user, assayMoveData); Map counts = assayMoveData.counts(); - int auditEventCount = expService.moveAuditEvents(targetContainer, runLsids); + int auditEventCount = moveAuditEvents(targetContainer, runLsids); counts.put("auditEvents", counts.getOrDefault("auditEvents", 0) + auditEventCount); if (auditBehavior != null && AuditBehaviorType.NONE != auditBehavior) { @@ -9977,8 +9985,7 @@ private boolean moveFile(AbstractAssayProvider.AssayFileMoveData renameData, Con return moveFileLinkFile(sourceFile, targetFile, sourceContainer, user, changeDetail, txAuditEvent, fieldName); } - @Override - public int moveAuditEvents(Container targetContainer, List runLsids) + private int moveAuditEvents(Container targetContainer, List runLsids) { ExperimentAuditProvider auditProvider = new ExperimentAuditProvider(); return auditProvider.moveEvents(targetContainer, runLsids); @@ -9993,7 +10000,7 @@ public int moveAuditEvents(Container targetContainer, List runLsids) int numUniqueIdCols = 0; SQLFragment unionSql; - DbSchema dbSchema = ExperimentService.get().getSchema(); + DbSchema dbSchema = getExpSchema(); SqlDialect dialect = dbSchema.getSqlDialect(); UserSchema samplesUserSchema = QueryService.get().getUserSchema(user, container, SamplesSchema.SCHEMA_NAME); List sampleTypes = SampleTypeServiceImpl.get().getSampleTypes(container, true); @@ -10035,7 +10042,7 @@ public int moveAuditEvents(Container targetContainer, List runLsids) .append("CAST (ST.").appendIdentifier(col.getSelectIdentifier()).append(" AS VARCHAR)") .append(" AS ").append(UNIQUE_ID_COL_NAME); query.append(" FROM ").append(provisioned, "ST"); - query.append(" INNER JOIN ").append(ExperimentService.get().getTinfoMaterial(), "M").append(" ON M.RowId = ST.RowId"); + query.append(" INNER JOIN ").append(getTinfoMaterial(), "M").append(" ON M.RowId = ST.RowId"); query.append(" WHERE ").appendIdentifier(col.getSelectIdentifier()).appendInClause(isIntegerField ? intIds : uniqueIds, dialect); unionAll = "\n UNION ALL\n"; } diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index ec3248cf7b4..83b083b723b 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -8083,7 +8083,7 @@ public void validateForm(CrossFolderSelectionForm form, Errors errors) @Override public Object execute(CrossFolderSelectionForm form, BindException errors) { - Pair result = ExperimentServiceImpl.getCurrentAndCrossFolderDataCount(form.getIds(false), form.getDataType(), getContainer()); + Pair result = ExperimentServiceImpl.get().getCurrentAndCrossFolderDataCount(form.getIds(false), form.getDataType(), getContainer()); ApiSimpleResponse resp = new ApiSimpleResponse(); resp.put("success", true);