diff --git a/experiment/src/org/labkey/experiment/FileLinkFileListener.java b/experiment/src/org/labkey/experiment/FileLinkFileListener.java index 19dba1fda75..c0cb65f4c7a 100644 --- a/experiment/src/org/labkey/experiment/FileLinkFileListener.java +++ b/experiment/src/org/labkey/experiment/FileLinkFileListener.java @@ -36,6 +36,7 @@ import org.labkey.api.exp.property.PropertyService; import org.labkey.api.files.FileListener; import org.labkey.api.files.TableUpdaterFileListener; +import org.labkey.api.module.ModuleLoader; import org.labkey.api.security.User; import org.labkey.api.util.FileUtil; import org.labkey.experiment.api.ExpMaterialTableImpl; @@ -187,13 +188,19 @@ private void hardTableFileLinkColumns(final ForEachFileLinkColumn block) sql.append(OntologyManager.getTinfoPropertyDescriptor(), "pd"); sql.unsafeAppend(", "); sql.append(OntologyManager.getTinfoPropertyDomain(), "m"); + // Domains can outlive their physical tables, which getSchemaTableInfo() can't load. GH Issue 1756. + sql.unsafeAppend(", information_schema.tables it"); sql.unsafeAppend(" WHERE dd.DomainId = m.DomainId AND pd.PropertyId = m.PropertyId AND pd.RangeURI = ? "); sql.add(PropertyType.FILE_LINK.getTypeUri()); sql.unsafeAppend(" AND dd.StorageTableName IS NOT NULL AND dd.StorageSchemaName IS NOT NULL AND pd.Name IS NOT NULL"); + sql.unsafeAppend(" AND it.table_schema = lower(dd.StorageSchemaName) AND it.table_name = lower(dd.StorageTableName)"); new SqlSelector(OntologyManager.getExpSchema(), sql).forEachMap(row -> { // Find the DbSchema/TableInfo/ColumnInfo for the FileLink column String storageSchemaName = row.get("StorageSchemaName").toString(); + // Domains can outlive the module that provisioned them, and DbSchema.get() throws for unclaimed schemas + if (null == ModuleLoader.getInstance().getModule(OntologyManager.getExpSchema().getScope(), storageSchemaName)) + return; DbSchema schema = DbSchema.get(storageSchemaName, DbSchemaType.Provisioned); Domain domain = PropertyService.get().getDomain((Integer) row.get("DomainId")); // Issue 50781: LKSM: File fields in Sample Types sometimes showing as Text Fields diff --git a/experiment/src/org/labkey/experiment/SpecialCharacterMetricsMaintenanceTask.java b/experiment/src/org/labkey/experiment/SpecialCharacterMetricsMaintenanceTask.java index f21c0ce2fc0..75b576eaf42 100644 --- a/experiment/src/org/labkey/experiment/SpecialCharacterMetricsMaintenanceTask.java +++ b/experiment/src/org/labkey/experiment/SpecialCharacterMetricsMaintenanceTask.java @@ -25,6 +25,7 @@ import org.labkey.api.data.Container; import org.labkey.api.data.DbScope; import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.SqlExecutor; import org.labkey.api.data.SqlSelector; import org.labkey.api.data.TableInfo; import org.labkey.api.data.dialect.SqlDialect; @@ -120,12 +121,12 @@ public void run(Logger log) private void collectChoiceFieldMetrics(DbScope scope, Map> counts) { - SQLFragment sql = new SQLFragment("SELECT pd.rangeuri AS rangeuri"); + SQLFragment sql = SQLFragment.unsafe("SELECT pd.rangeuri AS rangeuri"); for (String ck : CHAR_KEYS) { sql.unsafeAppend(", SUM(CASE WHEN ("); - appendBoolExpr(sql, ck, new SQLFragment("pv.expression")); - sql.unsafeAppend(") THEN 1 ELSE 0 END) AS ").append(ck.toLowerCase()); + appendBoolExpr(sql, ck, SQLFragment.unsafe("pv.expression")); + sql.unsafeAppend(") THEN 1 ELSE 0 END) AS ").appendIdentifier(ck.toLowerCase()); } sql.unsafeAppend(" FROM exp.propertyvalidator pv"); sql.unsafeAppend(" JOIN exp.propertydescriptor pd ON pv.propertyid = pd.propertyid"); @@ -154,15 +155,16 @@ else if (PropertyType.MULTI_CHOICE.getTypeUri().equals(rangeUri)) private void collectObjectPropertyMetrics(DbScope scope, Map> counts) { - SQLFragment sql = new SQLFragment("SELECT"); + SQLFragment sql = SQLFragment.unsafe("SELECT "); boolean first = true; for (String ck : CHAR_KEYS) { - sql.append(first ? " " : ", "); + if (!first) + sql.unsafeAppend(", "); first = false; sql.unsafeAppend("COUNT(DISTINCT CASE WHEN ("); - appendBoolExpr(sql, ck, new SQLFragment("stringvalue")); - sql.unsafeAppend(") THEN propertyid END) AS ").append(ck.toLowerCase()); + appendBoolExpr(sql, ck, SQLFragment.unsafe("stringvalue")); + sql.unsafeAppend(") THEN propertyid END) AS ").appendIdentifier(ck.toLowerCase()); } sql.unsafeAppend(" FROM exp.objectproperty WHERE stringvalue IS NOT NULL"); @@ -179,13 +181,26 @@ private void collectObjectPropertyMetrics(DbScope scope, Map> counts, Logger log) + { + for (Map.Entry> entry : getScanTargets(scope).entrySet()) + scanTable(scope, dialect, entry.getKey(), entry.getValue(), counts, log); + } + + // Domain metadata can outlive its physical table or column, so scan only what the catalog confirms exists. GH Issue 1756. + // Unquoted and illegal storage names both resolve to lowercase physical names (see PropertyDescriptor.getLegalSelectNameFromStorageName). + private static final String PHYSICAL_TABLE_JOIN = "JOIN information_schema.columns ic ON ic.table_schema = lower(dd.storageschemaname)" + + " AND ic.table_name = lower(dd.storagetablename)\n"; + + private Map> getScanTargets(DbScope scope) { // Enumerate provisioned Text/Multiline columns, excluding Text Choice fields. - SQLFragment enumSql = new SQLFragment( + SQLFragment enumSql = SQLFragment.unsafe( "SELECT dd.storageschemaname, dd.storagetablename, pd.storagecolumnname, pd.rangeuri AS rangeuri\n" + "FROM exp.propertydescriptor pd\n" + "JOIN exp.propertydomain pdm ON pd.propertyid = pdm.propertyid\n" + "JOIN exp.domaindescriptor dd ON pdm.domainid = dd.domainid\n" + + PHYSICAL_TABLE_JOIN + + "AND ic.column_name = lower(pd.storagecolumnname)\n" + "WHERE dd.storagetablename IS NOT NULL\n" + "AND pd.storagecolumnname IS NOT NULL\n" + "AND pd.rangeuri IN (?, ?)\n").add(PropertyType.STRING.getTypeUri()).add(PropertyType.MULTI_LINE.getTypeUri()); @@ -205,8 +220,7 @@ private void collectTextFieldMetrics(DbScope scope, SqlDialect dialect, Map> entry : byTable.entrySet()) - scanTable(scope, dialect, entry.getKey(), entry.getValue(), counts, log); + return byTable; } /** @@ -215,11 +229,13 @@ private void collectTextFieldMetrics(DbScope scope, SqlDialect dialect, Map> byTable) { - SQLFragment sql = new SQLFragment( + SQLFragment sql = SQLFragment.unsafe( "SELECT dd.storageschemaname, dd.storagetablename\n" + "FROM exp.domaindescriptor dd\n" + + PHYSICAL_TABLE_JOIN + + "AND ic.column_name = ?\n" + "WHERE dd.storagetablename IS NOT NULL AND dd.storageschemaname IN (?, ?)") - .add(SampleTypeDomainKind.PROVISIONED_SCHEMA_NAME).add(DataClassDomainKind.PROVISIONED_SCHEMA_NAME); + .add("name").add(SampleTypeDomainKind.PROVISIONED_SCHEMA_NAME).add(DataClassDomainKind.PROVISIONED_SCHEMA_NAME); new SqlSelector(scope, sql).forEach(rs -> { TableKey key = new TableKey(rs.getString("storageschemaname"), rs.getString("storagetablename")); @@ -231,7 +247,7 @@ private void addBaseNameColumns(DbScope scope, Map> byTable) private void scanTable(DbScope scope, SqlDialect dialect, TableKey table, List cols, Map> counts, Logger log) { - SQLFragment sql = new SQLFragment("SELECT "); + SQLFragment sql = SQLFragment.unsafe("SELECT "); Map aliasMeta = new LinkedHashMap<>(); boolean first = true; for (int i = 0; i < cols.size(); i++) @@ -245,7 +261,7 @@ private void scanTable(DbScope scope, SqlDialect dialect, TableKey table, List props = new ArrayList<>(); + props.add(new GWTPropertyDescriptor("Name", PropertyType.STRING.getTypeUri())); + props.add(new GWTPropertyDescriptor("Prop", PropertyType.STRING.getTypeUri())); + ExpSampleType st = SampleTypeService.get().createSampleType(c, user, ST_NAME, null, props, + Collections.emptyList(), -1, -1, -1, -1, null); + + DbScope scope = ExperimentService.get().getSchema().getScope(); + String tableName = st.getDomain().getStorageTableName(); + TableKey key = new TableKey(SampleTypeDomainKind.PROVISIONED_SCHEMA_NAME, tableName); + String movedName = tableName + "_moved"; + + List cols = new SpecialCharacterMetricsMaintenanceTask().getScanTargets(scope).get(key); + assertNotNull("existing table should be scanned", cols); + assertTrue("domain property should be scanned", cols.stream().anyMatch(col -> "prop".equalsIgnoreCase(col.storageName()))); + assertTrue("base name column should be scanned", cols.stream().anyMatch(col -> TYPE_DATA_NAME.equals(col.fieldType()))); + + // Move the table aside, rather than dropping it, so container cleanup can still drop it + renameTable(scope, tableName, movedName); + try + { + assertFalse("missing table should not be scanned", new SpecialCharacterMetricsMaintenanceTask().getScanTargets(scope).containsKey(key)); + } + finally + { + renameTable(scope, movedName, tableName); + } + } + + private static void renameTable(DbScope scope, String from, String to) + { + SqlDialect dialect = scope.getSqlDialect(); + new SqlExecutor(scope).execute(SQLFragment.unsafe("ALTER TABLE ") + .appendIdentifier(PropertyDescriptor.getLegalSelectNameFromStorageName(dialect, SampleTypeDomainKind.PROVISIONED_SCHEMA_NAME)).unsafeAppend(".") + .appendIdentifier(PropertyDescriptor.getLegalSelectNameFromStorageName(dialect, from)) + .unsafeAppend(" RENAME TO ") + .appendIdentifier(PropertyDescriptor.getLegalSelectNameFromStorageName(dialect, to))); + } + @SuppressWarnings("unchecked") private static long count(Map metrics, String type, String charKey) {