diff --git a/api/src/org/labkey/api/audit/query/DefaultAuditTypeTable.java b/api/src/org/labkey/api/audit/query/DefaultAuditTypeTable.java index dae043f66f0..3f3b0af48a2 100644 --- a/api/src/org/labkey/api/audit/query/DefaultAuditTypeTable.java +++ b/api/src/org/labkey/api/audit/query/DefaultAuditTypeTable.java @@ -34,9 +34,14 @@ import org.labkey.api.query.QueryUpdateService; import org.labkey.api.query.UserSchema; import org.labkey.api.query.column.BuiltInColumnTypes; +import org.labkey.api.security.SecurityManager; +import org.labkey.api.security.User; import org.labkey.api.security.UserPrincipal; import org.labkey.api.security.permissions.Permission; import org.labkey.api.security.permissions.ReadPermission; +import org.labkey.api.security.roles.CanSeeAuditLogRole; +import org.labkey.api.security.roles.Role; +import org.labkey.api.security.roles.RoleManager; import java.util.ArrayList; import java.util.List; @@ -114,7 +119,14 @@ protected void initColumns() @Override protected SimpleFilter.FilterClause getContainerFilterClause(ContainerFilter filter, FieldKey fieldKey) { - return filter.createFilterClause(getSchema(), fieldKey, CanSeeAuditLogPermission.class, Set.of()); + // TODO: Setting a contextual role on the container filter clause should not be necessary; the user passed + // (separately) to the ContainerFilter should have the appropriate permission. However, some app actions + // (GetTransactionRowIdsAction, maybe GetLocationHistoryAction, etc.) have been relying on this behavior. Clean + // this up soon, but not for 26.3. Note that this is the only code path that passes contextual roles into + // createFilterClause(), so we could eliminate that option during clean up. + User user = (null == getUserSchema()) ? null : getUserSchema().getUser(); + Set roles = SecurityManager.canSeeAuditLog(user) ? RoleManager.roleSet(CanSeeAuditLogRole.class) : null; + return filter.createFilterClause(getSchema(), fieldKey, CanSeeAuditLogPermission.class, roles); } // Subclasses may override this to provide customizations to the column diff --git a/api/src/org/labkey/api/data/ContainerFilter.java b/api/src/org/labkey/api/data/ContainerFilter.java index 4576d8ec25a..fb495327f14 100644 --- a/api/src/org/labkey/api/data/ContainerFilter.java +++ b/api/src/org/labkey/api/data/ContainerFilter.java @@ -169,9 +169,9 @@ public SimpleFilter.FilterClause createFilterClause(DbSchema schema, FieldKey co } /** Create a FilterClause that restricts based on the containers that meet the filter and user that meets the permission*/ - public SimpleFilter.FilterClause createFilterClause(DbSchema schema, FieldKey containerFilterColumn, Class permission, Set roles) + public SimpleFilter.FilterClause createFilterClause(DbSchema schema, FieldKey containerFilterColumn, Class permission, Set contextualRoles) { - return new ContainerClause(schema, containerFilterColumn, this, permission, roles); + return new ContainerClause(schema, containerFilterColumn, this, permission, contextualRoles); } diff --git a/api/src/org/labkey/api/data/RecordFactory.java b/api/src/org/labkey/api/data/RecordFactory.java index abd5470f2b0..4b4c5a201c6 100644 --- a/api/src/org/labkey/api/data/RecordFactory.java +++ b/api/src/org/labkey/api/data/RecordFactory.java @@ -52,7 +52,7 @@ public RecordFactory(Class clazz) { Object[] params = Arrays.stream(_parameters).map(p -> { Object value = m.get(p.getName()); - return ConvertUtils.convert(value, p.getType()); + return value != null ? ConvertUtils.convert(value, p.getType()) : null; }).toArray(); try diff --git a/api/src/org/labkey/api/data/SqlScriptExecutor.java b/api/src/org/labkey/api/data/SqlScriptExecutor.java index 756b29891b0..e8df99cc377 100644 --- a/api/src/org/labkey/api/data/SqlScriptExecutor.java +++ b/api/src/org/labkey/api/data/SqlScriptExecutor.java @@ -221,6 +221,7 @@ public void execute() { LOG.info("Executing {}", upgradeMethod.getDisplayName()); _moduleContext.invokeUpgradeMethod(upgradeMethod); + LOG.info("Finished executing {}", upgradeMethod.getDisplayName()); } } catch (NoSuchMethodException e) diff --git a/api/src/org/labkey/api/data/dialect/PostgreSqlService.java b/api/src/org/labkey/api/data/dialect/PostgreSqlService.java new file mode 100644 index 00000000000..fec37101cbc --- /dev/null +++ b/api/src/org/labkey/api/data/dialect/PostgreSqlService.java @@ -0,0 +1,18 @@ +package org.labkey.api.data.dialect; + +import org.labkey.api.services.ServiceRegistry; + +public interface PostgreSqlService +{ + static PostgreSqlService get() + { + return ServiceRegistry.get().getService(PostgreSqlService.class); + } + + static void setInstance(PostgreSqlService impl) + { + ServiceRegistry.get().registerService(PostgreSqlService.class, impl); + } + + BasePostgreSqlDialect getDialect(); +} diff --git a/api/src/org/labkey/api/query/QueryView.java b/api/src/org/labkey/api/query/QueryView.java index be99e20907c..bdf1e5c638e 100644 --- a/api/src/org/labkey/api/query/QueryView.java +++ b/api/src/org/labkey/api/query/QueryView.java @@ -1050,52 +1050,6 @@ public ActionButton createDeleteButton(boolean showConfirmation) return null; } - public ActionButton createDeleteAllRowsButton(String tableNoun) - { - ActionButton deleteAllRows = new ActionButton("Delete All Rows"); - deleteAllRows.setDisplayPermission(AdminPermission.class); - deleteAllRows.setActionType(ActionButton.Action.SCRIPT); - deleteAllRows.setScript( - "LABKEY.requiresExt4Sandbox(function() {" + - "Ext4.Msg.confirm('Confirm Deletion', 'Are you sure you wish to delete all rows in this " + tableNoun + "? This action cannot be undone and will result in an empty " + tableNoun + ".', function(button){" + - "if (button == 'yes'){" + - "var waitMask = Ext4.Msg.wait('Deleting Rows...', 'Delete Rows'); " + - "Ext4.Ajax.request({ " + - "url : LABKEY.ActionURL.buildURL('query', 'truncateTable'), " + - "method : 'POST', " + - "success: function(response) " + - "{" + - "waitMask.close(); " + - "var data = Ext4.JSON.decode(response.responseText); " + - "Ext4.Msg.show({ " + - "title : 'Success', " + - "buttons : Ext4.MessageBox.OK, " + - "msg : data.deletedRows + ' rows deleted', " + - "fn: function(btn) " + - "{ " + - "if(btn == 'ok') " + - "{ " + - "window.location.reload(); " + - "} " + - "} " + - "})" + - "}, " + - "failure : function(response, opts) " + - "{ " + - "waitMask.close(); " + - "Ext4.getBody().unmask(); " + - "LABKEY.Utils.displayAjaxErrorResponse(response, opts); " + - "}, " + - "jsonData : {schemaName : " + PageFlowUtil.jsString(getQueryDef().getSchema().getName()) + ", queryName : " + PageFlowUtil.jsString(getQueryDef().getName()) + "}, " + - "scope : this " + - "});" + - "}" + - "});" + - "});" - ); - return deleteAllRows; - } - public ActionButton createInsertMenuButton() { return createInsertMenuButton(null, null); diff --git a/audit/src/org/labkey/audit/AuditController.java b/audit/src/org/labkey/audit/AuditController.java index e44599c0749..acd22a9e9b5 100644 --- a/audit/src/org/labkey/audit/AuditController.java +++ b/audit/src/org/labkey/audit/AuditController.java @@ -383,11 +383,12 @@ public void validateForm(AuditTransactionForm form, Errors errors) public Object execute(AuditTransactionForm form, BindException errors) { List rowIds; - ContainerFilter cf = ContainerFilter.getContainerFilterByName(form.getContainerFilter(), getContainer(), getUser()); + User elevatedUser = ElevatedUser.ensureCanSeeAuditLogRole(getContainer(), getUser()); + ContainerFilter cf = ContainerFilter.getContainerFilterByName(form.getContainerFilter(), getContainer(), elevatedUser); if (form.isSampleType()) - rowIds = AuditLogImpl.get().getTransactionSampleIds(form.getTransactionAuditId(), ElevatedUser.ensureCanSeeAuditLogRole(getContainer(), getUser()), getContainer(), cf); + rowIds = AuditLogImpl.get().getTransactionSampleIds(form.getTransactionAuditId(), elevatedUser, getContainer(), cf); else - rowIds = AuditLogImpl.get().getTransactionSourceIds(form.getTransactionAuditId(), ElevatedUser.ensureCanSeeAuditLogRole(getContainer(), getUser()), getContainer(), cf); + rowIds = AuditLogImpl.get().getTransactionSourceIds(form.getTransactionAuditId(), elevatedUser, getContainer(), cf); ApiSimpleResponse response = new ApiSimpleResponse(); response.put("success", true); diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index 0698a8e0f08..4c5cc386958 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -77,6 +77,7 @@ import org.labkey.api.data.TestSchema; import org.labkey.api.data.WorkbookContainerType; import org.labkey.api.data.dialect.BasePostgreSqlDialect; +import org.labkey.api.data.dialect.PostgreSqlService; import org.labkey.api.data.dialect.SqlDialect; import org.labkey.api.data.dialect.SqlDialectManager; import org.labkey.api.data.dialect.SqlDialectRegistry; @@ -88,6 +89,7 @@ import org.labkey.api.files.FileBrowserConfigWriter; import org.labkey.api.files.FileContentService; import org.labkey.api.markdown.MarkdownService; +import org.labkey.api.mcp.McpService; import org.labkey.api.message.settings.MessageConfigService; import org.labkey.api.migration.DatabaseMigrationService; import org.labkey.api.module.FolderType; @@ -98,7 +100,6 @@ import org.labkey.api.module.SchemaUpdateType; import org.labkey.api.module.SpringModule; import org.labkey.api.module.Summary; -import org.labkey.api.mcp.McpService; import org.labkey.api.notification.EmailMessage; import org.labkey.api.notification.EmailService; import org.labkey.api.notification.NotificationMenuView; @@ -253,9 +254,9 @@ import org.labkey.core.login.DbLoginAuthenticationProvider; import org.labkey.core.login.DbLoginManager; import org.labkey.core.login.LoginController; +import org.labkey.core.mcp.McpServiceImpl; import org.labkey.core.metrics.SimpleMetricsServiceImpl; import org.labkey.core.metrics.WebSocketConnectionManager; -import org.labkey.core.mcp.McpServiceImpl; import org.labkey.core.notification.EmailPreferenceConfigServiceImpl; import org.labkey.core.notification.EmailPreferenceContainerListener; import org.labkey.core.notification.EmailPreferenceUserListener; @@ -560,11 +561,11 @@ public QuerySchema createSchema(DefaultSchema schema, Module module) ScriptEngineManagerImpl.registerEncryptionMigrationHandler(); McpService.get().register(new CoreMcp()); + PostgreSqlService.setInstance(PostgreSqlDialectFactory::getLatestSupportedDialect); deleteTempFiles(); } - private void deleteTempFiles() { try diff --git a/core/src/org/labkey/core/admin/sql/SqlScriptController.java b/core/src/org/labkey/core/admin/sql/SqlScriptController.java index 73f4e5678a5..a7868284993 100644 --- a/core/src/org/labkey/core/admin/sql/SqlScriptController.java +++ b/core/src/org/labkey/core/admin/sql/SqlScriptController.java @@ -1605,6 +1605,7 @@ public boolean handlePost(UpgradeCodeForm form, BindException errors) throws Exc { LOG.info("Executing {}.{}(ModuleContext moduleContext)", _method.getDeclaringClass().getSimpleName(), _method.getName()); _method.invoke(_code, _ctx); + LOG.info("Finished executing {}.{}(ModuleContext moduleContext)", _method.getDeclaringClass().getSimpleName(), _method.getName()); return true; } diff --git a/core/src/org/labkey/core/dialect/PostgreSqlDialectFactory.java b/core/src/org/labkey/core/dialect/PostgreSqlDialectFactory.java index 5e3fa1914f4..3a8a4845799 100644 --- a/core/src/org/labkey/core/dialect/PostgreSqlDialectFactory.java +++ b/core/src/org/labkey/core/dialect/PostgreSqlDialectFactory.java @@ -24,6 +24,7 @@ import org.junit.Assert; import org.junit.Test; import org.labkey.api.data.dialect.AbstractDialectRetrievalTestCase; +import org.labkey.api.data.dialect.BasePostgreSqlDialect; import org.labkey.api.data.dialect.DatabaseNotSupportedException; import org.labkey.api.data.dialect.JdbcHelperTest; import org.labkey.api.data.dialect.PostgreSqlServerType; @@ -145,6 +146,11 @@ public static PostgreSql_13_Dialect getOldestSupportedDialect() return new PostgreSql_13_Dialect(); } + public static BasePostgreSqlDialect getLatestSupportedDialect() + { + return new PostgreSql_18_Dialect(); + } + public static class DialectRetrievalTestCase extends AbstractDialectRetrievalTestCase { @Override diff --git a/experiment/resources/schemas/dbscripts/sqlserver/exp-26.004-26.005.sql b/experiment/resources/schemas/dbscripts/sqlserver/exp-26.004-26.005.sql new file mode 100644 index 00000000000..b6f3e71a4b5 --- /dev/null +++ b/experiment/resources/schemas/dbscripts/sqlserver/exp-26.004-26.005.sql @@ -0,0 +1,2 @@ +-- SQL Server only +EXEC core.executeJavaUpgradeCode 'shortenAllStorageNames'; diff --git a/experiment/src/client/test/integration/AssayImportRunAction.ispec.ts b/experiment/src/client/test/integration/AssayImportRunAction.ispec.ts index f38134375c1..04f1105f12e 100644 --- a/experiment/src/client/test/integration/AssayImportRunAction.ispec.ts +++ b/experiment/src/client/test/integration/AssayImportRunAction.ispec.ts @@ -118,7 +118,7 @@ async function verifyPropertiesFilesOnServer( // Note: this is a hack to allow us to make requests to the webdav controller. The IntegrationTestServer // request method uses ActionURL to generate URLS, but webdav URLs are not ActionURLs, so we need to // override the AgentProvider to generate the proper webdav URL. - url = `/_webdav/${requestOptions.containerPath}/%40files/assaydata?method=JSON`; + url = `${LABKEY.contextPath}/_webdav/${requestOptions.containerPath}/%40files/assaydata?method=JSON`; return agent.get(url); }, requestOptions diff --git a/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java index 0069378ea20..d9071d11b14 100644 --- a/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java @@ -178,11 +178,12 @@ public void afterSchema(DatabaseMigrationConfiguration configuration, DbSchema s DbScope sourceScope = configuration.getSourceScope(); DbScope targetScope = configuration.getTargetScope(); - DbSchema biologicsSourceSchema = sourceScope.getSchema("biologics", DbSchemaType.Migration); - DbSchema biologicsTargetSchema = targetScope.getSchema("biologics", DbSchemaType.Module); - if (biologicsSourceSchema.existsInDatabase() && biologicsTargetSchema.existsInDatabase()) + if (sourceScope.getSchemaNames().contains("biologics") && targetScope.getSchemaNames().contains("biologics")) { + DbSchema biologicsSourceSchema = sourceScope.getSchema("biologics", DbSchemaType.Migration); + DbSchema biologicsTargetSchema = targetScope.getSchema("biologics", DbSchemaType.Module); + TableInfo sourceTable = biologicsSourceSchema.getTable("SequenceIdentity"); TableInfo targetTable = biologicsTargetSchema.getTable("SequenceIdentity"); diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 3808f929df0..5d452027778 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -205,7 +205,7 @@ public String getName() @Override public Double getSchemaVersion() { - return 26.004; + return 26.005; } @Nullable diff --git a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java index 8c4a644d3cb..e693224a7ec 100644 --- a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java +++ b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java @@ -15,6 +15,8 @@ */ package org.labkey.experiment; +import org.apache.commons.collections4.MultiValuedMap; +import org.apache.commons.collections4.multimap.ArrayListValuedHashMap; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; @@ -25,11 +27,15 @@ import org.labkey.api.audit.SampleTimelineAuditEvent; import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.collections.CaseInsensitiveHashSet; +import org.labkey.api.collections.CsvSet; +import org.labkey.api.collections.LabKeyCollectors; import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.CompareType; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbScope; +import org.labkey.api.data.DbScope.Transaction; import org.labkey.api.data.DeferredUpgrade; import org.labkey.api.data.JdbcType; import org.labkey.api.data.Parameter; @@ -39,11 +45,16 @@ import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SchemaTableInfo; import org.labkey.api.data.Selector; +import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.SqlExecutor; import org.labkey.api.data.SqlSelector; +import org.labkey.api.data.Table; import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; import org.labkey.api.data.UpgradeCode; +import org.labkey.api.data.dialect.BasePostgreSqlDialect; +import org.labkey.api.data.dialect.PostgreSqlService; +import org.labkey.api.exp.OntologyManager; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.api.ExpSampleType; import org.labkey.api.exp.api.ExperimentService; @@ -64,6 +75,8 @@ import org.labkey.api.security.User; import org.labkey.api.security.roles.SiteAdminRole; import org.labkey.api.settings.AppProps; +import org.labkey.api.util.PageFlowUtil; +import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.util.logging.LogHelper; import org.labkey.experiment.api.DataClass; import org.labkey.experiment.api.DataClassDomainKind; @@ -73,11 +86,14 @@ import org.labkey.experiment.api.MaterialSource; import org.labkey.experiment.api.property.DomainImpl; import org.labkey.experiment.api.property.DomainPropertyImpl; +import org.labkey.experiment.api.property.StorageNameGenerator; import org.labkey.experiment.api.property.StorageProvisionerImpl; +import java.nio.charset.StandardCharsets; import java.sql.Connection; import java.sql.SQLException; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -153,7 +169,7 @@ public static void upgradeAmountsAndUnits(ModuleContext context) DbScope scope = ExperimentService.get().getSchema().getScope(); LimitedUser admin = new LimitedUser(context.getUpgradeUser(), SiteAdminRole.class); - try (DbScope.Transaction transaction = scope.ensureTransaction()) + try (Transaction transaction = scope.ensureTransaction()) { // create a single transaction event at the root container for use in tying all updates together TransactionAuditProvider.TransactionAuditEvent transactionEvent = AbstractQueryUpdateService.createTransactionAuditEvent(ContainerManager.getRoot(), QueryService.AuditAction.UPDATE); @@ -174,7 +190,6 @@ public static void upgradeAmountsAndUnits(ModuleContext context) } ExperimentService.get().clearCaches(); } - } private static void getAmountAndUnitUpdates(Map sampleMap, Parameter unitsCol, Set amountCols, Unit currentDisplayUnit, Map oldDataMap, Map newDataMap, Map sampleCounts, boolean aliquotFields) @@ -366,7 +381,7 @@ public static void dropProvisionedSampleTypeLsidColumn(ModuleContext context) if (context.isNewInstall()) return; - try (DbScope.Transaction tx = ExperimentService.get().ensureTransaction()) + try (Transaction tx = ExperimentService.get().ensureTransaction()) { // Process all sample types across all containers TableInfo sampleTypeTable = ExperimentServiceImpl.get().getTinfoSampleType(); @@ -500,7 +515,7 @@ public static void fixContainerForMovedSampleFiles(ModuleContext context) if (context.isNewInstall()) return; - try (DbScope.Transaction tx = ExperimentService.get().ensureTransaction()) + try (Transaction tx = ExperimentService.get().ensureTransaction()) { FileContentService service = FileContentService.get(); if (service == null) @@ -525,7 +540,7 @@ public static void addRowIdToProvisionedDataClassTables(ModuleContext context) if (context.isNewInstall()) return; - try (DbScope.Transaction tx = ExperimentService.get().ensureTransaction()) + try (Transaction tx = ExperimentService.get().ensureTransaction()) { TableInfo source = ExperimentServiceImpl.get().getTinfoDataClass(); new TableSelector(source, null, null).stream(DataClass.class) @@ -656,5 +671,149 @@ private static void fillRowId(ExpDataClassImpl dc, Domain domain, DbScope scope) LOG.info("DataClass '{}' ({}) populated 'rowId' column, count={}", dc.getName(), dc.getRowId(), count); } + record DomainRecord(Container container, int domainId, String name, String storageSchemaName, String storageTableName) + { + String fullName() + { + return storageSchemaName + "." + storageTableName; + } + } + + record Property(int domainId, int propertyId, String domainName, String name, String storageSchemaName, String storageTableName, String storageColumnName) + { + String fullName() + { + // Have to bracket storage column name since it could have special characters (like dots) + return storageSchemaName + "." + storageTableName + "." + bracketIt(storageColumnName); + } + + // Bracket name and escape any internal ending brackets + private String bracketIt(String name) + { + return "[" + name.replace("]", "]]") + "]"; + } + } + + /** + * Called from exp-26.004-26.005.sql, on SQL Server only + * GitHub Issue 869: Long table/column names cause SQL Server migration to fail + * Query all table & column storage names and rename the ones that are too long for PostgreSQL + * TODO: When this upgrade code is removed, get rid of the StorageProvisionerImpl.makeTableName() method it uses. + */ + @SuppressWarnings("unused") + public static void shortenAllStorageNames(ModuleContext context) + { + if (context.isNewInstall()) + return; + + // The PostgreSQL dialect knows which names are too long + BasePostgreSqlDialect dialect = PostgreSqlService.get().getDialect(); + DbScope scope = DbScope.getLabKeyScope(); + SqlExecutor executor = new SqlExecutor(scope); + // Stream all the storage table names and rename the ones that are too long for PostgreSQL. The filtering must + // be done in code by the dialect; SQL Server has BYTELENGTH(), but that function returns values that are not + // consistent with our dialect check. Also, it looks like the function's behavior changed starting in SS 2019. + TableInfo tinfoDomainDescriptor = OntologyManager.getTinfoDomainDescriptor(); + SimpleFilter filter = new SimpleFilter(FieldKey.fromString("StorageSchemaName"), null, CompareType.NONBLANK); + filter.addCondition(FieldKey.fromString("StorageTableName"), null, CompareType.NONBLANK); + + new TableSelector(tinfoDomainDescriptor, new CsvSet("Container, DomainId, Name, StorageSchemaName, StorageTableName"), filter, null) + .setJdbcCaching(false) + .stream(DomainRecord.class) + .filter(domain -> dialect.isIdentifierTooLong(domain.storageTableName())) + .forEach(domain -> { + String oldName = domain.fullName(); + String newName = StorageProvisionerImpl.get().makeTableName(dialect, domain.container(), domain.domainId(), domain.name()); + + try (Transaction transaction = scope.beginTransaction()) + { + executor.execute(new SQLFragment("EXEC sp_rename ?, ?").add(oldName).add(newName)); + Table.update(null, tinfoDomainDescriptor, PageFlowUtil.map("StorageTableName", newName), domain.domainId()); + transaction.commit(); + } + + LOG.info(" Table \"{}\" renamed to \"{}\" ({} bytes)", oldName, newName, newName.getBytes(StandardCharsets.UTF_8).length); + }); + + List badTableNames = new TableSelector(tinfoDomainDescriptor, new CsvSet("StorageTableName"), filter, null) + .setJdbcCaching(false) + .stream(String.class) + .filter(dialect::isIdentifierTooLong) + .toList(); + + if (!badTableNames.isEmpty()) + LOG.error("Some storage table names are still too long!! {}", badTableNames); + + // Collect all the domains that have one or more storage columns names that are too long for PostgreSQL + TableInfo tinfoPropertyDomain = OntologyManager.getTinfoPropertyDomain(); + TableInfo tinfoPropertyDescriptor = OntologyManager.getTinfoPropertyDescriptor(); + SQLFragment sql = new SQLFragment("SELECT dd.DomainId, dd.Name AS DomainName, px.PropertyId, StorageSchemaName, StorageTableName, StorageColumnName, px.Name FROM ") + .append(tinfoDomainDescriptor, "dd") + .append(" INNER JOIN ") + .append(tinfoPropertyDomain, "pd") + .append(" ON dd.DomainId = pd.DomainId INNER JOIN ") + .append(tinfoPropertyDescriptor, "px") + .append(" ON pd.PropertyId = px.PropertyId ") + .append("WHERE StorageSchemaName IS NOT NULL AND StorageTableName IS NOT NULL AND StorageColumnName IS NOT NULL"); + + MultiValuedMap badDomainMap = new SqlSelector(scope, sql) + .setJdbcCaching(false) + .stream(Property.class) + .filter(property -> dialect.isIdentifierTooLong(property.storageColumnName())) + .collect(LabKeyCollectors.toMultiValuedMap( + property -> new DomainRecord(null, property.domainId(), property.domainName(), property.storageSchemaName(), property.storageTableName()), + property -> property, + ArrayListValuedHashMap::new) + ); + + if (!badDomainMap.isEmpty()) + LOG.info(" Found {} with storage column names that are too long for PostgreSQL:", StringUtilsLabKey.pluralize(badDomainMap.keySet().size(), "domain")); + + // Now enumerate the bad domains and rename their bad storage columns using the PostgreSQL truncation rules + badDomainMap.keySet() + .forEach(domain -> { + Collection badColumns = badDomainMap.get(domain); + List badColumnNames = badColumns.stream().map(Property::storageColumnName).toList(); + + // First, populate a new StorageNameGenerator with all the "good" names in this domain so we don't + // accidentally try to re-use one of them + StorageNameGenerator nameGenerator = new StorageNameGenerator(dialect); + SQLFragment domainSql = new SQLFragment("SELECT StorageColumnName FROM ") + .append(tinfoPropertyDomain, "pd") + .append(" INNER JOIN ") + .append(tinfoPropertyDescriptor, "px") + .append(" ON pd.PropertyId = px.PropertyId ") + .append("WHERE DomainId = ? AND StorageColumnName NOT ") + .add(domain.domainId()) + .appendInClause(badColumnNames, scope.getSqlDialect()); + new SqlSelector(scope, domainSql).forEach(String.class, nameGenerator::claimName); + + LOG.info(" Renaming {} in table \"{}\"", StringUtilsLabKey.pluralize(badColumns.size(), "column"), domain.fullName()); + + // Now use that StorageNameGenerator to create new names. Rename the column and update the PropertyDescriptor table. + badColumns.forEach(property -> { + String oldName = property.fullName(); + String newName = nameGenerator.generateColumnName(property.name()); // No need to bracket or quote or escape: JDBC parameter takes care of all special characters + + try (Transaction transaction = scope.beginTransaction()) + { + executor.execute(new SQLFragment("EXEC sp_rename ?, ?, 'COLUMN'").add(oldName).add(newName)); + Table.update(null, tinfoPropertyDescriptor, PageFlowUtil.map("StorageColumnName", newName), property.propertyId()); + transaction.commit(); + } + + LOG.info(" Column \"{}\" renamed to \"{}\" ({} bytes)", oldName, newName, newName.getBytes(StandardCharsets.UTF_8).length); + }); + }); + + List badColumnNames = new TableSelector(tinfoPropertyDescriptor, new CsvSet("StorageColumnName"), new SimpleFilter(FieldKey.fromString("StorageColumnName"), null, CompareType.NONBLANK), null) + .setJdbcCaching(false) + .stream(String.class) + .filter(dialect::isIdentifierTooLong) + .toList(); + + if (!badColumnNames.isEmpty()) + LOG.error("Some storage column names are still too long!! {}", badColumnNames); + } } diff --git a/experiment/src/org/labkey/experiment/api/property/StorageNameGenerator.java b/experiment/src/org/labkey/experiment/api/property/StorageNameGenerator.java index 16e82fccc48..d6bdbf9c596 100644 --- a/experiment/src/org/labkey/experiment/api/property/StorageNameGenerator.java +++ b/experiment/src/org/labkey/experiment/api/property/StorageNameGenerator.java @@ -6,6 +6,7 @@ import org.junit.Test; import org.labkey.api.collections.CaseInsensitiveHashSet; import org.labkey.api.data.DbScope; +import org.labkey.api.data.dialect.PostgreSqlService; import org.labkey.api.data.dialect.SqlDialect; import org.labkey.api.exp.OntologyManager; import org.labkey.api.util.StringUtilsLabKey; @@ -32,7 +33,8 @@ public class StorageNameGenerator public StorageNameGenerator(@NotNull SqlDialect dialect) { - _dialect = dialect; + // GitHub Issue 869: Create SQL Server storage names using PostgreSQL's rules to ensure all tables and columns can migrate + _dialect = dialect.isSqlServer() ? PostgreSqlService.get().getDialect() : dialect; } public String claimName(String name) @@ -75,7 +77,7 @@ private String generateName(String candidateName, int reserved) ret = legalName + i; } - if (_dialect.isIdentifierTooLong(ret)) + if (isIdentifierTooLong(ret)) throw new IllegalStateException("generateName() produced a name that was too long for " + _dialect.getProductName() + ": \"" + ret + "\" was generated from \"" + candidateName + "\""); if (ret.length() > 255) @@ -84,6 +86,11 @@ private String generateName(String candidateName, int reserved) return claimName(ret); } + private boolean isIdentifierTooLong(String candidate) + { + return _dialect.isIdentifierTooLong(candidate); + } + public static class TestCase extends Assert { @Test @@ -99,11 +106,11 @@ public void testGenerateName() testCandidates(255, dialect, StringUtilsLabKey::generateSpecialCharacterString); // Test that the same string over and over again generates a unique name - testCandidates(255, dialect, i -> "kumquat"); + testCandidates(255, dialect, _ -> "kumquat"); // Same, but test case sensitivity testCandidates(255, dialect, i -> i % 2 == 0 ? "kumquat" : "KUMQUAT"); String randomString = StringUtilsLabKey.generateSpecialCharacterString(255); - testCandidates(255, dialect, i -> randomString); + testCandidates(255, dialect, _ -> randomString); } private void testCandidates(int count, SqlDialect dialect, Function candidateSupplier) @@ -117,7 +124,7 @@ private void testCandidates(int count, SqlDialect dialect, Function kind, Domain domain) { - String rawTableName = String.format("c%sd%s_%s", domain.getContainer().getRowId(), domain.getTypeId(), domain.getName()); - SqlDialect dialect = kind.getScope().getSqlDialect(); + return makeTableName(kind.getScope().getSqlDialect(), domain.getContainer(), domain.getTypeId(), domain.getName()); + } + + // Needed by ExperimentUpgradeCode.shortenAllStorageNames(). When that code is removed, combine this with above variant. + public String makeTableName(SqlDialect dialect, Container c, int typeId, String domainName) + { + String rawTableName = String.format("c%sd%s_%s", c.getRowId(), typeId, domainName); return new StorageNameGenerator(dialect).generateTableName(rawTableName); } diff --git a/list/src/org/labkey/list/controllers/ListController.java b/list/src/org/labkey/list/controllers/ListController.java index 44cda4aa4d5..2a5b90e4e8f 100644 --- a/list/src/org/labkey/list/controllers/ListController.java +++ b/list/src/org/labkey/list/controllers/ListController.java @@ -51,6 +51,7 @@ import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DataRegionSelection; +import org.labkey.api.data.DbScope; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.TableInfo; @@ -116,6 +117,7 @@ import org.labkey.list.model.ListDomainKindProperties; import org.labkey.list.model.ListManager; import org.labkey.list.model.ListManagerSchema; +import org.labkey.list.model.ListSchema; import org.labkey.list.model.ListWriter; import org.labkey.list.view.ListDefinitionForm; import org.labkey.list.view.ListItemAttachmentParent; @@ -425,6 +427,103 @@ public List> getListContainerMap() } } + @RequiresPermission(AdminPermission.class) + public static class TruncateListDataAction extends ConfirmAction + { + private boolean canTruncate(Container listContainer, int listId) + { + ListDef listDef = ListManager.get().getList(listContainer, listId); + ListDefinitionImpl list = ListDefinitionImpl.of(listDef); + + if (list == null || !list.getAllowDelete()) + return false; + + return list.getContainer().hasPermission(getUser(), AdminPermission.class); + } + + @Override + public String getConfirmText() + { + return "Confirm Delete All Data"; + } + + @Override + public void validateCommand(ListDeletionForm form, Errors errors) + { + Container currentContainer = getContainer(); + List errorMessages = new ArrayList<>(); + Collection listIDs; + if (form.getListIds() != null) + listIDs = form.getListIds(); + else + listIDs = DataRegionSelection.getSelected(form.getViewContext(), true); + + for (Pair pair : getListIdContainerPairs(listIDs, currentContainer, errorMessages)) + { + var listId = pair.first; + var listContainer = pair.second; + + if (canTruncate(listContainer, listId)) + { + form.getListContainerMap().add(pair); + } + else + errorMessages.add(String.format("You do not have permission to delete data for list %s in container %s", listId, listContainer.getName())); + } + + if (!errorMessages.isEmpty()) + errors.reject(ERROR_MSG, StringUtils.join(errorMessages, "\n")); + + if (form.getListContainerMap().isEmpty()) + errors.reject(ERROR_MSG, "You must specify a list or lists to delete data from."); + } + + @Override + public ModelAndView getConfirmView(ListDeletionForm form, BindException errors) + { + if (getPageConfig().getTitle() == null) + setTitle("Confirm Delete All Data"); + return new JspView<>("/org/labkey/list/view/truncateListData.jsp", form, errors); + } + + @Override + public boolean handlePost(ListDeletionForm form, BindException errors) + { + Container containerDataToDelete = getContainer(); + try (DbScope.Transaction transaction = ListSchema.getInstance().getSchema().getScope().ensureTransaction()) + { + for (Pair pair : form.getListContainerMap()) + { + Container listDefContainer = pair.second; + ListDefinition listDef = ListService.get().getList(listDefContainer, pair.first); + if (null != listDef) + { + try + { + TableInfo table = listDef.getTable(getUser(), listDefContainer); + if (table != null && table.getUpdateService() != null) + table.getUpdateService().truncateRows(getUser(), containerDataToDelete, null, null); + } + catch (Exception e) + { + errors.reject(ERROR_MSG, "Error deleting data from list '" + listDef.getName() + "': " + e.getMessage()); + return false; + } + } + } + + transaction.commit(); + } + + return !errors.hasErrors(); + } + + @Override @NotNull + public URLHelper getSuccessURL(ListDeletionForm form) + { + return form.getReturnUrlHelper(getBeginURL(getContainer())); + } + } @RequiresPermission(ReadPermission.class) public class GridAction extends SimpleViewAction @@ -958,10 +1057,11 @@ public Pair getAttachment(ListAttachmentForm form) if (listDef == null) throw new NotFoundException("List does not exist in this container"); - if (!listDef.hasListItemForEntityId(form.getEntityId(), getUser())) + Container dataContainer = listDef.getListItemContainerForDownload(form.getEntityId(), getUser(), ReadPermission.class); + if (dataContainer == null) throw new NotFoundException("List does not have an item for the entityid"); - AttachmentParent parent = new ListItemAttachmentParent(form.getEntityId(), getContainer()); + AttachmentParent parent = new ListItemAttachmentParent(form.getEntityId(), dataContainer); return new Pair<>(parent, form.getName()); } diff --git a/list/src/org/labkey/list/model/ListDefinitionImpl.java b/list/src/org/labkey/list/model/ListDefinitionImpl.java index b63b108c2e9..d70f9652601 100644 --- a/list/src/org/labkey/list/model/ListDefinitionImpl.java +++ b/list/src/org/labkey/list/model/ListDefinitionImpl.java @@ -54,6 +54,7 @@ import org.labkey.api.reader.DataLoader; import org.labkey.api.reader.MapLoader; import org.labkey.api.security.User; +import org.labkey.api.security.permissions.Permission; import org.labkey.api.util.ReentrantLockWithName; import org.labkey.api.util.URLHelper; import org.labkey.api.view.ActionURL; @@ -521,19 +522,38 @@ private ListItem getListItem(SimpleFilter filter, User user, Container c) return impl; } - public boolean hasListItemForEntityId(String entityId, User user) + public @Nullable Container getListItemContainerForDownload(String entityId, User user, Class permissionClass) { - return hasListItem(new SimpleFilter(FieldKey.fromParts("EntityId"), entityId), user, getContainer()); - } - - private boolean hasListItem(SimpleFilter filter, User user, Container c) - { - TableInfo tbl = getTable(user, c); + Container c = getContainer(); + SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("EntityId"), entityId); + // Use a relax CF to find the list items, permission will be validated later + ContainerFilter cf = ContainerFilter.Type.AllInProjectPlusShared.create(c, user); + TableInfo tbl = getTable(user, c, cf); if (null == tbl) - return false; + return null; + + Map row = null; + + try + { + row = new TableSelector(tbl, filter, null).getMap(); + } + catch (IllegalStateException e) + { + // More than one row matches the specified EntityId; log for diagnosis and return null as before + LOG.warn("Multiple list items match EntityId '{}' when resolving download container. List: '{}', Container: '{}'. Returning null.", + entityId, getName(), getContainer().getPath(), e); + } + + if (row == null) + return null; + + Container dataContainer = row.get("Container") != null ? ContainerManager.getForId(row.get("Container").toString()) : null; + if (dataContainer != null && dataContainer.hasPermission(user, permissionClass)) + return dataContainer; - return new TableSelector(tbl, filter, null).exists(); + return null; } @Override diff --git a/list/src/org/labkey/list/model/ListManagerSchema.java b/list/src/org/labkey/list/model/ListManagerSchema.java index ff16718f3d2..7b1854c022c 100644 --- a/list/src/org/labkey/list/model/ListManagerSchema.java +++ b/list/src/org/labkey/list/model/ListManagerSchema.java @@ -21,6 +21,8 @@ import org.labkey.api.data.ActionButton; import org.labkey.api.data.ButtonBar; import org.labkey.api.data.Container; +import org.labkey.api.data.DataRegion; +import org.labkey.api.data.MenuButton; import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DisplayColumn; @@ -38,6 +40,7 @@ import org.labkey.api.query.QueryView; import org.labkey.api.query.UserSchema; import org.labkey.api.security.User; +import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.util.ButtonBuilder; import org.labkey.api.util.LinkBuilder; import org.labkey.api.view.ActionURL; @@ -162,14 +165,37 @@ private ButtonBuilder.Button createImportListArchiveButton() @Override public ActionButton createDeleteButton() { + if (!getContainer().hasPermission(getUser(), DesignListPermission.class)) + return null; + + if (!getContainer().hasPermission(getUser(), AdminPermission.class)) + { + ActionURL urlDelete = new ActionURL(ListController.DeleteListDefinitionAction.class, getContainer()); + urlDelete.addReturnUrl(getReturnUrl()); + ActionButton btnDelete = new ActionButton(urlDelete, "Delete"); + btnDelete.setIconCls("trash"); + btnDelete.setActionType(ActionButton.Action.GET); + btnDelete.setRequiresSelection(true); + return btnDelete; + } + + MenuButton menuDelete = new MenuButton("Delete"); + menuDelete.setIconCls("trash"); + menuDelete.setDisplayPermission(DesignListPermission.class); + menuDelete.setRequiresSelection(true); + ActionURL urlDelete = new ActionURL(ListController.DeleteListDefinitionAction.class, getContainer()); urlDelete.addReturnUrl(getReturnUrl()); - ActionButton btnDelete = new ActionButton(urlDelete, "Delete"); - btnDelete.setIconCls("trash"); - btnDelete.setActionType(ActionButton.Action.GET); - btnDelete.setDisplayPermission(DesignListPermission.class); - btnDelete.setRequiresSelection(true); - return btnDelete; + menuDelete.addMenuItem("Delete List", "if (verifySelected(" + DataRegion.getJavaScriptObjectReference(getDataRegionName()) + ".form, \"" + + urlDelete.getLocalURIString() + "\", \"GET\", \"rows\")) " + DataRegion.getJavaScriptObjectReference(getDataRegionName()) + ".form.submit()"); + + + ActionURL urlTruncate = new ActionURL(ListController.TruncateListDataAction.class, getContainer()); + urlTruncate.addReturnUrl(getReturnUrl()); + menuDelete.addMenuItem("Delete All Data from List", "if (verifySelected(" + DataRegion.getJavaScriptObjectReference(getDataRegionName()) + ".form, \"" + + urlTruncate.getLocalURIString() + "\", \"GET\", \"rows\")) " + DataRegion.getJavaScriptObjectReference(getDataRegionName()) + ".form.submit()"); + + return menuDelete; } private ActionButton createExportArchiveButton() diff --git a/list/src/org/labkey/list/model/ListQueryUpdateService.java b/list/src/org/labkey/list/model/ListQueryUpdateService.java index 8110e0c2b13..936cf53af26 100644 --- a/list/src/org/labkey/list/model/ListQueryUpdateService.java +++ b/list/src/org/labkey/list/model/ListQueryUpdateService.java @@ -767,7 +767,7 @@ public void deleteRelatedListData(final User user, final Container container) ListManager.get().deleteIndexedList(_list); // Delete attachments and discussions associated with a list in batches of 1,000 - new TableSelector(getDbTable(), Collections.singleton("entityId")).forEachBatch(String.class, 1000, new ForEachBatchBlock<>() + new TableSelector(getDbTable(), Collections.singleton("entityId"), SimpleFilter.createContainerFilter(container), null).forEachBatch(String.class, 1000, new ForEachBatchBlock<>() { @Override public boolean accept(String entityId) diff --git a/list/src/org/labkey/list/view/ListQueryView.java b/list/src/org/labkey/list/view/ListQueryView.java index a8a79d81e3e..5e01aaa726d 100644 --- a/list/src/org/labkey/list/view/ListQueryView.java +++ b/list/src/org/labkey/list/view/ListQueryView.java @@ -78,8 +78,6 @@ protected void populateButtonBar(DataView view, ButtonBar bar) ActionButton btnUpload = new ActionButton("Design", designURL); bar.add(btnUpload); } - if (canDelete()) - bar.add(super.createDeleteAllRowsButton("list")); } public ListDefinition getList() diff --git a/list/src/org/labkey/list/view/truncateListData.jsp b/list/src/org/labkey/list/view/truncateListData.jsp new file mode 100644 index 00000000000..fae430c126f --- /dev/null +++ b/list/src/org/labkey/list/view/truncateListData.jsp @@ -0,0 +1,70 @@ +<% +/* + * Copyright (c) 2009-2016 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +%> +<%@ page import="org.labkey.api.data.Container" %> +<%@ page import="org.labkey.api.data.TableInfo" %> +<%@ page import="org.labkey.api.data.TableSelector" %> +<%@ page import="org.labkey.api.exp.list.ListDefinition" %> +<%@ page import="org.labkey.api.exp.list.ListService" %> +<%@ page import="org.labkey.list.controllers.ListController" %> +<%@ page import="java.util.ArrayList" %> +<%@ page import="java.util.HashMap" %> +<%@ page import="java.util.List" %> +<%@ page import="java.util.Map" %> +<%@ page import="java.util.Objects" %> +<%@ page import="org.labkey.list.model.ListQuerySchema" %> +<%@ page import="org.labkey.api.security.User" %> +<%@ page extends="org.labkey.api.jsp.FormPage" %> +<%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib"%> +<% + ListController.ListDeletionForm form = (ListController.ListDeletionForm)getModelBean(); + Map> definitions = new HashMap<>(); + Container currentContainer = form.getContainer(); + String currentPath = currentContainer.getPath(); + User user = form.getUser(); + ListQuerySchema listQuerySchema = new ListQuerySchema(user, currentContainer); + + form.getListContainerMap() + .stream() + .map(pair -> ListService.get().getList(pair.second, pair.first)) + .filter(Objects::nonNull) + .forEach(listDef -> { + var container = listDef.getContainer(); + if (!definitions.containsKey(container)) + definitions.put(container, new ArrayList<>()); + definitions.get(container).add(listDef); + }); +%> + +

Are you sure you want to delete all data in <%= h(currentPath) %> from the following Lists? This action cannot be undone and will result in an empty list.

+ +<% for (var entry : definitions.entrySet()) { %> +
+ List definitions defined in <%= h(entry.getKey().getPath()) %>: +
    + <% for (var listDef : entry.getValue()) { + TableInfo table = listQuerySchema.getTable(listDef.getName()); + long count = (table != null) ? new TableSelector(table).getRowCount() : 0; + %> +
  • + <%= simpleLink(listDef.getName(), listDef.urlFor(ListController.GridAction.class, currentContainer)) %> + — <%= count %> row<%= h(count != 1 ? "s" : "") %> in <%= h(currentPath) %> +
    • + <% } %> +
+
+<% } %> diff --git a/pipeline/gradle.properties b/pipeline/gradle.properties index 40a5496f023..7960fca9127 100644 --- a/pipeline/gradle.properties +++ b/pipeline/gradle.properties @@ -1,4 +1,4 @@ -activemqVersion=5.18.7 +activemqVersion=5.19.2 geronimoJ2eeConnector15SpecVersion=1.0.1 diff --git a/study/test/src/org/labkey/test/tests/study/TruncationTest.java b/study/test/src/org/labkey/test/tests/study/TruncationTest.java index 988f3be2b10..917b8e32e83 100644 --- a/study/test/src/org/labkey/test/tests/study/TruncationTest.java +++ b/study/test/src/org/labkey/test/tests/study/TruncationTest.java @@ -79,12 +79,17 @@ private void initTest() public void testTruncateList() { goToProjectHome(); - clickAndWait(Locator.linkWithText(LIST_NAME)); - click(Locator.linkContainingText("Delete All Rows")); - waitAndClick(Ext4Helper.Locators.ext4Button("Yes")); - waitForText("2 rows deleted"); - waitAndClickAndWait(Ext4Helper.Locators.ext4Button("OK")); - waitForText("No data to show."); + var listsPage = goToManageLists(); + var grid = listsPage.getGrid(); + grid.uncheckAllOnPage(); + grid.selectLists(List.of(LIST_NAME)); + grid.clickHeaderMenu("Delete", true, "Delete All Data from List"); + + // Verify confirmation page + assertTextPresent("Are you sure you want to delete all data"); + assertElementPresent(Locator.linkWithText(LIST_NAME)); + assertTextPresent("2 rows"); + clickButton("Confirm Delete All Data"); } @Test