Skip to content

@LongRunningScript, search logging, warning providers improvements #7482

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 8 commits into
base: develop
Choose a base branch
from
Open

@LongRunningScript, search logging, warning providers improvements #7482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
db5c32e
@LongRunningScript, search logging, warning providers improvements
labkey-adam Mar 9, 2026
a2944e4
Switch MailHelper warning provider to provide static warnings
labkey-adam Mar 9, 2026
9ccf24e
AttachmentContainerListener now detects for orphaned attachments
labkey-adam Mar 9, 2026
c6a28fa
Check for orphans before deleting containers
labkey-adam Mar 10, 2026
0b204a1
Log all orphaned attachments
labkey-adam Mar 10, 2026
29afe37
Merge remote-tracking branch 'origin/develop' into fb_long_running_sc…
labkey-adam Mar 10, 2026
a9c2369
Correct comment
labkey-adam Mar 10, 2026
1e8f795
Remove container param
labkey-adam Mar 10, 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
2 changes: 2 additions & 0 deletions api/src/org/labkey/api/attachments/AttachmentService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ static AttachmentService get()

HttpView<?> getFindAttachmentParentsView();

void detectOrphans();

class DuplicateFilenameException extends IOException implements SkipMothershipLogging
{
private final List<String> _errors = new ArrayList<>();
Expand Down
42 changes: 23 additions & 19 deletions api/src/org/labkey/api/data/ContainerManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.labkey.api.admin.FolderWriterImpl;
import org.labkey.api.admin.StaticLoggerGetter;
import org.labkey.api.attachments.AttachmentParent;
import org.labkey.api.attachments.AttachmentService;
import org.labkey.api.audit.AuditLogService;
import org.labkey.api.audit.AuditTypeEvent;
import org.labkey.api.audit.provider.ContainerAuditProvider;
Expand Down Expand Up @@ -422,6 +423,7 @@ public static Container createContainerFromTemplate(Container parent, String nam

// import objects into the target folder
XmlObject folderXml = vf.getXmlBean("folder.xml");

if (folderXml instanceof FolderDocument folderDoc)
{
FolderImportContext importCtx = new FolderImportContext(user, c, folderDoc, null, new StaticLoggerGetter(LogManager.getLogger(FolderImporterImpl.class)), vf);
Expand Down Expand Up @@ -1539,7 +1541,7 @@ else if (hasAncestryRead)
}

if (!addFolder)
LOG.debug("isNavAccessOpen restriction: \"" + f.getPath() + "\"");
LOG.debug("isNavAccessOpen restriction: \"{}\"", f.getPath());
}

if (addFolder)
Expand Down Expand Up @@ -1915,7 +1917,7 @@ private static boolean delete(final Container c, User user, @Nullable String com
throw new IllegalStateException("Container not flagged as being deleted: " + c.getPath());
}

LOG.debug("Starting container delete for " + c.getContainerNoun(true) + " " + c.getPath());
LOG.debug("Starting container delete for {} {}", c.getContainerNoun(true), c.getPath());

// Tell the search indexer to drop work for the container that's about to be deleted
SearchService.get().purgeForContainer(c);
Expand All @@ -1942,6 +1944,8 @@ private static boolean delete(final Container c, User user, @Nullable String com
setContainerTabDeleted(c.getParent(), c.getName(), c.getParent().getFolderType().getName());
}

AttachmentService.get().detectOrphans();

fireDeleteContainer(c, user);

SqlExecutor sqlExecutor = new SqlExecutor(CORE.getSchema());
Expand Down Expand Up @@ -1981,11 +1985,11 @@ private static boolean delete(final Container c, User user, @Nullable String com
boolean success = CORE.getSchema().getScope().executeWithRetry(tryDeleteContainer);
if (success)
{
LOG.debug("Completed container delete for " + c.getContainerNoun(true) + " " + c.getPath());
LOG.debug("Completed container delete for {} {}", c.getContainerNoun(true), c.getPath());
}
else
{
LOG.warn("Failed to delete container: " + c.getPath());
LOG.warn("Failed to delete container: {}", c.getPath());
}
return success;
}
Expand Down Expand Up @@ -2025,13 +2029,13 @@ public static void deleteAll(Container root, User user, @Nullable String comment
if (!hasTreePermission(root, user, DeletePermission.class))
throw new UnauthorizedException("You don't have delete permissions to all folders");

LOG.debug("Starting container (and children) delete for " + root.getContainerNoun(true) + " " + root.getPath());
LOG.debug("Starting container (and children) delete for {} {}", root.getContainerNoun(true), root.getPath());
Set<Container> depthFirst = getAllChildrenDepthFirst(root);
depthFirst.add(root);

delete(depthFirst, user, comment);

LOG.debug("Completed container (and children) delete for " + root.getContainerNoun(true) + " " + root.getPath());
LOG.debug("Completed container (and children) delete for {} {}", root.getContainerNoun(true), root.getPath());
}

public static void deleteAll(Container root, User user) throws UnauthorizedException
Expand Down Expand Up @@ -2426,7 +2430,7 @@ protected static void fireCreateContainer(Container c, User user, @Nullable Stri
}
catch (Throwable t)
{
LOG.error("fireCreateContainer for " + cl.getClass().getName(), t);
LOG.error("fireCreateContainer for {}", cl.getClass().getName(), t);
}
}
}
Expand All @@ -2437,14 +2441,14 @@ protected static void fireDeleteContainer(Container c, User user)

