Fix SQL Server migration issues

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;
@@ -177,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()
@@ -214,7 +216,7 @@ public void afterSchema(DatabaseMigrationConfiguration configuration, DbSchema s
             }
         });
 
-        DatabaseMigrationService.get().registerTableHandler(new MigrationTableHandler()
+        service.registerTableHandler(new MigrationTableHandler()
         {
             @Override
             public TableInfo getTableInfo()
@@ -227,34 +229,9 @@ 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");
-                }
-            }
         });
     }
 
-
     @Override
     public void startBackgroundThreads()
     {

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<Container> getChildren(Container parent)

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<String, SchemaTableInfoFactory> metaDataTableNames = DbSchema.loadTableMetaData(DbScope.getLabKeyScope(), metaDataName);
+            Map<String, SchemaTableInfoFactory> metaDataTableNames = DbSchema.loadTableMetaData(scope, metaDataName);
 
             return new MigrationDbSchema(metaDataName, this, scope, metaDataTableNames, module);
         }

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)));
         }
     }
 

api/src/org/labkey/api/exp/OntologyManager.java

@@ -122,7 +122,7 @@ public PropertyDescriptor load(@NotNull Pair<String, GUID> 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<PropertyDescriptor> pdArray = new SqlSelector(getExpSchema(), sql, propertyURI, proj, _sharedContainer.getId()).getArrayList(PropertyDescriptor.class);
+                List<PropertyDescriptor> 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<String, GUID> 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,15 +177,15 @@ 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.
             if (ddArray.size() > 1)
             {
                 _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<Pair<String, Boolean>> load(@NotNull Pair<String, GUID> 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<Pair<String, Boolean>> load(@NotNull Pair<String, GUID> 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<String> comparePropertyDescriptors(PropertyDescriptor pdIn,
         List<String> 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<DomainDescriptor> getDomainDescriptors(Container contai
                 }
             }
 
-            if (_sharedContainer.hasPermission(user, ReadPermission.class))
+            if (ContainerManager.getSharedContainer().hasPermission(user, ReadPermission.class))
             {
-                for (Map.Entry<String, DomainDescriptor> entry : getCachedDomainDescriptors(_sharedContainer, user).entrySet())
+                for (Map.Entry<String, DomainDescriptor> 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?

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)
     {

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");
+    }
+}

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<Pair<Class<? extends DefaultModule>, String>> INSTANTIATED_MODULES = new HashSet<>();
 
     static final ModuleResourceCache<ModuleXml> MODULE_XML_CACHE = ModuleResourceCaches.create("module.xml files", new ModuleXmlCacheHandler(), ResourceRootProvider.getStandard(new Path()));

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)
+    {
+    }
 }

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<Collection<SimpleWebPartFactory>>
 {

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;
 
 /**