From af3a89f5924b70215a5231bdb0a7b912d44a6eeb Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Thu, 10 Jul 2025 13:59:26 -0700 Subject: [PATCH 01/18] Add file system audit events for files added to domains and record original names along with modified names --- api/src/org/labkey/api/ApiModule.java | 2 - .../api/action/AbstractFileUploadAction.java | 4 +- .../api/assay/AbstractAssayProvider.java | 24 +++++- .../assay/AbstractTempDirDataCollector.java | 2 +- .../org/labkey/api/assay/AssayFileWriter.java | 76 +++---------------- .../api/assay/DefaultAssayRunCreator.java | 8 ++ .../assay/transform/DataTransformService.java | 3 +- .../api/audit/ExperimentAuditEvent.java | 8 +- .../api/audit/TransactionAuditProvider.java | 11 ++- .../provider/FileSystemAuditProvider.java | 20 +++++ .../api/data/AssayResultsFileConverter.java | 3 +- .../labkey/api/premium/AntiVirusService.java | 3 +- .../api/query/AbstractQueryImportAction.java | 5 +- .../api/query/AbstractQueryUpdateService.java | 20 ++++- api/src/org/labkey/api/util/FileUtil.java | 60 +++++++++++++++ .../api/webdav/AbstractWebdavResource.java | 1 + .../plate/PlateSampleFilePropertyHelper.java | 10 ++- .../src/org/labkey/assay/AssayController.java | 4 +- .../plate/AssayPlateMetadataServiceImpl.java | 2 +- .../experiment/ExperimentAuditProvider.java | 1 + .../org/labkey/experiment/api/ExpRunImpl.java | 2 +- .../experiment/api/SampleTypeServiceImpl.java | 6 ++ .../filecontent/FileContentServiceImpl.java | 3 +- .../pipeline/api/AbstractWorkDirectory.java | 3 +- 24 files changed, 185 insertions(+), 96 deletions(-) diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index 7faf690a140..f9bd4b34c69 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -25,7 +25,6 @@ import org.labkey.api.action.ApiXmlWriter; import org.labkey.api.action.SpringActionController; import org.labkey.api.admin.SubfolderWriter; -import org.labkey.api.assay.AssayFileWriter; import org.labkey.api.assay.AssayResultsFileWriter; import org.labkey.api.assay.ReplacedRunFilter; import org.labkey.api.assay.sample.MaterialInputRoleComparator; @@ -449,7 +448,6 @@ public void registerServlets(ServletContext servletCtx) Table.IsSelectTestCase.class, ValidEmail.TestCase.class, URIUtil.TestCase.class, - AssayFileWriter.TestCase.class, AssayResultsFileWriter.TestCase.class ); } diff --git a/api/src/org/labkey/api/action/AbstractFileUploadAction.java b/api/src/org/labkey/api/action/AbstractFileUploadAction.java index b9706c32a8c..86f01abd5ca 100644 --- a/api/src/org/labkey/api/action/AbstractFileUploadAction.java +++ b/api/src/org/labkey/api/action/AbstractFileUploadAction.java @@ -15,6 +15,8 @@ */ package org.labkey.api.action; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.FileUtil; import org.labkey.api.util.PageFlowUtil; @@ -28,8 +30,6 @@ import org.springframework.web.multipart.MultipartHttpServletRequest; import org.springframework.web.servlet.ModelAndView; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileOutputStream; diff --git a/api/src/org/labkey/api/assay/AbstractAssayProvider.java b/api/src/org/labkey/api/assay/AbstractAssayProvider.java index 423c76b42c7..226d1b83521 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayProvider.java +++ b/api/src/org/labkey/api/assay/AbstractAssayProvider.java @@ -31,6 +31,7 @@ import org.labkey.api.assay.transform.DataExchangeHandler; import org.labkey.api.assay.transform.DataTransformService; import org.labkey.api.audit.AuditLogService; +import org.labkey.api.audit.provider.FileSystemAuditProvider; import org.labkey.api.data.ActionButton; import org.labkey.api.data.ButtonBar; import org.labkey.api.data.ColumnInfo; @@ -136,7 +137,6 @@ import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; -import java.util.ListIterator; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -1798,7 +1798,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) throws ExperimentException + public void moveRuns(List runs, Container targetContainer, User user, AssayMoveData assayMoveData) throws ExperimentException { if (runs.isEmpty()) return; @@ -1991,6 +1991,11 @@ private void moveRunsBatch(List runs, Container sourceContainer, Contain 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); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(sourceContainer, "File moved from " + sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); + event.setProvidedFileName(sourceFile.getName()); + event.setFile(ref.updatedFile.getName()); + event.setDirectory(ref.updatedFile.getParent()); + AuditLogService.get().addEvent(user, event); } } @@ -2072,6 +2077,11 @@ private void updateRunFiles(List runs, Container sourceContainer, Contai 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); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(sourceContainer, "File moved from " + sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); + event.setProvidedFileName(sourceFile.getName()); + event.setFile(ref.updatedFile.getName()); + event.setDirectory(ref.updatedFile.getParent()); + AuditLogService.get().addEvent(user, event); } } @@ -2116,6 +2126,11 @@ private void updateDataFileUrl(List runs, Container sourceContainer, Con 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); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(sourceContainer, "File moved from " + sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); + event.setProvidedFileName(sourceFile.getName()); + event.setFile(updatedFile.getName()); + event.setDirectory(updatedFile.getParent()); + AuditLogService.get().addEvent(user, event); } } catch (Exception e) @@ -2234,6 +2249,11 @@ private void updateResultFiles(FilteredTable assayResultTable, List runs .append(" WHERE rowId ").appendInClause(fileMoveResultRowIds.get(sourceFileName), realTable.getSqlDialect()); new SqlExecutor(assayResultTable.getSchema()).execute(updateSql); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(sourceContainer, "File moved from " + sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); + event.setProvidedFileName(sourceFile.getName()); + event.setFile(updatedFile.getName()); + event.setDirectory(updatedFile.getParent()); + AuditLogService.get().addEvent(user, event); } } diff --git a/api/src/org/labkey/api/assay/AbstractTempDirDataCollector.java b/api/src/org/labkey/api/assay/AbstractTempDirDataCollector.java index ea9592fe28f..5abca709506 100644 --- a/api/src/org/labkey/api/assay/AbstractTempDirDataCollector.java +++ b/api/src/org/labkey/api/assay/AbstractTempDirDataCollector.java @@ -169,7 +169,7 @@ protected FileLike getFileTargetDir(ContextType context) throws ExperimentExcept protected FileLike getFilePath(ContextType context, @Nullable ExpRun run, FileLike tempDirFile) throws ExperimentException { FileLike assayDir = ensureUploadDirectory(context.getContainer()); - return findUniqueFileName(tempDirFile.getName(), assayDir); + return FileUtil.findUniqueFileName(tempDirFile.getName(), assayDir); } // This is the default case to move the primary file from the temp directory to the assayData directory diff --git a/api/src/org/labkey/api/assay/AssayFileWriter.java b/api/src/org/labkey/api/assay/AssayFileWriter.java index 67c6bd9c8df..557349312eb 100644 --- a/api/src/org/labkey/api/assay/AssayFileWriter.java +++ b/api/src/org/labkey/api/assay/AssayFileWriter.java @@ -20,8 +20,8 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; -import org.junit.Assert; -import org.junit.Test; +import org.labkey.api.audit.AuditLogService; +import org.labkey.api.audit.provider.FileSystemAuditProvider; import org.labkey.api.collections.CollectionUtils; import org.labkey.api.data.Container; import org.labkey.api.exp.ExperimentException; @@ -180,57 +180,6 @@ protected static PipeRoot getPipelineRoot(Container container) return pipelineRoot; } - public static File findUniqueFileName(String originalFilename, File dir) - { - if (originalFilename == null || originalFilename.isEmpty()) - { - originalFilename = "[unnamed]"; - } - File file; - int uniquifier = 0; - do - { - String fullName = getAppendedFileName(originalFilename, uniquifier); - file = FileUtil.appendName(dir, fullName); - uniquifier++; - } - while (file.exists()); - return file; - } - - public static FileLike findUniqueFileName(String originalFilename, FileLike dir) - { - if (originalFilename == null || originalFilename.isEmpty()) - { - originalFilename = "[unnamed]"; - } - FileLike file; - int uniquifier = 0; - do - { - String fullName = getAppendedFileName(originalFilename, uniquifier); - file = dir.resolveChild(fullName); - uniquifier++; - } - while (file.exists()); - return file; - } - - public static String getAppendedFileName(String originalFilename, int uniquifier) - { - String prefix = originalFilename; - String suffix = ""; - - int index = originalFilename.indexOf('.'); - if (index != -1) - { - prefix = originalFilename.substring(0, index); - suffix = originalFilename.substring(index); - } - - return prefix + (uniquifier == 0 ? "" : "-" + uniquifier) + suffix; - } - protected FileLike getFileTargetDir(ContextType context) throws ExperimentException { return ensureUploadDirectory(context.getContainer()); @@ -297,7 +246,10 @@ public Map savePostedFiles(ContextType context, Set pa boolean isAfterFirstFile = false; for (MultipartFile multipartFile : multipartFiles) { + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(); + String fileName = getFileName(multipartFile); + event.setProvidedFileName(fileName); if (!fileName.isEmpty() && !originalFileNames.add(fileName)) { throw new ExperimentException("The file '" + fileName + " ' was uploaded twice - all files must be unique"); @@ -305,8 +257,11 @@ public Map savePostedFiles(ContextType context, Set pa if (!multipartFile.isEmpty()) { FileLike dir = getFileTargetDir(context); - FileLike file = findUniqueFileName(fileName, dir); + FileLike file = FileUtil.findUniqueFileName(fileName, dir); + event.setFile(file.getName()); + event.setDirectory(dir.getName()); multipartFile.transferTo(toFileForWrite(file)); + AuditLogService.get().addEvent(context.getUser(), event); if (!isAfterFirstFile) // first file gets stored with multipartFile's name { files.put(multipartFile.getName(), file); @@ -345,7 +300,7 @@ public Map savePostedFiles(ContextType context, Set pa public FileLike safeDuplicate(ViewContext context, FileLike file) throws ExperimentException { FileLike dir = ensureUploadDirectory(context.getContainer()); - FileLike newFile = findUniqueFileName(file.getName(), dir); + FileLike newFile = FileUtil.findUniqueFileName(file.getName(), dir); try { FileUtils.copyFile(toFileForRead(file), newFile.openOutputStream()); @@ -357,15 +312,4 @@ public FileLike safeDuplicate(ViewContext context, FileLike file) throws Experim } } - public static class TestCase extends Assert - { - @Test - public void testGetAppendedFileName() - { - String originalFilename = "test.txt"; - assertEquals("test.txt", getAppendedFileName(originalFilename, 0)); - assertEquals("test-1.txt", getAppendedFileName(originalFilename, 1)); - assertEquals("test-2.txt", getAppendedFileName(originalFilename, 2)); - } - } } diff --git a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java index 82c29ade74b..ae2eaa8eef0 100644 --- a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java +++ b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java @@ -30,6 +30,7 @@ import org.labkey.api.assay.transform.DataTransformService; import org.labkey.api.assay.transform.TransformDataHandler; import org.labkey.api.assay.transform.TransformResult; +import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; @@ -67,8 +68,10 @@ import org.labkey.api.exp.property.ValidatorContext; import org.labkey.api.pipeline.PipelineService; import org.labkey.api.pipeline.PipelineValidationException; +import org.labkey.api.query.AbstractQueryUpdateService; import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.PropertyValidationError; +import org.labkey.api.query.QueryService; import org.labkey.api.query.SimpleValidationError; import org.labkey.api.query.ValidationError; import org.labkey.api.query.ValidationException; @@ -298,6 +301,11 @@ public ExpExperiment saveExperimentRun( DbScope scope = ExperimentService.get().getSchema().getScope(); try (DbScope.Transaction transaction = scope.ensureTransaction(ExperimentService.get().getProtocolImportLock())) { + if (transaction.getAuditId() == null) + { + TransactionAuditProvider.TransactionAuditEvent auditEvent = AbstractQueryUpdateService.createTransactionAuditEvent(container, context.getReRunId() == null ? QueryService.AuditAction.UPDATE : QueryService.AuditAction.INSERT); + AbstractQueryUpdateService.addTransactionAuditEvent(transaction, context.getUser(), auditEvent); + } boolean saveBatchProps = forceSaveBatchProps; // Add any material/data inputs related to the specimen IDs, etc in the incoming data. diff --git a/api/src/org/labkey/api/assay/transform/DataTransformService.java b/api/src/org/labkey/api/assay/transform/DataTransformService.java index 4c5fbcab145..5b65275fc45 100644 --- a/api/src/org/labkey/api/assay/transform/DataTransformService.java +++ b/api/src/org/labkey/api/assay/transform/DataTransformService.java @@ -3,7 +3,6 @@ import jakarta.servlet.http.HttpServletRequest; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.labkey.api.assay.AssayFileWriter; import org.labkey.api.assay.AssayProvider; import org.labkey.api.assay.AssayRunUploadContext; import org.labkey.api.assay.AssayService; @@ -280,7 +279,7 @@ private FileLike getScriptDir(ExpProtocol protocol, File scriptFile, boolean isD FileUtil.mkdir(tempRoot); FileLike tempParent = tempRoot.resolveChild("AssayId_" + protocol.getRowId()); - FileLike tempFolder = AssayFileWriter.findUniqueFileName("work", tempParent); + FileLike tempFolder = FileUtil.findUniqueFileName("work", tempParent); if (!tempFolder.exists()) FileUtil.mkdirs(tempFolder); diff --git a/api/src/org/labkey/api/audit/ExperimentAuditEvent.java b/api/src/org/labkey/api/audit/ExperimentAuditEvent.java index a6096c02cd2..3dec5354005 100644 --- a/api/src/org/labkey/api/audit/ExperimentAuditEvent.java +++ b/api/src/org/labkey/api/audit/ExperimentAuditEvent.java @@ -18,11 +18,16 @@ public class ExperimentAuditEvent extends AuditTypeEvent /** Important for reflection-based instantiation */ @SuppressWarnings("unused") - public ExperimentAuditEvent() {} + public ExperimentAuditEvent() + { + super(); + setTransactionId(TransactionAuditProvider.getCurrentTransactionAuditId()); + } public ExperimentAuditEvent(Container container, String comment) { super(EVENT_TYPE, container, comment); + setTransactionId(TransactionAuditProvider.getCurrentTransactionAuditId()); } public String getProtocolLsid() @@ -96,6 +101,7 @@ public Map getAuditLogMessageElements() elements.put("message", getMessage()); elements.put("qcState", getQcState()); elements.put("userComment", getUserComment()); + elements.put("transactionId", getTransactionId()); elements.putAll(super.getAuditLogMessageElements()); return elements; } diff --git a/api/src/org/labkey/api/audit/TransactionAuditProvider.java b/api/src/org/labkey/api/audit/TransactionAuditProvider.java index 164867fa90e..2d0d5fa79ea 100644 --- a/api/src/org/labkey/api/audit/TransactionAuditProvider.java +++ b/api/src/org/labkey/api/audit/TransactionAuditProvider.java @@ -5,6 +5,7 @@ import org.labkey.api.audit.query.DefaultAuditTypeTable; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.DbScope; import org.labkey.api.data.JdbcType; import org.labkey.api.data.MutableColumnInfo; import org.labkey.api.data.PropertyStorageSpec; @@ -35,11 +36,12 @@ public class TransactionAuditProvider extends AbstractAuditTypeProvider implemen static final List defaultVisibleColumns = new ArrayList<>(); static { + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_ROW_ID)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_TRANSACTION_TYPE)); + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_START_TIME)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CREATED)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CREATED_BY)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_IMPERSONATED_BY)); - defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_START_TIME)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_PROJECT_ID)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CONTAINER)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_COMMENT)); @@ -98,6 +100,13 @@ public List getDefaultVisibleColumns() return defaultVisibleColumns; } + public static Long getCurrentTransactionAuditId() + { + if (null == DbScope.getLabKeyScope().getCurrentTransaction()) + return null; + return DbScope.getLabKeyScope().getCurrentTransaction().getAuditId(); + } + public static class TransactionAuditEvent extends AuditTypeEvent { private Date _startTime; diff --git a/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java b/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java index 9734c2c4ca2..0607a6e7d7f 100644 --- a/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java +++ b/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java @@ -18,6 +18,7 @@ import org.labkey.api.audit.AbstractAuditTypeProvider; import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.audit.AuditTypeProvider; +import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.data.Container; import org.labkey.api.exp.PropertyDescriptor; @@ -42,6 +43,7 @@ public class FileSystemAuditProvider extends AbstractAuditTypeProvider implement public static final String COLUMN_NAME_DIRECTORY = "Directory"; public static final String COLUMN_NAME_FILE = "File"; + public static final String COLUMN_NAME_PROVIDED_FILE = "ProvidedFileName"; public static final String COLUMN_NAME_RESOURCE_PATH = "ResourcePath"; static final List defaultVisibleColumns = new ArrayList<>(); @@ -53,6 +55,7 @@ public class FileSystemAuditProvider extends AbstractAuditTypeProvider implement defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_IMPERSONATED_BY)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_DIRECTORY)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_FILE)); + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_PROVIDED_FILE)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_COMMENT)); } @@ -106,16 +109,19 @@ public static class FileSystemAuditEvent extends AuditTypeEvent private String _directory; // the directory name private String _file; // the file name private String _resourcePath; // the webdav resource path + private String _providedFileName; // the name of the file as provided by the user, before renaming to make it unique and/or legal /** Important for reflection-based instantiation */ public FileSystemAuditEvent() { super(); + setTransactionId(TransactionAuditProvider.getCurrentTransactionAuditId()); } public FileSystemAuditEvent(Container container, String comment) { super(EVENT_TYPE, container, comment); + setTransactionId(TransactionAuditProvider.getCurrentTransactionAuditId()); } public String getDirectory() @@ -148,6 +154,16 @@ public void setResourcePath(String resourcePath) _resourcePath = resourcePath; } + public String getProvidedFileName() + { + return _providedFileName; + } + + public void setProvidedFileName(String providedFileName) + { + _providedFileName = providedFileName; + } + @Override public Map getAuditLogMessageElements() { @@ -155,6 +171,8 @@ public Map getAuditLogMessageElements() elements.put("directory", getDirectory()); elements.put("file", getFile()); elements.put("resourcePath", getResourcePath()); + elements.put("providedFileName", getProvidedFileName()); + elements.put("transactionId", getTransactionId()); elements.putAll(super.getAuditLogMessageElements()); return elements; } @@ -174,7 +192,9 @@ public FileSystemAuditDomainKind() Set fields = new LinkedHashSet<>(); fields.add(createPropertyDescriptor(COLUMN_NAME_DIRECTORY, PropertyType.STRING)); fields.add(createPropertyDescriptor(COLUMN_NAME_FILE, PropertyType.STRING)); + fields.add(createPropertyDescriptor(COLUMN_NAME_PROVIDED_FILE, PropertyType.STRING)); fields.add(createPropertyDescriptor(COLUMN_NAME_RESOURCE_PATH, PropertyType.STRING)); + fields.add(createPropertyDescriptor(COLUMN_NAME_TRANSACTION_ID, PropertyType.BIGINT)); _fields = Collections.unmodifiableSet(fields); } diff --git a/api/src/org/labkey/api/data/AssayResultsFileConverter.java b/api/src/org/labkey/api/data/AssayResultsFileConverter.java index f079bbfb45a..659da4c6f8f 100644 --- a/api/src/org/labkey/api/data/AssayResultsFileConverter.java +++ b/api/src/org/labkey/api/data/AssayResultsFileConverter.java @@ -1,6 +1,5 @@ package org.labkey.api.data; -import org.labkey.api.assay.AssayFileWriter; import org.labkey.api.assay.AssayResultsFileWriter; import org.labkey.api.exp.api.ExpRun; import org.labkey.api.util.FileUtil; @@ -36,7 +35,7 @@ public Object convert(Class type, Object value) for (int i = 0; i < 5; i++) // try up to 5 times to find a case-sensitive match { - String resultsFileName = AssayFileWriter.getAppendedFileName(valueStr, i); + String resultsFileName = FileUtil.getAppendedFileName(valueStr, i); File resultsFile = FileUtil.appendName(runRoot, resultsFileName); if (!resultsFile.exists() || !URIUtil.isDescendant(runRoot.toURI(), resultsFile.toURI())) break; diff --git a/api/src/org/labkey/api/premium/AntiVirusService.java b/api/src/org/labkey/api/premium/AntiVirusService.java index 06f21056dc1..20a31e5d897 100644 --- a/api/src/org/labkey/api/premium/AntiVirusService.java +++ b/api/src/org/labkey/api/premium/AntiVirusService.java @@ -108,6 +108,7 @@ default void warnAndAuditLog(Logger log, String logmessage, ViewBackgroundInfo i if (null != info.getURL()) event.setDirectory(info.getURL().getPath()); event.setFile(originalName); + event.setProvidedFileName(originalName); AuditLogService.get().addEvent(info.getUser(), event); } @@ -192,4 +193,4 @@ public String getFileName() /** originalName and user are used for error reporting and logging */ ScanResult scan(@NotNull AntiVirusService.Scannable scannable, ViewBackgroundInfo info); -} \ No newline at end of file +} diff --git a/api/src/org/labkey/api/query/AbstractQueryImportAction.java b/api/src/org/labkey/api/query/AbstractQueryImportAction.java index e5ada4d6baa..f2add325252 100644 --- a/api/src/org/labkey/api/query/AbstractQueryImportAction.java +++ b/api/src/org/labkey/api/query/AbstractQueryImportAction.java @@ -28,7 +28,6 @@ import org.labkey.api.action.ExtFormResponseWriter; import org.labkey.api.action.FormApiAction; import org.labkey.api.action.SpringActionController; -import org.labkey.api.assay.AssayFileWriter; import org.labkey.api.attachments.FileAttachmentFile; import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.data.Container; @@ -554,7 +553,7 @@ else if (getViewContext().getRequest() instanceof MultipartHttpServletRequest) FileLike dataFileDir = root.getRootFileLike().resolveChild("QueryImportFiles"); if (dataFileDir.isFile()) { - dataFileDir = AssayFileWriter.findUniqueFileName("QueryImportFiles", root.getRootFileLike()); + dataFileDir = FileUtil.findUniqueFileName("QueryImportFiles", root.getRootFileLike()); if (!FileUtil.mkdir(dataFileDir)) throw new RuntimeException("Error attempting to create directory " + dataFileDir.toNioPathForRead() + " for uploaded query import file " + multipartfile.getOriginalFilename()); @@ -566,7 +565,7 @@ else if (!dataFileDir.exists()) + " for uploaded query import file " + multipartfile.getOriginalFilename()); } - dataFile = AssayFileWriter.findUniqueFileName(multipartfile.getOriginalFilename(), dataFileDir); + dataFile = FileUtil.findUniqueFileName(multipartfile.getOriginalFilename(), dataFileDir); } multipartfile.transferTo(dataFile.toNioPathForWrite()); diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 4a1ce0f0744..21859f59633 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -32,6 +32,7 @@ import org.labkey.api.attachments.SpringAttachmentFile; import org.labkey.api.audit.AuditLogService; import org.labkey.api.audit.TransactionAuditProvider; +import org.labkey.api.audit.provider.FileSystemAuditProvider; import org.labkey.api.collections.ArrayListMap; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; @@ -1015,15 +1016,26 @@ public static Object saveFile(User user, Container container, String name, Objec { throw new ValidationException("File " + multipartFile.getOriginalFilename() + " for field " + name + " has no content"); } - file = AssayFileWriter.findUniqueFileName(multipartFile.getOriginalFilename(), dir); - file = checkFileUnderRoot(container, file); + file = FileUtil.findUniqueFileName(multipartFile.getOriginalFilename(), dir); + checkFileUnderRoot(container, file); multipartFile.transferTo(toFileForWrite(file)); + + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(container, "Save file " + multipartFile.getOriginalFilename() + " for field " + name + " in folder " + container.getPath()); + event.setProvidedFileName(multipartFile.getOriginalFilename()); + event.setFile(file.getName()); + event.setDirectory(file.getParent().toURI().getPath()); + AuditLogService.get().addEvent(user, event); } else if (value instanceof SpringAttachmentFile saf) { - file = AssayFileWriter.findUniqueFileName(saf.getFilename(), dir); - file = checkFileUnderRoot(container, file); + file = FileUtil.findUniqueFileName(saf.getFilename(), dir); + checkFileUnderRoot(container, file); saf.saveTo(file); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(container, "Save file " + saf.getFilename() + " for field " + name + " in folder " + container.getPath()); + event.setProvidedFileName(saf.getFilename()); + event.setFile(file.getName()); + event.setDirectory(file.getParent().toURI().getPath()); + AuditLogService.get().addEvent(user, event); } } catch (IOException | ExperimentException e) diff --git a/api/src/org/labkey/api/util/FileUtil.java b/api/src/org/labkey/api/util/FileUtil.java index af69a349874..da3a24082f7 100644 --- a/api/src/org/labkey/api/util/FileUtil.java +++ b/api/src/org/labkey/api/util/FileUtil.java @@ -1881,6 +1881,57 @@ else if (!FileUtil.hasCloudScheme(path)) } } + public static File findUniqueFileName(String originalFilename, File dir) + { + if (originalFilename == null || originalFilename.isEmpty()) + { + originalFilename = "[unnamed]"; + } + File file; + int uniquifier = 0; + do + { + String fullName = getAppendedFileName(originalFilename, uniquifier); + file = appendName(dir, fullName); + uniquifier++; + } + while (file.exists()); + return file; + } + + public static FileLike findUniqueFileName(String originalFilename, FileLike dir) + { + if (originalFilename == null || originalFilename.isEmpty()) + { + originalFilename = "[unnamed]"; + } + FileLike file; + int uniquifier = 0; + do + { + String fullName = getAppendedFileName(originalFilename, uniquifier); + file = dir.resolveChild(fullName); + uniquifier++; + } + while (file.exists()); + return file; + } + + public static String getAppendedFileName(String originalFilename, int uniquifier) + { + String prefix = originalFilename; + String suffix = ""; + + int index = originalFilename.indexOf('.'); + if (index != -1) + { + prefix = originalFilename.substring(0, index); + suffix = originalFilename.substring(index); + } + + return prefix + (uniquifier == 0 ? "" : "-" + uniquifier) + suffix; + } + /* If you have a write once, read once text file/stream, you can use this class. * It wraps the calls to create and delete a temp file, and also will use @@ -2316,5 +2367,14 @@ public void testNoAcceptableExtensions() assertNull("No extension should be allowed, but wasn't", checkExtension("No extension", mockProps)); assertNull("Numeric extension should be allowed", checkExtension("test.1", mockProps)); } + + @Test + public void testGetAppendedFileName() + { + String originalFilename = "test.txt"; + assertEquals("test.txt", getAppendedFileName(originalFilename, 0)); + assertEquals("test-1.txt", getAppendedFileName(originalFilename, 1)); + assertEquals("test-2.txt", getAppendedFileName(originalFilename, 2)); + } } } diff --git a/api/src/org/labkey/api/webdav/AbstractWebdavResource.java b/api/src/org/labkey/api/webdav/AbstractWebdavResource.java index 9bb1ccafa7c..8cc47aeff6d 100644 --- a/api/src/org/labkey/api/webdav/AbstractWebdavResource.java +++ b/api/src/org/labkey/api/webdav/AbstractWebdavResource.java @@ -658,6 +658,7 @@ else if ("dirDeleteFailed".equalsIgnoreCase(message)) event.setDirectory(dir); event.setFile(name); + event.setProvidedFileName(name); event.setResourcePath(getPath().toString()); AuditLogService.get().addEvent(context.getUser(), event); diff --git a/assay/api-src/org/labkey/api/assay/plate/PlateSampleFilePropertyHelper.java b/assay/api-src/org/labkey/api/assay/plate/PlateSampleFilePropertyHelper.java index 12040e7ae65..33b67ee6cb2 100644 --- a/assay/api-src/org/labkey/api/assay/plate/PlateSampleFilePropertyHelper.java +++ b/assay/api-src/org/labkey/api/assay/plate/PlateSampleFilePropertyHelper.java @@ -23,6 +23,8 @@ import org.labkey.api.assay.actions.AssayRunUploadForm; import org.labkey.api.assay.actions.PlateUploadForm; import org.labkey.api.assay.nab.NabUrls; +import org.labkey.api.audit.AuditLogService; +import org.labkey.api.audit.provider.FileSystemAuditProvider; import org.labkey.api.data.Container; import org.labkey.api.data.DataRegion; import org.labkey.api.data.RenderContext; @@ -38,6 +40,7 @@ import org.labkey.api.reader.ExcelLoader; import org.labkey.api.security.User; import org.labkey.api.study.assay.SampleMetadataInputFormat; +import org.labkey.api.util.FileUtil; import org.labkey.api.util.HtmlString; import org.labkey.api.util.JavaScriptFragment; import org.labkey.api.util.LinkBuilder; @@ -164,11 +167,16 @@ protected File getSampleMetadata(HttpServletRequest request) throws ExperimentEx try { FileLike uploadDirectory = AssayFileWriter.ensureUploadDirectory(_container); - _metadataFile = AssayFileWriter.findUniqueFileName(metadata.getOriginalFilename(), uploadDirectory).toNioPathForWrite().toFile(); + _metadataFile = FileUtil.findUniqueFileName(metadata.getOriginalFilename(), uploadDirectory).toNioPathForWrite().toFile(); try (BufferedOutputStream fos = new BufferedOutputStream(new FileOutputStream(_metadataFile)); InputStream is = metadata.getInputStream()) { IOUtils.copy(is, fos); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(_container, "Adding sample metadata file " + metadata.getOriginalFilename() + "."); + event.setProvidedFileName(metadata.getOriginalFilename()); + event.setFile(_metadataFile.getName()); + event.setDirectory(_metadataFile.getParent()); + AuditLogService.get().addEvent(null, event); } } catch (IOException e) diff --git a/assay/src/org/labkey/assay/AssayController.java b/assay/src/org/labkey/assay/AssayController.java index de99bd8c05a..88d44e2ccd8 100644 --- a/assay/src/org/labkey/assay/AssayController.java +++ b/assay/src/org/labkey/assay/AssayController.java @@ -760,7 +760,7 @@ public ApiResponse execute(SimpleApiJsonForm form, BindException errors) throws if (f.exists()) { duplicate = true; - FileLike newFile = AssayFileWriter.findUniqueFileName(fileName, targetDirectory); + FileLike newFile = FileUtil.findUniqueFileName(fileName, targetDirectory); newFileNames.add(i, newFile.getName()); // will infer duplication by whether an element exists at that position or not ExpData expData = ExperimentService.get().getExpDataByURL(f.toNioPathForRead().toFile(), null); List runNames = new ArrayList<>(); @@ -822,7 +822,7 @@ protected File getTargetFile(String filename) throws IOException try { FileLike targetDirectory = AssayFileWriter.ensureUploadDirectory(getContainer()); - return AssayFileWriter.findUniqueFileName(filename, targetDirectory).toNioPathForWrite().toFile(); + return FileUtil.findUniqueFileName(filename, targetDirectory).toNioPathForWrite().toFile(); } catch (ExperimentException e) { diff --git a/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java b/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java index 912f8185966..d4c346372fb 100644 --- a/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java +++ b/assay/src/org/labkey/assay/plate/AssayPlateMetadataServiceImpl.java @@ -442,7 +442,7 @@ public DataIteratorBuilder mergeReRunData( // replace the contents of the uploaded data file with the new combined data FileLike dir = dataFile.getParent() != null ? dataFile.getParent() : AssayFileWriter.ensureUploadDirectory(container); String newName = FileUtil.getBaseName(dataFile.toNioPathForRead().toFile()) + ".tsv"; - FileLike newPath = AssayFileWriter.findUniqueFileName(newName, dir); + FileLike newPath = FileUtil.findUniqueFileName(newName, dir); try (TSVMapWriter writer = new TSVMapWriter(newRows)) { writer.write(newPath.toNioPathForWrite().toFile()); diff --git a/experiment/src/org/labkey/experiment/ExperimentAuditProvider.java b/experiment/src/org/labkey/experiment/ExperimentAuditProvider.java index 1a9e1a59aa3..a61d9ded474 100644 --- a/experiment/src/org/labkey/experiment/ExperimentAuditProvider.java +++ b/experiment/src/org/labkey/experiment/ExperimentAuditProvider.java @@ -201,6 +201,7 @@ public ExperimentAuditDomainKind() fields.add(createPropertyDescriptor(COLUMN_NAME_MESSAGE, PropertyType.STRING)); fields.add(createPropertyDescriptor(COLUMN_NAME_QCSTATE, PropertyType.INTEGER)); fields.add(createPropertyDescriptor(COLUMN_NAME_USER_COMMENT, PropertyType.STRING)); + fields.add(createPropertyDescriptor(COLUMN_NAME_TRANSACTION_ID, PropertyType.BIGINT)); _fields = Collections.unmodifiableSet(fields); } diff --git a/experiment/src/org/labkey/experiment/api/ExpRunImpl.java b/experiment/src/org/labkey/experiment/api/ExpRunImpl.java index e07a64df7e2..4ffccf76c40 100644 --- a/experiment/src/org/labkey/experiment/api/ExpRunImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpRunImpl.java @@ -969,7 +969,7 @@ public void archiveDataFiles(User user) LOG.warn("Unable to create an archive directory - discontinue archive and delete."); return; } - File targetFile = AssayFileWriter.findUniqueFileName(file.getName(), archivedDir); + File targetFile = FileUtil.findUniqueFileName(file.getName(), archivedDir); targetFile = FileUtil.getAbsoluteCaseSensitiveFile(targetFile); FileUtils.moveFile(file, targetFile); expData.setDataFileURI(targetFile.toURI()); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index 8068ef0714a..090b03533b2 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -30,6 +30,7 @@ import org.labkey.api.audit.DetailedAuditTypeEvent; import org.labkey.api.audit.SampleTimelineAuditEvent; import org.labkey.api.audit.TransactionAuditProvider; +import org.labkey.api.audit.provider.FileSystemAuditProvider; import org.labkey.api.cache.Cache; import org.labkey.api.cache.CacheManager; import org.labkey.api.data.AuditConfigurable; @@ -2137,6 +2138,11 @@ private Map> updateSampleFilePaths(ExpSampleT // TODO, support batch fireFileMoveEvent to avoid excessive FileLinkFileListener.hardTableFileLinkColumns calls fileService.fireFileMoveEvent(sourceFile, ref.targetFile, user, targetContainer); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(ref.sourceContainer, "File moved from " + ref.sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); + event.setProvidedFileName(sourceFile.getName()); + event.setFile(ref.targetFile.getName()); + event.setDirectory(ref.targetFile.getParent()); + AuditLogService.get().addEvent(user, event); } return sampleFileRenames; diff --git a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java index bc46de40b3e..436f9ff9940 100644 --- a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java +++ b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java @@ -27,7 +27,6 @@ import org.junit.Test; import org.labkey.api.action.SpringActionController; import org.labkey.api.admin.AdminUrls; -import org.labkey.api.assay.AssayFileWriter; import org.labkey.api.attachments.AttachmentDirectory; import org.labkey.api.cache.Cache; import org.labkey.api.cache.CacheManager; @@ -1668,7 +1667,7 @@ public File getMoveTargetFile(String absoluteFilePath, @NotNull Container source String targetPath = absoluteFilePath.replace(sourceRootPath, targetFileRoot.getAbsolutePath()); File targetFile = new File(targetPath); - return AssayFileWriter.findUniqueFileName(file.getName(), targetFile.getParentFile()); + return FileUtil.findUniqueFileName(file.getName(), targetFile.getParentFile()); } @Override diff --git a/pipeline/src/org/labkey/pipeline/api/AbstractWorkDirectory.java b/pipeline/src/org/labkey/pipeline/api/AbstractWorkDirectory.java index 480ed9fe743..803641f78ad 100644 --- a/pipeline/src/org/labkey/pipeline/api/AbstractWorkDirectory.java +++ b/pipeline/src/org/labkey/pipeline/api/AbstractWorkDirectory.java @@ -609,8 +609,7 @@ public void remove(boolean success) throws IOException { if (!success && _transferToDirOnFailure != null) { - AssayFileWriter writer = new AssayFileWriter(); - File dest = writer.findUniqueFileName(_dir.getName(), _transferToDirOnFailure); + File dest = FileUtil.findUniqueFileName(_dir.getName(), _transferToDirOnFailure); _jobLog.debug("after failure, moving working directory to: " + dest.getPath()); try From e66e4d480868264f80d2bd8f7e34541fd2081180 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Thu, 10 Jul 2025 15:23:27 -0700 Subject: [PATCH 02/18] Conditional creation of audit event in savePostedFiles --- .../org/labkey/api/assay/AssayFileWriter.java | 16 +++++++++------- .../labkey/api/assay/AssayResultsFileWriter.java | 4 ++-- .../api/assay/FileUploadDataCollector.java | 2 +- .../api/assay/actions/AssayRunUploadForm.java | 2 +- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/api/src/org/labkey/api/assay/AssayFileWriter.java b/api/src/org/labkey/api/assay/AssayFileWriter.java index 557349312eb..34ef3df20af 100644 --- a/api/src/org/labkey/api/assay/AssayFileWriter.java +++ b/api/src/org/labkey/api/assay/AssayFileWriter.java @@ -228,7 +228,7 @@ public String getFileName(MultipartFile file) return file.getOriginalFilename(); } - public Map savePostedFiles(ContextType context, Set parameterNames, boolean allowMultiple, boolean ensureExpData) throws ExperimentException, IOException + public Map savePostedFiles(ContextType context, Set parameterNames, boolean allowMultiple, boolean ensureExpData, boolean createAuditEvent) throws ExperimentException, IOException { Map files = CollectionUtils.enforceValueClass(new TreeMap<>(), FileLike.class); Set originalFileNames = new HashSet<>(); @@ -246,10 +246,7 @@ public Map savePostedFiles(ContextType context, Set pa boolean isAfterFirstFile = false; for (MultipartFile multipartFile : multipartFiles) { - FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(); - String fileName = getFileName(multipartFile); - event.setProvidedFileName(fileName); if (!fileName.isEmpty() && !originalFileNames.add(fileName)) { throw new ExperimentException("The file '" + fileName + " ' was uploaded twice - all files must be unique"); @@ -258,10 +255,15 @@ public Map savePostedFiles(ContextType context, Set pa { FileLike dir = getFileTargetDir(context); FileLike file = FileUtil.findUniqueFileName(fileName, dir); - event.setFile(file.getName()); - event.setDirectory(dir.getName()); multipartFile.transferTo(toFileForWrite(file)); - AuditLogService.get().addEvent(context.getUser(), event); + if (createAuditEvent) + { + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(context.getContainer(), "File provided for assay import"); + event.setProvidedFileName(fileName); + event.setFile(file.getName()); + event.setDirectory(dir.getParent().toURI().getPath()); + AuditLogService.get().addEvent(context.getUser(), event); + } if (!isAfterFirstFile) // first file gets stored with multipartFile's name { files.put(multipartFile.getName(), file); diff --git a/api/src/org/labkey/api/assay/AssayResultsFileWriter.java b/api/src/org/labkey/api/assay/AssayResultsFileWriter.java index 1c6b1bd7023..1d1a9a279bf 100644 --- a/api/src/org/labkey/api/assay/AssayResultsFileWriter.java +++ b/api/src/org/labkey/api/assay/AssayResultsFileWriter.java @@ -147,7 +147,7 @@ public Map savePostedFiles(ContextType context) throws Experim } } - Map files = super.savePostedFiles(context, Collections.singleton(FILE_INPUT_NAME), true, true); + Map files = super.savePostedFiles(context, Collections.singleton(FILE_INPUT_NAME), true, true, false); // if no files were written to the targetDir, delete the empty directory if (files.isEmpty()) @@ -211,4 +211,4 @@ public void testGetFileNameWithoutPath() assertEquals("file.txt", getFileNameWithoutPath("file.txt")); } } -} \ No newline at end of file +} diff --git a/api/src/org/labkey/api/assay/FileUploadDataCollector.java b/api/src/org/labkey/api/assay/FileUploadDataCollector.java index 8b6238d617f..2d20222894a 100644 --- a/api/src/org/labkey/api/assay/FileUploadDataCollector.java +++ b/api/src/org/labkey/api/assay/FileUploadDataCollector.java @@ -109,7 +109,7 @@ public Map createData(ContextType context) throws IOException, fileInputIndex++; } - Map files = savePostedFiles(context, fileInputs, false, false); + Map files = savePostedFiles(context, fileInputs, false, false, false); // Figure out if we have any data files to work with - boolean foundFiles = files.containsKey(_fileInputName); diff --git a/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java b/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java index b1930453c2d..d38465a8460 100644 --- a/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java +++ b/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java @@ -340,7 +340,7 @@ public Map getAdditionalPostedFiles(List(); - Map postedFiles = writer.savePostedFiles(this, fileParameters.keySet(), false, false); + Map postedFiles = writer.savePostedFiles(this, fileParameters.keySet(), false, false, true); for (Map.Entry entry : postedFiles.entrySet()) _additionalFiles.put(fileParameters.get(entry.getKey()), entry.getValue()); From 6fbc40fbeff6a040d15d4a3c08c27ea6fb3babab Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Fri, 11 Jul 2025 10:47:16 -0700 Subject: [PATCH 03/18] General cleanup and add log in proper place when tempdirectory is in use --- .../assay/AbstractTempDirDataCollector.java | 38 +++++--------- .../org/labkey/api/assay/AssayFileWriter.java | 8 ++- .../api/assay/AssayResultsFileWriter.java | 2 +- .../api/assay/DefaultAssayRunCreator.java | 49 +++++++++++-------- .../api/assay/FileUploadDataCollector.java | 4 +- .../api/assay/actions/AssayRunUploadForm.java | 2 +- .../provider/FileSystemAuditProvider.java | 1 + .../api/query/AbstractQueryUpdateService.java | 11 +++-- .../experiment/api/ExperimentServiceImpl.java | 3 +- 9 files changed, 58 insertions(+), 60 deletions(-) diff --git a/api/src/org/labkey/api/assay/AbstractTempDirDataCollector.java b/api/src/org/labkey/api/assay/AbstractTempDirDataCollector.java index 5abca709506..904bdfea196 100644 --- a/api/src/org/labkey/api/assay/AbstractTempDirDataCollector.java +++ b/api/src/org/labkey/api/assay/AbstractTempDirDataCollector.java @@ -16,16 +16,18 @@ package org.labkey.api.assay; import org.apache.commons.io.FileUtils; -import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; import org.labkey.api.assay.actions.AssayRunUploadForm; +import org.labkey.api.audit.AuditLogService; +import org.labkey.api.audit.provider.FileSystemAuditProvider; import org.labkey.api.exp.ExperimentException; import org.labkey.api.exp.api.ExpData; import org.labkey.api.exp.api.ExpRun; import org.labkey.api.query.BatchValidationException; import org.labkey.api.util.FileUtil; import org.labkey.api.util.NetworkDrive; +import org.labkey.api.util.logging.LogHelper; import org.labkey.vfs.FileLike; import java.io.File; @@ -43,9 +45,8 @@ public abstract class AbstractTempDirDataCollector> extends AbstractAssayDataCollector { protected boolean _uploadComplete = false; - private static final String TMPFILE = "tmp"; - private static final Logger LOG = LogManager.getLogger(AbstractTempDirDataCollector.class); + private static final Logger LOG = LogHelper.getLogger(AbstractTempDirDataCollector.class, "Activity related to data collection into a temporary directory"); private void removeTempDir(ContextType context) throws ExperimentException { @@ -53,7 +54,7 @@ private void removeTempDir(ContextType context) throws ExperimentException if (!(context instanceof AssayRunUploadForm)) return; - String uploadAttemptID = ((AssayRunUploadForm)context).getUploadAttemptID(); + String uploadAttemptID = ((AssayRunUploadForm) context).getUploadAttemptID(); // Cleanup files other than input generated by transform scripts FileLike tempDir = ensureSubdirectory(context.getContainer(), TEMP_DIR_NAME); @@ -71,24 +72,6 @@ private void removeTempDir(ContextType context) throws ExperimentException } } - private void removeFiles(ContextType context, Collection> assayFiles) throws ExperimentException - { - for (Map fileMap : assayFiles) - { - for (File file : fileMap.values()) - { - FileLike assayFile = ensureUploadDirectory(context.getContainer(), DIR_NAME).resolveChild(file.getName()); - FileUtils.deleteQuietly(assayFile.toNioPathForWrite().toFile()); - } - } - } - - protected void uploadFailed(ContextType context, List> assayFiles) throws ExperimentException - { - removeTempDir(context); - removeFiles(context, assayFiles); - } - @Override public void initDir(ContextType context) throws ExperimentException { @@ -99,7 +82,7 @@ public void initDir(ContextType context) throws ExperimentException if (!(context instanceof AssayRunUploadForm)) return; - String uploadAttemptID = ((AssayRunUploadForm)context).getUploadAttemptID(); + String uploadAttemptID = ((AssayRunUploadForm)context).getUploadAttemptID(); // Cleanup transform script output files if generated by warning feature FileLike tempDir = AssayFileWriter.ensureSubdirectory(context.getContainer(), TEMP_DIR_NAME); @@ -140,7 +123,7 @@ protected FileLike getFileTargetDir(ContextType context) throws ExperimentExcept try { FileLike tempDir = ensureSubdirectory(context.getContainer(), TEMP_DIR_NAME); - uploadAttemptDir = tempDir.resolveChild(((AssayRunUploadForm)context).getUploadAttemptID()); + uploadAttemptDir = tempDir.resolveChild(((AssayRunUploadForm)context).getUploadAttemptID()); if (!NetworkDrive.exists(uploadAttemptDir)) { @@ -229,10 +212,15 @@ public Map uploadComplete(ContextType context, @Nullable ExpRu run.save(context.getUser()); } handleTempFile(tempDirFile.toNioPathForWrite().toFile(), assayDirFile.toNioPathForWrite().toFile()); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(context.getContainer(), "Primary file provided for assay import"); + event.setProvidedFileName(tempDirFile.getName()); + event.setFile(assayDirFile.getName()); + event.setDirectory(assayDirFile.getParent().toURI().getPath()); + AuditLogService.get().addEvent(context.getUser(), event); } else { - LOG.warn("Unable to resolve import/upload file location for file: " + tempDirFile); + LOG.warn("Unable to resolve import/upload file location for file: {}", tempDirFile); } } FileUtils.deleteDirectory(tempDir.toNioPathForWrite().toFile()); diff --git a/api/src/org/labkey/api/assay/AssayFileWriter.java b/api/src/org/labkey/api/assay/AssayFileWriter.java index 34ef3df20af..7b1553f589f 100644 --- a/api/src/org/labkey/api/assay/AssayFileWriter.java +++ b/api/src/org/labkey/api/assay/AssayFileWriter.java @@ -32,9 +32,7 @@ import org.labkey.api.util.FileUtil; import org.labkey.api.util.NetworkDrive; import org.labkey.api.view.ViewContext; - import org.labkey.vfs.FileLike; - import org.labkey.vfs.FileSystemLike; import org.springframework.web.multipart.MultipartFile; import org.springframework.web.multipart.MultipartHttpServletRequest; @@ -228,7 +226,7 @@ public String getFileName(MultipartFile file) return file.getOriginalFilename(); } - public Map savePostedFiles(ContextType context, Set parameterNames, boolean allowMultiple, boolean ensureExpData, boolean createAuditEvent) throws ExperimentException, IOException + public Map savePostedFiles(ContextType context, Set parameterNames, boolean allowMultiple, boolean ensureExpData) throws ExperimentException, IOException { Map files = CollectionUtils.enforceValueClass(new TreeMap<>(), FileLike.class); Set originalFileNames = new HashSet<>(); @@ -256,9 +254,9 @@ public Map savePostedFiles(ContextType context, Set pa FileLike dir = getFileTargetDir(context); FileLike file = FileUtil.findUniqueFileName(fileName, dir); multipartFile.transferTo(toFileForWrite(file)); - if (createAuditEvent) + if (!dir.toURI().getPath().contains(TEMP_DIR_NAME)) { - FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(context.getContainer(), "File provided for assay import"); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(context.getContainer(), allowMultiple ? "File field provided for assay import" : "Primary file provided for assay import"); event.setProvidedFileName(fileName); event.setFile(file.getName()); event.setDirectory(dir.getParent().toURI().getPath()); diff --git a/api/src/org/labkey/api/assay/AssayResultsFileWriter.java b/api/src/org/labkey/api/assay/AssayResultsFileWriter.java index 1d1a9a279bf..007b68e1ff4 100644 --- a/api/src/org/labkey/api/assay/AssayResultsFileWriter.java +++ b/api/src/org/labkey/api/assay/AssayResultsFileWriter.java @@ -147,7 +147,7 @@ public Map savePostedFiles(ContextType context) throws Experim } } - Map files = super.savePostedFiles(context, Collections.singleton(FILE_INPUT_NAME), true, true, false); + Map files = super.savePostedFiles(context, Collections.singleton(FILE_INPUT_NAME), true, true); // if no files were written to the targetDir, delete the empty directory if (files.isEmpty()) diff --git a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java index ae2eaa8eef0..c87050409f3 100644 --- a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java +++ b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java @@ -148,33 +148,42 @@ public Pair saveExperimentRun( AssayProvider provider = context.getProvider(); ExpProtocol protocol = context.getProtocol(); ExpRun run = null; - context.init(); - // Check if assay protocol is configured to import in the background. - // Issue 26811: If we don't have a view, assume that we are on a background job thread already. - boolean importInBackground = forceAsync || (provider.isBackgroundUpload(protocol) && HttpView.hasCurrentView()); - if (!importInBackground) + try (DbScope.Transaction transaction = ExperimentService.get().getSchema().getScope().ensureTransaction()) { - if ((Object)context.getUploadedData().get(AssayDataCollector.PRIMARY_FILE) instanceof File errFile) + if (transaction.getAuditId() == null) { - throw new ClassCastException("FileLike expected: " + errFile + " context: " + context.getClass() + " " + context); + TransactionAuditProvider.TransactionAuditEvent auditEvent = AbstractQueryUpdateService.createTransactionAuditEvent(context.getContainer(), context.getReRunId() == null ? QueryService.AuditAction.UPDATE : QueryService.AuditAction.INSERT); + AbstractQueryUpdateService.addTransactionAuditEvent(transaction, context.getUser(), auditEvent); } - FileLike primaryFile = context.getUploadedData().get(AssayDataCollector.PRIMARY_FILE); - run = AssayService.get().createExperimentRun(context.getName(), context.getContainer(), protocol, null==primaryFile ? null : primaryFile.toNioPathForRead().toFile()); - run.setComments(context.getComments()); - run.setWorkflowTaskId(context.getWorkflowTask()); + context.init(); + // Check if assay protocol is configured to import in the background. + // Issue 26811: If we don't have a view, assume that we are on a background job thread already. + boolean importInBackground = forceAsync || (provider.isBackgroundUpload(protocol) && HttpView.hasCurrentView()); + if (!importInBackground) + { + if ((Object) context.getUploadedData().get(AssayDataCollector.PRIMARY_FILE) instanceof File errFile) + { + throw new ClassCastException("FileLike expected: " + errFile + " context: " + context.getClass() + " " + context); + } + FileLike primaryFile = context.getUploadedData().get(AssayDataCollector.PRIMARY_FILE); + run = AssayService.get().createExperimentRun(context.getName(), context.getContainer(), protocol, null == primaryFile ? null : primaryFile.toNioPathForRead().toFile()); + run.setComments(context.getComments()); + run.setWorkflowTaskId(context.getWorkflowTask()); - exp = saveExperimentRun(context, exp, run, false); + exp = saveExperimentRun(context, exp, run, false); - // re-fetch the run after it has been fully constructed - run = ExperimentService.get().getExpRun(run.getRowId()); + // re-fetch the run after it has been fully constructed + run = ExperimentService.get().getExpRun(run.getRowId()); - context.uploadComplete(run); - } - else - { - context.uploadComplete(null); - exp = saveExperimentRunAsync(context, exp); + context.uploadComplete(run); + } + else + { + context.uploadComplete(null); + exp = saveExperimentRunAsync(context, exp); + } + transaction.commit(); } return Pair.of(exp, run); diff --git a/api/src/org/labkey/api/assay/FileUploadDataCollector.java b/api/src/org/labkey/api/assay/FileUploadDataCollector.java index 2d20222894a..2f018065ee8 100644 --- a/api/src/org/labkey/api/assay/FileUploadDataCollector.java +++ b/api/src/org/labkey/api/assay/FileUploadDataCollector.java @@ -37,7 +37,7 @@ */ public class FileUploadDataCollector> extends AbstractTempDirDataCollector { - private int _maxFileInputs; + private final int _maxFileInputs; private final Map _reusableFiles; // Name of the form for the file. private final String _fileInputName; @@ -109,7 +109,7 @@ public Map createData(ContextType context) throws IOException, fileInputIndex++; } - Map files = savePostedFiles(context, fileInputs, false, false, false); + Map files = savePostedFiles(context, fileInputs, false, false); // Figure out if we have any data files to work with - boolean foundFiles = files.containsKey(_fileInputName); diff --git a/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java b/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java index d38465a8460..b1930453c2d 100644 --- a/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java +++ b/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java @@ -340,7 +340,7 @@ public Map getAdditionalPostedFiles(List(); - Map postedFiles = writer.savePostedFiles(this, fileParameters.keySet(), false, false, true); + Map postedFiles = writer.savePostedFiles(this, fileParameters.keySet(), false, false); for (Map.Entry entry : postedFiles.entrySet()) _additionalFiles.put(fileParameters.get(entry.getKey()), entry.getValue()); diff --git a/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java b/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java index 0607a6e7d7f..6eb3ef0edd0 100644 --- a/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java +++ b/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java @@ -115,6 +115,7 @@ public static class FileSystemAuditEvent extends AuditTypeEvent public FileSystemAuditEvent() { super(); + setEventType(EVENT_TYPE); setTransactionId(TransactionAuditProvider.getCurrentTransactionAuditId()); } diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 21859f59633..be9fda830ce 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -1004,11 +1004,14 @@ public static Object saveFile(User user, Container container, String name, Objec */ public static Object saveFile(User user, Container container, String name, Object value, @Nullable FileLike dirPath) throws ValidationException, QueryUpdateServiceException { + String auditMessageFormat = "Saved file '%s' for field '%s' in folder %s."; FileLike file = null; try { FileLike dir = AssayFileWriter.ensureUploadDirectory(dirPath); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(); + event.setContainer(container); if (value instanceof MultipartFile multipartFile) { // Once we've found one, write it to disk and replace the row's value with just the File reference to it @@ -1019,24 +1022,22 @@ public static Object saveFile(User user, Container container, String name, Objec file = FileUtil.findUniqueFileName(multipartFile.getOriginalFilename(), dir); checkFileUnderRoot(container, file); multipartFile.transferTo(toFileForWrite(file)); - - FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(container, "Save file " + multipartFile.getOriginalFilename() + " for field " + name + " in folder " + container.getPath()); + event.setComment(String.format(auditMessageFormat, multipartFile.getOriginalFilename(), name, container.getPath())); event.setProvidedFileName(multipartFile.getOriginalFilename()); event.setFile(file.getName()); event.setDirectory(file.getParent().toURI().getPath()); - AuditLogService.get().addEvent(user, event); } else if (value instanceof SpringAttachmentFile saf) { file = FileUtil.findUniqueFileName(saf.getFilename(), dir); checkFileUnderRoot(container, file); saf.saveTo(file); - FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(container, "Save file " + saf.getFilename() + " for field " + name + " in folder " + container.getPath()); + event.setComment(String.format(auditMessageFormat, saf.getFilename(), name, container.getPath())); event.setProvidedFileName(saf.getFilename()); event.setFile(file.getName()); event.setDirectory(file.getParent().toURI().getPath()); - AuditLogService.get().addEvent(user, event); } + AuditLogService.get().addEvent(user, event); } catch (IOException | ExperimentException e) { diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index a7ac6304235..d5abd5cc0c2 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -10056,7 +10056,8 @@ public boolean moveFileLinkFile(File sourceFile, File targetFile, Container sour } event.setDirectory(sourceFile.getParentFile().getAbsolutePath()); - event.setFile(sourceFile.getName()); + event.setFile(targetFile.getName()); + event.setProvidedFileName(sourceFile.getName()); event.setResourcePath(sourceFile.getAbsolutePath()); AuditLogService.get().addEvent(user, event); From d58b0a3d5390d8a3b87b6c2ee83a4a43d942d215 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Mon, 14 Jul 2025 14:35:15 -0700 Subject: [PATCH 04/18] Minor code tidying --- .../api/audit/query/DefaultAuditTypeTable.java | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/api/src/org/labkey/api/audit/query/DefaultAuditTypeTable.java b/api/src/org/labkey/api/audit/query/DefaultAuditTypeTable.java index 61e7497a9f0..31d549e1e7e 100644 --- a/api/src/org/labkey/api/audit/query/DefaultAuditTypeTable.java +++ b/api/src/org/labkey/api/audit/query/DefaultAuditTypeTable.java @@ -98,7 +98,7 @@ public DefaultAuditTypeTable(AuditTypeProvider provider, TableInfo storage, User created.setLabel("Date"); created.setFormat("DateTime"); - var container = getMutableColumn("Container"); + getMutableColumn("Container"); var project = getMutableColumn("ProjectId"); project.setLabel("Project"); @@ -178,22 +178,6 @@ protected ColumnInfo resolveColumn(String name) return a; } - - // Now check for 'Property/...' columns - if (name.equalsIgnoreCase("Property")) - { - // UNDONE: backwards compat to "Property/*" columns -// col = new BaseColumnInfo("Property", this); -// col.setFk(new LookupForeignKey() -// { -// @Override -// public TableInfo getLookupTableInfo() -// { -// return new VirtualPropertiesTable(); -// } -// }); - } - // Other legacy audit columns return null; From db581b20c974d8c602e7f73365a8855d267458d8 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Mon, 14 Jul 2025 15:52:43 -0700 Subject: [PATCH 05/18] @labkey/components v6.56.1-fileNameAudit.1 and add timeline event details for original data --- .../labkey/experiment/samples/SampleTimelineAuditProvider.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/samples/SampleTimelineAuditProvider.java b/experiment/src/org/labkey/experiment/samples/SampleTimelineAuditProvider.java index 57f150a4375..439a2201a65 100644 --- a/experiment/src/org/labkey/experiment/samples/SampleTimelineAuditProvider.java +++ b/experiment/src/org/labkey/experiment/samples/SampleTimelineAuditProvider.java @@ -173,7 +173,7 @@ public SampleTimelineAuditDomainKind() fields.add(createPropertyDescriptor(COLUMN_NAME_USER_COMMENT, PropertyType.STRING, -1)); fields.add(createOldDataMapPropertyDescriptor()); fields.add(createNewDataMapPropertyDescriptor()); - fields.add(createPropertyDescriptor(COLUMN_NAME_TRANSACTION_ID, PropertyType.BIGINT)); // varchar max + fields.add(createPropertyDescriptor(COLUMN_NAME_TRANSACTION_ID, PropertyType.BIGINT)); _fields = Collections.unmodifiableSet(fields); } From 082094f93f14cd92c05c17688b8231abede0058d Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Mon, 14 Jul 2025 15:53:35 -0700 Subject: [PATCH 06/18] For async assay import, set the transaction id in the context so all events are tied together --- .../labkey/api/assay/AssayRunUploadContext.java | 11 +++++++++++ .../api/assay/AssayRunUploadContextImpl.java | 10 ++++++++++ .../labkey/api/assay/DefaultAssayRunCreator.java | 16 +++++++++++++--- .../api/assay/pipeline/AssayRunAsyncContext.java | 8 ++++++++ .../audit/provider/FileSystemAuditProvider.java | 15 +++++++++++++++ .../api/query/AbstractQueryUpdateService.java | 2 ++ 6 files changed, 59 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/assay/AssayRunUploadContext.java b/api/src/org/labkey/api/assay/AssayRunUploadContext.java index 4c65940801c..c64fd23022d 100644 --- a/api/src/org/labkey/api/assay/AssayRunUploadContext.java +++ b/api/src/org/labkey/api/assay/AssayRunUploadContext.java @@ -150,6 +150,10 @@ default boolean isAllowLookupByAlternateKey() String getTargetStudy(); + default Long getTransactionAuditId() { return null; } + + default void setTransactionAuditId(Long transactionAuditId) { } + default String getAuditUserComment() { return null; } TransformResult getTransformResult(); @@ -240,6 +244,7 @@ abstract class Factory _uploadedData; protected String _jobDescription; protected String _jobNotificationProvider; + protected Long _transactionAuditId; protected String _auditUserComment; public Factory( @@ -410,6 +415,12 @@ public FACTORY setAuditUserComment(String auditUserComment) return self(); } + public FACTORY setTransactionAuditId(Long transactionAuditId) + { + _transactionAuditId = transactionAuditId; + return self(); + } + /** FACTORY and self() make it easier to chain setters while returning the correct subclass type. */ public abstract FACTORY self(); diff --git a/api/src/org/labkey/api/assay/AssayRunUploadContextImpl.java b/api/src/org/labkey/api/assay/AssayRunUploadContextImpl.java index 29ec65e980d..485cac54cde 100644 --- a/api/src/org/labkey/api/assay/AssayRunUploadContextImpl.java +++ b/api/src/org/labkey/api/assay/AssayRunUploadContextImpl.java @@ -80,6 +80,7 @@ public class AssayRunUploadContextImpl imple private final boolean _allowLookupByAlternateKey; private final String _auditUserComment; + // Lazily created fields private Map _uploadedData; private Map _runProperties; @@ -93,6 +94,7 @@ public class AssayRunUploadContextImpl imple protected String _jobDescription; protected String _jobNotificationProvider; protected String _pipelineJobGUID; + private Long _transactionAuditId; private boolean _autoFillDefaultResultColumns; @@ -395,6 +397,8 @@ public String getTargetStudy() return _targetStudy; } + @Override + public Long getTransactionAuditId() { return _transactionAuditId; } @Override public String getAuditUserComment() { return _auditUserComment; } @@ -440,6 +444,12 @@ public void setPipelineJobGUID(String jobGUID) _pipelineJobGUID = jobGUID; } + @Override + public void setTransactionAuditId(Long transactionAuditId) + { + _transactionAuditId = transactionAuditId; + } + @Override public Logger getLogger() { diff --git a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java index c87050409f3..b73de6969a9 100644 --- a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java +++ b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java @@ -149,7 +149,7 @@ public Pair saveExperimentRun( ExpProtocol protocol = context.getProtocol(); ExpRun run = null; - try (DbScope.Transaction transaction = ExperimentService.get().getSchema().getScope().ensureTransaction()) + try (DbScope.Transaction transaction = ExperimentService.get().getSchema().getScope().ensureTransaction(ExperimentService.get().getProtocolImportLock())) { if (transaction.getAuditId() == null) { @@ -181,6 +181,7 @@ public Pair saveExperimentRun( else { context.uploadComplete(null); + context.setTransactionAuditId(transaction.getAuditId()); exp = saveExperimentRunAsync(context, exp); } transaction.commit(); @@ -312,8 +313,17 @@ public ExpExperiment saveExperimentRun( { if (transaction.getAuditId() == null) { - TransactionAuditProvider.TransactionAuditEvent auditEvent = AbstractQueryUpdateService.createTransactionAuditEvent(container, context.getReRunId() == null ? QueryService.AuditAction.UPDATE : QueryService.AuditAction.INSERT); - AbstractQueryUpdateService.addTransactionAuditEvent(transaction, context.getUser(), auditEvent); + var auditAction = context.getReRunId() == null ? QueryService.AuditAction.UPDATE : QueryService.AuditAction.INSERT; + if (context.getTransactionAuditId() != null) + { + var auditEvent = new TransactionAuditProvider.TransactionAuditEvent(container, auditAction, context.getTransactionAuditId()); + transaction.setAuditEvent(auditEvent); + } + else + { + var auditEvent = AbstractQueryUpdateService.createTransactionAuditEvent(container, auditAction); + AbstractQueryUpdateService.addTransactionAuditEvent(transaction, context.getUser(), auditEvent); + } } boolean saveBatchProps = forceSaveBatchProps; diff --git a/api/src/org/labkey/api/assay/pipeline/AssayRunAsyncContext.java b/api/src/org/labkey/api/assay/pipeline/AssayRunAsyncContext.java index eaa4369ebc2..8f7fd242587 100644 --- a/api/src/org/labkey/api/assay/pipeline/AssayRunAsyncContext.java +++ b/api/src/org/labkey/api/assay/pipeline/AssayRunAsyncContext.java @@ -93,6 +93,7 @@ public class AssayRunAsyncContext implements // async fields protected String _jobDescription; protected String _jobNotificationProvider; + protected Long _transactionAuditId; // For serialization protected AssayRunAsyncContext() @@ -128,6 +129,7 @@ public AssayRunAsyncContext(AssayRunUploadContext originalContext) _jobDescription = originalContext.getJobDescription(); _jobNotificationProvider = originalContext.getJobNotificationProvider(); + _transactionAuditId = originalContext.getTransactionAuditId(); } /** Convert to a map that can be serialized - DomainProperty can't be */ @@ -423,6 +425,12 @@ public String getJobDescription() return _jobDescription; } + @Override + public Long getTransactionAuditId() + { + return _transactionAuditId; + } + @Override public String getJobNotificationProvider() { diff --git a/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java b/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java index 6eb3ef0edd0..b103f6ac0b8 100644 --- a/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java +++ b/api/src/org/labkey/api/audit/provider/FileSystemAuditProvider.java @@ -44,6 +44,7 @@ public class FileSystemAuditProvider extends AbstractAuditTypeProvider implement public static final String COLUMN_NAME_DIRECTORY = "Directory"; public static final String COLUMN_NAME_FILE = "File"; public static final String COLUMN_NAME_PROVIDED_FILE = "ProvidedFileName"; + public static final String COLUMN_NAME_FIELD_NAME = "FieldName"; public static final String COLUMN_NAME_RESOURCE_PATH = "ResourcePath"; static final List defaultVisibleColumns = new ArrayList<>(); @@ -56,6 +57,7 @@ public class FileSystemAuditProvider extends AbstractAuditTypeProvider implement defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_DIRECTORY)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_FILE)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_PROVIDED_FILE)); + defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_FIELD_NAME)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_COMMENT)); } @@ -110,6 +112,7 @@ public static class FileSystemAuditEvent extends AuditTypeEvent private String _file; // the file name private String _resourcePath; // the webdav resource path private String _providedFileName; // the name of the file as provided by the user, before renaming to make it unique and/or legal + private String _fieldName; // name of the field associated with the file, if any /** Important for reflection-based instantiation */ public FileSystemAuditEvent() @@ -165,6 +168,16 @@ public void setProvidedFileName(String providedFileName) _providedFileName = providedFileName; } + public String getFieldName() + { + return _fieldName; + } + + public void setFieldName(String fieldName) + { + _fieldName = fieldName; + } + @Override public Map getAuditLogMessageElements() { @@ -173,6 +186,7 @@ public Map getAuditLogMessageElements() elements.put("file", getFile()); elements.put("resourcePath", getResourcePath()); elements.put("providedFileName", getProvidedFileName()); + elements.put("fieldName", getFieldName()); elements.put("transactionId", getTransactionId()); elements.putAll(super.getAuditLogMessageElements()); return elements; @@ -194,6 +208,7 @@ public FileSystemAuditDomainKind() fields.add(createPropertyDescriptor(COLUMN_NAME_DIRECTORY, PropertyType.STRING)); fields.add(createPropertyDescriptor(COLUMN_NAME_FILE, PropertyType.STRING)); fields.add(createPropertyDescriptor(COLUMN_NAME_PROVIDED_FILE, PropertyType.STRING)); + fields.add(createPropertyDescriptor(COLUMN_NAME_FIELD_NAME, PropertyType.STRING)); fields.add(createPropertyDescriptor(COLUMN_NAME_RESOURCE_PATH, PropertyType.STRING)); fields.add(createPropertyDescriptor(COLUMN_NAME_TRANSACTION_ID, PropertyType.BIGINT)); _fields = Collections.unmodifiableSet(fields); diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index be9fda830ce..a4a9db138a9 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -1025,6 +1025,7 @@ public static Object saveFile(User user, Container container, String name, Objec event.setComment(String.format(auditMessageFormat, multipartFile.getOriginalFilename(), name, container.getPath())); event.setProvidedFileName(multipartFile.getOriginalFilename()); event.setFile(file.getName()); + event.setFieldName(name); event.setDirectory(file.getParent().toURI().getPath()); } else if (value instanceof SpringAttachmentFile saf) @@ -1035,6 +1036,7 @@ else if (value instanceof SpringAttachmentFile saf) event.setComment(String.format(auditMessageFormat, saf.getFilename(), name, container.getPath())); event.setProvidedFileName(saf.getFilename()); event.setFile(file.getName()); + event.setFieldName(name); event.setDirectory(file.getParent().toURI().getPath()); } AuditLogService.get().addEvent(user, event); From 639879b7ddb25148b2d350c327b666f57c9fe4a4 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Tue, 15 Jul 2025 08:42:54 -0700 Subject: [PATCH 07/18] Add audit event for run file uploading --- assay/src/org/labkey/assay/AssayController.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/assay/src/org/labkey/assay/AssayController.java b/assay/src/org/labkey/assay/AssayController.java index 88d44e2ccd8..44debe9ceef 100644 --- a/assay/src/org/labkey/assay/AssayController.java +++ b/assay/src/org/labkey/assay/AssayController.java @@ -66,6 +66,7 @@ import org.labkey.api.assay.transform.DataExchangeHandler; import org.labkey.api.assay.transform.DataTransformService; import org.labkey.api.audit.AuditLogService; +import org.labkey.api.audit.provider.FileSystemAuditProvider; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.CompareType; import org.labkey.api.data.Container; @@ -848,6 +849,11 @@ public String getResponse(AssayFileUploadForm form, Map Date: Tue, 22 Jul 2025 14:17:10 -0700 Subject: [PATCH 08/18] Set transaction id in audit event constructors --- .../api/audit/SampleTimelineAuditEvent.java | 7 ++++++- .../experiment/ExperimentAuditProvider.java | 4 ++++ .../experiment/SampleTypeAuditProvider.java | 5 +++++ .../experiment/api/SampleTypeServiceImpl.java | 20 ++++++------------- .../api/SampleTypeUpdateServiceDI.java | 3 --- .../labkey/list/model/ListAuditProvider.java | 9 ++++++++- .../org/labkey/query/QueryServiceImpl.java | 3 --- .../query/audit/QueryUpdateAuditProvider.java | 8 +++++++- 8 files changed, 36 insertions(+), 23 deletions(-) diff --git a/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java b/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java index 4af2814acb1..b21fcd012fd 100644 --- a/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java +++ b/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java @@ -92,11 +92,16 @@ public static SampleTimelineEventType getTypeFromName(@Nullable String name) private String _inventoryUpdateType; /** Important for reflection-based instantiation */ - public SampleTimelineAuditEvent() {} + public SampleTimelineAuditEvent() + { + super(); + setTransactionId(TransactionAuditProvider.getCurrentTransactionAuditId()); + } public SampleTimelineAuditEvent(Container container, String comment) { super(EVENT_TYPE, container, comment); + setTransactionId(TransactionAuditProvider.getCurrentTransactionAuditId()); } public String getSampleLsid() diff --git a/experiment/src/org/labkey/experiment/ExperimentAuditProvider.java b/experiment/src/org/labkey/experiment/ExperimentAuditProvider.java index a61d9ded474..af70cc21124 100644 --- a/experiment/src/org/labkey/experiment/ExperimentAuditProvider.java +++ b/experiment/src/org/labkey/experiment/ExperimentAuditProvider.java @@ -167,6 +167,10 @@ else if (COLUMN_NAME_USER_COMMENT.equalsIgnoreCase(col.getName())) { col.setLabel("User Comment"); } + else if (COLUMN_NAME_TRANSACTION_ID.equalsIgnoreCase(col.getName())) + { + col.setLabel("Transaction ID"); + } } }; } diff --git a/experiment/src/org/labkey/experiment/SampleTypeAuditProvider.java b/experiment/src/org/labkey/experiment/SampleTypeAuditProvider.java index 013d85d3886..0ad0a24832a 100644 --- a/experiment/src/org/labkey/experiment/SampleTypeAuditProvider.java +++ b/experiment/src/org/labkey/experiment/SampleTypeAuditProvider.java @@ -18,6 +18,7 @@ import org.labkey.api.audit.AbstractAuditTypeProvider; import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.audit.AuditTypeProvider; +import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; import org.labkey.api.data.Container; @@ -120,14 +121,18 @@ public static class SampleTypeAuditEvent extends AuditTypeEvent private String _sampleSetName; private String _insertUpdateChoice; + /** Important for reflection-based instantiation */ + @SuppressWarnings("unused") public SampleTypeAuditEvent() { super(); + setTransactionId(TransactionAuditProvider.getCurrentTransactionAuditId()); } public SampleTypeAuditEvent(Container container, String comment) { super(EVENT_TYPE, container, comment); + setTransactionId(TransactionAuditProvider.getCurrentTransactionAuditId()); } public String getSourceLsid() diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index 8c6d0d02ff6..db47a7d3a5d 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -631,7 +631,7 @@ public void deleteSampleType(int rowId, Container c, User user, @Nullable String executor.execute("UPDATE " + getTinfoProtocolInput() + " SET materialSourceId = NULL WHERE materialSourceId = ?", source.getRowId()); executor.execute("DELETE FROM " + getTinfoMaterialSource() + " WHERE RowId = ?", rowId); - addSampleTypeDeletedAuditEvent(user, c, source, transaction.getAuditId(), auditUserComment); + addSampleTypeDeletedAuditEvent(user, c, source, auditUserComment); ExperimentService.get().removeDataTypeExclusion(Collections.singleton(rowId), ExperimentService.DataTypeForExclusion.SampleType); ExperimentService.get().removeDataTypeExclusion(Collections.singleton(rowId), ExperimentService.DataTypeForExclusion.DashboardSampleType); @@ -663,19 +663,16 @@ public void deleteSampleType(int rowId, Container c, User user, @Nullable String LOG.info("Deleted SampleType '" + source.getName() + "' from '" + c.getPath() + "' in " + timer.getDuration()); } - private void addSampleTypeDeletedAuditEvent(User user, Container c, ExpSampleType sampleType, Long txAuditId, @Nullable String auditUserComment) + private void addSampleTypeDeletedAuditEvent(User user, Container c, ExpSampleType sampleType, @Nullable String auditUserComment) { - addSampleTypeAuditEvent(user, c, sampleType, txAuditId, String.format("Sample Type deleted: %1$s", sampleType.getName()),auditUserComment, "delete type"); + addSampleTypeAuditEvent(user, c, sampleType, String.format("Sample Type deleted: %1$s", sampleType.getName()),auditUserComment, "delete type"); } - private void addSampleTypeAuditEvent(User user, Container c, ExpSampleType sampleType, Long txAuditId, String comment, @Nullable String auditUserComment, String insertUpdateChoice) + private void addSampleTypeAuditEvent(User user, Container c, ExpSampleType sampleType, String comment, @Nullable String auditUserComment, String insertUpdateChoice) { SampleTypeAuditProvider.SampleTypeAuditEvent event = new SampleTypeAuditProvider.SampleTypeAuditEvent(c, comment); event.setUserComment(auditUserComment); - if (txAuditId != null) - event.setTransactionId(txAuditId); - if (sampleType != null) { event.setSourceLsid(sampleType.getLSID()); @@ -1214,9 +1211,6 @@ private SampleTimelineAuditEvent createAuditRecord(Container c, String comment, { SampleTimelineAuditEvent event = new SampleTimelineAuditEvent(c, comment); event.setUserComment(userComment); - var tx = getExpSchema().getScope().getCurrentTransaction(); - if (tx != null) - event.setTransactionId(tx.getAuditId()); var staticsRow = existingRow != null && !existingRow.isEmpty() ? existingRow : row; if (row != null) @@ -1276,8 +1270,6 @@ else if (event.getSampleLsid() != null) private SampleTimelineAuditEvent createAuditRecord(Container container, String comment, String userComment, ExpMaterial sample, @Nullable Map metadata) { SampleTimelineAuditEvent event = new SampleTimelineAuditEvent(container, comment); - if (getExpSchema().getScope().getCurrentTransaction() != null) - event.setTransactionId(getExpSchema().getScope().getCurrentTransaction().getAuditId()); event.setSampleName(sample.getName()); event.setSampleLsid(sample.getLSID()); event.setSampleId(sample.getRowId()); @@ -1904,9 +1896,9 @@ public Map moveSamples(Collection sample // create summary audit entries for the source and target containers String samplesPhrase = StringUtilsLabKey.pluralize(sampleIds.size(), "sample"); - addSampleTypeAuditEvent(user, sourceContainer, sampleType, transaction.getAuditId(), + addSampleTypeAuditEvent(user, sourceContainer, sampleType, "Moved " + samplesPhrase + " to " + targetContainer.getPath(), userComment, "moved from project"); - addSampleTypeAuditEvent(user, targetContainer, sampleType, transaction.getAuditId(), + addSampleTypeAuditEvent(user, targetContainer, sampleType, "Moved " + samplesPhrase + " from " + sourceContainer.getPath(), userComment, "moved to project"); // move the events associated with the samples that have moved diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index c80f7c0133a..faa739aa4f8 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -1403,9 +1403,6 @@ void audit(QueryService.AuditAction auditAction) assert _sampleType != null || auditAction == QueryService.AuditAction.DELETE : "SampleType required for insert/update, but not required for read/delete"; SampleTypeAuditProvider.SampleTypeAuditEvent event = new SampleTypeAuditProvider.SampleTypeAuditEvent( getContainer(), "Samples " + auditAction.getVerbPastTense() + " in: " + (_sampleType == null ? "" : _sampleType.getName())); - var tx = getSchema().getDbSchema().getScope().getCurrentTransaction(); - if (tx != null) - event.setTransactionId(tx.getAuditId()); if (_sampleType != null) { event.setSourceLsid(_sampleType.getLSID()); diff --git a/list/src/org/labkey/list/model/ListAuditProvider.java b/list/src/org/labkey/list/model/ListAuditProvider.java index 87415abdf58..3f25042cfd8 100644 --- a/list/src/org/labkey/list/model/ListAuditProvider.java +++ b/list/src/org/labkey/list/model/ListAuditProvider.java @@ -19,6 +19,7 @@ import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.audit.AuditTypeProvider; import org.labkey.api.audit.DetailedAuditTypeEvent; +import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; import org.labkey.api.data.Container; @@ -151,7 +152,11 @@ public static class ListAuditEvent extends DetailedAuditTypeEvent /** Important for reflection-based instantiation */ @SuppressWarnings("unused") - public ListAuditEvent() {} + public ListAuditEvent() + { + super(); + setTransactionId(TransactionAuditProvider.getCurrentTransactionAuditId()); + } public ListAuditEvent(Container container, String comment, ListDefinitionImpl list) { @@ -159,6 +164,7 @@ public ListAuditEvent(Container container, String comment, ListDefinitionImpl li setListDomainUri(list.getDomain().getTypeURI()); setListId(list.getListId()); setListName(list.getName()); + setTransactionId(TransactionAuditProvider.getCurrentTransactionAuditId()); } public int getListId() @@ -232,6 +238,7 @@ public ListAuditDomainKind() // to be indexed fields.add(createPropertyDescriptor(COLUMN_NAME_LIST_ITEM_ENTITY_ID, PropertyType.STRING, 300)); // UNDONE: is needed ? .setEntityId(true)); fields.add(createPropertyDescriptor(COLUMN_NAME_LIST_NAME, PropertyType.STRING)); + fields.add(createPropertyDescriptor(COLUMN_NAME_TRANSACTION_ID, PropertyType.BIGINT)); fields.add(createOldDataMapPropertyDescriptor()); fields.add(createNewDataMapPropertyDescriptor()); _fields = Collections.unmodifiableSet(fields); diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index 14850e8cbf6..2678e4de90b 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -3129,9 +3129,6 @@ protected DetailedAuditTypeEvent createDetailedAuditRecord(User user, Container private QueryUpdateAuditProvider.QueryUpdateAuditEvent createAuditRecord(Container c, AuditConfigurable tinfo, String comment, @Nullable Map row, @Nullable Map existingRow) { QueryUpdateAuditProvider.QueryUpdateAuditEvent event = new QueryUpdateAuditProvider.QueryUpdateAuditEvent(c, comment); - DbScope.Transaction tx = tinfo.getSchema().getScope().getCurrentTransaction(); - if (tx != null) - event.setTransactionId(tx.getAuditId()); event.setSchemaName(tinfo.getPublicSchemaName()); event.setQueryName(tinfo.getPublicName()); diff --git a/query/src/org/labkey/query/audit/QueryUpdateAuditProvider.java b/query/src/org/labkey/query/audit/QueryUpdateAuditProvider.java index 60cac79cb11..72249ba8166 100644 --- a/query/src/org/labkey/query/audit/QueryUpdateAuditProvider.java +++ b/query/src/org/labkey/query/audit/QueryUpdateAuditProvider.java @@ -20,6 +20,7 @@ import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.audit.AuditTypeProvider; import org.labkey.api.audit.DetailedAuditTypeEvent; +import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; import org.labkey.api.data.Container; @@ -210,11 +211,16 @@ public static class QueryUpdateAuditEvent extends DetailedAuditTypeEvent /** Important for reflection-based instantiation */ @SuppressWarnings("unused") - public QueryUpdateAuditEvent() {} + public QueryUpdateAuditEvent() + { + super(); + setTransactionId(TransactionAuditProvider.getCurrentTransactionAuditId()); + } public QueryUpdateAuditEvent(Container container, String comment) { super(QUERY_UPDATE_AUDIT_EVENT, container, comment); + setTransactionId(TransactionAuditProvider.getCurrentTransactionAuditId()); } public String getRowPk() From 6a4c5d71dfaa5bae630c131b03e06cff6478723a Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Wed, 23 Jul 2025 10:04:12 -0700 Subject: [PATCH 09/18] Comment --- api/src/org/labkey/api/assay/AssayRunUploadContextImpl.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/src/org/labkey/api/assay/AssayRunUploadContextImpl.java b/api/src/org/labkey/api/assay/AssayRunUploadContextImpl.java index 485cac54cde..f396d721d2e 100644 --- a/api/src/org/labkey/api/assay/AssayRunUploadContextImpl.java +++ b/api/src/org/labkey/api/assay/AssayRunUploadContextImpl.java @@ -94,6 +94,8 @@ public class AssayRunUploadContextImpl imple protected String _jobDescription; protected String _jobNotificationProvider; protected String _pipelineJobGUID; + // Used to assure that the audit logs for the asynchronous assay upload are linked to the ones for + // the file upload, if any, which happen before the async job starts. private Long _transactionAuditId; private boolean _autoFillDefaultResultColumns; From 8590108f9d47b29796eb9b07c27e840a032cb1a1 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Wed, 23 Jul 2025 14:16:32 -0700 Subject: [PATCH 10/18] Remove relabeling of "Created" as "End Time" (since it's not really accurate) --- api/src/org/labkey/api/audit/TransactionAuditProvider.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/api/src/org/labkey/api/audit/TransactionAuditProvider.java b/api/src/org/labkey/api/audit/TransactionAuditProvider.java index 2d0d5fa79ea..0dc6af6e889 100644 --- a/api/src/org/labkey/api/audit/TransactionAuditProvider.java +++ b/api/src/org/labkey/api/audit/TransactionAuditProvider.java @@ -39,7 +39,6 @@ public class TransactionAuditProvider extends AbstractAuditTypeProvider implemen defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_ROW_ID)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_TRANSACTION_TYPE)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_START_TIME)); - defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CREATED)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_CREATED_BY)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_IMPERSONATED_BY)); defaultVisibleColumns.add(FieldKey.fromParts(COLUMN_NAME_PROJECT_ID)); @@ -82,8 +81,6 @@ protected void initColumn(MutableColumnInfo col) col.setLabel("Start Time"); else if (COLUMN_NAME_TRANSACTION_TYPE.equalsIgnoreCase(col.getName())) col.setLabel("Transaction Type"); - else if (COLUMN_NAME_CREATED.equalsIgnoreCase(col.getName())) - col.setLabel("End Time"); } }; } From 5289a83631d4da07a4f02ddcbd23a35e7f6782e1 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Thu, 24 Jul 2025 10:45:19 -0700 Subject: [PATCH 11/18] Use non-default constructor --- api/src/org/labkey/api/query/AbstractQueryUpdateService.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 20438b6201d..4f5f0534ef2 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -1018,8 +1018,7 @@ public static Object saveFile(User user, Container container, String name, Objec { FileLike dir = AssayFileWriter.ensureUploadDirectory(dirPath); - FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(); - event.setContainer(container); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(container, null); if (value instanceof MultipartFile multipartFile) { // Once we've found one, write it to disk and replace the row's value with just the File reference to it From 4f809686154baead231cb358c80b5c772d87c1a2 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Mon, 28 Jul 2025 13:40:36 -0700 Subject: [PATCH 12/18] Fix directory --- api/src/org/labkey/api/assay/AssayFileWriter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/assay/AssayFileWriter.java b/api/src/org/labkey/api/assay/AssayFileWriter.java index 7b1553f589f..f39ae4a02cb 100644 --- a/api/src/org/labkey/api/assay/AssayFileWriter.java +++ b/api/src/org/labkey/api/assay/AssayFileWriter.java @@ -259,7 +259,7 @@ public Map savePostedFiles(ContextType context, Set pa FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(context.getContainer(), allowMultiple ? "File field provided for assay import" : "Primary file provided for assay import"); event.setProvidedFileName(fileName); event.setFile(file.getName()); - event.setDirectory(dir.getParent().toURI().getPath()); + event.setDirectory(dir.toURI().getPath()); AuditLogService.get().addEvent(context.getUser(), event); } if (!isAfterFirstFile) // first file gets stored with multipartFile's name From 7b44453f6c1aaeb83868c79cdaa8480c0a50efe4 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Mon, 28 Jul 2025 13:40:50 -0700 Subject: [PATCH 13/18] Use target file in event not source file --- .../src/org/labkey/experiment/api/ExperimentServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index da18effc589..2ea66d1bdfb 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -10061,7 +10061,7 @@ public boolean moveFileLinkFile(File sourceFile, File targetFile, Container sour } } - event.setDirectory(sourceFile.getParentFile().getAbsolutePath()); + event.setDirectory(targetFile.getParentFile().getAbsolutePath()); event.setFile(targetFile.getName()); event.setProvidedFileName(sourceFile.getName()); event.setResourcePath(sourceFile.getAbsolutePath()); From aac3452dd28bfc5e3f574082ac7fa8d8b090b747 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Mon, 28 Jul 2025 13:41:05 -0700 Subject: [PATCH 14/18] Move common code --- .../org/labkey/api/query/AbstractQueryUpdateService.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 230ab6880e3..bbb54df8096 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -1024,9 +1024,6 @@ public static Object saveFile(User user, Container container, String name, Objec multipartFile.transferTo(toFileForWrite(file)); event.setComment(String.format(auditMessageFormat, multipartFile.getOriginalFilename(), name, container.getPath())); event.setProvidedFileName(multipartFile.getOriginalFilename()); - event.setFile(file.getName()); - event.setFieldName(name); - event.setDirectory(file.getParent().toURI().getPath()); } else if (value instanceof SpringAttachmentFile saf) { @@ -1035,10 +1032,10 @@ else if (value instanceof SpringAttachmentFile saf) saf.saveTo(file); event.setComment(String.format(auditMessageFormat, saf.getFilename(), name, container.getPath())); event.setProvidedFileName(saf.getFilename()); - event.setFile(file.getName()); - event.setFieldName(name); - event.setDirectory(file.getParent().toURI().getPath()); } + event.setFile(file.getName()); + event.setFieldName(name); + event.setDirectory(file.getParent().toURI().getPath()); AuditLogService.get().addEvent(user, event); } catch (IOException | ExperimentException e) From efd74bbe9dd8da0f821f862656e851000b66e59e Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Mon, 28 Jul 2025 16:31:11 -0700 Subject: [PATCH 15/18] Add tests for data set file field auditing --- .../study/StudyDatasetFileFieldTest.java | 96 ++++++++++++++++--- 1 file changed, 85 insertions(+), 11 deletions(-) diff --git a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java index a5a6f7ba5ba..c25aab1ce1b 100644 --- a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java +++ b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java @@ -6,17 +6,21 @@ import org.junit.BeforeClass; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.labkey.remoteapi.CommandException; import org.labkey.test.BaseWebDriverTest; import org.labkey.test.Locator; import org.labkey.test.TestFileUtils; import org.labkey.test.TestTimeoutException; +import org.labkey.test.WebTestHelper; import org.labkey.test.categories.Daily; import org.labkey.test.components.domain.DomainFormPanel; import org.labkey.test.components.ext4.Checkbox; import org.labkey.test.pages.DatasetInsertPage; import org.labkey.test.pages.study.DatasetDesignerPage; import org.labkey.test.params.FieldDefinition; +import org.labkey.test.util.AuditLogHelper; import org.labkey.test.util.DataRegionTable; +import org.labkey.test.util.TestDataGenerator; import java.io.File; import java.io.IOException; @@ -24,6 +28,8 @@ import java.util.List; import java.util.Map; +import static org.junit.Assert.assertEquals; + /* Added the test to provide additional test coverage for below mentioned issue https://www.labkey.org/home/Developer/issues/Secure/issues-details.view?issueId=42309 @@ -33,7 +39,12 @@ @BaseWebDriverTest.ClassTimeout(minutes = 10) public class StudyDatasetFileFieldTest extends BaseWebDriverTest { - private final String IMPORT_PROJECT = "StudyDatasetFileFieldFolderImportProject"; + private static final String EXCLUDED_CHARS = "\""; // this gets encoded as %22 when the form data is sent. + private static final String IMPORT_PROJECT = "StudyDatasetFileFieldFolderImportProject"; + private static final String FILE_FIELD_1 = TestDataGenerator.randomFieldName("File Field 1", EXCLUDED_CHARS); + private static final String FILE_FIELD_2 = TestDataGenerator.randomFieldName("File Field 2", EXCLUDED_CHARS); + private static final String INT_FIELD = TestDataGenerator.randomFieldName("Int Field", EXCLUDED_CHARS); + private static final String TEXT_FIELD = TestDataGenerator.randomFieldName("Text Field", EXCLUDED_CHARS); @BeforeClass public static void doSetup() @@ -71,14 +82,14 @@ private void doCreateSteps() protected void doCleanup(boolean afterTest) throws TestTimeoutException { _containerHelper.deleteProject(getProjectName(), afterTest); - _containerHelper.deleteProject(IMPORT_PROJECT, afterTest); + _containerHelper.deleteProject(IMPORT_PROJECT, false); } @Test - public void testFileField() throws IOException + public void testFileField() throws IOException, CommandException { String datasetName = "Dataset-1"; - File inputFile = TestFileUtils.getSampleData("fileTypes/sample.txt"); //random file + File inputFile = TestFileUtils.getSampleData("fileTypes/sample.txt"); //arbitrary file goToProjectHome(); createDataset(datasetName); @@ -90,10 +101,13 @@ public void testFileField() throws IOException insertDataPage.insert(Map.of("ParticipantId", "1", "SequenceNum", "2", "date", "2020-08-04", - "fileField", inputFile.toString(), - "textField", "Hello World..!", - "intField", "25")); - + FILE_FIELD_1, inputFile.toString(), + TEXT_FIELD, "Hello World..!", + INT_FIELD, "25")); + verifyFileAuditLogs(List.of(Map.of( + AuditLogHelper.COL_FILE_AUDIT_FILE, inputFile.getName(), + AuditLogHelper.COL_FILE_AUDIT_PROVIDED_FILE, inputFile.getName() + ))); log("Edit the dataset"); DataRegionTable table = new DataRegionTable("Dataset", getDriver()); table.clickEditRow(0); @@ -160,6 +174,65 @@ public void testFileField() throws IOException clickButton("Submit"); } + @Test + public void testFileRenamingAndAuditing() throws IOException, CommandException + { + String datasetName = "Dataset-Multi-File"; + File inputFile = TestFileUtils.getSampleData("fileTypes/csv_sample.csv"); + goToProjectHome(); + createDataset(datasetName); + + DatasetInsertPage insertDataPage = _studyHelper.goToManageDatasets() + .selectDatasetByName(datasetName) + .clickViewData() + .insertDatasetRow(); + + insertDataPage.insert(Map.of("ParticipantId", "1", + "SequenceNum", "2", + "date", "2020-08-04", + FILE_FIELD_1, inputFile.toString(), + FILE_FIELD_2, inputFile.toString(), + INT_FIELD, "26")); + verifyFileAuditLogs(List.of( + Map.of( + AuditLogHelper.COL_FILE_AUDIT_FILE, "csv_sample-1.csv", + AuditLogHelper.COL_FILE_AUDIT_PROVIDED_FILE, "csv_sample.csv" + ), + Map.of( + AuditLogHelper.COL_FILE_AUDIT_FILE, inputFile.getName(), + AuditLogHelper.COL_FILE_AUDIT_PROVIDED_FILE, inputFile.getName() + ) + )); + + insertDataPage = _studyHelper.goToManageDatasets() + .selectDatasetByName(datasetName) + .clickViewData() + .insertDatasetRow(); + insertDataPage.insert(Map.of("ParticipantId", "1", + "SequenceNum", "3", + "date", "2020-08-04", + FILE_FIELD_2, inputFile.toString(), + INT_FIELD, "27")); + verifyFileAuditLogs(List.of( + Map.of( + AuditLogHelper.COL_FILE_AUDIT_FILE, "csv_sample-2.csv", + AuditLogHelper.COL_FILE_AUDIT_PROVIDED_FILE, "csv_sample.csv" + ) + )); + } + + protected void verifyFileAuditLogs( List> expectedFileData) throws IOException, CommandException + { + List columnNames = expectedFileData.get(0).keySet().stream().map(Object::toString).toList(); + AuditLogHelper auditLogHelper = new AuditLogHelper(this, () -> WebTestHelper.getRemoteApiConnection(false)); + List> events = auditLogHelper.getAuditLogsFromLKS(getProjectName(), AuditLogHelper.AuditEvent.FILE_SYSTEM_EVENT, columnNames, null, expectedFileData.size(), null).getRows(); + for (int i = 0; i < expectedFileData.size(); i++) + { + for (String key : columnNames) + assertEquals("Event value for " + key + " not as expected", expectedFileData.get(i).get(key), events.get(i).get(key)); + } + } + protected void createDataset(String name) { DatasetDesignerPage definitionPage = _studyHelper.goToManageDatasets() @@ -167,9 +240,10 @@ protected void createDataset(String name) .setName(name); DomainFormPanel panel = definitionPage.getFieldsPanel(); - panel.manuallyDefineFields("textField"); - panel.addField(new FieldDefinition("intField", FieldDefinition.ColumnType.Integer)); - panel.addField(new FieldDefinition("fileField", FieldDefinition.ColumnType.File)); + panel.manuallyDefineFields(TEXT_FIELD); + panel.addField(new FieldDefinition(INT_FIELD, FieldDefinition.ColumnType.Integer)); + panel.addField(new FieldDefinition(FILE_FIELD_1, FieldDefinition.ColumnType.File)); + panel.addField(new FieldDefinition(FILE_FIELD_2, FieldDefinition.ColumnType.File)); definitionPage.clickSave(); } } From 650f29de5ba52170feb2639d4d99453689e691a4 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Tue, 29 Jul 2025 11:27:23 -0700 Subject: [PATCH 16/18] Update locator for random field name --- .../org/labkey/test/tests/study/StudyDatasetFileFieldTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java index c25aab1ce1b..24e144e67db 100644 --- a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java +++ b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java @@ -111,7 +111,7 @@ public void testFileField() throws IOException, CommandException log("Edit the dataset"); DataRegionTable table = new DataRegionTable("Dataset", getDriver()); table.clickEditRow(0); - setFormElement(Locator.name("quf_textField"), "Welcome..!"); + setFormElement(Locator.name("quf_" + TEXT_FIELD), "Welcome..!"); checker().verifyTrue("File is not present ", isElementPresent(Locator.linkContainingText("remove"))); clickButton("Submit"); From d34ac95354b7144986a82ebeb64c51f955950ce4 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Tue, 29 Jul 2025 13:59:23 -0700 Subject: [PATCH 17/18] Adjust container for move audit events and add test for assay run moves --- .../api/assay/AbstractAssayProvider.java | 22 ++++++++++--------- .../experiment/api/ExperimentServiceImpl.java | 17 ++++++++------ .../experiment/api/SampleTypeServiceImpl.java | 15 +++++++------ 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/api/src/org/labkey/api/assay/AbstractAssayProvider.java b/api/src/org/labkey/api/assay/AbstractAssayProvider.java index 226d1b83521..b2c36c62629 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayProvider.java +++ b/api/src/org/labkey/api/assay/AbstractAssayProvider.java @@ -1968,7 +1968,7 @@ private void moveRunsBatch(List runs, Container sourceContainer, Contain if (updatedFile != null) { if (!fileMoveReferences.containsKey(sourceFileName)) - fileMoveReferences.put(sourceFileName, new AssayFileMoveReference(sourceFileName, updatedFile, experiment.getName())); + fileMoveReferences.put(sourceFileName, new AssayFileMoveReference(sourceFileName, updatedFile, experiment.getName(), fileProp.getName())); if (!fileMoveCounts.containsKey(sourceFileName)) fileMoveCounts.put(sourceFileName, 0); @@ -1991,7 +1991,7 @@ private void moveRunsBatch(List runs, Container sourceContainer, Contain 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); - FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(sourceContainer, "File moved from " + sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(targetContainer, "File moved from " + sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); event.setProvidedFileName(sourceFile.getName()); event.setFile(ref.updatedFile.getName()); event.setDirectory(ref.updatedFile.getParent()); @@ -2055,7 +2055,7 @@ private void updateRunFiles(List runs, Container sourceContainer, Contai if (updatedFile != null) { if (!fileMoveReferences.containsKey(sourceFileName)) - fileMoveReferences.put(sourceFileName, new AssayFileMoveReference(sourceFileName, updatedFile, run.getName())); + fileMoveReferences.put(sourceFileName, new AssayFileMoveReference(sourceFileName, updatedFile, run.getName(), fileProp.getName())); if (!fileMoveCounts.containsKey(sourceFileName)) fileMoveCounts.put(sourceFileName, 0); @@ -2077,10 +2077,11 @@ private void updateRunFiles(List runs, Container sourceContainer, Contai 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); - FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(sourceContainer, "File moved from " + sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(targetContainer, "File moved from " + sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); event.setProvidedFileName(sourceFile.getName()); event.setFile(ref.updatedFile.getName()); event.setDirectory(ref.updatedFile.getParent()); + event.setFieldName(ref.fieldName); AuditLogService.get().addEvent(user, event); } @@ -2126,7 +2127,7 @@ private void updateDataFileUrl(List runs, Container sourceContainer, Con 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); - FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(sourceContainer, "File moved from " + sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(targetContainer, "File moved from " + sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); event.setProvidedFileName(sourceFile.getName()); event.setFile(updatedFile.getName()); event.setDirectory(updatedFile.getParent()); @@ -2151,7 +2152,7 @@ protected void moveAssayResults(List runs, ExpProtocol protocol, Contain updateResultFiles(assayResultTable, runs, protocol, sourceContainer, targetContainer, user, assayMoveData); } - record AssayFileMoveReference(String sourceFilePath, File updatedFile, String runName) {} + record AssayFileMoveReference(String sourceFilePath, File updatedFile, String runName, String fieldName) {} private void updateResultFiles(FilteredTable assayResultTable, List runs, ExpProtocol assayProtocol, Container sourceContainer, Container targetContainer, User user, AssayMoveData assayMoveData) throws ExperimentException { @@ -2184,10 +2185,10 @@ 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) { + Map fileMoveReferences = new HashMap<>(); + Map> fileMoveResultRowIds = new HashMap<>(); var fileColumn = assayResultTable.getColumn(fileField); TableSelector ts = new TableSelector(assayResultTable, assayResultTable.getColumns("rowid", "run", fileField), filter, null); Map[] resultFiles = ts.getMapArray(); @@ -2209,7 +2210,7 @@ private void updateResultFiles(FilteredTable assayResultTable, List runs ExpRun run = runMap.get(resultRunId); if (!fileMoveReferences.containsKey(sourceFileName)) - fileMoveReferences.put(sourceFileName, new AssayFileMoveReference(sourceFileName, updatedFile, run.getName())); + fileMoveReferences.put(sourceFileName, new AssayFileMoveReference(sourceFileName, updatedFile, run.getName(),fileField)); movedFiles.putIfAbsent(resultRunId, new ArrayList<>()); movedFiles.get(resultRunId).add(new AssayFileMoveData(run, run.getContainer(), fileField, sourceFile, updatedFile)); @@ -2249,10 +2250,11 @@ private void updateResultFiles(FilteredTable assayResultTable, List runs .append(" WHERE rowId ").appendInClause(fileMoveResultRowIds.get(sourceFileName), realTable.getSqlDialect()); new SqlExecutor(assayResultTable.getSchema()).execute(updateSql); - FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(sourceContainer, "File moved from " + sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(targetContainer, "File moved from " + sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); event.setProvidedFileName(sourceFile.getName()); event.setFile(updatedFile.getName()); event.setDirectory(updatedFile.getParent()); + event.setFieldName(ref.fieldName); AuditLogService.get().addEvent(user, event); } diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 2ea66d1bdfb..26a9e59a5df 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9815,7 +9815,7 @@ public Map moveAssayRuns(@NotNull List assayR for (List runFileRenameData : assayMoveData.fileMovesByRunId().values()) { for (AbstractAssayProvider.AssayFileMoveData renameData : runFileRenameData) - moveFile(renameData, container, user); + moveFile(renameData, container, user, transaction.getAuditId()); } }, POSTCOMMIT); @@ -9825,7 +9825,7 @@ public Map moveAssayRuns(@NotNull List assayR return assayMoveData.counts(); } - private boolean moveFile(AbstractAssayProvider.AssayFileMoveData renameData, Container sourceContainer, User user) + private boolean moveFile(AbstractAssayProvider.AssayFileMoveData renameData, Container sourceContainer, User user, Long txAuditId) { String fieldName = renameData.fieldName() == null ? "datafileurl" : renameData.fieldName(); File targetFile = renameData.targetFile(); @@ -9847,8 +9847,8 @@ private boolean moveFile(AbstractAssayProvider.AssayFileMoveData renameData, Con } - String changeDetail = String.format("'%s' assay run '%s' (field: '%s')", assayName, runName, fieldName); - return moveFileLinkFile(sourceFile, targetFile, sourceContainer, user, changeDetail); + String changeDetail = String.format("assay '%s' run '%s'", assayName, runName); + return moveFileLinkFile(sourceFile, targetFile, sourceContainer, user, changeDetail, txAuditId, fieldName); } @Override @@ -10028,7 +10028,7 @@ public boolean isLookupToMaterials(DomainProperty dp) * * Move file (post-commit) after moving sample/assay data */ - public boolean moveFileLinkFile(File sourceFile, File targetFile, Container sourceFileContainer, User user, String actionComment) + public boolean moveFileLinkFile(File sourceFile, File targetFile, Container sourceFileContainer, User user, String actionComment, Long txAuditId, String fieldName) { if (!sourceFile.exists()) return false; @@ -10060,11 +10060,14 @@ public boolean moveFileLinkFile(File sourceFile, File targetFile, Container sour return false; } } - - event.setDirectory(targetFile.getParentFile().getAbsolutePath()); + if (txAuditId != null && event.getTransactionId() == null) + event.setTransactionId(txAuditId); + event.setDirectory(sourceFile.getParentFile().getAbsolutePath()); event.setFile(targetFile.getName()); event.setProvidedFileName(sourceFile.getName()); event.setResourcePath(sourceFile.getAbsolutePath()); + if (!"datafileurl".equals(fieldName)) // don't want to show this as a field name + event.setFieldName(fieldName); AuditLogService.get().addEvent(user, event); return true; diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index db47a7d3a5d..1171abc2f4e 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -1953,7 +1953,7 @@ public Map moveSamples(Collection sample for (List sampleFileRenameData : fileMovesBySampleId.values()) { for (FileFieldRenameData renameData : sampleFileRenameData) - moveFile(renameData, sourceContainer, user); + moveFile(renameData, sourceContainer, user, transaction.getAuditId()); } }, POSTCOMMIT); @@ -2064,7 +2064,7 @@ private int splitExperimentRuns(List runs, Map return runCount; } - record SampleFileMoveReference(String sourceFilePath, File targetFile, Container sourceContainer, String sampleName) {} + record SampleFileMoveReference(String sourceFilePath, File targetFile, Container sourceContainer, String sampleName, String fieldName) {} // return the map of file renames private Map> updateSampleFilePaths(ExpSampleType sampleType, List samples, Container targetContainer, User user) throws ExperimentException @@ -2109,7 +2109,7 @@ private Map> updateSampleFilePaths(ExpSampleT { if (!fileMoveReferences.containsKey(sourceFileName)) - fileMoveReferences.put(sourceFileName, new SampleFileMoveReference(sourceFileName, updatedFile, sample.getContainer(), sample.getName())); + fileMoveReferences.put(sourceFileName, new SampleFileMoveReference(sourceFileName, updatedFile, sample.getContainer(), sample.getName(), fileProp.getName())); if (!fileMoveCounts.containsKey(sourceFileName)) fileMoveCounts.put(sourceFileName, 0); fileMoveCounts.put(sourceFileName, fileMoveCounts.get(sourceFileName) + 1); @@ -2132,17 +2132,18 @@ private Map> updateSampleFilePaths(ExpSampleT // TODO, support batch fireFileMoveEvent to avoid excessive FileLinkFileListener.hardTableFileLinkColumns calls fileService.fireFileMoveEvent(sourceFile, ref.targetFile, user, targetContainer); - FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(ref.sourceContainer, "File moved from " + ref.sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); + FileSystemAuditProvider.FileSystemAuditEvent event = new FileSystemAuditProvider.FileSystemAuditEvent(targetContainer, "File moved from " + ref.sourceContainer.getPath() + " to " + targetContainer.getPath() + "."); event.setProvidedFileName(sourceFile.getName()); event.setFile(ref.targetFile.getName()); event.setDirectory(ref.targetFile.getParent()); + event.setFieldName(ref.fieldName); AuditLogService.get().addEvent(user, event); } return sampleFileRenames; } - private boolean moveFile(FileFieldRenameData renameData, Container sourceContainer, User user) + private boolean moveFile(FileFieldRenameData renameData, Container sourceContainer, User user, Long txAuditId) { if (!renameData.targetFile.getParentFile().exists()) { @@ -2166,8 +2167,8 @@ private boolean moveFile(FileFieldRenameData renameData, Container sourceContain } } - 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); + String changeDetail = String.format("sample type '%s' sample '%s'", renameData.sampleType.getName(), renameData.sampleName); + return ExperimentServiceImpl.get().moveFileLinkFile(renameData.sourceFile, renameData.targetFile, sourceContainer, user, changeDetail, txAuditId, renameData.fieldName); } @Override From b3b373c9f129a7783d16021251987eba7a3b6319 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Wed, 30 Jul 2025 08:11:38 -0700 Subject: [PATCH 18/18] Update test with domain kind and new field names --- .../tests/study/StudyDatasetFileFieldTest.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java index 24e144e67db..f7272198103 100644 --- a/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java +++ b/study/test/src/org/labkey/test/tests/study/StudyDatasetFileFieldTest.java @@ -20,6 +20,7 @@ import org.labkey.test.params.FieldDefinition; import org.labkey.test.util.AuditLogHelper; import org.labkey.test.util.DataRegionTable; +import org.labkey.test.util.DomainUtils; import org.labkey.test.util.TestDataGenerator; import java.io.File; @@ -41,10 +42,10 @@ public class StudyDatasetFileFieldTest extends BaseWebDriverTest { private static final String EXCLUDED_CHARS = "\""; // this gets encoded as %22 when the form data is sent. private static final String IMPORT_PROJECT = "StudyDatasetFileFieldFolderImportProject"; - private static final String FILE_FIELD_1 = TestDataGenerator.randomFieldName("File Field 1", EXCLUDED_CHARS); - private static final String FILE_FIELD_2 = TestDataGenerator.randomFieldName("File Field 2", EXCLUDED_CHARS); - private static final String INT_FIELD = TestDataGenerator.randomFieldName("Int Field", EXCLUDED_CHARS); - private static final String TEXT_FIELD = TestDataGenerator.randomFieldName("Text Field", EXCLUDED_CHARS); + private static final String FILE_FIELD_1 = TestDataGenerator.randomFieldName("File Field 1", EXCLUDED_CHARS, DomainUtils.DomainKind.StudyDatasetDate); + private static final String FILE_FIELD_2 = TestDataGenerator.randomFieldName("File Field 2", EXCLUDED_CHARS, DomainUtils.DomainKind.StudyDatasetDate); + private static final String INT_FIELD = TestDataGenerator.randomFieldName("Int Field", EXCLUDED_CHARS, DomainUtils.DomainKind.StudyDatasetDate); + private static final String TEXT_FIELD = TestDataGenerator.randomFieldName("Text Field", EXCLUDED_CHARS, DomainUtils.DomainKind.StudyDatasetDate); @BeforeClass public static void doSetup() @@ -159,7 +160,7 @@ public void testFileField() throws IOException, CommandException table = new DataRegionTable("Dataset", getDriver()); table.clickEditRow(0); checker().verifyTrue("File is not present ", isElementPresent(Locator.linkContainingText("remove"))); - setFormElement(Locator.name("quf_intField"), "NOT A NUMBER"); + setFormElement(Locator.name("quf_" + INT_FIELD), "NOT A NUMBER"); clickButton("Submit"); // assert correct reshow with error @@ -169,8 +170,8 @@ public void testFileField() throws IOException, CommandException // Issue : 53320. Update a file field with a different file click(Locator.linkContainingText("remove")); File updateFile = TestFileUtils.getSampleData("fileTypes/pdf_sample.pdf"); - setFormElement(Locator.name("quf_intField"), "2"); - setFormElement(Locator.name("quf_fileField"), updateFile.toString()); + setFormElement(Locator.name("quf_" + INT_FIELD), "2"); + setFormElement(Locator.name("quf_" + FILE_FIELD_1), updateFile.toString()); clickButton("Submit"); } @@ -221,11 +222,12 @@ public void testFileRenamingAndAuditing() throws IOException, CommandException )); } - protected void verifyFileAuditLogs( List> expectedFileData) throws IOException, CommandException + protected void verifyFileAuditLogs(List> expectedFileData) throws IOException, CommandException { List columnNames = expectedFileData.get(0).keySet().stream().map(Object::toString).toList(); AuditLogHelper auditLogHelper = new AuditLogHelper(this, () -> WebTestHelper.getRemoteApiConnection(false)); List> events = auditLogHelper.getAuditLogsFromLKS(getProjectName(), AuditLogHelper.AuditEvent.FILE_SYSTEM_EVENT, columnNames, null, expectedFileData.size(), null).getRows(); + assertEquals("Number of file audit log events not as expected", expectedFileData.size(), events.size()); for (int i = 0; i < expectedFileData.size(); i++) { for (String key : columnNames)