diff --git a/.chachalog/Kp7mQx2R.md b/.chachalog/Kp7mQx2R.md
index fcf32e38..198f3ac8 100644
--- a/.chachalog/Kp7mQx2R.md
+++ b/.chachalog/Kp7mQx2R.md
@@ -3,6 +3,8 @@
javascript-modules: minor
---
-`buildNodeUrl` now autocollects cache dependencies for the content it links to, preventing 404 errors when the linked content is renamed or moved. (#783)
+`buildNodeUrl` now autocollects cache dependencies for the content it links to, preventing 404 errors when the linked content is renamed or moved. (#783, #810)
This mechanism is enabled by default but can be disabled on a per-view basis by setting `"cache.autocollectDependencies": "false"` in the view's properties, or on a per-call basis with the new `autocollectDependency: false` option in `buildNodeUrl`.
+
+When a view links to more than 100 contents, a change in a folder that holds many of them also refreshes the view.
diff --git a/jahia-test-module/src/react/server/views/testAutocollectedCacheDependency/TestAutocollectedCacheDependency.tsx b/jahia-test-module/src/react/server/views/testAutocollectedCacheDependency/TestAutocollectedCacheDependency.tsx
index 634dbf28..825bc342 100644
--- a/jahia-test-module/src/react/server/views/testAutocollectedCacheDependency/TestAutocollectedCacheDependency.tsx
+++ b/jahia-test-module/src/react/server/views/testAutocollectedCacheDependency/TestAutocollectedCacheDependency.tsx
@@ -1,27 +1,27 @@
-import { buildNodeUrl, jahiaComponent } from "@jahia/javascript-modules-library";
+import { buildNodeUrl, jahiaComponent, Render } from "@jahia/javascript-modules-library";
import type { JCRNodeWrapper } from "org.jahia.services.content";
interface Props {
image: JCRNodeWrapper;
}
-const ImageAndRenderMarker = ({ image, testId }: { image: JCRNodeWrapper; testId: string }) => (
+const ImageAndRenderMarker = ({ src, testId }: { src: string; testId: string }) => (
-
})
+
{String(Date.now())}
+ * {@link CollectedCacheDependenciesFilter} puts a {@link Collector} for each
+ * cached fragment in a request attribute, with
+ * {@code RenderChain.pushAttribute}. A nested fragment puts its own, and the
+ * render chain restores the one of the parent when the nested render ends.
*/
public final class CollectedCacheDependencies {
@@ -25,10 +39,61 @@ public final class CollectedCacheDependencies {
*/
public static final int DEFAULT_LIMIT = 100;
+ /** The request attribute that holds the {@link Collector} of the rendered fragment. */
+ static final String ATTRIBUTE = CollectedCacheDependencies.class.getName();
+
+ /**
+ * The depth a merge never goes above: two segments, such as
+ * {@code /sites/mysite}.
+ */
+ static final int MIN_MERGE_DEPTH = 2;
+
private CollectedCacheDependencies() {
// static helper
}
+ /** The paths collected for one cached fragment. */
+ static final class Collector {
+ final Resource resource;
+ /** False when the view sets the opt-out property. */
+ final boolean enabled;
+ final Set paths = new LinkedHashSet<>();
+
+ Collector(Resource resource, boolean enabled) {
+ this.resource = resource;
+ this.enabled = enabled;
+ }
+ }
+
+ /**
+ * Collects a path for the fragment being rendered.
+ *
+ * @param request the current request
+ * @param path the canonical path of the node the view depends on
+ */
+ static void collect(ServletRequest request, String path) {
+ Object collector = request.getAttribute(ATTRIBUTE);
+ if (collector instanceof Collector && ((Collector) collector).enabled && StringUtils.isNotEmpty(path)) {
+ ((Collector) collector).paths.add(path);
+ }
+ }
+
+ /**
+ * Returns the paths collected for a resource.
+ *
+ * @param request the current request
+ * @param resource the rendered resource
+ * @return the collected paths, empty when none
+ */
+ static Set collected(ServletRequest request, Resource resource) {
+ Object collector = request.getAttribute(ATTRIBUTE);
+ // Resource overrides equals(): two renders of one node are two fragments
+ if (collector instanceof Collector && ((Collector) collector).resource == resource) {
+ return ((Collector) collector).paths;
+ }
+ return Set.of();
+ }
+
/**
* Reduces the collected paths and registers them on the resource, before the
* render chain stores its fragment.
@@ -42,11 +107,17 @@ public static void register(Resource resource, Collection paths, int lim
resource.getDependencies().addAll(reduced.paths);
resource.getRegexpDependencies().addAll(reduced.regexps);
- if (!reduced.regexps.isEmpty()) {
- logger.info(
- "{} autocollected {} node dependencies, more than the {} limit, so they were merged into {}. "
- + "If incorrect, narrow what the view lists, or set cache.autocollectDependencies=false on it and declare the dependency it needs.",
- resource, paths.size(), limit, reduced.regexps);
+ int entries = reduced.paths.size() + reduced.regexps.size();
+ if (entries > limit) {
+ logger.warn("{} autocollected {} node dependencies, and {} remain after the merge, more than the {} limit. "
+ + "No merge goes above /sites/. Narrow what the view links to, or set "
+ + "cache.autocollectDependencies=false on it and declare the dependency it needs.",
+ resource, reduced.collected, entries, limit);
+ } else if (!reduced.regexps.isEmpty()) {
+ logger.info("{} autocollected {} node dependencies, more than the {} limit, so some were merged into {}. "
+ + "If incorrect, narrow what the view links to, or set "
+ + "cache.autocollectDependencies=false on it and declare the dependency it needs.",
+ resource, reduced.collected, limit, reduced.regexps);
}
}
@@ -57,10 +128,13 @@ public static void register(Resource resource, Collection paths) {
/** The dependencies to register, after reduction. */
static final class Reduced {
+ /** The number of collected paths the reduction kept. */
+ final int collected;
final Collection paths;
final Collection regexps;
- private Reduced(Collection paths, Collection regexps) {
+ private Reduced(int collected, Collection paths, Collection regexps) {
+ this.collected = collected;
this.paths = paths;
this.regexps = regexps;
}
@@ -68,51 +142,105 @@ private Reduced(Collection paths, Collection regexps) {
/**
* Reduces a fragment's collected dependencies to at most {@code limit} cache
- * entries.
+ * entries, when a merge can reach that number.
+ *
+ * The paths under {@code /modules} are dropped, because the core cache
+ * filters store no dependency there. Above the limit, the deepest folder that
+ * covers two entries or more becomes one regexp, the biggest first at one
+ * depth. The merge goes up one depth at a time until the entries fit. A merge
+ * never covers a folder above {@value #MIN_MERGE_DEPTH} segments, such as
+ * {@code /sites}, so the result can stay above the limit.
*
* @param dependencies the paths the fragment resolved
* @param limit the number of entries a fragment may write
* @return the paths and regexps to register
*/
static Reduced reduce(Collection dependencies, int limit) {
- if (dependencies.size() <= limit) {
- return new Reduced(dependencies, Set.of());
+ Set kept = new LinkedHashSet<>();
+ for (String path : dependencies) {
+ if (!isUnderModules(path)) {
+ kept.add(path);
+ }
+ }
+ int collected = kept.size();
+ if (collected <= limit) {
+ return new Reduced(collected, kept, Set.of());
}
- return new Reduced(Set.of(), Set.of(commonRootRegex(dependencies)));
- }
-
- /**
- * A regexp matching the deepest path every dependency lives under and
- * everything
- * below it, or everything under the repository root when they share no first
- * segment.
- */
- private static String commonRootRegex(Collection paths) {
- String[] common = null;
- for (String path : paths) {
+ // the exact paths and the merged folders, each with its segments
+ Map exact = new LinkedHashMap<>();
+ Map folders = new LinkedHashMap<>();
+ int deepest = 0;
+ for (String path : kept) {
String[] segments = StringUtils.split(path, '/');
- if (common == null) {
- common = segments;
- continue;
- }
- int shared = 0;
- while (shared < common.length && shared < segments.length
- && common[shared].equals(segments[shared])) {
- shared++;
- }
- if (shared == 0) {
- return "/.*";
+ exact.put(path, segments);
+ deepest = Math.max(deepest, segments.length);
+ }
+
+ // Two folders at one depth never overlap, so a merge at one depth leaves the
+ // other folders of that depth as they were
+ for (int depth = deepest; depth >= MIN_MERGE_DEPTH && exact.size() + folders.size() > limit; depth--) {
+ Map atDepth = new HashMap<>();
+ group(exact, depth, atDepth, true);
+ group(folders, depth, atDepth, false);
+
+ List candidates = new ArrayList<>();
+ for (Folder folder : atDepth.values()) {
+ if (folder.size() >= 2) {
+ candidates.add(folder);
+ }
}
- if (shared < common.length) {
- common = Arrays.copyOf(common, shared);
+ candidates.sort(Comparator.comparingInt(Folder::size).reversed().thenComparing(folder -> folder.key));
+
+ for (Folder folder : candidates) {
+ if (exact.size() + folders.size() <= limit) {
+ break;
+ }
+ folder.exact.forEach(exact::remove);
+ folder.folders.forEach(folders::remove);
+ folders.put(folder.key, folder.segments);
}
}
- if (common == null || common.length == 0) {
- return "/.*";
+ Set regexps = new LinkedHashSet<>();
+ for (String folder : folders.keySet()) {
+ regexps.add("/" + Pattern.quote(folder) + "(/.*)?");
+ }
+ return new Reduced(collected, new LinkedHashSet<>(exact.keySet()), regexps);
+ }
+
+ private static boolean isUnderModules(String path) {
+ return "/modules".equals(path) || path.startsWith("/modules/");
+ }
+
+ /** The entries at or below one folder. An entry covers its own path, because a regexp on a folder matches it. */
+ private static final class Folder {
+ final String key;
+ final String[] segments;
+ final List exact = new ArrayList<>();
+ final List folders = new ArrayList<>();
+
+ Folder(String key, String[] segments) {
+ this.key = key;
+ this.segments = segments;
}
- return "/" + Pattern.quote(String.join("/", common)) + "(/.*)?";
+ int size() {
+ return exact.size() + folders.size();
+ }
+ }
+
+ /** Groups the entries deep enough by their folder at one depth. */
+ private static void group(Map entries, int depth, Map atDepth, boolean exact) {
+ for (Map.Entry entry : entries.entrySet()) {
+ String[] segments = entry.getValue();
+ if (segments.length < depth) {
+ continue;
+ }
+ String[] folderSegments = Arrays.copyOf(segments, depth);
+ Folder folder = atDepth.computeIfAbsent(String.join("/", folderSegments),
+ key -> new Folder(key, folderSegments));
+ (exact ? folder.exact : folder.folders).add(entry.getKey());
+ }
}
}
diff --git a/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/js/server/CollectedCacheDependenciesFilter.java b/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/js/server/CollectedCacheDependenciesFilter.java
new file mode 100644
index 00000000..a78d9d0f
--- /dev/null
+++ b/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/js/server/CollectedCacheDependenciesFilter.java
@@ -0,0 +1,94 @@
+package org.jahia.modules.javascript.modules.engine.js.server;
+
+import java.util.Properties;
+import java.util.Set;
+
+import javax.jcr.RepositoryException;
+
+import org.jahia.services.render.RenderContext;
+import org.jahia.services.render.Resource;
+import org.jahia.services.render.View;
+import org.jahia.services.render.filter.AbstractFilter;
+import org.jahia.services.render.filter.RenderChain;
+import org.jahia.services.render.filter.RenderFilter;
+import org.jahia.services.render.scripting.Script;
+import org.osgi.service.component.annotations.Component;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Gives each resource that the cache stores as a fragment a collector for the
+ * cache dependencies its view resolves, and registers what it collected.
+ *
+ * {@code AggregateCacheFilter} (16) keeps the key of the fragment it stores in
+ * the module param {@code cacheKey}, from its {@code prepare} to its
+ * {@code execute}. This filter runs only on a resource that holds that param,
+ * so it follows the decision of the cache on every core version. A resource the
+ * cache does not store, such as an include, a wrapper, an option or a fragment
+ * cached with its parent, gets no collector of its own. Its view then collects
+ * into the collector of the enclosing fragment, which holds its markup. The
+ * opt-out of the enclosing view then decides for both views, and the opt-out of
+ * the inner view has no effect.
+ *
+ * The deprecated cache implementation, {@code AggregateFilter} with
+ * {@code CacheFilter}, sets no such param, so this filter collects nothing
+ * with it.
+ *
+ * The priority puts {@link #prepare} after the cache filter (16) sets the
+ * param. It puts {@link #execute} after the script and the wrappers, and before
+ * the cache filter removes the param and stores the dependencies of the
+ * fragment. A render
+ * filter whose priority is below this one runs its own {@code execute} after
+ * this filter, so a path it collects there is not registered.
+ */
+@Component(service = RenderFilter.class, immediate = true)
+public class CollectedCacheDependenciesFilter extends AbstractFilter {
+
+ private static final Logger logger = LoggerFactory.getLogger(CollectedCacheDependenciesFilter.class);
+
+ /** The view property that turns autocollection off for a view. */
+ static final String DISABLE_PROPERTY = "cache.autocollectDependencies";
+
+ static final float PRIORITY = 17f;
+
+ /** The module param of the fragment key that {@code AggregateCacheFilter} stores. */
+ static final String CACHE_KEY = "cacheKey";
+
+ public CollectedCacheDependenciesFilter() {
+ setPriority(PRIORITY);
+ setDescription("Registers the cache dependencies a JavaScript view autocollected while it rendered.");
+ addCondition((renderContext, resource) -> resource.getModuleParams().get(CACHE_KEY) != null);
+ }
+
+ @Override
+ public String prepare(RenderContext renderContext, Resource resource, RenderChain chain) throws Exception {
+ chain.pushAttribute(renderContext.getRequest(), CollectedCacheDependencies.ATTRIBUTE,
+ new CollectedCacheDependencies.Collector(resource, isEnabled(renderContext, resource)));
+ return null;
+ }
+
+ @Override
+ public String execute(String previousOut, RenderContext renderContext, Resource resource, RenderChain chain)
+ throws Exception {
+ Set paths = CollectedCacheDependencies.collected(renderContext.getRequest(), resource);
+ CollectedCacheDependencies.register(resource, paths);
+ return previousOut;
+ }
+
+ private boolean isEnabled(RenderContext renderContext, Resource resource) {
+ try {
+ Properties properties = viewProperties(renderContext, resource);
+ return properties == null || !"false".equals(properties.getProperty(DISABLE_PROPERTY));
+ } catch (RepositoryException e) {
+ logger.debug("Cannot resolve the view of {}, its dependencies are collected", resource, e);
+ return true;
+ }
+ }
+
+ /** The properties of the view that renders the resource, or null when it has none. */
+ Properties viewProperties(RenderContext renderContext, Resource resource) throws RepositoryException {
+ Script script = resource.getScript(renderContext);
+ View view = script == null ? null : script.getView();
+ return view == null ? null : view.getProperties();
+ }
+}
diff --git a/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/js/server/RenderHelper.java b/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/js/server/RenderHelper.java
index db0b42be..89de4ac9 100644
--- a/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/js/server/RenderHelper.java
+++ b/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/js/server/RenderHelper.java
@@ -44,6 +44,7 @@
import javax.annotation.Nullable;
import javax.inject.Inject;
import javax.jcr.RepositoryException;
+import javax.servlet.http.HttpServletRequest;
import javax.servlet.jsp.JspException;
import javax.servlet.jsp.tagext.BodyTagSupport;
import javax.servlet.jsp.tagext.TagSupport;
@@ -343,6 +344,20 @@ public void addCacheDependency(Map attr, RenderContext renderCon
renderTag(new AddCacheDependencyTag(), attr, renderContext);
}
+ /**
+ * Collect a cache dependency for the fragment being rendered. Unlike
+ * {@link #addCacheDependency}, the dependency is registered once the view has
+ * rendered. A view that sets {@code cache.autocollectDependencies} to
+ * {@code false} collects nothing. Above 100 paths, the paths are merged into
+ * regular expressions.
+ *
+ * @param path the canonical path of the node to add as a dependency
+ * @param request the current request, which holds the collector of the fragment
+ */
+ public void autocollectCacheDependency(String path, HttpServletRequest request) {
+ CollectedCacheDependencies.collect(request, path);
+ }
+
/**
* Calculate the relative path from one node to another node.
*
diff --git a/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/views/JSScript.java b/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/views/JSScript.java
index ef1f7566..d0306735 100644
--- a/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/views/JSScript.java
+++ b/javascript-modules-engine-java/src/main/java/org/jahia/modules/javascript/modules/engine/views/JSScript.java
@@ -18,10 +18,6 @@
import org.graalvm.polyglot.Value;
import org.graalvm.polyglot.proxy.ProxyObject;
import org.jahia.modules.javascript.modules.engine.jsengine.GraalVMEngine;
-import java.util.ArrayList;
-import java.util.List;
-
-import org.jahia.modules.javascript.modules.engine.js.server.CollectedCacheDependencies;
import org.jahia.services.render.RenderContext;
import org.jahia.services.render.RenderException;
import org.jahia.services.render.Resource;
@@ -65,7 +61,7 @@ public String execute(Resource resource, RenderContext renderContext) throws Ren
viewValues.put("bundle", Value.asValue(jsView.getModule().getBundle()));
Object executionResult = Value.asValue(viewRenderer.get("render")).execute(resource, renderContext, ProxyObject.fromMap(viewValues));
- return readRenderResult(Value.asValue(executionResult), resource);
+ return Value.asValue(executionResult).asString();
}));
if (jsView.isTemplate()) {
@@ -81,26 +77,6 @@ public String execute(Resource resource, RenderContext renderContext) throws Ren
return output;
}
- /**
- * Reads what a view renderer returned: the markup, and the cache dependencies
- * the view autocollected while it rendered.
- *
- * @param result what the renderer returned
- * @param resource the resource that was rendered
- * @return the markup
- */
- private String readRenderResult(Value result, Resource resource) {
- Value dependencies = result.getMember("autocollectedDependencies");
- int length = (int) dependencies.getArraySize();
- List collected = new ArrayList<>(length);
- for (int i = 0; i < length; i++) {
- collected.add(dependencies.getArrayElement(i).asString());
- }
- CollectedCacheDependencies.register(resource, collected);
-
- return result.getMember("html").asString();
- }
-
@Override
public View getView() {
return jsView;
diff --git a/javascript-modules-engine-java/src/test/java/org/jahia/modules/javascript/modules/engine/js/server/CollectedCacheDependenciesFilterTest.java b/javascript-modules-engine-java/src/test/java/org/jahia/modules/javascript/modules/engine/js/server/CollectedCacheDependenciesFilterTest.java
new file mode 100644
index 00000000..e78669a1
--- /dev/null
+++ b/javascript-modules-engine-java/src/test/java/org/jahia/modules/javascript/modules/engine/js/server/CollectedCacheDependenciesFilterTest.java
@@ -0,0 +1,207 @@
+package org.jahia.modules.javascript.modules.engine.js.server;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+import java.lang.reflect.Proxy;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.jahia.services.render.RenderContext;
+import org.jahia.services.render.Resource;
+import org.jahia.services.render.filter.AbstractFilter;
+import org.jahia.services.render.filter.RenderChain;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Runs {@link CollectedCacheDependenciesFilter} in a real {@link RenderChain},
+ * between a cache filter and a view that stand in for those of Jahia.
+ */
+public class CollectedCacheDependenciesFilterTest {
+
+ private RenderContext renderContext;
+ // keyed by identity: Resource overrides equals, and two renders of one node are two fragments
+ /** The resources the cache stores, and the dependencies each one had when the cache stored it. */
+ private final Set cached = Collections.newSetFromMap(new IdentityHashMap<>());
+ private final Map> stored = new IdentityHashMap<>();
+ /** What each view does while it renders. */
+ private final Map views = new IdentityHashMap<>();
+ private final Set optedOut = Collections.newSetFromMap(new IdentityHashMap<>());
+
+ /** Stands in for AggregateCacheFilter: it sets the cache key of a fragment it stores, and removes it in execute. */
+ private final AbstractFilter cache = new AbstractFilter() {
+ {
+ setPriority(16);
+ }
+
+ @Override
+ public String prepare(RenderContext renderContext, Resource resource, RenderChain chain) {
+ if (cached.contains(resource)) {
+ resource.getModuleParams().put(CollectedCacheDependenciesFilter.CACHE_KEY, "key");
+ }
+ return null;
+ }
+
+ @Override
+ public String execute(String previousOut, RenderContext renderContext, Resource resource, RenderChain chain) {
+ if (resource.getModuleParams().remove(CollectedCacheDependenciesFilter.CACHE_KEY) != null) {
+ Set dependencies = new HashSet<>(resource.getDependencies());
+ dependencies.addAll(resource.getRegexpDependencies());
+ dependencies.remove(resource.getNodePath());
+ stored.put(resource, dependencies);
+ }
+ return previousOut;
+ }
+ };
+
+ private final CollectedCacheDependenciesFilter filter = new CollectedCacheDependenciesFilter() {
+ @Override
+ Properties viewProperties(RenderContext renderContext, Resource resource) {
+ Properties properties = new Properties();
+ if (optedOut.contains(resource)) {
+ properties.setProperty(DISABLE_PROPERTY, "false");
+ }
+ return properties;
+ }
+ };
+
+ /** Stands in for the script: it runs the view of the resource. */
+ private final AbstractFilter script = new AbstractFilter() {
+ {
+ setPriority(100);
+ }
+
+ @Override
+ public String prepare(RenderContext renderContext, Resource resource, RenderChain chain) {
+ views.getOrDefault(resource, () -> { }).run();
+ return "";
+ }
+ };
+
+ /** A request that only keeps its attributes. */
+ private static HttpServletRequest request() {
+ Map attributes = new HashMap<>();
+ return (HttpServletRequest) Proxy.newProxyInstance(HttpServletRequest.class.getClassLoader(),
+ new Class[] {HttpServletRequest.class}, (proxy, method, args) -> {
+ switch (method.getName()) {
+ case "getAttribute":
+ return attributes.get(args[0]);
+ case "setAttribute":
+ attributes.put(args[0], args[1]);
+ return null;
+ case "removeAttribute":
+ attributes.remove(args[0]);
+ return null;
+ default:
+ throw new UnsupportedOperationException(method.getName());
+ }
+ });
+ }
+
+ private static Resource resource(String path) {
+ return new Resource(path, path, null, "html", "default", "module");
+ }
+
+ @Before
+ public void setUp() {
+ renderContext = new RenderContext(request(), null, null);
+ }
+
+ private void render(Resource resource) throws Exception {
+ new RenderChain(cache, filter, script).doFilter(renderContext, resource);
+ }
+
+ private void collect(String path) {
+ CollectedCacheDependencies.collect(renderContext.getRequest(), path);
+ }
+
+ @Test
+ public void shouldRegisterWhatTheViewCollectedBeforeTheCacheStoresIt() throws Exception {
+ Resource page = resource("/sites/mysite/home");
+ cached.add(page);
+ views.put(page, () -> collect("/sites/mysite/home/about"));
+
+ render(page);
+
+ assertEquals(Set.of("/sites/mysite/home/about"), stored.get(page));
+ assertNull(renderContext.getRequest().getAttribute(CollectedCacheDependencies.ATTRIBUTE));
+ }
+
+ @Test
+ public void shouldKeepTheCollectorOfANestedFragmentApart() throws Exception {
+ Resource page = resource("/sites/mysite/home");
+ Resource teaser = resource("/sites/mysite/home/area/teaser");
+ cached.add(page);
+ cached.add(teaser);
+ views.put(teaser, () -> collect("/sites/mysite/files/logo.png"));
+ views.put(page, () -> {
+ collect("/sites/mysite/home/about");
+ try {
+ render(teaser);
+ } catch (Exception e) {
+ throw new IllegalStateException(e);
+ }
+ collect("/sites/mysite/home/contact");
+ });
+
+ render(page);
+
+ assertEquals(Set.of("/sites/mysite/files/logo.png"), stored.get(teaser));
+ assertEquals(Set.of("/sites/mysite/home/about", "/sites/mysite/home/contact"), stored.get(page));
+ }
+
+ @Test
+ public void shouldCollectIntoTheEnclosingFragmentWhatTheCacheDoesNotStore() throws Exception {
+ Resource page = resource("/sites/mysite/home");
+ Resource include = resource("/sites/mysite/home/area/include");
+ cached.add(page);
+ views.put(include, () -> collect("/sites/mysite/files/logo.png"));
+ views.put(page, () -> {
+ try {
+ render(include);
+ } catch (Exception e) {
+ throw new IllegalStateException(e);
+ }
+ });
+
+ render(page);
+
+ assertEquals(Set.of("/sites/mysite/files/logo.png"), stored.get(page));
+ assertEquals(List.of(page), new ArrayList<>(stored.keySet()));
+ }
+
+ @Test
+ public void shouldCollectNothingForAViewThatOptedOut() throws Exception {
+ Resource page = resource("/sites/mysite/home");
+ cached.add(page);
+ optedOut.add(page);
+ views.put(page, () -> collect("/sites/mysite/home/about"));
+
+ render(page);
+
+ assertEquals(Set.of(), stored.get(page));
+ }
+
+ @Test
+ public void shouldCollectNothingOutsideACachedFragment() throws Exception {
+ Resource page = resource("/sites/mysite/home");
+ views.put(page, () -> collect("/sites/mysite/home/about"));
+
+ render(page);
+
+ assertEquals(Set.of(), page.getDependencies().stream()
+ .filter(path -> !path.equals(page.getNodePath())).collect(Collectors.toSet()));
+ assertNull(renderContext.getRequest().getAttribute(CollectedCacheDependencies.ATTRIBUTE));
+ }
+}
diff --git a/javascript-modules-engine-java/src/test/java/org/jahia/modules/javascript/modules/engine/js/server/CollectedCacheDependenciesReduceTest.java b/javascript-modules-engine-java/src/test/java/org/jahia/modules/javascript/modules/engine/js/server/CollectedCacheDependenciesReduceTest.java
new file mode 100644
index 00000000..ecd63faa
--- /dev/null
+++ b/javascript-modules-engine-java/src/test/java/org/jahia/modules/javascript/modules/engine/js/server/CollectedCacheDependenciesReduceTest.java
@@ -0,0 +1,279 @@
+package org.jahia.modules.javascript.modules.engine.js.server;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.jahia.services.render.Resource;
+import org.junit.Test;
+
+/**
+ * The invariants of {@link CollectedCacheDependencies#reduce} on generated
+ * input, and its edge cases.
+ */
+public class CollectedCacheDependenciesReduceTest {
+
+ private static final Pattern FOLDER_OF_REGEXP = Pattern.compile("/\\\\Q(.*)\\\\E\\(/\\.\\*\\)\\?");
+
+ private static final long SEED = 20261013L;
+ private static final int CASES = 500;
+ private static final int[] LIMITS = {1, 2, 5, 20, 100};
+
+ private static String regexp(String folder) {
+ return "/\\Q" + folder.substring(1) + "\\E(/.*)?";
+ }
+
+ /** The folder a regexp of the reduction covers, without its leading slash. */
+ private static String folderOf(String regexp) {
+ Matcher matcher = FOLDER_OF_REGEXP.matcher(regexp);
+ assertTrue("not a regexp of the reduction: " + regexp, matcher.matches());
+ return matcher.group(1);
+ }
+
+ private static boolean isUnderModules(String path) {
+ return "/modules".equals(path) || path.startsWith("/modules/");
+ }
+
+ private static boolean isAtOrBelow(String path, String folder) {
+ String relative = path.startsWith("/") ? path.substring(1) : path;
+ return relative.equals(folder) || relative.startsWith(folder + "/");
+ }
+
+ /** Paths over a few sites, user folders and modules, where siblings share names and character prefixes. */
+ private static List generate(Random random) {
+ String[] roots = {"/sites/s0", "/sites/s1", "/sites/s10", "/users/ab/cd", "/modules/m"};
+ String[] names = {"n0", "n1", "n10", "a.b", "aXb"};
+ List paths = new ArrayList<>();
+ int count = 1 + random.nextInt(400);
+ for (int i = 0; i < count; i++) {
+ StringBuilder path = new StringBuilder(roots[random.nextInt(roots.length)]);
+ int depth = random.nextInt(5);
+ for (int d = 0; d < depth; d++) {
+ path.append('/').append(names[random.nextInt(names.length)]).append(random.nextInt(4));
+ }
+ paths.add(path.toString());
+ }
+ if (random.nextInt(10) == 0) {
+ paths.add("/sites");
+ }
+ if (random.nextInt(10) == 0) {
+ paths.add("/");
+ }
+ return paths;
+ }
+
+ @Test
+ public void shouldHoldItsInvariantsOnGeneratedInput() {
+ Random random = new Random(SEED);
+ for (int c = 0; c < CASES; c++) {
+ List input = generate(random);
+ int limit = LIMITS[random.nextInt(LIMITS.length)];
+ String context = "case " + c + " (seed " + SEED + "), limit " + limit + ", input " + input;
+
+ CollectedCacheDependencies.Reduced reduced = CollectedCacheDependencies.reduce(input, limit);
+ Set kept = new LinkedHashSet<>();
+ for (String path : input) {
+ if (!isUnderModules(path)) {
+ kept.add(path);
+ }
+ }
+ List folders = new ArrayList<>();
+ for (String regexp : reduced.regexps) {
+ folders.add(folderOf(regexp));
+ }
+
+ assertEquals(context, kept.size(), reduced.collected);
+ assertTrue("an exact path that was not collected: " + context, kept.containsAll(reduced.paths));
+
+ // nothing is lost: each kept path is exact or covered by a regexp
+ for (String path : kept) {
+ boolean covered = reduced.paths.contains(path)
+ || reduced.regexps.stream().anyMatch(path::matches);
+ assertTrue("lost " + path + ": " + context, covered);
+ }
+
+ // nothing is merged within the limit
+ if (kept.size() <= limit) {
+ assertEquals(context, kept, new HashSet<>(reduced.paths));
+ assertTrue(context, reduced.regexps.isEmpty());
+ continue;
+ }
+
+ for (String folder : folders) {
+ // the floor: never above /sites/
+ assertTrue("merged above the floor: " + folder + ": " + context,
+ folder.split("/").length >= CollectedCacheDependencies.MIN_MERGE_DEPTH);
+ assertFalse("merged a module folder: " + context, folder.startsWith("modules"));
+ // precision: a merged folder holds two collected paths or more
+ long holds = kept.stream().filter(path -> isAtOrBelow(path, folder)).count();
+ assertTrue("merged " + folder + " for " + holds + " path: " + context, holds >= 2);
+ // no redundancy: no entry lies inside a merged folder
+ for (String path : reduced.paths) {
+ assertFalse(path + " lies in " + folder + ": " + context, isAtOrBelow(path, folder));
+ }
+ for (String other : folders) {
+ assertTrue(other + " lies in " + folder + ": " + context,
+ other.equals(folder) || !isAtOrBelow(other, folder));
+ }
+ }
+
+ // the bound: above the limit, no folder of two segments or more holds two entries
+ int entries = reduced.paths.size() + reduced.regexps.size();
+ if (entries > limit) {
+ Map covered = new HashMap<>();
+ Collection all = new ArrayList<>(reduced.paths);
+ folders.forEach(folder -> all.add("/" + folder));
+ for (String entry : all) {
+ String[] segments = entry.substring(1).split("/");
+ for (int depth = CollectedCacheDependencies.MIN_MERGE_DEPTH; depth <= segments.length; depth++) {
+ covered.merge(String.join("/", Arrays.copyOf(segments, depth)), 1, Integer::sum);
+ }
+ }
+ covered.forEach((folder, count) -> {
+ if (count >= 2) {
+ fail(folder + " could still merge " + count + " entries: " + context);
+ }
+ });
+ }
+
+ // the result does not depend on the order of the input
+ List shuffled = new ArrayList<>(input);
+ java.util.Collections.shuffle(shuffled, new Random(c));
+ CollectedCacheDependencies.Reduced again = CollectedCacheDependencies.reduce(shuffled, limit);
+ assertEquals(context, new HashSet<>(reduced.paths), new HashSet<>(again.paths));
+ assertEquals(context, new HashSet<>(reduced.regexps), new HashSet<>(again.regexps));
+ }
+ }
+
+ @Test
+ public void shouldNotMergeAtTheLimit() {
+ Set paths = new HashSet<>();
+ for (int i = 0; i < 100; i++) {
+ paths.add("/sites/mysite/home/item-" + i);
+ }
+
+ assertTrue(CollectedCacheDependencies.reduce(paths, 100).regexps.isEmpty());
+
+ paths.add("/sites/mysite/home/item-100");
+ assertEquals(Set.of(regexp("/sites/mysite/home")), CollectedCacheDependencies.reduce(paths, 100).regexps);
+ }
+
+ @Test
+ public void shouldCountTheLimitAfterDroppingModulePaths() {
+ List paths = List.of("/sites/a/x/p1", "/modules", "/modules/m/views/one", "/sites/a/x/p2");
+
+ // four paths, but two once /modules is dropped, so a limit of two merges nothing
+ CollectedCacheDependencies.Reduced reduced = CollectedCacheDependencies.reduce(paths, 2);
+
+ assertEquals(2, reduced.collected);
+ assertEquals(List.of("/sites/a/x/p1", "/sites/a/x/p2"), new ArrayList<>(reduced.paths));
+ assertTrue(reduced.regexps.isEmpty());
+ }
+
+ @Test
+ public void shouldAbsorbADeeperMergeIntoAHigherOne() {
+ List paths = List.of("/sites/a/x/p1", "/sites/a/x/p2", "/sites/a/x/p3", "/sites/a/y/q1",
+ "/sites/a/z/r1");
+
+ CollectedCacheDependencies.Reduced toThree = CollectedCacheDependencies.reduce(paths, 3);
+ assertEquals(Set.of(regexp("/sites/a/x")), toThree.regexps);
+ assertEquals(Set.of("/sites/a/y/q1", "/sites/a/z/r1"), new HashSet<>(toThree.paths));
+
+ // x merges first, then the site folder takes x, y and z
+ CollectedCacheDependencies.Reduced toOne = CollectedCacheDependencies.reduce(paths, 1);
+ assertEquals(Set.of(regexp("/sites/a")), toOne.regexps);
+ assertTrue(toOne.paths.isEmpty());
+ }
+
+ @Test
+ public void shouldDropTheExactFolderItMerges() {
+ Set paths = new HashSet<>();
+ paths.add("/sites/mysite/contents/posts");
+ for (int i = 0; i < 150; i++) {
+ paths.add("/sites/mysite/contents/posts/item-" + i);
+ }
+
+ CollectedCacheDependencies.Reduced reduced = CollectedCacheDependencies.reduce(paths, 100);
+
+ assertTrue(reduced.paths.isEmpty());
+ assertEquals(Set.of(regexp("/sites/mysite/contents/posts")), reduced.regexps);
+ }
+
+ @Test
+ public void shouldKeepPathsAboveTheFloorExact() {
+ List paths = List.of("/", "/sites", "/sites/a", "/sites/b");
+
+ CollectedCacheDependencies.Reduced reduced = CollectedCacheDependencies.reduce(paths, 1);
+
+ assertEquals(new HashSet<>(paths), new HashSet<>(reduced.paths));
+ assertTrue(reduced.regexps.isEmpty());
+ }
+
+ @Test
+ public void shouldRegisterExactPathsWithinTheLimit() {
+ Resource resource = new Resource("/sites/mysite/home", "/sites/mysite/home", null, "html", "default", "module");
+
+ CollectedCacheDependencies.register(resource, List.of("/sites/mysite/files/logo.png", "/modules/m/x"));
+
+ assertEquals(Set.of("/sites/mysite/home", "/sites/mysite/files/logo.png"), resource.getDependencies());
+ assertTrue(resource.getRegexpDependencies().isEmpty());
+ }
+
+ @Test
+ public void shouldRegisterTheMergedFolders() {
+ Resource resource = new Resource("/sites/mysite/home", "/sites/mysite/home", null, "html", "default", "module");
+ List paths = new ArrayList<>();
+ for (int i = 0; i < 150; i++) {
+ paths.add("/sites/mysite/contents/posts/item-" + i);
+ }
+ paths.add("/sites/mysite/files/logo.png");
+
+ CollectedCacheDependencies.register(resource, paths);
+
+ assertEquals(Set.of("/sites/mysite/home", "/sites/mysite/files/logo.png"), resource.getDependencies());
+ assertEquals(Set.of(regexp("/sites/mysite/contents/posts")), resource.getRegexpDependencies());
+ }
+
+ @Test
+ public void shouldRegisterEveryPathWhenTheLimitCannotBeReached() {
+ Resource resource = new Resource("/sites/portal/home", "/sites/portal/home", null, "html", "default", "module");
+ List paths = new ArrayList<>();
+ for (int i = 0; i < 150; i++) {
+ paths.add("/sites/site-" + i + "/home");
+ }
+
+ CollectedCacheDependencies.register(resource, paths);
+
+ assertEquals(151, resource.getDependencies().size());
+ assertTrue(resource.getRegexpDependencies().isEmpty());
+ }
+
+ /** The worst case for a merge: many folders of two paths, so each merge removes one entry. */
+ @Test(timeout = 5000)
+ public void shouldReduceManySmallFoldersQuickly() {
+ List paths = new ArrayList<>();
+ for (int i = 0; i < 2500; i++) {
+ paths.add("/sites/mysite/folder-" + i + "/a");
+ paths.add("/sites/mysite/folder-" + i + "/b");
+ }
+
+ CollectedCacheDependencies.Reduced reduced = CollectedCacheDependencies.reduce(paths, 100);
+
+ assertEquals(Set.of(regexp("/sites/mysite")), reduced.regexps);
+ assertTrue(reduced.paths.isEmpty());
+ }
+}
diff --git a/javascript-modules-engine-java/src/test/java/org/jahia/modules/javascript/modules/engine/js/server/CollectedCacheDependenciesTest.java b/javascript-modules-engine-java/src/test/java/org/jahia/modules/javascript/modules/engine/js/server/CollectedCacheDependenciesTest.java
index b0e11cfd..d8dd4101 100644
--- a/javascript-modules-engine-java/src/test/java/org/jahia/modules/javascript/modules/engine/js/server/CollectedCacheDependenciesTest.java
+++ b/javascript-modules-engine-java/src/test/java/org/jahia/modules/javascript/modules/engine/js/server/CollectedCacheDependenciesTest.java
@@ -4,9 +4,15 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import java.lang.reflect.Proxy;
+import java.util.HashMap;
import java.util.HashSet;
+import java.util.Map;
import java.util.Set;
+import javax.servlet.http.HttpServletRequest;
+
+import org.jahia.services.render.Resource;
import org.junit.Test;
public class CollectedCacheDependenciesTest {
@@ -19,19 +25,46 @@ private static Set children(String parent, int count) {
return paths;
}
- private static String mergedInto(Set dependencies) {
- CollectedCacheDependencies.Reduced reduced = CollectedCacheDependencies.reduce(dependencies, 100);
- assertFalse("the dependencies were expected to be merged", reduced.regexps.isEmpty());
- assertTrue("a merged fragment keeps no exact dependency", reduced.paths.isEmpty());
- assertEquals(1, reduced.regexps.size());
- return reduced.regexps.iterator().next();
+ private static String regexp(String folder) {
+ return "/\\Q" + folder.substring(1) + "\\E(/.*)?";
+ }
+
+ private static CollectedCacheDependencies.Reduced reduce(Set dependencies) {
+ return CollectedCacheDependencies.reduce(dependencies, 100);
+ }
+
+ private static Resource resource(String path) {
+ return new Resource(path, path, null, "html", "default", "module");
+ }
+
+ /** A request that only keeps its attributes. */
+ private static HttpServletRequest request() {
+ Map attributes = new HashMap<>();
+ return (HttpServletRequest) Proxy.newProxyInstance(HttpServletRequest.class.getClassLoader(),
+ new Class[] {HttpServletRequest.class}, (proxy, method, args) -> {
+ switch (method.getName()) {
+ case "getAttribute":
+ return attributes.get(args[0]);
+ case "setAttribute":
+ attributes.put(args[0], args[1]);
+ return null;
+ default:
+ throw new UnsupportedOperationException(method.getName());
+ }
+ });
+ }
+
+ /** Starts the render of a cached fragment, as the filter does in prepare. */
+ private static void startRender(HttpServletRequest request, Resource resource, boolean enabled) {
+ request.setAttribute(CollectedCacheDependencies.ATTRIBUTE,
+ new CollectedCacheDependencies.Collector(resource, enabled));
}
@Test
public void shouldKeepEveryDependencyExactWithinBudget() {
Set dependencies = children("/sites/mysite/contents/posts", 100);
- CollectedCacheDependencies.Reduced reduced = CollectedCacheDependencies.reduce(dependencies, 100);
+ CollectedCacheDependencies.Reduced reduced = reduce(dependencies);
assertEquals(dependencies, reduced.paths);
assertTrue(reduced.regexps.isEmpty());
@@ -39,75 +72,177 @@ public void shouldKeepEveryDependencyExactWithinBudget() {
@Test
public void shouldMergeAListingIntoTheFolderItLivesIn() {
- String regexp = mergedInto(children("/sites/mysite/contents/posts", 150));
+ CollectedCacheDependencies.Reduced reduced = reduce(children("/sites/mysite/contents/posts", 150));
- assertTrue("/sites/mysite/contents/posts/item-1".matches(regexp));
+ assertTrue(reduced.paths.isEmpty());
+ assertEquals(Set.of(regexp("/sites/mysite/contents/posts")), reduced.regexps);
+ String regexp = reduced.regexps.iterator().next();
// an edited title lands on a subnode of the item
assertTrue("/sites/mysite/contents/posts/item-1/j:translation_en".matches(regexp));
- // the folder may itself be one of the linked nodes: renaming it must flush the
- // fragment
- assertTrue("the merged folder itself is covered",
- "/sites/mysite/contents/posts".matches(regexp));
- assertFalse("a sibling folder is not covered",
- "/sites/mysite/contents/events/item-1".matches(regexp));
+ // the folder may itself be one of the linked nodes: renaming it must flush the fragment
+ assertTrue("/sites/mysite/contents/posts".matches(regexp));
+ assertFalse("/sites/mysite/contents/events/item-1".matches(regexp));
}
@Test
- public void shouldMergeToTheDeepestSharedFolder() {
- Set dependencies = children("/sites/mysite/contents/posts", 80);
- dependencies.addAll(children("/sites/mysite/contents/events", 80));
+ public void shouldKeepALoneLinkOutsideTheListingExact() {
+ Set dependencies = children("/sites/mysite/home", 120);
+ dependencies.add("/sites/mysite/files/logo.png");
- String regexp = mergedInto(dependencies);
+ CollectedCacheDependencies.Reduced reduced = reduce(dependencies);
- assertTrue("/sites/mysite/contents/posts/item-1".matches(regexp));
- assertTrue("/sites/mysite/contents/events/item-1".matches(regexp));
- assertFalse("nothing above the shared folder is covered",
- "/sites/mysite/files/logo.png".matches(regexp));
+ assertEquals(Set.of("/sites/mysite/files/logo.png"), new HashSet<>(reduced.paths));
+ assertEquals(Set.of(regexp("/sites/mysite/home")), reduced.regexps);
}
@Test
- public void shouldCompareWholeSegmentsRatherThanCharacters() {
- Set dependencies = children("/sites/mysite/contents", 80);
- dependencies.addAll(children("/sites/mysite2/contents", 80));
+ public void shouldMergeOnlyWhatTheLimitNeeds() {
+ Set dependencies = children("/sites/mysite/contents/events", 80);
+ dependencies.addAll(children("/sites/mysite/contents/posts", 80));
+
+ CollectedCacheDependencies.Reduced reduced = reduce(dependencies);
+
+ // merging one folder brings 160 entries down to 81, so the other one stays exact
+ assertEquals(Set.of(regexp("/sites/mysite/contents/events")), reduced.regexps);
+ assertEquals(children("/sites/mysite/contents/posts", 80), new HashSet<>(reduced.paths));
+ }
+
+ @Test
+ public void shouldMergeTheBiggestFolderFirstAtOneDepth() {
+ Set dependencies = children("/sites/mysite/contents/events", 30);
+ dependencies.addAll(children("/sites/mysite/contents/posts", 90));
+
+ CollectedCacheDependencies.Reduced reduced = reduce(dependencies);
+
+ assertEquals(Set.of(regexp("/sites/mysite/contents/posts")), reduced.regexps);
+ assertEquals(30, reduced.paths.size());
+ }
+
+ @Test
+ public void shouldMergeUpwardsUntilTheEntriesFit() {
+ Set dependencies = new HashSet<>();
+ for (int i = 0; i < 150; i++) {
+ dependencies.add("/sites/mysite/home/section-" + i + "/page");
+ }
+
+ CollectedCacheDependencies.Reduced reduced = reduce(dependencies);
+
+ // no section holds two pages, so the merge goes up to the folder that holds the sections
+ assertTrue(reduced.paths.isEmpty());
+ assertEquals(Set.of(regexp("/sites/mysite/home")), reduced.regexps);
+ }
+
+ @Test
+ public void shouldCompareWholeSegmentsInsideOneSite() {
+ Set dependencies = children("/sites/mysite/contents", 60);
+ dependencies.addAll(children("/sites/mysite/contents2", 60));
- String regexp = mergedInto(dependencies);
+ CollectedCacheDependencies.Reduced reduced = reduce(dependencies);
- // /sites/mysite is a character prefix of /sites/mysite2, but not an ancestor of
- // it
- assertTrue("/sites/mysite2/contents/item-1".matches(regexp));
- assertTrue("/sites/mysite/contents/item-1".matches(regexp));
+ assertEquals(Set.of(regexp("/sites/mysite/contents")), reduced.regexps);
+ // contents is a character prefix of contents2, but not an ancestor of it
+ assertFalse("/sites/mysite/contents2/item-1".matches(reduced.regexps.iterator().next()));
+ assertEquals(children("/sites/mysite/contents2", 60), new HashSet<>(reduced.paths));
}
@Test
- public void shouldMergeToTheRepositoryRootWhenNothingIsShared() {
+ public void shouldNeverMergeAboveTheSite() {
+ Set dependencies = new HashSet<>();
+ for (int i = 0; i < 150; i++) {
+ dependencies.add("/sites/site-" + i + "/home");
+ }
+
+ CollectedCacheDependencies.Reduced reduced = reduce(dependencies);
+
+ // no folder below /sites holds two links: the paths stay exact, above the limit
+ assertEquals(dependencies, new HashSet<>(reduced.paths));
+ assertTrue(reduced.regexps.isEmpty());
+ }
+
+ @Test
+ public void shouldNeverMergeToTheRepositoryRoot() {
Set dependencies = children("/sites/mysite/contents", 80);
dependencies.addAll(children("/users/root/files", 80));
- String regexp = mergedInto(dependencies);
+ CollectedCacheDependencies.Reduced reduced = reduce(dependencies);
- assertTrue("/sites/mysite/contents/item-1".matches(regexp));
- assertTrue("/users/root/files/item-1".matches(regexp));
- assertTrue("a fragment merged to the root is flushed by anything",
- "/sites/other/contents/whatever".matches(regexp));
+ assertEquals(81, reduced.paths.size() + reduced.regexps.size());
+ for (String regexp : reduced.regexps) {
+ assertFalse(regexp + " covers another site", "/sites/other/contents/whatever".matches(regexp));
+ }
}
@Test
- public void shouldLetOneStrayDependencyWidenTheMerge() {
+ public void shouldDropModuleDependenciesAsTheCoreCacheDoes() {
Set dependencies = children("/sites/mysite/contents/posts", 150);
dependencies.add("/modules/mymodule/templates/one");
- String regexp = mergedInto(dependencies);
+ CollectedCacheDependencies.Reduced reduced = reduce(dependencies);
- // The stray path shares no first segment with the listing, so the merge reaches
- // the root
- assertTrue("/sites/other/contents/whatever".matches(regexp));
+ assertEquals(150, reduced.collected);
+ assertEquals(Set.of(regexp("/sites/mysite/contents/posts")), reduced.regexps);
+ assertTrue(reduce(Set.of("/modules/mymodule/templates/one")).paths.isEmpty());
}
@Test
public void shouldQuoteThePathSoADotIsNotAWildcard() {
- String regexp = mergedInto(children("/sites/mysite/files/my.docs", 150));
+ CollectedCacheDependencies.Reduced reduced = reduce(children("/sites/mysite/files/my.docs", 150));
+ String regexp = reduced.regexps.iterator().next();
assertTrue("/sites/mysite/files/my.docs/item-1".matches(regexp));
assertFalse("/sites/mysite/files/myXdocs/item-1".matches(regexp));
}
+
+ @Test
+ public void shouldCollectForTheFragmentBeingRendered() {
+ HttpServletRequest request = request();
+ Resource parent = resource("/sites/mysite/home");
+ Resource child = resource("/sites/mysite/home/area/child");
+
+ startRender(request, parent, true);
+ CollectedCacheDependencies.collect(request, "/sites/mysite/home/about");
+ // a nested fragment: the render chain pushes the child's collector, then restores the parent's
+ Object parentCollector = request.getAttribute(CollectedCacheDependencies.ATTRIBUTE);
+ startRender(request, child, true);
+ CollectedCacheDependencies.collect(request, "/sites/mysite/files/logo.png");
+ assertEquals(Set.of("/sites/mysite/files/logo.png"), CollectedCacheDependencies.collected(request, child));
+ request.setAttribute(CollectedCacheDependencies.ATTRIBUTE, parentCollector);
+ CollectedCacheDependencies.collect(request, "/sites/mysite/home/contact");
+
+ assertEquals(Set.of("/sites/mysite/home/about", "/sites/mysite/home/contact"),
+ CollectedCacheDependencies.collected(request, parent));
+ }
+
+ @Test
+ public void shouldTellTwoRendersOfOneNodeApart() {
+ HttpServletRequest request = request();
+ Resource first = resource("/sites/mysite/home/teaser");
+ Resource second = resource("/sites/mysite/home/teaser");
+
+ startRender(request, first, true);
+ CollectedCacheDependencies.collect(request, "/sites/mysite/home/about");
+
+ assertTrue(CollectedCacheDependencies.collected(request, second).isEmpty());
+ assertEquals(Set.of("/sites/mysite/home/about"), CollectedCacheDependencies.collected(request, first));
+ }
+
+ @Test
+ public void shouldCollectNothingOutsideAFragment() {
+ HttpServletRequest request = request();
+
+ CollectedCacheDependencies.collect(request, "/sites/mysite/home/about");
+
+ assertTrue(CollectedCacheDependencies.collected(request, resource("/sites/mysite/home")).isEmpty());
+ }
+
+ @Test
+ public void shouldCollectNothingForAViewThatOptedOut() {
+ HttpServletRequest request = request();
+ Resource resource = resource("/sites/mysite/home");
+
+ startRender(request, resource, false);
+ CollectedCacheDependencies.collect(request, "/sites/mysite/home/about");
+
+ assertTrue(CollectedCacheDependencies.collected(request, resource).isEmpty());
+ }
}
diff --git a/javascript-modules-engine/src/server/init-react.tsx b/javascript-modules-engine/src/server/init-react.tsx
index d3143805..4ec98129 100644
--- a/javascript-modules-engine/src/server/init-react.tsx
+++ b/javascript-modules-engine/src/server/init-react.tsx
@@ -28,11 +28,6 @@ server.registry.add("viewRenderer", "react", {
// But assumes a single language per page, so i18n.changeLanguage(lang) is safe in this context.
i18n.changeLanguage(language);
- const autocollectedDependencies =
- view.properties?.["cache.autocollectDependencies"] === "false"
- ? undefined
- : new Set();
-
// SSR
const currentNode = currentResource.getNode();
const mainNode = renderContext.getMainResource().getNode();
@@ -45,7 +40,6 @@ server.registry.add("viewRenderer", "react", {
mainNode={mainNode}
jcrSession={currentNode.getSession()}
bundleKey={bundleKey}
- autocollectedDependencies={autocollectedDependencies}
>
@@ -53,7 +47,7 @@ server.registry.add("viewRenderer", "react", {
);
- const html =
+ return (
// In page mode, prepend the rendered HTML with the HTML5 doctype
(currentResource.getContextConfiguration() === "page" ? "" : "") +
// We use a `` element to wrap raw HTML output because React does not allow
@@ -61,11 +55,7 @@ server.registry.add("viewRenderer", "react", {
// having them in the final output.
// `` SHOULD NOT be used in userland code, it is an internal implementation
// detail.
- ReactDOMServer.renderToString(element).replaceAll(/<\/?jsm-raw-html>/g, "");
-
- return {
- html,
- autocollectedDependencies: [...(autocollectedDependencies ?? [])],
- };
+ ReactDOMServer.renderToString(element).replaceAll(/<\/?jsm-raw-html>/g, "")
+ );
},
});
diff --git a/javascript-modules-library/src/hooks/useServerContext.tsx b/javascript-modules-library/src/hooks/useServerContext.tsx
index 0a43b3ea..bb60086a 100644
--- a/javascript-modules-library/src/hooks/useServerContext.tsx
+++ b/javascript-modules-library/src/hooks/useServerContext.tsx
@@ -25,13 +25,6 @@ export interface ServerContext {
jcrSession: JCRSessionWrapper;
/** The OSGi bundle key of the current module being rendered */
bundleKey: string;
-
- /**
- * Paths of all dependencies collected during render, unless autocollection is disabled.
- *
- * @internal Can change at any time without notice.
- */
- autocollectedDependencies?: Set;
}
const ServerContext = createContext({} as never);
diff --git a/javascript-modules-library/src/utils/urlBuilder/urlBuilder.ts b/javascript-modules-library/src/utils/urlBuilder/urlBuilder.ts
index 611ae199..491fa23c 100644
--- a/javascript-modules-library/src/utils/urlBuilder/urlBuilder.ts
+++ b/javascript-modules-library/src/utils/urlBuilder/urlBuilder.ts
@@ -83,14 +83,16 @@ export function buildNodeUrl(
context: {
renderContext?: RenderContext;
currentResource?: Resource;
- autocollectedDependencies?: Set;
} = useServerContext(),
): string {
if (!node) throw new Error("Expected a node in buildNodeUrl, received undefined");
- if (config.autocollectDependency !== false && context.autocollectedDependencies) {
+ if (config.autocollectDependency !== false && context.renderContext) {
// getCanonicalPath accounts for mounted and versioned nodes
- context.autocollectedDependencies.add(node.getCanonicalPath());
+ server.render.autocollectCacheDependency(
+ node.getCanonicalPath(),
+ context.renderContext.getRequest(),
+ );
}
// URL building is an old thing in Jahia, with a lot of branches and special cases:
diff --git a/tests/cypress/e2e/ui/cacheDependencyAutocollectTest.cy.ts b/tests/cypress/e2e/ui/cacheDependencyAutocollectTest.cy.ts
index e7a93531..14b98e9a 100644
--- a/tests/cypress/e2e/ui/cacheDependencyAutocollectTest.cy.ts
+++ b/tests/cypress/e2e/ui/cacheDependencyAutocollectTest.cy.ts
@@ -8,6 +8,10 @@ describe("Cache dependencies collected by the URL builder", () => {
const IMAGE = "autocollected.jpg";
const RENAMED = "autocollected-renamed.jpg";
const LIVE_PAGE = `/cms/render/live/en/sites/${GENERIC_SITE_KEY}/home/${PAGE}.html`;
+ /** The test ids of the views whose dependency is collected */
+ const COLLECTED = ["autocollected", "explicitcontext", "included", "virtual"];
+ /** The test ids of the views that opted out, for the whole view or for one call */
+ const OPTED_OUT = ["optedout", "callopout"];
/** Renders LIVE_PAGE as an anonymous visitor */
const renderLive = () =>
@@ -36,7 +40,7 @@ describe("Cache dependencies collected by the URL builder", () => {
],
});
- before("Create the page, the image and the two contents referencing it", () => {
+ before("Create the page, the image and the contents referencing it", () => {
cy.fixture("testData/image.jpg", "binary").then((content) => {
const blob = Cypress.Blob.binaryStringToBlob(content, "image/jpeg");
cy.apollo({
@@ -59,6 +63,26 @@ describe("Cache dependencies collected by the URL builder", () => {
["jmix:renderable"],
[{ name: "j:view", value: "noautocollect", type: "STRING" }],
);
+ addImageReference(
+ "explicitContext",
+ ["jmix:renderable"],
+ [{ name: "j:view", value: "explicitcontext", type: "STRING" }],
+ );
+ addImageReference(
+ "callOptOut",
+ ["jmix:renderable"],
+ [{ name: "j:view", value: "callopout", type: "STRING" }],
+ );
+ addImageReference(
+ "includeParent",
+ ["jmix:renderable"],
+ [{ name: "j:view", value: "includeparent", type: "STRING" }],
+ );
+ addImageReference(
+ "virtualParent",
+ ["jmix:renderable"],
+ [{ name: "j:view", value: "virtualparent", type: "STRING" }],
+ );
});
publishAndWaitJobEnding(`/sites/${GENERIC_SITE_KEY}`);
@@ -70,20 +94,20 @@ describe("Cache dependencies collected by the URL builder", () => {
publishAndWaitJobEnding(`/sites/${GENERIC_SITE_KEY}`);
});
- it("flushes the view that built the URL, and leaves the one that opted out", () => {
+ it("flushes the views that built the URL, and leaves the ones that opted out", () => {
renderLive().then((first) => {
- expect(imageSrc(first, "autocollected")).to.include(`/files/live${FILES}/${IMAGE}`);
- expect(imageSrc(first, "optedout")).to.include(`/files/live${FILES}/${IMAGE}`);
+ for (const view of [...COLLECTED, ...OPTED_OUT]) {
+ expect(imageSrc(first, view), view).to.include(`/files/live${FILES}/${IMAGE}`);
+ }
// Liveness: without cached fragments there is nothing for the rename to flush, and every
// assertion below would pass on a page that is simply rendered afresh every time.
renderLive().then((second) => {
- expect(renderedAt(second, "autocollected"), "the fragment must be cached").to.eq(
- renderedAt(first, "autocollected"),
- );
- expect(renderedAt(second, "optedout"), "the fragment must be cached").to.eq(
- renderedAt(first, "optedout"),
- );
+ for (const view of [...COLLECTED, ...OPTED_OUT]) {
+ expect(renderedAt(second, view), `the ${view} fragment must be cached`).to.eq(
+ renderedAt(first, view),
+ );
+ }
cy.apollo({
mutationFile: "graphql/jcrRenameNode.graphql",
@@ -102,16 +126,19 @@ describe("Cache dependencies collected by the URL builder", () => {
cy.request(`/files/live${FILES}/${RENAMED}`).its("status").should("eq", 200);
renderLive().then((third) => {
- expect(
- renderedAt(third, "autocollected"),
- "the collected view must be flushed",
- ).to.not.eq(renderedAt(first, "autocollected"));
- expect(imageSrc(third, "autocollected")).to.include(`/files/live${FILES}/${RENAMED}`);
+ for (const view of COLLECTED) {
+ expect(renderedAt(third, view), `the ${view} view must be flushed`).to.not.eq(
+ renderedAt(first, view),
+ );
+ expect(imageSrc(third, view), view).to.include(`/files/live${FILES}/${RENAMED}`);
+ }
- expect(renderedAt(third, "optedout"), "the opted-out view must be untouched").to.eq(
- renderedAt(first, "optedout"),
- );
- expect(imageSrc(third, "optedout")).to.include(`/files/live${FILES}/${IMAGE}`);
+ for (const view of OPTED_OUT) {
+ expect(renderedAt(third, view), `the ${view} view must be untouched`).to.eq(
+ renderedAt(first, view),
+ );
+ expect(imageSrc(third, view), view).to.include(`/files/live${FILES}/${IMAGE}`);
+ }
});
});
});