From 18acb5e3e2fd6074b785968ab282d8cf7a47561f Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Sun, 9 Aug 2026 15:39:35 -0700 Subject: [PATCH 1/2] GitHub Issue 1424: Prevent resource exhaustion from AnalyzeQueriesAction --- api/src/org/labkey/api/ApiModule.java | 2 + .../labkey/api/action/ConcurrencyLimit.java | 54 +++ .../labkey/api/action/ConcurrencyLimiter.java | 237 +++++++++++++ .../api/action/SpringActionController.java | 13 +- .../org/labkey/api/util/ExceptionUtil.java | 16 + .../api/view/TooManyRequestsException.java | 41 +++ .../api/property/PropertyServiceImpl.java | 37 +- .../query/controllers/QueryController.java | 18 +- query/webapp/query/browser/Caches.js | 318 +++++++++++++----- .../webapp/query/browser/view/Dependencies.js | 225 ++++++++++--- .../webapp/query/browser/view/QueryDetails.js | 32 +- 11 files changed, 825 insertions(+), 168 deletions(-) create mode 100644 api/src/org/labkey/api/action/ConcurrencyLimit.java create mode 100644 api/src/org/labkey/api/action/ConcurrencyLimiter.java create mode 100644 api/src/org/labkey/api/view/TooManyRequestsException.java diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index 057dcfa5b70..b8c0867b594 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -23,6 +23,7 @@ import org.jetbrains.annotations.NotNull; import org.json.JSONObject; import org.labkey.api.action.ApiXmlWriter; +import org.labkey.api.action.ConcurrencyLimiter; import org.labkey.api.action.SpringActionController; import org.labkey.api.admin.SubfolderWriter; import org.labkey.api.assay.AssayResultsFileWriter; @@ -412,6 +413,7 @@ public void registerServlets(ServletContext servletCtx) ChecksumUtil.TestCase.class, CollectionUtils.TestCase.class, Compress.TestCase.class, + ConcurrencyLimiter.TestCase.class, Constants.TestCase.class, ConvertHelper.TestCase.class, CspCommentScanner.TestCase.class, diff --git a/api/src/org/labkey/api/action/ConcurrencyLimit.java b/api/src/org/labkey/api/action/ConcurrencyLimit.java new file mode 100644 index 00000000000..923ade67d15 --- /dev/null +++ b/api/src/org/labkey/api/action/ConcurrencyLimit.java @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.action; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +/** + * Caps the number of requests executing an action at the same time, server-wide, for actions expensive enough that a + * few simultaneous callers can exhaust heap or the request thread pool. + *

+ * {@link ConcurrencyLimiter} enforces the limit from {@link SpringActionController#handleRequest}, after the permission + * check and around the whole action including rendering. A request that can't get a permit within + * {@link #timeoutSeconds()} is rejected with a 429 and never executes the action. + *

+ * Not inherited: a subclass of an annotated action is unlimited unless it declares its own {@code @ConcurrencyLimit}. + */ +@Retention(java.lang.annotation.RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +public @interface ConcurrencyLimit +{ + int DEFAULT_TIMEOUT_SECONDS = 2; + int DEFAULT_RETRY_AFTER_SECONDS = 30; + String DEFAULT_MESSAGE = ""; + + /** Maximum number of requests allowed to execute this action concurrently, across the whole server. Must be positive. */ + int value(); + + /** + * How long an incoming request waits for a permit before it's rejected with a 429. Keep it short - a parked thread + * still consumes a connector thread, so a long wait just moves the exhaustion problem. + */ + long timeoutSeconds() default DEFAULT_TIMEOUT_SECONDS; + + /** Value sent in the {@code Retry-After} response header when a request is rejected. */ + int retryAfterSeconds() default DEFAULT_RETRY_AFTER_SECONDS; + + /** Message sent to the client when a request is rejected. Defaults to a generic message when not specified. */ + String message() default DEFAULT_MESSAGE; +} diff --git a/api/src/org/labkey/api/action/ConcurrencyLimiter.java b/api/src/org/labkey/api/action/ConcurrencyLimiter.java new file mode 100644 index 00000000000..d4c392783dd --- /dev/null +++ b/api/src/org/labkey/api/action/ConcurrencyLimiter.java @@ -0,0 +1,237 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.action; + +import org.apache.logging.log4j.Logger; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.junit.Assert; +import org.junit.Test; +import org.labkey.api.util.logging.LogHelper; +import org.labkey.api.view.TooManyRequestsException; +import org.labkey.api.view.ViewContext; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * Enforces {@link ConcurrencyLimit} on behalf of {@link SpringActionController}. Holds one {@link Semaphore} per + * action class that declares the annotation. + */ +public class ConcurrencyLimiter +{ + private static final Logger LOG = LogHelper.getLogger(ConcurrencyLimiter.class, "Rejects requests to actions that limit their concurrency when too many are already in flight"); + + static final String GENERIC_MESSAGE = "The server is already handling as many simultaneous requests for this operation as it allows. Please retry in a few moments."; + + /** + * One limiter (and therefore one set of permits) per action class. {@link ConcurrentHashMap} won't store a null + * value, so unannotated actions map to {@link #UNLIMITED}. + */ + private static final Map, ConcurrencyLimiter> LIMITERS = new ConcurrentHashMap<>(); + + /** + * Limiter for an action that declares no limit. + */ + private static final ConcurrencyLimiter UNLIMITED = new ConcurrencyLimiter(); + + /** + * Handed out by {@link #UNLIMITED}; holds no permit, so closing it must do nothing. + */ + private static final Permit NOOP_PERMIT = () -> {}; + + /** + * Returned by {@link #acquire}; release the permit by closing it, ideally via try-with-resources + */ + public interface Permit extends AutoCloseable + { + @Override + void close(); + } + + /** + * Reserve one of the action's permits, if it declares a {@link ConcurrencyLimit}. + * + * @return a {@link Permit} that the caller must close once the action has finished executing + * @throws TooManyRequestsException if no permit becomes available within the action's configured timeout + */ + public static Permit acquire(@NotNull Class actionClass, @Nullable ViewContext context) + { + return LIMITERS.computeIfAbsent(actionClass, ConcurrencyLimiter::resolve).acquirePermit(context); + } + + private static ConcurrencyLimiter resolve(Class actionClass) + { + ConcurrencyLimit limit = actionClass.getDeclaredAnnotation(ConcurrencyLimit.class); + + return null == limit ? UNLIMITED : new ConcurrencyLimiter(actionClass, limit); + } + + /** + * All three are null for the {@link #UNLIMITED} sentinel and non-null for every other instance. + */ + private final Class _actionClass; + private final ConcurrencyLimit _limit; + private final Semaphore _semaphore; + + private ConcurrencyLimiter() + { + _actionClass = null; + _limit = null; + _semaphore = null; + } + + private ConcurrencyLimiter(@NotNull Class actionClass, @NotNull ConcurrencyLimit limit) + { + if (limit.value() < 1) + throw new IllegalStateException("@ConcurrencyLimit on " + actionClass.getName() + " must allow at least one concurrent request, but was " + limit.value()); + + _actionClass = actionClass; + _limit = limit; + // Fair, so a steady stream of new requests can't starve one that's already waiting + _semaphore = new Semaphore(limit.value(), true); + } + + private Permit acquirePermit(@Nullable ViewContext context) + { + if (this == UNLIMITED) + return NOOP_PERMIT; + + try + { + if (!_semaphore.tryAcquire(_limit.timeoutSeconds(), TimeUnit.SECONDS)) + throw reject(context); + } + catch (InterruptedException e) + { + Thread.currentThread().interrupt(); + throw reject(context); + } + + // Releasing twice would permanently inflate the pool and quietly defeat the limit, so make close() idempotent + AtomicBoolean released = new AtomicBoolean(); + return () -> { + if (released.compareAndSet(false, true)) + _semaphore.release(); + }; + } + + private TooManyRequestsException reject(@Nullable ViewContext context) + { + LOG.info("Rejecting request to {} for user {} in {}: {} requests already in progress", _actionClass.getName(), + null == context ? "" : context.getUser(), + null == context || null == context.getContainer() ? "" : context.getContainer().getPath(), + _limit.value()); + + String message = _limit.message().isEmpty() ? GENERIC_MESSAGE : _limit.message(); + return new TooManyRequestsException(message, _limit.retryAfterSeconds()); + } + + public static class TestCase extends Assert + { + @ConcurrencyLimit(value = 2, timeoutSeconds = 0, retryAfterSeconds = 7, message = "Slow down") + private static class LimitedAction + { + } + + /** + * Deliberately carries no annotation of its own - the limit is not inherited. + */ + private static class SubclassAction extends LimitedAction + { + } + + @ConcurrencyLimit(1) + private static class SingleRequestAction + { + } + + private static class UnlimitedAction + { + } + + @ConcurrencyLimit(0) + private static class BadLimitAction + { + } + + @Test + public void testUnlimitedActionIsNotThrottled() + { + // Many more than any limit would allow, all held at once + for (int i = 0; i < 100; i++) + //noinspection resource + ConcurrencyLimiter.acquire(UnlimitedAction.class, null); + } + + @Test + public void testRejectsBeyondLimit() + { + try (Permit p1 = acquire(LimitedAction.class, null); Permit p2 = acquire(LimitedAction.class, null)) + { + assertNotNull(p1); + assertNotNull(p2); + + @SuppressWarnings("resource") TooManyRequestsException e = assertThrows(TooManyRequestsException.class, () -> acquire(LimitedAction.class, null)); + assertEquals(TooManyRequestsException.SC_TOO_MANY_REQUESTS, e.getStatus()); + assertEquals("Slow down", e.getMessage()); + assertEquals(7, e.getRetryAfterSeconds()); + } + + // Permits are back after the try-with-resources released them + acquire(LimitedAction.class, null).close(); + } + + @Test + public void testLimitIsNotInherited() + { + // More than the superclass's limit of two + try (Permit p1 = acquire(SubclassAction.class, null); Permit p2 = acquire(SubclassAction.class, null); Permit p3 = acquire(SubclassAction.class, null)) + { + assertNotNull(p1); + assertNotNull(p2); + assertNotNull(p3); + + // The superclass still has both of its own permits available + try (Permit p4 = acquire(LimitedAction.class, null); Permit p5 = acquire(LimitedAction.class, null)) + { + assertNotNull(p4); + assertNotNull(p5); + } + } + } + + @Test + public void testGenericMessage() + { + try (Permit ignored = acquire(SingleRequestAction.class, null)) + { + //noinspection resource + assertEquals(GENERIC_MESSAGE, assertThrows(TooManyRequestsException.class, () -> acquire(SingleRequestAction.class, null)).getMessage()); + } + } + + @Test + public void testNonPositiveLimitIsRejected() + { + //noinspection resource + assertThrows(IllegalStateException.class, () -> acquire(BadLimitAction.class, null)); + } + } +} diff --git a/api/src/org/labkey/api/action/SpringActionController.java b/api/src/org/labkey/api/action/SpringActionController.java index cdbfe3c27c7..359eec144bc 100644 --- a/api/src/org/labkey/api/action/SpringActionController.java +++ b/api/src/org/labkey/api/action/SpringActionController.java @@ -532,11 +532,16 @@ public ModelAndView handleRequest(HttpServletRequest request, @NotNull HttpServl QueryService.get().setEnvironment(QueryService.Environment.ACTION, actionAnnotation.value()); } - beforeAction(controller); - ModelAndView mv = controller.handleRequest(request, response); - if (mv != null) + // After the permission check so unauthorized requests can't consume @ConcurrencyLimit permits, and held + // through rendering because the action's memory is live for that whole time. + try (ConcurrencyLimiter.Permit ignored = ConcurrencyLimiter.acquire(actionClass, context)) { - renderInTemplate(context, controller, pageConfig, mv); + beforeAction(controller); + ModelAndView mv = controller.handleRequest(request, response); + if (mv != null) + { + renderInTemplate(context, controller, pageConfig, mv); + } } } catch (HttpRequestMethodNotSupportedException x) diff --git a/api/src/org/labkey/api/util/ExceptionUtil.java b/api/src/org/labkey/api/util/ExceptionUtil.java index cacd968b269..07a2ee356a6 100644 --- a/api/src/org/labkey/api/util/ExceptionUtil.java +++ b/api/src/org/labkey/api/util/ExceptionUtil.java @@ -47,10 +47,12 @@ import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.ActionURL; import org.labkey.api.view.BadRequestException; +import org.labkey.api.view.HttpStatusException; import org.labkey.api.view.HttpView; import org.labkey.api.view.NotFoundException; import org.labkey.api.view.RedirectException; import org.labkey.api.view.RequestBasicAuthException; +import org.labkey.api.view.TooManyRequestsException; import org.labkey.api.view.UnauthorizedException; import org.labkey.api.view.ViewContext; import org.labkey.api.view.ViewServlet; @@ -975,6 +977,20 @@ else if (ex instanceof UnauthorizedException uae) unhandledException = null; } + // Must come after the more specific HttpStatusException subclasses (BadRequestException, NotFoundException, + // UnauthorizedException) + else if (ex instanceof HttpStatusException hse) + { + responseStatus = hse.getStatus(); + errorType = ErrorRenderer.ErrorType.notFound; + message = ex.getMessage(); + responseStatusMessage = message; + + if (ex instanceof TooManyRequestsException tmre) + headers.put("Retry-After", String.valueOf(tmre.getRetryAfterSeconds())); + + unhandledException = null; + } else if (ex instanceof SQLException) { responseStatus = HttpServletResponse.SC_INTERNAL_SERVER_ERROR; diff --git a/api/src/org/labkey/api/view/TooManyRequestsException.java b/api/src/org/labkey/api/view/TooManyRequestsException.java new file mode 100644 index 00000000000..2fefb0d4d04 --- /dev/null +++ b/api/src/org/labkey/api/view/TooManyRequestsException.java @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.labkey.api.view; + +/** + * The server is refusing to handle this request right now because too many similar requests are already in flight. + * Rendered as an HTTP 429 with a {@code Retry-After} header telling the client how long to wait. + * + * @see org.labkey.api.action.ConcurrencyLimit + */ +public class TooManyRequestsException extends HttpStatusException +{ + public static final int SC_TOO_MANY_REQUESTS = 429; + + private final int _retryAfterSeconds; + + public TooManyRequestsException(String message, int retryAfterSeconds) + { + super(message, null, SC_TOO_MANY_REQUESTS); + _retryAfterSeconds = retryAfterSeconds; + } + + /** @return the number of seconds to advertise in the {@code Retry-After} response header */ + public int getRetryAfterSeconds() + { + return _retryAfterSeconds; + } +} diff --git a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java index 5972ccb28ce..60e685e6a6b 100644 --- a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java @@ -98,6 +98,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -276,13 +277,7 @@ public List getDomains(Container container) @Override public List getDomains(Container container, User user, boolean includeProjectAndShared) { - List result = new ArrayList<>(); - for (DomainDescriptor dd : OntologyManager.getDomainDescriptors(container, user, includeProjectAndShared)) - { - result.add(new DomainImpl(dd)); - } - - return Collections.unmodifiableList(result); + return streamDomains(container, user, includeProjectAndShared, _ -> true).toList(); } @Override @@ -294,27 +289,33 @@ public List getDomains(Container container, User user, @Nullab @Override public List getDomains(Container container, User user, @NotNull DomainKind dk, boolean includeProjectAndShared) { - // Domain.getDomainKind() can be slow. Instead just ask the passed-in dk if the domain matches or not. - return getDomains(container, user, includeProjectAndShared) - .stream() - .filter(d -> dk.getPriority(d.getTypeURI()) != null) - .collect(Collectors.toList()); + // Domain.getDomainKind() can be slow. Instead, just ask the passed-in dk if the domain matches or not. + return streamDomains(container, user, includeProjectAndShared, dd -> dk.getPriority(dd.getDomainURI()) != null).toList(); } @Override public Stream getDomainsStream(Container container, User user, @Nullable Set domainKinds, @Nullable Set domainNames, boolean includeProjectAndShared) { - Stream stream = getDomains(container, user, includeProjectAndShared) - .stream() - .filter(d -> d.getDomainKind() != null); + Predicate filter = dd -> dd.getDomainKind() != null; if (domainKinds != null && !domainKinds.isEmpty()) - stream = stream.filter(d -> domainKinds.contains(d.getDomainKind().getKindName())); + filter = filter.and(dd -> domainKinds.contains(dd.getDomainKind().getKindName())); if (domainNames != null && !domainNames.isEmpty()) - stream = stream.filter(d -> domainNames.contains(d.getName())); + filter = filter.and(dd -> domainNames.contains(dd.getName())); - return stream; + return streamDomains(container, user, includeProjectAndShared, filter); + } + + /** + * Filter runs on the DomainDescriptor because constructing a DomainImpl loads and clones the domain's + * PropertyDescriptors, so materializing one per domain in the container is very expensive. + */ + private Stream streamDomains(Container container, User user, boolean includeProjectAndShared, Predicate filter) + { + return OntologyManager.getDomainDescriptors(container, user, includeProjectAndShared).stream() + .filter(filter) + .map(DomainImpl::new); } @Override diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index b4ebe9d8ed5..64743343c08 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -60,6 +60,7 @@ import org.labkey.api.action.ApiSimpleResponse; import org.labkey.api.action.ApiUsageException; import org.labkey.api.action.ApiVersion; +import org.labkey.api.action.ConcurrencyLimit; import org.labkey.api.action.ConfirmAction; import org.labkey.api.action.ExportAction; import org.labkey.api.action.ExportException; @@ -2719,9 +2720,7 @@ private boolean isFilterOrSort(String dataRegionName, String param) return true; if ("sort".equals(check)) return true; - if (check.equals("containerFilterName")) - return true; - return false; + return check.equals("containerFilterName"); } @RequiresPermission(ReadPermission.class) @@ -5649,7 +5648,7 @@ public boolean equals(Object o) if (o == null || getClass() != o.getClass()) return false; DataSourceInfo that = (DataSourceInfo) o; - return sourceName != null ? sourceName.equals(that.sourceName) : that.sourceName == null; + return Objects.equals(sourceName, that.sourceName); } @Override @@ -7576,11 +7575,16 @@ public void setSchemas(Map>> schemas) } } + /** + * Analyzing a folder holds the full TableInfo/ColumnInfo graph for every query in it for the life of the request, + * so avoid running to many concurrently to avoid overwhelming the heap. + */ + @ConcurrencyLimit(value = 10, message = "Too many query dependency analyses are already running. Please retry in a few moments.") @RequiresPermission(ReadPermission.class) public static class AnalyzeQueriesAction extends ReadOnlyApiAction { @Override - public Object execute(Object o, BindException errors) throws Exception + public Object execute(Object o, BindException errors) { JSONObject ret = new JSONObject(); @@ -7614,7 +7618,9 @@ public Object execute(Object o, BindException errors) throws Exception } else { - ret.put("success", false); + // must be an error rather than an empty graph, which the client reports as "no dependencies" + errors.reject(ERROR_MSG, "Query dependency analysis is not available on this server."); + return null; } return ret; } diff --git a/query/webapp/query/browser/Caches.js b/query/webapp/query/browser/Caches.js index daf03e13445..bb86d644ea0 100644 --- a/query/webapp/query/browser/Caches.js +++ b/query/webapp/query/browser/Caches.js @@ -169,6 +169,13 @@ Ext4.define('LABKEY.query.browser.cache.QueryDetails', { Ext4.define('LABKEY.query.browser.cache.QueryDependencies', { singleton: true, + /** + * Maximum number of analyzeQueries.api requests to keep in flight at once. Each request builds the full + * TableInfo/ColumnInfo graph for one folder and holds it for the life of the request, so dispatching one + * request per folder can exhaust the server's request thread pool and its heap on a site-level analysis. + */ + MAX_CONCURRENT_REQUESTS : 4, + constructor : function() { this.callParent(); @@ -178,9 +185,17 @@ Ext4.define('LABKEY.query.browser.cache.QueryDependencies', { clear : function() { this.queries = undefined; this.currentContainer = undefined; + this.analyzedContainerPath = undefined; this.totalContainers = 0; this.containers = []; - this.error = undefined; + this.activeContainers = {}; + this.activeRequests = {}; + this.activeCount = 0; + this.finishedCount = 0; + this.analysisComplete = false; + this.cancelled = false; + this.lastResponse = undefined; + this.errors = []; }, getCacheKey : function(container, schemaName, queryName) { @@ -191,11 +206,12 @@ Ext4.define('LABKEY.query.browser.cache.QueryDependencies', { return this.queries[this.getCacheKey(container, schemaName, queryName)]; }, - load : function(containerPath, success, failure, scope) { + load : function(containerPath, success, failure, scope, containers) { if (!this.queries) { this.analyzeQueries({ containerPath : containerPath, + containers : containers, success : function(resp, opts){ this.processDependencies(resp); if (Ext4.isFunction(success)){ @@ -244,79 +260,154 @@ Ext4.define('LABKEY.query.browser.cache.QueryDependencies', { // hits the server endpoint (premium only) to create the dependency graph analyzeQueries : function(config) { - function fixupJsonResponse(json, response, options, container) { - this.currentContainer = container; - if (json) { - if (json.success) { + // merge one container's response into the accumulated dependency lists + function accumulateResponse(container, json, response, options) { + // any response that isn't success:true failed, even when it carries no error message, and an unparseable + // one leaves json null; swallowing either makes the dependency report look empty rather than broken + if (!json || !json.success) { + this.errors.push({containerPath: container, response: response, options: options}); + return; + } - var key,toKey,fromKey; - var objects = json.objects; + var key,toKey,fromKey; + var objects = json.objects; - var dependantsMap = {}; - var dependeesMap = {}; + var dependantsMap = {}; + var dependeesMap = {}; - for (var edge = 0; edge < json.graph.length; edge++) { - fromKey = json.graph[edge][0]; - toKey = json.graph[edge][1]; + for (var edge = 0; edge < json.graph.length; edge++) { + fromKey = json.graph[edge][0]; + toKey = json.graph[edge][1]; - // objects I am dependant on are my dependees - dependeesMap[fromKey] = dependeesMap[fromKey] || []; - dependeesMap[fromKey].push(objects[toKey]); + // objects I am dependant on are my dependees + dependeesMap[fromKey] = dependeesMap[fromKey] || []; + dependeesMap[fromKey].push(objects[toKey]); - // objects are dependant on me are my dependants - dependantsMap[toKey] = dependantsMap[toKey] || []; - dependantsMap[toKey].push(objects[fromKey]); - } + // objects are dependant on me are my dependants + dependantsMap[toKey] = dependantsMap[toKey] || []; + dependantsMap[toKey].push(objects[fromKey]); + } - for (key in dependeesMap) { - if (dependeesMap.hasOwnProperty(key)) { - let from = objects[key]; - // limit dependants to only queries in the current folder - if (LABKEY.container.id === from.containerId) { - this.dependeesList.push({from: from, to: dependeesMap[key]}); - } - } + for (key in dependeesMap) { + if (dependeesMap.hasOwnProperty(key)) { + let from = objects[key]; + // limit dependants to only queries in the current folder + if (LABKEY.container.id === from.containerId) { + this.dependeesList.push({from: from, to: dependeesMap[key]}); } + } + } - for (key in dependantsMap) { - if (dependantsMap.hasOwnProperty(key)) { - let to = objects[key]; - // limit dependants to only queries in the current folder - if (LABKEY.container.id === to.containerId) { - this.dependantsList.push({to:to, from:dependantsMap[key]}); - } - } + for (key in dependantsMap) { + if (dependantsMap.hasOwnProperty(key)) { + let to = objects[key]; + // limit dependants to only queries in the current folder + if (LABKEY.container.id === to.containerId) { + this.dependantsList.push({to:to, from:dependantsMap[key]}); } } - else if (json.error) { - // only save the first error (if multiple) - if (!this.error) - this.error = {response: response, options: options}; - } } + } - this.removeContainer(container); - if (this.containers.length === 0) { - var callback = this.error ? LABKEY.Utils.getOnFailure(config) : LABKEY.Utils.getOnSuccess(config); - var resp = response; - var opts = options; - if (this.error) { - resp = this.error.response; - opts = this.error.options; - } + // the analysis is finished only once the queue has drained AND every dispatched request has returned + function checkComplete() { + if (this.cancelled || this.analysisComplete || this.activeCount > 0 || this.containers.length > 0) { + return; + } + this.analysisComplete = true; + + var failed = this.errors.length > 0; + var callback = failed ? LABKEY.Utils.getOnFailure(config) : LABKEY.Utils.getOnSuccess(config); + var last = this.lastResponse || {}; + var resp = last.response; + var opts = last.options; + if (failed) { + resp = this.errors[0].response; + opts = this.errors[0].options; + } - if (callback) { - var success = this.error ? false : (json ? json.success : false); - callback.call(this, {success: success, dependants: this.dependantsList, dependees: this.dependeesList}, resp, opts); + if (callback) { + var success = failed ? false : (last.json ? last.json.success : false); + callback.call(this, {success: success, dependants: this.dependantsList, dependees: this.dependeesList}, resp, opts); + } + } + + function requestComplete(container, json, response, options) { + // ignore the abort callbacks that cancel() triggers, and any duplicate callback for a container that has + // already been accounted for + if (this.cancelled || !this.activeContainers[container]) { + return; + } + delete this.activeContainers[container]; + delete this.activeRequests[container]; + this.activeCount--; + this.finishedCount++; + this.lastResponse = {response: response, options: options, json: json}; + + accumulateResponse.call(this, container, json, response, options); + pump.call(this); + } + + function dispatch(container) { + this.activeContainers[container] = true; + this.activeCount++; + this.currentContainer = container; + + this.activeRequests[container] = LABKEY.Ajax.request({ + url: LABKEY.ActionURL.buildURL('query', 'analyzeQueries.api', container), + method: 'GET', + scope: this, + success: function(resp, options){ + var json = null; + try { + json = LABKEY.Utils.decode(resp.responseText); + } + catch (e) { + console.warn('Invalid JSON returned from analyzeQueries.api : ' + resp.responseText); + console.warn('Response URL : ' + resp.responseURL); + + // leave json null and finish processing this container + } + requestComplete.call(this, container, json, resp, options); + }, + failure: function(resp, options){ + console.warn('Analyze query request failed : ' + resp.responseText); + requestComplete.call(this, container, {error: true}, resp, options); } + }); + } + + // keep up to MAX_CONCURRENT_REQUESTS requests in flight, then test for completion + function pump() { + while (this.activeCount < this.MAX_CONCURRENT_REQUESTS && this.containers.length > 0) { + dispatch.call(this, this.containers.shift()); } + checkComplete.call(this); } // initialize class data structures this.dependantsList = []; this.dependeesList = []; - this.containers = []; + this.containers = []; // container paths queued but not yet requested + this.activeContainers = {}; // container path -> true for each request in flight + this.activeRequests = {}; // container path -> XMLHttpRequest, so cancel() can abort them + this.activeCount = 0; + this.finishedCount = 0; + this.analysisComplete = false; + this.cancelled = false; + this.lastResponse = undefined; + this.errors = []; + this.analyzedContainerPath = config.containerPath; + + // the caller may have already resolved the scope via loadContainerCounts(), in which case there is nothing to look up + if (config.containers) { + this.containers = config.containers.slice(); + this.totalContainers = this.containers.length; + pump.call(this); + return; + } + this.containers.push(config.containerPath || LABKEY.container.path); let includeSubfolders = config.containerPath != null; @@ -324,6 +415,9 @@ Ext4.define('LABKEY.query.browser.cache.QueryDependencies', { LABKEY.Security.getContainers({ containerPath : config.containerPath, includeSubfolders : includeSubfolders, + includeWorkbookChildren : false, + includeStandardProperties : false, + includeEffectivePermissions : false, scope : this, success : function(json){ if (includeSubfolders) { @@ -332,31 +426,9 @@ Ext4.define('LABKEY.query.browser.cache.QueryDependencies', { }, this); } - // analyze queries for each container + // analyze queries for each container, at most MAX_CONCURRENT_REQUESTS at a time this.totalContainers = this.containers.length; - Ext4.each(this.containers, function(c){ - LABKEY.Ajax.request({ - url: LABKEY.ActionURL.buildURL('query', 'analyzeQueries.api', c), - method: 'GET', - scope: this, - success: function(resp, options){ - try { - fixupJsonResponse.call(this, LABKEY.Utils.decode(resp.responseText), resp, options, c); - } - catch (e) { - console.warn('Invalid JSON returned from analyzeQueries.api : ' + resp.responseText); - console.warn('Response URL : ' + resp.responseURL); - - // pass in a null json response and finish processing this container - fixupJsonResponse.call(this, null, resp, options, c); - } - }, - failure: function(resp, options){ - console.warn('Analyze query request failed : ' + resp.responseText); - fixupJsonResponse.call(this, {error: true}, resp, options, c); - } - }); - }, this); + pump.call(this); }, failure : function(json, resp, options) { var callback = LABKEY.Utils.getOnFailure(config); @@ -374,18 +446,98 @@ Ext4.define('LABKEY.query.browser.cache.QueryDependencies', { }, this); }, - removeContainer : function(container) { - let idx = this.containers.indexOf(container); - if (idx != -1) { - this.containers.splice(idx, 1); + /** + * Resolve, once per page load, the folders each analysis scope would process, so the UI can show the cost of an + * analysis before starting one and then hand the resolved list straight to analyzeQueries(). The site-level tree is + * a superset of the project-level one, so a single request covers both scopes. + * + * Workbooks are excluded: they hold no custom queries, but on some sites they outnumber real folders by orders of + * magnitude, and each one would otherwise cost an analyzeQueries.api request. + */ + loadContainerCounts : function(success, failure, scope) { + if (this.containerScopes) { + success.call(scope || this, this.containerScopes); + return; } + + LABKEY.Security.getContainers({ + containerPath : '/', + includeSubfolders : true, + includeWorkbookChildren : false, + // only id/name/path are needed, and resolving effective permissions for every folder on the site is by far + // the most expensive part of this call + includeStandardProperties : false, + includeEffectivePermissions : false, + scope : this, + success : function(json) { + let site = []; + + // a folder the user can't read is still in the tree if it has readable descendants, but analyzing it + // would just 403, so key off the id that toJSON() only emits when the user has read permission + function collect(container) { + if (container.id && container.path) { + site.push(container.path); + } + Ext4.each(container.children, collect); + } + collect(json); + + this.containerScopes = {'/' : site}; + + if (LABKEY.project) { + let projectPath = LABKEY.project.path.replace(/\/+$/, ''); + this.containerScopes[LABKEY.project.path] = Ext4.Array.filter(site, function(path) { + return path === projectPath || path.indexOf(projectPath + '/') === 0; + }); + } + + success.call(scope || this, this.containerScopes); + }, + failure : function(json, resp, options) { + if (Ext4.isFunction(failure)) { + failure.call(scope || this, json, resp, options); + } + } + }); + }, + + // one entry per container whose analysis failed, in the order the responses came back + getErrors : function() { + return this.errors || []; + }, + + // what the cached dependency graph covers: a null containerPath means only the folder the analysis was run from + getAnalysisScope : function() { + return {containerPath: this.analyzedContainerPath, folderCount: this.totalContainers}; + }, + + // the folders loadContainerCounts() resolved for an analysis scope, or undefined if it hasn't run + getScopeContainers : function(containerPath) { + return this.containerScopes ? this.containerScopes[containerPath] : undefined; + }, + + // abort an analysis in progress; responses that are already in flight are ignored rather than accumulated + cancel : function() { + this.cancelled = true; + this.containers = []; + this.activeContainers = {}; + this.activeCount = 0; + + let requests = this.activeRequests || {}; + this.activeRequests = {}; + Ext4.Object.each(requests, function(container, request) { + if (request && Ext4.isFunction(request.abort)) { + request.abort(); + } + }); }, // return the current progress for this loader getProgress : function() { return { currentContainer: this.currentContainer, - progress: 1.0 - (this.containers.length / this.totalContainers) + // totalContainers isn't known until the getContainers() call returns + progress: this.totalContainers ? (this.finishedCount / this.totalContainers) : 0 }; } }); \ No newline at end of file diff --git a/query/webapp/query/browser/view/Dependencies.js b/query/webapp/query/browser/view/Dependencies.js index bd7e948bb03..f80cc0de77b 100644 --- a/query/webapp/query/browser/view/Dependencies.js +++ b/query/webapp/query/browser/view/Dependencies.js @@ -14,28 +14,37 @@ Ext4.define('LABKEY.query.browser.view.Dependencies', { this.addEvents('dependencychanged'); this.errorTpl = new Ext4.XTemplate( - '
An Error Occurred Analyzing Queries : {exception}
', - '
',
-                '
{url}
',
-                '
{exceptionClass}
',
-                '',
-                '
{.}
',
+                '
',
+                '
Errors during analysis
',
+                '',
+                    '
',
+                        '
{containerPath:htmlEncode}
',
+                        '',
+                            '
{.:htmlEncode}
',
+                        '',
+                    '
',
                 '',
-                '
', - '' + '' ); }, initComponent : function() { this.enableBubble('dependencychanged'); this.dependencyCache = LABKEY.query.browser.cache.QueryDependencies; + + // the query browser can be opened at the site root, where there is no current project to analyze + this.projectPath = LABKEY.project ? LABKEY.project.path : undefined; + this.analysisPath = this.projectPath || '/'; + + const loading = ''; + this.items = [{ xtype: 'box', cls: 'lk-cf-instructions', width: '75%', - html: 'This will allow an administrator to perform a query dependency analysis across folders. The user has ' + - 'the option to analyze at the site wide level which will include all folders on the server or at the project level. ' + - 'which will include the current project and all sub folders.' + html: 'By default, the Dependency Report shown when you view a query or table in the schema browser covers ' + + 'only the current folder. This analysis proactively loads references in other folders as well, ' + + 'so that dependencies from elsewhere on the server are included.' },{ xtype: 'form', border: false, @@ -46,56 +55,118 @@ Ext4.define('LABKEY.query.browser.view.Dependencies', { }, items: [{ xtype: 'radio', - fieldLabel: 'Site Level', - checked: true, + itemId: 'lk-cfd-project-scope', + fieldLabel: this.projectPath ? 'Current project, ' + Ext4.htmlEncode(this.projectPath) : 'Current project', + boxLabel: loading, + checked: !!this.projectPath, + disabled: !this.projectPath, name: 'depth', scope: this, handler: function (cmp, checked) { if (checked) - this.analysisPath = '/'; + this.analysisPath = this.projectPath; } },{ xtype: 'radio', - fieldLabel: 'Current Project Level', + itemId: 'lk-cfd-site-scope', + fieldLabel: 'Site-wide', + boxLabel: loading, + checked: !this.projectPath, name: 'depth', scope: this, handler: function (cmp, checked) { if (checked) - this.analysisPath = LABKEY.project.path; + this.analysisPath = '/'; } + },{ + xtype: 'box', + itemId: 'lk-cfd-scope-status', + padding: '5 0 0 0', + html: loading + ' Counting the folders each option would analyze. This can take a moment on a large site.' }], buttonAlign : 'left', buttons : [ - {text : 'Start Analysis', handler : this.startAnalysis, scope : this} + {text : 'Start Analysis', itemId: 'lk-cfd-start', disabled: true, handler : this.startAnalysis, scope : this} ] },{ xtype: 'box', + itemId: 'lk-cfd-error', padding: 10, - id: 'lk-dependency-progress-bar' + hidden: true }]; this.callParent(); + + this.on('afterrender', this.loadScopeCounts, this); + }, + + // resolve the folder counts up front so the cost of each option is visible before an analysis is started + loadScopeCounts : function() { + this.dependencyCache.loadContainerCounts(function(scopes) { + // the tab is closable, so it may be gone by the time the container tree comes back + if (this.isDestroyed) + return; + + this.setScopeCount('#lk-cfd-site-scope', scopes['/']); + this.setScopeCount('#lk-cfd-project-scope', this.projectPath ? scopes[this.projectPath] : undefined); + this.down('#lk-cfd-scope-status').hide(); + this.down('#lk-cfd-start').enable(); + }, function() { + if (this.isDestroyed) + return; + + this.setScopeCount('#lk-cfd-site-scope', undefined); + this.setScopeCount('#lk-cfd-project-scope', undefined); + this.down('#lk-cfd-scope-status').update('Unable to determine how many folders each option would analyze.'); + }, this); + }, + + setScopeCount : function(itemId, containers) { + let radio = this.down(itemId); + if (radio) { + let label = ''; + if (containers) { + label = containers.length === 1 ? '1 folder' : containers.length.toLocaleString() + ' folders'; + } + radio.setBoxLabel(label); + } }, startAnalysis : function() { - // display a progress bar (even if it renders under the mask) - let pb = Ext4.create('Ext.ProgressBar', { - renderTo: 'lk-dependency-progress-bar', - width: 500 + // resolved by loadScopeCounts(), so the analysis doesn't walk the container tree a second time + let containers = this.dependencyCache.getScopeContainers(this.analysisPath); + + this.analysisRunning = true; + this.down('#lk-cfd-start').disable(); + this.hideError(); + + this.progressBar = Ext4.create('Ext.ProgressBar', {width: 500}); + + // modal, both to keep Stop Analysis reachable and because QueryDependencies is a singleton that a query + // details page would otherwise start a second, competing analysis on + this.progressWindow = Ext4.create('Ext.window.Window', { + title: 'Analyzing Query Dependencies', + modal: true, + closable: false, + draggable: false, + bodyPadding: 10, + items: [this.progressBar], + buttons: [{text: 'Stop Analysis', handler: this.stopAnalysis, scope: this}] }); - Ext4.TaskManager.start({ + this.progressWindow.show(); + + this.progressTask = Ext4.TaskManager.start({ interval: 250, delay: 1000, scope: this, run: function(){ let info = this.dependencyCache.getProgress(); - pb.updateProgress(info.progress, info.currentContainer, true); + if (this.progressBar) + this.progressBar.updateProgress(info.progress, info.currentContainer, true); } }); function loadSuccessHandler(json, resp, opts) { - pb.destroy(); - Ext4.TaskManager.stopAll(); - this.parent.getEl().unmask(); + this.endAnalysis(); Ext4.Msg.alert('Cross Folder Dependencies', 'The query analysis has completed successfully', function () { this.fireEvent('dependencychanged'); @@ -104,37 +175,83 @@ Ext4.define('LABKEY.query.browser.view.Dependencies', { } function loadFailureHandler(json, resp, opts) { - pb.destroy(); - Ext4.TaskManager.stopAll(); - this.parent.getEl().unmask(); - var error = this.getErrorMessageFromResponse(resp, opts); - var dialog = Ext4.create('Ext.window.Window', { - layout: 'fit', - draggable: false, - modal: true, - closable: true, - title: 'Error', - items: [{ - xtype: 'box', - tpl: this.errorTpl, - data: error, - autoScroll: true - }], - buttons: [{ - text: 'Close', - handler: function() { - dialog.close(); - } - }], - scope: this - }); - dialog.show(); + this.endAnalysis(); + this.showErrors(resp, opts); } // clear the cache and re-load using the configured path - this.parent.getEl().mask(); this.dependencyCache.clear(); - this.dependencyCache.load(this.analysisPath, loadSuccessHandler, loadFailureHandler, this); + this.dependencyCache.load(this.analysisPath, loadSuccessHandler, loadFailureHandler, this, containers); + }, + + stopAnalysis : function() { + if (!this.analysisRunning) + return; + + this.dependencyCache.cancel(); + this.endAnalysis(); + + // the partial graph isn't usable, so drop it and let a later analysis rebuild it + this.dependencyCache.clear(); + Ext4.Msg.alert('Cross Folder Dependencies', 'The query analysis was stopped.'); + }, + + endAnalysis : function() { + this.analysisRunning = false; + + if (this.progressTask) { + Ext4.TaskManager.stop(this.progressTask); + this.progressTask = undefined; + } + if (this.progressWindow) { + let win = this.progressWindow; + this.progressWindow = undefined; + this.progressBar = undefined; + + // destroys the progress bar along with it + win.close(); + } + this.down('#lk-cfd-start').enable(); + }, + + /** + * Lists every container whose analysis failed. resp/opts describe the failure that ended the analysis and are only + * used when it failed before any container was requested, such as when the container list itself couldn't be loaded. + */ + showErrors : function(resp, opts) { + let errors = this.dependencyCache.getErrors(); + if (errors.length === 0) + errors = [{containerPath: this.analysisPath, response: resp, options: opts}]; + + let byContainer = {}; + let grouped = []; + Ext4.each(errors, function(error) { + let messages = byContainer[error.containerPath]; + if (!messages) { + messages = byContainer[error.containerPath] = []; + grouped.push({containerPath: error.containerPath, messages: messages}); + } + messages.push(this.getErrorMessage(error.response, error.options)); + }, this); + + let box = this.down('#lk-cfd-error'); + box.update(this.errorTpl.apply(grouped)); + box.show(); + }, + + getErrorMessage : function(response, opts) { + // getErrorMessageFromResponse() reads responseURL off the response unconditionally + if (!response) + return 'Unknown error'; + + // a JSON body that carries no exception (a bare success:false, say) leaves nothing to show + return this.getErrorMessageFromResponse(response, opts).exception || 'Unknown error'; + }, + + hideError : function() { + let box = this.down('#lk-cfd-error'); + box.update(''); + box.hide(); }, getErrorMessageFromResponse : function (response, opts){ diff --git a/query/webapp/query/browser/view/QueryDetails.js b/query/webapp/query/browser/view/QueryDetails.js index 404569d7f78..e960241ef34 100644 --- a/query/webapp/query/browser/view/QueryDetails.js +++ b/query/webapp/query/browser/view/QueryDetails.js @@ -483,6 +483,9 @@ Ext4.define('LABKEY.query.browser.view.QueryDetails', { formatDependencies : function () { const dependencies = this.queriesCache.getDependencies(LABKEY.container.id, this.schemaName, this.queryName); + const heading = '

Dependency Report

'; + const subject = this.queryDetails.isUserDefined ? 'query' : 'table'; + const scope = this.formatDependencyScope(); // issue : 40993 sort dependencies by type, schemaName and name let sortFn = function(a, b){ @@ -505,8 +508,6 @@ Ext4.define('LABKEY.query.browser.view.QueryDetails', { dependencies.dependents.sort(sortFn); let tpl = new Ext4.XTemplate( - '

Dependency Report

', - 'The queries or tables that this query or table depends on and the queries or tables that depend on it.', '', '', '
', @@ -619,9 +620,34 @@ Ext4.define('LABKEY.query.browser.view.QueryDetails', { return { tag: 'div', cls: 'lk-qd-dependencies', - html: tpl.apply(dependencies) + // the heading and intro stay out of the XTemplate, which would parse a '{' in the scope's folder path + html: heading + + 'The queries or tables that this ' + subject + ' depends on and the queries or tables ' + + 'that depend on it. ' + scope + '' + + tpl.apply(dependencies) }; } + + // the analysis ran and found nothing; a failed analysis renders its own message instead + return { + tag: 'div', + cls: 'lk-qd-dependencies', + html: heading + 'There are no dependencies to or from this ' + subject + '. ' + scope + '' + }; + }, + + // states which folders the cached dependency graph was built from, since it covers only the current folder until a + // cross folder analysis is run + formatDependencyScope : function() { + const scope = this.queriesCache.getAnalysisScope(); + + if (!scope.containerPath) + return 'Searched this folder only.'; + + if (scope.containerPath === '/') + return 'Searched all folders on this site.'; + + return 'Searched ' + Ext4.htmlEncode(scope.containerPath) + ' and its subfolders.'; }, hasProperties: function (o) { From d9f732cd358e91101d430ac38a4300301063e8e2 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Sun, 9 Aug 2026 19:20:37 -0700 Subject: [PATCH 2/2] Claude code review updates --- .../labkey/api/action/ConcurrencyLimiter.java | 9 ++++++++- query/webapp/query/browser/Caches.js | 12 +++++++----- query/webapp/query/browser/view/Dependencies.js | 3 +++ query/webapp/query/browser/view/QueryDetails.js | 16 ++++++++++++++-- 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/api/src/org/labkey/api/action/ConcurrencyLimiter.java b/api/src/org/labkey/api/action/ConcurrencyLimiter.java index d4c392783dd..edfe754761a 100644 --- a/api/src/org/labkey/api/action/ConcurrencyLimiter.java +++ b/api/src/org/labkey/api/action/ConcurrencyLimiter.java @@ -73,7 +73,14 @@ public interface Permit extends AutoCloseable */ public static Permit acquire(@NotNull Class actionClass, @Nullable ViewContext context) { - return LIMITERS.computeIfAbsent(actionClass, ConcurrencyLimiter::resolve).acquirePermit(context); + // This runs on every request, and computeIfAbsent() locks the bin even on a hit unless the key happens to be + // its first node, so take the lock-free get() whenever the limiter is already resolved. + ConcurrencyLimiter limiter = LIMITERS.get(actionClass); + + if (null == limiter) + limiter = LIMITERS.computeIfAbsent(actionClass, ConcurrencyLimiter::resolve); + + return limiter.acquirePermit(context); } private static ConcurrencyLimiter resolve(Class actionClass) diff --git a/query/webapp/query/browser/Caches.js b/query/webapp/query/browser/Caches.js index bb86d644ea0..40c899f0af9 100644 --- a/query/webapp/query/browser/Caches.js +++ b/query/webapp/query/browser/Caches.js @@ -212,14 +212,16 @@ Ext4.define('LABKEY.query.browser.cache.QueryDependencies', { this.analyzeQueries({ containerPath : containerPath, containers : containers, - success : function(resp, opts){ - this.processDependencies(resp); + // callbacks take the accumulated result plus the response and options of the request that ended the + // analysis; dropping either leaves the caller unable to report what actually went wrong + success : function(result, response, options){ + this.processDependencies(result); if (Ext4.isFunction(success)){ - success.call(scope || this, resp, opts); + success.call(scope || this, result, response, options); } }, - failure : function(resp, opts) { - failure.call(scope || this, resp, opts); + failure : function(result, response, options) { + failure.call(scope || this, result, response, options); }, scope : this }); diff --git a/query/webapp/query/browser/view/Dependencies.js b/query/webapp/query/browser/view/Dependencies.js index f80cc0de77b..9bd8668af11 100644 --- a/query/webapp/query/browser/view/Dependencies.js +++ b/query/webapp/query/browser/view/Dependencies.js @@ -117,6 +117,9 @@ Ext4.define('LABKEY.query.browser.view.Dependencies', { this.setScopeCount('#lk-cfd-site-scope', undefined); this.setScopeCount('#lk-cfd-project-scope', undefined); this.down('#lk-cfd-scope-status').update('Unable to determine how many folders each option would analyze.'); + + // still startable: analyzeQueries() walks the container tree itself when it isn't handed a folder list + this.down('#lk-cfd-start').enable(); }, this); }, diff --git a/query/webapp/query/browser/view/QueryDetails.js b/query/webapp/query/browser/view/QueryDetails.js index e960241ef34..646d29454b0 100644 --- a/query/webapp/query/browser/view/QueryDetails.js +++ b/query/webapp/query/browser/view/QueryDetails.js @@ -865,6 +865,12 @@ Ext4.define('LABKEY.query.browser.view.QueryDetails', { renderQueryDetails : function() { this.getContent().removeAll(); + // analyzeQueries.api is backed by a premium service, and without it there is no graph to report on + if (!this.parent.hasQueryAnalysisService) { + this.getContent().add(this.formatQueryDetails(this.queryDetails)); + return; + } + // add a temporary placeholder for the query dependencies but don't block the entire page this.getContent().add(this.formatQueryDetails(this.queryDetails), { xtype : 'box', @@ -875,14 +881,20 @@ Ext4.define('LABKEY.query.browser.view.QueryDetails', { scope : this, fn : function(cmp) { cmp.getEl().mask('loading dependencies'); - this.queriesCache.load(null, this.refreshQueryDependencies, LABKEY.Utils.getCallbackWrapper(function(error) { + + let onError = LABKEY.Utils.getCallbackWrapper(function(error) { this.removeQueryDependencies(); this.getContent().add({ xtype : 'box', itemId : 'lk-dependency-report', html : '
Failed to load dependency information. ' + Ext4.htmlEncode(error.exception ? error.exception : ''), }); - }, this, true), this); + }, this, true); + + this.queriesCache.load(null, this.refreshQueryDependencies, function(result, response, options) { + // load() leads with the accumulated result, but the server's message is on the response + onError.call(this, response, options); + }, this); } } }