From 8bf8fe9687935a4510536de1e0184a145def022a Mon Sep 17 00:00:00 2001 From: Cory Nathe Date: Tue, 25 Nov 2025 15:42:21 -0600 Subject: [PATCH 01/12] GitHub Issue #443: AbstractAssayTsvDataHandler to lookup field values by titleColumn before primaryKey in checkData (#7226) - similar to LookupResolutionType.alternateThenPrimaryKey, we want to check if the string value remaps using alternate keys (titleColumn) first --- .../assay/AbstractAssayTsvDataHandler.java | 12 +++++++--- .../labkey/test/tests/study/AssayTest.java | 23 ++++++++++++++++++- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java index 9373ee83ee9..6968e221d64 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java +++ b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java @@ -706,7 +706,8 @@ private DataIterator checkData( DomainProperty wellLocationPropFinder = null; DomainProperty wellLsidPropFinder = null; - RemapCache cache = new RemapCache(); + RemapCache cacheWithoutPkLookup = new RemapCache(); + RemapCache cacheWithPkLookup = new RemapCache(); Map remappableLookup = new HashMap<>(); Map materialCache = new LongHashMap<>(); Map> plateWellCache = new LongHashMap<>(); @@ -879,7 +880,12 @@ else if (entry.getKey().equalsIgnoreCase(ProvenanceService.PROVENANCE_INPUT_PROP { String s = o instanceof String ? (String) o : o.toString(); TableInfo lookupTable = remappableLookup.get(pd); - Object remapped = cache.remap(lookupTable, s, true); + + // GitHub Issue #443: similar to LookupResolutionType.alternateThenPrimaryKey, we want to check if the string value remaps using alternate keys (titleColumn) first + Object remapped = cacheWithoutPkLookup.remap(lookupTable, s, false); + if (remapped == null) + remapped = cacheWithPkLookup.remap(lookupTable, s, true); + if (remapped == null) { if (SAMPLE_CONCEPT_URI.equals(pd.getConceptURI())) @@ -1026,7 +1032,7 @@ else if (o instanceof MvFieldWrapper mvWrapper) try { if (material == null) - material = exp.findExpMaterial(lookupContainer, user, materialName, byNameSS, cache, materialCache); + material = exp.findExpMaterial(lookupContainer, user, materialName, byNameSS, cacheWithoutPkLookup, materialCache); } catch (ValidationException ve) { diff --git a/study/test/src/org/labkey/test/tests/study/AssayTest.java b/study/test/src/org/labkey/test/tests/study/AssayTest.java index 5812f79bedf..b455a46c0a7 100644 --- a/study/test/src/org/labkey/test/tests/study/AssayTest.java +++ b/study/test/src/org/labkey/test/tests/study/AssayTest.java @@ -1036,7 +1036,10 @@ public void testAssayLookupValidatorConversion() _listHelper.bulkImportData(TestDataUtils.tsvStringFromRowMaps(List.of( Map.of(valueField.getName(), "One"), Map.of(valueField.getName(), "Two"), - Map.of(valueField.getName(), "123") + Map.of(valueField.getName(), "123"), + // GitHub Issue #443: value is the primary key for another row + Map.of(valueField.getName(), "5"), // pk = 4 + Map.of(valueField.getName(), "6") // pk = 5 ), List.of(valueField.getName()), true)); log("Create an assay with a results lookup field to the list, with lookup validator set"); @@ -1060,6 +1063,24 @@ public void testAssayLookupValidatorConversion() .setLookupValidatorEnabled(false); designerPage.clickFinish(); verifyAssayImportForLookupValidator(ISSUE_53625_ASSAY, lookupField, "RunWithoutLookupValidator", false); + + log("GitHub Issue #443: Verify that importing a value that is also a primary key maps to the titleColumn value"); + verifyAssayImportForPKValueThatIsTitleColumn(ISSUE_53625_ASSAY, lookupField, "RunWithPKandTitleColumn"); + } + + private void verifyAssayImportForPKValueThatIsTitleColumn(String assayName, FieldInfo lookupField, String runName) + { + String runDataStr = TestDataUtils.tsvStringFromRowMaps(List.of( + Map.of(lookupField.getName(), "4"), // pk 4, value 5 + Map.of(lookupField.getName(), "5"), // pk 4, value 5 + Map.of(lookupField.getName(), "6")), // pk 5, value 6 + List.of(lookupField.getName()), true + ); + importAssayData(assayName, runName, runDataStr); + clickAndWait(Locator.linkWithText(runName)); + DataRegionTable dataTable = new DataRegionTable("Data", getDriver()); + checker().verifyEquals("Incorrect number of results shown.", 3, dataTable.getDataRowCount()); + checker().fatal().verifyEquals("Lookup values not as expected.", List.of("5", "5", "6"), dataTable.getColumnDataAsText(lookupField.getLabel())); } private void verifyAssayImportForLookupValidator(String assayName, FieldInfo lookupField, String runName, boolean validatorOn) From d2edbe9cb40a9821f3fcc3cd2fc97dce85424699 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 25 Nov 2025 18:00:04 -0800 Subject: [PATCH 02/12] Standardize IN clause generation (#7221) --- .../attachments/LookAndFeelResourceType.java | 1 + api/src/org/labkey/api/data/SimpleFilter.java | 35 ++++ .../api/data/TempTableInClauseGenerator.java | 2 + .../data/dialect/BasePostgreSqlDialect.java | 35 ---- .../data/dialect/DatabaseMaintenanceTask.java | 12 +- .../api/data/dialect/SimpleSqlDialect.java | 2 +- .../labkey/api/data/dialect/SqlDialect.java | 32 +++- .../migration/DatabaseMigrationService.java | 2 +- .../DefaultMigrationSchemaHandler.java | 8 +- api/src/org/labkey/api/util/JobRunner.java | 37 ++-- .../core/CoreMigrationSchemaHandler.java | 5 +- core/src/org/labkey/core/CoreModule.java | 6 +- .../core/dialect/PostgreSql92Dialect.java | 37 +++- .../core/dialect/PostgreSqlInClauseTest.java | 59 ------ .../ExperimentMigrationSchemaHandler.java | 171 +++++++++++------- .../labkey/experiment/ExperimentModule.java | 24 +-- 16 files changed, 250 insertions(+), 218 deletions(-) delete mode 100644 core/src/org/labkey/core/dialect/PostgreSqlInClauseTest.java diff --git a/api/src/org/labkey/api/attachments/LookAndFeelResourceType.java b/api/src/org/labkey/api/attachments/LookAndFeelResourceType.java index 0cf38f92686..4495a645f8d 100644 --- a/api/src/org/labkey/api/attachments/LookAndFeelResourceType.java +++ b/api/src/org/labkey/api/attachments/LookAndFeelResourceType.java @@ -41,6 +41,7 @@ private LookAndFeelResourceType() @Override public void addWhereSql(SQLFragment sql, String parentColumn, String documentNameColumn) { + // Keep in sync with CoreMigrationSchemaHandler.copyAttachments() sql.append(parentColumn).append(" IN (SELECT EntityId FROM ").append(CoreSchema.getInstance().getTableInfoContainers(), "c").append(") AND ("); sql.append(documentNameColumn).append(" IN (?, ?) OR "); sql.add(AttachmentCache.FAVICON_FILE_NAME); diff --git a/api/src/org/labkey/api/data/SimpleFilter.java b/api/src/org/labkey/api/data/SimpleFilter.java index 8dff53f20ff..5393bbe41aa 100644 --- a/api/src/org/labkey/api/data/SimpleFilter.java +++ b/api/src/org/labkey/api/data/SimpleFilter.java @@ -1865,6 +1865,41 @@ private int howManyLessThan(List userIdsDesc, int max) } return howMany; } + + /** + * This used to be a PostgreSQL-specific test, but it should run and pass on SQL Server as well. It's largely + * redundant with testLargeInClause() above, but causes no harm. + */ + @Test + public void testTempTableInClause() + { + DbSchema core = CoreSchema.getInstance().getSchema(); + SqlDialect d = core.getSqlDialect(); + + Collection allUserIds = new TableSelector(CoreSchema.getInstance().getTableInfoUsersData(), Collections.singleton("UserId")).getCollection(Integer.class); + SQLFragment shortSql = new SQLFragment("SELECT * FROM core.UsersData WHERE UserId"); + d.appendInClauseSql(shortSql, allUserIds); + assertEquals(allUserIds.size(), new SqlSelector(core, shortSql).getRowCount()); + + ArrayList l = new ArrayList<>(allUserIds); + while (l.size() < SqlDialect.TEMP_TABLE_GENERATOR_MIN_SIZE) + l.addAll(allUserIds); + SQLFragment longSql = new SQLFragment("SELECT * FROM core.UsersData WHERE UserId"); + d.appendInClauseSql(longSql, l); + assertEquals(allUserIds.size(), new SqlSelector(core, longSql).getRowCount()); + + Collection allDisplayNames = new TableSelector(CoreSchema.getInstance().getTableInfoUsersData(), Collections.singleton("DisplayName")).getCollection(String.class); + SQLFragment shortSqlStr = new SQLFragment("SELECT * FROM core.UsersData WHERE DisplayName"); + d.appendInClauseSql(shortSqlStr, allDisplayNames); + assertEquals(allDisplayNames.size(), new SqlSelector(core, shortSqlStr).getRowCount()); + + l = new ArrayList<>(allDisplayNames); + while (l.size() < SqlDialect.TEMP_TABLE_GENERATOR_MIN_SIZE) + l.addAll(allDisplayNames); + SQLFragment longSqlStr = new SQLFragment("SELECT * FROM core.UsersData WHERE DisplayName"); + d.appendInClauseSql(longSqlStr, l); + assertEquals(allDisplayNames.size(), new SqlSelector(core, longSqlStr).getRowCount()); + } } public static class BetweenClauseTestCase extends ClauseTestCase diff --git a/api/src/org/labkey/api/data/TempTableInClauseGenerator.java b/api/src/org/labkey/api/data/TempTableInClauseGenerator.java index 14276fb551d..3649ea4b7e6 100644 --- a/api/src/org/labkey/api/data/TempTableInClauseGenerator.java +++ b/api/src/org/labkey/api/data/TempTableInClauseGenerator.java @@ -142,7 +142,9 @@ else if (jdbcType == JdbcType.INTEGER) try (var ignored = SpringActionController.ignoreSqlUpdates()) { new SqlExecutor(tempSchema).execute(indexSql); + tempSchema.getSqlDialect().updateStatistics(tempTableInfo); // Immediately analyze the newly populated & indexed table } + TempTableInfo cacheEntry = tempTableInfo; // Don't bother caching if we're in a transaction diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index ecdb80e1507..7f367ef89c7 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -34,7 +34,6 @@ import org.labkey.api.data.DbSchemaType; import org.labkey.api.data.DbScope; import org.labkey.api.data.DbScope.LabKeyDataSource; -import org.labkey.api.data.InClauseGenerator; import org.labkey.api.data.JdbcType; import org.labkey.api.data.MetadataSqlSelector; import org.labkey.api.data.PropertyStorageSpec; @@ -49,7 +48,6 @@ import org.labkey.api.data.Table; import org.labkey.api.data.TableChange; import org.labkey.api.data.TableInfo; -import org.labkey.api.data.TempTableInClauseGenerator; import org.labkey.api.data.TempTableTracker; import org.labkey.api.data.dialect.LimitRowsSqlGenerator.LimitRowsCustomizer; import org.labkey.api.data.dialect.LimitRowsSqlGenerator.StandardLimitRowsCustomizer; @@ -92,19 +90,15 @@ public abstract class BasePostgreSqlDialect extends SqlDialect { // Issue 52190: Expose troubleshooting data that supports postgreSQL-specific analysis public static final String POSTGRES_SCHEMA_NAME = "postgres"; - public static final int TEMPTABLE_GENERATOR_MINSIZE = 1000; private final Map _domainScaleMap = new CopyOnWriteHashMap<>(); private final AtomicBoolean _arraySortFunctionExists = new AtomicBoolean(false); - private final InClauseGenerator _tempTableInClauseGenerator = new TempTableInClauseGenerator(); private HtmlString _adminWarning = null; // Default to 9 and let newer versions be refreshed later private int _majorVersion = 9; - protected InClauseGenerator _inClauseGenerator = null; - // Specifies if this PostgreSQL server treats backslashes in string literals as normal characters (as per the SQL // standard) or as escape characters (old, non-standard behavior). As of PostgreSQL 9.1, the setting // standard_conforming_strings is on by default; before 9.1, it was off by default. We check the server setting @@ -289,24 +283,6 @@ public String addReselect(SQLFragment sql, ColumnInfo column, @Nullable String p return proposedVariable; } - @Override - public SQLFragment appendInClauseSql(SQLFragment sql, @NotNull Collection params) - { - return appendInClauseSqlWithCustomInClauseGenerator(sql, params, _tempTableInClauseGenerator); - } - - @Override - public SQLFragment appendInClauseSqlWithCustomInClauseGenerator(SQLFragment sql, @NotNull Collection params, InClauseGenerator tempTableGenerator) - { - if (params.size() >= TEMPTABLE_GENERATOR_MINSIZE) - { - SQLFragment ret = tempTableGenerator.appendInClauseSql(sql, params); - if (null != ret) - return ret; - } - return _inClauseGenerator.appendInClauseSql(sql, params); - } - @Override public @NotNull ResultSet executeWithResults(@NotNull PreparedStatement stmt) throws SQLException { @@ -543,12 +519,6 @@ public boolean isSystemSchema(String schemaName) schemaName.startsWith("pg_toast_temp_"); } - @Override - public String getAnalyzeCommandForTable(String tableName) - { - return "ANALYZE " + tableName; - } - @Override protected String getSIDQuery() { @@ -783,7 +753,6 @@ public void prepare(LabKeyDataSource dataSource) public String prepare(DbScope scope) { initializeUserDefinedTypes(scope); - initializeInClauseGenerator(scope); determineSettings(scope); determineIfArraySortFunctionExists(scope); return super.prepare(scope); @@ -835,10 +804,6 @@ private String getDomainKey(String schemaName, String domainName) return ("public".equals(schemaName) ? domainName : "\"" + schemaName + "\".\"" + domainName + "\""); } - - abstract protected void initializeInClauseGenerator(DbScope scope); - - // Query any settings that may affect dialect behavior. Right now, only "standard_conforming_strings". protected void determineSettings(DbScope scope) { diff --git a/api/src/org/labkey/api/data/dialect/DatabaseMaintenanceTask.java b/api/src/org/labkey/api/data/dialect/DatabaseMaintenanceTask.java index a2545f3bf8a..bd446d602ef 100644 --- a/api/src/org/labkey/api/data/dialect/DatabaseMaintenanceTask.java +++ b/api/src/org/labkey/api/data/dialect/DatabaseMaintenanceTask.java @@ -24,7 +24,7 @@ import org.labkey.api.util.SystemMaintenance.MaintenanceTask; import org.springframework.jdbc.BadSqlGrammarException; -class DatabaseMaintenanceTask implements MaintenanceTask +public class DatabaseMaintenanceTask implements MaintenanceTask { @Override public String getDescription() @@ -41,13 +41,17 @@ public String getName() @Override public void run(Logger log) { - DbScope scope = DbScope.getLabKeyScope(); + run(DbScope.getLabKeyScope(), log); + } + + public static void run(DbScope scope, Logger log) + { String url = null; try { url = scope.getDataSourceProperties().getUrl(); - log.info("Database maintenance on " + url + " started"); + log.info("Database maintenance on {} started", url); } catch (Exception e) { @@ -69,6 +73,6 @@ public void run(Logger log) } if (null != url) - log.info("Database maintenance on " + url + " complete"); + log.info("Database maintenance on {} complete", url); } } diff --git a/api/src/org/labkey/api/data/dialect/SimpleSqlDialect.java b/api/src/org/labkey/api/data/dialect/SimpleSqlDialect.java index d597dd10232..73337dd3feb 100644 --- a/api/src/org/labkey/api/data/dialect/SimpleSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/SimpleSqlDialect.java @@ -382,7 +382,7 @@ public void overrideAutoIncrement(StringBuilder statements, TableInfo tinfo) } @Override - public String getAnalyzeCommandForTable(String tableName) + public SQLFragment getAnalyzeCommandForTable(String tableName) { throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement"); } diff --git a/api/src/org/labkey/api/data/dialect/SqlDialect.java b/api/src/org/labkey/api/data/dialect/SqlDialect.java index b11a782b34a..cac58227ec7 100644 --- a/api/src/org/labkey/api/data/dialect/SqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/SqlDialect.java @@ -102,6 +102,7 @@ public abstract class SqlDialect { public static final String GENERIC_ERROR_MESSAGE = "The database experienced an unexpected problem. Please check your input and try again."; public static final String CUSTOM_UNIQUE_ERROR_MESSAGE = "Constraint violation: cannot insert duplicate value for column"; + public static final int TEMP_TABLE_GENERATOR_MIN_SIZE = 1000; protected static final Logger LOG = LogHelper.getLogger(SqlDialect.class, "Database warnings and errors"); protected static final int MAX_VARCHAR_SIZE = 4000; //Any length over this will be set to nvarchar(max)/text @@ -527,16 +528,33 @@ protected Set getJdbcKeywords(SqlExecutor executor) throws SQLException, private static final InClauseGenerator DEFAULT_GENERATOR = new ParameterMarkerInClauseGenerator(); + public InClauseGenerator getDefaultInClauseGenerator() + { + return DEFAULT_GENERATOR; + } + + // Dialects that support temp-table IN clauses must override this method + public InClauseGenerator getTempTableInClauseGenerator() + { + return null; + } + // Most callers should use this method - public SQLFragment appendInClauseSql(SQLFragment sql, @NotNull Collection params) + public final SQLFragment appendInClauseSql(SQLFragment sql, @NotNull Collection params) { - return appendInClauseSqlWithCustomInClauseGenerator(sql, params, null); + return appendInClauseSqlWithCustomInClauseGenerator(sql, params, getTempTableInClauseGenerator()); } - // Use only in cases where the default temp-table generator won't do, e.g., you need to apply a large IN clause in an external data source - public SQLFragment appendInClauseSqlWithCustomInClauseGenerator(SQLFragment sql, @NotNull Collection params, InClauseGenerator tempTableGenerator) + // Call directly only in cases where the default temp-table generator won't do, e.g., you need to apply a large IN clause in an external data source + public final SQLFragment appendInClauseSqlWithCustomInClauseGenerator(SQLFragment sql, @NotNull Collection params, @Nullable InClauseGenerator largeInClauseGenerator) { - return DEFAULT_GENERATOR.appendInClauseSql(sql, params); + if (params.size() >= TEMP_TABLE_GENERATOR_MIN_SIZE && largeInClauseGenerator != null) + { + SQLFragment ret = largeInClauseGenerator.appendInClauseSql(sql, params); + if (null != ret) + return ret; + } + return getDefaultInClauseGenerator().appendInClauseSql(sql, params); } public SQLFragment appendCaseInsensitiveLikeClause(SQLFragment sql, @NotNull String matchStr, @Nullable String wildcardPrefix, @Nullable String wildcardSuffix, char escapeChar) @@ -1352,7 +1370,7 @@ public String getMessage() } } - public abstract String getAnalyzeCommandForTable(String tableName); + public abstract SQLFragment getAnalyzeCommandForTable(String tableName); protected abstract String getSIDQuery(); @@ -1370,7 +1388,7 @@ public Integer getSPID(Connection conn) throws SQLException public boolean updateStatistics(TableInfo table) { - String sql = getAnalyzeCommandForTable(table.getSelectName()); + SQLFragment sql = getAnalyzeCommandForTable(table.getSelectName()); if (sql != null) { new SqlExecutor(table.getSchema()).execute(sql); diff --git a/api/src/org/labkey/api/migration/DatabaseMigrationService.java b/api/src/org/labkey/api/migration/DatabaseMigrationService.java index b5ec00fac29..3543f5565cb 100644 --- a/api/src/org/labkey/api/migration/DatabaseMigrationService.java +++ b/api/src/org/labkey/api/migration/DatabaseMigrationService.java @@ -74,7 +74,7 @@ static void addDataFilter(String filterName, List dataFilters, Set df.column().equals(column) && df.condition().equals(clause)) .findFirst() diff --git a/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java b/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java index e54fe4953a0..36fb4089026 100644 --- a/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java +++ b/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java @@ -275,7 +275,7 @@ protected InClauseGenerator getTempTableInClauseGenerator(DbScope sourceScope) } private static final Set SEEN = new HashSet<>(); - private static final JobRunner ATTACHMENT_JOB_RUNNER = new JobRunner("Attachment JobRunner", 1); + private static final JobRunner ATTACHMENT_JOB_RUNNER = new JobRunner("Attachment JobRunner", 1, () -> "Attachments"); // Copy all core.Documents rows that match the provided filter clause protected final void copyAttachments(DatabaseMigrationConfiguration configuration, DbSchema sourceSchema, FilterClause filterClause, AttachmentType... type) @@ -309,12 +309,12 @@ public static void afterMigration() throws InterruptedException throw new ConfigurationException("These AttachmentTypes have not been seen: " + unseen.stream().map(type -> type.getClass().getSimpleName()).collect(Collectors.joining(", "))); // Shut down the attachment JobRunner - LOG.info("Waiting for core.Documents background transfer to complete"); + LOG.info("Waiting for attachments background transfer to complete"); ATTACHMENT_JOB_RUNNER.shutdown(); if (ATTACHMENT_JOB_RUNNER.awaitTermination(1, TimeUnit.HOURS)) - LOG.info("core.Documents background transfer is complete"); + LOG.info("Attachments background transfer is complete"); else - LOG.error("core.Documents background transfer did not complete after one hour! Giving up."); + LOG.error("Attachments background transfer did not complete after one hour! Giving up."); } @Override diff --git a/api/src/org/labkey/api/util/JobRunner.java b/api/src/org/labkey/api/util/JobRunner.java index 9b1998dbc58..7b478856fb1 100644 --- a/api/src/org/labkey/api/util/JobRunner.java +++ b/api/src/org/labkey/api/util/JobRunner.java @@ -16,10 +16,11 @@ package org.labkey.api.util; -import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.labkey.api.data.DbScope; +import org.labkey.api.util.logging.LogHelper; import java.util.HashMap; import java.util.Map; @@ -30,6 +31,7 @@ import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; /** * This is a simple Executor, that can be used to implement more advanced services @@ -48,7 +50,7 @@ */ public class JobRunner implements Executor { - static final Logger _log = LogManager.getLogger(JobRunner.class); + static final Logger _log = LogHelper.getLogger(JobRunner.class, "JobRunner status and errors"); private static final JobRunner _defaultJobRunner = new JobRunner("Default", 1); @@ -58,14 +60,18 @@ public class JobRunner implements Executor public JobRunner(String name, int max) { - this(name, max, Thread.MIN_PRIORITY); + this(name, max, null); } + public JobRunner(String name, int max, @Nullable Supplier threadNameFactory) + { + this(name, max, threadNameFactory, Thread.MIN_PRIORITY); + } - private JobRunner(String name, int max, int priority) + private JobRunner(String name, int max, @Nullable Supplier threadNameFactory, int priority) { _executor = new JobThreadPoolExecutor(max); - _executor.setThreadFactory(new JobThreadFactory(priority)); + _executor.setThreadFactory(new JobThreadFactory(threadNameFactory, priority)); ContextListener.addShutdownListener(new ShutdownListener() { @Override @@ -263,25 +269,26 @@ protected void afterExecute(Runnable r, Throwable t) } - static class JobThreadFactory implements ThreadFactory + private static class JobThreadFactory implements ThreadFactory { - static final AtomicInteger poolNumber = new AtomicInteger(1); - final ThreadGroup group; - final AtomicInteger threadNumber = new AtomicInteger(1); - final String namePrefix; - final int priority; + private static final AtomicInteger POOL_NUMBER = new AtomicInteger(1); + + private final ThreadGroup _group; + private final AtomicInteger _threadNumber = new AtomicInteger(1); + private final Supplier _threadNameFactory; + private final int priority; - JobThreadFactory(int priority) + JobThreadFactory(@Nullable Supplier threadNameFactory, int priority) { - group = Thread.currentThread().getThreadGroup(); - namePrefix = "JobThread-" + poolNumber.getAndIncrement() + "."; + _group = Thread.currentThread().getThreadGroup(); + _threadNameFactory = threadNameFactory != null ? threadNameFactory : () -> "JobThread-" + POOL_NUMBER.getAndIncrement() + "." + _threadNumber.getAndIncrement(); this.priority = priority; } @Override public Thread newThread(@NotNull Runnable r) { - Thread t = new Thread(group, r, namePrefix + threadNumber.getAndIncrement(), 0); + Thread t = new Thread(_group, r, _threadNameFactory.get(), 0); if (t.isDaemon()) t.setDaemon(false); if (t.getPriority() != priority) diff --git a/core/src/org/labkey/core/CoreMigrationSchemaHandler.java b/core/src/org/labkey/core/CoreMigrationSchemaHandler.java index 2e42998c397..3387000a510 100644 --- a/core/src/org/labkey/core/CoreMigrationSchemaHandler.java +++ b/core/src/org/labkey/core/CoreMigrationSchemaHandler.java @@ -198,10 +198,11 @@ public void copyAttachments(DatabaseMigrationConfiguration configuration, DbSche // Default handling for core's standard attachment types super.copyAttachments(configuration, sourceSchema, targetSchema, copyContainers); - // Special handling for LookAndFeelResourceType, which must select from the source database + // Special handling for LookAndFeelResourceType, which must select from the source database. Keep in sync with + // LookAndFeelResourceType.addWhereSql(). SQLFragment sql = new SQLFragment() .append("Parent").appendInClause(copyContainers, sourceSchema.getSqlDialect()) - .append("AND (DocumentName IN (?, ?) OR ") + .append(" AND (DocumentName IN (?, ?) OR ") .add(AttachmentCache.FAVICON_FILE_NAME) .add(AttachmentCache.STYLESHEET_FILE_NAME) .append("DocumentName LIKE '" + AttachmentCache.LOGO_FILE_NAME_PREFIX + "%' OR ") diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index 23391a9f3cb..5461566888c 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -244,7 +244,6 @@ import org.labkey.core.analytics.AnalyticsServiceImpl; import org.labkey.core.attachment.AttachmentServiceImpl; import org.labkey.core.dialect.PostgreSqlDialectFactory; -import org.labkey.core.dialect.PostgreSqlInClauseTest; import org.labkey.core.dialect.PostgreSqlVersion; import org.labkey.core.junit.JunitController; import org.labkey.core.login.DbLoginAuthenticationProvider; @@ -537,11 +536,11 @@ public QuerySchema createSchema(DefaultSchema schema, Module module) false); OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag(TabLoader.FEATUREFLAG_UNESCAPE_BACKSLASH, "Unescape backslash character on import", - "Treat backslash '\\' character as an escape character when loading data from file.", + "Treat backslash '\\' character as an escape character when loading data from file. This option will be removed in LabKey Server v26.3.", false, false, OptionalFeatureService.FeatureType.Deprecated )); OptionalFeatureService.get().addFeatureFlag(new OptionalFeatureFlag(AppProps.GENERATE_CONTROLLER_FIRST_URLS, - "Restore controller-first URLS", + "Restore controller-first URLs", "Generate URLs in a legacy format that puts the controller name before the folder path. This option will be removed in LabKey Server 26.3.", false, false, OptionalFeatureService.FeatureType.Deprecated )); @@ -1440,7 +1439,6 @@ public TabDisplayMode getTabDisplayMode() ModuleStaticResolverImpl.TestCase.class, NotificationServiceImpl.TestCase.class, PortalJUnitTest.class, - PostgreSqlInClauseTest.class, ProductRegistry.TestCase.class, RadeoxRenderer.RadeoxRenderTest.class, RhinoService.TestCase.class, diff --git a/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java b/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java index 8e171ab8689..cb351949890 100644 --- a/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java +++ b/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java @@ -15,13 +15,15 @@ */ package org.labkey.core.dialect; -import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; import org.jetbrains.annotations.NotNull; import org.labkey.api.data.DbScope; +import org.labkey.api.data.InClauseGenerator; import org.labkey.api.data.ParameterMarkerInClauseGenerator; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SqlSelector; import org.labkey.api.data.TableInfo; +import org.labkey.api.data.TempTableInClauseGenerator; import org.labkey.api.data.dialect.BasePostgreSqlDialect; import org.labkey.api.data.dialect.DialectStringHandler; import org.labkey.api.data.dialect.JdbcHelper; @@ -53,6 +55,16 @@ abstract class PostgreSql92Dialect extends BasePostgreSqlDialect // each database. private int _maxIdentifierByteLength = 63; + private InClauseGenerator _inClauseGenerator; + private final TempTableInClauseGenerator _tempTableInClauseGenerator = new TempTableInClauseGenerator(); + + @Override + public String prepare(DbScope scope) + { + initializeInClauseGenerator(scope); + return super.prepare(scope); + } + @NotNull @Override protected Set getReservedWords() @@ -127,19 +139,36 @@ public Collection getScriptWarnings(String name, String sql) // Split statements by semicolon and CRLF for (String statement : noComments.split(";[\\n\\r]+")) { - if (StringUtils.startsWithIgnoreCase(statement.trim(), "SET ")) + if (Strings.CI.startsWith(statement.trim(), "SET ")) warnings.add(statement); } return warnings; } - @Override - protected void initializeInClauseGenerator(DbScope scope) + private void initializeInClauseGenerator(DbScope scope) { _inClauseGenerator = getJdbcVersion(scope) >= 4 ? new ArrayParameterInClauseGenerator(scope) : new ParameterMarkerInClauseGenerator(); } + @Override + public SQLFragment getAnalyzeCommandForTable(String tableName) + { + return new SQLFragment("ANALYZE ").appendIdentifier(tableName); + } + + @Override + public InClauseGenerator getDefaultInClauseGenerator() + { + return _inClauseGenerator; + } + + @Override + public TempTableInClauseGenerator getTempTableInClauseGenerator() + { + return _tempTableInClauseGenerator; + } + @Override public void addAdminWarningMessages(Warnings warnings, boolean showAllWarnings) { diff --git a/core/src/org/labkey/core/dialect/PostgreSqlInClauseTest.java b/core/src/org/labkey/core/dialect/PostgreSqlInClauseTest.java deleted file mode 100644 index adb2da26498..00000000000 --- a/core/src/org/labkey/core/dialect/PostgreSqlInClauseTest.java +++ /dev/null @@ -1,59 +0,0 @@ -package org.labkey.core.dialect; - -import org.junit.Assert; -import org.junit.Test; -import org.labkey.api.data.CoreSchema; -import org.labkey.api.data.DbSchema; -import org.labkey.api.data.SQLFragment; -import org.labkey.api.data.SqlSelector; -import org.labkey.api.data.dialect.BasePostgreSqlDialect; -import org.labkey.api.data.dialect.SqlDialect; - -import java.util.ArrayList; -import java.util.Arrays; - -/* - TestCase migrated from PostgreSql91Dialect when that class promoted to api. - */ -public class PostgreSqlInClauseTest extends Assert -{ - private PostgreSql_11_Dialect getDialect() - { - DbSchema core = CoreSchema.getInstance().getSchema(); - SqlDialect d = core.getSqlDialect(); - if (d instanceof PostgreSql_11_Dialect) - return (PostgreSql_11_Dialect) d; - return null; - } - - @Test - public void testInClause() - { - PostgreSql_11_Dialect d = getDialect(); - if (null == d) - return; - DbSchema core = CoreSchema.getInstance().getSchema(); - - SQLFragment shortSql = new SQLFragment("SELECT COUNT(*) FROM core.usersdata WHERE userid "); - d.appendInClauseSql(shortSql, Arrays.asList(1, 2, 3)); - assertEquals(1, new SqlSelector(core, shortSql).getRowCount()); - - ArrayList l = new ArrayList<>(); - for (int i = 1; i <= BasePostgreSqlDialect.TEMPTABLE_GENERATOR_MINSIZE + 1; i++) - l.add(i); - SQLFragment longSql = new SQLFragment("SELECT COUNT(*) FROM core.usersdata WHERE userid "); - d.appendInClauseSql(longSql, l); - assertEquals(1, new SqlSelector(core, longSql).getRowCount()); - - SQLFragment shortSqlStr = new SQLFragment("SELECT COUNT(*) FROM core.usersdata WHERE displayname "); - d.appendInClauseSql(shortSqlStr, Arrays.asList("1", "2", "3")); - assertEquals(1, new SqlSelector(core, shortSqlStr).getRowCount()); - - l = new ArrayList<>(); - for (int i = 1; i <= BasePostgreSqlDialect.TEMPTABLE_GENERATOR_MINSIZE + 1; i++) - l.add(String.valueOf(i)); - SQLFragment longSqlStr = new SQLFragment("SELECT COUNT(*) FROM core.usersdata WHERE displayname "); - d.appendInClauseSql(longSqlStr, l); - assertEquals(1, new SqlSelector(core, longSqlStr).getRowCount()); - } -} diff --git a/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java index 75ade1977e5..3a1bfb1d6bb 100644 --- a/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java @@ -5,10 +5,10 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.attachments.AttachmentType; import org.labkey.api.collections.CsvSet; +import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.CompareType; import org.labkey.api.data.CompareType.CompareClause; import org.labkey.api.data.DbSchema; -import org.labkey.api.data.DbSchemaType; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.SimpleFilter.AndClause; @@ -20,6 +20,7 @@ import org.labkey.api.data.SqlExecutor; import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; +import org.labkey.api.data.dialect.SqlDialect; import org.labkey.api.exp.OntologyManager; import org.labkey.api.exp.api.ExpProtocolAttachmentType; import org.labkey.api.exp.api.ExpRunAttachmentType; @@ -33,8 +34,8 @@ import org.labkey.experiment.api.ExperimentServiceImpl; import java.util.Collection; -import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Set; class ExperimentMigrationSchemaHandler extends DefaultMigrationSchemaHandler @@ -86,104 +87,134 @@ public FilterClause getTableFilterClause(TableInfo sourceTable, Set contai FilterClause containerClause = getContainerClause(sourceTable, containers); return switch (sourceTable.getName()) { - case "ExperimentRun" -> getExcludedExperimentRunsFilter(sourceTable, containerClause, FieldKey.fromParts("RowId"), false); - case "ProtocolApplication" -> getExcludedExperimentRunsFilter(sourceTable, containerClause, FieldKey.fromParts("RunId"), false); - case "Data", "Edge" -> getExcludedExperimentRunsFilter(sourceTable, containerClause, FieldKey.fromParts("RunId"), true); - case "DataInput", "MaterialInput" -> getExcludedExperimentRunsFilter(sourceTable, containerClause, FieldKey.fromParts("TargetApplicationId", "RunId"), false); - case "RunList" -> getExcludedExperimentRunsFilter(sourceTable, containerClause, FieldKey.fromParts("ExperimentRunId"), false); - case "DataAncestors" -> getExcludedExperimentRunsFilter(sourceTable, containerClause, FieldKey.fromParts("RowId", "RunId"), true); + case "ExperimentRun" -> { + createIncludedExperimentRunRowIdCollection(sourceTable, containerClause); + yield getIncludedExperimentRunFilter(sourceTable, containerClause, FieldKey.fromParts("RowId"), false); + } + case "ProtocolApplication" -> getIncludedExperimentRunFilter(sourceTable, containerClause, FieldKey.fromParts("RunId"), false); + case "Data", "Edge" -> getIncludedExperimentRunFilter(sourceTable, containerClause, FieldKey.fromParts("RunId"), true); + case "DataInput", "MaterialInput" -> getIncludedExperimentRunFilter(sourceTable, containerClause, FieldKey.fromParts("TargetApplicationId", "RunId"), false); + case "RunList" -> getIncludedExperimentRunFilter(sourceTable, containerClause, FieldKey.fromParts("ExperimentRunId"), false); + case "DataAncestors" -> getIncludedExperimentRunFilter(sourceTable, containerClause, FieldKey.fromParts("RowId", "RunId"), true); default -> containerClause; }; } - // Combine the full container clause with the assay experiment run exclusion filter, if present - private FilterClause getExcludedExperimentRunsFilter(TableInfo sourceTable, FilterClause containerClause, FieldKey runIdFieldKey, boolean nullable) + // Combine the full container clause with the assay experiment run inclusion filter, if present + private FilterClause getIncludedExperimentRunFilter(TableInfo sourceTable, FilterClause containerClause, FieldKey runIdFieldKey, boolean nullable) { - FilterClause excludedRowIdClause = getExcludedRowIdClause(sourceTable, runIdFieldKey); + FilterClause includedRowIdClause = getIncludedRowIdClause(sourceTable, runIdFieldKey); - return excludedRowIdClause == null ? + return includedRowIdClause == null ? containerClause : new AndClause( containerClause, nullable ? new OrClause( new CompareClause(runIdFieldKey, CompareType.ISBLANK, null), - excludedRowIdClause + includedRowIdClause ) : - excludedRowIdClause + includedRowIdClause ); } - @Nullable FilterClause getExcludedRowIdClause(TableInfo sourceTable, FieldKey runIdFieldKey) + @Nullable FilterClause getIncludedRowIdClause(TableInfo sourceTable, FieldKey runIdFieldKey) { - Collection experimentRunsToExclude = getExcludedExperimentRunRowIds(sourceTable.getSchema()); + Collection experimentRunsToInclude = getIncludedExperimentRunRowIds(); + + if (null == experimentRunsToInclude) + return null; - return experimentRunsToExclude.isEmpty() ? - null : - new NotClause(new InClause(runIdFieldKey, experimentRunsToExclude, getTempTableInClauseGenerator(sourceTable.getSchema().getScope()))); + return new InClause(runIdFieldKey, experimentRunsToInclude, getTempTableInClauseGenerator(sourceTable.getSchema().getScope())) + { + @Override + public SQLFragment toSQLFragment(Map columnMap, SqlDialect dialect) + { + // Hackfest: turn temp-table IN clause into the equivalent EXISTS() clause because performance on PostgreSQL is so much better + SQLFragment fragment = super.toSQLFragment(columnMap, dialect); + String sql = fragment.getSQL(); + int idx = sql.indexOf("temp.InClause$"); + if (idx > -1) + { + SQLFragment newFragment = new SQLFragment("EXISTS (SELECT 1 FROM ") + .append(sql.substring(idx, idx + 46)) + .append(" WHERE ") + .append(sql.substring(1, sql.indexOf(" IN "))) + .append(" = Id)"); + newFragment.addAll(fragment.getParams()); + newFragment.addTempTokens(fragment); + fragment = newFragment; + } + return fragment; + } + }; } - private Collection _excludedExperimentRunRowIds = null; + // Collection of all exp.ExperimentRun RowIds in all copy containers that should be copied. If an AssaySkipFilter is + // provided, this collection excludes assay runs in those containers plus runs that list one of the excluded runs + // as a replacement, etc. A null value means include all experiment runs in all copy containers. + private Collection _includedExperimentRunRowIds = null; - private @NotNull Collection getExcludedExperimentRunRowIds(DbSchema schema) + private @Nullable Collection getIncludedExperimentRunRowIds() { - if (null == _excludedExperimentRunRowIds) + return _includedExperimentRunRowIds; + } + + private void createIncludedExperimentRunRowIdCollection(TableInfo sourceExperimentRunsTable, FilterClause containerClause) + { + if (AssaySkipContainers.getContainers().isEmpty()) { - if (AssaySkipContainers.getContainers().isEmpty()) - { - _excludedExperimentRunRowIds = Collections.emptyList(); - } - else - { - // We need the source exp schema; if it wasn't passed in, retrieve it from the scope. - DbSchema expSchema = "exp".equals(schema.getName()) ? schema : schema.getScope().getSchema("exp", DbSchemaType.Migration); + _includedExperimentRunRowIds = null; + } + else + { + DbSchema sourceSchema = sourceExperimentRunsTable.getSchema(); - // Select all the assay runs (same filter used by assay.AssayRuns) - SQLFragment assayRunSql = new SQLFragment( - "ProtocolLSID IN (SELECT LSID FROM exp.Protocol x WHERE (ApplicationType = 'ExperimentRun') AND " + - "((SELECT MAX(pd.PropertyId) from exp.Object o, exp.ObjectProperty op, exp.PropertyDescriptor pd WHERE " + - "pd.PropertyId = op.PropertyId and op.ObjectId = o.ObjectId and o.ObjectURI = LSID AND pd.PropertyURI LIKE '%AssayDomain-Run%') IS NOT NULL))" - ); + // Selects all assay runs (same filter used by assay.AssayRuns) + SQLFragment assayRunSql = new SQLFragment( + "ProtocolLSID IN (SELECT LSID FROM exp.Protocol x WHERE (ApplicationType = 'ExperimentRun') AND " + + "((SELECT MAX(pd.PropertyId) from exp.Object o, exp.ObjectProperty op, exp.PropertyDescriptor pd WHERE " + + "pd.PropertyId = op.PropertyId and op.ObjectId = o.ObjectId and o.ObjectURI = LSID AND pd.PropertyURI LIKE '%AssayDomain-Run%') IS NOT NULL))" + ); - // Select all assay runs in the assay-skip containers - FilterClause assayRunClause = new AndClause( - new InClause(FieldKey.fromParts("Container"), AssaySkipContainers.getContainers()), - new SQLClause(assayRunSql) - ); + // Selects all assay runs in the configured assay-skip containers + FilterClause assayRunClause = new AndClause( + new InClause(FieldKey.fromParts("Container"), AssaySkipContainers.getContainers()), + new SQLClause(assayRunSql) + ); - // Select assay runs (regardless of their container) that were replaced by assay runs that are being - // excluded - FilterClause replaceByRunIdClause = new SQLClause( - new SQLFragment("ReplacedByRunId IN (SELECT RowId FROM exp.ExperimentRun WHERE ") - .append(assayRunClause.toSQLFragment(null, expSchema.getSqlDialect())) - .append(")") - ); + // Selects assay runs (regardless of their container) that were replaced by assay runs that are being + // excluded + FilterClause replaceByRunIdClause = new SQLClause( + new SQLFragment("ReplacedByRunId IS NOT NULL AND ReplacedByRunId IN (SELECT RowId FROM exp.ExperimentRun WHERE ") + .append(assayRunClause.toSQLFragment(null, sourceSchema.getSqlDialect())) + .append(")") + ); - // Select assay runs that were replaced by assay runs that are being excluded because they were replaced - // by an excluded assay run. Yes, we actually have to do this... - FilterClause replaceByReplacedRunIdClause = new SQLClause( - new SQLFragment("ReplacedByRunId IN (SELECT RowId FROM exp.ExperimentRun WHERE ") - .append(replaceByRunIdClause.toSQLFragment(null, expSchema.getSqlDialect())) - .append(")") - ); + // Selects assay runs that were replaced by assay runs that are being excluded because they were replaced + // by an excluded assay run. Yes, we actually have to do this... + FilterClause replaceByReplacedRunIdClause = new SQLClause( + new SQLFragment("ReplacedByRunId IS NOT NULL AND ReplacedByRunId IN (SELECT RowId FROM exp.ExperimentRun WHERE ") + .append(replaceByRunIdClause.toSQLFragment(null, sourceSchema.getSqlDialect())) + .append(")") + ); - // Select all assay runs that need to be excluded - SimpleFilter filter = new SimpleFilter( - new OrClause( - assayRunClause, - replaceByRunIdClause, - replaceByReplacedRunIdClause - ) - ); + // Selects all assay runs that need to be included -- all rows in exp.ExperimentRuns in all copy containers, + // except assay runs in the assay-skip containers or runs replaced by one of those excluded runs. + SimpleFilter filter = new SimpleFilter( + containerClause, + new NotClause(new OrClause( + assayRunClause, + replaceByRunIdClause, + replaceByReplacedRunIdClause + )) + ); - // Select the excluded assay experiment run RowIds. All tables with FKs to ExperimentRun (or FKs to - // other tables with FKs to ExperimentRun) must exclude these run IDs. - _excludedExperimentRunRowIds = new TableSelector(expSchema.getTable("ExperimentRun"), new CsvSet("RowId, ProtocolLSID, ReplacedByRunId"), filter, null).getCollection(Integer.class); - LOG.info(" {} being excluded due to the configured AssaySkipContainers parameter", StringUtilsLabKey.pluralize(_excludedExperimentRunRowIds.size(), "assay experiment run is", "assay experiment runs are")); - } + // Select the experiment run RowIds to transfer. All tables with FKs to ExperimentRun (or FKs to other + // tables with FKs to ExperimentRun) must add these run IDs as an include filter to avoid FK violations. + _includedExperimentRunRowIds = new TableSelector(sourceExperimentRunsTable, new CsvSet("RowId, ProtocolLSID, ReplacedByRunId"), filter, null).getCollection(Integer.class); + LOG.info(" {} being included due to the configured AssaySkipContainers parameter", StringUtilsLabKey.pluralize(_includedExperimentRunRowIds.size(), "assay experiment run is", "assay experiment runs are")); } - - return _excludedExperimentRunRowIds; } @Override diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 592f80e9d7b..333f1f28b7c 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -890,10 +890,10 @@ public TableInfo getTableInfo() @Override public void adjustFilter(TableInfo sourceTable, SimpleFilter filter, Set containers) { - // Exclude assay experiment runs that weren't copied - FilterClause excludedClause = handler.getExcludedRowIdClause(sourceTable, FieldKey.fromParts("RunId")); - if (excludedClause != null) - filter.addClause(excludedClause); + // Include experiment runs that were copied + FilterClause includedClause = handler.getIncludedRowIdClause(sourceTable, FieldKey.fromParts("RunId")); + if (includedClause != null) + filter.addClause(includedClause); } }); DatabaseMigrationService.get().registerTableHandler(new MigrationTableHandler() @@ -907,10 +907,10 @@ public TableInfo getTableInfo() @Override public void adjustFilter(TableInfo sourceTable, SimpleFilter filter, Set containers) { - // Exclude assay experiment runs that weren't copied - FilterClause excludedClause = handler.getExcludedRowIdClause(sourceTable, FieldKey.fromParts("ExclusionId", "RunId")); - if (excludedClause != null) - filter.addClause(excludedClause); + // Include experiment runs that were copied + FilterClause includedClause = handler.getIncludedRowIdClause(sourceTable, FieldKey.fromParts("ExclusionId", "RunId")); + if (includedClause != null) + filter.addClause(includedClause); } }); DatabaseMigrationService.get().registerTableHandler(new MigrationTableHandler() @@ -924,10 +924,10 @@ public TableInfo getTableInfo() @Override public void adjustFilter(TableInfo sourceTable, SimpleFilter filter, Set containers) { - // Exclude assay experiment runs that weren't copied - FilterClause excludedClause = handler.getExcludedRowIdClause(sourceTable, FieldKey.fromParts("RunId")); - if (excludedClause != null) - filter.addClause(excludedClause); + // Include experiment runs that were copied + FilterClause includedClause = handler.getIncludedRowIdClause(sourceTable, FieldKey.fromParts("RunId")); + if (includedClause != null) + filter.addClause(includedClause); } }); DatabaseMigrationService.get().registerSchemaHandler(new SampleTypeMigrationSchemaHandler()); From 2893e15d5ec70a528d072a590ad258425faabef9 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 3 Dec 2025 14:06:41 -0800 Subject: [PATCH 03/12] Update PostgreSQL version support (#7241) --- core/src/org/labkey/core/dialect/PostgreSql92Dialect.java | 1 - .../org/labkey/core/dialect/PostgreSqlDialectFactory.java | 5 +++-- core/src/org/labkey/core/dialect/PostgreSqlVersion.java | 8 ++++++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java b/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java index cb351949890..f9de6d15401 100644 --- a/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java +++ b/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java @@ -48,7 +48,6 @@ abstract class PostgreSql92Dialect extends BasePostgreSqlDialect { public static final String PRODUCT_NAME = "PostgreSQL"; - public static final String RECOMMENDED = PRODUCT_NAME + " 17.x is the recommended version."; // This has been the standard PostgreSQL identifier max byte length for many years. However, this could change in // the future plus servers can be compiled with a different limit, so we query this setting on first connection to diff --git a/core/src/org/labkey/core/dialect/PostgreSqlDialectFactory.java b/core/src/org/labkey/core/dialect/PostgreSqlDialectFactory.java index 8775fecbce5..fdf9b7a75ee 100644 --- a/core/src/org/labkey/core/dialect/PostgreSqlDialectFactory.java +++ b/core/src/org/labkey/core/dialect/PostgreSqlDialectFactory.java @@ -17,6 +17,7 @@ package org.labkey.core.dialect; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -65,7 +66,7 @@ public PostgreSqlDialectFactory() @Override public @Nullable SqlDialect createFromMetadata(DatabaseMetaData md, boolean logWarnings, boolean primaryDataSource) throws SQLException, DatabaseNotSupportedException { - if (!(StringUtils.startsWithIgnoreCase(md.getURL(), JDBC_PREFIX))) + if (!(Strings.CI.startsWith(md.getURL(), JDBC_PREFIX))) return null; String databaseProductVersion = md.getDatabaseProductVersion(); @@ -115,7 +116,7 @@ else if (psv.isDeprecated()) public static String getStandardWarningMessage(String warning, String databaseProductVersion) { - return "LabKey Server " + warning + " " + PostgreSql92Dialect.PRODUCT_NAME + " version " + databaseProductVersion + ". " + PostgreSql92Dialect.RECOMMENDED; + return "LabKey Server " + warning + " " + PostgreSql92Dialect.PRODUCT_NAME + " version " + databaseProductVersion + ". " + PostgreSqlVersion.RECOMMENDED; } @Override diff --git a/core/src/org/labkey/core/dialect/PostgreSqlVersion.java b/core/src/org/labkey/core/dialect/PostgreSqlVersion.java index 9b87e657611..7fb9bd0bf9e 100644 --- a/core/src/org/labkey/core/dialect/PostgreSqlVersion.java +++ b/core/src/org/labkey/core/dialect/PostgreSqlVersion.java @@ -10,20 +10,24 @@ import java.util.function.Supplier; import java.util.stream.Collectors; +import static org.labkey.core.dialect.PostgreSql92Dialect.PRODUCT_NAME; + /** * Enum that specifies the versions of PostgreSQL that LabKey supports plus their properties */ public enum PostgreSqlVersion { POSTGRESQL_UNSUPPORTED(-1, true, false, null), - POSTGRESQL_13(130, false, true, PostgreSql_13_Dialect::new), + POSTGRESQL_13(130, true, true, PostgreSql_13_Dialect::new), POSTGRESQL_14(140, false, true, PostgreSql_14_Dialect::new), POSTGRESQL_15(150, false, true, PostgreSql_15_Dialect::new), POSTGRESQL_16(160, false, true, PostgreSql_16_Dialect::new), POSTGRESQL_17(170, false, true, PostgreSql_17_Dialect::new), - POSTGRESQL_18(180, false, false, PostgreSql_18_Dialect::new), + POSTGRESQL_18(180, false, true, PostgreSql_18_Dialect::new), POSTGRESQL_FUTURE(Integer.MAX_VALUE, true, false, PostgreSql_18_Dialect::new); + public static final String RECOMMENDED = PRODUCT_NAME + " 18.x is the recommended version."; + private final int _version; private final boolean _deprecated; private final boolean _tested; From 1f7f37bf95e47db7efc43db862a1bbaa6785387e Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Thu, 4 Dec 2025 13:27:17 -0800 Subject: [PATCH 04/12] Avoid error logging on bad SQL parameter values (#7233) --- .../api/data/SQLParameterException.java | 29 +++++++++++++++++++ .../labkey/api/data/SqlExecutingSelector.java | 4 ++- api/src/org/labkey/api/data/Table.java | 10 ++++--- .../org/labkey/api/data/TableSelector.java | 2 +- .../org/labkey/api/util/ExceptionUtil.java | 8 +++++ .../api/util/SkipMothershipLogging.java | 10 ++----- .../org/labkey/query/QueryServiceImpl.java | 10 +++++-- 7 files changed, 57 insertions(+), 16 deletions(-) create mode 100644 api/src/org/labkey/api/data/SQLParameterException.java diff --git a/api/src/org/labkey/api/data/SQLParameterException.java b/api/src/org/labkey/api/data/SQLParameterException.java new file mode 100644 index 00000000000..0d13252128a --- /dev/null +++ b/api/src/org/labkey/api/data/SQLParameterException.java @@ -0,0 +1,29 @@ +/* + * Copyright (c) 2011 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.data; + +import org.labkey.api.util.SkipMothershipLogging; + +/** + * Signals there was a problem with a SQL parameter, such as a conversion problem. + */ +public class SQLParameterException extends SQLGenerationException implements SkipMothershipLogging +{ + public SQLParameterException(String message) + { + super(message); + } +} diff --git a/api/src/org/labkey/api/data/SqlExecutingSelector.java b/api/src/org/labkey/api/data/SqlExecutingSelector.java index aeadad8bf99..71245935a1f 100644 --- a/api/src/org/labkey/api/data/SqlExecutingSelector.java +++ b/api/src/org/labkey/api/data/SqlExecutingSelector.java @@ -567,7 +567,9 @@ public void handleSqlException(SQLException e, @Nullable Connection conn) if (null != _sql) ExceptionUtil.decorateException(e, ExceptionUtil.ExceptionInfo.DialectSQL, _sql.toDebugString(), false); - throw getExceptionFramework().translate(getScope(), "ExecutingSelector", e); + RuntimeException translated = getExceptionFramework().translate(getScope(), "ExecutingSelector", e); + ExceptionUtil.copyDecorations(e, translated); + throw translated; } } } diff --git a/api/src/org/labkey/api/data/Table.java b/api/src/org/labkey/api/data/Table.java index e9bf16703fc..86962b4d295 100644 --- a/api/src/org/labkey/api/data/Table.java +++ b/api/src/org/labkey/api/data/Table.java @@ -19,6 +19,7 @@ import org.apache.commons.beanutils.BeanUtils; import org.apache.commons.beanutils.PropertyUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; @@ -50,6 +51,7 @@ import org.labkey.api.query.RuntimeValidationException; import org.labkey.api.security.User; import org.labkey.api.util.BaseScanner; +import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.GUID; import org.labkey.api.util.JunitUtil; import org.labkey.api.util.MemTracker; @@ -571,7 +573,7 @@ Object getObject(ResultSet rs) throws SQLException // Standard SQLException catch block: log exception, query SQL, and params static void logException(@Nullable SQLFragment sql, @Nullable Connection conn, SQLException e, Level logLevel) { - if (SqlDialect.isCancelException(e)) + if (SqlDialect.isCancelException(e) || ExceptionUtil.isIgnorable(e)) { return; } @@ -585,7 +587,7 @@ static void logException(@Nullable SQLFragment sql, @Nullable Connection conn, S String trim = sql.getSQL().trim(); // Treat a ConstraintException during INSERT/UPDATE as a WARNING. Treat all other SQLExceptions as an ERROR. - if (RuntimeSQLException.isConstraintException(e) && (StringUtils.startsWithIgnoreCase(trim, "INSERT") || StringUtils.startsWithIgnoreCase(trim, "UPDATE"))) + if (RuntimeSQLException.isConstraintException(e) && (Strings.CI.startsWith(trim, "INSERT") || Strings.CI.startsWith(trim, "UPDATE"))) { // Log this ConstraintException if log Level is WARN (the default) or lower. Skip logging for callers that request just ERRORs. if (Level.WARN.isMoreSpecificThan(logLevel)) @@ -1409,7 +1411,7 @@ public void testSelect() throws SQLException //noinspection EmptyTryBlock,UnusedDeclaration try (ResultSet rs = new TableSelector(tinfo).getResultSet()){} - Map[] maps = new TableSelector(tinfo).getMapArray(); + Map[] maps = new TableSelector(tinfo).getMapArray(); assertNotNull(maps); Principal[] principals = new TableSelector(tinfo).getArray(Principal.class); @@ -1739,7 +1741,7 @@ public static ParameterMapStatement deleteStatement(Connection conn, TableInfo t // Domain domain = tableDelete.getDomain(); - DomainKind domainKind = tableDelete.getDomainKind(); + DomainKind domainKind = tableDelete.getDomainKind(); if (null != domain && null != domainKind && StringUtils.isEmpty(domainKind.getStorageSchemaName())) { if (!d.isPostgreSQL() && !d.isSqlServer()) diff --git a/api/src/org/labkey/api/data/TableSelector.java b/api/src/org/labkey/api/data/TableSelector.java index f64a43fb55e..dc2bde07a69 100644 --- a/api/src/org/labkey/api/data/TableSelector.java +++ b/api/src/org/labkey/api/data/TableSelector.java @@ -396,7 +396,7 @@ public Results getResultsAsync(final boolean cache, final boolean scrollable, Ht } /** - * Setting this options asks the TableSelector to add additional display columns to the generated SQL, as well + * Setting this option asks the TableSelector to add additional display columns to the generated SQL, as well * as forcing the results to be sorted. * @return this diff --git a/api/src/org/labkey/api/util/ExceptionUtil.java b/api/src/org/labkey/api/util/ExceptionUtil.java index 3649564a0ba..4e2555cf644 100644 --- a/api/src/org/labkey/api/util/ExceptionUtil.java +++ b/api/src/org/labkey/api/util/ExceptionUtil.java @@ -106,6 +106,14 @@ public class ExceptionUtil */ private static final Map EXCEPTION_TALLIES = Collections.synchronizedMap(new HashMap<>()); + public static void copyDecorations(Throwable source, Throwable target) + { + for (Map.Entry, String> entry : getExceptionDecorations(source).entrySet()) + { + decorateException(target, entry.getKey(), entry.getValue(), false); + } + } + private static class ExceptionTally { /** Total number of times the exception has happened */ diff --git a/api/src/org/labkey/api/util/SkipMothershipLogging.java b/api/src/org/labkey/api/util/SkipMothershipLogging.java index 674e41cb756..414b06d3a5e 100644 --- a/api/src/org/labkey/api/util/SkipMothershipLogging.java +++ b/api/src/org/labkey/api/util/SkipMothershipLogging.java @@ -16,13 +16,9 @@ package org.labkey.api.util; -/* -* User: adam -* Date: Aug 10, 2009 -* Time: 2:03:50 PM -*/ - -// Exceptions implement this interface to tell mothership not to log them +/** + * Exceptions implement this interface as a tag to tell mothership not to log them + */ public interface SkipMothershipLogging { } diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index 009b239df58..0883a1ec48d 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -23,6 +23,7 @@ import org.apache.commons.collections4.MultiValuedMap; import org.apache.commons.collections4.SetValuedMap; 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.apache.xmlbeans.XmlError; @@ -65,7 +66,7 @@ import org.labkey.api.data.ResultsImpl; import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SQLFragment; -import org.labkey.api.data.SQLGenerationException; +import org.labkey.api.data.SQLParameterException; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Sort; import org.labkey.api.data.SqlSelector; @@ -120,6 +121,7 @@ import org.labkey.api.util.CSRFUtil; import org.labkey.api.util.ConfigurationException; import org.labkey.api.util.ContainerContext; +import org.labkey.api.util.ExceptionUtil; import org.labkey.api.util.GUID; import org.labkey.api.util.JunitUtil; import org.labkey.api.util.Pair; @@ -1360,7 +1362,7 @@ public MultiValuedMap load(Stream { // Note: Can't use standard filter (getFilter(suffix)) below since we must allow ".qview.xml" return unmodifiable(resources - .filter(resource -> StringUtils.endsWithIgnoreCase(resource.getName(), CustomViewXmlReader.XML_FILE_EXTENSION)) + .filter(resource -> Strings.CI.endsWith(resource.getName(), CustomViewXmlReader.XML_FILE_EXTENSION)) .map(ModuleCustomViewDef::new) .collect(LabKeyCollectors.toMultiValuedMap(def -> def.getPath().getParent(), def -> def))); } @@ -2683,7 +2685,9 @@ public void bindNamedParameters(SQLFragment frag, @Nullable Map } catch (ConversionException e) { - throw new RuntimeSQLException(new SQLGenerationException(ConvertHelper.getStandardConversionErrorMessage(value, p.getName(), p.getJdbcType().getJavaClass()))); + SQLParameterException paramException = new SQLParameterException(ConvertHelper.getStandardConversionErrorMessage(value, p.getName(), p.getJdbcType().getJavaClass())); + ExceptionUtil.decorateException(paramException, ExceptionUtil.ExceptionInfo.SkipMothershipLogging, "true", true); + throw new RuntimeSQLException(paramException); } } } From e8c888f9c09ab8f6ea6811f3a493a916ad9faf0f Mon Sep 17 00:00:00 2001 From: Karl Lum Date: Fri, 5 Dec 2025 15:12:51 -0800 Subject: [PATCH 05/12] Don't save bulk import data or offer a download link (#7248) --- study/src/org/labkey/study/StudyModule.java | 6 +- .../study/assay/StudyPublishManager.java | 88 ++++--------------- .../study/controllers/StudyController.java | 39 ++------ .../dataset/DatasetSnapshotProvider.java | 6 +- .../labkey/study/model/DatasetDefinition.java | 37 ++++++-- .../org/labkey/study/model/StudyManager.java | 6 +- .../src/org/labkey/study/model/UploadLog.java | 11 --- 7 files changed, 58 insertions(+), 135 deletions(-) diff --git a/study/src/org/labkey/study/StudyModule.java b/study/src/org/labkey/study/StudyModule.java index 8df47d6aa96..69570d36931 100644 --- a/study/src/org/labkey/study/StudyModule.java +++ b/study/src/org/labkey/study/StudyModule.java @@ -42,8 +42,6 @@ import org.labkey.api.exp.PropertyType; import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.property.PropertyService; -import org.labkey.api.files.FileContentService; -import org.labkey.api.files.TableUpdaterFileListener; import org.labkey.api.message.digest.ReportAndDatasetChangeDigestProvider; import org.labkey.api.migration.DatabaseMigrationService; import org.labkey.api.migration.DefaultMigrationSchemaHandler; @@ -228,7 +226,7 @@ public String getName() @Override public Double getSchemaVersion() { - return 25.003; + return 25.004; } @Override @@ -391,8 +389,6 @@ protected void startupAfterSpringConfig(ModuleContext moduleContext) folderRegistry.addFactories(new StudyWriterFactory(), new StudyImporterFactory()); } - FileContentService.get().addFileListener(new TableUpdaterFileListener(StudySchema.getInstance().getTableInfoUploadLog(), "FilePath", TableUpdaterFileListener.Type.filePath, "RowId")); - DatasetDefinition.cleanupOrphanedDatasetDomains(); OptionalFeatureService.get().addExperimentalFeatureFlag(StudyQuerySchema.EXPERIMENTAL_STUDY_SUBSCHEMAS, "Use sub-schemas in Study", diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index d4a2cc2c894..7fb7c09303c 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -18,13 +18,11 @@ import org.apache.commons.beanutils.ConvertUtils; import org.apache.commons.collections4.MapUtils; -import org.apache.commons.io.IOUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.assay.AbstractAssayProvider; -import org.labkey.api.assay.AssayFileWriter; import org.labkey.api.assay.AssayProtocolSchema; import org.labkey.api.assay.AssayProvider; import org.labkey.api.assay.AssayService; @@ -113,9 +111,7 @@ import org.labkey.api.study.publish.StudyDatasetLinkedColumn; import org.labkey.api.study.publish.StudyPublishService; import org.labkey.api.study.query.PublishResultsQueryView; -import org.labkey.api.util.DateUtil; import org.labkey.api.util.FileStream; -import org.labkey.api.util.FileUtil; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; import org.labkey.api.util.StringExpressionFactory; @@ -140,10 +136,7 @@ import java.io.Closeable; import java.io.IOException; -import java.io.OutputStream; import java.math.BigDecimal; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -932,60 +925,10 @@ private static String createUniqueDatasetName(Study study, String assayName) return name; } - public UploadLog saveUploadData(User user, Dataset dsd, FileStream tsv, String filename) throws IOException - { - PipeRoot pipelineRoot = PipelineService.get().findPipelineRoot(dsd.getContainer()); - if (null == pipelineRoot || !pipelineRoot.isValid()) - throw new IOException("Please have your administrator set up a pipeline root for this folder."); - - Path dir = pipelineRoot.resolveToNioPath(AssayFileWriter.DIR_NAME); - if (null == dir) - throw new IOException("Cannot create directory uploaded data: " + AssayFileWriter.DIR_NAME); - - if (!Files.exists(dir)) - { - FileUtil.createDirectory(dir); - } - - //File name is studyname_datasetname_date_hhmm.ss - Date dateCreated = new Date(); - String dateString = DateUtil.formatDateTime(dateCreated, "yyy-MM-dd-HHmm"); - int id = 0; - Path file; - do - { - String extension = Objects.toString(filename == null ? "tsv" : FileUtil.getExtension(filename), "tsv"); - String extra = id++ == 0 ? "" : String.valueOf(id); - String fileName = dsd.getStudy().getLabel() + "-" + dsd.getLabel() + "-" + dateString + extra + "." + extension; - fileName = fileName.replace('\\', '_').replace('/', '_').replace(':', '_'); - file = FileUtil.appendName(dir, fileName); - } - while (Files.exists(file)); - - try (OutputStream out = Files.newOutputStream(file)) - { - IOUtils.copy(tsv.openInputStream(), out); - tsv.closeInputStream(); - } - - UploadLog ul = new UploadLog(); - ul.setContainer(dsd.getContainer()); - ul.setDatasetId(dsd.getDatasetId()); - ul.setCreated(dateCreated); - ul.setUserId(user.getUserId()); - ul.setStatus("Initializing"); - String filePath = FileUtil.hasCloudScheme(file) ? FileUtil.pathToString(file) : file.toFile().getPath(); - ul.setFilePath(filePath); - - return Table.insert(user, getTinfoUpdateLog(), ul); - } - - /** - * Return an array of LSIDs from the newly created dataset entries, - * along with the upload log. + * Return an array of LSIDs from the newly created dataset entries. */ - public Pair, UploadLog> importDatasetTSV(User user, StudyImpl study, DatasetDefinition dsd, DataLoader dl, LookupResolutionType lookupResolutionType, FileStream fileIn, String originalFileName, Map columnMap, BatchValidationException errors, QueryUpdateService.InsertOption insertOption, @Nullable AuditBehaviorType auditBehaviorType) + public List importDatasetTSV(User user, StudyImpl study, DatasetDefinition dsd, DataLoader dl, LookupResolutionType lookupResolutionType, FileStream fileIn, String originalFileName, Map columnMap, BatchValidationException errors, QueryUpdateService.InsertOption insertOption, @Nullable AuditBehaviorType auditBehaviorType) { DbScope scope = StudySchema.getInstance().getScope(); @@ -995,7 +938,13 @@ public Pair, UploadLog> importDatasetTSV(User user, StudyImpl study try { if (null != fileIn) - ul = saveUploadData(user, dsd, fileIn, originalFileName); + { + ul = new UploadLog(); + ul.setContainer(dsd.getContainer()); + ul.setDatasetId(dsd.getDatasetId()); + ul.setUserId(user.getUserId()); + ul.setStatus("Initializing"); + } try (DbScope.Transaction transaction = scope.ensureTransaction()) { @@ -1025,8 +974,9 @@ public Pair, UploadLog> importDatasetTSV(User user, StudyImpl study ul.setStatus("ERROR"); String description = ul.getDescription(); ul.setDescription(description == null ? "" : description + "\n" + new Date() + ":" + x.getMessage()); - ul = Table.update(user, StudySchema.getInstance().getTableInfoUploadLog(), ul, ul.getRowId()); - return Pair.of(lsids, ul); + Table.insert(user, getTinfoUpdateLog(), ul); + + return lsids; } } @@ -1035,7 +985,7 @@ public Pair, UploadLog> importDatasetTSV(User user, StudyImpl study //Update the status assert ul != null : "Upload log should always exist if no errors have occurred."; ul.setStatus("SUCCESS"); - ul = Table.update(user, getTinfoUpdateLog(), ul, ul.getRowId()); + Table.insert(user, getTinfoUpdateLog(), ul); } else if (ul != null) { @@ -1048,17 +998,9 @@ else if (ul != null) sep = "\n"; } ul.setDescription(sb.toString()); - ul = Table.update(user, getTinfoUpdateLog(), ul, ul.getRowId()); + Table.insert(user, getTinfoUpdateLog(), ul); } - return Pair.of(lsids, ul); - } - - public UploadLog getUploadLog(Container c, int id) - { - SimpleFilter filter = SimpleFilter.createContainerFilter(c); - filter.addCondition(FieldKey.fromParts("rowId"), id); - - return new TableSelector(getTinfoUpdateLog(), filter, null).getObject(UploadLog.class); + return lsids; } @Override diff --git a/study/src/org/labkey/study/controllers/StudyController.java b/study/src/org/labkey/study/controllers/StudyController.java index 4cd599b94ee..bb845ca7f24 100644 --- a/study/src/org/labkey/study/controllers/StudyController.java +++ b/study/src/org/labkey/study/controllers/StudyController.java @@ -90,7 +90,6 @@ import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.ShowRows; -import org.labkey.api.data.SimpleDisplayColumn; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Sort; import org.labkey.api.data.SqlExecutor; @@ -227,7 +226,6 @@ import org.labkey.api.view.template.EmptyView; import org.labkey.api.view.template.PageConfig; import org.labkey.api.writer.FileSystemFile; -import org.labkey.api.writer.HtmlWriter; import org.labkey.api.writer.VirtualFile; import org.labkey.data.xml.TablesDocument; import org.labkey.study.CohortFilterFactory; @@ -2712,19 +2710,19 @@ protected int importData(DataLoader dl, FileStream file, String originalName, Ba columnMap.put(_form.getSequenceNum(), column); } - Pair, UploadLog> result = StudyPublishManager.getInstance().importDatasetTSV(getUser(), _study, _def, dl, getLookupResolutionType(), file, originalName, columnMap, errors, _form.getInsertOption(), auditBehaviorType); + List lsids = StudyPublishManager.getInstance().importDatasetTSV(getUser(), _study, _def, dl, getLookupResolutionType(), file, originalName, columnMap, errors, _form.getInsertOption(), auditBehaviorType); - if (!result.getKey().isEmpty()) + if (!lsids.isEmpty()) { // Log the import when SUMMARY is configured, if DETAILED is configured the DetailedAuditLogDataIterator will handle each row change. // It would be nice in the future to replace the DetailedAuditLogDataIterator with a general purpose AuditLogDataIterator // that can delegate the audit behavior type to the AuditDataHandler, so this code can go away // - String comment = "Dataset data imported. " + result.getKey().size() + " rows imported"; - new DatasetDefinition.DatasetAuditHandler(_def).addAuditEvent(getUser(), getContainer(), AuditBehaviorType.SUMMARY, comment, result.getValue()); + String comment = "Dataset data imported. " + lsids.size() + " rows imported"; + new DatasetDefinition.DatasetAuditHandler(_def).addAuditEvent(getUser(), getContainer(), AuditBehaviorType.SUMMARY, comment); } - return result.getKey().size(); + return lsids.size(); } @Override @@ -2855,15 +2853,6 @@ public ModelAndView getView(IdForm form, BindException errors) DataRegion dr = new DataRegion(); dr.addColumns(tInfo, "RowId,Created,CreatedBy,Status,Description"); GridView gv = new GridView(dr, errors); - DisplayColumn dc = new SimpleDisplayColumn(null) { - @Override - public void renderGridCellContents(RenderContext ctx, HtmlWriter out) - { - ActionURL url = new ActionURL(DownloadTsvAction.class, ctx.getContainer()).addParameter("id", String.valueOf(ctx.get("RowId"))); - out.write(LinkBuilder.labkeyLink("Download Data File", url)); - } - }; - dr.addDisplayColumn(dc); SimpleFilter filter = SimpleFilter.createContainerFilter(getContainer()); if (form.getId() != 0) @@ -2885,24 +2874,6 @@ public void addNavTrail(NavTree root) } } - @RequiresPermission(UpdatePermission.class) - public static class DownloadTsvAction extends SimpleViewAction - { - @Override - public ModelAndView getView(IdForm form, BindException errors) throws Exception - { - UploadLog ul = StudyPublishManager.getInstance().getUploadLog(getContainer(), form.getId()); - PageFlowUtil.streamFile(getViewContext().getResponse(), new File(ul.getFilePath()).toPath(), true); - - return null; - } - - @Override - public void addNavTrail(NavTree root) - { - } - } - @RequiresPermission(ReadPermission.class) public static class DatasetItemDetailsAction extends SimpleViewAction { diff --git a/study/src/org/labkey/study/dataset/DatasetSnapshotProvider.java b/study/src/org/labkey/study/dataset/DatasetSnapshotProvider.java index 435c41ae701..a75d69d22a0 100644 --- a/study/src/org/labkey/study/dataset/DatasetSnapshotProvider.java +++ b/study/src/org/labkey/study/dataset/DatasetSnapshotProvider.java @@ -487,7 +487,7 @@ public synchronized ActionURL updateSnapshot(QuerySnapshotForm form, BindExcepti ViewContext context = form.getViewContext(); new DatasetDefinition.DatasetAuditHandler(dsDef).addAuditEvent(context.getUser(), context.getContainer(), AuditBehaviorType.DETAILED, - "Dataset snapshot was updated. " + numRowsDeleted + " rows were removed and replaced with " + newRows.size() + " rows.", null); + "Dataset snapshot was updated. " + numRowsDeleted + " rows were removed and replaced with " + newRows.size() + " rows."); def.setLastUpdated(new Date()); def.save(form.getViewContext().getUser()); @@ -504,7 +504,7 @@ public synchronized ActionURL updateSnapshot(QuerySnapshotForm form, BindExcepti { ViewContext context = form.getViewContext(); new DatasetDefinition.DatasetAuditHandler(dsDef).addAuditEvent(context.getUser(), context.getContainer(), AuditBehaviorType.DETAILED, - "Dataset snapshot was not updated. Cause of failure: " + e.getMessage(), null); + "Dataset snapshot was not updated. Cause of failure: " + e.getMessage()); } } } @@ -791,7 +791,7 @@ public void run() DatasetDefinition dsDef = StudyManager.getInstance().getDatasetDefinitionByName(study, _def.getName()); if (dsDef != null) new DatasetDefinition.DatasetAuditHandler(dsDef).addAuditEvent(context.getUser(), context.getContainer(), AuditBehaviorType.DETAILED, - "Dataset snapshot was not updated. Cause of failure: " + errors.getMessage(), null); + "Dataset snapshot was not updated. Cause of failure: " + errors.getMessage()); } } } diff --git a/study/src/org/labkey/study/model/DatasetDefinition.java b/study/src/org/labkey/study/model/DatasetDefinition.java index 387169c205c..6dc3e2bae93 100644 --- a/study/src/org/labkey/study/model/DatasetDefinition.java +++ b/study/src/org/labkey/study/model/DatasetDefinition.java @@ -36,8 +36,37 @@ import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; import org.labkey.api.collections.Sets; -import org.labkey.api.data.*; +import org.labkey.api.data.AuditConfigurable; +import org.labkey.api.data.BaseColumnInfo; +import org.labkey.api.data.BeanObjectFactory; +import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.ConnectionWrapper; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.DataColumn; +import org.labkey.api.data.DatabaseCache; +import org.labkey.api.data.DatabaseTableType; +import org.labkey.api.data.DbSchema; +import org.labkey.api.data.DbScope; import org.labkey.api.data.DbScope.Transaction; +import org.labkey.api.data.DisplayColumn; +import org.labkey.api.data.DisplayColumnFactory; +import org.labkey.api.data.ExceptionFramework; +import org.labkey.api.data.JdbcType; +import org.labkey.api.data.NullColumnInfo; +import org.labkey.api.data.ObjectFactory; +import org.labkey.api.data.PropertyManager; +import org.labkey.api.data.RuntimeSQLException; +import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.SchemaTableInfo; +import org.labkey.api.data.SimpleFilter; +import org.labkey.api.data.SqlExecutor; +import org.labkey.api.data.SqlSelector; +import org.labkey.api.data.Table; +import org.labkey.api.data.TableInfo; +import org.labkey.api.data.TableSelector; +import org.labkey.api.data.Transient; +import org.labkey.api.data.UpdateableTableInfo; import org.labkey.api.data.dialect.SqlDialect; import org.labkey.api.dataiterator.DataIterator; import org.labkey.api.dataiterator.DataIteratorBuilder; @@ -1843,7 +1872,7 @@ else if (existingRecord != null && !existingRecord.isEmpty()) * @param requiredAuditType The expected audit behavior type. If this does not match the type set on the * dataset, then the event will not be logged. */ - public void addAuditEvent(User user, Container c, AuditBehaviorType requiredAuditType, String comment, @Nullable UploadLog ul) + public void addAuditEvent(User user, Container c, AuditBehaviorType requiredAuditType, String comment) { TableInfo table = _dataset.getTableInfo(user); @@ -1851,10 +1880,6 @@ public void addAuditEvent(User user, Container c, AuditBehaviorType requiredAudi return; DatasetAuditProvider.DatasetAuditEvent event = new DatasetAuditProvider.DatasetAuditEvent(c, comment, _dataset.getDatasetId()); - if (ul != null) - { - event.setLsid(ul.getFilePath()); - } AuditLogService.get().addEvent(user, event); } } diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index 207daa48924..2cdcd6926f4 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -16,9 +16,9 @@ package org.labkey.study.model; -import org.apache.commons.collections4.MapUtils; import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.collections4.ListUtils; +import org.apache.commons.collections4.MapUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.Level; @@ -226,12 +226,12 @@ import java.util.stream.Collectors; import static org.labkey.api.action.SpringActionController.ERROR_MSG; -import static org.labkey.api.util.IntegerUtils.asInteger; import static org.labkey.api.studydesign.query.StudyDesignQuerySchema.PERSONNEL_TABLE_NAME; import static org.labkey.api.studydesign.query.StudyDesignQuerySchema.PRODUCT_ANTIGEN_TABLE_NAME; import static org.labkey.api.studydesign.query.StudyDesignQuerySchema.PRODUCT_TABLE_NAME; import static org.labkey.api.studydesign.query.StudyDesignQuerySchema.TREATMENT_PRODUCT_MAP_TABLE_NAME; import static org.labkey.api.studydesign.query.StudyDesignQuerySchema.TREATMENT_TABLE_NAME; +import static org.labkey.api.util.IntegerUtils.asInteger; public class StudyManager { @@ -2566,7 +2566,7 @@ public void deleteDataset(StudyImpl study, User user, DatasetDefinition ds, bool SchemaKey schemaPath = SchemaKey.fromParts(SCHEMA.getSchemaName()); QueryService.get().fireQueryDeleted(user, study.getContainer(), null, schemaPath, Collections.singleton(ds.getName())); - new DatasetDefinition.DatasetAuditHandler(ds).addAuditEvent(user, study.getContainer(), AuditBehaviorType.DETAILED, "Dataset deleted: " + ds.getName(), null); + new DatasetDefinition.DatasetAuditHandler(ds).addAuditEvent(user, study.getContainer(), AuditBehaviorType.DETAILED, "Dataset deleted: " + ds.getName()); transaction.addCommitTask(() -> unindexDataset(ds), diff --git a/study/src/org/labkey/study/model/UploadLog.java b/study/src/org/labkey/study/model/UploadLog.java index ff7e40e56fb..9b640f57034 100644 --- a/study/src/org/labkey/study/model/UploadLog.java +++ b/study/src/org/labkey/study/model/UploadLog.java @@ -33,7 +33,6 @@ public class UploadLog private Date created; private int userId; private String description; - private String filePath; private int datasetId; private String status; @@ -87,16 +86,6 @@ public void setDescription(String description) this.description = description; } - public String getFilePath() - { - return filePath; - } - - public void setFilePath(String filePath) - { - this.filePath = filePath; - } - public int getDatasetId() { return datasetId; From 3ba91c623246e4fa6510ee09fcd28ce87f05f018 Mon Sep 17 00:00:00 2001 From: Karl Lum Date: Mon, 8 Dec 2025 16:39:47 -0800 Subject: [PATCH 06/12] Drop unique constraint on study.uploadLog filePath field (#7251) --- .../schemas/dbscripts/postgresql/study-25.004-25.005.sql | 1 + .../schemas/dbscripts/sqlserver/study-25.004-25.005.sql | 1 + study/src/org/labkey/study/StudyModule.java | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 study/resources/schemas/dbscripts/postgresql/study-25.004-25.005.sql create mode 100644 study/resources/schemas/dbscripts/sqlserver/study-25.004-25.005.sql diff --git a/study/resources/schemas/dbscripts/postgresql/study-25.004-25.005.sql b/study/resources/schemas/dbscripts/postgresql/study-25.004-25.005.sql new file mode 100644 index 00000000000..dc5448ecf70 --- /dev/null +++ b/study/resources/schemas/dbscripts/postgresql/study-25.004-25.005.sql @@ -0,0 +1 @@ +ALTER TABLE study.uploadlog DROP CONSTRAINT UQ_UploadLog_FilePath; \ No newline at end of file diff --git a/study/resources/schemas/dbscripts/sqlserver/study-25.004-25.005.sql b/study/resources/schemas/dbscripts/sqlserver/study-25.004-25.005.sql new file mode 100644 index 00000000000..dc5448ecf70 --- /dev/null +++ b/study/resources/schemas/dbscripts/sqlserver/study-25.004-25.005.sql @@ -0,0 +1 @@ +ALTER TABLE study.uploadlog DROP CONSTRAINT UQ_UploadLog_FilePath; \ No newline at end of file diff --git a/study/src/org/labkey/study/StudyModule.java b/study/src/org/labkey/study/StudyModule.java index 69570d36931..fbb6ab9dc93 100644 --- a/study/src/org/labkey/study/StudyModule.java +++ b/study/src/org/labkey/study/StudyModule.java @@ -226,7 +226,7 @@ public String getName() @Override public Double getSchemaVersion() { - return 25.004; + return 25.005; } @Override From 5dd0eed5545c5f4f95d84de79c30dedf46aa8cf0 Mon Sep 17 00:00:00 2001 From: Binal Patel Date: Wed, 10 Dec 2025 09:40:31 -0800 Subject: [PATCH 07/12] Issue 54076: Antivirus Support: Improve error messaging for ClamAV-flagged uploads with link to client guidance (#7238) * Update error message * Move error msg to a constant in AntiVirusService --- api/src/org/labkey/api/premium/AntiVirusService.java | 1 + core/src/org/labkey/core/webdav/DavController.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/premium/AntiVirusService.java b/api/src/org/labkey/api/premium/AntiVirusService.java index 20a31e5d897..5e15f09145e 100644 --- a/api/src/org/labkey/api/premium/AntiVirusService.java +++ b/api/src/org/labkey/api/premium/AntiVirusService.java @@ -35,6 +35,7 @@ public interface AntiVirusService { // NOTE purposefully this is not the same as the standard test file: ...EICAR-STANDARD-ANTIVIRUS-TEST-FILE... String TEST_VIRUS_CONTENT="X5O!P%@AP[4\\PZX54(P^)7CC)7}$LABKEY-ANTIVIRUS-TEST-FILE!$H+H*"; + String MALICIOUS_FILE_ERROR_MESSAGE = "For security reasons, we did not upload this file. Please contact your internal IT or Information Security department for assistance in dealing with this potentially harmful file."; static @Nullable AntiVirusService get() { diff --git a/core/src/org/labkey/core/webdav/DavController.java b/core/src/org/labkey/core/webdav/DavController.java index 58c7bbb0112..6c680148437 100644 --- a/core/src/org/labkey/core/webdav/DavController.java +++ b/core/src/org/labkey/core/webdav/DavController.java @@ -3002,7 +3002,7 @@ public void closeInputStream() throws IOException if (result.result == AntiVirusService.Result.CONFIGURATION_ERROR) throw new ConfigurationException(result.message); else - throw new DavException(WebdavStatus.SC_BAD_REQUEST, result.message); + throw new DavException(WebdavStatus.SC_BAD_REQUEST, result.message + ". " + AntiVirusService.MALICIOUS_FILE_ERROR_MESSAGE); } } From 91d368119fbe2957521761d38a749c8900463f7a Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 10 Dec 2025 09:58:01 -0800 Subject: [PATCH 08/12] Write file paths, etc. (#7229) --- .../ImportProgress.java} | 49 +++++++++---------- api/src/org/labkey/api/dataiterator/Pump.java | 5 +- .../labkey/api/exp/list/ListDefinition.java | 5 +- .../DatabaseMigrationConfiguration.java | 8 ++- .../DefaultMigrationSchemaHandler.java | 11 +++-- .../labkey/api/migration/FilePathWriter.java | 42 ++++++++++++++++ .../api/migration/MigrationSchemaHandler.java | 3 ++ .../org/labkey/api/module/ModuleLoader.java | 3 -- .../DataClassMigrationSchemaHandler.java | 14 ++++-- .../ExperimentMigrationSchemaHandler.java | 9 ++++ .../SampleTypeMigrationSchemaHandler.java | 8 +++ .../labkey/list/model/ListDefinitionImpl.java | 6 +-- .../list/model/ListQueryUpdateService.java | 4 +- .../src/org/labkey/search/SearchModule.java | 8 +++ .../specimen/importer/SpecimenImporter.java | 4 +- 15 files changed, 130 insertions(+), 49 deletions(-) rename api/src/org/labkey/api/{exp/list/ListImportProgress.java => dataiterator/ImportProgress.java} (81%) create mode 100644 api/src/org/labkey/api/migration/FilePathWriter.java diff --git a/api/src/org/labkey/api/exp/list/ListImportProgress.java b/api/src/org/labkey/api/dataiterator/ImportProgress.java similarity index 81% rename from api/src/org/labkey/api/exp/list/ListImportProgress.java rename to api/src/org/labkey/api/dataiterator/ImportProgress.java index 0210a1fd341..f118003f82c 100644 --- a/api/src/org/labkey/api/exp/list/ListImportProgress.java +++ b/api/src/org/labkey/api/dataiterator/ImportProgress.java @@ -1,27 +1,22 @@ -/* - * Copyright (c) 2009-2014 LabKey Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.labkey.api.exp.list; - -/* -* User: adam -* Date: Dec 23, 2009 -* Time: 3:05:51 PM -*/ -public interface ListImportProgress -{ - void setTotalRows(int rows); - void setCurrentRow(int currentRow); -} +/* + * Copyright (c) 2009-2014 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.dataiterator; + +public interface ImportProgress +{ + void setTotalRows(int rows); + void setCurrentRow(int currentRow); +} diff --git a/api/src/org/labkey/api/dataiterator/Pump.java b/api/src/org/labkey/api/dataiterator/Pump.java index fd15dd76940..83e5b41cfa8 100644 --- a/api/src/org/labkey/api/dataiterator/Pump.java +++ b/api/src/org/labkey/api/dataiterator/Pump.java @@ -17,7 +17,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.labkey.api.exp.list.ListImportProgress; import org.labkey.api.query.BatchValidationException; import java.io.IOException; @@ -34,7 +33,7 @@ public class Pump implements Runnable final BatchValidationException _errors; int _errorLimit = Integer.MAX_VALUE; long _rowCount = 0; - ListImportProgress _progress = null; + ImportProgress _progress = null; public Pump(DataIterator it, DataIteratorContext context) { @@ -50,7 +49,7 @@ public Pump(DataIteratorBuilder builder, DataIteratorContext context) _errors = context.getErrors(); } - public void setProgress(ListImportProgress progress) + public void setProgress(ImportProgress progress) { _progress = progress; } diff --git a/api/src/org/labkey/api/exp/list/ListDefinition.java b/api/src/org/labkey/api/exp/list/ListDefinition.java index 69d4473c22b..9963ad6b0bd 100644 --- a/api/src/org/labkey/api/exp/list/ListDefinition.java +++ b/api/src/org/labkey/api/exp/list/ListDefinition.java @@ -23,6 +23,7 @@ import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.LookupResolutionType; import org.labkey.api.data.TableInfo; +import org.labkey.api.dataiterator.ImportProgress; import org.labkey.api.exp.DomainNotFoundException; import org.labkey.api.exp.PropertyType; import org.labkey.api.exp.property.Domain; @@ -269,8 +270,8 @@ public static BodySetting getForValue(int value) ListItem getListItemForEntityId(String entityId, User user); int insertListItems(User user, Container container, List listItems) throws IOException; - int insertListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, LookupResolutionType lookupResolutionType) throws IOException; - int importListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, LookupResolutionType lookupResolutionType, QueryUpdateService.InsertOption insertOption) throws IOException; + int insertListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ImportProgress progress, boolean supportAutoIncrementKey, LookupResolutionType lookupResolutionType) throws IOException; + int importListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ImportProgress progress, boolean supportAutoIncrementKey, LookupResolutionType lookupResolutionType, QueryUpdateService.InsertOption insertOption) throws IOException; @Nullable TableInfo getTable(User user); @Nullable TableInfo getTable(User user, Container c); diff --git a/api/src/org/labkey/api/migration/DatabaseMigrationConfiguration.java b/api/src/org/labkey/api/migration/DatabaseMigrationConfiguration.java index 49cd45b4ff5..60a48a6e791 100644 --- a/api/src/org/labkey/api/migration/DatabaseMigrationConfiguration.java +++ b/api/src/org/labkey/api/migration/DatabaseMigrationConfiguration.java @@ -7,7 +7,10 @@ import org.labkey.api.data.DbScope; import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; +import org.labkey.api.util.GUID; +import org.labkey.api.util.Pair; +import java.io.PrintWriter; import java.util.Set; import java.util.function.Predicate; @@ -21,5 +24,8 @@ default void beforeMigration(){} Predicate getColumnNameFilter(); @Nullable TableSelector getTableSelector(DbSchemaType schemaType, TableInfo sourceTable, TableInfo targetTable, Set selectColumnNames, MigrationSchemaHandler schemaHandler, @Nullable MigrationTableHandler tableHandler); default void copyAttachments(DbSchema sourceSchema, DbSchema targetSchema, MigrationSchemaHandler schemaHandler){} - default void afterMigration(){} + default @Nullable Pair> initializeFilePathWriter() + { + return null; + } } diff --git a/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java b/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java index 36fb4089026..5eb1348abc3 100644 --- a/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java +++ b/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java @@ -306,15 +306,15 @@ public static void afterMigration() throws InterruptedException if (unseen.isEmpty()) LOG.info("All AttachmentTypes have been seen"); else - throw new ConfigurationException("These AttachmentTypes have not been seen: " + unseen.stream().map(type -> type.getClass().getSimpleName()).collect(Collectors.joining(", "))); + LOG.error("These AttachmentTypes have not been seen: {}", unseen.stream().map(type -> type.getClass().getSimpleName()).collect(Collectors.joining(", "))); // Shut down the attachment JobRunner LOG.info("Waiting for attachments background transfer to complete"); ATTACHMENT_JOB_RUNNER.shutdown(); - if (ATTACHMENT_JOB_RUNNER.awaitTermination(1, TimeUnit.HOURS)) + if (ATTACHMENT_JOB_RUNNER.awaitTermination(2, TimeUnit.HOURS)) LOG.info("Attachments background transfer is complete"); else - LOG.error("Attachments background transfer did not complete after one hour! Giving up."); + LOG.error("Attachments background transfer did not complete after two hours! Giving up."); } @Override @@ -332,4 +332,9 @@ public void afterSchema(DatabaseMigrationConfiguration configuration, DbSchema s public void afterMigration(DatabaseMigrationConfiguration configuration) { } + + @Override + public void writeFilePaths(FilePathWriter writer, Set guids) + { + } } diff --git a/api/src/org/labkey/api/migration/FilePathWriter.java b/api/src/org/labkey/api/migration/FilePathWriter.java new file mode 100644 index 00000000000..b034541c009 --- /dev/null +++ b/api/src/org/labkey/api/migration/FilePathWriter.java @@ -0,0 +1,42 @@ +package org.labkey.api.migration; + +import org.labkey.api.data.ContainerManager; +import org.labkey.api.files.FileContentService; + +import java.io.Closeable; +import java.io.File; +import java.io.PrintWriter; +import java.nio.file.Path; + +public class FilePathWriter implements Closeable +{ + private final PrintWriter _out; + private final Path _rootPath; + + public FilePathWriter(PrintWriter out) + { + _out = out; + _rootPath = FileContentService.get().getFileRoot(ContainerManager.getRoot()).toPath(); + } + + public void write(File file) + { + _out.println(_rootPath.relativize(file.toPath()).normalize()); + } + + public void println(String s) + { + _out.println(s); + } + + public void println() + { + _out.println(); + } + + @Override + public void close() + { + _out.close(); + } +} diff --git a/api/src/org/labkey/api/migration/MigrationSchemaHandler.java b/api/src/org/labkey/api/migration/MigrationSchemaHandler.java index daa6aaa02e3..ebb41405841 100644 --- a/api/src/org/labkey/api/migration/MigrationSchemaHandler.java +++ b/api/src/org/labkey/api/migration/MigrationSchemaHandler.java @@ -12,6 +12,7 @@ import org.labkey.api.query.FieldKey; import org.labkey.api.util.GUID; +import java.io.PrintWriter; import java.util.Collection; import java.util.List; import java.util.Set; @@ -57,4 +58,6 @@ public interface MigrationSchemaHandler @NotNull Collection getAttachmentTypes(); void afterMigration(DatabaseMigrationConfiguration configuration); + + void writeFilePaths(FilePathWriter writer, Set guids); } diff --git a/api/src/org/labkey/api/module/ModuleLoader.java b/api/src/org/labkey/api/module/ModuleLoader.java index 49d5743d602..b8839fd0947 100644 --- a/api/src/org/labkey/api/module/ModuleLoader.java +++ b/api/src/org/labkey/api/module/ModuleLoader.java @@ -678,9 +678,6 @@ private void doInit(Execution execution) throws ServletException _log.info("Server installation GUID: {}, server session GUID: {}", AppProps.getInstance().getServerGUID(), AppProps.getInstance().getServerSessionGUID()); _log.info("Deploying to context path {}", AppProps.getInstance().getContextPath()); - // Temporary logging to help track down issues we're having with upgrading XMLBeans from v5.2.0. TODO: Remove after upgrade - _log.info("XMLBeans version: {}", XmlBeans.getVersion()); - synchronized (_modulesLock) { checkForRenamedModules(); diff --git a/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java index 2e93cba7b5d..296b2b349b5 100644 --- a/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/DataClassMigrationSchemaHandler.java @@ -24,6 +24,7 @@ import org.labkey.api.migration.DatabaseMigrationService.DataFilter; import org.labkey.api.migration.DefaultMigrationSchemaHandler; import org.labkey.api.migration.ExperimentDeleteService; +import org.labkey.api.migration.FilePathWriter; import org.labkey.api.query.FieldKey; import org.labkey.api.util.GUID; import org.labkey.api.util.StringUtilsLabKey; @@ -95,8 +96,8 @@ public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilte // Select all ObjectIds associated with the not-copied rows from the source database SQLFragment objectIdSql = new SQLFragment("SELECT ObjectId FROM exp.Data d INNER JOIN ") - .appendIdentifier(sourceTable.getSelectName()) - .append(" dc ON d.LSID = dc.LSID"); + .appendIdentifier(sourceTable.getSelectName()) + .append(" dc ON d.LSID = dc.LSID"); // Don't create an empty IN clause; need to work around issue where "NOT xxx IN (NULL)" evaluates to NULL. if (!copiedLsids.isEmpty()) @@ -134,7 +135,7 @@ public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilte } }) .forEach(SEQUENCE_IDS::add); - LOG.info("{} added to the SequenceIdentity set", + LOG.info(" {} added to the SequenceIdentity set", StringUtilsLabKey.pluralize(SEQUENCE_IDS.size() - startSize, "unique SequenceId was", "unique SequenceIds were")); } } @@ -202,4 +203,11 @@ public FilterClause getTableFilterClause(TableInfo sourceTable, Set contai { return List.of(ExpDataClassType.get()); } + + @Override + public void writeFilePaths(FilePathWriter writer, Set guids) + { + // TODO: Enumerate FileLink fields in data classes in the filtered containers (guids) and write out those file + // paths. Current client has none. + } } diff --git a/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java index 3a1bfb1d6bb..6d844f2864b 100644 --- a/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java @@ -261,6 +261,15 @@ public FilterClause getContainerClause(TableInfo sourceTable, Set containe .appendInClause(containers, sourceTable.getSqlDialect()) .append(")") ); + case "MaterialAliasMap" -> new AndClause( + new InClause(FieldKey.fromParts("Container"), containers), + // The below effectively matches the "Material" conditions above, since MaterialAliasMap has an FK to Material + new InClause(FieldKey.fromParts("LSID", "Container"), containers), + new OrClause( + new CompareClause(FieldKey.fromParts("LSID", "RunId"), CompareType.ISBLANK, null), + new InClause(FieldKey.fromParts("LSID", "RunId", "Container"), containers) + ) + ); case "ObjectLegacyNames" -> new SQLClause( new SQLFragment("ObjectId IN (SELECT ObjectId FROM exp.Object WHERE Container") .appendInClause(containers, sourceTable.getSqlDialect()) diff --git a/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java index 4c2d2f89d42..0aae0fa7aa6 100644 --- a/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/SampleTypeMigrationSchemaHandler.java @@ -14,6 +14,7 @@ import org.labkey.api.exp.api.SampleTypeDomainKind; import org.labkey.api.migration.DatabaseMigrationService.DataFilter; import org.labkey.api.migration.DefaultMigrationSchemaHandler; +import org.labkey.api.migration.FilePathWriter; import org.labkey.api.util.GUID; import org.labkey.api.util.logging.LogHelper; @@ -168,4 +169,11 @@ public void afterTable(TableInfo sourceTable, TableInfo targetTable, SimpleFilte ExperimentMigrationSchemaHandler.deleteObjectIds(objectIdClause); } } + + @Override + public void writeFilePaths(FilePathWriter writer, Set guids) + { + // TODO: Enumerate FileLink fields in sample types in the filtered containers (guids) and write out those file + // paths. Current client has none. + } } diff --git a/list/src/org/labkey/list/model/ListDefinitionImpl.java b/list/src/org/labkey/list/model/ListDefinitionImpl.java index 764607d36b3..b63b108c2e9 100644 --- a/list/src/org/labkey/list/model/ListDefinitionImpl.java +++ b/list/src/org/labkey/list/model/ListDefinitionImpl.java @@ -30,13 +30,13 @@ import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; +import org.labkey.api.dataiterator.ImportProgress; import org.labkey.api.exp.DomainNotFoundException; import org.labkey.api.exp.Lsid; import org.labkey.api.exp.ObjectProperty; import org.labkey.api.exp.TemplateInfo; import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.list.ListDefinition; -import org.labkey.api.exp.list.ListImportProgress; import org.labkey.api.exp.list.ListItem; import org.labkey.api.exp.property.Domain; import org.labkey.api.exp.property.DomainProperty; @@ -610,13 +610,13 @@ public int insertListItems(User user, Container container, List listIt @Override - public int insertListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, LookupResolutionType lookupResolutionType) + public int insertListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ImportProgress progress, boolean supportAutoIncrementKey, LookupResolutionType lookupResolutionType) { return importListItems(user, container, loader, errors, attachmentDir, progress, supportAutoIncrementKey, lookupResolutionType, QueryUpdateService.InsertOption.INSERT); } @Override - public int importListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, LookupResolutionType lookupResolutionType, QueryUpdateService.InsertOption insertOption) + public int importListItems(User user, Container container, DataLoader loader, @NotNull BatchValidationException errors, @Nullable VirtualFile attachmentDir, @Nullable ImportProgress progress, boolean supportAutoIncrementKey, LookupResolutionType lookupResolutionType, QueryUpdateService.InsertOption insertOption) { ListQuerySchema schema = new ListQuerySchema(user, container); TableInfo table = schema.getTable(_def.getName()); diff --git a/list/src/org/labkey/list/model/ListQueryUpdateService.java b/list/src/org/labkey/list/model/ListQueryUpdateService.java index 9e6fae5c12f..8848d3d8b76 100644 --- a/list/src/org/labkey/list/model/ListQueryUpdateService.java +++ b/list/src/org/labkey/list/model/ListQueryUpdateService.java @@ -42,11 +42,11 @@ import org.labkey.api.dataiterator.DataIteratorBuilder; import org.labkey.api.dataiterator.DataIteratorContext; import org.labkey.api.dataiterator.DetailedAuditLogDataIterator; +import org.labkey.api.dataiterator.ImportProgress; import org.labkey.api.exp.ObjectProperty; import org.labkey.api.exp.PropertyType; import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.list.ListDefinition; -import org.labkey.api.exp.list.ListImportProgress; import org.labkey.api.exp.list.ListItem; import org.labkey.api.exp.list.ListService; import org.labkey.api.exp.property.Domain; @@ -229,7 +229,7 @@ private User getListUser(User user, Container container) } public int insertUsingDataIterator(DataLoader loader, User user, Container container, BatchValidationException errors, @Nullable VirtualFile attachmentDir, - @Nullable ListImportProgress progress, boolean supportAutoIncrementKey, InsertOption insertOption, LookupResolutionType lookupResolutionType) + @Nullable ImportProgress progress, boolean supportAutoIncrementKey, InsertOption insertOption, LookupResolutionType lookupResolutionType) { if (!_list.isVisible(user)) throw new UnauthorizedException("You do not have permission to insert data into this table."); diff --git a/search/src/org/labkey/search/SearchModule.java b/search/src/org/labkey/search/SearchModule.java index d4cd37bca6e..f12c4ea4294 100644 --- a/search/src/org/labkey/search/SearchModule.java +++ b/search/src/org/labkey/search/SearchModule.java @@ -29,6 +29,7 @@ import org.labkey.api.data.UpgradeCode; import org.labkey.api.mbean.LabKeyManagement; import org.labkey.api.mbean.SearchMXBean; +import org.labkey.api.migration.DatabaseMigrationConfiguration; import org.labkey.api.migration.DatabaseMigrationService; import org.labkey.api.migration.DefaultMigrationSchemaHandler; import org.labkey.api.module.DefaultModule; @@ -195,6 +196,13 @@ public List getTablesToCopy() { return List.of(); // Leave empty -- target server will re-index all documents } + + @Override + public void afterMigration(DatabaseMigrationConfiguration configuration) + { + // Clear index and all last indexed tracking + SearchService.get().deleteIndex("Database was just migrated"); + } }); } diff --git a/specimen/src/org/labkey/specimen/importer/SpecimenImporter.java b/specimen/src/org/labkey/specimen/importer/SpecimenImporter.java index 13e5ed7ba1b..86e5d0465a8 100644 --- a/specimen/src/org/labkey/specimen/importer/SpecimenImporter.java +++ b/specimen/src/org/labkey/specimen/importer/SpecimenImporter.java @@ -57,6 +57,7 @@ import org.labkey.api.dataiterator.DataIteratorBuilder; import org.labkey.api.dataiterator.DataIteratorContext; import org.labkey.api.dataiterator.DataIteratorUtil; +import org.labkey.api.dataiterator.ImportProgress; import org.labkey.api.dataiterator.ListofMapsDataIterator; import org.labkey.api.dataiterator.LoggingDataIterator; import org.labkey.api.dataiterator.MapDataIterator; @@ -69,7 +70,6 @@ import org.labkey.api.exp.api.ExpSampleType; import org.labkey.api.exp.api.ExperimentService; import org.labkey.api.exp.api.SampleTypeService; -import org.labkey.api.exp.list.ListImportProgress; import org.labkey.api.iterator.MarkableIterator; import org.labkey.api.pipeline.PipelineJob; import org.labkey.api.query.DefaultSchema; @@ -1993,7 +1993,7 @@ public Object getValue(Map row) DataIteratorBuilder standardEtl = StandardDataIteratorBuilder.forInsert(target, specimenWrapped, getContainer(), getUser(), dix); DataIteratorBuilder persist = ((UpdateableTableInfo)target).persistRows(standardEtl, dix); Pump pump = new Pump(persist, dix); - pump.setProgress(new ListImportProgress() + pump.setProgress(new ImportProgress() { long heartBeat = HeartBeat.currentTimeMillis(); From f814e445d418566f7566550ade5c391532ec3c2f Mon Sep 17 00:00:00 2001 From: Cory Nathe Date: Wed, 10 Dec 2025 12:11:45 -0600 Subject: [PATCH 09/12] GitHub Issue 726: Fix typo in URLDisplayColumn img style width and height semicolon location (#7255) - make sure semicolon is added after img style width and height --- api/src/org/labkey/api/data/URLDisplayColumn.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/data/URLDisplayColumn.java b/api/src/org/labkey/api/data/URLDisplayColumn.java index 55bb96d703d..4b5e202fc2c 100644 --- a/api/src/org/labkey/api/data/URLDisplayColumn.java +++ b/api/src/org/labkey/api/data/URLDisplayColumn.java @@ -182,7 +182,7 @@ public Renderable createThumbnailImage() String thumbnailUrl = _fileIconUrl != null ? ensureAbsoluteUrl(_ctx, _fileIconUrl) : ensureAbsoluteUrl(_ctx, _url); return IMG(at( - style, "display:block; vertical-align:middle;" + (renderSize ? (_thumbnailWidth != null ? " width:" + _thumbnailWidth : " max-width:32px" + "; ") + (_thumbnailHeight != null ? " height:" + _thumbnailHeight : " height:auto" + ";") : ""), + style, "display:block; vertical-align:middle;" + (renderSize ? (_thumbnailWidth != null ? " width:" + _thumbnailWidth : " max-width:32px") + "; " + (_thumbnailHeight != null ? " height:" + _thumbnailHeight : " height:auto") + ";" : ""), src, thumbnailUrl, title, _displayName )); } From 0be86528c31977cb4194f57057ea7a4c07381c2e Mon Sep 17 00:00:00 2001 From: bbimber Date: Sat, 13 Dec 2025 14:25:35 -0800 Subject: [PATCH 10/12] Do not relativize ExpData URIs when making the LSID unless under file root (#7261) --- .../experiment/api/ExperimentServiceImpl.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index f3d51bf42b7..705e05c82d6 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -230,13 +230,13 @@ import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.util.SubstitutionFormat; import org.labkey.api.util.TestContext; +import org.labkey.api.util.URIUtil; import org.labkey.api.util.UnexpectedException; import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.ActionURL; import org.labkey.api.view.HttpView; import org.labkey.api.view.JspTemplate; import org.labkey.api.view.JspView; -import org.labkey.api.view.NotFoundException; import org.labkey.api.view.UnauthorizedException; import org.labkey.api.view.ViewBackgroundInfo; import org.labkey.api.view.ViewContext; @@ -244,7 +244,6 @@ import org.labkey.experiment.FileLinkFileListener; import org.labkey.experiment.MissingFilesCheckInfo; import org.labkey.experiment.XarExportType; -import org.labkey.experiment.XarExporter; import org.labkey.experiment.XarReader; import org.labkey.experiment.api.property.DomainPropertyManager; import org.labkey.experiment.controllers.exp.ExperimentController; @@ -253,13 +252,11 @@ import org.labkey.experiment.pipeline.ExperimentPipelineJob; import org.labkey.experiment.pipeline.MoveRunsPipelineJob; import org.labkey.experiment.xar.AutoFileLSIDReplacer; -import org.labkey.experiment.xar.XarExportSelection; import org.labkey.vfs.FileLike; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.dao.PessimisticLockingFailureException; import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; import java.io.UncheckedIOException; import java.net.MalformedURLException; @@ -778,7 +775,17 @@ public ExpDataImpl createData(URI uri, XarSource source) throws XarFormatExcepti { path = FileUtil.stringToPath(source.getXarContext().getContainer(), source.getCanonicalDataFileURL(FileUtil.pathToString(path))); - pathStr = FileUtil.relativizeUnix(source.getRootPath(), path, false); + + // Only convert to a relative path if this is a descendant of the root: + path = path.normalize(); + if (URIUtil.isDescendant(source.getRootPath().toUri(), path.toUri())) + { + pathStr = FileUtil.relativizeUnix(source.getRootPath(), path, false); + } + else + { + pathStr = FileUtil.pathToString(path); + } } catch (IOException e) { From 186295bd8113ccbd2e835fd49be81c25e2358176 Mon Sep 17 00:00:00 2001 From: Cory Nathe Date: Mon, 15 Dec 2025 08:11:25 -0600 Subject: [PATCH 11/12] Allow site admins to see all views (private and shared) from the hidden query-manageViews page (#7262) - allow site admin to see all container custom views from query-manageViews action - set table css classes to make the static table more readable --- .../query/controllers/InternalViewForm.java | 3 +- .../src/org/labkey/query/view/manageViews.jsp | 37 +++++++++++-------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/query/src/org/labkey/query/controllers/InternalViewForm.java b/query/src/org/labkey/query/controllers/InternalViewForm.java index c8bc14c560a..0a8612e3b96 100644 --- a/query/src/org/labkey/query/controllers/InternalViewForm.java +++ b/query/src/org/labkey/query/controllers/InternalViewForm.java @@ -69,7 +69,8 @@ public static void checkEdit(ViewContext context, CstmView view) } else { - if (view.getCustomViewOwner().intValue() != context.getUser().getUserId()) + // must be owner or site admin + if (!context.getUser().hasSiteAdminPermission() && view.getCustomViewOwner().intValue() != context.getUser().getUserId()) { throw new UnauthorizedException(); } diff --git a/query/src/org/labkey/query/view/manageViews.jsp b/query/src/org/labkey/query/view/manageViews.jsp index e5196c99437..d4f52746f04 100644 --- a/query/src/org/labkey/query/view/manageViews.jsp +++ b/query/src/org/labkey/query/view/manageViews.jsp @@ -59,14 +59,16 @@ QueryManager mgr = QueryManager.get(); List views = new ArrayList<>(); - if (getViewContext().hasPermission(UpdatePermission.class)) + if (getViewContext().getUser().hasSiteAdminPermission()) { - views.addAll(mgr.getCstmViews(c, schemaName, queryName, null, null, false, true)); + views.addAll(mgr.getCstmViews(c, schemaName, queryName, null, null, false, false)); } - - if (!user.isGuest()) + else { - views.addAll(mgr.getCstmViews(c, schemaName, queryName, null, user, false, false)); + if (getViewContext().hasPermission(UpdatePermission.class)) + views.addAll(mgr.getCstmViews(c, schemaName, queryName, null, null, false, true)); + if (!user.isGuest()) + views.addAll(mgr.getCstmViews(c, schemaName, queryName, null, user, false, false)); } // UNDONE: Requires queryName and schemaName for now. We need a method to get all session views in a container. @@ -107,22 +109,25 @@ <% } %>

