Skip to content

Rework startup/upgrade/bootstrap process to prune modules before core upgrade #7419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
labkey-adam wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Rework startup/upgrade/bootstrap process to prune modules before core upgrade #7419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
855412a
Enforce that getWebPartFactories() is called after upgrade is complete
labkey-adam Feb 15, 2026
748bf46
Prune modules with missing dependencies before core upgrade to preven…
labkey-adam Feb 16, 2026
2c63180
Update comment
labkey-adam Feb 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions api/src/org/labkey/api/module/DefaultModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions api/src/org/labkey/api/module/Module.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ default boolean canBeEnabled(Container c)
/** License name: e.g. "Apache 2.0", "LabKey Software License" */
@Nullable String getLicense();

/** License URL: e.g. "http://www.apache.org/licenses/LICENSE-2.0" */
/** License URL: e.g. "https://www.apache.org/licenses/LICENSE-2.0" */
@Nullable String getLicenseUrl();

/**
Expand Down Expand Up @@ -182,16 +182,15 @@ default void startBackgroundThreads()

/**
* The application is shutting down "gracefully". Module
* should do any cleanup (file handles etc) that is required.
* should do any cleanup (file handles, etc.) that is required.
* Note: There is no guarantee that this will run if the server
* process is without a nice shutdown.
*/
void destroy();

/**
* Return Collection of WebPartFactory objects for this module.
* NOTE: This may be called before startup, but will never be called
* before upgrade is complete.
* NOTE: This may be called early, before init() and startup(), but after core upgrade.
*
* @return Collection of WebPartFactory (empty collection if none)
*/
Expand Down
81 changes: 41 additions & 40 deletions api/src/org/labkey/api/module/ModuleLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.commons.collections4.MultiValuedMap;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.Logger;
import org.apache.xmlbeans.XmlBeans;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.labkey.api.Constants;
Expand Down Expand Up @@ -428,7 +427,7 @@ public void updateModuleDirectory(File dir, File archive)
{
throw new IllegalStateException("Not a valid module: " + archive.getName());
}
moduleCreated = moduleList.get(0);
moduleCreated = moduleList.getFirst();
if (null != archive)
moduleCreated.setZippedPath(archive);

Expand Down Expand Up @@ -480,7 +479,7 @@ public void updateModuleDirectory(File dir, File archive)
// avoid error in startup, DefaultModule does not expect to see module with same name initialized again
((DefaultModule) moduleCreated).unregister();
_moduleFailures.remove(moduleCreated.getName());
pruneModules(moduleList);
ensureModulesSupportLabKeyDatabase(moduleList);
initializeModules(moduleList);

Throwable t = _moduleFailures.get(moduleCreated.getName());
Expand Down Expand Up @@ -576,7 +575,7 @@ private void doInit(Execution execution) throws ServletException

// set the project source root before calling .initialize() on modules
var modules = getModules();
Module coreModule = modules.isEmpty() ? null : modules.get(0);
Module coreModule = modules.isEmpty() ? null : modules.getFirst();
if (coreModule == null || !DefaultModule.CORE_MODULE_NAME.equals(coreModule.getName()))
throw new IllegalStateException("Core module was not first or could not find the Core module. Ensure that Tomcat user can create directories under the <LABKEY_HOME>/modules directory.");
setProjectRoot(coreModule);
Expand Down Expand Up @@ -622,7 +621,9 @@ private void doInit(Execution execution) throws ServletException
// Prune modules before upgrading core module, see Issue 42150
synchronized (_modulesLock)
{
pruneModules(_modules);
// _modules is in dependency order
ensureModulesSupportLabKeyDatabase(_modules);
verifyDependencies(_modules);
}

if (getTableInfoModules().getTableType() == DatabaseTableType.NOT_IN_DB)
Expand Down Expand Up @@ -678,7 +679,6 @@ private void doInit(Execution execution) throws ServletException
synchronized (_modulesLock)
{
checkForRenamedModules();
// use _modules here because this List<> needs to be modifiable
initializeModules(_modules);
}