for (ContainerListener l : list)
{
LOG.debug("Deleting " + c.getPath() + ": fireDeleteContainer for " + l.getClass().getName());
LOG.debug("Deleting {}: fireDeleteContainer for {}", c.getPath(), l.getClass().getName());
try
{
l.containerDeleted(c, user);
}
catch (RuntimeException e)
{
LOG.error("fireDeleteContainer for " + l.getClass().getName(), e);
LOG.error("fireDeleteContainer for {}", l.getClass().getName(), e);

// Issue 17560: Fail fast (first Throwable aborts iteration)
throw e;
Expand Down Expand Up @@ -2490,7 +2494,7 @@ public static void firePropertyChangeEvent(ContainerPropertyChangeEvent evt)
}
catch (Throwable t)
{
LOG.error("firePropertyChangeEvent for " + l.getClass().getName(), t);
LOG.error("firePropertyChangeEvent for {}", l.getClass().getName(), t);
}
}
}
Expand Down Expand Up @@ -2523,8 +2527,8 @@ public static Container createDefaultSupportContainer()
// create a "support" container. Admins can do anything,
// Users can read/write, Guests can read.
return bootstrapContainer(DEFAULT_SUPPORT_PROJECT_PATH,
RoleManager.getRole(AuthorRole.class),
RoleManager.getRole(ReaderRole.class)
RoleManager.getRole(AuthorRole.class),
RoleManager.getRole(ReaderRole.class)
);
}

