From 6f4ce901c2dae7a64d2c93c320421c48a1de752f Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Wed, 19 Nov 2025 09:53:29 -0800 Subject: [PATCH 1/5] Filter signed snapshots based on notebook filter (#7207) --- api/src/org/labkey/api/data/SimpleFilter.java | 13 +- .../data/dialect/BasePostgreSqlDialect.java | 4 +- .../labkey/api/data/dialect/SqlDialect.java | 6 +- .../DatabaseMigrationConfiguration.java | 2 +- .../migration/DatabaseMigrationService.java | 1 + ...DefaultDatabaseMigrationConfiguration.java | 2 +- .../DefaultMigrationSchemaHandler.java | 43 ++++-- .../api/migration/MigrationTableHandler.java | 18 +++ api/src/org/labkey/api/util/JobRunner.java | 26 ++-- .../ExperimentMigrationSchemaHandler.java | 127 ++++++++++++++++-- .../labkey/experiment/ExperimentModule.java | 63 ++++++++- .../src/org/labkey/search/SearchModule.java | 2 +- 12 files changed, 260 insertions(+), 47 deletions(-) create mode 100644 api/src/org/labkey/api/migration/MigrationTableHandler.java diff --git a/api/src/org/labkey/api/data/SimpleFilter.java b/api/src/org/labkey/api/data/SimpleFilter.java index e114942285d..8dff53f20ff 100644 --- a/api/src/org/labkey/api/data/SimpleFilter.java +++ b/api/src/org/labkey/api/data/SimpleFilter.java @@ -664,11 +664,19 @@ public static DatabaseIdentifier getAliasForColumnFilter(SqlDialect dialect, Col public static class InClause extends MultiValuedFilterClause { + private InClauseGenerator _tempTableGenerator = null; + public InClause(FieldKey fieldKey, Collection params) { this(fieldKey, params, false, false); } + public InClause(FieldKey fieldKey, Collection params, InClauseGenerator tempTableGenerator) + { + this(fieldKey, params, false, false); + _tempTableGenerator = tempTableGenerator; + } + public InClause(FieldKey fieldKey, Collection params, boolean urlClause) { this(fieldKey, params, urlClause, false); @@ -837,7 +845,10 @@ public SQLFragment toSQLFragment(Map columnMap, in.appendIdentifier(alias); // Dialect may want to generate database-specific SQL, especially for very large IN clauses - dialect.appendInClauseSql(in, convertedParams); + if (null == _tempTableGenerator) + dialect.appendInClauseSql(in, convertedParams); + else + dialect.appendInClauseSqlWithCustomInClauseGenerator(in, convertedParams, _tempTableGenerator); if (isIncludeNull()) { diff --git a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java index 5cba2821b96..ecdb80e1507 100644 --- a/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/BasePostgreSqlDialect.java @@ -292,11 +292,11 @@ public String addReselect(SQLFragment sql, ColumnInfo column, @Nullable String p @Override public SQLFragment appendInClauseSql(SQLFragment sql, @NotNull Collection params) { - return appendInClauseSql(sql, params, _tempTableInClauseGenerator); + return appendInClauseSqlWithCustomInClauseGenerator(sql, params, _tempTableInClauseGenerator); } @Override - public SQLFragment appendInClauseSql(SQLFragment sql, @NotNull Collection params, InClauseGenerator tempTableGenerator) + public SQLFragment appendInClauseSqlWithCustomInClauseGenerator(SQLFragment sql, @NotNull Collection params, InClauseGenerator tempTableGenerator) { if (params.size() >= TEMPTABLE_GENERATOR_MINSIZE) { diff --git a/api/src/org/labkey/api/data/dialect/SqlDialect.java b/api/src/org/labkey/api/data/dialect/SqlDialect.java index 5c19524f10c..b11a782b34a 100644 --- a/api/src/org/labkey/api/data/dialect/SqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/SqlDialect.java @@ -530,11 +530,11 @@ protected Set getJdbcKeywords(SqlExecutor executor) throws SQLException, // Most callers should use this method public SQLFragment appendInClauseSql(SQLFragment sql, @NotNull Collection params) { - return appendInClauseSql(sql, params, null); + return appendInClauseSqlWithCustomInClauseGenerator(sql, params, null); } - // Use in cases where the default temp schema won't do, e.g., you need to apply a large IN clause in an external data source - public SQLFragment appendInClauseSql(SQLFragment sql, @NotNull Collection params, InClauseGenerator tempTableGenerator) + // 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) { return DEFAULT_GENERATOR.appendInClauseSql(sql, params); } diff --git a/api/src/org/labkey/api/migration/DatabaseMigrationConfiguration.java b/api/src/org/labkey/api/migration/DatabaseMigrationConfiguration.java index 9d0a549cb12..49cd45b4ff5 100644 --- a/api/src/org/labkey/api/migration/DatabaseMigrationConfiguration.java +++ b/api/src/org/labkey/api/migration/DatabaseMigrationConfiguration.java @@ -19,7 +19,7 @@ default void beforeMigration(){} DbScope getTargetScope(); @NotNull Set getSkipSchemas(); Predicate getColumnNameFilter(); - @Nullable TableSelector getTableSelector(DbSchemaType schemaType, TableInfo sourceTable, TableInfo targetTable, Set selectColumnNames, MigrationSchemaHandler schemaHandler); + @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(){} } diff --git a/api/src/org/labkey/api/migration/DatabaseMigrationService.java b/api/src/org/labkey/api/migration/DatabaseMigrationService.java index ec2c0760a7c..b5ec00fac29 100644 --- a/api/src/org/labkey/api/migration/DatabaseMigrationService.java +++ b/api/src/org/labkey/api/migration/DatabaseMigrationService.java @@ -48,6 +48,7 @@ default void migrate(DatabaseMigrationConfiguration configuration) // By default, no-op implementations default void registerSchemaHandler(MigrationSchemaHandler schemaHandler) {} + default void registerTableHandler(MigrationTableHandler tableHandler) {} default void registerMigrationFilter(MigrationFilter filter) {} default @Nullable MigrationFilter getMigrationFilter(String propertyName) diff --git a/api/src/org/labkey/api/migration/DefaultDatabaseMigrationConfiguration.java b/api/src/org/labkey/api/migration/DefaultDatabaseMigrationConfiguration.java index 7dea199479d..8b5017a0edf 100644 --- a/api/src/org/labkey/api/migration/DefaultDatabaseMigrationConfiguration.java +++ b/api/src/org/labkey/api/migration/DefaultDatabaseMigrationConfiguration.java @@ -43,7 +43,7 @@ public Predicate getColumnNameFilter() } @Override - public TableSelector getTableSelector(DbSchemaType schemaType, TableInfo sourceTable, TableInfo targetTable, Set selectColumnNames, MigrationSchemaHandler schemaHandler) + public TableSelector getTableSelector(DbSchemaType schemaType, TableInfo sourceTable, TableInfo targetTable, Set selectColumnNames, MigrationSchemaHandler schemaHandler, @Nullable MigrationTableHandler tableHandler) { return null; } diff --git a/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java b/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java index 08f4fcd3c2c..e54fe4953a0 100644 --- a/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java +++ b/api/src/org/labkey/api/migration/DefaultMigrationSchemaHandler.java @@ -1,5 +1,6 @@ package org.labkey.api.migration; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.attachments.AttachmentService; @@ -27,8 +28,11 @@ import org.labkey.api.query.FieldKey; import org.labkey.api.query.SchemaKey; import org.labkey.api.query.TableSorter; +import org.labkey.api.util.ConfigurationException; import org.labkey.api.util.GUID; +import org.labkey.api.util.JobRunner; import org.labkey.api.util.StringUtilsLabKey; +import org.labkey.api.util.logging.LogHelper; import java.util.ArrayList; import java.util.Arrays; @@ -38,10 +42,13 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; public class DefaultMigrationSchemaHandler implements MigrationSchemaHandler { + private static final Logger LOG = LogHelper.getLogger(DefaultMigrationSchemaHandler.class, "Migration shutdown status"); + private final DbSchema _schema; public DefaultMigrationSchemaHandler(DbSchema schema) @@ -77,7 +84,7 @@ public List getTablesToCopy() if (!allTables.isEmpty()) { - DatabaseMigrationService.LOG.info("These tables were removed by TableSorter: {}", allTables); + LOG.info("These tables were removed by TableSorter: {}", allTables); } return sortedTables.stream() @@ -250,12 +257,13 @@ public void copyAttachments(DatabaseMigrationConfiguration configuration, DbSche Collection entityIds = new SqlSelector(targetSchema, sql).getCollection(String.class); SQLFragment selectParents = new SQLFragment("Parent"); // This query against the source database is likely to contain a large IN clause, so use an alternative InClauseGenerator - sourceSchema.getSqlDialect().appendInClauseSql(selectParents, entityIds, getTempTableInClauseGenerator(sourceSchema.getScope())); + sourceSchema.getSqlDialect().appendInClauseSqlWithCustomInClauseGenerator(selectParents, entityIds, getTempTableInClauseGenerator(sourceSchema.getScope())); copyAttachments(configuration, sourceSchema, new SQLClause(selectParents), type); } - - // TODO: fail if type.getSelectParentEntityIdsSql() returns null? - // TODO: throw if some registered AttachmentType is not seen + else + { + throw new ConfigurationException("AttachmentType \"" + type.getUniqueName() + "\" is not configured to find parent EntityIds!"); + } }); } @@ -267,33 +275,46 @@ protected InClauseGenerator getTempTableInClauseGenerator(DbScope sourceScope) } private static final Set SEEN = new HashSet<>(); + private static final JobRunner ATTACHMENT_JOB_RUNNER = new JobRunner("Attachment JobRunner", 1); // Copy all core.Documents rows that match the provided filter clause - protected void copyAttachments(DatabaseMigrationConfiguration configuration, DbSchema sourceSchema, FilterClause filterClause, AttachmentType... type) + protected final void copyAttachments(DatabaseMigrationConfiguration configuration, DbSchema sourceSchema, FilterClause filterClause, AttachmentType... type) { SEEN.addAll(Arrays.asList(type)); String additionalMessage = " associated with " + Arrays.stream(type).map(t -> t.getClass().getSimpleName()).collect(Collectors.joining(", ")); TableInfo sourceDocumentsTable = sourceSchema.getScope().getSchema("core", DbSchemaType.Migration).getTable("Documents"); TableInfo targetDocumentsTable = CoreSchema.getInstance().getTableInfoDocuments(); - DatabaseMigrationService.get().copySourceTableToTargetTable(configuration, sourceDocumentsTable, targetDocumentsTable, DbSchemaType.Module, false, additionalMessage, new DefaultMigrationSchemaHandler(CoreSchema.getInstance().getSchema()) + + // Queue up the core.Documents transfers and let them run in the background + ATTACHMENT_JOB_RUNNER.execute(() -> DatabaseMigrationService.get().copySourceTableToTargetTable(configuration, sourceDocumentsTable, targetDocumentsTable, DbSchemaType.Module, false, additionalMessage, new DefaultMigrationSchemaHandler(CoreSchema.getInstance().getSchema()) { @Override public FilterClause getTableFilterClause(TableInfo sourceTable, Set containers) { return filterClause; } - }); + })); } - public static void logUnseenAttachmentTypes() + // Global (not schema- or configuration-specific) cleanup + public static void afterMigration() throws InterruptedException { + // Report any unseen attachment types Set unseen = new HashSet<>(AttachmentService.get().getAttachmentTypes()); unseen.removeAll(SEEN); if (unseen.isEmpty()) - DatabaseMigrationService.LOG.info("All AttachmentTypes have been seen"); + 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(", "))); + + // Shut down the attachment JobRunner + LOG.info("Waiting for core.Documents background transfer to complete"); + ATTACHMENT_JOB_RUNNER.shutdown(); + if (ATTACHMENT_JOB_RUNNER.awaitTermination(1, TimeUnit.HOURS)) + LOG.info("core.Documents background transfer is complete"); else - DatabaseMigrationService.LOG.info("These AttachmentTypes have not been seen: {}", unseen.stream().map(type -> type.getClass().getSimpleName()).collect(Collectors.joining(", "))); + LOG.error("core.Documents background transfer did not complete after one hour! Giving up."); } @Override diff --git a/api/src/org/labkey/api/migration/MigrationTableHandler.java b/api/src/org/labkey/api/migration/MigrationTableHandler.java new file mode 100644 index 00000000000..37ae71f7206 --- /dev/null +++ b/api/src/org/labkey/api/migration/MigrationTableHandler.java @@ -0,0 +1,18 @@ +package org.labkey.api.migration; + +import org.labkey.api.data.SimpleFilter; +import org.labkey.api.data.TableInfo; +import org.labkey.api.util.GUID; + +import java.util.Set; + +/** + * Rarely needed, this interface lets a module filter the rows of another module's table. The specific use case: LabBook + * needs to filter the compliance.SignedSnapshots table of snapshots associated with Notebooks that are excluded by a + * NotebookFilter. + */ +public interface MigrationTableHandler +{ + TableInfo getTableInfo(); + void adjustFilter(TableInfo sourceTable, SimpleFilter filter, Set containers); +} diff --git a/api/src/org/labkey/api/util/JobRunner.java b/api/src/org/labkey/api/util/JobRunner.java index a0d0b59ca00..9b1998dbc58 100644 --- a/api/src/org/labkey/api/util/JobRunner.java +++ b/api/src/org/labkey/api/util/JobRunner.java @@ -18,9 +18,11 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.NotNull; import org.labkey.api.data.DbScope; import java.util.HashMap; +import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.Future; @@ -51,7 +53,7 @@ public class JobRunner implements Executor private static final JobRunner _defaultJobRunner = new JobRunner("Default", 1); private final ScheduledThreadPoolExecutor _executor; - private final HashMap _jobs = new HashMap<>(); + private final Map, Job> _jobs = new HashMap<>(); public JobRunner(String name, int max) @@ -77,11 +79,6 @@ public void shutdownPre() { _executor.shutdown(); } - - @Override - public void shutdownStarted() - { - } }); } @@ -111,11 +108,16 @@ public void shutdown() _executor.shutdown(); } + public boolean awaitTermination(long timeout, @NotNull TimeUnit unit) throws InterruptedException + { + return _executor.awaitTermination(timeout, unit); + } + /** * This will schedule the runnable to execute immediately, with no delay */ @Override - public void execute(Runnable command) + public void execute(@NotNull Runnable command) { execute(command, 0); } @@ -132,7 +134,7 @@ public void execute(Runnable command, long delay) { synchronized (_jobs) { - Future task = _executor.schedule(command, delay, TimeUnit.MILLISECONDS); + Future task = _executor.schedule(command, delay, TimeUnit.MILLISECONDS); if (command instanceof Job job) { job._task = task; @@ -141,7 +143,7 @@ public void execute(Runnable command, long delay) } } - public Future submit(Runnable run) + public Future submit(Runnable run) { if (run instanceof Job) { @@ -221,13 +223,13 @@ protected void afterExecute(Runnable r, Throwable t) } else { - if (r instanceof Future) + if (r instanceof Future f) { if (null == t) { try { - ((Future)r).get(); + f.get(); } catch (ExecutionException x) { @@ -277,7 +279,7 @@ static class JobThreadFactory implements ThreadFactory } @Override - public Thread newThread(Runnable r) + public Thread newThread(@NotNull Runnable r) { Thread t = new Thread(group, r, namePrefix + threadNumber.getAndIncrement(), 0); if (t.isDaemon()) diff --git a/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java b/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java index db5b16f572d..75ade1977e5 100644 --- a/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java +++ b/experiment/src/org/labkey/experiment/ExperimentMigrationSchemaHandler.java @@ -4,34 +4,42 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.attachments.AttachmentType; +import org.labkey.api.collections.CsvSet; 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; import org.labkey.api.data.SimpleFilter.FilterClause; import org.labkey.api.data.SimpleFilter.InClause; +import org.labkey.api.data.SimpleFilter.NotClause; import org.labkey.api.data.SimpleFilter.OrClause; import org.labkey.api.data.SimpleFilter.SQLClause; import org.labkey.api.data.SqlExecutor; import org.labkey.api.data.TableInfo; +import org.labkey.api.data.TableSelector; import org.labkey.api.exp.OntologyManager; import org.labkey.api.exp.api.ExpProtocolAttachmentType; import org.labkey.api.exp.api.ExpRunAttachmentType; +import org.labkey.api.migration.AssaySkipContainers; import org.labkey.api.migration.DatabaseMigrationConfiguration; import org.labkey.api.migration.DefaultMigrationSchemaHandler; import org.labkey.api.query.FieldKey; import org.labkey.api.util.GUID; +import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.util.logging.LogHelper; import org.labkey.experiment.api.ExperimentServiceImpl; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Set; class ExperimentMigrationSchemaHandler extends DefaultMigrationSchemaHandler { - private static final Logger LOG = LogHelper.getLogger(ExperimentMigrationSchemaHandler.class, "Progress of exp deletes during database migration"); + private static final Logger LOG = LogHelper.getLogger(ExperimentMigrationSchemaHandler.class, "Progress of database migration"); public ExperimentMigrationSchemaHandler() { @@ -65,27 +73,124 @@ public void beforeSchema() @Override public List getTablesToCopy() { - // No need to populate the MaterialIndexed or DataIndexed tables -- new server should be completely re-indexed after migration + // No need to populate the MaterialIndexed or DataIndexed tables -- new server will be completely re-indexed after migration List tables = super.getTablesToCopy(); tables.remove(ExperimentServiceImpl.get().getTinfoMaterialIndexed()); tables.remove(ExperimentServiceImpl.get().getTinfoDataIndexed()); return tables; } + @Override + public FilterClause getTableFilterClause(TableInfo sourceTable, Set containers) + { + 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); + 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) + { + FilterClause excludedRowIdClause = getExcludedRowIdClause(sourceTable, runIdFieldKey); + + return excludedRowIdClause == null ? + containerClause : + new AndClause( + containerClause, + nullable ? + new OrClause( + new CompareClause(runIdFieldKey, CompareType.ISBLANK, null), + excludedRowIdClause + ) : + excludedRowIdClause + ); + } + + @Nullable FilterClause getExcludedRowIdClause(TableInfo sourceTable, FieldKey runIdFieldKey) + { + Collection experimentRunsToExclude = getExcludedExperimentRunRowIds(sourceTable.getSchema()); + + return experimentRunsToExclude.isEmpty() ? + null : + new NotClause(new InClause(runIdFieldKey, experimentRunsToExclude, getTempTableInClauseGenerator(sourceTable.getSchema().getScope()))); + } + + private Collection _excludedExperimentRunRowIds = null; + + private @NotNull Collection getExcludedExperimentRunRowIds(DbSchema schema) + { + if (null == _excludedExperimentRunRowIds) + { + 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); + + // 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))" + ); + + // Select all assay runs in the 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(")") + ); + + // 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(")") + ); + + // Select all assay runs that need to be excluded + SimpleFilter filter = new SimpleFilter( + 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")); + } + } + + return _excludedExperimentRunRowIds; + } + @Override public FilterClause getContainerClause(TableInfo sourceTable, Set containers) { -// Set assayFilteredContainers = assayFilteredContainers(containers); return switch (sourceTable.getName()) { -// case "ExperimentRun", "ProtocolApplication" -> super.getContainerClause(sourceTable, assayFilteredContainers); -// case "Data" -> new AndClause( -// new InClause(FieldKey.fromParts("Container"), containers), -// new OrClause( -// new CompareClause(FieldKey.fromParts("RunId"), CompareType.ISBLANK, null), -// new InClause(FieldKey.fromParts("RunId", "Container"), assayFilteredContainers) -// ) -// ); case "DataInput" -> new AndClause( new InClause(FieldKey.fromParts("DataId", "Container"), containers), new InClause(FieldKey.fromParts("TargetApplicationId", "RunId", "Container"), containers) diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 67ab8161805..592f80e9d7b 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -16,7 +16,6 @@ package org.labkey.experiment; import org.apache.commons.lang3.math.NumberUtils; -import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.admin.FolderSerializationRegistry; @@ -31,9 +30,12 @@ import org.labkey.api.data.ContainerManager; import org.labkey.api.data.CoreSchema; import org.labkey.api.data.DbSchema; +import org.labkey.api.data.DbSchemaType; import org.labkey.api.data.JdbcType; import org.labkey.api.data.NameGenerator; import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.SimpleFilter; +import org.labkey.api.data.SimpleFilter.FilterClause; import org.labkey.api.data.SqlSelector; import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; @@ -74,6 +76,7 @@ import org.labkey.api.files.TableUpdaterFileListener; import org.labkey.api.migration.DatabaseMigrationService; import org.labkey.api.migration.ExperimentDeleteService; +import org.labkey.api.migration.MigrationTableHandler; import org.labkey.api.module.ModuleContext; import org.labkey.api.module.ModuleLoader; import org.labkey.api.module.SpringModule; @@ -82,6 +85,7 @@ import org.labkey.api.ontology.Quantity; import org.labkey.api.ontology.Unit; import org.labkey.api.pipeline.PipelineService; +import org.labkey.api.query.FieldKey; import org.labkey.api.query.FilteredTable; import org.labkey.api.query.QueryService; import org.labkey.api.query.UserSchema; @@ -91,11 +95,11 @@ import org.labkey.api.settings.AppProps; import org.labkey.api.settings.OptionalFeatureService; import org.labkey.api.usageMetrics.UsageMetricsService; +import org.labkey.api.util.GUID; import org.labkey.api.util.JspTestCase; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.util.SystemMaintenance; -import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.AlwaysAvailableWebPartFactory; import org.labkey.api.view.BaseWebPartFactory; import org.labkey.api.view.HttpView; @@ -179,7 +183,6 @@ public class ExperimentModule extends SpringModule { - private static final Logger LOG = LogHelper.getLogger(ExperimentModule.class, "Database migration status"); private static final String SAMPLE_TYPE_WEB_PART_NAME = "Sample Types"; private static final String PROTOCOL_WEB_PART_NAME = "Protocols"; @@ -874,7 +877,59 @@ SELECT COUNT(DISTINCT DD.DomainURI) FROM }); } - DatabaseMigrationService.get().registerSchemaHandler(new ExperimentMigrationSchemaHandler()); + ExperimentMigrationSchemaHandler handler = new ExperimentMigrationSchemaHandler(); + DatabaseMigrationService.get().registerSchemaHandler(handler); + DatabaseMigrationService.get().registerTableHandler(new MigrationTableHandler() + { + @Override + public TableInfo getTableInfo() + { + return DbSchema.get("premium", DbSchemaType.Bare).getTable("Exclusions"); + } + + @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); + } + }); + DatabaseMigrationService.get().registerTableHandler(new MigrationTableHandler() + { + @Override + public TableInfo getTableInfo() + { + return DbSchema.get("premium", DbSchemaType.Bare).getTable("ExclusionMaps"); + } + + @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); + } + }); + DatabaseMigrationService.get().registerTableHandler(new MigrationTableHandler() + { + @Override + public TableInfo getTableInfo() + { + return DbSchema.get("assayrequest", DbSchemaType.Bare).getTable("RequestRunsJunction"); + } + + @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); + } + }); DatabaseMigrationService.get().registerSchemaHandler(new SampleTypeMigrationSchemaHandler()); DataClassMigrationSchemaHandler dcHandler = new DataClassMigrationSchemaHandler(); DatabaseMigrationService.get().registerSchemaHandler(dcHandler); diff --git a/search/src/org/labkey/search/SearchModule.java b/search/src/org/labkey/search/SearchModule.java index f4f9a3a7603..d4cd37bca6e 100644 --- a/search/src/org/labkey/search/SearchModule.java +++ b/search/src/org/labkey/search/SearchModule.java @@ -193,7 +193,7 @@ public void handle(Map properties @Override public List getTablesToCopy() { - return List.of(); // Leave empty -- target server needs to index all documents + return List.of(); // Leave empty -- target server will re-index all documents } }); } From 37ce815d52aae163f8dba4aa213422332784d4c2 Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Wed, 19 Nov 2025 12:35:39 -0800 Subject: [PATCH 2/5] Avoid leaving connection in bad state after exception during precommit task (#7212) --- api/src/org/labkey/api/data/DbScope.java | 31 +++++++++++++++++++----- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/data/DbScope.java b/api/src/org/labkey/api/data/DbScope.java index 70f28bece57..502ff95c684 100644 --- a/api/src/org/labkey/api/data/DbScope.java +++ b/api/src/org/labkey/api/data/DbScope.java @@ -26,6 +26,7 @@ import org.jetbrains.annotations.Nullable; import org.junit.Assert; import org.junit.Test; +import org.labkey.api.action.ApiUsageException; import org.labkey.api.audit.TransactionAuditProvider; import org.labkey.api.cache.Cache; import org.labkey.api.data.ConnectionWrapper.Closer; @@ -2166,8 +2167,7 @@ public static void closeAllConnectionsForCurrentThread() } try { - LOG.warn("Forcing close of still-pending transaction object. Current stack is ", new Throwable()); - LOG.warn("Forcing close of still-pending transaction object started at ", t._creation); + LOG.warn("Forcing close of still-pending transaction object started at {}. Current stack is ", t._creation, new Throwable()); t.close(); } catch (ConnectionAlreadyReleasedException ignored) @@ -2715,16 +2715,17 @@ public void commit() conn.commit(); conn.setAutoCommit(true); LOG.debug("setAutoCommit(true)"); - if (null != _closeOnClose) - try { _closeOnClose.close(); } catch (Exception ignore) {} } finally { + if (null != _closeOnClose) + try { _closeOnClose.close(); } catch (Exception ignore) {} if (null != conn) conn.internalClose(); - } - popCurrentTransaction(); + // Make sure to pop whether we successfully committed or not + popCurrentTransaction(); + } CommitTaskOption.POSTCOMMIT.run(this); } @@ -3164,6 +3165,24 @@ public void testAutoCommitFailure() closeAllConnectionsForCurrentThread(); } + @Test + public void tesCommitTaskFailure() + { + String message = "Expected failure"; + try (Transaction t = getLabKeyScope().ensureTransaction()) + { + t.addCommitTask(() -> { throw new ApiUsageException("Expected failure"); }, CommitTaskOption.PRECOMMIT); + t.commit(); + fail("Shouldn't have gotten here, expected ApiUsageException"); + } + catch (ApiUsageException e) + { + assertEquals("Bad message", message, e.getMessage()); + } + assertFalse(getLabKeyScope().isTransactionActive()); + closeAllConnectionsForCurrentThread(); + } + @Test public void testLockReleasedException() { From 6918fbfba44533d997729ca5b293eb61b90cc649 Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Thu, 20 Nov 2025 10:56:24 -0800 Subject: [PATCH 3/5] Shut down transaction caches when commit tasks throw exceptions (#7220) * Avoid leaving connection in bad state after exception during precommit task * Ensure caches are closed regardless of success --- api/src/org/labkey/api/data/DbScope.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/api/src/org/labkey/api/data/DbScope.java b/api/src/org/labkey/api/data/DbScope.java index 502ff95c684..afe69fbff21 100644 --- a/api/src/org/labkey/api/data/DbScope.java +++ b/api/src/org/labkey/api/data/DbScope.java @@ -2385,12 +2385,17 @@ public void run(TransactionImpl transaction) // Copy to avoid ConcurrentModificationExceptions, need to retain original order from LinkedHashMap List tasks = new ArrayList<>(getRunnables(transaction).keySet()); - for (Runnable task : tasks) + try { - task.run(); + for (Runnable task : tasks) + { + task.run(); + } + } + finally + { + transaction.closeCaches(); } - - transaction.closeCaches(); } public T add(TransactionImpl transaction, T task) From 3dd0e4fe75766fb649319bfb928504f9119c2017 Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Thu, 20 Nov 2025 15:31:21 -0800 Subject: [PATCH 4/5] Parameter to bypass CSP log deduplication (#7222) --- core/src/org/labkey/core/admin/AdminController.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/src/org/labkey/core/admin/AdminController.java b/core/src/org/labkey/core/admin/AdminController.java index 8afc00320ac..4ec26471185 100644 --- a/core/src/org/labkey/core/admin/AdminController.java +++ b/core/src/org/labkey/core/admin/AdminController.java @@ -11965,8 +11965,12 @@ public Object execute(SimpleApiJsonForm form, BindException errors) throws Excep String urlString = cspReport.optString("document-uri", null); if (urlString != null) { - String path = new URLHelper(urlString).deleteParameters().getURIString(); - if (null == reports.put(path, Boolean.TRUE) || _log.isDebugEnabled()) + URLHelper urlHelper = new URLHelper(urlString); + // URL parameter that tells us to bypass suppression of redundant logging + // Used to make sure that tests of CSP logging are deterministic and convenient + boolean bypassCspDedupe = "true".equals(urlHelper.getParameter("bypassCspDedupe")); + String path = urlHelper.deleteParameters().getURIString(); + if (null == reports.put(path, Boolean.TRUE) || _log.isDebugEnabled() || bypassCspDedupe) { // Don't modify forwarded reports; they already have user, ip, user-agent, etc. from the forwarding server. boolean forwarded = jsonObj.optBoolean("forwarded", false); From 51877cd947de308284dca1d4a4ce411a20c58e79 Mon Sep 17 00:00:00 2001 From: Josh Eckels Date: Thu, 20 Nov 2025 17:13:52 -0800 Subject: [PATCH 5/5] Rudimentary integration with GitHub issues from exception reports (#7216) * Rudimentary integration with GitHub issues from exception reports * Hide Create Issue button when repo is empty string --- .../postgresql/mothership-25.002-25.003.sql | 3 + mothership/resources/schemas/mothership.xml | 7 +- .../mothership/CreateIssueDisplayColumn.java | 102 +++++++++++- .../mothership/ExceptionStackTrace.java | 21 ++- .../labkey/mothership/IssueDisplayColumn.java | 65 ++++++++ .../mothership/MothershipController.java | 150 ++---------------- .../labkey/mothership/MothershipManager.java | 10 +- .../labkey/mothership/MothershipModule.java | 2 +- .../labkey/mothership/editUpgradeMessage.jsp | 4 +- .../mothership/query/MothershipSchema.java | 12 +- .../labkey/mothership/view/createIssue.jsp | 29 ---- .../org/labkey/mothership/view/linkBar.jsp | 2 +- .../mothership/StackTraceDetailsPage.java | 6 +- .../test/tests/mothership/MothershipTest.java | 82 +--------- .../util/mothership/MothershipHelper.java | 6 +- 15 files changed, 224 insertions(+), 277 deletions(-) create mode 100644 mothership/resources/schemas/dbscripts/postgresql/mothership-25.002-25.003.sql create mode 100644 mothership/src/org/labkey/mothership/IssueDisplayColumn.java delete mode 100644 mothership/src/org/labkey/mothership/view/createIssue.jsp diff --git a/mothership/resources/schemas/dbscripts/postgresql/mothership-25.002-25.003.sql b/mothership/resources/schemas/dbscripts/postgresql/mothership-25.002-25.003.sql new file mode 100644 index 00000000000..22d1ae134cc --- /dev/null +++ b/mothership/resources/schemas/dbscripts/postgresql/mothership-25.002-25.003.sql @@ -0,0 +1,3 @@ +ALTER TABLE mothership.ExceptionStackTrace ADD COLUMN GitHubIssue INT; + +ALTER TABLE mothership.ExceptionStackTrace RENAME COLUMN BugNumber TO LabKeyIssue; \ No newline at end of file diff --git a/mothership/resources/schemas/mothership.xml b/mothership/resources/schemas/mothership.xml index 1e0c9b2cefd..84f1fc4ebb0 100644 --- a/mothership/resources/schemas/mothership.xml +++ b/mothership/resources/schemas/mothership.xml @@ -60,7 +60,12 @@ core - + + LabKey Issue + + + GitHub Issue + 4 diff --git a/mothership/src/org/labkey/mothership/CreateIssueDisplayColumn.java b/mothership/src/org/labkey/mothership/CreateIssueDisplayColumn.java index 4e636ca9d6d..0d99eeb040a 100644 --- a/mothership/src/org/labkey/mothership/CreateIssueDisplayColumn.java +++ b/mothership/src/org/labkey/mothership/CreateIssueDisplayColumn.java @@ -16,13 +16,24 @@ package org.labkey.mothership; +import org.apache.commons.lang3.StringUtils; import org.labkey.api.data.ActionButton; import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.ConnectionWrapper; import org.labkey.api.data.DataColumn; import org.labkey.api.data.RenderContext; import org.labkey.api.util.PageFlowUtil; +import org.labkey.api.util.UnexpectedException; +import org.labkey.api.view.HttpView; +import org.labkey.api.view.ViewServlet; import org.labkey.api.writer.HtmlWriter; +import java.io.BufferedReader; +import java.io.IOException; +import java.io.StringReader; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; + public class CreateIssueDisplayColumn extends DataColumn { private final ActionButton _saveButton; @@ -39,11 +50,90 @@ public CreateIssueDisplayColumn(ColumnInfo column, ActionButton saveButton) public void renderDetailsCellContents(RenderContext ctx, HtmlWriter out) { _saveButton.render(ctx, out); - out.write("\t"); - PageFlowUtil.button("Create Issue") - .onClick("document.forms.CreateIssue.elements['AssignedTo'].value = document.forms[" + - PageFlowUtil.jsString(ctx.getCurrentRegion().getFormId()) + - "].elements['AssignedTo'].value; document.forms.CreateIssue.submit();") - .appendTo(out); + + String repo = MothershipManager.get().getGitHubRepo(); + if (!StringUtils.isEmpty(repo)) + { + out.write("\t"); + + StringBuilder body = new StringBuilder(); + + body.append("Created from crash report: "); + body.append(HttpView.currentContext().getRequest().getAttribute(ViewServlet.ORIGINAL_URL_STRING)); + body.append("\n\n"); + + StringBuilder title = new StringBuilder(); + try + { + String stackTraceString = ctx.get(getBoundColumn().getFieldKey(), String.class); + BufferedReader reader = new BufferedReader(new StringReader(stackTraceString)); + String firstLine = reader.readLine(); + // Grab the exception class + String className = firstLine.split(":")[0]; + if (className.lastIndexOf('.') != -1) + { + // Strip off the package name to make the title a little shorter + className = className.substring(className.lastIndexOf('.') + 1); + } + title.append(className); + body.append(firstLine); + String nextLine; + String separator = " in "; + String suffix = ""; + String bestLocation = null; + String firstLocation = null; + while ((nextLine = reader.readLine()) != null) + { + if (firstLocation == null) + { + firstLocation = nextLine; + } + if (bestLocation == null && + ((nextLine.contains("org.labkey") && !nextLine.contains(ConnectionWrapper.class.getPackage().getName())) || + nextLine.contains("org.fhcrc"))) + { + bestLocation = nextLine; + separator = " from "; + } + + if (body.length() + nextLine.length() < 6000) // Don't exceed GitHub's GET URL limit + { + body.append(nextLine); + body.append("\n"); + } + else + { + suffix = "...\n"; + } + } + body.append(suffix); + + if (bestLocation == null) + { + bestLocation = firstLocation; + } + if (bestLocation != null) + { + bestLocation = bestLocation.trim(); + if (bestLocation.startsWith("at ")) + { + bestLocation = bestLocation.substring("at ".length()); + } + title.append(separator); + title.append(bestLocation.split("\\(")[0]); + title.append("()"); + } + } + catch (IOException e) + { + throw UnexpectedException.wrap(e); + } + + String url = "https://github.com/LabKey/" + repo + "/issues/new?title=" + + URLEncoder.encode(title.toString(), StandardCharsets.UTF_8) + + "&body=" + URLEncoder.encode(body.toString(), StandardCharsets.UTF_8); + + PageFlowUtil.button("Create Issue").target("_blank").href(url).appendTo(out); + } } } diff --git a/mothership/src/org/labkey/mothership/ExceptionStackTrace.java b/mothership/src/org/labkey/mothership/ExceptionStackTrace.java index ac0663056d6..4744985c79d 100644 --- a/mothership/src/org/labkey/mothership/ExceptionStackTrace.java +++ b/mothership/src/org/labkey/mothership/ExceptionStackTrace.java @@ -32,7 +32,8 @@ public class ExceptionStackTrace private String _stackTrace; private String _stackTraceHash; private Integer _assignedTo; - private Integer _bugNumber; + private Integer _labkeyIssue; + private Integer _githubIssue; private String _comments; private Date _modified; private User _modifiedBy; @@ -94,14 +95,24 @@ public void setAssignedTo(Integer assignedTo) _assignedTo = assignedTo; } - public Integer getBugNumber() + public Integer getLabKeyIssue() { - return _bugNumber; + return _labkeyIssue; } - public void setBugNumber(Integer bugNumber) + public void setLabKeyIssue(Integer labkeyIssue) { - _bugNumber = bugNumber; + _labkeyIssue = labkeyIssue; + } + + public Integer getGithubIssue() + { + return _githubIssue; + } + + public void setGithubIssue(Integer githubIssue) + { + _githubIssue = githubIssue; } public String getComments() diff --git a/mothership/src/org/labkey/mothership/IssueDisplayColumn.java b/mothership/src/org/labkey/mothership/IssueDisplayColumn.java new file mode 100644 index 00000000000..fab0ef95a45 --- /dev/null +++ b/mothership/src/org/labkey/mothership/IssueDisplayColumn.java @@ -0,0 +1,65 @@ +package org.labkey.mothership; + +import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.DataColumn; +import org.labkey.api.data.RenderContext; +import org.labkey.api.issues.IssuesUrls; +import org.labkey.api.query.FieldKey; +import org.labkey.api.util.PageFlowUtil; +import org.labkey.api.view.ActionURL; + +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; + +import java.util.Set; + +public class IssueDisplayColumn extends DataColumn +{ + public IssueDisplayColumn(ColumnInfo col) + { + super(col); + } + + @Override + public String renderURL(RenderContext ctx) + { + Integer gitHubIssue = ctx.get(getGitHubIssueFieldKey(), Integer.class); + if (gitHubIssue != null) + { + String repo = MothershipManager.get().getGitHubRepo(); + return String.format("https://github.com/LabKey/%s/issues/%d", URLEncoder.encode(repo, StandardCharsets.UTF_8), gitHubIssue); + } + Integer labkeyIssue = ctx.get(getLabKeyIssueFieldKey(), Integer.class); + if (labkeyIssue != null) + { + String path = MothershipManager.get().getIssuesContainer(); + if (path != null) + { + ActionURL url = PageFlowUtil.urlProvider(IssuesUrls.class).getDetailsURL(ContainerManager.getForPath(path)); + url.addParameter("issueId", labkeyIssue); + return url.getLocalURIString(); + } + } + + return super.renderURL(ctx); + } + + @Override + public void addQueryFieldKeys(Set keys) + { + super.addQueryFieldKeys(keys); + keys.add(getGitHubIssueFieldKey()); + keys.add(getLabKeyIssueFieldKey()); + } + + private FieldKey getGitHubIssueFieldKey() + { + return FieldKey.fromString(getBoundColumn().getFieldKey().getParent(), "GitHubIssue"); + } + + private FieldKey getLabKeyIssueFieldKey() + { + return FieldKey.fromString(getBoundColumn().getFieldKey().getParent(), "LabKeyIssue"); + } +} diff --git a/mothership/src/org/labkey/mothership/MothershipController.java b/mothership/src/org/labkey/mothership/MothershipController.java index f3284469b78..cce8cc007db 100644 --- a/mothership/src/org/labkey/mothership/MothershipController.java +++ b/mothership/src/org/labkey/mothership/MothershipController.java @@ -40,7 +40,6 @@ import org.labkey.api.data.ButtonBar; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.CompareType; -import org.labkey.api.data.ConnectionWrapper; import org.labkey.api.data.Container; import org.labkey.api.data.DataColumn; import org.labkey.api.data.DataRegion; @@ -89,7 +88,6 @@ import org.labkey.api.view.UnauthorizedException; import org.labkey.api.view.UpdateView; import org.labkey.api.view.VBox; -import org.labkey.api.view.ViewServlet; import org.labkey.mothership.query.MothershipSchema; import org.labkey.mothership.view.ExceptionListWebPart; import org.labkey.mothership.view.LinkBar; @@ -97,9 +95,7 @@ import org.springframework.validation.Errors; import org.springframework.web.servlet.ModelAndView; -import java.io.BufferedReader; import java.io.IOException; -import java.io.StringReader; import java.net.InetAddress; import java.net.UnknownHostException; import java.sql.SQLException; @@ -107,7 +103,6 @@ import java.util.Collection; import java.util.Collections; import java.util.Date; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -266,32 +261,6 @@ public void addNavTrail(NavTree root) } } - @RequiresPermission(UpdatePermission.class) - public static class CreateIssueFinishedAction extends FormHandlerAction - { - @Override - public void validateCommand(CreateIssueFinishedForm target, Errors errors) - { - } - - @Override - public boolean handlePost(CreateIssueFinishedForm form, BindException errors) - { - ExceptionStackTrace stackTrace = MothershipManager.get().getExceptionStackTrace(form.getExceptionStackTraceId(), getContainer()); - stackTrace.setBugNumber(form.getIssueId()); - stackTrace.setAssignedTo(form.getAssignedTo()); - MothershipManager.get().updateExceptionStackTrace(stackTrace, getUser()); - - return true; - } - - @Override - public URLHelper getSuccessURL(CreateIssueFinishedForm createIssueFinishedForm) - { - return new ActionURL(BeginAction.class, getContainer()); - } - } - @RequiresPermission(UpdatePermission.class) public static class EditUpgradeMessageAction extends SimpleViewAction { @@ -302,7 +271,7 @@ public ModelAndView getView(Object o, BindException errors) form.setCurrentBuildDate(MothershipManager.get().getCurrentBuildDate()); form.setMessage(MothershipManager.get().getUpgradeMessage()); - form.setCreateIssueURL(MothershipManager.get().getCreateIssueURL()); + form.setGithubRepo(MothershipManager.get().getGitHubRepo()); form.setIssuesContainer(MothershipManager.get().getIssuesContainer()); form.setMarketingMessage(MothershipManager.get().getMarketingMessage()); form.setStatusCakeApiKey(StringUtils.trimToNull(MothershipManager.get().getStatusCakeApiKey())); @@ -332,7 +301,7 @@ public boolean handlePost(UpgradeMessageForm form, BindException errors) { MothershipManager.get().setCurrentBuildDate(form.getCurrentBuildDate()); MothershipManager.get().setUpgradeMessage(form.getMessage()); - MothershipManager.get().setCreateIssueURL(form.getCreateIssueURL()); + MothershipManager.get().setGitHubRepo(form.getGithubRepo()); MothershipManager.get().setIssuesContainer(form.getIssuesContainer()); MothershipManager.get().setMarketingMessage(form.getMarketingMessage()); MothershipManager.get().setUptimeContainer(form.getUptimeContainer()); @@ -432,7 +401,7 @@ public void addNavTrail(NavTree root) public static class ShowStackTraceDetailAction extends SimpleViewAction { @Override - public ModelAndView getView(ExceptionStackTraceForm form, BindException errors) throws Exception + public ModelAndView getView(ExceptionStackTraceForm form, BindException errors) { ExceptionStackTrace stackTrace; try @@ -459,67 +428,7 @@ public ModelAndView getView(ExceptionStackTraceForm form, BindException errors) summaryGridView.setShowBorders(true); summaryGridView.setShadeAlternatingRows(true); summaryGridView.setButtonBarPosition(DataRegion.ButtonBarPosition.TOP); - return new VBox(updateView, summaryGridView, constructCreateIssueForm(stackTrace)); - } - - private JspView> constructCreateIssueForm(ExceptionStackTrace stackTrace) throws IOException - { - // Moved from CreateIssueDisplayColumn. Instead of piggybacking off the ExceptionStackTraceForm, - // we now have a separate hidden form on the page to have control over exactly which fields - // are submitted. - ActionURL callbackURL = getViewContext().getActionURL().clone(); - callbackURL.setAction(MothershipController.CreateIssueFinishedAction.class); - Map cifModel = new HashMap<>(); - cifModel.put("callbackURL", callbackURL.toString()); - String originalURL = (String)getViewContext().getRequest().getAttribute(ViewServlet.ORIGINAL_URL_STRING); - StringBuilder body = new StringBuilder(); - body.append("Created from crash report: "); - body.append(originalURL); - body.append("\n\n"); - String stackTraceString = stackTrace.getStackTrace(); - body.append(stackTraceString); - - StringBuilder title = new StringBuilder(); - BufferedReader reader = new BufferedReader(new StringReader(stackTraceString)); - // Grab the exception class - String className = reader.readLine().split(":")[0]; - if (className.lastIndexOf('.') != -1) - { - // Strip off the package name to make the title a little shorter - className = className.substring(className.lastIndexOf('.') + 1); - } - title.append(className); - String firstLocation = reader.readLine(); - String location = firstLocation; - String separator = " in "; - while (location != null && - (!location.contains("org.labkey") || location.contains(ConnectionWrapper.class.getPackage().getName())) && - !location.contains("org.fhcrc")) - { - location = reader.readLine(); - separator = " from "; - } - - if (location == null) - { - location = firstLocation; - } - if (location != null) - { - location = location.trim(); - if (location.startsWith("at ")) - { - location = location.substring("at ".length()); - } - title.append(separator); - title.append(location.split("\\(")[0]); - title.append("()"); - } - cifModel.put("body", body.toString()); - cifModel.put("title", title.toString()); - cifModel.put("action", MothershipManager.get().getCreateIssueURL()); - - return new JspView<>("/org/labkey/mothership/view/createIssue.jsp", cifModel); + return new VBox(updateView, summaryGridView); } @Override @@ -1584,7 +1493,7 @@ else if (!form.isIgnore()) } else { - exceptionStackTrace.setBugNumber(-1); + exceptionStackTrace.setGithubIssue(-1); } } MothershipManager.get().updateExceptionStackTrace(exceptionStackTrace, getUser()); @@ -1734,7 +1643,7 @@ public ExceptionStackTraceUpdateView(ExceptionStackTraceForm form, Container c, TableInfo exceptionStackTraceTable = new MothershipSchema(getViewContext().getUser(), c).getTable("ExceptionStackTrace"); getDataRegion().setTable(exceptionStackTraceTable); - getDataRegion().addColumns(exceptionStackTraceTable, "ExceptionStackTraceId,StackTrace,BugNumber,Comments"); + getDataRegion().addColumns(exceptionStackTraceTable, "ExceptionStackTraceId,StackTrace,GitHubIssue,LabKeyIssue,Comments"); getDataRegion().addHiddenFormField("exceptionStackTraceId", Integer.toString(form.getBean().getExceptionStackTraceId())); getDataRegion().addDisplayColumn(new AssignedToDisplayColumn(exceptionStackTraceTable.getColumn("AssignedTo"), c)); getDataRegion().getDisplayColumn(1).setVisible(false); @@ -1843,43 +1752,6 @@ public ServerSessionForm() } } - public static class CreateIssueFinishedForm - { - private int _exceptionStackTraceId; - private int _issueId; - private Integer _assignedTo; - - public int getExceptionStackTraceId() - { - return _exceptionStackTraceId; - } - - public void setExceptionStackTraceId(int exceptionStackTraceId) - { - _exceptionStackTraceId = exceptionStackTraceId; - } - - public int getIssueId() - { - return _issueId; - } - - public void setIssueId(int issueId) - { - _issueId = issueId; - } - - public Integer getAssignedTo() - { - return _assignedTo; - } - - public void setAssignedTo(Integer assignedTo) - { - _assignedTo = assignedTo; - } - } - public static class SoftwareReleaseForm extends BeanViewForm { public SoftwareReleaseForm(SoftwareRelease release) @@ -1899,7 +1771,7 @@ public static class UpgradeMessageForm { private Date _currentBuildDate; private String _message; - private String _createIssueURL; + private String _githubRepo; private String _issuesContainer; private String _marketingMessage; private String _statusCakeApiKey; @@ -1925,14 +1797,14 @@ public void setMessage(String message) _message = message; } - public void setCreateIssueURL(String createIssueURL) + public void setGithubRepo(String githubRepo) { - _createIssueURL = createIssueURL; + _githubRepo = githubRepo; } - public String getCreateIssueURL() + public String getGithubRepo() { - return _createIssueURL; + return _githubRepo; } public void setIssuesContainer(String issuesContainer) diff --git a/mothership/src/org/labkey/mothership/MothershipManager.java b/mothership/src/org/labkey/mothership/MothershipManager.java index f5a9c22f104..07d407a1a5a 100644 --- a/mothership/src/org/labkey/mothership/MothershipManager.java +++ b/mothership/src/org/labkey/mothership/MothershipManager.java @@ -61,8 +61,8 @@ public class MothershipManager private static final String MOTHERSHIP_SECURE_CATEGORY = "mothershipSecure"; private static final String CURRENT_BUILD_DATE_PROP = "currentBuildDate"; private static final String UPGRADE_MESSAGE_PROP = "upgradeMessage"; - private static final String CREATE_ISSUE_URL_PROP = "createIssueURL"; private static final String ISSUES_CONTAINER_PROP = "issuesContainer"; + private static final String GITHUB_REPO = "githubRepo"; private static final String MARKETING_MESSAGE_PROP = "marketingMessage"; private static final String UPTIME_CONTAINER_PROP = "uptimeContainer"; private static final String STATUS_CAKE_API_KEY_PROP = "statusCakeApiKey"; @@ -550,14 +550,14 @@ public void setMarketingMessage(String message) saveProperty(MARKETING_MESSAGE_PROP, message); } - public String getCreateIssueURL() + public String getGitHubRepo() { - return getStringProperty(CREATE_ISSUE_URL_PROP); + return getStringProperty(GITHUB_REPO); } - public void setCreateIssueURL(String url) + public void setGitHubRepo(String repo) { - saveProperty(CREATE_ISSUE_URL_PROP, url); + saveProperty(GITHUB_REPO, repo); } public void updateExceptionStackTrace(ExceptionStackTrace stackTrace, User user) diff --git a/mothership/src/org/labkey/mothership/MothershipModule.java b/mothership/src/org/labkey/mothership/MothershipModule.java index 9bf476a5bde..f62c0ef3801 100644 --- a/mothership/src/org/labkey/mothership/MothershipModule.java +++ b/mothership/src/org/labkey/mothership/MothershipModule.java @@ -55,7 +55,7 @@ public String getName() @Override public Double getSchemaVersion() { - return 25.002; + return 25.003; } @Override diff --git a/mothership/src/org/labkey/mothership/editUpgradeMessage.jsp b/mothership/src/org/labkey/mothership/editUpgradeMessage.jsp index b12d812b9a7..4ca241a59f0 100644 --- a/mothership/src/org/labkey/mothership/editUpgradeMessage.jsp +++ b/mothership/src/org/labkey/mothership/editUpgradeMessage.jsp @@ -54,10 +54,10 @@ - + - + diff --git a/mothership/src/org/labkey/mothership/query/MothershipSchema.java b/mothership/src/org/labkey/mothership/query/MothershipSchema.java index 39aead99dbf..560f4f729cc 100644 --- a/mothership/src/org/labkey/mothership/query/MothershipSchema.java +++ b/mothership/src/org/labkey/mothership/query/MothershipSchema.java @@ -54,6 +54,7 @@ import org.labkey.api.util.StringExpressionFactory; import org.labkey.api.view.ActionURL; import org.labkey.api.view.ViewContext; +import org.labkey.mothership.IssueDisplayColumn; import org.labkey.mothership.MothershipController; import org.labkey.mothership.MothershipManager; import org.labkey.mothership.StackTraceDisplayColumn; @@ -356,10 +357,15 @@ public FilteredTable createExceptionStackTraceTable(ContainerF result.wrapAllColumns(true); result.getMutableColumnOrThrow("StackTrace").setDisplayColumnFactory(StackTraceDisplayColumn::new); + SQLFragment issueSql = new SQLFragment("COALESCE(GitHubIssue, LabKeyIssue)"); + ExprColumn issueCol = new ExprColumn(result, "Issue", issueSql, JdbcType.VARCHAR); + issueCol.setDisplayColumnFactory(IssueDisplayColumn::new); + result.addColumn(issueCol); + String path = MothershipManager.get().getIssuesContainer(); ActionURL issueURL = PageFlowUtil.urlProvider(IssuesUrls.class).getDetailsURL(ContainerManager.getForPath(path)); - issueURL.addParameter("issueId", "${BugNumber}"); - result.getMutableColumnOrThrow("BugNumber").setURL(StringExpressionFactory.createURL(issueURL)); + issueURL.addParameter("issueId", "${LabKeyIssue}"); + result.getMutableColumnOrThrow("LabKeyIssue").setURL(StringExpressionFactory.createURL(issueURL)); result.setTitleColumn("ExceptionStackTraceId"); DetailsURL url = new DetailsURL(new ActionURL(MothershipController.ShowStackTraceDetailAction.class, getContainer()), Collections.singletonMap("exceptionStackTraceId", "ExceptionStackTraceId")); @@ -378,7 +384,7 @@ public FilteredTable createExceptionStackTraceTable(ContainerF defaultCols.add(FieldKey.fromParts("ExceptionStackTraceId")); defaultCols.add(FieldKey.fromParts("Instances")); defaultCols.add(FieldKey.fromParts("LastReport")); - defaultCols.add(FieldKey.fromParts("BugNumber")); + defaultCols.add(FieldKey.fromParts("Issue")); defaultCols.add(FieldKey.fromParts("AssignedTo")); defaultCols.add(FieldKey.fromParts("StackTrace")); defaultCols.add(FieldKey.fromParts("ModifiedBy")); diff --git a/mothership/src/org/labkey/mothership/view/createIssue.jsp b/mothership/src/org/labkey/mothership/view/createIssue.jsp deleted file mode 100644 index a39aa163111..00000000000 --- a/mothership/src/org/labkey/mothership/view/createIssue.jsp +++ /dev/null @@ -1,29 +0,0 @@ -<% -/* - * Copyright (c) 2013-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. - */ -%> -<%@ page import="java.util.Map" %> -<%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib" %> -<%@ page extends="org.labkey.api.jsp.JspBase" %> -<% Map ctx = (Map)getModelBean(); %> - - - "/> - "/> - "/> - - - \ No newline at end of file diff --git a/mothership/src/org/labkey/mothership/view/linkBar.jsp b/mothership/src/org/labkey/mothership/view/linkBar.jsp index 1fd54ccf62e..86ad972900e 100644 --- a/mothership/src/org/labkey/mothership/view/linkBar.jsp +++ b/mothership/src/org/labkey/mothership/view/linkBar.jsp @@ -37,7 +37,7 @@ <%}%> <% if (getUser() != null && !getUser().isGuest()) { ActionURL myExceptions = new ActionURL(MothershipController.ShowExceptionsAction.class, c); - myExceptions.addFilter("ExceptionSummary", FieldKey.fromParts("BugNumber"), CompareType.ISBLANK, null); + myExceptions.addFilter("ExceptionSummary", FieldKey.fromParts("Issue"), CompareType.ISBLANK, null); myExceptions.addFilter("ExceptionSummary", FieldKey.fromParts("AssignedTo", "DisplayName"), CompareType.EQUAL, getUser().getDisplayName(getUser())); %> <%= link("My Exceptions", myExceptions)%> diff --git a/mothership/test/src/org/labkey/test/pages/mothership/StackTraceDetailsPage.java b/mothership/test/src/org/labkey/test/pages/mothership/StackTraceDetailsPage.java index 0cb6c0b49bb..d2a7c88b4cd 100644 --- a/mothership/test/src/org/labkey/test/pages/mothership/StackTraceDetailsPage.java +++ b/mothership/test/src/org/labkey/test/pages/mothership/StackTraceDetailsPage.java @@ -48,9 +48,9 @@ public static StackTraceDetailsPage beginAt(WebDriverWrapper driver, String cont return new StackTraceDetailsPage(driver.getDriver()); } - public Input bugNumber() + public Input githubIssue() { - return elementCache().bugNumberInput; + return elementCache().githubIssueInput; } public Input comments() @@ -94,7 +94,7 @@ protected ElementCache newElementCache() protected class ElementCache extends LabKeyPage.ElementCache { - Input bugNumberInput = new Input(Locator.name("BugNumber").findWhenNeeded(this), getDriver()); + Input githubIssueInput = new Input(Locator.name("GitHubIssue").findWhenNeeded(this), getDriver()); Input commentsTextArea = new Input(Locator.name("Comments").findWhenNeeded(this), getDriver()); OptionSelect assignedToSelect = OptionSelect(Locator.name("AssignedTo")).findWhenNeeded(this); WebElement saveButton = Locator.lkButton("Save").findWhenNeeded(this); diff --git a/mothership/test/src/org/labkey/test/tests/mothership/MothershipTest.java b/mothership/test/src/org/labkey/test/tests/mothership/MothershipTest.java index 5ea92ca5312..26ce1e22c4c 100644 --- a/mothership/test/src/org/labkey/test/tests/mothership/MothershipTest.java +++ b/mothership/test/src/org/labkey/test/tests/mothership/MothershipTest.java @@ -28,20 +28,15 @@ import org.labkey.test.Locators; import org.labkey.test.SortDirection; import org.labkey.test.TestTimeoutException; -import org.labkey.test.WebTestHelper; import org.labkey.test.categories.Daily; import org.labkey.test.pages.issues.InsertPage; import org.labkey.test.pages.mothership.ClientExceptionPage; -import org.labkey.test.pages.mothership.EditUpgradeMessagePage; import org.labkey.test.pages.mothership.ShowExceptionsPage; import org.labkey.test.pages.mothership.ShowExceptionsPage.ExceptionSummaryDataRegion; import org.labkey.test.pages.mothership.StackTraceDetailsPage; import org.labkey.test.util.ApiPermissionsHelper; -import org.labkey.test.util.IssuesHelper; -import org.labkey.test.util.Maps; import org.labkey.test.util.PermissionsHelper.MemberType; import org.labkey.test.util.PostgresOnlyTest; -import org.labkey.test.util.TextSearcher; import org.labkey.test.util.mothership.MothershipHelper; import java.util.ArrayList; @@ -68,9 +63,6 @@ public class MothershipTest extends BaseWebDriverTest implements PostgresOnlyTes private static final String ASSIGNEE2 = "assignee2@mothership.test"; private static final String NON_ASSIGNEE = "non_assignee@mothership.test"; private static final String MOTHERSHIP_GROUP = "Mothership Test Group"; - private static final String ISSUES_PROJECT = "MothershipTest Issues"; - private static final String ISSUES_GROUP = "Issues Group"; - public static final String ISSUES_LIST = "mothershipissues"; private static MothershipHelper _mothershipHelper; // Static to remember site settings between tests private final ApiPermissionsHelper permissionsHelper = new ApiPermissionsHelper(this); @@ -78,7 +70,6 @@ public class MothershipTest extends BaseWebDriverTest implements PostgresOnlyTes @Override protected void doCleanup(boolean afterTest) throws TestTimeoutException { - _containerHelper.deleteProject(ISSUES_PROJECT, false); // Don't delete mothership project _userHelper.deleteUsers(afterTest, ASSIGNEE, ASSIGNEE2, NON_ASSIGNEE); permissionsHelper.deleteGroup(MOTHERSHIP_GROUP, MOTHERSHIP_PROJECT, false); @@ -105,25 +96,6 @@ private void doSetup() permissionsHelper.addUserToProjGroup(ASSIGNEE, MOTHERSHIP_PROJECT, MOTHERSHIP_GROUP); permissionsHelper.addUserToProjGroup(ASSIGNEE2, MOTHERSHIP_PROJECT, MOTHERSHIP_GROUP); permissionsHelper.addMemberToRole(NON_ASSIGNEE, "Project Admin", MemberType.user, MOTHERSHIP_PROJECT); - - EditUpgradeMessagePage configurePage = EditUpgradeMessagePage.beginAt(this); - configurePage.setCreateIssueURL(WebTestHelper.getContextPath() + - WebTestHelper.buildRelativeUrl("issues", ISSUES_PROJECT, "insert", Maps.of("issueDefName", ISSUES_LIST))); - configurePage.setIssuesContainer("/" + ISSUES_PROJECT); - configurePage.save(); - - _containerHelper.createProject(ISSUES_PROJECT, null); - IssuesHelper helper = new IssuesHelper(this); - helper.createNewIssuesList(ISSUES_LIST, _containerHelper); - goToModule("Issues"); - helper.goToAdmin(); - helper.setIssueAssignmentList(null); - clickButton("Save"); - - ApiPermissionsHelper permHelper = new ApiPermissionsHelper(this); - permHelper.createProjectGroup(ISSUES_GROUP, ISSUES_PROJECT); - permHelper.addUserToProjGroup(ASSIGNEE, ISSUES_PROJECT, ISSUES_GROUP); - permHelper.addMemberToRole(ISSUES_GROUP, EDITOR_ROLE, MemberType.group, ISSUES_PROJECT); } @Before @@ -134,37 +106,6 @@ public void preTest() throws Exception _mothershipHelper.setIgnoreExceptions(false); } - @Test - public void testCreateIssue() - { - IssuesHelper issuesHelper = new IssuesHelper(this); - Integer highestIssueId = issuesHelper.getHighestIssueId(ISSUES_PROJECT, ISSUES_LIST); - int stackTraceId = ensureUnassignedException(); - - ShowExceptionsPage showExceptionsPage = ShowExceptionsPage.beginAt(this); - ExceptionSummaryDataRegion exceptionSummary = showExceptionsPage.exceptionSummary(); - StackTraceDetailsPage detailsPage = exceptionSummary.clickStackTrace(stackTraceId); - String stackDetailsUrl = getDriver().getCurrentUrl(); - InsertPage insertPage = detailsPage.clickCreateIssue(); - String expectedTitle = "NullPointerException in org.labkey.devtools.TestController$NpeAction.getView()"; - String issueTitle = insertPage.title().get(); - assertEquals("Wrong issue title", expectedTitle, issueTitle); - String[] expectedComments = new String[] { - "Created from crash report: " + stackDetailsUrl, - "java.lang.NullPointerException", - "TestController.java"}; - assertTextPresentInThisOrder(new TextSearcher(insertPage.comment().get()), expectedComments); - assertEquals("New issue shouldn't be assigned by default", "", insertPage.assignedTo().get().trim()); - insertPage.assignedTo().set(_userHelper.getDisplayNameForEmail(ASSIGNEE)); - insertPage.save(); - Integer newIssueId = issuesHelper.getHighestIssueId(ISSUES_PROJECT, ISSUES_LIST); - assertNotEquals("Didn't create a new issue.", highestIssueId, newIssueId); - detailsPage = ShowExceptionsPage.beginAt(this) - .exceptionSummary() - .clickStackTrace(stackTraceId); - assertEquals("Exception's related issue not set", newIssueId.toString(), detailsPage.bugNumber().get()); - } - @Test public void testAssignException() { @@ -201,24 +142,7 @@ public void testIgnoreExceptionFromDataRegion() exceptionSummary.ignoreSelected(); StackTraceDetailsPage detailsPage = exceptionSummary.clickStackTrace(stackTraceId); - assertEquals("Ignoring exception should set bugNumber", "-1", detailsPage.bugNumber().get()); - } - - @Test - public void testCreateIssueForAssignedException() - { - int stackTraceId = ensureUnassignedException(); - - ShowExceptionsPage showExceptionsPage = ShowExceptionsPage.beginAt(this); - ExceptionSummaryDataRegion exceptionSummary = showExceptionsPage.exceptionSummary(); - exceptionSummary.uncheckAllOnPage(); - exceptionSummary.checkCheckboxByPrimaryKey(stackTraceId); - exceptionSummary.assignSelectedTo(_userHelper.getDisplayNameForEmail(ASSIGNEE)); - - StackTraceDetailsPage detailsPage = exceptionSummary.clickStackTrace(stackTraceId); - InsertPage insertPage = detailsPage.clickCreateIssue(); - assertEquals("Exception assignment != New issue assignment", - _userHelper.getDisplayNameForEmail(ASSIGNEE), insertPage.assignedTo().get()); + assertEquals("Ignoring exception should set GitHub Issue", "-1", detailsPage.githubIssue().get()); } @Test @@ -229,7 +153,7 @@ public void testCombiningIdenticalExceptions() assertEquals("Should group identical exceptions", exceptionIds.get(0), exceptionIds.get(1)); } - @Test @Ignore("These don't actually get grouped") + @Test public void testCombiningSimilarExceptions() { List> actions = new ArrayList<>(); @@ -238,7 +162,7 @@ public void testCombiningSimilarExceptions() List exceptionIds = _mothershipHelper.triggerExceptions(actions); goToMothership(); // Make sure failure screenshot shows new exceptions - assertEquals("Should group same exception type from same action", exceptionIds.get(0), exceptionIds.get(1)); + assertNotEquals("Shouldn't group same exception type from same action with different line numbers", exceptionIds.get(0), exceptionIds.get(1)); } @Test diff --git a/mothership/test/src/org/labkey/test/util/mothership/MothershipHelper.java b/mothership/test/src/org/labkey/test/util/mothership/MothershipHelper.java index 84265168689..88650a972a5 100644 --- a/mothership/test/src/org/labkey/test/util/mothership/MothershipHelper.java +++ b/mothership/test/src/org/labkey/test/util/mothership/MothershipHelper.java @@ -181,14 +181,14 @@ public void resetStackTrace(int exceptionStackTraceId) updateStackTrace(exceptionStackTraceId, "", "", ""); } - public void updateStackTrace(int exceptionStackTraceId, String bugNumber, String comments, String assignedToEmail) + public void updateStackTrace(int exceptionStackTraceId, String githubIssue, String comments, String assignedToEmail) { Connection connection = createDefaultConnection(); SimplePostCommand command = new SimplePostCommand("mothership", "updateStackTrace"); Map params = new HashMap<>(); params.put(ID_COLUMN, exceptionStackTraceId); - if (bugNumber != null) - params.put("BugNumber", bugNumber); + if (githubIssue != null) + params.put("GitHubIssue", githubIssue); if (comments != null) params.put("Comments", comments); if (assignedToEmail != null)