diff --git a/api/resources/queries/core/DocumentsGroupedByParentTypeAdmin.query.xml b/api/resources/queries/core/DocumentsGroupedByParentTypeAdmin.query.xml new file mode 100644 index 00000000000..9ae9c9126f8 --- /dev/null +++ b/api/resources/queries/core/DocumentsGroupedByParentTypeAdmin.query.xml @@ -0,0 +1,14 @@ + diff --git a/api/resources/queries/core/DocumentsGroupedByParentTypeAdmin.sql b/api/resources/queries/core/DocumentsGroupedByParentTypeAdmin.sql new file mode 100644 index 00000000000..84423cf106a --- /dev/null +++ b/api/resources/queries/core/DocumentsGroupedByParentTypeAdmin.sql @@ -0,0 +1,5 @@ +-- Identical to DocumentsGroupedByParentType, but this query's .query.xml provides an admin-console-specific Count URL +SELECT ParentType, COUNT(*) AS "Count" +FROM Documents +GROUP BY ParentType +ORDER BY ParentType diff --git a/api/src/org/labkey/api/attachments/AttachmentService.java b/api/src/org/labkey/api/attachments/AttachmentService.java index 10541a07aff..5524e0aa545 100644 --- a/api/src/org/labkey/api/attachments/AttachmentService.java +++ b/api/src/org/labkey/api/attachments/AttachmentService.java @@ -138,8 +138,6 @@ static AttachmentService get() **/ Collection getAttachmentParentTypes(); - HttpView getAdminView(ActionURL currentUrl); - HttpView getFindAttachmentParentsView(); class DuplicateFilenameException extends IOException implements SkipMothershipLogging diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index 64d05aac816..19266ce4987 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -64,6 +64,7 @@ import org.labkey.api.action.Marshaller; import org.labkey.api.action.MutatingApiAction; import org.labkey.api.action.QueryViewAction; +import org.labkey.api.action.QueryViewAction.QueryExportForm; import org.labkey.api.action.ReadOnlyApiAction; import org.labkey.api.action.ReturnUrlForm; import org.labkey.api.action.SimpleApiJsonForm; @@ -105,6 +106,7 @@ import org.labkey.api.data.ConnectionWrapper; import org.labkey.api.data.Container; import org.labkey.api.data.Container.ContainerException; +import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.ContainerType; import org.labkey.api.data.ConvertHelper; @@ -171,11 +173,9 @@ import org.labkey.api.products.ProductRegistry; import org.labkey.api.query.DefaultSchema; import org.labkey.api.query.FieldKey; -import org.labkey.api.query.QueryParam; import org.labkey.api.query.QuerySchema; import org.labkey.api.query.QueryService; import org.labkey.api.query.QuerySettings; -import org.labkey.api.query.QueryUrls; import org.labkey.api.query.QueryView; import org.labkey.api.query.RuntimeValidationException; import org.labkey.api.query.SchemaKey; @@ -189,6 +189,7 @@ import org.labkey.api.security.AdminConsoleAction; import org.labkey.api.security.CSRF; import org.labkey.api.security.Directive; +import org.labkey.api.security.ElevatedUser; import org.labkey.api.security.Group; import org.labkey.api.security.GroupManager; import org.labkey.api.security.IgnoresTermsOfUse; @@ -477,9 +478,7 @@ public static void registerAdminConsoleLinks() // Diagnostics AdminConsole.addLink(Diagnostics, "actions", new ActionURL(ActionsAction.class, root)); - AdminConsole.addLink(Diagnostics, "attachments", PageFlowUtil.urlProvider(QueryUrls.class).urlExecuteQuery(root, "core", "DocumentsGroupedByParentType") - .addParameter("query." + QueryParam.containerFilterName, "AllFolders"), ApplicationAdminPermission.class); - AdminConsole.addLink(Diagnostics, "attachments - old", new ActionURL(AttachmentsAction.class, root)); + AdminConsole.addLink(Diagnostics, "attachments", new ActionURL(AttachmentsAction.class, root)); AdminConsole.addLink(Diagnostics, "caches", new ActionURL(CachesAction.class, root)); AdminConsole.addLink(Diagnostics, "check database", new ActionURL(DbCheckerAction.class, root), AdminOperationsPermission.class); AdminConsole.addLink(Diagnostics, "credits", new ActionURL(CreditsAction.class, root)); @@ -2609,14 +2608,17 @@ public void addNavTrail(NavTree root) } } - private abstract class AbstractPostgresAction extends QueryViewAction + private abstract class AbstractPostgresAction extends AbstractAdminQueryAction { - private final String _queryName; - protected AbstractPostgresAction(String queryName) { - super(QueryExportForm.class); - _queryName = queryName; + super("query", queryName); + } + + @Override + protected UserSchema getUserSchema() + { + return new PostgresUserSchema(getUser(), getContainer()); } @Override @@ -2624,34 +2626,18 @@ protected QueryView createQueryView(QueryExportForm form, BindException errors, { if (!CoreSchema.getInstance().getSqlDialect().isPostgreSQL()) { - throw new NotFoundException("Only available with Postgres as the primary database"); + throw new NotFoundException("Available only with Postgres as the primary database"); } - QuerySettings qSettings = new QuerySettings(getViewContext(), "query", _queryName); - QueryView result = new QueryView(new PostgresUserSchema(getUser(), getContainer()), qSettings, errors) - { - @Override - public DataView createDataView() - { - // Troubleshooters don't have normal read access to the root container so grant them special access - // for these queries - DataView view = super.createDataView(); - view.getRenderContext().getViewContext().addContextualRole(ReaderRole.class); - return view; - } - }; - result.setTitle(_queryName); - result.setFrame(WebPartView.FrameType.PORTAL); - return result; + return super.createQueryView(form, errors, forExport, dataRegion); } @Override public void addNavTrail(NavTree root) { setHelpTopic("postgresActivity"); - addAdminNavTrail(root, "Postgres " + _queryName, this.getClass()); + addAdminNavTrail(root, "Postgres " + getQueryName(), this.getClass()); } - } @AdminConsoleAction @@ -3581,22 +3567,96 @@ public URLHelper getSuccessURL(SystemMaintenanceForm form) } } + private abstract static class AbstractAdminQueryAction extends QueryViewAction + { + private final String _schemaName; + private final String _queryName; + + protected AbstractAdminQueryAction(String schemaName, String queryName) + { + super(QueryExportForm.class); + _schemaName = schemaName; + _queryName = queryName; + } + + @Override + public void setViewContext(ViewContext context) + { + // Troubleshooters don't have read permissions but DataRegion requires it. I don't love poking an elevated + // user into the ViewContext, but this is the only way I could get DataRegion to see read permission on + // tables that are wrapped by a query (e.g., core.Documents used by DocumentsGroupedByParentType.sql). + context.setUser(ElevatedUser.getElevatedUser(context.getUser(), ReaderRole.class)); + super.setViewContext(context); + } + + @Override + protected QueryView createQueryView(QueryExportForm form, BindException errors, boolean forExport, @Nullable String dataRegion) throws Exception + { + QuerySettings qSettings = new QuerySettings(getViewContext(), _schemaName, _queryName); + if (qSettings.getContainerFilterName() == null) + qSettings.setContainerFilterName(ContainerFilter.Type.AllFolders.name()); + QueryView result = new QueryView(getUserSchema(), qSettings, errors); + result.setTitle(_queryName); + result.setFrame(WebPartView.FrameType.PORTAL); + return result; + } + + protected String getQueryName() + { + return _queryName; + } + + abstract protected UserSchema getUserSchema(); + } + @AdminConsoleAction - public class AttachmentsAction extends SimpleViewAction + public class AttachmentsAction extends AbstractAdminQueryAction { + @SuppressWarnings("unused") // Invoked via reflection + public AttachmentsAction() + { + super("core", "DocumentsGroupedByParentTypeAdmin"); + } + @Override - public ModelAndView getView(Object o, BindException errors) + protected UserSchema getUserSchema() + { + return new CoreQuerySchema(getUser(), getContainer(), false); + } + + @Override + public void addNavTrail(NavTree root) + { + addAdminNavTrail(root, "Documents Grouped by Parent Type", getClass()); + } + } + + @SuppressWarnings("unused") // Linked from core.DocumentsGroupedByParentTypeAdmin + @AdminConsoleAction + public class AttachmentsForTypeAction extends AbstractAdminQueryAction + { + @SuppressWarnings("unused") // Invoked via reflection + public AttachmentsForTypeAction() + { + super("core", "Documents"); + } + + @Override + protected UserSchema getUserSchema() { - return AttachmentService.get().getAdminView(getViewContext().getActionURL()); + return new CoreQuerySchema(getUser(), getContainer(), false); } @Override public void addNavTrail(NavTree root) { - addAdminNavTrail(root, "Attachments", getClass()); + String parentType = getViewContext().getActionURL().getParameter("core.ParentType~eq"); + addAdminNavTrail(root, "Documents Belonging to Parent Type" + (parentType != null ? " \"" + parentType + "\"" : ""), getClass()); } } + // Left behind with no link in the UI; this could be useful to track down orphaned attachments during the migration + // process. Should delete after attachment migration is complete (late 2026?). @AdminConsoleAction public class FindAttachmentParentsAction extends SimpleViewAction { diff --git a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java index 8debe09b8f1..b5a3ff7198b 100644 --- a/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java +++ b/core/src/org/labkey/core/attachment/AttachmentServiceImpl.java @@ -29,8 +29,8 @@ import org.labkey.api.attachments.AttachmentDirectory; import org.labkey.api.attachments.AttachmentFile; import org.labkey.api.attachments.AttachmentParent; -import org.labkey.api.attachments.AttachmentService; import org.labkey.api.attachments.AttachmentParentType; +import org.labkey.api.attachments.AttachmentService; import org.labkey.api.attachments.DocumentWriter; import org.labkey.api.attachments.FileAttachmentFile; import org.labkey.api.attachments.SpringAttachmentFile; @@ -93,10 +93,8 @@ import org.labkey.api.view.ActionURL; import org.labkey.api.view.HttpView; import org.labkey.api.view.JspView; -import org.labkey.api.view.NavTree; import org.labkey.api.view.NotFoundException; import org.labkey.api.view.UnauthorizedException; -import org.labkey.api.view.VBox; import org.labkey.api.view.ViewContext; import org.labkey.api.view.WebPartView; import org.labkey.api.webdav.AbstractDocumentResource; @@ -104,7 +102,6 @@ import org.labkey.api.webdav.DavException; import org.labkey.api.webdav.WebdavResolver; import org.labkey.api.webdav.WebdavResource; -import org.labkey.core.admin.AdminController; import org.labkey.core.query.AttachmentAuditProvider; import org.springframework.http.ContentDisposition; import org.springframework.mock.web.MockMultipartFile; @@ -762,129 +759,7 @@ public Collection getAttachmentParentTypes() } @Override - public HttpView getAdminView(ActionURL currentUrl) - { - String requestedType = currentUrl.getParameter("type"); - AttachmentParentType attachmentParentType = null != requestedType ? ATTACHMENT_TYPE_MAP.get(requestedType) : null; - - if (null == attachmentParentType) - { - boolean findAttachmentParents = "1".equals(currentUrl.getParameter("find")); - - // The first query lists all the attachment types and the attachment counts for each. A separate select from - // core.Documents for each type is needed to associate the Type values with the associated rows. - List selectStatements = new LinkedList<>(); - - for (AttachmentParentType type : getAttachmentParentTypes()) - { - SQLFragment selectStatement = new SQLFragment(); - - // Adding unique column RowId ensures we get the proper count - selectStatement.append("SELECT RowId, CAST(").appendValue(type.getUniqueName()).append(" AS VARCHAR(500)) AS Type FROM ") - .append(CoreSchema.getInstance().getTableInfoDocuments(), "d") - .append(" WHERE "); - addAndVerifyWhereSql(type, selectStatement); - selectStatement.append("\n"); - - selectStatements.add(selectStatement); - } - - SQLFragment allSql = new SQLFragment("SELECT Type, COUNT(*) AS Count FROM (\n"); - allSql.append(SQLFragment.join(selectStatements, "UNION\n")); - allSql.append(") u\nGROUP BY Type\nORDER BY Type"); - ActionURL linkUrl = currentUrl.clone().deleteParameters().addParameter("type", null); - - // The second query shows all attachments that we can't associate with a type. We just need to assemble a big - // WHERE NOT clause that ORs the conditions from every registered type. - SQLFragment whereSql = new SQLFragment(); - String sep = ""; - - for (AttachmentParentType type : getAttachmentParentTypes()) - { - whereSql.append(sep); - sep = " OR"; - whereSql.append("\n("); - addAndVerifyWhereSql(type, whereSql); - whereSql.append(")"); - } - - SQLFragment unknownSql = new SQLFragment("SELECT d.Container, c.Name, d.Parent, d.DocumentName"); - - if (findAttachmentParents) - unknownSql.append(", e.TableName"); - - unknownSql.append(" FROM core.Documents d\n"); - unknownSql.append("INNER JOIN core.Containers c ON c.EntityId = d.Container\n"); - - Set schemasToIgnore = Sets.newCaseInsensitiveHashSet(currentUrl.getParameterValues("ignore")); - - if (findAttachmentParents) - { - unknownSql.append("LEFT OUTER JOIN (\n"); - addSelectAllEntityIdsSql(unknownSql, schemasToIgnore); - unknownSql.append(") e ON e.EntityId = d.Parent\n"); - } - - unknownSql.append("WHERE NOT ("); - unknownSql.append(whereSql); - unknownSql.append(")\n"); - unknownSql.append("ORDER BY Container, Parent, DocumentName"); - - WebPartView unknownView = getResultSetView(unknownSql, "Unknown Attachments", null); - NavTree navMenu = new NavTree(); - - if (!findAttachmentParents) - { - navMenu.addChild(new NavTree("Search for Attachment Parents (Be Patient)", - new ActionURL(AdminController.AttachmentsAction.class, ContainerManager.getRoot()).addParameter("find", 1).addParameter("ignore", "Audit")) - ); - } - else - { - navMenu.addChild(new NavTree("Remove TableName Column", - new ActionURL(AdminController.AttachmentsAction.class, ContainerManager.getRoot())) - ); - - if (schemasToIgnore.isEmpty()) - { - navMenu.addChild(new NavTree("Ignore Audit Schema", - new ActionURL(AdminController.AttachmentsAction.class, ContainerManager.getRoot()).addParameter("find", 1).addParameter("ignore", "Audit")) - ); - } - else - { - navMenu.addChild(new NavTree("Include All Schemas", - new ActionURL(AdminController.AttachmentsAction.class, ContainerManager.getRoot()).addParameter("find", 1)) - ); - } - } - unknownView.setNavMenu(navMenu); - - return new VBox(getResultSetView(allSql, "Attachment Types and Counts", linkUrl), unknownView); - } - else - { - // This query lists all the documents associated with a single type. - SQLFragment oneTypeSql = new SQLFragment("SELECT d.Container, c.Name, d.Parent, d.DocumentName FROM core.Documents d\n" + - "INNER JOIN core.Containers c ON c.EntityId = d.Container\n" + - "WHERE "); - addAndVerifyWhereSql(attachmentParentType, oneTypeSql); - oneTypeSql.append("\nORDER BY Container, Parent, DocumentName"); - - return getResultSetView(oneTypeSql, attachmentParentType.getUniqueName() + " Attachments", null); - } - } - - private void addAndVerifyWhereSql(AttachmentParentType attachmentType, SQLFragment sql) - { - int initialLength = sql.length(); - attachmentType.addWhereSql(sql, "d.Parent", "d.DocumentName"); - if (initialLength == sql.length()) - throw new UnsupportedOperationException("AttachmentType: '" + attachmentType.getUniqueName() + "' did not update attachment WHERE clause."); - } - - @Override - // Joins each row of core.Documents to the table(s) (if any) that contain an entityid matching the document's parent + // Joins each row of core.Documents to the table(s) (if any) that contain an EntityId matching the document's parent public WebPartView getFindAttachmentParentsView() { SQLFragment sql = new SQLFragment("SELECT RowId, CreatedBy, Created, ModifiedBy, Modified, Container, DocumentName, TableName FROM core.Documents LEFT OUTER JOIN (\n"); diff --git a/core/src/org/labkey/core/query/CoreQuerySchema.java b/core/src/org/labkey/core/query/CoreQuerySchema.java index cc213d5f611..1606e415333 100644 --- a/core/src/org/labkey/core/query/CoreQuerySchema.java +++ b/core/src/org/labkey/core/query/CoreQuerySchema.java @@ -17,7 +17,30 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.labkey.api.data.*; +import org.labkey.api.data.BaseColumnInfo; +import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.ContainerForeignKey; +import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.ContainerTable; +import org.labkey.api.data.CoreSchema; +import org.labkey.api.data.DataColumn; +import org.labkey.api.data.DisplayColumn; +import org.labkey.api.data.DisplayColumnFactory; +import org.labkey.api.data.ForeignKey; +import org.labkey.api.data.JdbcType; +import org.labkey.api.data.LookupColumn; +import org.labkey.api.data.MultiValuedForeignKey; +import org.labkey.api.data.MultiValuedLookupColumn; +import org.labkey.api.data.MutableColumnInfo; +import org.labkey.api.data.MvUtil; +import org.labkey.api.data.NullColumnInfo; +import org.labkey.api.data.RenderContext; +import org.labkey.api.data.Results; +import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.SimpleFilter; +import org.labkey.api.data.TableInfo; import org.labkey.api.exp.property.Domain; import org.labkey.api.exp.property.DomainProperty; import org.labkey.api.exp.property.PropertyService; @@ -119,6 +142,7 @@ public Set getTableNames() CONTAINERS_TABLE_NAME, WORKBOOKS_TABLE_NAME, QCSTATE_TABLE_NAME, DATA_STATES_TABLE_NAME, VIEW_CATEGORY_TABLE_NAME, MISSING_VALUE_INDICATOR_TABLE_NAME); + // Don't show troubleshooters the query in the schema browser since query-execute.view requires read permissions if (getUser().hasRootPermission(ApplicationAdminPermission.class)) names.add(DOCUMENTS_TABLE_NAME); @@ -180,7 +204,8 @@ public TableInfo createTable(String name, ContainerFilter cf) return getMVIndicatorTable(cf); if (SHORT_URL_TABLE_NAME.equalsIgnoreCase(name) && ShortUrlTableInfo.canDisplayTable(getUser(), getContainer())) return new ShortUrlTableInfo(this); - if (DOCUMENTS_TABLE_NAME.equalsIgnoreCase(name) && getUser().hasRootPermission(ApplicationAdminPermission.class)) + // Allow troubleshooters to view this query from the admin console + if (DOCUMENTS_TABLE_NAME.equalsIgnoreCase(name) && getUser().hasRootPermission(TroubleshooterPermission.class)) return new DocumentsTable(this, cf); return null; diff --git a/core/test/src/org/labkey/test/tests/AttachmentsTest.java b/core/test/src/org/labkey/test/tests/AttachmentsTest.java index f8c00ffe9eb..94ed6779a0d 100644 --- a/core/test/src/org/labkey/test/tests/AttachmentsTest.java +++ b/core/test/src/org/labkey/test/tests/AttachmentsTest.java @@ -27,6 +27,8 @@ import java.util.List; import java.util.Map; +import static org.labkey.test.util.PermissionsHelper.READER_ROLE; + /** * Regression coverage for attachment parent types in audit log and attachment queries. Related PR * Create a few issues, wiki pages, and messages with attachments. Verify that rows containing the expected parent @@ -107,10 +109,18 @@ public void testParentTypesInAuditLog() throws IOException, CommandException @Test public void testParentTypesInAttachmentQueries() throws ParseException + { + testQueries(); + impersonateRole("Troubleshooter"); + testQueries(); + stopImpersonating(); + } + + private void testQueries() throws ParseException { ShowAdminPage.beginAt(this); clickAndWait(Locator.linkWithText("Attachments")); - DataRegionTable table = DataRegionTable.DataRegion(getDriver()).withName("query").waitFor(); + DataRegionTable table = DataRegionTable.DataRegion(getDriver()).withName("core").waitFor(); assertTextPresent("DocumentsGroupedByParentType"); verifyRow(table, "IssueComment", ISSUE_ATTACHMENTS); int wikiCount = verifyRow(table, "Wiki", WIKI_ATTACHMENTS); @@ -118,7 +128,7 @@ public void testParentTypesInAttachmentQueries() throws ParseException int wikiRowIndex = table.getRowIndex("ParentType", "Wiki"); clickAndWait(table.link(wikiRowIndex, "Count")); - table = DataRegionTable.DataRegion(getDriver()).withName("query").waitFor(); + table = DataRegionTable.DataRegion(getDriver()).withName("core").waitFor(); assertTextPresent("Documents"); if (wikiCount > 100) table.assertPaginationText(1, 100, wikiCount); @@ -136,4 +146,16 @@ private int verifyRow(DataRegionTable table, String parentType, int minimum) thr Assert.assertTrue("Count for " + parentType + " was less than expected: " + count + " vs. " + minimum, count >= minimum); return count; } + + @Test + public void testNoDocumentsTableForReaders() + { + goToProjectHome(); + goToSchemaBrowser(); + // Admins should have access to core.Documents + viewQueryData("core", "Documents"); + // Readers should have no access + impersonateRole(READER_ROLE); + assertTextPresent("The specified query 'Documents' does not exist in schema 'core'"); + } }