From 772b926e3d137a8e70083df1f0c50867198064eb Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 14 Feb 2026 08:22:56 -0800 Subject: [PATCH 01/11] Lowercase linked schema GUIDs that are stored in the DataSource column --- query/src/org/labkey/query/QueryModule.java | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/query/src/org/labkey/query/QueryModule.java b/query/src/org/labkey/query/QueryModule.java index b9a63544ada..4a31f698e8e 100644 --- a/query/src/org/labkey/query/QueryModule.java +++ b/query/src/org/labkey/query/QueryModule.java @@ -23,14 +23,19 @@ import org.labkey.api.audit.DefaultAuditProvider; import org.labkey.api.cache.CacheManager; import org.labkey.api.data.Aggregate; +import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DataRegionSelection; import org.labkey.api.data.JdbcType; +import org.labkey.api.data.TableInfo; import org.labkey.api.data.views.DataViewService; import org.labkey.api.exp.property.PropertyService; import org.labkey.api.message.digest.DailyMessageDigest; import org.labkey.api.message.digest.ReportAndDatasetChangeDigestProvider; +import org.labkey.api.migration.DatabaseMigrationService; +import org.labkey.api.migration.GuidMapperColumn; +import org.labkey.api.migration.MigrationTableHandler; import org.labkey.api.module.AdminLinkManager; import org.labkey.api.module.DefaultModule; import org.labkey.api.module.Module; @@ -339,6 +344,22 @@ public void doStartup(ModuleContext moduleContext) Role trustedAnalystRole = RoleManager.getRole("org.labkey.api.security.roles.TrustedAnalystRole"); if (null != trustedAnalystRole) trustedAnalystRole.addPermission(EditQueriesPermission.class); + + DatabaseMigrationService.get().registerTableHandler(new MigrationTableHandler() + { + @Override + public TableInfo getTableInfo() + { + return QueryManager.get().getTableInfoExternalSchema(); + } + + @Override + public ColumnInfo handleColumn(ColumnInfo col) + { + // In the LinkedSchema case, the container GUID is stored in the "DataSource" column + return "DataSource".equals(col.getName()) ? new GuidMapperColumn(col) : col; + } + }); } @Override From f51f73b15b7670a29ede027a825a2b0c2f7589ec Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 14 Feb 2026 08:24:37 -0800 Subject: [PATCH 02/11] Stop stashing the shared container --- .../org/labkey/api/data/ContainerManager.java | 3 +- .../org/labkey/api/exp/OntologyManager.java | 36 +++++++++---------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/api/src/org/labkey/api/data/ContainerManager.java b/api/src/org/labkey/api/data/ContainerManager.java index 8ad9432e469..8e7f4765bb9 100644 --- a/api/src/org/labkey/api/data/ContainerManager.java +++ b/api/src/org/labkey/api/data/ContainerManager.java @@ -971,11 +971,12 @@ public static void uncache(Container c) } public static final String SHARED_CONTAINER_PATH = "/Shared"; + private static final Path SHARED_CONTAINER_PATH_OBJECT = Path.parse(SHARED_CONTAINER_PATH); @NotNull public static Container getSharedContainer() { - return ensureContainer(Path.parse(SHARED_CONTAINER_PATH), User.getAdminServiceUser()); + return ensureContainer(SHARED_CONTAINER_PATH_OBJECT, User.getAdminServiceUser()); } public static List getChildren(Container parent) diff --git a/api/src/org/labkey/api/exp/OntologyManager.java b/api/src/org/labkey/api/exp/OntologyManager.java index 6a09190c79d..9095da165d6 100644 --- a/api/src/org/labkey/api/exp/OntologyManager.java +++ b/api/src/org/labkey/api/exp/OntologyManager.java @@ -122,7 +122,7 @@ public PropertyDescriptor load(@NotNull Pair key, @Nullable Object proj = c; _log.debug("Loading a property descriptor for key " + key + " using project " + proj); String sql = " SELECT * FROM " + getTinfoPropertyDescriptor() + " WHERE PropertyURI = ? AND Project IN (?,?)"; - List pdArray = new SqlSelector(getExpSchema(), sql, propertyURI, proj, _sharedContainer.getId()).getArrayList(PropertyDescriptor.class); + List pdArray = new SqlSelector(getExpSchema(), sql, propertyURI, proj, ContainerManager.getSharedContainer().getId()).getArrayList(PropertyDescriptor.class); if (!pdArray.isEmpty()) { PropertyDescriptor pd = pdArray.get(0); @@ -132,7 +132,7 @@ public PropertyDescriptor load(@NotNull Pair key, @Nullable Object if (pdArray.size() > 1) { _log.debug("Multiple PropertyDescriptors found for " + propertyURI); - if (pd.getProject().equals(_sharedContainer)) + if (pd.getProject().equals(ContainerManager.getSharedContainer())) pd = pdArray.get(1); } _log.debug("Loaded property descriptor " + pd); @@ -177,7 +177,7 @@ public static DomainDescriptor fetchDomainDescriptorFromDB(String uriOrName, Con DomainDescriptor dd = null; if (!ddArray.isEmpty()) { - dd = ddArray.get(0); + dd = ddArray.getFirst(); // if someone has explicitly inserted a descriptor with the same URI as an existing one , // and one of the two is in the shared project, use the project-level descriptor. @@ -185,7 +185,7 @@ public static DomainDescriptor fetchDomainDescriptorFromDB(String uriOrName, Con { _log.debug("Multiple DomainDescriptors found for " + uriOrName); if (dd.getProject().equals(ContainerManager.getSharedContainer())) - dd = ddArray.get(0); + dd = ddArray.getFirst(); } } return dd; @@ -210,8 +210,8 @@ public List> load(@NotNull Pair key, @Nullab sql.addAll( typeURI, // protect against null project, just double-up shared project - c.isRoot() ? c.getId() : (c.getProject() == null ? _sharedContainer.getProject().getId() : c.getProject().getId()), - _sharedContainer.getProject().getId() + c.isRoot() ? c.getId() : (c.getProject() == null ? ContainerManager.getSharedContainer().getProject().getId() : c.getProject().getId()), + ContainerManager.getSharedContainer().getProject().getId() ); return new SqlSelector(getExpSchema(), sql).mapStream() @@ -231,8 +231,6 @@ public List> load(@NotNull Pair key, @Nullab return unmodifiableMap(dds); }); - private static final Container _sharedContainer = ContainerManager.getSharedContainer(); - public static final String MV_INDICATOR_SUFFIX = "mvindicator"; static public String PropertyOrderURI = "urn:exp.labkey.org/#PropertyOrder"; @@ -1579,7 +1577,7 @@ public static void moveContainer(@NotNull final Container c, @NotNull Container if (_log.isDebugEnabled()) { - try (ResultSet rs = new SqlSelector(getExpSchema(), sql, c, _sharedContainer, newProject).getResultSet()) + try (ResultSet rs = new SqlSelector(getExpSchema(), sql, c, ContainerManager.getSharedContainer(), newProject).getResultSet()) { ResultSetUtil.logData(rs, _log); } @@ -1590,7 +1588,7 @@ public static void moveContainer(@NotNull final Container c, @NotNull Container final StringBuilder sqlIn = new StringBuilder(); final StringBuilder sep = new StringBuilder(); - new SqlSelector(getExpSchema(), sql, c, _sharedContainer, newProject).forEach(rs -> { + new SqlSelector(getExpSchema(), sql, c, ContainerManager.getSharedContainer(), newProject).forEach(rs -> { String objURI = rs.getString(1); String propURI = rs.getString(2); Integer propId = rs.getInt(3); @@ -1701,7 +1699,7 @@ private static PropertyDescriptor ensurePropertyDescriptor(PropertyDescriptor pd if (null == pdIn.getContainer()) { assert false : "Container should be set on PropertyDescriptor"; - pdIn.setContainer(_sharedContainer); + pdIn.setContainer(ContainerManager.getSharedContainer()); } PropertyDescriptor pd = getPropertyDescriptor(pdIn.getPropertyURI(), pdIn.getContainer()); @@ -1806,7 +1804,7 @@ private static List comparePropertyDescriptors(PropertyDescriptor pdIn, List colDiffs = new ArrayList<>(); // if the returned pd is in a different project, it better be the shared project - if (!pd.getProject().equals(pdIn.getProject()) && !pd.getProject().equals(_sharedContainer)) + if (!pd.getProject().equals(pdIn.getProject()) && !pd.getProject().equals(ContainerManager.getSharedContainer())) colDiffs.add("Project"); // check the pd values that can't change @@ -1980,7 +1978,7 @@ public static PropertyDescriptor ensurePropertyDomain(PropertyDescriptor pd, Dom // Consider: We should check that the pd and dd have been persisted (aka have a non-zero id) if (!pd.getContainer().equals(dd.getContainer()) - && !pd.getProject().equals(_sharedContainer)) + && !pd.getProject().equals(ContainerManager.getSharedContainer())) throw new IllegalStateException("ensurePropertyDomain: property " + pd.getPropertyURI() + " not in same container as domain " + dd.getDomainURI()); SQLFragment sqlInsert = new SQLFragment("INSERT INTO " + getTinfoPropertyDomain() + " ( PropertyId, DomainId, Required, SortOrder ) " + @@ -2282,7 +2280,7 @@ public static PropertyDescriptor getPropertyDescriptor(String propertyURI, Conta if (null != pd) return pd; - key = getCacheKey(propertyURI, _sharedContainer); + key = getCacheKey(propertyURI, ContainerManager.getSharedContainer()); return PROP_DESCRIPTOR_CACHE.get(key); } @@ -2485,7 +2483,7 @@ public static DomainDescriptor getDomainDescriptor(String domainURI, Container c return dd; // Try in the /Shared container too - key = getCacheKey(domainURI, _sharedContainer); + key = getCacheKey(domainURI, ContainerManager.getSharedContainer()); return DOMAIN_DESCRIPTORS_BY_URI_CACHE.get(key); } @@ -2497,7 +2495,7 @@ private static DomainDescriptor getDomainDescriptorForUpdate(String domainURI, C DomainDescriptor dd = fetchDomainDescriptorFromDB(domainURI, c); if (dd == null) - dd = fetchDomainDescriptorFromDB(domainURI, _sharedContainer); + dd = fetchDomainDescriptorFromDB(domainURI, ContainerManager.getSharedContainer()); return dd; } @@ -2531,9 +2529,9 @@ public static Collection getDomainDescriptors(Container contai } } - if (_sharedContainer.hasPermission(user, ReadPermission.class)) + if (ContainerManager.getSharedContainer().hasPermission(user, ReadPermission.class)) { - for (Map.Entry entry : getCachedDomainDescriptors(_sharedContainer, user).entrySet()) + for (Map.Entry entry : getCachedDomainDescriptors(ContainerManager.getSharedContainer(), user).entrySet()) { dds.putIfAbsent(entry.getKey(), entry.getValue()); } @@ -2636,7 +2634,7 @@ public static PropertyDescriptor insertOrUpdatePropertyDescriptor(PropertyDescri DomainDescriptor dexist = ensureDomainDescriptor(dd); if (!dexist.getContainer().equals(pd.getContainer()) - && !pd.getProject().equals(_sharedContainer)) + && !pd.getProject().equals(ContainerManager.getSharedContainer())) { // domain is defined in a different container. //ToDO define property in the domains container? what security? From f1123f71109864f62c581a81af480f3f043aba51 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 14 Feb 2026 08:26:50 -0800 Subject: [PATCH 03/11] Don't delete issues and comments if no filter is in place --- .../issue/IssueMigrationSchemaHandler.java | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/issues/src/org/labkey/issue/IssueMigrationSchemaHandler.java b/issues/src/org/labkey/issue/IssueMigrationSchemaHandler.java index f0c3f77af81..3cafbeb18b1 100644 --- a/issues/src/org/labkey/issue/IssueMigrationSchemaHandler.java +++ b/issues/src/org/labkey/issue/IssueMigrationSchemaHandler.java @@ -32,6 +32,8 @@ public class IssueMigrationSchemaHandler extends DefaultMigrationSchemaHandler private final Set COPIED_ISSUE_IDS = new HashSet<>(); + private boolean _filtered = false; + public IssueMigrationSchemaHandler() { super(DbSchema.get(IssuesSchema.ISSUE_DEF_SCHEMA_NAME, DbSchemaType.Provisioned)); @@ -40,6 +42,9 @@ public IssueMigrationSchemaHandler() @Override public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilter notCopiedFilter) { + // Remember that issues tables are being filtered so afterSchema() can clean up associated tables (or not) + _filtered = true; + // Collect the issue IDs that were copied into the target table. We're assuming this set is much smaller than // the set of issues IDs that *weren't* copied. int startSize = COPIED_ISSUE_IDS.size(); @@ -59,23 +64,26 @@ public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilte @Override public void afterSchema(DatabaseMigrationConfiguration configuration, DbSchema sourceSchema, DbSchema targetSchema) { - LOG.info("{} were copied. Now deleting related issues, comments, and issues rows associated with all issues that were not copied.", StringUtilsLabKey.pluralize(COPIED_ISSUE_IDS.size(), "issue")); + if (_filtered) + { + LOG.info("{} were copied. Now deleting related issues, comments, and issues rows associated with all issues that were not copied.", StringUtilsLabKey.pluralize(COPIED_ISSUE_IDS.size(), "issue")); - // Delete all issues, comments, and related issues that were NOT copied - SimpleFilter deleteRelatedFilter = new SimpleFilter( - new InClause(FieldKey.fromParts("RelatedIssueId"), COPIED_ISSUE_IDS, false, true) // Negated - ); - int deletedRowCount = Table.delete(IssuesSchema.getInstance().getTableInfoRelatedIssues(), deleteRelatedFilter); - LOG.info(" Deleted {} from RelatedIssues (RelatedIssueId)", StringUtilsLabKey.pluralize(deletedRowCount, "row")); - SimpleFilter deleteFilter = new SimpleFilter( - new InClause(FieldKey.fromParts("IssueId"), COPIED_ISSUE_IDS, false, true) // Negated - ); - deletedRowCount = Table.delete(IssuesSchema.getInstance().getTableInfoRelatedIssues(), deleteFilter); - LOG.info(" Deleted {} from RelatedIssues (IssueId)", StringUtilsLabKey.pluralize(deletedRowCount, "row")); - deletedRowCount = Table.delete(IssuesSchema.getInstance().getTableInfoComments(), deleteFilter); - LOG.info(" Deleted {} from Comments", StringUtilsLabKey.pluralize(deletedRowCount, "row")); - deletedRowCount = Table.delete(IssuesSchema.getInstance().getTableInfoIssues(), deleteFilter); - LOG.info(" Deleted {} from Issues", StringUtilsLabKey.pluralize(deletedRowCount, "row")); + // Delete all issues, comments, and related issues that were NOT copied + SimpleFilter deleteRelatedFilter = new SimpleFilter( + new InClause(FieldKey.fromParts("RelatedIssueId"), COPIED_ISSUE_IDS, false, true) // Negated + ); + int deletedRowCount = Table.delete(IssuesSchema.getInstance().getTableInfoRelatedIssues(), deleteRelatedFilter); + LOG.info(" Deleted {} from RelatedIssues (RelatedIssueId)", StringUtilsLabKey.pluralize(deletedRowCount, "row")); + SimpleFilter deleteFilter = new SimpleFilter( + new InClause(FieldKey.fromParts("IssueId"), COPIED_ISSUE_IDS, false, true) // Negated + ); + deletedRowCount = Table.delete(IssuesSchema.getInstance().getTableInfoRelatedIssues(), deleteFilter); + LOG.info(" Deleted {} from RelatedIssues (IssueId)", StringUtilsLabKey.pluralize(deletedRowCount, "row")); + deletedRowCount = Table.delete(IssuesSchema.getInstance().getTableInfoComments(), deleteFilter); + LOG.info(" Deleted {} from Comments", StringUtilsLabKey.pluralize(deletedRowCount, "row")); + deletedRowCount = Table.delete(IssuesSchema.getInstance().getTableInfoIssues(), deleteFilter); + LOG.info(" Deleted {} from Issues", StringUtilsLabKey.pluralize(deletedRowCount, "row")); + } } @Override From 089d45cca001769b47641b944da3686f22d04881 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 14 Feb 2026 08:28:13 -0800 Subject: [PATCH 04/11] Correct scope used in DbSchemaType.Migration --- api/src/org/labkey/api/data/DbSchemaType.java | 2 +- api/src/org/labkey/api/data/DbScope.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/data/DbSchemaType.java b/api/src/org/labkey/api/data/DbSchemaType.java index cd54c930c5d..0e2e2b7e527 100644 --- a/api/src/org/labkey/api/data/DbSchemaType.java +++ b/api/src/org/labkey/api/data/DbSchemaType.java @@ -73,7 +73,7 @@ public Module getModule(DbScope scope, String schemaName) @Override DbSchema createDbSchema(DbScope scope, String metaDataName, Module module) throws SQLException { - Map metaDataTableNames = DbSchema.loadTableMetaData(DbScope.getLabKeyScope(), metaDataName); + Map metaDataTableNames = DbSchema.loadTableMetaData(scope, metaDataName); return new MigrationDbSchema(metaDataName, this, scope, metaDataTableNames, module); } diff --git a/api/src/org/labkey/api/data/DbScope.java b/api/src/org/labkey/api/data/DbScope.java index 6e9663bf496..022e7ef2870 100644 --- a/api/src/org/labkey/api/data/DbScope.java +++ b/api/src/org/labkey/api/data/DbScope.java @@ -1542,7 +1542,7 @@ public static class SchemaNameTestCase extends Assert public void testSchemaNames() { ModuleLoader.getInstance().getModules() - .forEach(m -> assertEquals(getSchemaNames(m, true), getSchemaNames(m, false))); + .forEach(m -> assertEquals(getSchemaNames(m, true), getSchemaNames(m, false))); } } From 3ca74629f4a2cc8234dd94af6334aa1026117511 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 14 Feb 2026 08:29:24 -0800 Subject: [PATCH 05/11] Hoist GuidMapperColumn into API --- .../announcements/AnnouncementModule.java | 30 ++----------------- .../DefaultMigrationSchemaHandler.java | 1 + .../api/migration/GuidMapperColumn.java | 30 +++++++++++++++++++ .../DataClassMigrationSchemaHandler.java | 2 +- 4 files changed, 34 insertions(+), 29 deletions(-) create mode 100644 api/src/org/labkey/api/migration/GuidMapperColumn.java diff --git a/announcements/src/org/labkey/announcements/AnnouncementModule.java b/announcements/src/org/labkey/announcements/AnnouncementModule.java index e43846c1198..72b0d1b82b5 100644 --- a/announcements/src/org/labkey/announcements/AnnouncementModule.java +++ b/announcements/src/org/labkey/announcements/AnnouncementModule.java @@ -30,23 +30,22 @@ import org.labkey.api.admin.FolderSerializationRegistry; import org.labkey.api.announcements.CommSchema; import org.labkey.api.announcements.api.AnnouncementService; -import org.labkey.api.attachments.AttachmentService; import org.labkey.api.attachments.AttachmentParentType; +import org.labkey.api.attachments.AttachmentService; import org.labkey.api.audit.AuditLogService; import org.labkey.api.audit.provider.MessageAuditProvider; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DbSchema; -import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SqlExecutor; import org.labkey.api.data.TableInfo; -import org.labkey.api.data.WrappedColumn; import org.labkey.api.message.digest.DailyMessageDigest; import org.labkey.api.message.settings.MessageConfigService; import org.labkey.api.migration.DatabaseMigrationConfiguration; import org.labkey.api.migration.DatabaseMigrationService; import org.labkey.api.migration.DefaultMigrationSchemaHandler; +import org.labkey.api.migration.GuidMapperColumn; import org.labkey.api.migration.MigrationTableHandler; import org.labkey.api.module.DefaultModule; import org.labkey.api.module.ModuleContext; @@ -56,7 +55,6 @@ import org.labkey.api.security.roles.EditorRole; import org.labkey.api.security.roles.Role; import org.labkey.api.security.roles.RoleManager; -import org.labkey.api.util.GUID; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.emailTemplate.EmailTemplateService; import org.labkey.api.view.AlwaysAvailableWebPartFactory; @@ -227,30 +225,6 @@ public ColumnInfo handleColumn(ColumnInfo col) { return "DiscussionSrcIdentifier".equals(col.getName()) ? new GuidMapperColumn(col) : col; } - - // In this column, map any value that exactly matches a GUID to lowercase - private static final class GuidMapperColumn extends WrappedColumn - { - public GuidMapperColumn(ColumnInfo col) - { - super(col, col.getName()); - } - - @Override - public SQLFragment getValueSql(String tableAlias) - { - SQLFragment columnAlias = super.getValueSql(tableAlias); - //noinspection StringConcatenationInsideStringBufferAppend - SQLFragment flips out about unmatched quotes, so we're forced to use string concatenation - return new SQLFragment("CASE WHEN ") - .append(columnAlias) - .append(" LIKE '" + GUID.SQL_LIKE_GUID_PATTERN + "'") - .append(" THEN LOWER(") - .append(columnAlias) - .append(") ELSE ") - .append(columnAlias) - .append(" END"); - } - } }); } diff --git a/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java b/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java index e78365dec28..8c244813fcb 100644 --- a/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java +++ b/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java @@ -232,6 +232,7 @@ protected String rowsNotCopied(int count) return " " + StringUtilsLabKey.pluralize(count, "row") + " not copied"; } + // afterTable() is called only if the table was filtered by a configuration filter @Override public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilter notCopiedFilter) { diff --git a/api/src/org/labkey/api/migration/GuidMapperColumn.java b/api/src/org/labkey/api/migration/GuidMapperColumn.java new file mode 100644 index 00000000000..64a49435af3 --- /dev/null +++ b/api/src/org/labkey/api/migration/GuidMapperColumn.java @@ -0,0 +1,30 @@ +package org.labkey.api.migration; + +import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.WrappedColumn; +import org.labkey.api.util.GUID; + +// In this column, map any value that exactly matches a GUID to lowercase +public final class GuidMapperColumn extends WrappedColumn +{ + public GuidMapperColumn(ColumnInfo col) + { + super(col, col.getName()); + } + + @Override + public SQLFragment getValueSql(String tableAlias) + { + SQLFragment columnAlias = super.getValueSql(tableAlias); + //noinspection StringConcatenationInsideStringBufferAppend - SQLFragment flips out about unmatched quotes, so we're forced to use string concatenation + return new SQLFragment("CASE WHEN ") + .append(columnAlias) + .append(" LIKE '" + GUID.SQL_LIKE_GUID_PATTERN + "'") + .append(" THEN LOWER(") + .append(columnAlias) + .append(") ELSE ") + .append(columnAlias) + .append(" END"); + } +} diff --git a/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java index 091aceca4f4..0069378ea20 100644 --- a/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java @@ -174,7 +174,7 @@ public void deleteDataRows(Collection objectIds) @Override public void afterSchema(DatabaseMigrationConfiguration configuration, DbSchema sourceSchema, DbSchema targetSchema) { - // Experiment shouldn't mess with Biologics tables, but it gets the job done + // Experiment shouldn't really mess with Biologics tables, but it gets the job done DbScope sourceScope = configuration.getSourceScope(); DbScope targetScope = configuration.getTargetScope(); From 458c710b3c171851a2cf0a5fa61272f19f153883 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 14 Feb 2026 10:49:05 -0800 Subject: [PATCH 06/11] Normalize tableName2 to fix FK ordering & highlighting. Recognize constraint names. --- .../core/admin/sql/ScriptReorderer.java | 72 +++++++++++-------- 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/core/src/org/labkey/core/admin/sql/ScriptReorderer.java b/core/src/org/labkey/core/admin/sql/ScriptReorderer.java index c2f2b963ea3..7dbef273ae3 100644 --- a/core/src/org/labkey/core/admin/sql/ScriptReorderer.java +++ b/core/src/org/labkey/core/admin/sql/ScriptReorderer.java @@ -18,6 +18,7 @@ import org.apache.commons.lang3.Strings; import org.jetbrains.annotations.Nullable; +import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.DbSchema; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.StringUtilsLabKey; @@ -36,6 +37,7 @@ public class ScriptReorderer private final List>> _statementLists = new LinkedList<>(); private final List _endingStatements = new LinkedList<>(); + private final Map _constraintTables = CaseInsensitiveHashMap.of(); // Track tables associated with constraints private Map> _currentStatements; @@ -61,7 +63,7 @@ public class ScriptReorderer TABLE_NAME_REGEX = "(?" + SCHEMA_NAME_REGEX + "((#?\\w+)|(\\[#?\\w+\\])))"; // # allows for temp table names, optional [] around table name TABLE_NAME_NO_UNDERSCORE_REGEX = null; STATEMENT_ENDING_REGEX = "((; GO\\s*$)|(;\\s*$)|( GO\\s*$))\\s*"; // Semicolon, GO, or both - CONSTRAINT_NAME_REGEX = "((\\w+)|(\\[\\w+\\]))"; // optional [] around name + CONSTRAINT_NAME_REGEX = "(?((\\w+)|(\\[\\w+\\])))"; // optional [] around name } else { @@ -69,7 +71,7 @@ public class ScriptReorderer TABLE_NAME_REGEX = "(?
" + SCHEMA_NAME_REGEX + "(\\w+))"; TABLE_NAME_NO_UNDERSCORE_REGEX = "(?
" + SCHEMA_NAME_REGEX + "([[a-zA-Z0-9]]+))"; STATEMENT_ENDING_REGEX = ";(\\s*?)((--)[^\\n]*)?$(\\s*)"; - CONSTRAINT_NAME_REGEX = "(\\w+)"; + CONSTRAINT_NAME_REGEX = "(?(\\w+))"; } TABLE_NAME2_REGEX = TABLE_NAME_REGEX.replace("table", "table2"); @@ -116,6 +118,7 @@ public String getReorderedScript(boolean isHtml) patterns.add(new SqlPattern("DROP INDEX (IF EXISTS )?\\w+ ON " + TABLE_NAME_REGEX + STATEMENT_ENDING_REGEX, Type.Table, Operation.Other)); patterns.add(new SqlPattern("(CREATE|ALTER) PROCEDURE .+?" + STATEMENT_ENDING_REGEX, Type.NonTable, Operation.Other)); + patterns.add(new SqlPattern("ALTER TABLE " + TABLE_NAME_REGEX + " CHECK CONSTRAINT " + CONSTRAINT_NAME_REGEX + STATEMENT_ENDING_REGEX, Type.Table, Operation.Other)); } else { @@ -204,6 +207,7 @@ public String getReorderedScript(boolean isHtml) if (m.pattern().pattern().contains("(?")) { tableName2 = m.group("table2"); + assert tableName2 != null; } if (pattern.getOperation() == Operation.RenameTable) @@ -220,7 +224,19 @@ public String getReorderedScript(boolean isHtml) newStatementList(); } - addStatement(tableName, tableName2, comments + m.group()); + String tableKey = addStatement(tableName, tableName2, comments + m.group()); + + if (m.pattern().pattern().contains("(?")) + { + String constraintName = m.group("constraint"); + assert constraintName != null; + String constraintKey = normalizeName(constraintName); + if (!_constraintTables.containsKey(constraintKey)) + { + _constraintTables.put(constraintKey, tableKey); + } + } + _contents = _contents.substring(m.end()); recognized = true; break; @@ -318,18 +334,32 @@ private Pattern compile(String regEx) return Pattern.compile(regEx.replaceAll(" ", "\\\\s+"), Pattern.CASE_INSENSITIVE + Pattern.DOTALL + Pattern.MULTILINE); } - private void addStatement(String tableName, @Nullable String tableName2, String statement) + // Return table key that's associated with this statement + private String addStatement(String tableName, @Nullable String tableName2, String statement) { + // Remove brackets for map key + String key = normalizeName(tableName); + String key2 = normalizeName(tableName2); + // If there's a second table in the statement that's referenced later in the script then associate the statement // with the second table. For example, an FK definition will end up after BOTH tables have been created. - if (null != tableName2 && index(tableName2) > index(tableName)) + if (null != key2 && index(key2) > index(key)) + { tableName = tableName2; - - String key = tableName.replace("[", "").replace("]", "").toLowerCase(); + key = key2; + } Collection tableStatements = _currentStatements.computeIfAbsent(key, k -> new LinkedList<>()); tableStatements.add(new Statement(tableName, statement)); + + return key; + } + + // Remove brackets and lower case + private @Nullable String normalizeName(@Nullable String name) + { + return name != null ? name.replace("[", "").replace("]", "").toLowerCase() : null; } private int index(String tableName) @@ -369,14 +399,14 @@ private void appendStatement(StringBuilder sb, Statement statement, boolean html } else { - sb.append(statement.getSql()); + sb.append(statement.sql()); } } private void appendStatement(StringBuilder sb, Statement statement) { - String sql = PageFlowUtil.filter(statement.getSql(), true); - String tableName = statement.getTableName(); + String sql = PageFlowUtil.filter(statement.sql(), true); + String tableName = statement.tableName(); // If we have a table name then try to highlight the first occurrence in statement if (null != tableName) @@ -441,25 +471,5 @@ public Matcher getMatcher(CharSequence input) } // Saving the original table name helps with highlighting, especially in the case of a table rename - private static class Statement - { - private final @Nullable String _tableName; - private final String _sql; - - private Statement(@Nullable String tableName, String sql) - { - _tableName = tableName; - _sql = sql; - } - - public @Nullable String getTableName() - { - return _tableName; - } - - public String getSql() - { - return _sql; - } - } + private record Statement(@Nullable String tableName, String sql) {} } From d72f6cc7feea1756c26aa2df4f68267601e06ff4 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 14 Feb 2026 11:22:21 -0800 Subject: [PATCH 07/11] Find a constraint's owning table and use it to determine statement ordering --- .../core/admin/sql/ScriptReorderer.java | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/core/src/org/labkey/core/admin/sql/ScriptReorderer.java b/core/src/org/labkey/core/admin/sql/ScriptReorderer.java index 7dbef273ae3..b72cf64d750 100644 --- a/core/src/org/labkey/core/admin/sql/ScriptReorderer.java +++ b/core/src/org/labkey/core/admin/sql/ScriptReorderer.java @@ -203,6 +203,7 @@ public String getReorderedScript(boolean isHtml) } String tableName2 = null; + String constraintKey = null; if (m.pattern().pattern().contains("(?")) { @@ -210,6 +211,22 @@ public String getReorderedScript(boolean isHtml) assert tableName2 != null; } + if (m.pattern().pattern().contains("(?")) + { + // Stash a normalized version of the constraint name so we can save it to the map associated + // with its table (below). Also, if a second table is not specified and we can determine the + // constraint's table from the map, then set it as tableName2. This ensures the statement is + // output after its CREATE statement. For example, a SQL Server statement like ALTER TABLE + // MyTable CHECK CONSTRAINT MyConstraint may need to be output after the second table from the + // original CREATE statement. + String constraintName = m.group("constraint"); + assert constraintName != null; + constraintKey = normalizeName(constraintName); + + if (tableName2 == null) + tableName2 = _constraintTables.get(constraintKey); // Could be null + } + if (pattern.getOperation() == Operation.RenameTable) { assert null != tableName2; @@ -226,15 +243,9 @@ public String getReorderedScript(boolean isHtml) String tableKey = addStatement(tableName, tableName2, comments + m.group()); - if (m.pattern().pattern().contains("(?")) + if (constraintKey != null && !_constraintTables.containsKey(constraintKey)) { - String constraintName = m.group("constraint"); - assert constraintName != null; - String constraintKey = normalizeName(constraintName); - if (!_constraintTables.containsKey(constraintKey)) - { - _constraintTables.put(constraintKey, tableKey); - } + _constraintTables.put(constraintKey, tableKey); } _contents = _contents.substring(m.end()); From b3ee7181239b0a489922f90a855e6ae8c9c6507f Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 14 Feb 2026 20:04:25 -0800 Subject: [PATCH 08/11] Enforce that getWebPartFactories() is called after upgrade is complete --- api/src/org/labkey/api/module/DefaultModule.java | 11 ++++++----- .../api/view/SimpleWebPartFactoryCacheHandler.java | 3 --- api/src/org/labkey/api/view/WebPartCache.java | 1 - 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/api/src/org/labkey/api/module/DefaultModule.java b/api/src/org/labkey/api/module/DefaultModule.java index 2e9c6a7956d..667e34f91a8 100644 --- a/api/src/org/labkey/api/module/DefaultModule.java +++ b/api/src/org/labkey/api/module/DefaultModule.java @@ -21,7 +21,6 @@ import jakarta.servlet.http.HttpServletResponse; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; -import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -65,6 +64,7 @@ import org.labkey.api.util.Path; import org.labkey.api.util.ResponseHelper; import org.labkey.api.util.URLHelper; +import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.ActionURL; import org.labkey.api.view.HttpView; import org.labkey.api.view.NotFoundException; @@ -107,7 +107,7 @@ public abstract class DefaultModule implements Module, ApplicationContextAware { public static final String CORE_MODULE_NAME = "Core"; - private static final Logger _log = LogManager.getLogger(DefaultModule.class); + private static final Logger _log = LogHelper.getLogger(DefaultModule.class, "Module issues"); private static final Set, String>> INSTANTIATED_MODULES = new HashSet<>(); static final ModuleResourceCache MODULE_XML_CACHE = ModuleResourceCaches.create("module.xml files", new ModuleXmlCacheHandler(), ResourceRootProvider.getStandard(new Path())); @@ -341,9 +341,6 @@ public void afterUpdate(ModuleContext moduleContext) { } - // TODO: Move getWebPartFactories() and _webPartFactories into Portal... shouldn't be the module's responsibility - // This should also allow moving SimpleWebPartFactoryCache and dependencies into Internal - private final Object FACTORY_LOCK = new Object(); @Override @@ -353,6 +350,10 @@ public void afterUpdate(ModuleContext moduleContext) { if (null == _webPartFactories) { + // We promise that this method is called after upgrade is complete. Enforce that. + if (ModuleLoader.getInstance().isUpgradeRequired()) + throw new IllegalStateException("getWebPartFactories() is being called before upgrade is complete"); + Collection wpf = new ArrayList<>(); // Get all the Java webpart factories diff --git a/api/src/org/labkey/api/view/SimpleWebPartFactoryCacheHandler.java b/api/src/org/labkey/api/view/SimpleWebPartFactoryCacheHandler.java index d0498ce727e..8fc3ea6bd59 100644 --- a/api/src/org/labkey/api/view/SimpleWebPartFactoryCacheHandler.java +++ b/api/src/org/labkey/api/view/SimpleWebPartFactoryCacheHandler.java @@ -29,9 +29,6 @@ /** * Creates and caches the file-based web parts defined by modules. File changes result in dynamic reloading and re-initialization of webpart-related maps. - * User: adam - * Date: 12/29/13 - * Time: 12:38 PM */ public class SimpleWebPartFactoryCacheHandler implements ModuleResourceCacheHandler> { diff --git a/api/src/org/labkey/api/view/WebPartCache.java b/api/src/org/labkey/api/view/WebPartCache.java index daad39b90c0..482e23b1468 100644 --- a/api/src/org/labkey/api/view/WebPartCache.java +++ b/api/src/org/labkey/api/view/WebPartCache.java @@ -41,7 +41,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.Map; /** From 0ae69bb516ac8bd2c457eb0326437dd961248183 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 16 Feb 2026 12:33:02 -0800 Subject: [PATCH 09/11] Remove check by which we can't abide --- api/src/org/labkey/api/module/DefaultModule.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/api/src/org/labkey/api/module/DefaultModule.java b/api/src/org/labkey/api/module/DefaultModule.java index 667e34f91a8..fac4f7c7294 100644 --- a/api/src/org/labkey/api/module/DefaultModule.java +++ b/api/src/org/labkey/api/module/DefaultModule.java @@ -341,6 +341,9 @@ public void afterUpdate(ModuleContext moduleContext) { } + // TODO: Move getWebPartFactories() and _webPartFactories into Portal... shouldn't be the module's responsibility + // This should also allow moving SimpleWebPartFactoryCache and dependencies into Internal + private final Object FACTORY_LOCK = new Object(); @Override @@ -350,10 +353,6 @@ public void afterUpdate(ModuleContext moduleContext) { if (null == _webPartFactories) { - // We promise that this method is called after upgrade is complete. Enforce that. - if (ModuleLoader.getInstance().isUpgradeRequired()) - throw new IllegalStateException("getWebPartFactories() is being called before upgrade is complete"); - Collection wpf = new ArrayList<>(); // Get all the Java webpart factories From f548b09b4546311d79eca2e39a266f572c9032ea Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 16 Feb 2026 20:53:38 -0800 Subject: [PATCH 10/11] Don't bother constructing and registering migration handlers unless they're needed --- .../labkey/announcements/AnnouncementModule.java | 9 ++++++--- api/src/org/labkey/api/module/Module.java | 7 +++++++ assay/src/org/labkey/assay/AssayModule.java | 10 +++++++--- .../labkey/core/CoreMigrationSchemaHandler.java | 14 +++++++------- core/src/org/labkey/core/CoreModule.java | 8 +++++++- .../org/labkey/experiment/ExperimentModule.java | 16 ++++++++++------ issues/src/org/labkey/issue/IssuesModule.java | 6 +++++- list/src/org/labkey/list/ListModule.java | 6 +++++- query/src/org/labkey/query/QueryModule.java | 6 +++++- search/src/org/labkey/search/SearchModule.java | 6 +++++- study/src/org/labkey/study/StudyModule.java | 10 +++++++--- 11 files changed, 71 insertions(+), 27 deletions(-) diff --git a/announcements/src/org/labkey/announcements/AnnouncementModule.java b/announcements/src/org/labkey/announcements/AnnouncementModule.java index 72b0d1b82b5..388d5317bb6 100644 --- a/announcements/src/org/labkey/announcements/AnnouncementModule.java +++ b/announcements/src/org/labkey/announcements/AnnouncementModule.java @@ -175,9 +175,13 @@ public void doStartup(ModuleContext moduleContext) { fsr.addFactories(new NotificationSettingsWriterFactory(), new NotificationSettingsImporterFactory()); } + } + @Override + public void registerMigrationHandlers(@NotNull DatabaseMigrationService service) + { // AnnouncementModule owns the schema, so it registers the schema handler... even though it's mostly about wiki - DatabaseMigrationService.get().registerSchemaHandler(new DefaultMigrationSchemaHandler(CommSchema.getInstance().getSchema()) + service.registerSchemaHandler(new DefaultMigrationSchemaHandler(CommSchema.getInstance().getSchema()) { @Override public void beforeSchema() @@ -212,7 +216,7 @@ public void afterSchema(DatabaseMigrationConfiguration configuration, DbSchema s } }); - DatabaseMigrationService.get().registerTableHandler(new MigrationTableHandler() + service.registerTableHandler(new MigrationTableHandler() { @Override public TableInfo getTableInfo() @@ -228,7 +232,6 @@ public ColumnInfo handleColumn(ColumnInfo col) }); } - @Override public void startBackgroundThreads() { diff --git a/api/src/org/labkey/api/module/Module.java b/api/src/org/labkey/api/module/Module.java index 51221c02a6a..5e72b9e455c 100644 --- a/api/src/org/labkey/api/module/Module.java +++ b/api/src/org/labkey/api/module/Module.java @@ -30,6 +30,7 @@ import org.labkey.api.data.SchemaTableInfoFactory; import org.labkey.api.data.UpgradeCode; import org.labkey.api.data.dialect.SqlDialect; +import org.labkey.api.migration.DatabaseMigrationService; import org.labkey.api.module.DefaultModule.UpgradeMethod; import org.labkey.api.query.OlapSchemaInfo; import org.labkey.api.resource.Resource; @@ -439,4 +440,10 @@ default String getBuildTime() */ void lock(); + /** + * Register MigrationSchemaHandlers, MigrationTableHandlers, and MigrationFilters + */ + default void registerMigrationHandlers(@NotNull DatabaseMigrationService service) + { + } } diff --git a/assay/src/org/labkey/assay/AssayModule.java b/assay/src/org/labkey/assay/AssayModule.java index 647d7546c55..57b2d22fc0e 100644 --- a/assay/src/org/labkey/assay/AssayModule.java +++ b/assay/src/org/labkey/assay/AssayModule.java @@ -293,8 +293,12 @@ public void moduleStartupComplete(ServletContext servletContext) { svc.registerUsageMetrics(getName(), new PlateMetricsProvider()); } + } - DatabaseMigrationService.get().registerSchemaHandler(new DefaultMigrationSchemaHandler(AssayDbSchema.getInstance().getSchema()) + @Override + public void registerMigrationHandlers(@NotNull DatabaseMigrationService service) + { + service.registerSchemaHandler(new DefaultMigrationSchemaHandler(AssayDbSchema.getInstance().getSchema()) { @Override public @Nullable FieldKey getContainerFieldKey(TableInfo sourceTable) @@ -311,7 +315,7 @@ public FilterClause getContainerClause(TableInfo sourceTable, Set containe }); // Tables in the "assaywell" provisioned schema join to assay.Well to find their container - DatabaseMigrationService.get().registerSchemaHandler(new DefaultMigrationSchemaHandler(PlateMetadataDomainKind.getSchema()) + service.registerSchemaHandler(new DefaultMigrationSchemaHandler(PlateMetadataDomainKind.getSchema()) { @Override public FilterClause getContainerClause(TableInfo sourceTable, Set containers) @@ -324,7 +328,7 @@ public FilterClause getContainerClause(TableInfo sourceTable, Set containe } }); - DatabaseMigrationService.get().registerSchemaHandler(new AssayResultMigrationSchemaHandler()); + service.registerSchemaHandler(new AssayResultMigrationSchemaHandler()); } @Override diff --git a/core/src/org/labkey/core/CoreMigrationSchemaHandler.java b/core/src/org/labkey/core/CoreMigrationSchemaHandler.java index 35a55e10684..d5e6f19108f 100644 --- a/core/src/org/labkey/core/CoreMigrationSchemaHandler.java +++ b/core/src/org/labkey/core/CoreMigrationSchemaHandler.java @@ -45,12 +45,12 @@ class CoreMigrationSchemaHandler extends DefaultMigrationSchemaHandler implements MigrationFilter { - static void register() + static void register(@NotNull DatabaseMigrationService service) { CoreMigrationSchemaHandler schemaHandler = new CoreMigrationSchemaHandler(); - DatabaseMigrationService.get().registerSchemaHandler(schemaHandler); - DatabaseMigrationService.get().registerMigrationFilter(schemaHandler); - DatabaseMigrationService.get().registerTableHandler(new MigrationTableHandler() + service.registerSchemaHandler(schemaHandler); + service.registerMigrationFilter(schemaHandler); + service.registerTableHandler(new MigrationTableHandler() { @Override public TableInfo getTableInfo() @@ -65,7 +65,7 @@ public ColumnInfo handleColumn(ColumnInfo col) } }); - DatabaseMigrationService.get().registerSchemaHandler(new DefaultMigrationSchemaHandler(PropertySchema.getInstance().getSchema()){ + service.registerSchemaHandler(new DefaultMigrationSchemaHandler(PropertySchema.getInstance().getSchema()){ @Override public @Nullable FieldKey getContainerFieldKey(TableInfo sourceTable) { @@ -73,7 +73,7 @@ public ColumnInfo handleColumn(ColumnInfo col) } }); - DatabaseMigrationService.get().registerSchemaHandler(new DefaultMigrationSchemaHandler(TestSchema.getInstance().getSchema()){ + service.registerSchemaHandler(new DefaultMigrationSchemaHandler(TestSchema.getInstance().getSchema()){ @Override public List getTablesToCopy() { @@ -83,7 +83,7 @@ public List getTablesToCopy() if (ModuleLoader.getInstance().getModule(DbScope.getLabKeyScope(), "vehicle") != null) { - DatabaseMigrationService.get().registerSchemaHandler(new DefaultMigrationSchemaHandler(DbSchema.get("vehicle", DbSchemaType.Module)) + service.registerSchemaHandler(new DefaultMigrationSchemaHandler(DbSchema.get("vehicle", DbSchemaType.Module)) { @Override public List getTablesToCopy() diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index 68f872192cf..80137c134f5 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -89,6 +89,7 @@ import org.labkey.api.files.FileContentService; import org.labkey.api.markdown.MarkdownService; import org.labkey.api.message.settings.MessageConfigService; +import org.labkey.api.migration.DatabaseMigrationService; import org.labkey.api.module.FolderType; import org.labkey.api.module.FolderTypeManager; import org.labkey.api.module.Module; @@ -1287,7 +1288,6 @@ public void moduleStartupComplete(ServletContext servletContext) ContainerManager.addContainerListener(new EmailPreferenceContainerListener()); UserManager.addUserListener(new EmailPreferenceUserListener()); - CoreMigrationSchemaHandler.register(); Encryption.checkMigration(); McpServiceImpl.get().startMpcServer(); @@ -1310,6 +1310,12 @@ private void checkForMissingDbViews() }); } + @Override + public void registerMigrationHandlers(@NotNull DatabaseMigrationService service) + { + CoreMigrationSchemaHandler.register(service); + } + @Override public void registerServlets(ServletContext servletCtx) { diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 4b588980747..b08c142a599 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -906,10 +906,14 @@ SELECT COUNT(DISTINCT DD.DomainURI) FROM return results; }); } + } + @Override + public void registerMigrationHandlers(@NotNull DatabaseMigrationService service) + { ExperimentMigrationSchemaHandler handler = new ExperimentMigrationSchemaHandler(); - DatabaseMigrationService.get().registerSchemaHandler(handler); - DatabaseMigrationService.get().registerTableHandler(new MigrationTableHandler() + service.registerSchemaHandler(handler); + service.registerTableHandler(new MigrationTableHandler() { @Override public TableInfo getTableInfo() @@ -926,7 +930,7 @@ public void adjustFilter(TableInfo sourceTable, SimpleFilter filter, Set c filter.addClause(includedClause); } }); - DatabaseMigrationService.get().registerTableHandler(new MigrationTableHandler() + service.registerTableHandler(new MigrationTableHandler() { @Override public TableInfo getTableInfo() @@ -943,7 +947,7 @@ public void adjustFilter(TableInfo sourceTable, SimpleFilter filter, Set c filter.addClause(includedClause); } }); - DatabaseMigrationService.get().registerTableHandler(new MigrationTableHandler() + service.registerTableHandler(new MigrationTableHandler() { @Override public TableInfo getTableInfo() @@ -960,9 +964,9 @@ public void adjustFilter(TableInfo sourceTable, SimpleFilter filter, Set c filter.addClause(includedClause); } }); - DatabaseMigrationService.get().registerSchemaHandler(new SampleTypeMigrationSchemaHandler()); + service.registerSchemaHandler(new SampleTypeMigrationSchemaHandler()); DataClassMigrationSchemaHandler dcHandler = new DataClassMigrationSchemaHandler(); - DatabaseMigrationService.get().registerSchemaHandler(dcHandler); + service.registerSchemaHandler(dcHandler); ExperimentDeleteService.setInstance(dcHandler); } diff --git a/issues/src/org/labkey/issue/IssuesModule.java b/issues/src/org/labkey/issue/IssuesModule.java index 22940c93327..50015f781de 100644 --- a/issues/src/org/labkey/issue/IssuesModule.java +++ b/issues/src/org/labkey/issue/IssuesModule.java @@ -162,8 +162,12 @@ public void doStartup(ModuleContext moduleContext) return metric; }); } + } - DatabaseMigrationService.get().registerSchemaHandler(new IssueMigrationSchemaHandler()); + @Override + public void registerMigrationHandlers(@NotNull DatabaseMigrationService service) + { + service.registerSchemaHandler(new IssueMigrationSchemaHandler()); } @NotNull diff --git a/list/src/org/labkey/list/ListModule.java b/list/src/org/labkey/list/ListModule.java index 9fde0beea55..5ee59fbf316 100644 --- a/list/src/org/labkey/list/ListModule.java +++ b/list/src/org/labkey/list/ListModule.java @@ -163,8 +163,12 @@ public void startupAfterSpringConfig(ModuleContext moduleContext) return metric; }); } + } - DatabaseMigrationService.get().registerSchemaHandler(new DefaultMigrationSchemaHandler(ListSchema.getInstance().getSchema()){ + @Override + public void registerMigrationHandlers(@NotNull DatabaseMigrationService service) + { + service.registerSchemaHandler(new DefaultMigrationSchemaHandler(ListSchema.getInstance().getSchema()){ @Override public @NotNull Collection getAttachmentTypes() { diff --git a/query/src/org/labkey/query/QueryModule.java b/query/src/org/labkey/query/QueryModule.java index 4a31f698e8e..4a8da53b3fb 100644 --- a/query/src/org/labkey/query/QueryModule.java +++ b/query/src/org/labkey/query/QueryModule.java @@ -344,8 +344,12 @@ public void doStartup(ModuleContext moduleContext) Role trustedAnalystRole = RoleManager.getRole("org.labkey.api.security.roles.TrustedAnalystRole"); if (null != trustedAnalystRole) trustedAnalystRole.addPermission(EditQueriesPermission.class); + } - DatabaseMigrationService.get().registerTableHandler(new MigrationTableHandler() + @Override + public void registerMigrationHandlers(@NotNull DatabaseMigrationService service) + { + service.registerTableHandler(new MigrationTableHandler() { @Override public TableInfo getTableInfo() diff --git a/search/src/org/labkey/search/SearchModule.java b/search/src/org/labkey/search/SearchModule.java index 3623744b825..679d8bbe5c9 100644 --- a/search/src/org/labkey/search/SearchModule.java +++ b/search/src/org/labkey/search/SearchModule.java @@ -195,8 +195,12 @@ public void handle(Map properties long count = new TableSelector(auditTable).getRowCount(); return Collections.singletonMap("fullTextSearches", count); }); + } - DatabaseMigrationService.get().registerSchemaHandler(new DefaultMigrationSchemaHandler(SearchSchema.getInstance().getSchema()) + @Override + public void registerMigrationHandlers(@NotNull DatabaseMigrationService service) + { + service.registerSchemaHandler(new DefaultMigrationSchemaHandler(SearchSchema.getInstance().getSchema()) { @Override public List getTablesToCopy() diff --git a/study/src/org/labkey/study/StudyModule.java b/study/src/org/labkey/study/StudyModule.java index d4e62df819d..6695df3a7fc 100644 --- a/study/src/org/labkey/study/StudyModule.java +++ b/study/src/org/labkey/study/StudyModule.java @@ -529,8 +529,12 @@ SELECT COUNT(DISTINCT DD.DomainURI) FROM AdminConsole.addLink(AdminConsole.SettingsLinkType.Premium, "Master Patient Index", new ActionURL(StudyController.MasterPatientProviderAction.class, ContainerManager.getRoot()), AdminPermission.class); DataStateImportExportHelper.registerProvider(new StudyQCImportExportHelper()); + } - DatabaseMigrationService.get().registerSchemaHandler(new DefaultMigrationSchemaHandler(StudySchema.getInstance().getSchema()) + @Override + public void registerMigrationHandlers(@NotNull DatabaseMigrationService service) + { + service.registerSchemaHandler(new DefaultMigrationSchemaHandler(StudySchema.getInstance().getSchema()) { @Override public @Nullable FieldKey getContainerFieldKey(TableInfo sourceTable) @@ -552,7 +556,7 @@ SELECT COUNT(DISTINCT DD.DomainURI) FROM } }); - DatabaseMigrationService.get().registerSchemaHandler(new DefaultMigrationSchemaHandler(StudySchema.getInstance().getDatasetSchema()) + service.registerSchemaHandler(new DefaultMigrationSchemaHandler(StudySchema.getInstance().getDatasetSchema()) { @Override public @Nullable FieldKey getContainerFieldKey(TableInfo sourceTable) @@ -562,7 +566,7 @@ SELECT COUNT(DISTINCT DD.DomainURI) FROM } }); - DatabaseMigrationService.get().registerSchemaHandler(new DefaultMigrationSchemaHandler(DbSchema.get(SpecimenTablesProvider.SCHEMA_NAME, DbSchemaType.Provisioned)) + service.registerSchemaHandler(new DefaultMigrationSchemaHandler(DbSchema.get(SpecimenTablesProvider.SCHEMA_NAME, DbSchemaType.Provisioned)) { @Override public @Nullable FieldKey getContainerFieldKey(TableInfo sourceTable) From 1c888c8dc3a68766e7f614f56b3f5c23c3227921 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 17 Feb 2026 16:50:52 -0800 Subject: [PATCH 11/11] Tweak the issue handler and comment on questionable code --- api/src/org/labkey/api/exp/OntologyManager.java | 1 + .../org/labkey/issue/IssueMigrationSchemaHandler.java | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/exp/OntologyManager.java b/api/src/org/labkey/api/exp/OntologyManager.java index 9095da165d6..5297e1ff8e6 100644 --- a/api/src/org/labkey/api/exp/OntologyManager.java +++ b/api/src/org/labkey/api/exp/OntologyManager.java @@ -184,6 +184,7 @@ public static DomainDescriptor fetchDomainDescriptorFromDB(String uriOrName, Con if (ddArray.size() > 1) { _log.debug("Multiple DomainDescriptors found for " + uriOrName); + // TODO: This check and assignment look wrong. We always return the first element. if (dd.getProject().equals(ContainerManager.getSharedContainer())) dd = ddArray.getFirst(); } diff --git a/issues/src/org/labkey/issue/IssueMigrationSchemaHandler.java b/issues/src/org/labkey/issue/IssueMigrationSchemaHandler.java index 3cafbeb18b1..a7ab04b6ba8 100644 --- a/issues/src/org/labkey/issue/IssueMigrationSchemaHandler.java +++ b/issues/src/org/labkey/issue/IssueMigrationSchemaHandler.java @@ -32,17 +32,24 @@ public class IssueMigrationSchemaHandler extends DefaultMigrationSchemaHandler private final Set COPIED_ISSUE_IDS = new HashSet<>(); - private boolean _filtered = false; + private boolean _filtered; public IssueMigrationSchemaHandler() { super(DbSchema.get(IssuesSchema.ISSUE_DEF_SCHEMA_NAME, DbSchemaType.Provisioned)); } + @Override + public void beforeSchema() + { + _filtered = false; + } + @Override public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilter notCopiedFilter) { - // Remember that issues tables are being filtered so afterSchema() can clean up associated tables (or not) + // afterTable() is called only when rows are filtered (i.e., a DomainDataFilter is configured). Remember this so + // afterSchema() can clean up associated tables (or not). _filtered = true; // Collect the issue IDs that were copied into the target table. We're assuming this set is much smaller than