raducotescu commented on a change in pull request #4:
URL:
https://github.com/apache/sling-scriptingbundle-maven-plugin/pull/4#discussion_r685215457
##########
File path: pom.xml
##########
@@ -49,6 +49,7 @@
<maven.version>3.6.0</maven.version>
<site.jira.version.id>12344664,12347157,12348422,12350010</site.jira.version.id>
<maven.site.path>${project.artifactId}-archives/${project.artifactId}-LATEST</maven.site.path>
+
<project.build.outputTimestamp>2020-01-22T15:10:38Z</project.build.outputTimestamp>
Review comment:
What's this property supposed to do?
##########
File path:
src/main/java/org/apache/sling/scriptingbundle/plugin/processor/FileProcessor.java
##########
@@ -118,81 +123,86 @@ void processRequiresFile(@NotNull Path requiresFile,
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);
}
}
public void processScriptFile(@NotNull Path resourceTypeDirectory,
@NotNull Path scriptPath,
@NotNull ResourceType resourceType,
@NotNull Set<ProvidedResourceTypeCapability> providedCapabilities) {
- String filePath = scriptPath.toString();
- String extension = FilenameUtils.getExtension(filePath);
- if (StringUtils.isNotEmpty(extension) &&
scriptEngineMappings.containsKey(extension)) {
- Path scriptFile = scriptPath.getFileName();
- if (scriptFile != null) {
- Path relativeResourceTypeFolder =
resourceTypeDirectory.relativize(scriptPath);
- int pathSegments = relativeResourceTypeFolder.getNameCount();
- LinkedHashSet<String> selectors = new LinkedHashSet<>();
- if (pathSegments > 1) {
- for (int i = 0; i < pathSegments - 1; i++) {
-
selectors.add(relativeResourceTypeFolder.getName(i).toString());
- }
+ Path scriptFile = scriptPath.getFileName();
Review comment:
What was the point of refactoring this chunk? It doesn't seem to be
related to the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
