diff --git a/query/src/org/labkey/query/MetadataTableJSON.java b/query/src/org/labkey/query/MetadataTableJSON.java index 53c5b0dd00e..ac9138daa88 100644 --- a/query/src/org/labkey/query/MetadataTableJSON.java +++ b/query/src/org/labkey/query/MetadataTableJSON.java @@ -152,9 +152,14 @@ public static void saveMetadata(String schemaName, String queryName, @Nullable S TableInfo rawTableInfo = getRawTableInfo(schema, queryName, updatedQueryName); TablesDocument doc = null; TableType xmlTable = null; + String savedQueryName = updatedQueryName != null ? updatedQueryName : queryName; if (queryDef != null) { + // GH Issue 1736: make sure to put the updated query name into the queryDef + if (updatedQueryName != null) + queryDef.setName(updatedQueryName); + try { doc = parseDocument(queryDef.getMetaData()); @@ -180,7 +185,7 @@ else if (updatedQueryName != null) queryDef = new QueryDef(); queryDef.setSchema(schemaName); queryDef.setContainer(container.getId()); - queryDef.setName(queryName); + queryDef.setName(savedQueryName); } if (doc == null) @@ -192,7 +197,7 @@ else if (updatedQueryName != null) { TablesType tables = doc.addNewTables(); xmlTable = tables.addNewTable(); - xmlTable.setTableName(queryName); + xmlTable.setTableName(savedQueryName); } if (xmlTable.getColumns() == null) diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index acc72e1a864..481b915c852 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -53,6 +53,7 @@ import org.labkey.api.data.ConvertHelper; import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbSchemaType; +import org.labkey.api.data.DbScope; import org.labkey.api.data.DisplayColumn; import org.labkey.api.data.Filter; import org.labkey.api.data.ForeignKey; @@ -76,6 +77,7 @@ import org.labkey.api.data.TableSelector; import org.labkey.api.data.VirtualTable; import org.labkey.api.data.dialect.SqlDialect; +import org.labkey.api.exp.PropertyType; import org.labkey.api.exp.property.DomainKind; import org.labkey.api.exp.query.ExpTable; import org.labkey.api.gwt.client.AuditBehaviorType; @@ -206,6 +208,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -3878,5 +3881,66 @@ public void testRightAndIsnumeric() throws SQLException assertEquals("isnumeric(NULL) on " + dialect, 0, results.getInt("n4")); } } + + // GH Issue 1736 + @Test + public void testRenameWithConcurrentCacheReload() throws Exception + { + DbScope scope = QueryManager.get().getDbSchema().getScope(); + Container c = JunitUtil.getTestContainer(); + User user = TestContext.get().getUser(); + String oldName = "Users"; + String newName = "junit" + GUID.makeHash(); + + QueryDef def = new QueryDef(); + def.setSchema("core"); + def.setName(oldName); + def.setContainer(c.getId()); + def.setMetaData("" + + "http://www.labkey.org/exp/xml#calculated1" + + "
"); + int queryDefId = QueryManager.get().insert(user, def).getQueryDefId(); + + GWTPropertyDescriptor calcField = new GWTPropertyDescriptor("junitCalc", PropertyType.INTEGER.getTypeUri()); + calcField.setValueExpression("1"); + // reloads the cache from committed data, as search indexing can while the rename is uncommitted + Runnable concurrentReload = () -> QueryManager.get().getQueryDef(c, "core", oldName, false); + + try + { + try (DbScope.Transaction tx = scope.ensureTransaction()) + { + QueryManager.get().renameQuery(user, c, "core", oldName, newName); + runOnOtherThread(concurrentReload); + assertNull("Expected the cache to hold the pre-rename row", QueryManager.get().getQueryDef(c, "core", newName, false)); + + QueryService.get().saveCalculatedFieldsMetadata("core", oldName, newName, List.of(calcField), true, user, c); + runOnOtherThread(concurrentReload); + tx.commit(); + } + + QueryDef saved = new TableSelector(QueryManager.get().getTableInfoQueryDef()).getObject(queryDefId, QueryDef.class); + assertEquals(newName, saved.getName()); + assertTrue(saved.getMetaData(), saved.getMetaData().contains("tableName=\"" + newName + "\"")); + assertTrue(saved.getMetaData(), saved.getMetaData().contains("junitCalc")); + + assertNotNull("Cache should reload after commit", QueryManager.get().getQueryDef(c, "core", newName, false)); + assertNull(QueryManager.get().getQueryDef(c, "core", oldName, false)); + } + finally + { + QueryDef toDelete = new TableSelector(QueryManager.get().getTableInfoQueryDef()).getObject(queryDefId, QueryDef.class); + if (toDelete != null) + QueryManager.get().delete(toDelete); + } + } + + private void runOnOtherThread(Runnable r) throws InterruptedException + { + Thread t = new Thread(r); + t.start(); + t.join(TimeUnit.SECONDS.toMillis(30)); + assertFalse("Reader thread did not finish", t.isAlive()); + } } } diff --git a/query/src/org/labkey/query/persist/QueryDefCache.java b/query/src/org/labkey/query/persist/QueryDefCache.java index 8efc33ef413..105a2c5d44e 100644 --- a/query/src/org/labkey/query/persist/QueryDefCache.java +++ b/query/src/org/labkey/query/persist/QueryDefCache.java @@ -23,6 +23,7 @@ import org.labkey.api.collections.CaseInsensitiveMapWrapper; import org.labkey.api.collections.IntHashMap; import org.labkey.api.data.Container; +import org.labkey.api.data.DbScope; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.TableSelector; import org.labkey.query.QueryDefinitionImpl; @@ -166,9 +167,20 @@ QueryDef getQueryDefById(Container container, int queryDefId) return QUERY_DEF_DB_CACHE.get(container)._queryDefIdMap.get(queryDefId); } + // GH Issue 1736: a thread outside the transaction can reload pre-commit rows after the immediate remove, so remove again after commit public static void uncache(Container c) { - QUERY_DEF_DB_CACHE.remove(c); - QueryDefinitionImpl.clearHasPkColumnCache(); + QueryManager.get().getDbSchema().getScope().addCommitTask(new UncacheTask(c), DbScope.CommitTaskOption.IMMEDIATE, DbScope.CommitTaskOption.POSTCOMMIT, DbScope.CommitTaskOption.POSTROLLBACK); + } + + // record equality lets the transaction dedupe repeated uncaches of the same container + private record UncacheTask(Container c) implements Runnable + { + @Override + public void run() + { + QUERY_DEF_DB_CACHE.remove(c); + QueryDefinitionImpl.clearHasPkColumnCache(); + } } } \ No newline at end of file