You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by kw...@apache.org on 2021/06/25 08:48:43 UTC
[jackrabbit-filevault] branch master updated: fix test after
adjusting default severity for orphaned filter
This is an automated email from the ASF dual-hosted git repository.
kwin 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 1e32476 fix test after adjusting default severity for orphaned filter
1e32476 is described below
commit 1e32476a8073197a2abfdfc4fce9fae9dd95cfc3
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Fri Jun 25 10:48:32 2021 +0200
fix test after adjusting default severity for orphaned filter
clean up option
---
src/site/markdown/validation.md | 2 +-
.../spi/impl/AdvancedFilterValidator.java | 29 ++++++++++-------
.../spi/impl/AdvancedFilterValidatorFactory.java | 29 ++++++++++-------
.../spi/impl/AdvancedFilterValidatorTest.java | 38 +++++++++++++---------
4 files changed, 59 insertions(+), 39 deletions(-)
diff --git a/src/site/markdown/validation.md b/src/site/markdown/validation.md
index 166635e..b8e1271 100644
--- a/src/site/markdown/validation.md
+++ b/src/site/markdown/validation.md
@@ -51,7 +51,7 @@ It is possible to run validation only on a subset of files contained in the pack
ID | Description | Options | Incremental Execution Limitations
--- | --- | --- | ---
-`jackrabbit-filter` | Checks for validity of the [filter.xml](./filter.html) (according to a predefined XML schema). In addition checks that every [docview xml node](./docview.html) is contained in the filter. It also makes sure that all filter root's ancestors are either known/valid roots or are contained in the package dependencies. For ancestor nodes which are not covered by a filter at least a `warn` is emitted. Also it makes sure that `pattern` values for includes/excludes as well [...]
+`jackrabbit-filter` | Checks for validity of the [filter.xml](./filter.html) (according to a predefined XML schema). In addition checks that every [docview xml node](./docview.html) is contained in the filter. It also makes sure that all filter root's ancestors are either known/valid roots or are contained in the package dependencies. For ancestor nodes which are not covered by a filter at least a `warn` is emitted. Also it makes sure that `pattern` values for includes/excludes as well [...]
`jackrabbit-properties ` | Checks for validity of the [properties.xml](./properties.html) | none | none
`jackrabbit-dependencies` | Checks for overlapping filter roots of the referenced package dependencies as well as for valid package dependency references (i.e. references which can be resolved). | *severityForUnresolvedDependencies*: severity of validation messages for unresolved dependencies (default = `warn`) | none
`jackrabbit-docviewparser` | Checks if all docview files in the package are compliant with the [(extended) Document View Format](docview.html). This involves checking for XML validity as well as checking for correct property types. | none | none
diff --git a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/AdvancedFilterValidator.java b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/AdvancedFilterValidator.java
index 1f099de..d479cc0 100644
--- a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/AdvancedFilterValidator.java
+++ b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/AdvancedFilterValidator.java
@@ -70,11 +70,11 @@ public final class AdvancedFilterValidator implements GenericMetaInfDataValidato
protected static final String MESSAGE_INVALID_PATTERN = "Invalid pattern given ('%s') which will never match for any descendants of the root path '%s'.";
protected static final String MESSAGE_ROOT_PATH_NOT_ABSOLUTE = "Root path must be absolute, but does not start with a '/': '%s'.";
protected static final String MESSAGE_INVALID_FILTER_XML = "Invalid filter.xml";
- protected static final String MESSAGE_FILTER_ROOT_ANCESTOR_COVERED_BUT_EXCLUDED = "Filter root's ancestor '%s' is covered by dependency '%s' but excluded by its patterns.";
- protected static final String MESSAGE_FILTER_ROOT_ANCESTOR_UNCOVERED = "Filter root's ancestor '%s' is not covered by any of the specified dependencies nor a valid root.";
+ protected static final String MESSAGE_FILTER_ROOT_ANCESTOR_COVERED_BUT_EXCLUDED = "Filter root's ancestor '%s' is defined by dependency '%s' but excluded by its patterns.";
+ protected static final String MESSAGE_FILTER_ROOT_ANCESTOR_UNDEFINED = "Filter root's ancestor '%s' is not covered by any of the specified dependencies nor a valid root.";
protected static final String MESSAGE_NODE_NOT_CONTAINED = "Node '%s' is not contained in any of the filter rules";
protected static final String MESSAGE_ANCESTOR_NODE_NOT_COVERED = "Ancestor node '%s' is not covered by any of the filter rules. Preferably depend on a package that provides this node or include it in the filter rules!";
- protected static final String MESSAGE_ANCESTOR_NODE_NOT_COVERED_BUT_VALID_ROOT = "Ancestor node '%s' is not covered by any of the filter rules but that node is a given root (either by a dependency or by the known roots). Remove that node!";
+ protected static final String MESSAGE_ANCESTOR_NODE_NOT_COVERED_BUT_VALID_ROOT = "Ancestor node '%s' is not covered by any of the filter rules but that node is a given root (either by a dependency or by the known roots). Remove the file(s) representing that node!";
protected static final String MESSAGE_NODE_BELOW_CLEANUP_FILTER = "Node '%s' is covered by a 'cleanup' filter rule. That filter type is only supposed to be used for removing nodes during import!";
static final Path FILTER_XML_PATH = Paths.get(Constants.VAULT_DIR, Constants.FILTER_XML);
@@ -84,7 +84,7 @@ public final class AdvancedFilterValidator implements GenericMetaInfDataValidato
private final Collection<String> validRoots;
private final @NotNull ValidationMessageSeverity defaultSeverity;
private final @NotNull ValidationMessageSeverity severityForUncoveredAncestorNode;
- private final @NotNull ValidationMessageSeverity severityForUncoveredFilterRootAncestors;
+ private final @NotNull ValidationMessageSeverity severityForUndefinedFilterRootAncestors;
private final @NotNull ValidationMessageSeverity severityForOrphanedFilterEntries;
private final Collection<PackageInfo> dependenciesMetaInfo;
private final WorkspaceFilter filter;
@@ -92,13 +92,13 @@ public final class AdvancedFilterValidator implements GenericMetaInfDataValidato
private final Collection<String> danglingNodePaths;
private final Map<PathFilterSet, List<Entry<PathFilter>>> orphanedFilterSets;
- public AdvancedFilterValidator(@NotNull DocumentBuilderFactory factory, @NotNull ValidationMessageSeverity defaultSeverity, @NotNull ValidationMessageSeverity severityForUncoveredAncestorNodes, @NotNull ValidationMessageSeverity severityForUncoveredFilterRootAncestors, @NotNull ValidationMessageSeverity severityForOrphanedFilterEntries, boolean isSubPackage, @NotNull Collection<PackageInfo> dependenciesMetaInfo, @NotNull WorkspaceFilter filter, @NotNull Collection<String> validRoots) {
+ public AdvancedFilterValidator(@NotNull DocumentBuilderFactory factory, @NotNull ValidationMessageSeverity defaultSeverity, @NotNull ValidationMessageSeverity severityForUncoveredAncestorNodes, @NotNull ValidationMessageSeverity severityForUndefinedFilterRootAncestors, @NotNull ValidationMessageSeverity severityForOrphanedFilterEntries, boolean isSubPackage, @NotNull Collection<PackageInfo> dependenciesMetaInfo, @NotNull WorkspaceFilter filter, @NotNull Collection<String> validRoots) {
this.factory = factory;
this.isSubPackage = isSubPackage;
this.filterValidators = new HashMap<>();
this.defaultSeverity = defaultSeverity;
this.severityForUncoveredAncestorNode = severityForUncoveredAncestorNodes;
- this.severityForUncoveredFilterRootAncestors = severityForUncoveredFilterRootAncestors;
+ this.severityForUndefinedFilterRootAncestors = severityForUndefinedFilterRootAncestors;
this.severityForOrphanedFilterEntries = severityForOrphanedFilterEntries;
this.dependenciesMetaInfo = dependenciesMetaInfo;
this.filter = filter;
@@ -200,11 +200,11 @@ public final class AdvancedFilterValidator implements GenericMetaInfDataValidato
if (!isContained) {
String msg;
if (coveringPackageId == null) {
- msg = String.format(MESSAGE_FILTER_ROOT_ANCESTOR_UNCOVERED, root);
+ msg = String.format(MESSAGE_FILTER_ROOT_ANCESTOR_UNDEFINED, root);
} else {
msg = String.format(MESSAGE_FILTER_ROOT_ANCESTOR_COVERED_BUT_EXCLUDED, root, coveringPackageId);
}
- messages.add(new ValidationMessage(severityForUncoveredFilterRootAncestors, msg));
+ messages.add(new ValidationMessage(severityForUndefinedFilterRootAncestors, msg));
}
}
return messages;
@@ -234,7 +234,12 @@ public final class AdvancedFilterValidator implements GenericMetaInfDataValidato
return messages;
}
- private Collection<ValidationMessage> validateFileNodePath(@NotNull String nodePath) {
+ /**
+ * Only called for node's which are not only defined by folders
+ * @param nodePath
+ * @return
+ */
+ private Collection<ValidationMessage> validateNodePath(@NotNull String nodePath) {
if (isSubPackage) {
return null; // not relevant for sub packages
}
@@ -273,7 +278,7 @@ public final class AdvancedFilterValidator implements GenericMetaInfDataValidato
String danglingNodePath = getDanglingAncestorNodePath(nodePath, filter);
if (danglingNodePath != null) {
return Collections.singleton(
- new ValidationMessage(defaultSeverity, "Ancestor node (" + danglingNodePath + ") of Node '" + nodePath +"' which is contained in a filter include element is not included!"));
+ new ValidationMessage(severityForUncoveredAncestorNode, String.format(MESSAGE_ANCESTOR_NODE_NOT_COVERED, danglingNodePath)));
}
return null;
}
@@ -282,7 +287,7 @@ public final class AdvancedFilterValidator implements GenericMetaInfDataValidato
public @Nullable Collection<ValidationMessage> validateJcrPath(@NotNull NodeContext nodeContext,
boolean isFolder, boolean isDocViewXml) {
if (!isFolder) {
- return validateFileNodePath(nodeContext.getNodePath());
+ return validateNodePath(nodeContext.getNodePath());
} else {
return null;
}
@@ -294,7 +299,7 @@ public final class AdvancedFilterValidator implements GenericMetaInfDataValidato
// skip root node, as it has been processed with validateJcrPath(...) and empty nodes only used for ordering
if (!isRoot && node.props.size() > 0) {
// root has been validated already with validateJcrPath(...)
- return validateFileNodePath(nodeContext.getNodePath());
+ return validateNodePath(nodeContext.getNodePath());
}
return null;
}
diff --git a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/AdvancedFilterValidatorFactory.java b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/AdvancedFilterValidatorFactory.java
index b7f48c9..170c012 100644
--- a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/AdvancedFilterValidatorFactory.java
+++ b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/AdvancedFilterValidatorFactory.java
@@ -50,14 +50,16 @@ public final class AdvancedFilterValidatorFactory implements ValidatorFactory {
public static final String ID = ID_PREFIX_JACKRABBIT + "filter";
public static final String OPTION_SEVERITY_FOR_UNCOVERED_ANCESTOR_NODES = "severityForUncoveredAncestorNodes";
- private static final Object OPTION_SEVERITY_FOR_UNCOVERED_FILTER_ROOT_ANCESTORS = "severityForUncoveredFilterRootAncestors";
+ @Deprecated
+ private static final Object OPTION_SEVERITY_FOR_UNCOVERED_FILTER_ROOT_ANCESTORS = "severityForUncoveredFilterRootAncestors"; // TODO: confusing naming
+ private static final Object OPTION_SEVERITY_FOR_UNDEFINED_FILTER_ROOT_ANCESTORS = "severityForUndefinedFilterRootAncestors";
public static final String OPTION_SEVERITY_FOR_ORPHANED_FILTER_RULES = "severityForOrphanedFilterRules";
// should take comma-separated list of valid root paths
public static final String OPTION_VALID_ROOTS = "validRoots";
static final ValidationMessageSeverity DEFAULT_SEVERITY_FOR_UNCOVERED_ANCESTOR_NODES = ValidationMessageSeverity.WARN;
- private static final ValidationMessageSeverity DEFAULT_SEVERITY_FOR_UNCOVERED_FILTER_ROOT_ANCESTORS = ValidationMessageSeverity.WARN;
- static final ValidationMessageSeverity DEFAULT_SEVERITY_FOR_ORPHANED_FILTER_RULES = ValidationMessageSeverity.INFO;
+ static final ValidationMessageSeverity DEFAULT_SEVERITY_FOR_UNDEFINED_FILTER_ROOT_ANCESTORS = ValidationMessageSeverity.WARN;
+ static final ValidationMessageSeverity DEFAULT_SEVERITY_FOR_ORPHANED_FILTER_RULES = ValidationMessageSeverity.WARN;
static final Collection<String> DEFAULT_VALID_ROOTS = new LinkedList<>(Arrays.asList("/","/libs","/apps","/etc","/var","/tmp","/content","/etc/packages"));
@NotNull private final DocumentBuilderFactory factory;
@@ -104,16 +106,21 @@ public final class AdvancedFilterValidatorFactory implements ValidatorFactory {
severityForUncoveredAncestorNode = DEFAULT_SEVERITY_FOR_UNCOVERED_ANCESTOR_NODES;
}
// severity for ancestor of filter rules
- final ValidationMessageSeverity severityForUncoveredFilterRootAncestors;
- if (settings.getOptions().containsKey(OPTION_SEVERITY_FOR_UNCOVERED_FILTER_ROOT_ANCESTORS)) {
- String optionValue = settings.getOptions().get(OPTION_SEVERITY_FOR_UNCOVERED_FILTER_ROOT_ANCESTORS);
- severityForUncoveredFilterRootAncestors = ValidationMessageSeverity.valueOf(optionValue.toUpperCase());
+ final ValidationMessageSeverity severityForUndefinedFilterRootAncestors;
+ if (settings.getOptions().containsKey(OPTION_SEVERITY_FOR_UNDEFINED_FILTER_ROOT_ANCESTORS)) {
+ String optionValue = settings.getOptions().get(OPTION_SEVERITY_FOR_UNDEFINED_FILTER_ROOT_ANCESTORS);
+ severityForUndefinedFilterRootAncestors = ValidationMessageSeverity.valueOf(optionValue.toUpperCase());
} else {
+ // application packages must define every ancestor via package dependencies according to https://issues.apache.org/jira/browse/JCRVLT-170
if (PackageType.APPLICATION.equals(context.getProperties().getPackageType())) {
- log.debug("Due to package type 'application' emit error for every uncovered filter root ancestor");
- severityForUncoveredFilterRootAncestors = ValidationMessageSeverity.ERROR;
+ log.info("Due to package type 'application' emit error for every undefined filter root ancestor");
+ severityForUndefinedFilterRootAncestors = ValidationMessageSeverity.ERROR;
+ } else if(settings.getOptions().containsKey(OPTION_SEVERITY_FOR_UNCOVERED_FILTER_ROOT_ANCESTORS)) {
+ log.warn("Using deprecated option " + OPTION_SEVERITY_FOR_UNCOVERED_FILTER_ROOT_ANCESTORS + ". Please switch to " + OPTION_SEVERITY_FOR_UNDEFINED_FILTER_ROOT_ANCESTORS + " instead!");
+ String optionValue = settings.getOptions().get(OPTION_SEVERITY_FOR_UNCOVERED_FILTER_ROOT_ANCESTORS);
+ severityForUndefinedFilterRootAncestors = ValidationMessageSeverity.valueOf(optionValue.toUpperCase());
} else {
- severityForUncoveredFilterRootAncestors = DEFAULT_SEVERITY_FOR_UNCOVERED_FILTER_ROOT_ANCESTORS;
+ severityForUndefinedFilterRootAncestors = DEFAULT_SEVERITY_FOR_UNDEFINED_FILTER_ROOT_ANCESTORS;
}
}
@@ -137,7 +144,7 @@ public final class AdvancedFilterValidatorFactory implements ValidatorFactory {
} else {
validRoots.addAll(DEFAULT_VALID_ROOTS);
}
- return new AdvancedFilterValidator(factory, settings.getDefaultSeverity(), severityForUncoveredAncestorNode, severityForUncoveredFilterRootAncestors, severityForOrphanedFilterRules, context.getContainerValidationContext() != null, context.getDependenciesPackageInfo(), context.getFilter(), validRoots);
+ return new AdvancedFilterValidator(factory, settings.getDefaultSeverity(), severityForUncoveredAncestorNode, severityForUndefinedFilterRootAncestors, severityForOrphanedFilterRules, context.getContainerValidationContext() != null, context.getDependenciesPackageInfo(), context.getFilter(), validRoots);
}
@Override
diff --git a/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/spi/impl/AdvancedFilterValidatorTest.java b/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/spi/impl/AdvancedFilterValidatorTest.java
index f0ed13b..b237f65 100644
--- a/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/spi/impl/AdvancedFilterValidatorTest.java
+++ b/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/spi/impl/AdvancedFilterValidatorTest.java
@@ -220,28 +220,28 @@ public class AdvancedFilterValidatorTest {
try (InputStream input = this.getClass().getResourceAsStream("/filter.xml")) {
filter.load(input);
}
+
+ // default severity WARN
validator = new AdvancedFilterValidator(
factory,
ValidationMessageSeverity.ERROR,
- AdvancedFilterValidatorFactory.DEFAULT_SEVERITY_FOR_UNCOVERED_ANCESTOR_NODES,
+ AdvancedFilterValidatorFactory.DEFAULT_SEVERITY_FOR_UNDEFINED_FILTER_ROOT_ANCESTORS,
ValidationMessageSeverity.ERROR,
ValidationMessageSeverity.ERROR,
false,
dependenciesMetaInfo,
filter, // this is per test
validRoots);
-
- // default severity INFO
ValidationExecutorTest.assertViolation(validator.validateJcrPath(getStandardNodeContext("/apps"), false, false),
ValidationMessageSeverity.INFO,
- new ValidationMessage(ValidationMessageSeverity.INFO,
+ new ValidationMessage(ValidationMessageSeverity.WARN,
String.format(AdvancedFilterValidator.MESSAGE_ANCESTOR_NODE_NOT_COVERED_BUT_VALID_ROOT, "/apps")));
ValidationExecutorTest.assertViolation(validator.validateJcrPath(getStandardNodeContext("/apps/test4"), false, false),
ValidationMessageSeverity.INFO,
- new ValidationMessage(ValidationMessageSeverity.INFO,
+ new ValidationMessage(ValidationMessageSeverity.WARN,
String.format(AdvancedFilterValidator.MESSAGE_ANCESTOR_NODE_NOT_COVERED, "/apps/test4")));
- // default severity ERROR
+ // severity ERROR
validator = new AdvancedFilterValidator(
factory,
ValidationMessageSeverity.ERROR,
@@ -304,12 +304,12 @@ public class AdvancedFilterValidatorTest {
Collection<ValidationMessage> messages = validator.validate(filter);
ValidationExecutorTest.assertViolation(messages, ValidationMessageSeverity.INFO,
new ValidationMessage(ValidationMessageSeverity.ERROR,
- String.format(AdvancedFilterValidator.MESSAGE_FILTER_ROOT_ANCESTOR_UNCOVERED, "/apps/uncovered")),
+ String.format(AdvancedFilterValidator.MESSAGE_FILTER_ROOT_ANCESTOR_UNDEFINED, "/apps/uncovered")),
new ValidationMessage(ValidationMessageSeverity.ERROR,
String.format(AdvancedFilterValidator.MESSAGE_FILTER_ROOT_ANCESTOR_COVERED_BUT_EXCLUDED, "/apps/covered2/excluded",
"group:dependency1")),
new ValidationMessage(ValidationMessageSeverity.ERROR,
- String.format(AdvancedFilterValidator.MESSAGE_FILTER_ROOT_ANCESTOR_UNCOVERED, "/invalidroot")));
+ String.format(AdvancedFilterValidator.MESSAGE_FILTER_ROOT_ANCESTOR_UNDEFINED, "/invalidroot")));
}
@Test
@@ -320,25 +320,33 @@ public class AdvancedFilterValidatorTest {
}
validator = new AdvancedFilterValidator(
factory,
- ValidationMessageSeverity.INFO,
- AdvancedFilterValidatorFactory.DEFAULT_SEVERITY_FOR_UNCOVERED_ANCESTOR_NODES,
ValidationMessageSeverity.ERROR,
ValidationMessageSeverity.ERROR,
+ ValidationMessageSeverity.INFO,
+ AdvancedFilterValidatorFactory.DEFAULT_SEVERITY_FOR_ORPHANED_FILTER_RULES,
false,
dependenciesMetaInfo,
filter, // this is per test
validRoots);
+
Collection<ValidationMessage> messages = validator.validate(filter);
+ ValidationExecutorTest.assertViolation(messages, ValidationMessageSeverity.INFO,
+ new ValidationMessage(ValidationMessageSeverity.INFO,
+ String.format(AdvancedFilterValidator.MESSAGE_FILTER_ROOT_ANCESTOR_UNDEFINED, "/apps/test4")));
messages = validator.validateJcrPath(getStandardNodeContext("/apps/test3"), false, false);
ValidationExecutorTest.assertViolation(messages, ValidationMessageSeverity.INFO,
- new ValidationMessage(ValidationMessageSeverity.INFO,
+ new ValidationMessage(ValidationMessageSeverity.ERROR,
String.format(AdvancedFilterValidator.MESSAGE_NODE_BELOW_CLEANUP_FILTER, "/apps/test3")));
- MatcherAssert.assertThat(validator.validateJcrPath(getStandardNodeContext("/apps/test2/something/anothervalid"), false, false),
- AnyValidationMessageMatcher.noValidationInCollection());
- messages = validator.done();
+
+ messages = validator.validateJcrPath(getStandardNodeContext("/apps/test2/something/anothervalid"), false, false);
ValidationExecutorTest.assertViolation(messages, ValidationMessageSeverity.INFO,
new ValidationMessage(ValidationMessageSeverity.ERROR,
+ String.format(AdvancedFilterValidator.MESSAGE_ANCESTOR_NODE_NOT_COVERED, "/apps/test2/something")));
+
+ messages = validator.done();
+ ValidationExecutorTest.assertViolation(messages, ValidationMessageSeverity.INFO,
+ new ValidationMessage(ValidationMessageSeverity.WARN,
String.format(AdvancedFilterValidator.MESSAGE_ORPHANED_FILTER_ENTRIES,
"entry with root '/apps/test', includes [regex: .*/valid] below root '/apps/test2', entry with root '/apps/test4/test'")));
}
@@ -362,7 +370,7 @@ public class AdvancedFilterValidatorTest {
Collection<ValidationMessage> messages = validator.validate(filter);
ValidationExecutorTest.assertViolation(messages, ValidationMessageSeverity.INFO,
new ValidationMessage(ValidationMessageSeverity.INFO,
- String.format(AdvancedFilterValidator.MESSAGE_FILTER_ROOT_ANCESTOR_UNCOVERED, "/apps/test4")));
+ String.format(AdvancedFilterValidator.MESSAGE_FILTER_ROOT_ANCESTOR_UNDEFINED, "/apps/test4")));
}
@Test