kwin commented on a change in pull request #4:
URL:
https://github.com/apache/sling-scriptingbundle-maven-plugin/pull/4#discussion_r685347317
##########
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:
Initially I thought that we should also throw an exception if no engine
was found for an extension. This was too strict though, but it is important to
always log a WARN to make users aware of potential misconfigurations. I only
removed the condition of line 140 because of this. I am gonna restore the
condition for the empty extension to make it more visible.
--
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]
