SLING-11068 split up analyser task for unversioned packages - clean up code and use newer apis
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleExportsImports.java b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleExportsImports.java
index 0a5797c..fd84ca1 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleExportsImports.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleExportsImports.java
@@ -18,10 +18,8 @@
*/
package org.apache.sling.feature.extension.apiregions.analyser;
-import java.io.IOException;
import java.util.AbstractMap;
import java.util.ArrayList;
-import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
@@ -32,19 +30,14 @@
import java.util.TreeMap;
import java.util.stream.Collectors;
import java.util.stream.Stream;
-import javax.json.JsonArray;
import org.apache.sling.feature.ArtifactId;
-import org.apache.sling.feature.Extension;
-import org.apache.sling.feature.Extensions;
-import org.apache.sling.feature.Feature;
import org.apache.sling.feature.analyser.task.AnalyserTask;
import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
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.PackageInfo;
-import org.osgi.framework.Version;
public class CheckApiRegionsBundleExportsImports implements AnalyserTask {
@@ -77,12 +70,7 @@
}
private Report getReport(final Map<BundleDescriptor, Report> reports, final BundleDescriptor info) {
- Report report = reports.get(info);
- if ( report == null ) {
- report = new Report();
- reports.put(info, report);
- }
- return report;
+ return reports.computeIfAbsent(info, key -> new Report());
}
@Override
@@ -94,11 +82,7 @@
final SortedMap<Integer, List<BundleDescriptor>> bundlesMap = new TreeMap<>();
for(final BundleDescriptor bi : ctx.getFeatureDescriptor().getBundleDescriptors()) {
- List<BundleDescriptor> list = bundlesMap.get(bi.getArtifact().getStartOrder());
- if ( list == null ) {
- list = new ArrayList<>();
- bundlesMap.put(bi.getArtifact().getStartOrder(), list);
- }
+ final List<BundleDescriptor> list = bundlesMap.computeIfAbsent(bi.getArtifact().getStartOrder(), key -> new ArrayList<>());
list.add(bi);
}
@@ -107,104 +91,97 @@
if ( ctx.getFrameworkDescriptor() != null ) {
exportingBundles.add(ctx.getFrameworkDescriptor());
}
- ApiRegions apiRegions = new ApiRegions(); // Empty API Regions;
-
- Feature feature = ctx.getFeature();
// extract and check the api-regions
-
- Extensions extensions = feature.getExtensions();
- Extension apiRegionsExtension = extensions.getByName(ApiRegions.EXTENSION_NAME);
- if (apiRegionsExtension != null && apiRegionsExtension.getJSONStructure() != null) {
- try {
- apiRegions = ApiRegions.parse((JsonArray) apiRegionsExtension.getJSONStructure());
- } catch (IOException e) {
- ctx.reportError("API Regions '" + apiRegionsExtension.getJSON()
- + "' does not represent a valid JSON 'api-regions': " + e.getMessage());
- return;
- }
+ ApiRegions apiRegions = null;
+ try {
+ apiRegions = ApiRegions.getApiRegions(ctx.getFeature());
+ } catch (final IllegalArgumentException e) {
+ ctx.reportError("API Region does not represent a valid JSON 'api-regions': " + e.getMessage());
+ return;
+ }
+ if ( apiRegions == null ) {
+ apiRegions = new ApiRegions(); // Empty region as default
}
for(final Map.Entry<Integer, List<BundleDescriptor>> entry : bundlesMap.entrySet()) {
// first add all exporting bundles
for(final BundleDescriptor info : entry.getValue()) {
- if ( info.getExportedPackages() != null ) {
+ if ( !info.getExportedPackages().isEmpty() ) {
exportingBundles.add(info);
}
}
// check importing bundles
for(final BundleDescriptor info : entry.getValue()) {
- if ( info.getImportedPackages() != null ) {
- for(final PackageInfo pck : info.getImportedPackages() ) {
- final Map<BundleDescriptor, Set<String>> candidates =
- getCandidates(exportingBundles, pck, info, apiRegions, ignoreAPIRegions);
- if ( candidates.isEmpty() ) {
+ for(final PackageInfo pck : info.getImportedPackages() ) {
+ final Map<BundleDescriptor, Set<String>> candidates =
+ getCandidates(exportingBundles, pck, info, apiRegions, ignoreAPIRegions);
+ if ( candidates.isEmpty() ) {
+ if ( pck.isOptional() ) {
+ getReport(reports, info).missingExportsForOptional.add(pck);
+ } else {
+ getReport(reports, info).missingExports.add(pck);
+ }
+ } else {
+ final List<BundleDescriptor> matchingCandidates = new ArrayList<>();
+
+ Set<String> exportingRegions = new HashSet<>();
+ Set<String> importingRegions = new HashSet<>();
+ for(final Map.Entry<BundleDescriptor, Set<String>> candidate : candidates.entrySet()) {
+ BundleDescriptor bd = candidate.getKey();
+ if (bd.isExportingPackage(pck)) {
+ Set<String> exRegions = candidate.getValue();
+ if (exRegions.contains(NO_REGION)) {
+ // If an export is defined outside of a region, it always matches
+ matchingCandidates.add(bd);
+ continue;
+ }
+ if (exRegions.contains(GLOBAL_REGION)) {
+ // Everyone can import from the global regin
+ matchingCandidates.add(bd);
+ continue;
+ }
+ if (exRegions.contains(OWN_FEATURE)) {
+ // A feature can always import packages from bundles in itself
+ matchingCandidates.add(bd);
+ continue;
+ }
+
+ // Find out what regions the importing bundle is in
+ Set<String> imRegions =
+ getBundleRegions(info, apiRegions, ignoreAPIRegions);
+
+ // Record the exporting and importing regions for diagnostics
+ exportingRegions.addAll(exRegions);
+
+ Set<String> regions = new HashSet<>();
+ for (String region : imRegions) {
+ for (ApiRegion r = apiRegions.getRegionByName(region); r != null; r = r.getParent()) {
+ regions.add(r.getName());
+ }
+ }
+
+ importingRegions.addAll(regions);
+
+ // Only keep the regions that also export the package
+ regions.retainAll(exRegions);
+
+ if (!regions.isEmpty()) {
+ // there is an overlapping region
+ matchingCandidates.add(bd);
+ }
+ }
+ }
+
+ if ( matchingCandidates.isEmpty() ) {
if ( pck.isOptional() ) {
getReport(reports, info).missingExportsForOptional.add(pck);
} else {
- getReport(reports, info).missingExports.add(pck);
+ getReport(reports, info).missingExportsWithVersion.add(pck);
+ getReport(reports, info).regionInfo.put(pck, new AbstractMap.SimpleEntry<>(exportingRegions, importingRegions));
}
- } else {
- final List<BundleDescriptor> matchingCandidates = new ArrayList<>();
-
- Set<String> exportingRegions = new HashSet<>();
- Set<String> importingRegions = new HashSet<>();
- for(final Map.Entry<BundleDescriptor, Set<String>> candidate : candidates.entrySet()) {
- BundleDescriptor bd = candidate.getKey();
- if (bd.isExportingPackage(pck)) {
- Set<String> exRegions = candidate.getValue();
- if (exRegions.contains(NO_REGION)) {
- // If an export is defined outside of a region, it always matches
- matchingCandidates.add(bd);
- continue;
- }
- if (exRegions.contains(GLOBAL_REGION)) {
- // Everyone can import from the global regin
- matchingCandidates.add(bd);
- continue;
- }
- if (exRegions.contains(OWN_FEATURE)) {
- // A feature can always import packages from bundles in itself
- matchingCandidates.add(bd);
- continue;
- }
-
- // Find out what regions the importing bundle is in
- Set<String> imRegions =
- getBundleRegions(info, apiRegions, ignoreAPIRegions);
-
- // Record the exporting and importing regions for diagnostics
- exportingRegions.addAll(exRegions);
-
- Set<String> regions = new HashSet<>();
- for (String region : imRegions) {
- for (ApiRegion r = apiRegions.getRegionByName(region); r != null; r = r.getParent()) {
- regions.add(r.getName());
- }
- }
-
- importingRegions.addAll(regions);
-
- // Only keep the regions that also export the package
- regions.retainAll(exRegions);
-
- if (!regions.isEmpty()) {
- // there is an overlapping region
- matchingCandidates.add(bd);
- }
- }
- }
-
- if ( matchingCandidates.isEmpty() ) {
- if ( pck.isOptional() ) {
- getReport(reports, info).missingExportsForOptional.add(pck);
- } else {
- getReport(reports, info).missingExportsWithVersion.add(pck);
- getReport(reports, info).regionInfo.put(pck, new AbstractMap.SimpleEntry<>(exportingRegions, importingRegions));
- }
- } else if ( matchingCandidates.size() > 1 ) {
- getReport(reports, info).exportMatchingSeveral.add(pck);
- }
+ } else if ( matchingCandidates.size() > 1 ) {
+ getReport(reports, info).exportMatchingSeveral.add(pck);
}
}
}
@@ -311,11 +288,7 @@
|| apiRegions.getRegionByName(region).getAllExportByName(pck.getName()) == null))
continue;
- Set<String> regions = candidates.get(info);
- if (regions == null) {
- regions = new HashSet<>();
- candidates.put(info, regions);
- }
+ final Set<String> regions = candidates.computeIfAbsent(info, key -> new HashSet<>());
regions.add(region);
}
}
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackages.java b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackages.java
index affe8cd..cda708d 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackages.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackages.java
@@ -18,6 +18,10 @@
*/
package org.apache.sling.feature.extension.apiregions.analyser;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
import org.apache.sling.feature.analyser.task.AnalyserTask;
import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
import org.apache.sling.feature.extension.apiregions.api.ApiRegions;
@@ -25,7 +29,6 @@
import org.apache.sling.feature.scanner.PackageInfo;
import org.osgi.framework.Version;
-import java.util.*;
public class CheckApiRegionsBundleUnversionedPackages implements AnalyserTask {
@@ -42,35 +45,6 @@
return ApiRegions.EXTENSION_NAME + "-unversioned-packages";
}
- public static final class Report {
-
- public List<PackageInfo> exportWithoutVersion = new ArrayList<>();
-
- public List<PackageInfo> importWithoutVersion = new ArrayList<>();
-
- }
-
- private Report getReport(final Map<BundleDescriptor, Report> reports, final BundleDescriptor info) {
- Report report = reports.get(info);
- if ( report == null ) {
- report = new Report();
- reports.put(info, report);
- }
- return report;
- }
-
- private void checkForVersionOnExportedPackages(final AnalyserTaskContext ctx, final Map<BundleDescriptor, Report> reports) {
- for(final BundleDescriptor info : ctx.getFeatureDescriptor().getBundleDescriptors()) {
- if ( info.getExportedPackages() != null ) {
- for(final PackageInfo i : info.getExportedPackages()) {
- if ( i.getPackageVersion().compareTo(Version.emptyVersion) == 0 ) {
- getReport(reports, info).exportWithoutVersion.add(i);
- }
- }
- }
- }
- }
-
private boolean ignoreImportPackage(final String name) {
for(final String prefix : IGNORED_IMPORT_PREFIXES) {
if ( name.startsWith(prefix) ) {
@@ -80,43 +54,43 @@
return false;
}
- private void checkForVersionOnImportingPackages(final AnalyserTaskContext ctx, final Map<BundleDescriptor, Report> reports) {
- for(final BundleDescriptor info : ctx.getFeatureDescriptor().getBundleDescriptors()) {
- if ( info.getImportedPackages() != null ) {
- for(final PackageInfo i : info.getImportedPackages()) {
- if ( i.getVersion() == null && !ignoreImportPackage(i.getName()) ) {
- getReport(reports, info).importWithoutVersion.add(i);
- }
- }
- }
- }
- }
-
@Override
public void execute(final AnalyserTaskContext ctx) throws Exception {
- final Map<BundleDescriptor, Report> reports = new HashMap<>();
- checkForVersionOnExportedPackages(ctx, reports);
- checkForVersionOnImportingPackages(ctx, reports);
+ for(final BundleDescriptor info : ctx.getFeatureDescriptor().getBundleDescriptors()) {
- for(final Map.Entry<BundleDescriptor, Report> entry : reports.entrySet()) {
- final String key = "Bundle " + entry.getKey().getArtifact().getId().getArtifactId() + ":" + entry.getKey().getArtifact().getId().getVersion();
-
- if ( !entry.getValue().importWithoutVersion.isEmpty() ) {
- ctx.reportArtifactWarning(entry.getKey().getArtifact().getId(),
- key + " is importing package(s) " + getPackageInfo(entry.getValue().importWithoutVersion) + " without specifying a version range.");
+
+ final List<PackageInfo> exportWithoutVersion = new ArrayList<>();
+ for(final PackageInfo i : info.getExportedPackages()) {
+ if ( i.getPackageVersion().compareTo(Version.emptyVersion) == 0 ) {
+ exportWithoutVersion.add(i);
+ }
}
- if ( !entry.getValue().exportWithoutVersion.isEmpty() ) {
- ctx.reportArtifactWarning(entry.getKey().getArtifact().getId(),
- key + " is exporting package(s) " + getPackageInfo(entry.getValue().exportWithoutVersion) + " without a version.");
+ final List<PackageInfo> importWithoutVersion = new ArrayList<>();
+ for(final PackageInfo i : info.getImportedPackages()) {
+ if ( i.getVersion() == null && !ignoreImportPackage(i.getName()) ) {
+ importWithoutVersion.add(i);
+ }
+ }
+
+ final String key = "Bundle ".concat(info.getArtifact().getId().getArtifactId())
+ .concat(":").concat(info.getArtifact().getId().getVersion());
+
+ if ( !importWithoutVersion.isEmpty() ) {
+ ctx.reportArtifactWarning(info.getArtifact().getId(),
+ key.concat(" is importing ").concat(getPackageInfo(importWithoutVersion)).concat(" without specifying a version range."));
+ }
+ if ( !exportWithoutVersion.isEmpty() ) {
+ ctx.reportArtifactWarning(info.getArtifact().getId(),
+ key.concat(" is exporting ").concat(getPackageInfo(exportWithoutVersion)).concat(" without a version."));
}
}
}
private String getPackageInfo(final List<PackageInfo> pcks) {
if ( pcks.size() == 1 ) {
- return pcks.get(0).getName();
+ return "package ".concat(pcks.get(0).getName());
}
- final StringBuilder sb = new StringBuilder();
+ final StringBuilder sb = new StringBuilder("packages ");
boolean first = true;
sb.append('[');
for(final PackageInfo info : pcks) {
@@ -130,5 +104,4 @@
sb.append(']');
return sb.toString();
}
-
}
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackagesTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackagesTest.java
index 37747c1..ff4695c 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackagesTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleUnversionedPackagesTest.java
@@ -31,10 +31,6 @@
import java.io.File;
import java.io.IOException;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-
import static org.junit.Assert.assertEquals;
public class CheckApiRegionsBundleUnversionedPackagesTest {
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckArtifactRulesTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckArtifactRulesTest.java
index 4689d91..96814e1 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckArtifactRulesTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckArtifactRulesTest.java
@@ -18,11 +18,6 @@
import static org.mockito.Mockito.when;
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
import org.apache.sling.feature.Artifact;
import org.apache.sling.feature.ArtifactId;
import org.apache.sling.feature.Extension;
