Delete orphaned attachments (#7513)

labkey-adam · web-flow · commit cb2b140caef6 · 2026-03-26T11:25:19.000-07:00
diff --git 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
@@ -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
@@ -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/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<String> hosts = AppProps.getInstance().getExternalSourceHosts();
         List<AllowedHost> allowedHosts = hosts.stream()
             .map(host -> new AllowedHost(Directive.Connection, host))
@@ -106,4 +107,23 @@ public static void populateAttachmentParentTypeColumn(ModuleContext context)
             new SqlExecutor(CoreSchema.getInstance().getSchema()).execute(updateSql);
         }
     }
-}
+
+    /**
+     * 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")
+    public static void deleteOrphanedAttachments(ModuleContext context)
+    {
+        if (context.isNewInstall())
+            return;
+
+        AttachmentService svc = AttachmentService.get();
+
+        if (svc != null)
+        {
+            svc.deleteOrphanedAttachments();
+        }
+    }
+}
diff --git 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;
@@ -45,6 +46,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;
@@ -113,6 +115,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;
@@ -203,7 +206,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());
@@ -1098,7 +1101,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 +1138,69 @@ public void detectOrphans()
         }
     }
 
+    record OrphanedAttachment(String container, String parent, String parentType, String documentName)
+    {
+        AttachmentParent getAttachmentParent()
+        {
+            return new AttachmentParent()
+            {
+                @Override
+                public String getEntityId()
+                {
+                    return parent;
+                }
+
+                @Override
+                public String getContainerId()
+                {
+                    return container;
+                }
+
+                @Override
+                public @NotNull AttachmentParentType getAttachmentParentType()
+                {
+                    // 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;
+                }
+            };
+        }
+    }
+
+    @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)
+        {
+            // 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);
+                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);
+                    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.intValue(), "orphaned attachment"));
+                event.setAttachment("All orphaned attachments");
+                AuditLogService.get().addEvent(user, event);
+            }
+        }
+    }
+
     private CoreSchema coreTables()
     {
         return CoreSchema.getInstance();
diff --git 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;
@@ -28,15 +29,19 @@
 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;
+import org.labkey.api.util.logging.LogHelper;
 
 import java.util.LinkedList;
 import java.util.List;
 
 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()
     {
@@ -74,22 +79,36 @@ 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());
+
+                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)
+                            .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() ?

Original file line number	Diff line number	Diff line change
`@@ -140,7 +140,9 @@ static AttachmentService get()`
`140`	`140`
`141`	`141`	`HttpView<?> getFindAttachmentParentsView();`
`142`	`142`
`143`		`- void detectOrphans();`
	`143`	`+ void logOrphanedAttachments();`
	`144`	`+`
	`145`	`+ void deleteOrphanedAttachments();`
`144`	`146`
`145`	`147`	`class DuplicateFilenameException extends IOException implements SkipMothershipLogging`
`146`	`148`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1944,7 +1944,7 @@ private static boolean delete(final Container c, User user, @Nullable String com`
`1944`	`1944`	`setContainerTabDeleted(c.getParent(), c.getName(), c.getParent().getFolderType().getName());`
`1945`	`1945`	`}`
`1946`	`1946`
`1947`		`- AttachmentService.get().detectOrphans();`
	`1947`	`+ AttachmentService.get().logOrphanedAttachments();`
`1948`	`1948`
`1949`	`1949`	`fireDeleteContainer(c, user);`
`1950`	`1950`