diff --git a/api/src/org/labkey/api/assay/AbstractAssayProvider.java b/api/src/org/labkey/api/assay/AbstractAssayProvider.java index a4bf95f51c7..d021b65453d 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayProvider.java +++ b/api/src/org/labkey/api/assay/AbstractAssayProvider.java @@ -74,7 +74,6 @@ import org.labkey.api.exp.api.IAssayDomainType; import org.labkey.api.exp.property.Domain; 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.exp.query.ExpRunTable; import org.labkey.api.files.FileContentService; @@ -1787,7 +1786,7 @@ public record AssayFileMoveData(ExpRun run, Container sourceContainer, String fi public record AssayMoveData(Map counts, Map> fileMovesByRunId) {} @Override - public void moveRuns(List runs, Container targetContainer, User user, AbstractAssayProvider.AssayMoveData assayMoveData) + public void moveRuns(List runs, Container targetContainer, User user, AbstractAssayProvider.AssayMoveData assayMoveData) throws ExperimentException { if (runs.isEmpty()) return; @@ -1867,7 +1866,7 @@ public void moveRuns(List runs, Container targetContainer, User user, Ab moveAssayResults(runs, assayProtocol, sourceContainer, targetContainer, user, assayMoveData); } - private void moveRunsBatch(List runs, Container sourceContainer, Container targetContainer, User user, AssayMoveData assayMoveData) + private void moveRunsBatch(List runs, Container sourceContainer, Container targetContainer, User user, AssayMoveData assayMoveData) throws ExperimentException { Map> movedFiles = assayMoveData.fileMovesByRunId(); @@ -1926,6 +1925,9 @@ private void moveRunsBatch(List runs, Container sourceContainer, Contain .getProperties().stream() .filter(prop -> PropertyType.FILE_LINK.getTypeUri().equals(prop.getRangeURI())).toList(); + Map fileMoveReferences = new HashMap<>(); + Map fileMoveCounts = new HashMap<>(); + FileContentService fileContentService = FileContentService.get(); if (fileContentService != null && !fileDomainProps.isEmpty()) { @@ -1953,15 +1955,32 @@ private void moveRunsBatch(List runs, Container sourceContainer, Contain File updatedFile = fileContentService.getMoveTargetFile(sourceFileName, sourceContainer, targetContainer); if (updatedFile != null) { + if (!fileMoveReferences.containsKey(sourceFileName)) + fileMoveReferences.put(sourceFileName, new AssayFileMoveReference(sourceFileName, updatedFile, experiment.getName())); + + if (!fileMoveCounts.containsKey(sourceFileName)) + fileMoveCounts.put(sourceFileName, 0); + fileMoveCounts.put(sourceFileName, fileMoveCounts.get(sourceFileName) + 1); + ExpRun run = batchRun.get(experiment.getRowId()); Integer runId = run.getRowId(); movedFiles.putIfAbsent(runId, new ArrayList<>()); movedFiles.get(runId).add(new AssayFileMoveData(run, run.getContainer(), fileProp.getName(), sourceFile, updatedFile)); - fileContentService.fireFileMoveEvent(sourceFile.toPath(), updatedFile.toPath(), user, sourceContainer, targetContainer); } } } + for (String sourceFileName : fileMoveReferences.keySet()) + { + AssayFileMoveReference ref = fileMoveReferences.get(sourceFileName); + int count = fileMoveCounts.get(sourceFileName); + File sourceFile = new File(sourceFileName); + if (!ExperimentService.get().canMoveFileReference(user, sourceContainer, sourceFile, count)) + throw new ExperimentException("Assay batch " + ref.runName + " cannot be moved since it references a shared file: " + sourceFile.getName()); + + fileContentService.fireFileMoveEvent(sourceFile.toPath(), ref.updatedFile.toPath(), user, sourceContainer, targetContainer); + } + } // update Exp.Object where object.objecturi IN batchLsids @@ -1974,7 +1993,7 @@ private void moveRunsBatch(List runs, Container sourceContainer, Contain updateCounts.put("expObject", updateCounts.getOrDefault("expObject", 0) + expObjectCount); } - private void updateRunFiles(List runs, Container sourceContainer, Container targetContainer, User user, AssayMoveData assayMoveData) + private void updateRunFiles(List runs, Container sourceContainer, Container targetContainer, User user, AssayMoveData assayMoveData) throws ExperimentException { Map> movedFiles = assayMoveData.fileMovesByRunId(); @@ -1991,6 +2010,9 @@ private void updateRunFiles(List runs, Container sourceContainer, Contai if (fileContentService == null) return; + Map fileMoveReferences = new HashMap<>(); + Map fileMoveCounts = new HashMap<>(); + for (ExpRun run : runs) { for (DomainProperty fileProp : fileDomainProps) @@ -2015,13 +2037,31 @@ private void updateRunFiles(List runs, Container sourceContainer, Contai File updatedFile = fileContentService.getMoveTargetFile(sourceFileName, sourceContainer, targetContainer); if (updatedFile != null) { + if (!fileMoveReferences.containsKey(sourceFileName)) + fileMoveReferences.put(sourceFileName, new AssayFileMoveReference(sourceFileName, updatedFile, run.getName())); + + if (!fileMoveCounts.containsKey(sourceFileName)) + fileMoveCounts.put(sourceFileName, 0); + fileMoveCounts.put(sourceFileName, fileMoveCounts.get(sourceFileName) + 1); + Integer runId = run.getRowId(); movedFiles.putIfAbsent(runId, new ArrayList<>()); movedFiles.get(runId).add(new AssayFileMoveData(run, run.getContainer(), fileProp.getName(), sourceFile, updatedFile)); - fileContentService.fireFileMoveEvent(sourceFile.toPath(), updatedFile.toPath(), user, sourceContainer, targetContainer); } } } + + for (String sourceFileName : fileMoveReferences.keySet()) + { + AssayFileMoveReference ref = fileMoveReferences.get(sourceFileName); + int count = fileMoveCounts.get(sourceFileName); + File sourceFile = new File(sourceFileName); + if (!ExperimentService.get().canMoveFileReference(user, sourceContainer, sourceFile, count)) + throw new ExperimentException("Assay run " + ref.runName + " cannot be moved since it references a shared file: " + sourceFile.getName()); + + fileContentService.fireFileMoveEvent(sourceFile.toPath(), ref.updatedFile.toPath(), user, sourceContainer, targetContainer); + } + } private void updateDataFileUrl(List runs, Container sourceContainer, Container targetContainer, User user, AssayMoveData assayMoveData) @@ -2058,6 +2098,9 @@ private void updateDataFileUrl(List runs, Container sourceContainer, Con if (updatedFile != null) { ExpRun run = runMap.get(runId); + if (!ExperimentService.get().canMoveFileReference(user, sourceContainer, sourceFile, 1)) + throw new ExperimentException("Assay run " + run.getName() + " cannot be moved since it references a shared file: " + sourceFile.getName()); + movedFiles.putIfAbsent(runId, new ArrayList<>()); movedFiles.get(runId).add(new AssayFileMoveData(run, run.getContainer(), null, sourceFile, updatedFile)); fileContentService.fireFileMoveEvent(sourceFile.toPath(), updatedFile.toPath(), user, sourceContainer, targetContainer); @@ -2070,7 +2113,7 @@ private void updateDataFileUrl(List runs, Container sourceContainer, Con } } - protected void moveAssayResults(List runs, ExpProtocol protocol, Container sourceContainer, Container targetContainer, User user, AssayMoveData assayMoveData) + protected void moveAssayResults(List runs, ExpProtocol protocol, Container sourceContainer, Container targetContainer, User user, AssayMoveData assayMoveData) throws ExperimentException { String tableName = AssayProtocolSchema.DATA_TABLE_NAME; AssaySchema schema = createProtocolSchema(user, targetContainer, protocol, null); @@ -2081,7 +2124,9 @@ protected void moveAssayResults(List runs, ExpProtocol protocol, Contain updateResultFiles(assayResultTable, runs, protocol, sourceContainer, targetContainer, user, assayMoveData); } - private void updateResultFiles(FilteredTable assayResultTable, List runs, ExpProtocol assayProtocol, Container sourceContainer, Container targetContainer, User user, AssayMoveData assayMoveData) + record AssayFileMoveReference(String sourceFilePath, File updatedFile, String runName) {} + + private void updateResultFiles(FilteredTable assayResultTable, List runs, ExpProtocol assayProtocol, Container sourceContainer, Container targetContainer, User user, AssayMoveData assayMoveData) throws ExperimentException { FileContentService fileContentService = FileContentService.get(); if (fileContentService == null) @@ -2112,6 +2157,8 @@ private void updateResultFiles(FilteredTable assayResultTable, List runs Map runMap = new HashMap<>(); runs.forEach(run -> runMap.put(run.getRowId(), run)); + Map fileMoveReferences = new HashMap<>(); + Map> fileMoveResultRowIds = new HashMap<>(); for (String fileField : fileFields) { var fileColumn = assayResultTable.getColumn(fileField); @@ -2125,40 +2172,58 @@ private void updateResultFiles(FilteredTable assayResultTable, List runs continue; Integer resultRowId = (Integer) resultRow.get("rowid"); Integer resultRunId = (Integer) resultRow.get("run"); + if (!fileMoveResultRowIds.containsKey(sourceFileName)) + fileMoveResultRowIds.put(sourceFileName, new ArrayList<>()); + fileMoveResultRowIds.get(sourceFileName).add(resultRowId); File updatedFile = fileContentService.getMoveTargetFile(sourceFileName, sourceContainer, targetContainer); if (updatedFile != null) { File sourceFile = new File(sourceFileName); ExpRun run = runMap.get(resultRunId); + if (!fileMoveReferences.containsKey(sourceFileName)) + fileMoveReferences.put(sourceFileName, new AssayFileMoveReference(sourceFileName, updatedFile, run.getName())); + movedFiles.putIfAbsent(resultRunId, new ArrayList<>()); movedFiles.get(resultRunId).add(new AssayFileMoveData(run, run.getContainer(), fileField, sourceFile, updatedFile)); + } + } - // update the exp.Object row for the file results row - // TODO refactor so that the updates to the related exp.Object rows are done in the ExpDataFileListener (see similar updates in moveRuns() and moveRunsBatch()) - // TODO also use exp.data.objectId instead of objecturi - TableInfo expDataTable = ExperimentService.get().getTinfoData(); - TableInfo expObjectTable = OntologyManager.getTinfoObject(); - SQLFragment updateSql = new SQLFragment("UPDATE ").append(expObjectTable) - .append(" SET container = ").appendValue(targetContainer.getEntityId()) - .append(" WHERE objecturi IN ( SELECT lsid FROM ") - .append(expDataTable) - .append(" WHERE datafileurl = ").appendValue(FileUtil.pathToString(sourceFile.toPath())); - updateSql.append(")"); - new SqlExecutor(expObjectTable.getSchema()).execute(updateSql); - - fileContentService.fireFileMoveEvent(sourceFile.toPath(), updatedFile.toPath(), user, sourceContainer, targetContainer); + for (String sourceFileName : fileMoveReferences.keySet()) + { + AssayFileMoveReference ref = fileMoveReferences.get(sourceFileName); + int count = fileMoveResultRowIds.get(sourceFileName).size(); + File sourceFile = new File(sourceFileName); + if (!ExperimentService.get().canMoveFileReference(user, sourceContainer, sourceFile, count)) + throw new ExperimentException("Assay run " + ref.runName + " cannot be moved since it references a shared file: " + sourceFile.getName()); + + File updatedFile = ref.updatedFile; + // update the exp.Object row for the file results row + // TODO refactor so that the updates to the related exp.Object rows are done in the ExpDataFileListener (see similar updates in moveRuns() and moveRunsBatch()) + // TODO also use exp.data.objectId instead of objecturi + TableInfo expDataTable = ExperimentService.get().getTinfoData(); + TableInfo expObjectTable = OntologyManager.getTinfoObject(); + SQLFragment updateSql = new SQLFragment("UPDATE ").append(expObjectTable) + .append(" SET container = ").appendValue(targetContainer.getEntityId()) + .append(" WHERE objecturi IN ( SELECT lsid FROM ") + .append(expDataTable) + .append(" WHERE datafileurl = ").appendValue(FileUtil.pathToString(sourceFile.toPath())); + updateSql.append(")"); + new SqlExecutor(expObjectTable.getSchema()).execute(updateSql); + + fileContentService.fireFileMoveEvent(sourceFile.toPath(), updatedFile.toPath(), user, sourceContainer, targetContainer); + + var realTable = assayResultTable.getRealTable(); + var realFileColumn = realTable.getColumn(fileField); + updateSql = new SQLFragment("UPDATE ").append(assayResultTable.getRealTable()) + .append(" SET ") + .appendIdentifier(realFileColumn.getSelectIdentifier()) + .append(" = ").appendValue(updatedFile.getAbsolutePath()) + .append(" WHERE rowId ").appendInClause(fileMoveResultRowIds.get(sourceFileName), realTable.getSqlDialect()); + new SqlExecutor(assayResultTable.getSchema()).execute(updateSql); - var realTable = assayResultTable.getRealTable(); - var realFileColumn = realTable.getColumn(fileField); - updateSql = new SQLFragment("UPDATE ").append(assayResultTable.getRealTable()) - .append(" SET ") - .appendIdentifier(realFileColumn.getSelectIdentifier()) - .append(" = ").appendValue(updatedFile.getAbsolutePath()) - .append(" WHERE rowId = ").appendValue(resultRowId); - new SqlExecutor(assayResultTable.getSchema()).execute(updateSql); - } } + } } } diff --git a/api/src/org/labkey/api/assay/AssayProvider.java b/api/src/org/labkey/api/assay/AssayProvider.java index 925d74281ca..4e74be4acbd 100644 --- a/api/src/org/labkey/api/assay/AssayProvider.java +++ b/api/src/org/labkey/api/assay/AssayProvider.java @@ -389,7 +389,7 @@ default Long getResultRowCount(List protocols) return null; } - void moveRuns(List runs, Container targetContainer, User user, AbstractAssayProvider.AssayMoveData assayMoveData); + void moveRuns(List runs, Container targetContainer, User user, AbstractAssayProvider.AssayMoveData assayMoveData) throws ExperimentException; default void ensurePropertyDomainName(ExpProtocol protocol, ObjectProperty prop) { diff --git a/api/src/org/labkey/api/data/NameGeneratorState.java b/api/src/org/labkey/api/data/NameGeneratorState.java index 79c5d4eff52..5011de75e1e 100644 --- a/api/src/org/labkey/api/data/NameGeneratorState.java +++ b/api/src/org/labkey/api/data/NameGeneratorState.java @@ -89,7 +89,7 @@ public NameGeneratorState(@NotNull NameGenerator nameGenerator, boolean incremen if (incrementSampleCounts) // determine if need to incrementRootSampleCount { DbSequence sampleCountSeq = SampleTypeService.get().getSampleCountSequence(_container, false); - if (expressionSummary.hasProjectSampleCounter() || sampleCountSeq.current() > 0) // if ${sampleCount} is present, or if ${sampleCount} was previously evaluated + if ((expressionSummary != null && expressionSummary.hasProjectSampleCounter()) || sampleCountSeq.current() > 0) // if ${sampleCount} is present, or if ${sampleCount} was previously evaluated { sampleCounterSequence = sampleCountSeq; if (sampleCounterSequence != null) @@ -105,7 +105,7 @@ else if (sampleCountSeq.current() < expressionMin) sampleCounterSequence = null; DbSequence rootCountSeq = SampleTypeService.get().getSampleCountSequence(_container, true); - if (expressionSummary.hasProjectSampleRootCounter() || rootCountSeq.current() > 0) // if ${rootSampleCount} is present, or if ${rootSampleCount} was previously evaluated + if ((expressionSummary != null && expressionSummary.hasProjectSampleRootCounter()) || rootCountSeq.current() > 0) // if ${rootSampleCount} is present, or if ${rootSampleCount} was previously evaluated { rootCounterSequence = rootCountSeq; if (rootCounterSequence != null) @@ -228,7 +228,7 @@ private String genName(@NotNull Map rowMap, Map sampleCounts = null; if (_incrementSampleCounts) { - if (!_nameGenerator.getExpressionSummary().hasDateBasedSampleCounter()) + if (!(_nameGenerator.getExpressionSummary() != null && _nameGenerator.getExpressionSummary().hasDateBasedSampleCounter())) { if (null == getSampleCountsFunction) { @@ -281,12 +281,16 @@ private String genName(@NotNull Map rowMap, return currName.trim(); } + // allow using alternative expression for evaluation. + // for example, use AliquotNameExpression instead of NameExpression if sample is aliquot + NameGenerator activeNameGenerator = getActiveNameGenerator(altNameGenerator); + if (!activeNameGenerator.getSyntaxErrors().isEmpty()) + throw new IllegalArgumentException("Invalid naming expression. " + StringUtils.join(activeNameGenerator.getSyntaxErrors(), "\n")); + // Add extra context variables Map ctx = additionalContext(rowMap, parentDatas, parentSamples, sampleCounts, extraProps, altNameGenerator); - // allow using alternative expression for evaluation. - // for example, use AliquotNameExpression instead of NameExpression if sample is aliquot - StringExpressionFactory.FieldKeyStringExpression expression = getActiveNameGenerator(altNameGenerator).getParsedNameExpression(); + StringExpressionFactory.FieldKeyStringExpression expression = activeNameGenerator.getParsedNameExpression(); String name = null; if (expression instanceof NameGenerator.NameGenerationExpression) name = ((NameGenerator.NameGenerationExpression) expression).eval(ctx, _prefixCounterSequences); diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index db2d42348bf..729dcb1f9af 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -138,6 +138,8 @@ static void setInstance(ExperimentService impl) ServiceRegistry.get().registerService(ExperimentService.class, impl); } + boolean canMoveFileReference(User user, Container sourceContainer, File sourceFile, int moveCount); + enum QueryOptions { UseLsidForUpdate, @@ -1124,7 +1126,7 @@ List getExpProtocolsWithParameterValue( int moveExperimentRuns(List runs, Container targetContainer, User user); - Map moveAssayRuns(List assayRuns, Container container, Container targetContainer, User user, String userComment, AuditBehaviorType auditBehavior); + Map moveAssayRuns(List assayRuns, Container container, Container targetContainer, User user, String userComment, AuditBehaviorType auditBehavior) throws ExperimentException; int aliasMapRowContainerUpdate(TableInfo aliasMapTable, List dataIds, Container targetContainer); diff --git a/api/src/org/labkey/api/files/TableUpdaterFileListener.java b/api/src/org/labkey/api/files/TableUpdaterFileListener.java index e76a38a45db..ce46e143c87 100644 --- a/api/src/org/labkey/api/files/TableUpdaterFileListener.java +++ b/api/src/org/labkey/api/files/TableUpdaterFileListener.java @@ -15,6 +15,7 @@ */ package org.labkey.api.files; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; @@ -195,6 +196,14 @@ public String getSourceName() return _table.getSchema().getName() + "." + _table.getName() + "." + _pathColumn.getName(); } + public String getSourceSelect(SqlDialect sqlDialect) + { + String schema = sqlDialect.getStringHandler().quoteStringLiteral(_table.getSchema().getName()); + String table = sqlDialect.getStringHandler().quoteStringLiteral(_table.getName()); + String column = sqlDialect.getStringHandler().quoteStringLiteral(_pathColumn.getName()); + return schema + "." + table + "." + column; + } + @Override public void fileCreated(@NotNull File created, @Nullable User user, @Nullable Container container) { @@ -344,12 +353,11 @@ public Collection listFiles(@Nullable Container container) @Override public SQLFragment listFilesQuery() { - return listFilesQuery(false); + return listFilesQuery(false, null); } - public SQLFragment listFilesQuery(boolean skipCreatedModified) + public SQLFragment listFilesQuery(boolean skipCreatedModified, String filePath) { - SqlDialect dialect = _table.getSqlDialect(); SQLFragment selectFrag = new SQLFragment(); selectFrag.append("SELECT\n"); @@ -393,10 +401,15 @@ else if (_table.getColumn("Folder") != null) selectFrag.append(" NULL AS SourceKey,\n"); //selectFrag.append(" ? AS SourceName\n").add(getName()); - selectFrag.append(" ").append(_table.getSchema().getSqlDialect().getStringHandler().quoteStringLiteral(getSourceName())).append(" AS SourceName\n"); + selectFrag.append(" ").appendValue(getSourceSelect(_table.getSchema().getSqlDialect())).append(" AS SourceName\n"); selectFrag.append("FROM ").append(_table, TABLE_ALIAS).append("\n"); - selectFrag.append("WHERE ").appendIdentifier(_pathColumn.getSelectIdentifier()).append(" IS NOT NULL\n"); + selectFrag.append("WHERE ").appendIdentifier(_pathColumn.getSelectIdentifier()); + + if (StringUtils.isEmpty(filePath)) + selectFrag.append(" IS NOT NULL\n"); + else + selectFrag.append(" = ").appendStringLiteral(filePath, _table.getSchema().getSqlDialect()).append("\n"); return selectFrag; } diff --git a/experiment/src/org/labkey/experiment/ExpDataFileListener.java b/experiment/src/org/labkey/experiment/ExpDataFileListener.java index 76f6374869a..42f66d904db 100644 --- a/experiment/src/org/labkey/experiment/ExpDataFileListener.java +++ b/experiment/src/org/labkey/experiment/ExpDataFileListener.java @@ -56,6 +56,12 @@ public int fileMoved(@NotNull Path src, @NotNull Path dest, @Nullable User user, @Override public int fileMoved(@NotNull Path src, @NotNull Path dest, @Nullable User user, @Nullable Container sourceContainer, @Nullable Container targetContainer) { + // Issue 53070: Moving files could result in duplicate exp.data records + boolean deleteSrc = false; + ExpData existingConflict = ExperimentService.get().getExpDataByURL(dest, targetContainer); + if (existingConflict != null) + deleteSrc = true; + ExpData data = ExperimentService.get().getExpDataByURL(src, sourceContainer); if (data == null) @@ -71,13 +77,21 @@ public int fileMoved(@NotNull Path src, @NotNull Path dest, @Nullable User user, int extra = 0; if (data != null && src.equals(data.getFilePath()) && !src.equals(dest)) { - // The file has been renamed, so rename the exp.data row if its name matches - data.setName(FileUtil.getFileName(dest)); - // if the data object moved containers, set that as well - if (targetContainer != null && !targetContainer.equals(sourceContainer)) - data.setContainer(targetContainer); - data.save(user); // FIXME: Issue 53070: Moving files could result in duplicate exp.data records - extra = 1; + if (deleteSrc) + { + data.delete(user); + return 0; + } + else + { + // The file has been renamed, so rename the exp.data row if its name matches + data.setName(FileUtil.getFileName(dest)); + // if the data object moved containers, set that as well + if (targetContainer != null && !targetContainer.equals(sourceContainer)) + data.setContainer(targetContainer); + data.save(user); + extra = 1; + } } return super.fileMoved(src, dest, user, sourceContainer) + extra; diff --git a/experiment/src/org/labkey/experiment/FileLinkFileListener.java b/experiment/src/org/labkey/experiment/FileLinkFileListener.java index 650c3bd45c8..7edfdc154d2 100644 --- a/experiment/src/org/labkey/experiment/FileLinkFileListener.java +++ b/experiment/src/org/labkey/experiment/FileLinkFileListener.java @@ -15,6 +15,7 @@ */ package org.labkey.experiment; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; @@ -265,6 +266,11 @@ public SQLFragment listFilesQuery() } public SQLFragment listFilesQuery(boolean skipCreatedModified) + { + return listFilesQuery(skipCreatedModified, null); + } + + public SQLFragment listFilesQuery(boolean skipCreatedModified, String filePath) { final SQLFragment frag = new SQLFragment(); @@ -285,7 +291,10 @@ public SQLFragment listFilesQuery(boolean skipCreatedModified) frag.append(" ").append(OntologyManager.getTinfoObjectProperty(), "op").append(",\n"); frag.append(" ").append(OntologyManager.getTinfoObject(), "o").append("\n"); frag.append("WHERE\n"); - frag.append(" op.StringValue IS NOT NULL AND\n"); + if (StringUtils.isEmpty(filePath)) + frag.append(" op.StringValue IS NOT NULL AND\n"); + else + frag.append(" op.StringValue = ").appendStringLiteral(filePath, OntologyManager.getTinfoObject().getSqlDialect()).append(" AND\n"); frag.append(" o.ObjectId = op.ObjectId AND\n"); frag.append(" PropertyId IN (\n"); frag.append(" SELECT PropertyId\n"); @@ -296,8 +305,8 @@ public SQLFragment listFilesQuery(boolean skipCreatedModified) hardTableFileLinkColumns((schema, table, pathColumn, containerId, domainUri) -> { SQLFragment containerFrag = new SQLFragment("?", containerId); TableUpdaterFileListener updater = new TableUpdaterFileListener(table, pathColumn.getColumnName(), TableUpdaterFileListener.Type.filePath, null, containerFrag); - frag.append("UNION\n"); - frag.append(updater.listFilesQuery(skipCreatedModified)); + frag.append("UNION").append(StringUtils.isEmpty(filePath) ? "" : " ALL" /*keep duplicate*/).append("\n"); + frag.append(updater.listFilesQuery(skipCreatedModified, filePath)); }); return frag; diff --git a/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java index fc5e86aa57a..760093d8b3b 100644 --- a/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java @@ -27,6 +27,7 @@ import org.labkey.api.collections.NamedObjectList; import org.labkey.api.data.*; import org.labkey.api.dataiterator.DetailedAuditLogDataIterator; +import org.labkey.api.exp.ExperimentException; import org.labkey.api.exp.PropertyColumn; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.PropertyType; @@ -1139,7 +1140,7 @@ public Map moveRows(User user, Container container, Container ta Map response = ExperimentService.get().moveAssayRuns(expRuns.get(c), c, targetContainer, user, auditUserComment, auditType); incrementCounts(allContainerResponse, response); } - catch (IllegalArgumentException e) + catch (IllegalArgumentException | ExperimentException e) { throw new BatchValidationException(new ValidationException(e.getMessage())); } diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 2e8a6b23471..4ca0b31b3ba 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -52,6 +52,7 @@ import org.labkey.api.audit.ExperimentAuditEvent; import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.audit.provider.ContainerAuditProvider; +import org.labkey.api.audit.provider.FileSystemAuditProvider; import org.labkey.api.cache.Cache; import org.labkey.api.cache.CacheLoader; import org.labkey.api.cache.CacheManager; @@ -210,6 +211,7 @@ import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.security.permissions.DeletePermission; import org.labkey.api.security.permissions.ReadPermission; +import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.security.roles.ProjectAdminRole; import org.labkey.api.settings.AppProps; import org.labkey.api.study.Dataset; @@ -258,6 +260,7 @@ import java.io.UncheckedIOException; import java.net.MalformedURLException; import java.net.URI; +import java.nio.file.Files; import java.nio.file.Path; import java.sql.Connection; import java.sql.PreparedStatement; @@ -9761,7 +9764,7 @@ private int splitExperimentRuns(List runs, Map> mo } @Override - public Map moveAssayRuns(@NotNull List assayRuns, Container container, Container targetContainer, User user, String userComment, AuditBehaviorType auditBehavior) + public Map moveAssayRuns(@NotNull List assayRuns, Container container, Container targetContainer, User user, String userComment, AuditBehaviorType auditBehavior) throws ExperimentException { if (assayRuns.isEmpty()) throw new IllegalArgumentException("No assayRuns provided to move operation."); @@ -9816,7 +9819,7 @@ public Map moveAssayRuns(@NotNull List assayR for (List runFileRenameData : assayMoveData.fileMovesByRunId().values()) { for (AbstractAssayProvider.AssayFileMoveData renameData : runFileRenameData) - moveFile(renameData); + moveFile(renameData, container, user); } }, POSTCOMMIT); @@ -9826,7 +9829,7 @@ public Map moveAssayRuns(@NotNull List assayR return assayMoveData.counts(); } - private boolean moveFile(AbstractAssayProvider.AssayFileMoveData renameData) + private boolean moveFile(AbstractAssayProvider.AssayFileMoveData renameData, Container sourceContainer, User user) { String fieldName = renameData.fieldName() == null ? "datafileurl" : renameData.fieldName(); File targetFile = renameData.targetFile(); @@ -9846,18 +9849,10 @@ private boolean moveFile(AbstractAssayProvider.AssayFileMoveData renameData) return false; } } - if (!sourceFile.renameTo(targetFile)) - { - LOG.warn(String.format("Rename of '%s' to '%s' for '%s' assay run '%s' (field: '%s') failed.", - sourceFile.getAbsolutePath(), - targetFile.getAbsolutePath(), - assayName, - runName, - fieldName)); - return false; - } - return true; + + String changeDetail = String.format("'%s' assay run '%s' (field: '%s')", assayName, runName, fieldName); + return moveFileLinkFile(sourceFile, targetFile, sourceContainer, user, changeDetail); } @Override @@ -10033,6 +10028,68 @@ public boolean isLookupToMaterials(DomainProperty dp) return type.isText() || type.isInteger(); } + /** + * + * Move file (post-commit) after moving sample/assay data + */ + public boolean moveFileLinkFile(File sourceFile, File targetFile, Container sourceFileContainer, User user, String actionComment) + { + if (!sourceFile.exists()) + return false; + + // if not otherwise referenced, move the file. Otherwise, copy the file + boolean isMove = getFileReferenceCount(user, sourceFileContainer, sourceFile) == 0; // source record already moved to target folder + FileSystemAuditProvider.FileSystemAuditEvent event = null; + if (isMove) + { + boolean success = sourceFile.renameTo(targetFile); + if (!success) + { + LOG.warn(String.format("Rename of '%s' to '%s' failed for %s" , sourceFile.getAbsolutePath(), targetFile.getAbsolutePath(), actionComment)); + return false; + } + + event = new FileSystemAuditProvider.FileSystemAuditEvent(sourceFileContainer, "File moved to: " + targetFile.getAbsolutePath() + " for " + actionComment); + } + else + { + try + { + Files.copy(sourceFile.toPath(), targetFile.toPath()); + event = new FileSystemAuditProvider.FileSystemAuditEvent(sourceFileContainer, "File copied to: " + targetFile.getAbsolutePath() + " for " + actionComment); + } + catch (IOException e) + { + LOG.warn(String.format("Copy of '%s' to '%s' failed for " + actionComment, sourceFile.getAbsolutePath(), targetFile.getAbsolutePath())); + return false; + } + } + + event.setDirectory(sourceFile.getParentFile().getAbsolutePath()); + event.setFile(sourceFile.getName()); + event.setResourcePath(sourceFile.getAbsolutePath()); + + AuditLogService.get().addEvent(user, event); + return true; + } + + public long getFileReferenceCount(User user, Container container, File file) + { + if (!container.hasPermission(user, UpdatePermission.class)) + throw new UnauthorizedException("You don't have the required permission to perform this action"); + + FileLinkFileListener fileListener = new FileLinkFileListener(); + SQLFragment unionSql = fileListener.listFilesQuery(true, file.getAbsolutePath()); + + return new SqlSelector(CoreSchema.getInstance().getSchema(), unionSql).getRowCount(); + } + + @Override + public boolean canMoveFileReference(User user, Container container, File file, int moveCount) + { + return getFileReferenceCount(user, container, file) <= moveCount; + } + public Map> doMissingFilesCheck(User user, Container container, boolean trackMissingFiles) throws SQLException { if (container == null) diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index b9b43484560..b068c133637 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -1941,7 +1941,7 @@ public Map moveSamples(Collection sample for (List sampleFileRenameData : fileMovesBySampleId.values()) { for (FileFieldRenameData renameData : sampleFileRenameData) - moveFile(renameData); + moveFile(renameData, sourceContainer, user); } }, POSTCOMMIT); @@ -2052,8 +2052,10 @@ private int splitExperimentRuns(List runs, Map return runCount; } + record SampleFileMoveReference(String sourceFilePath, File targetFile, Container sourceContainer, String sampleName) {} + // return the map of file renames - private Map> updateSampleFilePaths(ExpSampleType sampleType, List samples, Container targetContainer, User user) + private Map> updateSampleFilePaths(ExpSampleType sampleType, List samples, Container targetContainer, User user) throws ExperimentException { Map> sampleFileRenames = new HashMap<>(); @@ -2077,6 +2079,8 @@ private Map> updateSampleFilePaths(ExpSampleT return sampleFileRenames; Map hasFileRoot = new HashMap<>(); + Map fileMoveCounts = new HashMap<>(); + Map fileMoveReferences = new HashMap<>(); for (ExpMaterial sample : samples) { boolean hasSourceRoot = hasFileRoot.computeIfAbsent(sample.getContainer(), (container) -> fileService.getFileRoot(container) != null); @@ -2086,10 +2090,20 @@ private Map> updateSampleFilePaths(ExpSampleT for (DomainProperty fileProp : fileDomainProps ) { String sourceFileName = (String) sample.getProperty(fileProp); + if (StringUtils.isBlank(sourceFileName)) + continue; File updatedFile = FileContentService.get().getMoveTargetFile(sourceFileName, sample.getContainer(), targetContainer); if (updatedFile != null) { - FileFieldRenameData renameData = new FileFieldRenameData(sampleType, sample.getName(), fileProp.getName(), new File(sourceFileName), updatedFile); + + if (!fileMoveReferences.containsKey(sourceFileName)) + fileMoveReferences.put(sourceFileName, new SampleFileMoveReference(sourceFileName, updatedFile, sample.getContainer(), sample.getName())); + if (!fileMoveCounts.containsKey(sourceFileName)) + fileMoveCounts.put(sourceFileName, 0); + fileMoveCounts.put(sourceFileName, fileMoveCounts.get(sourceFileName) + 1); + + File sourceFile = new File(sourceFileName); + FileFieldRenameData renameData = new FileFieldRenameData(sampleType, sample.getName(), fileProp.getName(), sourceFile, updatedFile); sampleFileRenames.putIfAbsent(sample.getRowId(), new ArrayList<>()); List fieldRenameData = sampleFileRenames.get(sample.getRowId()); fieldRenameData.add(renameData); @@ -2097,18 +2111,21 @@ private Map> updateSampleFilePaths(ExpSampleT } } - // TODO, support batch fireFileMoveEvent to avoid excessive FileLinkFileListener.hardTableFileLinkColumns calls - for (int sampleId: sampleFileRenames.keySet()) + for (String filePath : fileMoveReferences.keySet()) { - List fieldRenameRecords = sampleFileRenames.get(sampleId); - for (FileFieldRenameData renameData : fieldRenameRecords) - fileService.fireFileMoveEvent(renameData.sourceFile, renameData.targetFile, user, targetContainer); + SampleFileMoveReference ref = fileMoveReferences.get(filePath); + File sourceFile = new File(filePath); + if (!ExperimentServiceImpl.get().canMoveFileReference(user, ref.sourceContainer, sourceFile, fileMoveCounts.get(filePath))) + throw new ExperimentException("Sample " + ref.sampleName + " cannot be moved since it references a shared file: " + sourceFile.getName()); + + // TODO, support batch fireFileMoveEvent to avoid excessive FileLinkFileListener.hardTableFileLinkColumns calls + fileService.fireFileMoveEvent(sourceFile, ref.targetFile, user, targetContainer); } return sampleFileRenames; } - private boolean moveFile(FileFieldRenameData renameData) + private boolean moveFile(FileFieldRenameData renameData, Container sourceContainer, User user) { if (!renameData.targetFile.getParentFile().exists()) { @@ -2131,18 +2148,9 @@ private boolean moveFile(FileFieldRenameData renameData) LOG.warn(errorMsg + e.getMessage()); } } - if (!renameData.sourceFile.renameTo(renameData.targetFile)) - { - LOG.warn(String.format("Rename of '%s' to '%s' for '%s' sample '%s' (field: '%s') failed.", - renameData.sourceFile.getAbsolutePath(), - renameData.targetFile.getAbsolutePath(), - renameData.sampleType.getName(), - renameData.sampleName, - renameData.fieldName)); - return false; - } - return true; + String changeDetail = String.format("'%s' sample '%s' (field: '%s')", renameData.sampleType.getName(), renameData.sampleName, renameData.fieldName); + return ExperimentServiceImpl.get().moveFileLinkFile(renameData.sourceFile, renameData.targetFile, sourceContainer, user, changeDetail); } @Override diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 41f41da54ab..3323988eed5 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -554,7 +554,7 @@ public List> updateRows(User user, Container container, List @Override public Map moveRows(User user, Container container, Container targetContainer, List> rows, BatchValidationException errors, @Nullable Map configParameters, @Nullable Map extraScriptContext) - throws BatchValidationException, QueryUpdateServiceException + throws BatchValidationException { Map allContainerResponse = new HashMap<>(); @@ -576,7 +576,7 @@ public Map moveRows(User user, Container container, Container ta } catch (ExperimentException e) { - throw new QueryUpdateServiceException(e); + throw new BatchValidationException(new ValidationException(e.getMessage())); } }