SLING-10915 - Refactor CheckContentPackagesForPaths.Rules (#34)
diff --git a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPaths.java b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPaths.java
index 641772b..230bb5b 100644
--- a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPaths.java
+++ b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPaths.java
@@ -16,6 +16,9 @@
*/
package org.apache.sling.feature.analyser.task.impl;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
import org.apache.sling.feature.analyser.task.AnalyserTask;
import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
import org.apache.sling.feature.scanner.ContentPackageDescriptor;
@@ -42,50 +45,58 @@
@Override
public void execute(final AnalyserTaskContext ctx)
throws Exception {
- final Rules rules = getRules(ctx);
-
- if (rules != null ) {
- for (final ContentPackageDescriptor d : ctx.getFeatureDescriptor().getDescriptors(ContentPackageDescriptor.class)) {
- checkPackage(ctx, d, rules);
- }
- } else {
+ final Rules rules = new Rules(ctx);
+ if (!rules.isConfigured()) {
ctx.reportError("Configuration for task " + getId() + " is missing.");
+ return;
}
- }
-
- static final class Rules {
- public String[] includes;
- public String[] excludes;
- }
-
- Rules getRules(final AnalyserTaskContext ctx) {
- final String inc = ctx.getConfiguration().get(PROP_INCLUDES);
- final String exc = ctx.getConfiguration().get(PROP_EXCLUDES);
-
- if ( inc != null || exc != null ) {
- final Rules r = new Rules();
- r.includes = inc == null ? null : inc.split(",");
- clean(r.includes);
- r.excludes = exc == null ? null : exc.split(",");
- clean(r.excludes);
- return r;
- }
- return null;
- }
- private static void clean(final String[] array) {
- if ( array != null ) {
- for(int i=0;i<array.length;i++) {
- array[i] = array[i].trim();
- }
+
+ for (final ContentPackageDescriptor d : ctx.getFeatureDescriptor().getDescriptors(ContentPackageDescriptor.class)) {
+ checkPackage(ctx, d, rules);
}
}
void checkPackage(final AnalyserTaskContext ctx, final ContentPackageDescriptor desc, final Rules rules) {
- for(final String path : desc.getContentPaths()) {
- boolean isAllowed = rules.includes == null;
+ desc.getContentPaths().stream()
+ .filter(rules::isDisAllowed)
+ .forEach(path -> ctx.reportArtifactError(desc.getArtifact().getId(), "Content not allowed: " + path));
+ }
+
+ static final class Rules {
+ final String[] includes;
+ final String[] excludes;
+
+ Rules(final AnalyserTaskContext ctx) {
+ final String inc = ctx.getConfiguration().get(PROP_INCLUDES);
+ final String exc = ctx.getConfiguration().get(PROP_EXCLUDES);
+ includes = splitAndTrim(inc);
+ excludes = splitAndTrim(exc);
+ }
+
+ private String[] splitAndTrim(String property) {
+ if (property == null) {
+ return new String[] {};
+ }
+
+ return Stream.of(property.split(","))
+ .map(String::trim)
+ .collect(Collectors.toList())
+ .toArray(new String[] {});
+ }
+
+ boolean isConfigured() {
+ return includes.length > 0 || excludes.length > 0;
+ }
+
+ boolean isDisAllowed(String path) {
+ return !isAllowed(path);
+ }
+
+ boolean isAllowed(String path) {
+ boolean isAllowed = includes.length == 0;
int matchLength = 0;
if ( !isAllowed ) {
- for(final String i : rules.includes) {
+ for(final String i : includes) {
if ( path.equals(i) || path.startsWith(i.concat("/")) ) {
isAllowed = true;
matchLength = i.length();
@@ -93,17 +104,16 @@
}
}
}
- if ( isAllowed && rules.excludes != null ) {
- for(final String i : rules.excludes) {
+ if ( isAllowed && excludes.length > 0 ) {
+ for(final String i : excludes) {
if ( path.equals(i) || path.startsWith(i.concat("/")) && i.length() > matchLength ) {
isAllowed = false;
break;
}
}
}
- if ( !isAllowed ) {
- ctx.reportArtifactError(desc.getArtifact().getId(), "Content not allowed: ".concat(path));
- }
+ return isAllowed;
}
}
+
}
diff --git a/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPathsTest.java b/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPathsTest.java
index a241f96..2866566 100644
--- a/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPathsTest.java
+++ b/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckContentPackagesForPathsTest.java
@@ -17,7 +17,6 @@
package org.apache.sling.feature.analyser.task.impl;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import java.io.IOException;
@@ -26,51 +25,19 @@
import org.apache.sling.feature.Artifact;
import org.apache.sling.feature.ArtifactId;
-import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
import org.apache.sling.feature.analyser.task.impl.CheckContentPackagesForPaths.Rules;
import org.apache.sling.feature.scanner.impl.ContentPackageDescriptorImpl;
import org.junit.Test;
public class CheckContentPackagesForPathsTest {
- @Test public void testNoRulesConfiguration() {
- final CheckContentPackagesForPaths analyser = new CheckContentPackagesForPaths();
- final AnalyserTaskContext ctx = new AnalyserTaskContextImpl();
-
- assertNull(analyser.getRules(ctx));
- }
-
- @Test public void testIncludesRulesConfiguration() {
- final CheckContentPackagesForPaths analyser = new CheckContentPackagesForPaths();
- final AnalyserTaskContext ctx = new AnalyserTaskContextImpl();
- ctx.getConfiguration().put("includes", "/a, /b");
-
- final Rules r = analyser.getRules(ctx);
- assertNull(r.excludes);
- assertEquals(2, r.includes.length);
- assertEquals("/a", r.includes[0]);
- assertEquals("/b", r.includes[1]);
- }
-
- @Test public void testExcludesRulesConfiguration() {
- final CheckContentPackagesForPaths analyser = new CheckContentPackagesForPaths();
- final AnalyserTaskContext ctx = new AnalyserTaskContextImpl();
- ctx.getConfiguration().put("excludes", "/a, /b");
-
- final Rules r = analyser.getRules(ctx);
- assertNull(r.includes);
- assertEquals(2, r.excludes.length);
- assertEquals("/a", r.excludes[0]);
- assertEquals("/b", r.excludes[1]);
- }
-
@Test public void testPathsCheck() throws IOException {
final CheckContentPackagesForPaths analyser = new CheckContentPackagesForPaths();
final AnalyserTaskContextImpl ctx = new AnalyserTaskContextImpl();
ctx.getConfiguration().put("excludes", "/a");
ctx.getConfiguration().put("includes", "/ab,/b");
- final Rules r = analyser.getRules(ctx);
+ final Rules r = new Rules(ctx);
final ContentPackageDescriptorImpl desc = new ContentPackageDescriptorImpl("name", new Artifact(ArtifactId.parse("g:a:1")),
new URL("https://sling.apache.org"), null, null, null, null, new Properties());
desc.getContentPaths().add("/b/foo");
@@ -94,7 +61,7 @@
ctx.getConfiguration().put("excludes", "/a");
ctx.getConfiguration().put("includes", "/a/foo");
- final Rules r = analyser.getRules(ctx);
+ final Rules r = new Rules(ctx);
final ContentPackageDescriptorImpl desc = new ContentPackageDescriptorImpl("name", new Artifact(ArtifactId.parse("g:a:1")),
new URL("https://sling.apache.org"), null, null, null, null, new Properties());
desc.getContentPaths().add("/a/foo");
diff --git a/src/test/java/org/apache/sling/feature/analyser/task/impl/RulesTest.java b/src/test/java/org/apache/sling/feature/analyser/task/impl/RulesTest.java
new file mode 100644
index 0000000..a795a23
--- /dev/null
+++ b/src/test/java/org/apache/sling/feature/analyser/task/impl/RulesTest.java
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.sling.feature.analyser.task.impl;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
+import org.apache.sling.feature.analyser.task.impl.CheckContentPackagesForPaths.Rules;
+import org.junit.Test;
+
+public class RulesTest {
+ @Test
+ public void testNoRulesConfiguration() {
+ final AnalyserTaskContext ctx = new AnalyserTaskContextImpl();
+ assertThat(new Rules(ctx).isConfigured(), equalTo(false));
+ }
+
+ @Test
+ public void testIncludesRulesConfiguration() {
+ final AnalyserTaskContext ctx = new AnalyserTaskContextImpl();
+ ctx.getConfiguration().put("includes", "/a, /b");
+
+ final Rules r = new Rules(ctx);
+ assertEquals(0, r.excludes.length);
+ assertEquals(2, r.includes.length);
+ assertEquals("/a", r.includes[0]);
+ assertEquals("/b", r.includes[1]);
+ }
+
+ @Test
+ public void testExcludesRulesConfiguration() {
+ final AnalyserTaskContext ctx = new AnalyserTaskContextImpl();
+ ctx.getConfiguration().put("excludes", "/a, /b");
+
+ final Rules r = new Rules(ctx);
+ assertEquals(0, r.includes.length);
+ assertEquals(2, r.excludes.length);
+ assertEquals("/a", r.excludes[0]);
+ assertEquals("/b", r.excludes[1]);
+ }
+}
