You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by kw...@apache.org on 2021/08/09 17:07:32 UTC
[sling-scriptingbundle-maven-plugin] branch master updated:
SLING-10693 fail build in case of invalid extends/requires file (#4)
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/sling-scriptingbundle-maven-plugin.git
The following commit(s) were added to refs/heads/master by this push:
new 30dd4c0 SLING-10693 fail build in case of invalid extends/requires file (#4)
30dd4c0 is described below
commit 30dd4c0054de53365378bf7fa2f1ee5b2548b61f
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Mon Aug 9 19:06:59 2021 +0200
SLING-10693 fail build in case of invalid extends/requires file (#4)
---
pom.xml | 2 +
.../plugin/processor/FileProcessor.java | 33 +++--
.../processor/ResourceTypeFolderAnalyser.java | 5 +-
.../plugin/processor/FileProcessorTest.java | 135 +++++++++++++++++++++
.../plugin/processor/Slf4jLogger.java | 52 ++++++++
src/test/resources/extends/invalid-attributes | 1 +
src/test/resources/extends/multiple-clauses | 1 +
src/test/resources/extends/multiple-lines | 2 +
.../extends => extends/valid} | 2 +-
.../main/scripts/org.apache.sling.foobar/extends | 2 +-
src/test/resources/requires/invalid-attributes | 1 +
src/test/resources/requires/multiple-clauses | 1 +
.../extends => requires/valid} | 2 +-
13 files changed, 224 insertions(+), 15 deletions(-)
diff --git a/pom.xml b/pom.xml
index c550988..732a97c 100644
--- a/pom.xml
+++ b/pom.xml
@@ -99,6 +99,8 @@
<exclude>src/test/resources/**/extends</exclude>
<exclude>src/test/resources/**/requires</exclude>
<exclude>src/test/resources/**/wrongbar</exclude>
+ <exclude>src/test/resources/extends/*</exclude>
+ <exclude>src/test/resources/requires/*</exclude>
</excludes>
</configuration>
</plugin>
diff --git a/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/FileProcessor.java b/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/FileProcessor.java
index 73ee002..dab50f7 100644
--- a/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/FileProcessor.java
+++ b/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/FileProcessor.java
@@ -19,6 +19,7 @@
package org.apache.sling.scriptingbundle.plugin.processor;
import java.io.IOException;
+import java.io.UncheckedIOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -51,7 +52,9 @@ public class FileProcessor {
private final Map<String, String> scriptEngineMappings;
private static final Collection<String> EXTENDS_ALLOWED_ATTRIBUTE_NAMES = Arrays.asList(aQute.bnd.osgi.Constants.RESOLUTION_DIRECTIVE, aQute.bnd.osgi.Constants.VERSION_ATTRIBUTE);
+ private static final Collection<String> REQUIRES_ALLOWED_ATTRIBUTE_NAMES = Arrays.asList(aQute.bnd.osgi.Constants.RESOLUTION_DIRECTIVE, aQute.bnd.osgi.Constants.VERSION_ATTRIBUTE);
+
public FileProcessor(Logger log, Set<String> searchPaths, Map<String, String> scriptEngineMappings) {
this.log = log;
this.searchPaths = searchPaths;
@@ -60,19 +63,19 @@ public class FileProcessor {
public void processExtendsFile(@NotNull ResourceType resourceType, @NotNull Path extendsFile,
@NotNull Set<ProvidedResourceTypeCapability> providedCapabilities,
- @NotNull Set<RequiredResourceTypeCapability> requiredCapabilities) {
+ @NotNull Set<RequiredResourceTypeCapability> requiredCapabilities) throws IllegalArgumentException {
try {
List<String> extendResources = Files.readAllLines(extendsFile, StandardCharsets.UTF_8);
if (extendResources.size() == 1) {
String extend = extendResources.get(0);
Parameters parameters = OSGiHeader.parseHeader(extend);
if (parameters.size() < 1 || parameters.size() > 1) {
- log.error(String.format("The file '%s' must contain one clause only (not multiple ones separated by ','", extendsFile));
+ throw new IllegalArgumentException(String.format("The file '%s' must contain one clause only (not multiple ones separated by ',')", extendsFile));
}
Entry<String, Attrs> extendsParameter = parameters.entrySet().iterator().next();
for (String attributeName : extendsParameter.getValue().keySet()) {
if (!EXTENDS_ALLOWED_ATTRIBUTE_NAMES.contains(attributeName)) {
- log.error(String.format("Found attribute/directive %s in file %s. Only the following attributes or directives may be used in the extends file: %s", attributeName, extendsFile, String.join(",", EXTENDS_ALLOWED_ATTRIBUTE_NAMES)));
+ throw new IllegalArgumentException(String.format("Found unsupported attribute/directive '%s' in file '%s'. Only the following attributes or directives may be used in the extends file: %s", attributeName, extendsFile, String.join(",", EXTENDS_ALLOWED_ATTRIBUTE_NAMES)));
}
}
String extendedResourceType = FilenameUtils.normalize(extendsParameter.getKey(), true);
@@ -105,9 +108,11 @@ public class FileProcessor {
}
extractVersionRange(extendsFile, requiredBuilder, extendsParameter.getValue().getVersion());
requiredCapabilities.add(requiredBuilder.build());
+ } else {
+ throw new IllegalArgumentException(String.format("The file '%s' must contain one line only (not multiple ones)", extendsFile));
}
} catch (IOException e) {
- log.error(String.format("Unable to read file %s.", extendsFile.toString()), e);
+ throw new UncheckedIOException(String.format("Unable to read file %s.", extendsFile.toString()), e);
}
}
@@ -118,18 +123,26 @@ public class FileProcessor {
for (String requiredResourceType : requiredResourceTypes) {
Parameters parameters = OSGiHeader.parseHeader(requiredResourceType);
if (parameters.size() < 1 || parameters.size() > 1) {
- log.error(String.format("Each line in file '%s' must contain one clause only (not multiple ones separated by ',', skipping line", requiresFile));
- continue;
+ throw new IllegalArgumentException(String.format("Each line in file '%s' must contain one clause only (not multiple ones separated by ',')", requiresFile));
}
Entry<String, Attrs> requiresParameter = parameters.entrySet().iterator().next();
+ for (String attributeName : requiresParameter.getValue().keySet()) {
+ if (!REQUIRES_ALLOWED_ATTRIBUTE_NAMES.contains(attributeName)) {
+ throw new IllegalArgumentException(String.format("Found unsupported attribute/directive '%s' in file '%s'. Only the following attributes or directives may be used in the requires file: %s", attributeName, requiresFile, String.join(",", REQUIRES_ALLOWED_ATTRIBUTE_NAMES)));
+ }
+ }
String resourceType = FilenameUtils.normalize(requiresParameter.getKey(), true);
+ boolean isOptional = aQute.bnd.osgi.Constants.OPTIONAL.equals(requiresParameter.getValue().get(aQute.bnd.osgi.Constants.RESOLUTION_DIRECTIVE));
RequiredResourceTypeCapability.Builder requiredBuilder =
RequiredResourceTypeCapability.builder().withResourceType(resourceType);
+ if (isOptional) {
+ requiredBuilder.withIsOptional();
+ }
extractVersionRange(requiresFile, requiredBuilder, requiresParameter.getValue().getVersion());
requiredCapabilities.add(requiredBuilder.build());
}
} catch (IOException e) {
- log.error(String.format("Unable to read file %s.", requiresFile.toString()), e);
+ throw new UncheckedIOException(String.format("Unable to read file %s.", requiresFile.toString()), e);
}
}
@@ -137,7 +150,7 @@ public class FileProcessor {
@NotNull ResourceType resourceType, @NotNull Set<ProvidedResourceTypeCapability> providedCapabilities) {
String filePath = scriptPath.toString();
String extension = FilenameUtils.getExtension(filePath);
- if (StringUtils.isNotEmpty(extension) && scriptEngineMappings.containsKey(extension)) {
+ if (StringUtils.isNotEmpty(extension)) {
Path scriptFile = scriptPath.getFileName();
if (scriptFile != null) {
Path relativeResourceTypeFolder = resourceTypeDirectory.relativize(scriptPath);
@@ -187,10 +200,10 @@ public class FileProcessor {
log.warn(String.format("Cannot find a script engine mapping for script %s.", scriptPath));
}
} else {
- log.warn(String.format("File %s does not denote a script.", scriptPath));
+ log.warn(String.format("Skipping file %s not denoting a script as it does not follow the filename patterns outlined at https://sling.apache.org/documentation/the-sling-engine/url-to-script-resolution.html#script-naming-conventions", scriptPath));
}
} else {
- log.warn(String.format("Skipping path %s since it has 0 elements.", scriptPath));
+ throw new IllegalArgumentException(String.format("Invalid path given: '%s'.", scriptPath));
}
}
}
diff --git a/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/ResourceTypeFolderAnalyser.java b/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/ResourceTypeFolderAnalyser.java
index 8eff945..b7b6820 100644
--- a/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/ResourceTypeFolderAnalyser.java
+++ b/src/main/java/org/apache/sling/scriptingbundle/plugin/processor/ResourceTypeFolderAnalyser.java
@@ -19,6 +19,7 @@
package org.apache.sling.scriptingbundle.plugin.processor;
import java.io.IOException;
+import java.io.UncheckedIOException;
import java.nio.file.DirectoryStream;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -88,8 +89,8 @@ public class ResourceTypeFolderAnalyser {
}
}
});
- } catch (IOException | IllegalArgumentException e) {
- logger.warn(String.format("Cannot analyse folder %s.", scriptsDirectory.resolve(resourceTypeDirectory).toString()), e);
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
}
}
diff --git a/src/test/java/org/apache/sling/scriptingbundle/plugin/processor/FileProcessorTest.java b/src/test/java/org/apache/sling/scriptingbundle/plugin/processor/FileProcessorTest.java
new file mode 100644
index 0000000..f692ee2
--- /dev/null
+++ b/src/test/java/org/apache/sling/scriptingbundle/plugin/processor/FileProcessorTest.java
@@ -0,0 +1,135 @@
+/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ ~ 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.scriptingbundle.plugin.processor;
+
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+import org.apache.sling.api.resource.type.ResourceType;
+import org.apache.sling.scriptingbundle.plugin.capability.ProvidedResourceTypeCapability;
+import org.apache.sling.scriptingbundle.plugin.capability.RequiredResourceTypeCapability;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.osgi.framework.VersionRange;
+
+public class FileProcessorTest {
+
+ private FileProcessor processor;
+ Set<ProvidedResourceTypeCapability> providedCapabilities;
+ Set<RequiredResourceTypeCapability> requiredCapabilities;
+ private static final ResourceType MY_RESOURCE_TYPE = ResourceType.parseResourceType("apps/my/resource");
+
+ @Before
+ public void setUp() {
+ processor = new FileProcessor(new Slf4jLogger(), Constants.DEFAULT_SEARCH_PATHS, Constants.DEFAULT_EXTENSION_TO_SCRIPT_ENGINE_MAPPING);
+ providedCapabilities = new HashSet<>();
+ requiredCapabilities = new HashSet<>();
+ }
+
+ @Test
+ public void testExtendsValid() {
+ Path extendsFile = Paths.get("src", "test", "resources", "extends", "valid");
+ processor.processExtendsFile(MY_RESOURCE_TYPE, extendsFile, providedCapabilities, requiredCapabilities);
+ Assert.assertEquals(1, requiredCapabilities.size());
+ RequiredResourceTypeCapability expectedRequiredCapability = RequiredResourceTypeCapability.builder()
+ .withResourceType("org/apache/sling/bar")
+ .withVersionRange(new VersionRange("[1.0.0,2.0.0)"))
+ .withIsOptional().build();
+ Assert.assertEquals(expectedRequiredCapability, requiredCapabilities.iterator().next());
+ Assert.assertEquals(1, providedCapabilities.size());
+ ProvidedResourceTypeCapability expectedProvidedCapability = ProvidedResourceTypeCapability.builder()
+ .withResourceTypes(new HashSet<String>(Arrays.asList("my/resource", "/apps/my/resource")))
+ .withVersion(MY_RESOURCE_TYPE.getVersion())
+ .withExtendsResourceType("org/apache/sling/bar")
+ .build();
+ Assert.assertEquals(expectedProvidedCapability, providedCapabilities.iterator().next());
+ }
+
+ @Test
+ public void testExtendsMultipleClauses() {
+ Path extendsFile = Paths.get("src", "test", "resources", "extends", "multiple-clauses");
+ Assert.assertThrows(IllegalArgumentException.class, () -> { processor.processExtendsFile(MY_RESOURCE_TYPE, extendsFile, providedCapabilities, requiredCapabilities); });
+ }
+
+ @Test
+ public void testExtendsInvalidAttributes() {
+ Path extendsFile = Paths.get("src", "test", "resources", "extends", "invalid-attributes");
+ Assert.assertThrows(IllegalArgumentException.class, () -> { processor.processExtendsFile(MY_RESOURCE_TYPE, extendsFile, providedCapabilities, requiredCapabilities); });
+ }
+
+ @Test
+ public void testExtendsMultipleLines() {
+ Path extendsFile = Paths.get("src", "test", "resources", "extends", "multiple-lines");
+ Assert.assertThrows(IllegalArgumentException.class, () -> { processor.processExtendsFile(MY_RESOURCE_TYPE, extendsFile, providedCapabilities, requiredCapabilities); });
+ }
+
+ @Test
+ public void testRequiresValid() {
+ Path requiresFile = Paths.get("src", "test", "resources", "requires", "valid");
+ processor.processRequiresFile(requiresFile, requiredCapabilities);
+ Assert.assertEquals(1, requiredCapabilities.size());
+ RequiredResourceTypeCapability expectedCapability = RequiredResourceTypeCapability.builder()
+ .withResourceType("org/apache/sling/bar")
+ .withVersionRange(new VersionRange("[1.0.0,2.0.0)"))
+ .withIsOptional().build();
+ Assert.assertEquals(expectedCapability, requiredCapabilities.iterator().next());
+ }
+
+ @Test
+ public void testRequiresMultipleClauses() {
+ Path requiresFile = Paths.get("src", "test", "resources", "requires", "multiple-clauses");
+ Assert.assertThrows(IllegalArgumentException.class, () -> { processor.processRequiresFile(requiresFile, requiredCapabilities); });
+ }
+
+ @Test
+ public void testRequiresInvalidAttributes() {
+ Path requiresFile = Paths.get("src", "test", "resources", "extends", "invalid-attributes");
+ Assert.assertThrows(IllegalArgumentException.class, () -> { processor.processRequiresFile(requiresFile, requiredCapabilities); });
+ }
+
+ @Test
+ public void testScriptValid() {
+ Path resourceTypeFolder = Paths.get("apps", "my", "resource", "2.0");
+ Path script = Paths.get("apps", "my", "resource", "2.0", "selectorb", "selectora.POST.html");
+ processor.processScriptFile(resourceTypeFolder, script, MY_RESOURCE_TYPE, providedCapabilities);
+ Assert.assertEquals(1, providedCapabilities.size());
+ ProvidedResourceTypeCapability expectedProvidedCapability = ProvidedResourceTypeCapability.builder()
+ .withResourceTypes(new HashSet<>(Arrays.asList("my/resource", "/apps/my/resource")))
+ .withVersion(MY_RESOURCE_TYPE.getVersion())
+ .withRequestMethod("POST")
+ .withSelectors(new HashSet<>(Arrays.asList("selectorb", "selectora")))
+ .withScriptEngine("htl")
+ .withScriptExtension("html")
+ .build();
+ Assert.assertEquals(expectedProvidedCapability, providedCapabilities.iterator().next());
+ }
+
+ @Test
+ public void testScriptUnknownExtension() {
+ Path resourceTypeFolder = Paths.get("scripts", "apps", "my", "resource", "2.0");
+ Path script = Paths.get("scripts", "apps", "my", "resource", "2.0", "selectorb", "selectora.POST.abc");
+ processor.processScriptFile(resourceTypeFolder, script, MY_RESOURCE_TYPE, providedCapabilities);
+ // this must not throw an exception but a WARN should be emitted in the log to make users aware of potential misconfigurations
+ Assert.assertEquals(0, providedCapabilities.size());
+ }
+}
diff --git a/src/test/java/org/apache/sling/scriptingbundle/plugin/processor/Slf4jLogger.java b/src/test/java/org/apache/sling/scriptingbundle/plugin/processor/Slf4jLogger.java
new file mode 100644
index 0000000..e56dceb
--- /dev/null
+++ b/src/test/java/org/apache/sling/scriptingbundle/plugin/processor/Slf4jLogger.java
@@ -0,0 +1,52 @@
+/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ ~ 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.scriptingbundle.plugin.processor;
+
+import org.jetbrains.annotations.NotNull;
+
+public class Slf4jLogger implements Logger {
+
+ private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(Slf4jLogger.class);
+
+ @Override
+ public void error(@NotNull String message) {
+ logger.error(message);
+ }
+
+ @Override
+ public void error(@NotNull String message, @NotNull Throwable t) {
+ logger.error(message, t);
+ }
+
+ @Override
+ public void info(@NotNull String message) {
+ logger.info(message);
+ }
+
+ @Override
+ public void warn(@NotNull String message) {
+ logger.warn(message);
+ }
+
+ @Override
+ public void warn(@NotNull String message, @NotNull Throwable t) {
+ logger.warn(message, t);
+ }
+
+}
diff --git a/src/test/resources/extends/invalid-attributes b/src/test/resources/extends/invalid-attributes
new file mode 100644
index 0000000..f273579
--- /dev/null
+++ b/src/test/resources/extends/invalid-attributes
@@ -0,0 +1 @@
+clause1;attribute1=value1
\ No newline at end of file
diff --git a/src/test/resources/extends/multiple-clauses b/src/test/resources/extends/multiple-clauses
new file mode 100644
index 0000000..d9f9961
--- /dev/null
+++ b/src/test/resources/extends/multiple-clauses
@@ -0,0 +1 @@
+clause1,clause2
\ No newline at end of file
diff --git a/src/test/resources/extends/multiple-lines b/src/test/resources/extends/multiple-lines
new file mode 100644
index 0000000..a999ba7
--- /dev/null
+++ b/src/test/resources/extends/multiple-lines
@@ -0,0 +1,2 @@
+clause1
+clause2
\ No newline at end of file
diff --git a/src/test/resources/project-1/src/main/scripts/org.apache.sling.foobar/extends b/src/test/resources/extends/valid
similarity index 96%
copy from src/test/resources/project-1/src/main/scripts/org.apache.sling.foobar/extends
copy to src/test/resources/extends/valid
index 9e61a9b..bde342d 100644
--- a/src/test/resources/project-1/src/main/scripts/org.apache.sling.foobar/extends
+++ b/src/test/resources/extends/valid
@@ -1 +1 @@
-org/apache/sling/bar;version="[1.0.0,2.0.0)";resolution:=optional
+org/apache/sling/bar;version="[1.0.0,2.0.0)";resolution:=optional;
\ No newline at end of file
diff --git a/src/test/resources/project-1/src/main/scripts/org.apache.sling.foobar/extends b/src/test/resources/project-1/src/main/scripts/org.apache.sling.foobar/extends
index 9e61a9b..a2631cb 100644
--- a/src/test/resources/project-1/src/main/scripts/org.apache.sling.foobar/extends
+++ b/src/test/resources/project-1/src/main/scripts/org.apache.sling.foobar/extends
@@ -1 +1 @@
-org/apache/sling/bar;version="[1.0.0,2.0.0)";resolution:=optional
+org/apache/sling/bar;version="[1.0.0,2.0.0)";resolution:=optional;
diff --git a/src/test/resources/requires/invalid-attributes b/src/test/resources/requires/invalid-attributes
new file mode 100644
index 0000000..f273579
--- /dev/null
+++ b/src/test/resources/requires/invalid-attributes
@@ -0,0 +1 @@
+clause1;attribute1=value1
\ No newline at end of file
diff --git a/src/test/resources/requires/multiple-clauses b/src/test/resources/requires/multiple-clauses
new file mode 100644
index 0000000..d9f9961
--- /dev/null
+++ b/src/test/resources/requires/multiple-clauses
@@ -0,0 +1 @@
+clause1,clause2
\ No newline at end of file
diff --git a/src/test/resources/project-1/src/main/scripts/org.apache.sling.foobar/extends b/src/test/resources/requires/valid
similarity index 96%
copy from src/test/resources/project-1/src/main/scripts/org.apache.sling.foobar/extends
copy to src/test/resources/requires/valid
index 9e61a9b..bde342d 100644
--- a/src/test/resources/project-1/src/main/scripts/org.apache.sling.foobar/extends
+++ b/src/test/resources/requires/valid
@@ -1 +1 @@
-org/apache/sling/bar;version="[1.0.0,2.0.0)";resolution:=optional
+org/apache/sling/bar;version="[1.0.0,2.0.0)";resolution:=optional;
\ No newline at end of file