diff --git a/query/src/org/labkey/query/ModuleCustomQueryDefinition.java b/query/src/org/labkey/query/ModuleCustomQueryDefinition.java index 54c919ac8fe..c4be3934890 100644 --- a/query/src/org/labkey/query/ModuleCustomQueryDefinition.java +++ b/query/src/org/labkey/query/ModuleCustomQueryDefinition.java @@ -16,6 +16,7 @@ package org.labkey.query; import org.apache.commons.io.IOUtils; +import org.jetbrains.annotations.Nullable; import org.labkey.api.data.Container; import org.labkey.api.module.ModuleLoader; import org.labkey.api.moduleeditor.api.ModuleEditorService; @@ -88,6 +89,14 @@ public File getSqlFile() return _resourceSqlFile; } + // GH Issue 1512: file-based module queries carry no QueryDef.Modified, so bust the has-PK cache on the .sql mtime + @Nullable + @Override + protected String getHasPkCacheVersion() + { + return null != _resourceSqlFile ? "module:" + _resourceSqlFile.lastModified() : null; + } + public File getModuleXmlFile() { return _resourceQueryXmlFile; diff --git a/query/src/org/labkey/query/QueryDefinitionImpl.java b/query/src/org/labkey/query/QueryDefinitionImpl.java index 02033b9333a..b038b39e397 100644 --- a/query/src/org/labkey/query/QueryDefinitionImpl.java +++ b/query/src/org/labkey/query/QueryDefinitionImpl.java @@ -27,6 +27,8 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.action.SpringActionController; +import org.labkey.api.cache.Cache; +import org.labkey.api.cache.CacheManager; import org.labkey.api.collections.CaseInsensitiveLinkedHashMap; import org.labkey.api.data.AbstractTableInfo; import org.labkey.api.data.ColumnInfo; @@ -77,6 +79,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -106,6 +109,11 @@ public abstract class QueryDefinitionImpl implements QueryDefinition // private static Map, TableInfo> _cache = new HashMap<>(); private final Map, TableInfo> _cache = new HashMap<>(); + // GH Issue 1512: does this query expose a PK? Lets lookup-target enumeration skip re-resolving known no-PK queries. + // Keyed by resolving container + schema path + name + Modified; cleared on any QueryDef/schema change (a query's PK + // can shift without its own row changing, via chained queries, source metadata, or schema reloads). + private static final Cache HAS_PK_COLUMN_CACHE = CacheManager.getCache(CacheManager.UNLIMITED, CacheManager.DAY, "Query has-PK-column flags"); + private Map _metadataTableMap = null; public QueryDefinitionImpl(User user, Container container, QueryDef queryDef) @@ -814,6 +822,46 @@ public boolean isIncludedForLookups() return _includedForLookups; } + // GH Issue 1512: cache-busting token for the has-PK cache. DB queries use Modified; subclasses override (e.g. a + // file-based module query uses its .sql mtime). Null means "don't cache" (unsaved/transient def). + @Nullable + protected String getHasPkCacheVersion() + { + Date modified = _queryDef.getModified(); + return null == modified ? null : String.valueOf(modified.getTime()); + } + + // GH Issue 1512: key on the resolving container, not the defining one: an inheritable/shared query compiles to a + // different table, and PK, per folder. + @Nullable + private String getHasPkColumnCacheKey() + { + String version = getHasPkCacheVersion(); + if (null == version || null == getContainer() || null == getName()) + return null; + return getContainer().getId() + "/" + getSchemaPath() + "/" + getName() + "/" + version; + } + + public static void clearHasPkColumnCache() + { + HAS_PK_COLUMN_CACHE.clear(); + } + + /** @return cached PK-presence for this query, or null if not cached */ + @Nullable + public Boolean getCachedHasPkColumn() + { + String key = getHasPkColumnCacheKey(); + return null == key ? null : HAS_PK_COLUMN_CACHE.get(key); + } + + public void cacheHasPkColumn(boolean hasPkColumn) + { + String key = getHasPkColumnCacheKey(); + if (null != key) + HAS_PK_COLUMN_CACHE.put(key, hasPkColumn); + } + @Override public void setIsIncludedForLookups(boolean included) { diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index 8e71be82b2d..39a7602032d 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -283,6 +283,7 @@ import org.labkey.query.LinkedTableInfo; import org.labkey.query.MetadataTableJSON; import org.labkey.query.ModuleCustomQueryDefinition; +import org.labkey.query.QueryDefinitionImpl; import org.labkey.query.ModuleCustomView; import org.labkey.query.QueryServiceImpl; import org.labkey.query.QueryServiceImpl.CalculatedColumnParseResult; @@ -6849,6 +6850,7 @@ public static class GetQueriesForm { private String _schemaName; private boolean _includeUserQueries = true; + private boolean _includeUserQueriesForLookups = false; private boolean _includeSystemQueries = true; private boolean _includeColumns = true; private boolean _includeViewDataUrl = true; @@ -6875,6 +6877,16 @@ public void setIncludeUserQueries(boolean includeUserQueries) _includeUserQueries = includeUserQueries; } + public boolean isIncludeUserQueriesForLookups() + { + return _includeUserQueriesForLookups; + } + + public void setIncludeUserQueriesForLookups(boolean includeUserQueriesForLookups) + { + _includeUserQueriesForLookups = includeUserQueriesForLookups; + } + public boolean isIncludeSystemQueries() { return _includeSystemQueries; @@ -6948,14 +6960,25 @@ public ApiResponse execute(GetQueriesForm form, BindException errors) List> qinfos = new ArrayList<>(); //user-defined queries - if (form.isIncludeUserQueries()) + if (form.isIncludeUserQueries() || form.isIncludeUserQueriesForLookups()) { + // GH Issue 1512: includeUserQueries returns them all; includeUserQueriesForLookups (only when the former is off) + // restricts to queries that expose a primary key + boolean requirePk = form.isIncludeUserQueriesForLookups() && !form.isIncludeUserQueries(); for (QueryDefinition qdef : uschema.getQueryDefs().values()) { if (!qdef.isTemporary()) { + if (requirePk) + { + QueryDefinitionImpl impl = qdef instanceof QueryDefinitionImpl q ? q : null; + if (impl != null && Boolean.FALSE.equals(impl.getCachedHasPkColumn())) + continue; + } ActionURL viewDataUrl = form.isIncludeViewDataUrl() ? uschema.urlFor(QueryAction.executeQuery, qdef) : null; - qinfos.add(getQueryProps(qdef, viewDataUrl, true, uschema, form.isIncludeColumns(), form.isQueryDetailColumns(), form.isIncludeTitle())); + Map props = getQueryProps(qdef, viewDataUrl, true, uschema, form.isIncludeColumns(), form.isQueryDetailColumns(), form.isIncludeTitle(), requirePk); + if (props != null) + qinfos.add(props); } } } @@ -6971,7 +6994,7 @@ public ApiResponse execute(GetQueriesForm form, BindException errors) if (qdef != null) { ActionURL viewDataUrl = form.isIncludeViewDataUrl() ? uschema.urlFor(QueryAction.executeQuery, qdef) : null; - qinfos.add(getQueryProps(qdef, viewDataUrl, false, uschema, form.isIncludeColumns(), form.isQueryDetailColumns(), form.isIncludeTitle())); + qinfos.add(getQueryProps(qdef, viewDataUrl, false, uschema, form.isIncludeColumns(), form.isQueryDetailColumns(), form.isIncludeTitle(), false)); } } } @@ -6980,8 +7003,10 @@ public ApiResponse execute(GetQueriesForm form, BindException errors) return response; } - protected Map getQueryProps(QueryDefinition qdef, ActionURL viewDataUrl, boolean isUserDefined, UserSchema schema, boolean includeColumns, boolean useQueryDetailColumns, boolean includeTitle) + private Map getQueryProps(QueryDefinition qdef, ActionURL viewDataUrl, boolean isUserDefined, UserSchema schema, boolean includeColumns, boolean useQueryDetailColumns, boolean includeTitle, boolean requirePk) { + QueryDefinitionImpl impl = qdef instanceof QueryDefinitionImpl q ? q : null; + Map qinfo = new HashMap<>(); qinfo.put("hidden", qdef.isHidden()); qinfo.put("snapshot", qdef.isSnapshot()); @@ -7008,15 +7033,22 @@ protected Map getQueryProps(QueryDefinition qdef, ActionURL view String title = qdef.getName(); String name = qdef.getName(); + boolean hasPk = false; try { - // get the TableInfo if the user requested column info or title, otherwise skip (it can be expensive) - if (includeColumns || includeTitle) + // get the TableInfo if the user requested column info or title or a PK filter, otherwise skip (it can be expensive) + if (includeColumns || includeTitle || requirePk) { TableInfo table = qdef.getTable(schema, null, true); if (null != table) { + hasPk = table.getPkColumns().stream().anyMatch(col -> !col.isAdditionalQueryColumn()); + if (isUserDefined && impl != null) + impl.cacheHasPkColumn(hasPk); + if (requirePk && !hasPk) + return null; + if (includeColumns) { Collection> columns; @@ -7062,6 +7094,10 @@ protected Map getQueryProps(QueryDefinition qdef, ActionURL view //may happen due to query failing parse } + // GH Issue 1512: a query that didn't resolve (null table or parse failure) can't be confirmed as a lookup target + if (requirePk && !hasPk) + return null; + qinfo.put("title", title); qinfo.put("name", name); return qinfo; diff --git a/query/src/org/labkey/query/persist/QueryDefCache.java b/query/src/org/labkey/query/persist/QueryDefCache.java index c4acb06d0d7..21ac9651711 100644 --- a/query/src/org/labkey/query/persist/QueryDefCache.java +++ b/query/src/org/labkey/query/persist/QueryDefCache.java @@ -25,6 +25,7 @@ import org.labkey.api.data.Container; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.TableSelector; +import org.labkey.query.QueryDefinitionImpl; import java.util.ArrayList; import java.util.Collection; @@ -168,5 +169,6 @@ QueryDef getQueryDefById(Container container, int queryDefId) public static void uncache(Container c) { QUERY_DEF_DB_CACHE.remove(c); + QueryDefinitionImpl.clearHasPkColumnCache(); } } \ No newline at end of file diff --git a/query/src/org/labkey/query/persist/QueryManager.java b/query/src/org/labkey/query/persist/QueryManager.java index 0c11951341c..332def0a7b3 100644 --- a/query/src/org/labkey/query/persist/QueryManager.java +++ b/query/src/org/labkey/query/persist/QueryManager.java @@ -67,6 +67,7 @@ import org.labkey.api.view.NotFoundException; import org.labkey.query.ExternalSchema; import org.labkey.query.ExternalSchemaDocumentProvider; +import org.labkey.query.QueryDefinitionImpl; import org.labkey.query.audit.GridViewAuditProvider; import org.labkey.query.audit.GridViewAuditProvider.GridViewAuditEvent; import org.labkey.query.audit.QueryExportAuditProvider; @@ -433,6 +434,8 @@ public void updateExternalSchemas(Container c) { ExternalSchemaDefCache.uncache(c); ExternalSchemaDocumentProvider.getInstance().enumerateDocuments(SearchService.get().defaultTask().getQueue(c, SearchService.PRIORITY.modified), null); + // GH Issue 1512: a schema reload can change a source-table PK without touching the query row + QueryDefinitionImpl.clearHasPkColumnCache(); } } @@ -444,6 +447,7 @@ public void reloadAllExternalSchemas(Container c) public void reloadExternalSchema(ExternalSchemaDef def) { ExternalSchema.uncache(def); + QueryDefinitionImpl.clearHasPkColumnCache(); } public boolean canInherit(int flag)