Expand Down Expand Up @@ -920,28 +920,45 @@ private boolean isDevelopmentBuild(Module module)
}

/**
* Enumerates all the modules, removing the ones that don't support the core database
* Enumerates all the modules, removing the ones that don't support the primary database
*/
private void pruneModules(List<Module> modules)
private void ensureModulesSupportLabKeyDatabase(List<Module> modules)
{
Module core = getCoreModule();

SupportedDatabase coreType = SupportedDatabase.get(CoreSchema.getInstance().getSqlDialect());
// Need to enumerate a copy of the list to avoid ConcurrentModificationException
SqlDialect dialect = DbScope.getLabKeyScope().getSqlDialect();
SupportedDatabase primaryType = SupportedDatabase.get(dialect);
// Enumerate a copy of the list to avoid ConcurrentModificationException
for (Module module : new ArrayList<>(modules))
{
if (module == core)
continue;
if (!module.getSupportedDatabasesSet().contains(coreType))
if (!module.getSupportedDatabasesSet().contains(primaryType))
{
var e = new DatabaseNotSupportedException("This module does not support " + CoreSchema.getInstance().getSqlDialect().getProductName());
var e = new DatabaseNotSupportedException("This module does not support " + dialect.getProductName());
// In production mode, treat these exceptions as a module initialization error
// In dev mode, make them warnings so devs can easily switch databases
removeModule(modules, module, !AppProps.getInstance().isDevMode(), e);
}
}
}

/**
* Remove modules if any of their module dependencies are missing
*/
private void verifyDependencies(List<Module> modules)
{
for (Module module : modules)
{
try
{
verifyDependencies(module);
}
catch (ModuleDependencyException e)
{
// In production mode, treat module dependency exceptions as errors
// In dev mode, make them warnings so devs can easily switch databases
removeModule(modules, module, !AppProps.getInstance().isDevMode(), e);
}
}
}

/**
* Enumerates all remaining modules, initializing them and removing any that fail to initialize
*/
Expand All @@ -950,14 +967,10 @@ private void initializeModules(List<Module> modules)
Module core = getCoreModule();

/*
* NOTE: Module.initialize() really should not ask for resources from _other_ modules,
* as they may have not initialized themselves yet. However, we did not enforce that
* so this cross-module behavior may have crept in.
*
* To help mitigate this a little, we remove modules that do not support this DB type
* before calling initialize().
*
* NOTE: see FolderTypeManager.get().registerFolderType() for an example of enforcing this
* NOTE: Module.initialize() really should not ask for resources from _other_ modules, as they may have not
* initialized themselves yet. However, we did not enforce that so this cross-module behavior may have crept
* in. To help mitigate this a little, we previously removed modules based on supported DB type and missing
* dependencies. See FolderTypeManager.registerFolderType() for an example of enforcing this
*/

//initialize each module in turn
Expand All @@ -968,21 +981,9 @@ private void initializeModules(List<Module> modules)

try
{
try
{
// Make sure all its dependencies initialized successfully
verifyDependencies(module);
module.initialize();
}
catch (DatabaseNotSupportedException | ModuleDependencyException e)
{
// In production mode, treat these exceptions as a module initialization error
if (!AppProps.getInstance().isDevMode())
throw e;

// In dev mode, make them warnings so devs can easily switch databases
removeModule(modules, module, false, e);
}
// Make sure all its dependencies initialized successfully
verifyDependencies(module);
module.initialize();
}
catch (Throwable t)
{
Expand All @@ -995,7 +996,7 @@ private void initializeModules(List<Module> modules)
initControllerToModule();
}

// Check a module's dependencies and throw on the first one that's not present (i.e., it was removed because its initialize() failed)
// Check a module's dependencies and throw on the first one that's not present (i.e., dependency is not present or was removed because its initialize() failed)
private void verifyDependencies(Module module)
{
synchronized (_modulesLock)
Expand Down