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/src/org/labkey/core/CoreUpgradeCode.java b/core/src/org/labkey/core/CoreUpgradeCode.java index f12197cc99a..fdc15f89175 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,23 @@ public static void populateAttachmentParentTypeColumn(ModuleContext context) new SqlExecutor(CoreSchema.getInstance().getSchema()).execute(updateSql); } } -} \ No newline at end of file + + /** + * 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 index ed266908800..5ccd4c0477f 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; @@ -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 index 749137c049e..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; @@ -28,8 +29,11 @@ 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; @@ -37,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() { @@ -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() ?