Expand Down Expand Up @@ -2693,7 +2697,7 @@ public static Container bootstrapContainer(String path, @NotNull Role userRole,

if (c == null)
{
LOG.debug("Creating new container for path '" + path + "'");
LOG.debug("Creating new container for path '{}'", path);
newContainer = true;
c = ensureContainer(path, user);
}
Expand All @@ -2710,7 +2714,7 @@ public static Container bootstrapContainer(String path, @NotNull Role userRole,

if (newContainer || 0 == policyCount.intValue())
{
LOG.debug("Setting permissions for '" + path + "'");
LOG.debug("Setting permissions for '{}'", path);
MutableSecurityPolicy policy = new MutableSecurityPolicy(c);
policy.addRoleAssignment(SecurityManager.getGroup(Group.groupUsers), userRole);
if (guestRole != null)
Expand Down Expand Up @@ -2881,25 +2885,25 @@ public void testFolderType()

private void testOneFolderType(FolderType folderType)
{
LOG.info("testOneFolderType(" + folderType.getName() + "): creating container");
LOG.info("testOneFolderType({}): creating container", folderType.getName());
Container newFolder = createContainer(_testRoot, "folderTypeTest", TestContext.get().getUser());
FolderType ft = newFolder.getFolderType();
assertEquals(FolderType.NONE, ft);

Container newFolderFromCache = getForId(newFolder.getId());
assertNotNull(newFolderFromCache);
assertEquals(FolderType.NONE, newFolderFromCache.getFolderType());
LOG.info("testOneFolderType(" + folderType.getName() + "): setting folder type");
LOG.info("testOneFolderType({}): setting folder type", folderType.getName());
newFolder.setFolderType(folderType, TestContext.get().getUser());

newFolderFromCache = getForId(newFolder.getId());
assertNotNull(newFolderFromCache);
assertEquals(newFolderFromCache.getFolderType().getName(), folderType.getName());
assertEquals(newFolderFromCache.getFolderType().getDescription(), folderType.getDescription());

LOG.info("testOneFolderType(" + folderType.getName() + "): deleteAll");
LOG.info("testOneFolderType({}): deleteAll", folderType.getName());
deleteAll(newFolder, TestContext.get().getUser()); // There might be subfolders because of container tabs
LOG.info("testOneFolderType(" + folderType.getName() + "): deleteAll complete");
LOG.info("testOneFolderType({}): deleteAll complete", folderType.getName());
Container deletedContainer = getForId(newFolder.getId());

if (deletedContainer != null)
Expand Down Expand Up @@ -2946,7 +2950,7 @@ private static void logNode(MultiValuedMap<String, String> mm, String name, int

for (String childName : nodes)
{
LOG.debug(StringUtils.repeat(" ", offset) + childName);
LOG.debug("{}{}", StringUtils.repeat(" ", offset), childName);
logNode(mm, childName, offset + 1);
}
}
Expand Down
99 changes: 48 additions & 51 deletions api/src/org/labkey/api/data/SqlScriptManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;


/**
Expand Down Expand Up @@ -172,7 +174,27 @@ public Set<SqlScript> getPreviouslyRunScripts()
return runScripts;
}

@NotNull
public Collection<String> getPreviouslyRunSqlScriptNames()
{
TableInfo tinfo = getTableInfoSqlScripts();

// Skip if the table hasn't been created yet (bootstrap case)
if (getTableInfoSqlScripts().getTableType() == DatabaseTableType.NOT_IN_DB)
return Collections.emptySet();

SimpleFilter filter = new SimpleFilter();
ColumnInfo fileNameColumn = tinfo.getColumn("FileName");
filter.addCondition(tinfo.getColumn("ModuleName"), _provider.getProviderName());
filter.addCondition(tinfo.getColumn("FileName"), _schema.getResourcePrefix() + "-", CompareType.STARTS_WITH);

return new TableSelector(tinfo, Collections.singleton(fileNameColumn), filter, null).getCollection(String.class);
}

private static final String SKIP_SCRIPT_ANNOTATION = "@SkipScriptIfSchemaExists";
// Use to annotate a script that may take a long time to execute. A message will be logged, including a reason and
// strongly discouraging server shutdown or restart during upgrade.
private static final Pattern LONG_RUNNING_SCRIPT_ANNOTATION_PATTERN = Pattern.compile("@LongRunningScript\\('(?<reason>.+)'\\)");

public void runScript(@Nullable User user, SqlScript script, ModuleContext moduleContext, @Nullable Connection conn) throws SqlScriptException
{
Expand Down Expand Up @@ -204,74 +226,49 @@ public void runScript(@Nullable User user, SqlScript script, ModuleContext modul
}
else
{
Matcher matcher = LONG_RUNNING_SCRIPT_ANNOTATION_PATTERN.matcher(contents);
if (matcher.find())
{
// Reason is expected to be a gerund phrase that summarizes the time-consuming action(s) that the
// script is taking. It should start with a lowercase letter and should not end with punctuation.
// Examples:
// - updating all ObjectId columns to BIGINT
// - restructuring the way workflow jobs are stored
String reason = matcher.group("reason");
LOG.info(
"""
This script could take a long time to execute because it is {}.
Do NOT shut down or restart the server until this script and the rest of the upgrade is complete.
Any interruption will likely corrupt the schemas, requiring a database restore and a restart of the upgrade process.""",
reason
);
}
dialect.runSql(description, schema, contents, moduleContext, conn);
LOG.info("Finished running script: {}", description);
}
}
catch(Throwable t)
catch (Throwable t)
{
throw new SqlScriptException(t, description);
}

if (script.isValidName())
{
// Should never be true, unless getNewScripts() isn't doing its job. TODO: Remove update() branch below.
assert !hasBeenRun(script);
if (hasBeenRun(script))
update(user, script);
else
insert(user, script);
}
insert(user, script);
}

@NotNull
public Collection<String> getPreviouslyRunSqlScriptNames()
private void insert(@Nullable User user, SqlScript script)
{
TableInfo tinfo = getTableInfoSqlScripts();

// Skip if the table hasn't been created yet (bootstrap case)
if (getTableInfoSqlScripts().getTableType() == DatabaseTableType.NOT_IN_DB)
return Collections.emptySet();

SimpleFilter filter = new SimpleFilter();
ColumnInfo fileNameColumn = tinfo.getColumn("FileName");
filter.addCondition(tinfo.getColumn("ModuleName"), _provider.getProviderName());
filter.addCondition(tinfo.getColumn("FileName"), _schema.getResourcePrefix() + "-", CompareType.STARTS_WITH);

return new TableSelector(tinfo, Collections.singleton(fileNameColumn), filter, null).getCollection(String.class);
}

public boolean hasBeenRun(SqlScript script)
{
TableInfo tinfo = getTableInfoSqlScripts();

// Make sure DbSchema thinks SqlScript table is in the database. If not, we're bootstrapping and it's either just before or just after the first
// script is run. In either case, invalidate to force reloading schema from database meta data.
// Make sure DbSchema thinks SqlScripts table is in the database. If not, we're bootstrapping, and it's just
// after the first script has run. Invalidate to force reloading the schema from database metadata.
if (tinfo.getTableType() == DatabaseTableType.NOT_IN_DB)
{
CacheManager.clearAllKnownCaches();
return false;
tinfo = getTableInfoSqlScripts(); // Reload to update table type
}

PkFilter filter = new PkFilter(tinfo, new String[]{_provider.getProviderName(), script.getDescription()});

return new TableSelector(getTableInfoSqlScripts(), filter, null).exists();
}


public void insert(@Nullable User user, SqlScript script)
{
SqlScriptBean ss = new SqlScriptBean(script.getProvider().getProviderName(), script.getDescription());

Table.insert(user, getTableInfoSqlScripts(), ss);
}


public void update(@Nullable User user, SqlScript script)
{
Object[] pk = new Object[]{script.getProvider().getProviderName(), script.getDescription()};

Table.update(user, getTableInfoSqlScripts(), new HashMap<>(), pk); // Update user and modified date
Table.insert(user, tinfo, new SqlScriptBean(script.getProvider().getProviderName(), script.getDescription()));
}

// Allow null version for oddball cases like gel_reports, which claims to have schemas but no schema version. That
Expand Down Expand Up @@ -301,15 +298,15 @@ public void updateSchemaVersion(Double version)
}


public @NotNull SchemaBean ensureSchemaBean()
protected @NotNull SchemaBean ensureSchemaBean()
{
SchemaBean bean = getSchemaBean();

return null != bean ? bean : new SchemaBean(_schema.getDisplayName(), _provider.getProviderName(), 0);
}


protected @Nullable SchemaBean getSchemaBean()
private @Nullable SchemaBean getSchemaBean()
{
TableInfo tinfo = getTableInfoSchemas();

Expand Down
1 change: 0 additions & 1 deletion api/src/org/labkey/api/reports/model/ReportPropsManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@

import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;

Expand Down
5 changes: 2 additions & 3 deletions api/src/org/labkey/api/search/NoopSearchService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.labkey.api.webdav.WebdavResource;

import java.io.Reader;
import java.time.Duration;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
Expand Down Expand Up @@ -292,7 +291,7 @@ public void deleteIndex(String reason)
}

@Override
public void clearLastIndexed()
public void clearLastIndexed(String reason)
{
}

Expand Down Expand Up @@ -379,7 +378,7 @@ public boolean isRunning()
}

@Override
public void updateIndex()
public void updateIndex(String reason)
{
}

Expand Down
5 changes: 2 additions & 3 deletions api/src/org/labkey/api/search/SearchService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.Reader;
import java.net.URISyntaxException;
import java.sql.SQLException;
import java.sql.Timestamp;
import java.util.ArrayList;
Expand Down Expand Up @@ -438,7 +437,7 @@ public String normalizeHref(Path contextPath, Container c)
void resetIndex();
void startCrawler();
void pauseCrawler();
void updateIndex();
void updateIndex(String reason);
void refreshNow();

@Nullable Throwable getConfigurationError();
Expand Down Expand Up @@ -472,7 +471,7 @@ public String normalizeHref(Path contextPath, Container c)
void deleteContainer(String id);

void deleteIndex(String reason); // close the index if it's been initialized, then delete the index directory and reset lastIndexed values
void clearLastIndexed(); // reset lastIndexed values and initiate aggressive crawl. must be callable before (and after) start() has been called.
void clearLastIndexed(String reason); // reset lastIndexed values and initiate aggressive crawl. must be callable before (and after) start() has been called.
void maintenance();

//
Expand Down
Loading