- +
- - - - - - - - - + + + + + + + + + + <% if (getViewContext().hasPermission(UpdatePermission.class)) { + int count = 1; for (CstmView view : views) { + count++; List flags = new ArrayList<>(); if (view.getCustomViewId() == 0) flags.add("session"); @@ -133,7 +138,7 @@ if (mgr.isSnapshot(view.getFlags())) flags.add("shapshot"); %> - +
SchemaQueryView NameFlagsOwnerCreatedCreated ByModifiedModified BySchemaQueryView NameFlagsOwnerCreatedCreated ByModifiedModified By
<%=h(view.getSchema())%> <%=h(view.getQueryName())%> From e1da7bd1f13c265e41e31999ec9e88925d83d04b Mon Sep 17 00:00:00 2001 From: Nick Kerr Date: Mon, 15 Dec 2025 17:47:12 -0800 Subject: [PATCH 12/12] Additional action logging (#7270) --- .../api/action/PermissionCheckableAction.java | 17 ++++++++++++++++- api/src/org/labkey/api/data/Container.java | 13 +++++++++++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/action/PermissionCheckableAction.java b/api/src/org/labkey/api/action/PermissionCheckableAction.java index 5795a2e2693..984aa69a8a4 100644 --- a/api/src/org/labkey/api/action/PermissionCheckableAction.java +++ b/api/src/org/labkey/api/action/PermissionCheckableAction.java @@ -16,6 +16,7 @@ package org.labkey.api.action; import jakarta.servlet.http.HttpServletResponse; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.Container; import org.labkey.api.module.IgnoresForbiddenProjectCheck; @@ -39,6 +40,7 @@ import org.labkey.api.security.roles.RoleManager; import org.labkey.api.util.ConfigurationException; import org.labkey.api.util.HttpUtil; +import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.BadRequestException; import org.labkey.api.view.NotFoundException; import org.labkey.api.view.RedirectException; @@ -55,6 +57,7 @@ public abstract class PermissionCheckableAction implements Controller, PermissionCheckable, HasViewContext { + private static final Logger LOG = LogHelper.getLogger(PermissionCheckableAction.class, "Permission checks for actions"); private static final HttpUtil.Method[] arrayGetPost = new HttpUtil.Method[] {Method.GET, Method.POST}; private ViewContext _context = null; UnauthorizedException.Type _unauthorizedType = UnauthorizedException.Type.redirectToLogin; @@ -148,6 +151,8 @@ private void _checkActionPermissions(Set contextualRoles) throws Unauthori Container c = context.getContainer(); User user = context.getUser(); Class actionClass = getClass(); + if (LOG.isDebugEnabled()) + LOG.debug(actionClass.getName() + ": checking permissions for user " + (user == null ? "" : user.getName() + " (impersonated=" + user.isImpersonated() + ")")); if (!actionClass.isAnnotationPresent(IgnoresForbiddenProjectCheck.class)) c.throwIfForbiddenProject(user); @@ -159,18 +164,22 @@ private void _checkActionPermissions(Set contextualRoles) throws Unauthori methodsAllowed = methodsAllowedAnnotation.value(); if (Arrays.stream(methodsAllowed).noneMatch(s -> s.equals(method))) { - throw new BadRequestException("Method Not Allowed: " + method, null, HttpServletResponse.SC_METHOD_NOT_ALLOWED); + String msg = "Method Not Allowed: " + method; + LOG.debug(msg); + throw new BadRequestException(msg, null, HttpServletResponse.SC_METHOD_NOT_ALLOWED); } boolean requiresSiteAdmin = actionClass.isAnnotationPresent(RequiresSiteAdmin.class); if (requiresSiteAdmin && !user.hasSiteAdminPermission()) { + LOG.debug(actionClass.getName() + ": action requires site admin permissions"); throw new UnauthorizedException(); } boolean requiresLogin = actionClass.isAnnotationPresent(RequiresLogin.class); if (requiresLogin && user.isGuest()) { + LOG.debug(actionClass.getName() + ": action requires login (non-guest)"); throw new UnauthorizedException(); } @@ -214,7 +223,10 @@ private void _checkActionPermissions(Set contextualRoles) throws Unauthori // Must have all permissions in permissionsRequired if (!SecurityManager.hasAllPermissions(this.getClass().getName()+"_checkActionPermissions", c, user, permissionsRequired, contextualRoles)) + { + LOG.debug(actionClass.getName() + ": action requires all permissions: " + permissionsRequired); throw new UnauthorizedException(); + } CSRF.Method csrfCheck = actionClass.isAnnotationPresent(CSRF.class) ? actionClass.getAnnotation(CSRF.class).value() : CSRF.Method.POST; csrfCheck.validate(context); @@ -228,7 +240,10 @@ private void _checkActionPermissions(Set contextualRoles) throws Unauthori Collections.addAll(permissionsAnyOf, requiresAnyOf.value()); if (!SecurityManager.hasAnyPermissions(this.getClass().getName() + "_checkActionPermissions", c, user, permissionsAnyOf, contextualRoles)) + { + LOG.debug(actionClass.getName() + ": action requires any permissions: " + permissionsAnyOf); throw new UnauthorizedException(); + } } boolean requiresNoPermission = actionClass.isAnnotationPresent(RequiresNoPermission.class); diff --git a/api/src/org/labkey/api/data/Container.java b/api/src/org/labkey/api/data/Container.java index c55c45cd561..d8922b085fc 100644 --- a/api/src/org/labkey/api/data/Container.java +++ b/api/src/org/labkey/api/data/Container.java @@ -41,6 +41,7 @@ import org.labkey.api.query.QueryService; import org.labkey.api.security.HasPermission; import org.labkey.api.security.SecurableResource; +import org.labkey.api.security.SecurityLogger; import org.labkey.api.security.SecurityManager; import org.labkey.api.security.SecurityPolicy; import org.labkey.api.security.SecurityPolicyManager; @@ -549,7 +550,11 @@ private boolean handleForbiddenProject(User user, Set contextualRoles, boo if (null != impersonationProject && !impersonationProject.equals(currentProject)) { if (shouldThrow) - throw new ForbiddenProjectException("You are not allowed to access this folder while impersonating within a different project."); + { + String msg = "You are not allowed to access this folder while impersonating within a different project."; + SecurityLogger.log(msg, user, null, null); + throw new ForbiddenProjectException(msg); + } return true; } @@ -562,7 +567,11 @@ private boolean handleForbiddenProject(User user, Set contextualRoles, boo if (lockState.isLocked() && ContainerManager.LOCKED_PROJECT_HANDLER.isForbidden(currentProject, user, contextualRoles, lockState)) { if (shouldThrow) - throw new ForbiddenProjectException("You are not allowed to access this folder; it is " + lockState.getDescription() + "."); + { + String msg = "You are not allowed to access this folder; it is " + lockState.getDescription() + "."; + SecurityLogger.log(msg, user, null, null); + throw new ForbiddenProjectException(msg); + } return true; }