Merge pull request #6 from bosschaert/SLING-9136
SLING-9136 Check if a feature model exports overlapping packages with an API Region
diff --git a/README.md b/README.md
index 3dffa02..bd06f86 100644
--- a/README.md
+++ b/README.md
@@ -38,6 +38,20 @@
* `order`: A comma separated list of the region names declaring the order in which they should be found. Not all regions declared must be present, but if they are present this
order must be obeyed.
+* `api-regions-crossfeature-dups`: This analyser checks whether there are exported packages in a feature model
+that does _not_ opt in to the API Regions (i.e. it does not have an API-Regions section) that overlap with exported
+packages from API regions in other feature models. It can prevent against unwanted results when packages are
+exported from the outside which should be exported from an API Region.
+This analyser only provides a useful result when run on
+an aggregate feature model, i.e. a feature model that was created by aggregating a number of other feature models. It uses the
+`feature-origins` metadata to find the features that bundles were initially declared in. It then matches this with the `feature-origins` found in the `api-regions` section. Exports from bundles from features that don't
+declare `api-regions` are compared to declared exports in the `api-regions` section. If there is overlap an error
+is reported.
+ * Configuration parameters:
+ * `regions`: a comma separated list of regions to check. If not specified all regions found are checked. This configuration item can be used to exclude certain regions from the check.
+ * `warningPackages`: if packages listed here are found to overlap, a warning instead of an error is reported. Supports either literal package names (e.g. `javax.servlet`) or wildcards with an asterisk at the end (e.g. `javax.*`).
+ * `ignoredPackages`: packages listed here are completely ignored in the analysis. Supports literal package names or wildcards with an asterisk at the end.
+
# Additional Extensions
The following extensions are also implemented by this component and made available through the Service Loader mechanism:
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsCrossFeatureDups.java b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsCrossFeatureDups.java
new file mode 100644
index 0000000..7c9c871
--- /dev/null
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsCrossFeatureDups.java
@@ -0,0 +1,130 @@
+/*
+ * 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.extension.apiregions.analyser;
+
+import org.apache.sling.feature.ArtifactId;
+import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
+import org.apache.sling.feature.extension.apiregions.api.ApiExport;
+import org.apache.sling.feature.extension.apiregions.api.ApiRegion;
+import org.apache.sling.feature.extension.apiregions.api.ApiRegions;
+import org.apache.sling.feature.scanner.BundleDescriptor;
+import org.apache.sling.feature.scanner.FeatureDescriptor;
+import org.apache.sling.feature.scanner.PackageInfo;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+public class CheckApiRegionsCrossFeatureDups extends AbstractApiRegionsAnalyserTask {
+
+ @Override
+ public String getId() {
+ return ApiRegions.EXTENSION_NAME + "-crossfeature-dups";
+ }
+
+ @Override
+ public String getName() {
+ return "Api Regions cross-feature duplicate export task";
+ }
+
+ @Override
+ protected void execute(ApiRegions apiRegions, AnalyserTaskContext ctx) throws Exception {
+ Set<String> checkedRegions = splitListConfig(ctx.getConfiguration().get("regions"));
+ Set<String> ignoredPackages = splitListConfig(ctx.getConfiguration().get("ignoredPackages"));
+ Set<String> warningPackages = splitListConfig(ctx.getConfiguration().get("warningPackages"));
+
+ Map<String, Set<String>> regionExports = new HashMap<>();
+
+ List<ArtifactId> apiRegionsFeatures = new ArrayList<>();
+ for (ApiRegion r : apiRegions.listRegions()) {
+ apiRegionsFeatures.addAll(Arrays.asList(r.getFeatureOrigins()));
+ if (checkedRegions.isEmpty() || checkedRegions.contains(r.getName())) {
+ Set<String> exports = regionExports.get(r.getName());
+ if (exports == null) {
+ exports = new HashSet<>();
+ regionExports.put(r.getName(), exports);
+ }
+ exports.addAll(r.listExports().stream().map(ApiExport::getName).collect(Collectors.toSet()));
+ }
+ }
+
+ FeatureDescriptor f = ctx.getFeatureDescriptor();
+ for (BundleDescriptor bd : f.getBundleDescriptors()) {
+ List<ArtifactId> borgs = new ArrayList<>(Arrays.asList(bd.getArtifact().getFeatureOrigins()));
+ borgs.removeAll(apiRegionsFeatures);
+
+ if (!borgs.isEmpty()) {
+ // This bundle was contributed by a feature that did not opt-in to the API Regions
+ Set<String> reportedPackages = new HashSet<>();
+ for (PackageInfo pi : bd.getExportedPackages()) {
+ String pkgName = pi.getName();
+ for (Map.Entry<String, Set<String>> entry : regionExports.entrySet()) {
+ if (entry.getValue().contains(pkgName) && !reportedPackages.contains(pkgName)) {
+ if (matchesSet(pkgName, ignoredPackages)) {
+ continue;
+ }
+
+ reportedPackages.add(pi.getName());
+
+ String msg = "Package overlap found between region " + entry.getKey()
+ + " and bundle " + bd.getBundleSymbolicName() + " " + bd.getBundleVersion()
+ + " which comes from a feature without API Regions: " + borgs
+ + ". Both export package: " + pi.getName();
+ if (matchesSet(pkgName, warningPackages)) {
+ ctx.reportWarning(msg);
+ } else {
+ ctx.reportError(msg);
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
+ private boolean matchesSet(String pkg, Set<String> set) {
+ for (String e : set) {
+ if (e.endsWith("*")) {
+ if (pkg.startsWith(e.substring(0, e.length() - 1)) ) {
+ return true;
+ }
+ } else {
+ if (pkg.equals(e)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ private Set<String> splitListConfig(String value) {
+ if (value == null) {
+ return Collections.emptySet();
+ } else {
+ return Arrays.asList(value.split(","))
+ .stream()
+ .map(String::trim)
+ .collect(Collectors.toSet());
+ }
+ }
+}
diff --git a/src/main/resources/META-INF/services/org.apache.sling.feature.analyser.task.AnalyserTask b/src/main/resources/META-INF/services/org.apache.sling.feature.analyser.task.AnalyserTask
index 5448550..0744c72 100644
--- a/src/main/resources/META-INF/services/org.apache.sling.feature.analyser.task.AnalyserTask
+++ b/src/main/resources/META-INF/services/org.apache.sling.feature.analyser.task.AnalyserTask
@@ -3,3 +3,4 @@
org.apache.sling.feature.extension.apiregions.analyser.CheckApiRegionsDuplicates
org.apache.sling.feature.extension.apiregions.analyser.CheckApiRegionsOrder
org.apache.sling.feature.extension.apiregions.analyser.CheckApiRegionsBundleExportsImports
+org.apache.sling.feature.extension.apiregions.analyser.CheckApiRegionsCrossFeatureDups
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsCrossFeatureDupsTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsCrossFeatureDupsTest.java
new file mode 100644
index 0000000..3765fe3
--- /dev/null
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsCrossFeatureDupsTest.java
@@ -0,0 +1,216 @@
+/*
+ * 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.extension.apiregions.analyser;
+
+import org.apache.sling.feature.ArtifactId;
+import org.apache.sling.feature.Feature;
+import org.apache.sling.feature.analyser.Analyser;
+import org.apache.sling.feature.analyser.AnalyserResult;
+import org.apache.sling.feature.analyser.task.AnalyserTask;
+import org.apache.sling.feature.builder.ArtifactProvider;
+import org.apache.sling.feature.extension.apiregions.scanner.ApiRegionsExtensionScanner;
+import org.apache.sling.feature.io.json.FeatureJSONReader;
+import org.apache.sling.feature.scanner.Scanner;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.StringReader;
+import java.net.URL;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class CheckApiRegionsCrossFeatureDupsTest {
+ @Test
+ public void testOverlapError() throws Exception {
+ Path fp = new File(getClass().getResource("/crossfeatdups/fm1.json").getFile()).toPath();
+ String fm = new String(Files.readAllBytes(fp));
+
+ Feature f = FeatureJSONReader.read(new StringReader(fm), null);
+
+ Scanner scanner = getScanner();
+ AnalyserTask at = new CheckApiRegionsCrossFeatureDups();
+ Map<String, Map<String,String>> configs =
+ Collections.singletonMap("api-regions-crossfeature-dups",
+ Collections.singletonMap("warningPackages", "x.y.z"));
+ Analyser a = new Analyser(scanner, configs, at);
+ AnalyserResult res = a.analyse(f);
+ assertEquals(1, res.getErrors().size());
+ assertEquals(0, res.getWarnings().size());
+
+ String err = res.getErrors().get(0);
+ assertTrue(err.contains("a.b.c"));
+ assertTrue(err.contains("g:f3:1"));
+ assertTrue(err.contains("feature-export2"));
+ }
+
+ @Test
+ public void testOverlapWarning() throws Exception {
+ Path fp = new File(getClass().getResource("/crossfeatdups/fm1.json").getFile()).toPath();
+ String fm = new String(Files.readAllBytes(fp));
+
+ Feature f = FeatureJSONReader.read(new StringReader(fm), null);
+
+ Scanner scanner = getScanner();
+ CheckApiRegionsCrossFeatureDups at = new CheckApiRegionsCrossFeatureDups();
+ Map<String, String> cfg = new HashMap<>();
+ cfg.put("warningPackages", "a.b.c");
+ cfg.put("ignoredPackages", "x.y.z");
+ Map<String, Map<String,String>> configs =
+ Collections.singletonMap("api-regions-crossfeature-dups", cfg);
+ Analyser a = new Analyser(scanner, configs, at);
+ AnalyserResult res = a.analyse(f);
+ assertEquals(0, res.getErrors().size());
+ assertEquals(1, res.getWarnings().size());
+
+ String err = res.getWarnings().get(0);
+ assertTrue(err.contains("a.b.c"));
+ assertTrue(err.contains("g:f3:1"));
+ assertTrue(err.contains("feature-export2"));
+ }
+
+ @Test
+ public void testOverlapWarning2() throws Exception {
+ Path fp = new File(getClass().getResource("/crossfeatdups/fm1.json").getFile()).toPath();
+ String fm = new String(Files.readAllBytes(fp));
+
+ Feature f = FeatureJSONReader.read(new StringReader(fm), null);
+
+ Scanner scanner = getScanner();
+ CheckApiRegionsCrossFeatureDups at = new CheckApiRegionsCrossFeatureDups();
+ Map<String, String> cfg = new HashMap<>();
+ cfg.put("warningPackages", "x.*, a.*");
+ Map<String, Map<String,String>> configs =
+ Collections.singletonMap("api-regions-crossfeature-dups", cfg);
+ Analyser a = new Analyser(scanner, configs, at);
+ AnalyserResult res = a.analyse(f);
+ assertEquals(0, res.getErrors().size());
+ assertEquals(1, res.getWarnings().size());
+
+ String err = res.getWarnings().get(0);
+ assertTrue(err.contains("a.b.c"));
+ assertTrue(err.contains("g:f3:1"));
+ assertTrue(err.contains("feature-export2"));
+ }
+
+ @Test
+ public void testOverlapIgnore() throws Exception {
+ Path fp = new File(getClass().getResource("/crossfeatdups/fm1.json").getFile()).toPath();
+ String fm = new String(Files.readAllBytes(fp));
+
+ Feature f = FeatureJSONReader.read(new StringReader(fm), null);
+
+ Scanner scanner = getScanner();
+ CheckApiRegionsCrossFeatureDups at = new CheckApiRegionsCrossFeatureDups();
+ Map<String, String> cfg = new HashMap<>();
+ cfg.put("ignoredPackages", "a.b.c");
+ Map<String, Map<String,String>> configs =
+ Collections.singletonMap("api-regions-crossfeature-dups", cfg);
+ Analyser a = new Analyser(scanner, configs, at);
+ AnalyserResult res = a.analyse(f);
+ assertEquals(0, res.getErrors().size());
+ assertEquals(0, res.getWarnings().size());
+ }
+
+ @Test
+ public void testOverlapError2() throws Exception {
+ Path fp = new File(getClass().getResource("/crossfeatdups/fm1.json").getFile()).toPath();
+ String fm = new String(Files.readAllBytes(fp));
+
+ Feature f = FeatureJSONReader.read(new StringReader(fm), null);
+
+ Scanner scanner = getScanner2();
+ AnalyserTask at = new CheckApiRegionsCrossFeatureDups();
+ Analyser a = new Analyser(scanner, at);
+ AnalyserResult res = a.analyse(f);
+ assertEquals(2, res.getErrors().size());
+ assertEquals(0, res.getWarnings().size());
+
+ String allErrs = res.getErrors().get(0) + res.getErrors().get(1);
+ assertTrue(allErrs.contains("a.b.c"));
+ assertTrue(allErrs.contains("zzz.zzz"));
+ }
+
+ @Test
+ public void testOverlapErrorOnlyGlobal() throws Exception {
+ Path fp = new File(getClass().getResource("/crossfeatdups/fm1.json").getFile()).toPath();
+ String fm = new String(Files.readAllBytes(fp));
+
+ Feature f = FeatureJSONReader.read(new StringReader(fm), null);
+
+ Scanner scanner = getScanner2();
+ AnalyserTask at = new CheckApiRegionsCrossFeatureDups();
+ Map<String, Map<String,String>> configs =
+ Collections.singletonMap("api-regions-crossfeature-dups",
+ Collections.singletonMap("regions", "something,global"));
+ Analyser a = new Analyser(scanner, configs, at);
+ AnalyserResult res = a.analyse(f);
+ assertEquals(1, res.getErrors().size());
+ assertEquals(0, res.getWarnings().size());
+
+ String err = res.getErrors().get(0);
+ assertTrue(err.contains("zzz.zzz"));
+ }
+
+ private Scanner getScanner() throws IOException {
+ ArtifactProvider ap = new ArtifactProvider() {
+ @Override
+ public URL provide(ArtifactId id) {
+ switch (id.toMvnId()) {
+ case "g:exp1:1":
+ return getClass().getResource("/crossfeatdups/test-bundles/feature-export.jar");
+ case "g:exp2:1":
+ return getClass().getResource("/crossfeatdups/test-bundles/feature-export2.jar");
+ case "g:noexp:1":
+ return getClass().getResource("/crossfeatdups/test-bundles/no-exports.jar");
+ case "g:extra:1":
+ return getClass().getResource("/crossfeatdups/test-bundles/no-exports.jar");
+ }
+ return null;
+ }
+ };
+ Scanner scanner = new Scanner(ap, Collections.singletonList(new ApiRegionsExtensionScanner()), Collections.emptyList());
+ return scanner;
+ }
+
+ private Scanner getScanner2() throws IOException {
+ ArtifactProvider ap = new ArtifactProvider() {
+ @Override
+ public URL provide(ArtifactId id) {
+ switch (id.toMvnId()) {
+ case "g:exp1:1":
+ return getClass().getResource("/crossfeatdups/test-bundles/feature-export.jar");
+ case "g:exp2:1":
+ return getClass().getResource("/crossfeatdups/test-bundles/feature-export2.jar");
+ case "g:noexp:1":
+ return getClass().getResource("/crossfeatdups/test-bundles/no-exports.jar");
+ case "g:extra:1":
+ return getClass().getResource("/crossfeatdups/test-bundles/feature-export3.jar");
+ }
+ return null;
+ }
+ };
+ Scanner scanner = new Scanner(ap, Collections.singletonList(new ApiRegionsExtensionScanner()), Collections.emptyList());
+ return scanner;
+ }
+}
diff --git a/src/test/resources/crossfeatdups/fm1.json b/src/test/resources/crossfeatdups/fm1.json
new file mode 100644
index 0000000..b0a037b
--- /dev/null
+++ b/src/test/resources/crossfeatdups/fm1.json
@@ -0,0 +1,35 @@
+{
+ "id":"g:fm1:1.2.3",
+ "bundles":[
+ {
+ "id":"g:exp1:1",
+ "feature-origins":"g:f1:1"
+ },
+ {
+ "id":"g:exp2:1",
+ "feature-origins":"g:f2:1,g:f3:1"
+ },
+ "g:noexp:1",
+ {
+ "id": "g:extra:1",
+ "feature-origins":"g:f3:1"
+ }
+ ],
+ "api-regions:JSON|false":[
+ {
+ "name":"foo",
+ "exports":[
+ "a.b.c",
+ "d.e.f"
+ ],
+ "feature-origins":[
+ "g:f1:1","g:f2:1"
+ ]
+ },{
+ "name":"global",
+ "exports":[
+ "zzz.zzz"
+ ]
+ }
+ ]
+}
diff --git a/src/test/resources/crossfeatdups/test-bundles/feature-export.jar b/src/test/resources/crossfeatdups/test-bundles/feature-export.jar
new file mode 100644
index 0000000..fa3c6ed
--- /dev/null
+++ b/src/test/resources/crossfeatdups/test-bundles/feature-export.jar
Binary files differ
diff --git a/src/test/resources/crossfeatdups/test-bundles/feature-export2.jar b/src/test/resources/crossfeatdups/test-bundles/feature-export2.jar
new file mode 100644
index 0000000..28de57d
--- /dev/null
+++ b/src/test/resources/crossfeatdups/test-bundles/feature-export2.jar
Binary files differ
diff --git a/src/test/resources/crossfeatdups/test-bundles/feature-export3.jar b/src/test/resources/crossfeatdups/test-bundles/feature-export3.jar
new file mode 100644
index 0000000..81861ea
--- /dev/null
+++ b/src/test/resources/crossfeatdups/test-bundles/feature-export3.jar
Binary files differ
diff --git a/src/test/resources/crossfeatdups/test-bundles/no-exports.jar b/src/test/resources/crossfeatdups/test-bundles/no-exports.jar
new file mode 100644
index 0000000..070fc16
--- /dev/null
+++ b/src/test/resources/crossfeatdups/test-bundles/no-exports.jar
Binary files differ
