You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by re...@apache.org on 2022/11/26 13:00:47 UTC
[jackrabbit-filevault] branch master updated: JCRVLT-666: improve performance of DefaultWorkspaceFilter.dumpCoverage (Session variant) (#257)
This is an automated email from the ASF dual-hosted git repository.
reschke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git
The following commit(s) were added to refs/heads/master by this push:
new 3e610b8a JCRVLT-666: improve performance of DefaultWorkspaceFilter.dumpCoverage (Session variant) (#257)
3e610b8a is described below
commit 3e610b8aa9fdad7f9d6919acc960106af3c7cc93
Author: Julian Reschke <ju...@gmx.de>
AuthorDate: Sat Nov 26 14:00:42 2022 +0100
JCRVLT-666: improve performance of DefaultWorkspaceFilter.dumpCoverage (Session variant) (#257)
* JCRVLT-666: improve performance of DefaultWorkspaceFilter.dumpCoverage (Session variant)
* JCRVLT-666: add some trace-level logging for the new code
* JCRVLT-666: improve test coverage
* JCRVLT-666: fall back to root when filter root is not accessible, add more test coverage
* JCRVLT-666: adjust API doc that gave away implementation details
* JCRVLT-666: adjust API doc that gave away implementation details
* JCRVLT-666: just skip filter roots that we can not access
* JCRVLT-666: adjust method name
* JCRVLT-666: remove unused var
* JCRVLT-666: code cleanup
* JCRVLT-666: clarify Filter API re listeners
---
.../jackrabbit/vault/fs/api/WorkspaceFilter.java | 7 +-
.../vault/fs/config/DefaultWorkspaceFilter.java | 65 ++++++++++------
.../packaging/integration/DumpCoverageIT.java | 89 ++++++++++++++--------
3 files changed, 102 insertions(+), 59 deletions(-)
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/WorkspaceFilter.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/WorkspaceFilter.java
index b0880cea..2fbfdeef 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/WorkspaceFilter.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/WorkspaceFilter.java
@@ -139,19 +139,18 @@ public interface WorkspaceFilter extends Dumpable {
/**
* Dumps the coverage of this filter against the given node to the listener.
* @param rootNode root node
- * @param listener listener
+ * @param listener listener which receives coverage information
* @throws RepositoryException if an error occurs
*/
void dumpCoverage(@NotNull Node rootNode, @NotNull ProgressTrackerListener listener)
throws RepositoryException;
/**
- * Dumps the coverage of this filter using the given session. The traversal starts
- * at the common ancestor of all filter sets. If {@code skipJcrContent} is {@code true}
+ * Dumps the coverage of this filter using the given session. If {@code skipJcrContent} is {@code true}
* the jcr:content nodes are excluded from traversal and reporting.
*
* @param session session
- * @param listener listener to report progress
+ * @param listener listener which receives coverage information
* @param skipJcrContent {@code true} to skip jcr:content nodes
* @throws RepositoryException if an error occurs
*/
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java
index c209fdb2..f0122147 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/config/DefaultWorkspaceFilter.java
@@ -26,12 +26,14 @@ import java.io.InputStream;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.io.UnsupportedEncodingException;
+import java.util.ArrayList;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
+import java.util.Set;
+import java.util.TreeSet;
import javax.jcr.NodeIterator;
-import javax.jcr.PathNotFoundException;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
import javax.xml.parsers.DocumentBuilder;
@@ -53,7 +55,6 @@ import org.apache.jackrabbit.vault.fs.api.WorkspaceFilter;
import org.apache.jackrabbit.vault.fs.filter.DefaultPathFilter;
import org.apache.jackrabbit.vault.fs.spi.ProgressTracker;
import org.apache.jackrabbit.vault.util.RejectingEntityResolver;
-import org.apache.jackrabbit.vault.util.Tree;
import org.apache.jackrabbit.vault.util.xml.serialize.FormattingXmlStreamWriter;
import org.apache.jackrabbit.vault.util.xml.serialize.OutputFormat;
import org.slf4j.Logger;
@@ -580,27 +581,47 @@ public class DefaultWorkspaceFilter implements Dumpable, WorkspaceFilter {
/**
* {@inheritDoc}
*/
- public void dumpCoverage(Session session, ProgressTrackerListener listener, boolean skipJcrContent)
- throws RepositoryException {
+ public void dumpCoverage(Session session, ProgressTrackerListener listener, boolean skipJcrContent) throws RepositoryException {
ProgressTracker tracker = new ProgressTracker(listener);
- // get common ancestor
- Tree<PathFilterSet> tree = new Tree<>();
- for (PathFilterSet set: nodesFilterSets) {
- tree.put(set.getRoot(), set);
- }
- String rootPath = tree.getRootPath();
- javax.jcr.Node rootNode;
- if (session.nodeExists(rootPath)) {
- rootNode = session.getNode(rootPath);
- } else if (session.nodeExists("/")) {
- log.warn("Common ancestor {} not found. Using root node", rootPath);
- rootNode = session.getRootNode();
- rootPath = "/";
- } else {
- throw new PathNotFoundException("Common ancestor " + rootPath+ " not found.");
- }
- log.debug("Starting coverage dump at {} (skipJcrContent={})", rootPath, skipJcrContent);
- dumpCoverage(rootNode, tracker, skipJcrContent);
+
+ for (String path : getEntryNodesRelevantForCoverage()) {
+ if (session.nodeExists(path)) {
+ dumpCoverage(session.getNode(path), tracker, skipJcrContent);
+ } else {
+ log.warn("Node {} not found (and thus will not be dumped)", path);
+ }
+ }
+ }
+
+ /**
+ * @return list of nodes to descend from for dumping
+ */
+ private List<String> getEntryNodesRelevantForCoverage() {
+
+ // compute unique set of paths
+ Set<String> uniquePaths = new TreeSet<>();
+ for (PathFilterSet set : nodesFilterSets) {
+ String path = set.getRoot();
+ uniquePaths.add(path.endsWith("/") ? path : path + "/");
+ }
+ if (log.isTraceEnabled()) {
+ log.trace("Unique paths to cover: {}", uniquePaths);
+ }
+
+ // exclude descendants
+ List<String> pathsToTraverse = new ArrayList<>();
+ String last = null;
+ for (String path : uniquePaths) {
+ if (last == null || !Text.isDescendant(last, path)) {
+ pathsToTraverse.add(path);
+ last = path;
+ }
+ }
+ if (log.isTraceEnabled()) {
+ log.trace("Unique paths to cover with descendants removed: {}", pathsToTraverse);
+ }
+
+ return pathsToTraverse;
}
private void dumpCoverage(javax.jcr.Node node, ProgressTracker tracker, boolean skipJcrContent)
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/DumpCoverageIT.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/DumpCoverageIT.java
index 4f00550b..96be76dd 100644
--- a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/DumpCoverageIT.java
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/DumpCoverageIT.java
@@ -20,6 +20,7 @@ package org.apache.jackrabbit.vault.packaging.integration;
import static org.junit.Assert.assertEquals;
import java.io.IOException;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedList;
@@ -28,50 +29,47 @@ import java.util.List;
import javax.jcr.RepositoryException;
import org.apache.jackrabbit.commons.JcrUtils;
+import org.apache.jackrabbit.util.Text;
import org.apache.jackrabbit.vault.fs.api.PathFilterSet;
import org.apache.jackrabbit.vault.fs.api.ProgressTrackerListener;
import org.apache.jackrabbit.vault.fs.config.ConfigurationException;
import org.apache.jackrabbit.vault.fs.config.DefaultWorkspaceFilter;
-import org.apache.jackrabbit.util.Text;
import org.junit.Before;
import org.junit.Test;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
- * {@code ImportTests}...
+ * Tests of coverage dump functionality.
*/
public class DumpCoverageIT extends IntegrationTestBase {
- /**
- * default logger
- */
- private static final Logger log = LoggerFactory.getLogger(DumpCoverageIT.class);
-
public static final String TEST_ROOT = "/testroot";
- public static final String[] ALL_PAGES = {
- TEST_ROOT + "/content",
- TEST_ROOT + "/content/en",
- TEST_ROOT + "/content/en/foo",
- TEST_ROOT + "/content/en/bar",
- TEST_ROOT + "/content/fr",
- TEST_ROOT + "/content/fr/foo"
- };
- public static final String[] LANGUAGE_PAGES = {
- TEST_ROOT + "/content/en",
- TEST_ROOT + "/content/en/foo",
- TEST_ROOT + "/content/en/bar",
- TEST_ROOT + "/content/fr",
- TEST_ROOT + "/content/fr/foo"
- };
- public static String[] ALL_PATHS;
+ public static final List<String> ENGLISH_PAGES = Arrays.asList(TEST_ROOT + "/content/en", TEST_ROOT + "/content/en/foo",
+ TEST_ROOT + "/content/en/bar");
+
+ public static final List<String> FRENCH_PAGES = Arrays.asList(TEST_ROOT + "/content/fr", TEST_ROOT + "/content/fr/foo");
+
+ public static final List<String> LANGUAGE_PAGES;
+ static {
+ LANGUAGE_PAGES = new ArrayList<>();
+ LANGUAGE_PAGES.addAll(ENGLISH_PAGES);
+ LANGUAGE_PAGES.addAll(FRENCH_PAGES);
+ }
+
+ public static final List<String> ALL_PAGES;
+ static {
+ ALL_PAGES = new ArrayList<>();
+ ALL_PAGES.add(TEST_ROOT + "/content");
+ ALL_PAGES.addAll(LANGUAGE_PAGES);
+ }
+
+ public static List<String> ALL_PATHS;
static {
- ALL_PATHS = new String[ALL_PAGES.length*2];
- for (int i=0; i<ALL_PAGES.length;i++) {
- ALL_PATHS[i*2] = ALL_PAGES[i];
- ALL_PATHS[i*2+1] = ALL_PAGES[i] + "/jcr:content";
+ ALL_PATHS = new ArrayList<>();
+ for (String page : ALL_PAGES) {
+ ALL_PATHS.add(page);
+ ALL_PATHS.add(page + "/jcr:content");
}
}
@@ -119,6 +117,31 @@ public class DumpCoverageIT extends IntegrationTestBase {
checkResults("Split roots", LANGUAGE_PAGES, listener.paths);
}
+ @Test
+ public void testNestedRootsCoverage() throws IOException, RepositoryException, ConfigurationException {
+ DefaultWorkspaceFilter filter = new DefaultWorkspaceFilter();
+ PathFilterSet set1 = new PathFilterSet(TEST_ROOT + "/content/en/foo");
+ PathFilterSet set2 = new PathFilterSet(TEST_ROOT + "/content/fr");
+ PathFilterSet set3 = new PathFilterSet(TEST_ROOT + "/content/en");
+ filter.add(set1);
+ filter.add(set2);
+ filter.add(set3);
+ Collector listener = new Collector();
+ filter.dumpCoverage(admin, listener, true);
+ checkResults("nested roots", LANGUAGE_PAGES, listener.paths);
+ }
+
+ @Test
+ public void testMissingRootsCoverage() throws IOException, RepositoryException, ConfigurationException {
+ DefaultWorkspaceFilter filter = new DefaultWorkspaceFilter();
+ PathFilterSet set1 = new PathFilterSet(TEST_ROOT + "/content/f");
+ PathFilterSet set2 = new PathFilterSet(TEST_ROOT + "/content/en");
+ filter.add(set1);
+ filter.add(set2);
+ Collector listener = new Collector();
+ filter.dumpCoverage(admin, listener, true);
+ checkResults("missing roots", ENGLISH_PAGES, listener.paths);
+ }
private static class Collector implements ProgressTrackerListener {
private final List<String> paths = new LinkedList<String>();
@@ -131,11 +154,11 @@ public class DumpCoverageIT extends IntegrationTestBase {
}
}
- public static void checkResults(String msg, String[] expected, List<String> result) {
- Arrays.sort(expected);
+ public static void checkResults(String msg, List<String> expected, List<String> result) {
+ Collections.sort(expected);
Collections.sort(result);
- String left = Text.implode(expected, "\n");
- String right = Text.implode(result.toArray(new String[result.size()]), "\n");
+ String left = Text.implode(expected.toArray(new String[0]), "\n");
+ String right = Text.implode(result.toArray(new String[0]), "\n");
assertEquals(msg, left, right);
}
}
\ No newline at end of file