This is an automated email from the ASF dual-hosted git repository.
cziegeler pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-extension-apiregions.git
The following commit(s) were added to refs/heads/master by this push:
new 784cf4a SLING-11068 split up analyser task for unversioned packages -
clean up code and use newer apis
784cf4a is described below
commit 784cf4ae4789828c0c10f6b5854f682cf0765f18
Author: Carsten Ziegeler <[email protected]>
AuthorDate: Tue Jan 18 07:24:02 2022 +0100
SLING-11068 split up analyser task for unversioned packages - clean up code
and use newer apis
---
.../CheckApiRegionsBundleExportsImports.java | 167 +++++++++------------
.../CheckApiRegionsBundleUnversionedPackages.java | 83 ++++------
...eckApiRegionsBundleUnversionedPackagesTest.java | 4 -
.../analyser/CheckArtifactRulesTest.java | 5 -
4 files changed, 98 insertions(+), 161 deletions(-)
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.SortedMap;
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 @@ public class CheckApiRegionsBundleExportsImports implements
AnalyserTask {
}
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 @@ public class CheckApiRegionsBundleExportsImports implements
AnalyserTask {
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 @@ public class CheckApiRegionsBundleExportsImports
implements AnalyserTask {
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() ) {
- if ( pck.isOptional() ) {
- getReport(reports,
info).missingExportsForOptional.add(pck);
- } else {
- getReport(reports,
info).missingExports.add(pck);
- }
+ 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 {
- 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;
- }
+ 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);
+ // 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);
+ // 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());
- }
+ 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);
+ importingRegions.addAll(regions);
- // Only keep the regions that also export
the package
- regions.retainAll(exRegions);
+ // Only keep the regions that also export the
package
+ regions.retainAll(exRegions);
- if (!regions.isEmpty()) {
- // there is an overlapping region
- matchingCandidates.add(bd);
- }
+ 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);
+ 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);
}
}
}
@@ -311,11 +288,7 @@ public class CheckApiRegionsBundleExportsImports
implements AnalyserTask {
||
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.BundleDescriptor;
import org.apache.sling.feature.scanner.PackageInfo;
import org.osgi.framework.Version;
-import java.util.*;
public class CheckApiRegionsBundleUnversionedPackages implements AnalyserTask {
@@ -42,35 +45,6 @@ public class CheckApiRegionsBundleUnversionedPackages
implements AnalyserTask {
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 @@ public class CheckApiRegionsBundleUnversionedPackages
implements AnalyserTask {
return false;
}
- private void checkForVersionOnImportingPackages(final AnalyserTaskContext
ctx, final Map<BundleDescriptor, Report> reports) {
+ @Override
+ public void execute(final AnalyserTaskContext ctx) throws Exception {
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);
- }
+
+
+ final List<PackageInfo> exportWithoutVersion = new ArrayList<>();
+ for(final PackageInfo i : info.getExportedPackages()) {
+ if ( i.getPackageVersion().compareTo(Version.emptyVersion) ==
0 ) {
+ exportWithoutVersion.add(i);
+ }
+ }
+ final List<PackageInfo> importWithoutVersion = new ArrayList<>();
+ for(final PackageInfo i : info.getImportedPackages()) {
+ if ( i.getVersion() == null &&
!ignoreImportPackage(i.getName()) ) {
+ 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 Map.Entry<BundleDescriptor, Report> entry :
reports.entrySet()) {
- final String key = "Bundle " +
entry.getKey().getArtifact().getId().getArtifactId() + ":" +
entry.getKey().getArtifact().getId().getVersion();
+ final String key = "Bundle
".concat(info.getArtifact().getId().getArtifactId())
+ .concat(":").concat(info.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.");
+ if ( !importWithoutVersion.isEmpty() ) {
+ ctx.reportArtifactWarning(info.getArtifact().getId(),
+ key.concat(" is importing
").concat(getPackageInfo(importWithoutVersion)).concat(" without specifying a
version range."));
}
- if ( !entry.getValue().exportWithoutVersion.isEmpty() ) {
- ctx.reportArtifactWarning(entry.getKey().getArtifact().getId(),
- key + " is exporting package(s) " +
getPackageInfo(entry.getValue().exportWithoutVersion) + " without a version.");
+ 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 @@ public class CheckApiRegionsBundleUnversionedPackages
implements AnalyserTask {
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 org.mockito.Mockito;
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 @@ package
org.apache.sling.feature.extension.apiregions.analyser;
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;
