From 6c2f90559e47d1d1813db9b6c1ace1b6acca0001 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 21 Mar 2026 20:07:56 -0700 Subject: [PATCH 1/7] Delete orphaned attachments --- .../api/attachments/AttachmentService.java | 4 +- .../org/labkey/api/data/ContainerManager.java | 2 +- core/module.properties | 2 +- .../postgresql/core-26.002-26.003.sql | 1 + .../sqlserver/core-26.002-26.003.sql | 1 + core/src/org/labkey/core/CoreUpgradeCode.java | 19 +++++++ .../attachment/AttachmentServiceImpl.java | 51 ++++++++++++++++++- 7 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 core/resources/schemas/dbscripts/postgresql/core-26.002-26.003.sql create mode 100644 core/resources/schemas/dbscripts/sqlserver/core-26.002-26.003.sql diff --git a/api/src/org/labkey/api/attachments/AttachmentService.java b/api/src/org/labkey/api/attachments/AttachmentService.java index 5b10805aa3d..39e0eca2425 100644 --- a/api/src/org/labkey/api/attachments/AttachmentService.java +++ b/api/src/org/labkey/api/attachments/AttachmentService.java @@ -140,7 +140,9 @@ static AttachmentService get() HttpView getFindAttachmentParentsView(); - void detectOrphans(); + void logOrphanedAttachments(); + + void deleteOrphanedAttachments(); class DuplicateFilenameException extends IOException implements SkipMothershipLogging { diff --git a/api/src/org/labkey/api/data/ContainerManager.java b/api/src/org/labkey/api/data/ContainerManager.java index 74507dfb6ac..fd459df101d 100644 --- a/api/src/org/labkey/api/data/ContainerManager.java +++ b/api/src/org/labkey/api/data/ContainerManager.java @@ -1944,7 +1944,7 @@ private static boolean delete(final Container c, User user, @Nullable String com setContainerTabDeleted(c.getParent(), c.getName(), c.getParent().getFolderType().getName()); } - AttachmentService.get().detectOrphans(); + AttachmentService.get().logOrphanedAttachments(); fireDeleteContainer(c, user); diff --git a/core/module.properties b/core/module.properties index 26d5f3f5eb5..90a5223381f 100644 --- a/core/module.properties +++ b/core/module.properties @@ -1,6 +1,6 @@ Name: Core ModuleClass: org.labkey.core.CoreModule -SchemaVersion: 26.002 +SchemaVersion: 26.003 Label: Administration and Essential Services Description: The Core module provides central services such as login, \ security, administration, folder management, user management, \ diff --git a/core/resources/schemas/dbscripts/postgresql/core-26.002-26.003.sql b/core/resources/schemas/dbscripts/postgresql/core-26.002-26.003.sql new file mode 100644 index 00000000000..407b5538278 --- /dev/null +++ b/core/resources/schemas/dbscripts/postgresql/core-26.002-26.003.sql @@ -0,0 +1 @@ +SELECT core.executeJavaUpgradeCode('deleteOrphanedAttachments'); diff --git a/core/resources/schemas/dbscripts/sqlserver/core-26.002-26.003.sql b/core/resources/schemas/dbscripts/sqlserver/core-26.002-26.003.sql new file mode 100644 index 00000000000..23425e25857 --- /dev/null +++ b/core/resources/schemas/dbscripts/sqlserver/core-26.002-26.003.sql @@ -0,0 +1 @@ +EXEC core.executeJavaUpgradeCode 'deleteOrphanedAttachments'; diff --git a/core/src/org/labkey/core/CoreUpgradeCode.java b/core/src/org/labkey/core/CoreUpgradeCode.java index f12197cc99a..eb5d1040372 100644 --- a/core/src/org/labkey/core/CoreUpgradeCode.java +++ b/core/src/org/labkey/core/CoreUpgradeCode.java @@ -71,6 +71,7 @@ public static void migrateAllowedExternalConnectionHosts(ModuleContext context) if (context.isNewInstall()) return; + // TODO: Remove getExternalSourceHosts() method when this upgrade code is deleted List hosts = AppProps.getInstance().getExternalSourceHosts(); List allowedHosts = hosts.stream() .map(host -> new AllowedHost(Directive.Connection, host)) @@ -106,4 +107,22 @@ public static void populateAttachmentParentTypeColumn(ModuleContext context) new SqlExecutor(CoreSchema.getInstance().getSchema()).execute(updateSql); } } + + /** + * Called from core-26.002-26.003.sql + */ + @DeferredUpgrade // Need to execute this after AttachmentTypes are registered + @SuppressWarnings("unused") + public static void deleteOrphanedAttachments(ModuleContext context) + { + if (context.isNewInstall()) + return; + + AttachmentService svc = AttachmentService.get(); + + if (svc != null) + { + svc.deleteOrphanedAttachments(); + } + } } \ No newline at end of file diff --git a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java index ed266908800..2e7dfb70f71 100644 --- a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java +++ b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java @@ -45,6 +45,7 @@ import org.labkey.api.data.ColumnRenderProperties; import org.labkey.api.data.CompareType; import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.ContainerFilter.AllFolders; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.CoreSchema; @@ -1098,7 +1099,7 @@ public int available() private record Orphan(String documentName, String parentType){} @Override - public void detectOrphans() + public void logOrphanedAttachments() { // Log orphaned attachments in this server, but in dev mode only, since this is for our testing. Also, we // don't yet offer a way to delete orphaned attachments via the UI, so it's not helpful to inform admins. @@ -1135,6 +1136,54 @@ public void detectOrphans() } } + record OrphanedAttachment(String container, String parent, String documentName) + { + AttachmentParent getAttachmentParent() + { + return new AttachmentParent() + { + @Override + public String getEntityId() + { + return parent; + } + + @Override + public String getContainerId() + { + return container; + } + + @Override + public @NotNull AttachmentParentType getAttachmentParentType() + { + return AttachmentParentType.UNKNOWN; + } + }; + } + } + + @Override + public void deleteOrphanedAttachments() + { + User user = ElevatedUser.getElevatedUser(User.getSearchUser(), TroubleshooterRole.class); + UserSchema core = DefaultSchema.get(user, ContainerManager.getRoot()).getUserSchema(CoreQuerySchema.NAME); + if (core != null) + { + // Use "unsafe everything" container filter because it's possible that orphaned attachments have a container + // that no longer exists. + TableInfo documents = core.getTable(CoreQuerySchema.DOCUMENTS_TABLE_NAME, ContainerFilter.getUnsafeEverythingFilter()); + if (null != documents) + { + SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Orphaned"), true); + new TableSelector(documents, new CsvSet("Container, Parent, DocumentName"), filter, null).forEach(OrphanedAttachment.class, orphan -> { + LOG.info("Deleting orphaned attachment: {}", orphan); + deleteAttachment(orphan.getAttachmentParent(), orphan.documentName(), user); + }); + } + } + } + private CoreSchema coreTables() { return CoreSchema.getInstance(); From f47cbae424c03a4ede2cdcd9eb4b16f030261c36 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sun, 22 Mar 2026 12:23:35 -0700 Subject: [PATCH 2/7] Select and attempt to resolve ParentType so it gets included in the audit log delete event --- .../org/labkey/core/attachment/AttachmentServiceImpl.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java index 2e7dfb70f71..7ed40c5743c 100644 --- a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java +++ b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java @@ -1136,7 +1136,7 @@ public void logOrphanedAttachments() } } - record OrphanedAttachment(String container, String parent, String documentName) + record OrphanedAttachment(String container, String parent, String parentType, String documentName) { AttachmentParent getAttachmentParent() { @@ -1157,7 +1157,9 @@ public String getContainerId() @Override public @NotNull AttachmentParentType getAttachmentParentType() { - return AttachmentParentType.UNKNOWN; + // Attempt to resolve the parent type. This will get written to the audit log. + AttachmentParentType type = ATTACHMENT_TYPE_MAP.get(parentType()); + return type != null ? type : AttachmentParentType.UNKNOWN; } }; } @@ -1176,7 +1178,7 @@ public void deleteOrphanedAttachments() if (null != documents) { SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Orphaned"), true); - new TableSelector(documents, new CsvSet("Container, Parent, DocumentName"), filter, null).forEach(OrphanedAttachment.class, orphan -> { + new TableSelector(documents, new CsvSet("Container, Parent, ParentType, DocumentName"), filter, null).forEach(OrphanedAttachment.class, orphan -> { LOG.info("Deleting orphaned attachment: {}", orphan); deleteAttachment(orphan.getAttachmentParent(), orphan.documentName(), user); }); From 7e6e3b838a600db80bd4cf576bc233889076c334 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 23 Mar 2026 16:02:41 -0700 Subject: [PATCH 3/7] Fix detection of Compound.Structure2D attachment column --- .../experiment/api/ExpDataClassType.java | 42 ++++++++++++------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassType.java b/experiment/src/org/labkey/experiment/api/ExpDataClassType.java index 749137c049e..b164988d205 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassType.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassType.java @@ -28,6 +28,8 @@ import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.property.Domain; import org.labkey.api.exp.property.PropertyService; +import org.labkey.api.exp.query.DataClassUserSchema; +import org.labkey.api.security.User; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; @@ -74,22 +76,30 @@ public static AttachmentParentType get() String lsid = rs.getString("LSID"); Domain domain = PropertyService.get().getDomain(c, lsid); - // Add a select for the ObjectIds in this ExpDataClass if the domain includes an attachment column. ExpDataClass attachments - // use the LSID's ObjectId as the attachment parent EntityId, so we need to use a SQL expression to extract it. - if (null != domain && domain.getProperties().stream().anyMatch(p -> p.getPropertyType() == PropertyType.ATTACHMENT)) - selectStatements.add( - new SQLFragment("\n SELECT ") - .append(expressionToExtractObjectId) - .append(" AS EntityId, ") - .append(dialect.concatenate( - new SQLFragment("?", domain.getName()), - new SQLFragment("':'"), - new SQLFragment("Name") - )) - .append(" AS Description FROM expdataclass.") - .append(domain.getStorageTableName()) - .append(" WHERE ").append(where) - ); + if (null != domain) + { + // Enumerate columns on the data class TableInfo since it includes the vocabulary domain columns. + // For example, Compound has a built-in Structure2D attachment column supplied by a vocabulary domain. + TableInfo dataClassTable = new DataClassUserSchema(c, User.getSearchUser()).getTable(domain.getName()); + + // Add a select for the ObjectIds in this ExpDataClass if the domain includes an attachment column. + // ExpDataClass attachments use the LSID's ObjectId as the attachment parent EntityId, so we need to use + // a SQL expression to extract it. + if (dataClassTable != null && dataClassTable.getColumns().stream().anyMatch(col -> col.getPropertyType() == PropertyType.ATTACHMENT)) + selectStatements.add( + new SQLFragment("\n SELECT ") + .append(expressionToExtractObjectId) + .append(" AS EntityId, ") + .append(dialect.concatenate( + new SQLFragment("?", domain.getName()), + new SQLFragment("':'"), + new SQLFragment("Name") + )) + .append(" AS Description FROM expdataclass.") + .append(domain.getStorageTableName()) + .append(" WHERE ").append(where) + ); + } }); return selectStatements.isEmpty() ? From af7814cd21f0f56345c28e2a4d79169609675f9c Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 23 Mar 2026 16:08:29 -0700 Subject: [PATCH 4/7] Remove upgrade scripts - we'll let admins review and delete orphaned attachments for now --- .../schemas/dbscripts/postgresql/core-26.002-26.003.sql | 1 - .../schemas/dbscripts/sqlserver/core-26.002-26.003.sql | 1 - core/src/org/labkey/core/CoreUpgradeCode.java | 3 ++- 3 files changed, 2 insertions(+), 3 deletions(-) delete mode 100644 core/resources/schemas/dbscripts/postgresql/core-26.002-26.003.sql delete mode 100644 core/resources/schemas/dbscripts/sqlserver/core-26.002-26.003.sql diff --git a/core/resources/schemas/dbscripts/postgresql/core-26.002-26.003.sql b/core/resources/schemas/dbscripts/postgresql/core-26.002-26.003.sql deleted file mode 100644 index 407b5538278..00000000000 --- a/core/resources/schemas/dbscripts/postgresql/core-26.002-26.003.sql +++ /dev/null @@ -1 +0,0 @@ -SELECT core.executeJavaUpgradeCode('deleteOrphanedAttachments'); diff --git a/core/resources/schemas/dbscripts/sqlserver/core-26.002-26.003.sql b/core/resources/schemas/dbscripts/sqlserver/core-26.002-26.003.sql deleted file mode 100644 index 23425e25857..00000000000 --- a/core/resources/schemas/dbscripts/sqlserver/core-26.002-26.003.sql +++ /dev/null @@ -1 +0,0 @@ -EXEC core.executeJavaUpgradeCode 'deleteOrphanedAttachments'; diff --git a/core/src/org/labkey/core/CoreUpgradeCode.java b/core/src/org/labkey/core/CoreUpgradeCode.java index eb5d1040372..dabe3840a8c 100644 --- a/core/src/org/labkey/core/CoreUpgradeCode.java +++ b/core/src/org/labkey/core/CoreUpgradeCode.java @@ -109,7 +109,8 @@ public static void populateAttachmentParentTypeColumn(ModuleContext context) } /** - * Called from core-26.002-26.003.sql + * This is not invoked from any script yet. We want to make sure our orphaned attachment detection is perfect + * before blanket deleting them. */ @DeferredUpgrade // Need to execute this after AttachmentTypes are registered @SuppressWarnings("unused") From a4ff61cf40620e1798e1300dd75d178ed2404ce9 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 24 Mar 2026 19:16:44 -0700 Subject: [PATCH 5/7] Audit event for deleting orphaned attachments --- .../org/labkey/core/attachment/AttachmentServiceImpl.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java index 7ed40c5743c..fd25d2bc686 100644 --- a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java +++ b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java @@ -114,6 +114,7 @@ import org.labkey.api.webdav.WebdavResolver; import org.labkey.api.webdav.WebdavResource; import org.labkey.core.query.AttachmentAuditProvider; +import org.labkey.core.query.AttachmentAuditProvider.AttachmentAuditEvent; import org.labkey.core.query.CoreQuerySchema; import org.springframework.http.ContentDisposition; import org.springframework.mock.web.MockMultipartFile; @@ -204,7 +205,7 @@ public void addAuditEvent(User user, AttachmentParent parent, String filename, S if (parent != null) { Container c = ContainerManager.getForId(parent.getContainerId()); - AttachmentAuditProvider.AttachmentAuditEvent attachmentEvent = new AttachmentAuditProvider.AttachmentAuditEvent(c == null ? ContainerManager.getRoot() : c, comment); + AttachmentAuditEvent attachmentEvent = new AttachmentAuditEvent(c == null ? ContainerManager.getRoot() : c, comment); attachmentEvent.setAttachmentParentEntityId(parent.getEntityId()); attachmentEvent.setParentType(parent.getAttachmentParentType().getUniqueName()); @@ -1178,10 +1179,13 @@ public void deleteOrphanedAttachments() if (null != documents) { SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Orphaned"), true); - new TableSelector(documents, new CsvSet("Container, Parent, ParentType, DocumentName"), filter, null).forEach(OrphanedAttachment.class, orphan -> { + int count = new TableSelector(documents, new CsvSet("Container, Parent, ParentType, DocumentName"), filter, null).forEach(OrphanedAttachment.class, orphan -> { LOG.info("Deleting orphaned attachment: {}", orphan); deleteAttachment(orphan.getAttachmentParent(), orphan.documentName(), user); }); + AttachmentAuditEvent event = new AttachmentAuditEvent(ContainerManager.getRoot(), "Deleted " + StringUtilsLabKey.pluralize(count, "orphaned attachment")); + event.setAttachment("All orphaned attachments"); + AuditLogService.get().addEvent(user, event); } } } From 43e654724055bc57748ad87527ff606a808d69cc Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 24 Mar 2026 20:13:23 -0700 Subject: [PATCH 6/7] Claude you are so picky --- .../core/attachment/AttachmentServiceImpl.java | 16 +++++++++++++--- .../labkey/experiment/api/ExpDataClassType.java | 17 +++++++++++++---- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java index fd25d2bc686..716e481d821 100644 --- a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java +++ b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java @@ -21,6 +21,7 @@ import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; +import org.apache.commons.lang3.mutable.MutableInt; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -1179,11 +1180,20 @@ public void deleteOrphanedAttachments() if (null != documents) { SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("Orphaned"), true); - int count = new TableSelector(documents, new CsvSet("Container, Parent, ParentType, DocumentName"), filter, null).forEach(OrphanedAttachment.class, orphan -> { + MutableInt count = new MutableInt(0); + new TableSelector(documents, new CsvSet("Container, Parent, ParentType, DocumentName"), filter, null).forEach(OrphanedAttachment.class, orphan -> { LOG.info("Deleting orphaned attachment: {}", orphan); - deleteAttachment(orphan.getAttachmentParent(), orphan.documentName(), user); + try + { + deleteAttachment(orphan.getAttachmentParent(), orphan.documentName(), user); + count.increment(); + } + catch (Exception e) + { + LOG.error("Exception while deleting orphaned attachment: {}", orphan, e); + } }); - AttachmentAuditEvent event = new AttachmentAuditEvent(ContainerManager.getRoot(), "Deleted " + StringUtilsLabKey.pluralize(count, "orphaned attachment")); + AttachmentAuditEvent event = new AttachmentAuditEvent(ContainerManager.getRoot(), "Deleted " + StringUtilsLabKey.pluralize(count.intValue(), "orphaned attachment")); event.setAttachment("All orphaned attachments"); AuditLogService.get().addEvent(user, event); } diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassType.java b/experiment/src/org/labkey/experiment/api/ExpDataClassType.java index b164988d205..d4f6457166d 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassType.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassType.java @@ -15,6 +15,7 @@ */ package org.labkey.experiment.api; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.labkey.api.attachments.AttachmentParentType; import org.labkey.api.data.Container; @@ -32,6 +33,7 @@ import org.labkey.api.security.User; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; +import org.labkey.api.util.logging.LogHelper; import java.util.LinkedList; import java.util.List; @@ -39,6 +41,7 @@ public class ExpDataClassType implements AttachmentParentType { private static final AttachmentParentType INSTANCE = new ExpDataClassType(); + private static final Logger LOG = LogHelper.getLogger(ExpDataClassType.class, "Issues selecting entityIds"); private ExpDataClassType() { @@ -82,10 +85,15 @@ public static AttachmentParentType get() // For example, Compound has a built-in Structure2D attachment column supplied by a vocabulary domain. TableInfo dataClassTable = new DataClassUserSchema(c, User.getSearchUser()).getTable(domain.getName()); - // Add a select for the ObjectIds in this ExpDataClass if the domain includes an attachment column. - // ExpDataClass attachments use the LSID's ObjectId as the attachment parent EntityId, so we need to use - // a SQL expression to extract it. - if (dataClassTable != null && dataClassTable.getColumns().stream().anyMatch(col -> col.getPropertyType() == PropertyType.ATTACHMENT)) + if (dataClassTable == null) + { + LOG.warn("DataClass table not found for {}", domain.getName()); + } + else if (dataClassTable.getColumns().stream().anyMatch(col -> col.getPropertyType() == PropertyType.ATTACHMENT)) + { + // Add a select for the ObjectIds in this ExpDataClass if the table includes an attachment column. + // ExpDataClass attachments use the LSID's ObjectId as the attachment parent EntityId, so we need + // to use a SQL expression to extract it. selectStatements.add( new SQLFragment("\n SELECT ") .append(expressionToExtractObjectId) @@ -99,6 +107,7 @@ public static AttachmentParentType get() .append(domain.getStorageTableName()) .append(" WHERE ").append(where) ); + } } }); From 6d163616a5d99a19e633dcd5d401d809ff75f409 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 24 Mar 2026 20:19:15 -0700 Subject: [PATCH 7/7] More picky --- core/src/org/labkey/core/CoreUpgradeCode.java | 2 +- core/src/org/labkey/core/attachment/AttachmentServiceImpl.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/org/labkey/core/CoreUpgradeCode.java b/core/src/org/labkey/core/CoreUpgradeCode.java index dabe3840a8c..fdc15f89175 100644 --- a/core/src/org/labkey/core/CoreUpgradeCode.java +++ b/core/src/org/labkey/core/CoreUpgradeCode.java @@ -126,4 +126,4 @@ public static void deleteOrphanedAttachments(ModuleContext context) svc.deleteOrphanedAttachments(); } } -} \ No newline at end of file +} diff --git a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java index 716e481d821..5ccd4c0477f 100644 --- a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java +++ b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java @@ -1170,6 +1170,7 @@ public String getContainerId() @Override public void deleteOrphanedAttachments() { + // TroubleShooterRole provides ability to read the Documents table. deleteAttachments() does not check perms. User user = ElevatedUser.getElevatedUser(User.getSearchUser(), TroubleshooterRole.class); UserSchema core = DefaultSchema.get(user, ContainerManager.getRoot()).getUserSchema(CoreQuerySchema.NAME); if (core != null)