You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by da...@apache.org on 2020/11/13 13:15:37 UTC
[sling-org-apache-sling-feature-extension-apiregions] branch master
updated: SLING-9897 Update API Regions Analysers to use artifact-aware
error reporting
This is an automated email from the ASF dual-hosted git repository.
davidb 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 9687ce6 SLING-9897 Update API Regions Analysers to use artifact-aware error reporting
new a8d2c8c Merge pull request #11 from bosschaert/SLING-9897
9687ce6 is described below
commit 9687ce6164ea8cbcd2eb82ae93d95b5e68e507c8
Author: David Bosschaert <bo...@adobe.com>
AuthorDate: Wed Nov 11 16:05:48 2020 +0000
SLING-9897 Update API Regions Analysers to use artifact-aware error reporting
Also updated some error messages to be a little easier to understand.
---
pom.xml | 2 +-
.../apiregions/analyser/CheckApiRegions.java | 2 +-
.../CheckApiRegionsBundleExportsImports.java | 11 +++---
.../analyser/CheckApiRegionsCrossFeatureDups.java | 4 +--
.../analyser/CheckApiRegionsDependencies.java | 11 +++---
.../AbstractApiRegionsAnalyserTaskTest.java | 6 ++++
.../CheckApiRegionsBundleExportsImportsTest.java | 39 ++++++++++++++++------
.../analyser/CheckApiRegionsDependenciesTest.java | 4 +--
8 files changed, 51 insertions(+), 28 deletions(-)
diff --git a/pom.xml b/pom.xml
index 2a337b4..5396cbd 100644
--- a/pom.xml
+++ b/pom.xml
@@ -62,7 +62,7 @@
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.feature.analyser</artifactId>
- <version>1.3.8</version>
+ <version>1.3.10</version>
<scope>provided</scope>
</dependency>
<dependency>
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegions.java b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegions.java
index 088e105..84ca66b 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegions.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegions.java
@@ -36,7 +36,7 @@ public class CheckApiRegions extends AbstractApiRegionsAnalyserTask {
@Override
public String getName() {
- return "Api Regions analyser task";
+ return "Api Regions analyser task that checks that listed packages are actually exported";
}
@Override
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 e3ca7cb..39d64bb 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
@@ -250,14 +250,17 @@ public class CheckApiRegionsBundleExportsImports implements AnalyserTask {
final String key = "Bundle " + entry.getKey().getArtifact().getId().getArtifactId() + ":" + entry.getKey().getArtifact().getId().getVersion();
if ( !entry.getValue().importWithoutVersion.isEmpty() ) {
- ctx.reportWarning(key + " is importing package(s) " + getPackageInfo(entry.getValue().importWithoutVersion, false) + " without specifying a version range.");
+ ctx.reportArtifactWarning(entry.getKey().getArtifact().getId(),
+ key + " is importing package(s) " + getPackageInfo(entry.getValue().importWithoutVersion, false) + " without specifying a version range.");
}
if ( !entry.getValue().exportWithoutVersion.isEmpty() ) {
- ctx.reportWarning(key + " is exporting package(s) " + getPackageInfo(entry.getValue().importWithoutVersion, false) + " without a version.");
+ ctx.reportArtifactWarning(entry.getKey().getArtifact().getId(),
+ key + " is exporting package(s) " + getPackageInfo(entry.getValue().importWithoutVersion, false) + " without a version.");
}
if ( !entry.getValue().missingExports.isEmpty() ) {
- ctx.reportError(key + " is importing package(s) " + getPackageInfo(entry.getValue().missingExports, false) + " in start level " +
+ ctx.reportArtifactError(entry.getKey().getArtifact().getId(),
+ key + " is importing package(s) " + getPackageInfo(entry.getValue().missingExports, false) + " in start level " +
String.valueOf(entry.getKey().getArtifact().getStartOrder())
+ " but no bundle is exporting these for that start level.");
errorReported = true;
@@ -274,7 +277,7 @@ public class CheckApiRegionsBundleExportsImports implements AnalyserTask {
message.append("\n" + pkg.getName() + " is exported in regions " + regions.getKey() + " but it is imported in regions " + regions.getValue());
}
}
- ctx.reportError(message.toString());
+ ctx.reportArtifactError(entry.getKey().getArtifact().getId(), message.toString());
errorReported = true;
}
}
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
index 7c9c871..4c0c8af 100644
--- 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
@@ -91,9 +91,9 @@ public class CheckApiRegionsCrossFeatureDups extends AbstractApiRegionsAnalyserT
+ " which comes from a feature without API Regions: " + borgs
+ ". Both export package: " + pi.getName();
if (matchesSet(pkgName, warningPackages)) {
- ctx.reportWarning(msg);
+ ctx.reportArtifactWarning(bd.getArtifact().getId(), msg);
} else {
- ctx.reportError(msg);
+ ctx.reportArtifactError(bd.getArtifact().getId(), msg);
}
}
}
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsDependencies.java b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsDependencies.java
index fe65540..0987b97 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsDependencies.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsDependencies.java
@@ -22,7 +22,6 @@ 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 org.osgi.framework.Constants;
public class CheckApiRegionsDependencies extends AbstractApiRegionsAnalyserTask {
@@ -55,27 +54,25 @@ public class CheckApiRegionsDependencies extends AbstractApiRegionsAnalyserTask
if (exportingApisName.getExportByName(exportedPackage) != null) {
if (hidingApisName.getExportByName(exportedPackage) != null) {
String errorMessage = String.format(
- "Bundle '%s' (defined in feature '%s') declares '%s' in the '%s' header that is enlisted in both exporting '%s' and hiding '%s' APIs regions, please adjust Feature settings",
+ "Bundle '%s' (defined in feature '%s') exports package '%s' that is declared in both visible '%s' and non-visible '%s' APIs regions",
bundleDescriptor.getArtifact().getId(),
ctx.getFeature().getId(),
exportedPackage,
- Constants.EXPORT_PACKAGE,
exportingApisName.getName(),
hidingApisName.getName());
- ctx.reportError(errorMessage);
+ ctx.reportArtifactError(bundleDescriptor.getArtifact().getId(), errorMessage);
} else {
for (String uses : packageInfo.getUses()) {
if (hidingApisName.getExportByName(uses) != null) {
String errorMessage = String.format(
- "Bundle '%s' (defined in feature '%s') declares '%s' in the '%s' header, enlisted in the '%s' region, which uses '%s' package that is in the '%s' region",
+ "Bundle '%s' (defined in feature '%s') exports package '%s' that is declared in the visible '%s' region, which uses package '%s' that is in the non-visible '%s' region",
bundleDescriptor.getArtifact().getId(),
ctx.getFeature().getId(),
exportedPackage,
- Constants.EXPORT_PACKAGE,
exportingApisName.getName(),
uses,
hidingApisName.getName());
- ctx.reportError(errorMessage);
+ ctx.reportArtifactError(bundleDescriptor.getArtifact().getId(), errorMessage);
}
}
}
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTaskTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTaskTest.java
index ecee258..ba362d0 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTaskTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTaskTest.java
@@ -49,6 +49,7 @@ import org.mockito.stubbing.Answer;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
@@ -155,6 +156,11 @@ public abstract class AbstractApiRegionsAnalyserTaskTest<T extends AbstractApiRe
errors.add(error);
return null;
}).when(ctx).reportError(anyString());
+ doAnswer(invocation -> {
+ String error = invocation.getArgument(1);
+ errors.add(error);
+ return null;
+ }).when(ctx).reportArtifactError(any(), anyString());
analyserTask.execute(ctx);
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleExportsImportsTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleExportsImportsTest.java
index 3801cc7..1322b41 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleExportsImportsTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleExportsImportsTest.java
@@ -39,6 +39,7 @@ import org.apache.sling.feature.scanner.impl.FeatureDescriptorImpl;
import org.junit.BeforeClass;
import org.junit.Test;
import org.mockito.Mockito;
+
import static org.junit.Assert.assertEquals;
public class CheckApiRegionsBundleExportsImportsTest {
@@ -76,8 +77,12 @@ public class CheckApiRegionsBundleExportsImportsTest {
Mockito.when(ctx.getFeatureDescriptor()).thenReturn(fd);
t.execute(ctx);
- Mockito.verify(ctx, Mockito.times(2)).reportError(Mockito.anyString());
- Mockito.verify(ctx).reportError(Mockito.contains("org.foo.e"));
+ Mockito.verify(ctx, Mockito.times(1)).reportArtifactError(Mockito.any(), Mockito.anyString());
+ Mockito.verify(ctx, Mockito.times(1)).reportError(Mockito.anyString());
+
+ Mockito.verify(ctx).reportArtifactError(
+ Mockito.eq(ArtifactId.fromMvnId("g:b3:1")),
+ Mockito.contains("org.foo.e"));
Mockito.verify(ctx).reportError(Mockito.contains("marked as 'complete'"));
Mockito.verify(ctx, Mockito.never()).reportWarning(Mockito.anyString());
}
@@ -119,9 +124,13 @@ public class CheckApiRegionsBundleExportsImportsTest {
Mockito.when(ctx.getFeatureDescriptor()).thenReturn(fd);
t.execute(ctx);
- Mockito.verify(ctx).reportError(Mockito.contains("org.foo.e"));
- Mockito.verify(ctx, Mockito.times(1)).reportError(Mockito.anyString());
+ Mockito.verify(ctx).reportArtifactError(
+ Mockito.eq(ArtifactId.fromMvnId("g:b3:1")),
+ Mockito.contains("org.foo.e"));
+ Mockito.verify(ctx, Mockito.times(1)).reportArtifactError(Mockito.any(), Mockito.anyString());
+ Mockito.verify(ctx, Mockito.never()).reportError(Mockito.anyString());
Mockito.verify(ctx, Mockito.never()).reportWarning(Mockito.anyString());
+ Mockito.verify(ctx, Mockito.never()).reportArtifactWarning(Mockito.any(), Mockito.anyString());
}
@Test
@@ -149,9 +158,9 @@ public class CheckApiRegionsBundleExportsImportsTest {
Mockito.when(ctx.getFeatureDescriptor()).thenReturn(fd);
t.execute(ctx);
- Mockito.verify(ctx).reportError(Mockito.contains("org.foo.b"));
- Mockito.verify(ctx, Mockito.times(1)).reportError(Mockito.anyString());
- Mockito.verify(ctx, Mockito.never()).reportWarning(Mockito.anyString());
+ Mockito.verify(ctx).reportArtifactError(Mockito.eq(ArtifactId.fromMvnId("g:b2:1")), Mockito.contains("org.foo.b"));
+ Mockito.verify(ctx, Mockito.times(1)).reportArtifactError(Mockito.any(), Mockito.anyString());
+ Mockito.verify(ctx, Mockito.never()).reportArtifactWarning(Mockito.any(), Mockito.anyString());
}
@Test
@@ -188,10 +197,18 @@ public class CheckApiRegionsBundleExportsImportsTest {
Mockito.when(ctx.getConfiguration()).thenReturn(cfgMap);
t.execute(ctx);
- Mockito.verify(ctx).reportError(Mockito.contains("org.foo.b"));
- Mockito.verify(ctx).reportError(Mockito.contains("something"));
- Mockito.verify(ctx).reportError(Mockito.contains("somethingelse"));
- Mockito.verify(ctx, Mockito.times(1)).reportError(Mockito.anyString());
+ Mockito.verify(ctx).reportArtifactError(
+ Mockito.eq(ArtifactId.fromMvnId("g:b2:1")),
+ Mockito.contains("org.foo.b"));
+ Mockito.verify(ctx).reportArtifactError(
+ Mockito.eq(ArtifactId.fromMvnId("g:b2:1")),
+ Mockito.contains("something"));
+ Mockito.verify(ctx).reportArtifactError(
+ Mockito.eq(ArtifactId.fromMvnId("g:b2:1")),
+ Mockito.contains("somethingelse"));
+ Mockito.verify(ctx, Mockito.times(1)).reportArtifactError(Mockito.any(), Mockito.anyString());
+ Mockito.verify(ctx, Mockito.never()).reportArtifactWarning(Mockito.any(), Mockito.anyString());
+ Mockito.verify(ctx, Mockito.never()).reportError(Mockito.anyString());
Mockito.verify(ctx, Mockito.never()).reportWarning(Mockito.anyString());
}
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsDependenciesTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsDependenciesTest.java
index bde744d..51def05 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsDependenciesTest.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsDependenciesTest.java
@@ -36,7 +36,7 @@ public class CheckApiRegionsDependenciesTest extends AbstractApiRegionsAnalyserT
assertFalse(errors.isEmpty());
assertEquals(
- "Bundle 'org.osgi:org.osgi.util.function:1.0.0' (defined in feature 'org.apache.sling.testing:org.apache.sling.testing.apiregions:1.0.0') declares 'org.osgi.util.function' in the 'Export-Package' header, enlisted in the 'global' region, which uses 'org.objectweb.asm' package that is in the 'deprecated' region",
+ "Bundle 'org.osgi:org.osgi.util.function:1.0.0' (defined in feature 'org.apache.sling.testing:org.apache.sling.testing.apiregions:1.0.0') exports package 'org.osgi.util.function' that is declared in the visible 'global' region, which uses package 'org.objectweb.asm' that is in the non-visible 'deprecated' region",
errors.iterator().next()
);
}
@@ -47,7 +47,7 @@ public class CheckApiRegionsDependenciesTest extends AbstractApiRegionsAnalyserT
assertFalse(errors.isEmpty());
assertEquals(
- "Bundle 'org.osgi:org.osgi.util.function:1.0.0' (defined in feature 'org.apache.sling.testing:org.apache.sling.testing.apiregions:1.0.0') declares 'org.osgi.util.function' in the 'Export-Package' header that is enlisted in both exporting 'global' and hiding 'deprecated' APIs regions, please adjust Feature settings",
+ "Bundle 'org.osgi:org.osgi.util.function:1.0.0' (defined in feature 'org.apache.sling.testing:org.apache.sling.testing.apiregions:1.0.0') exports package 'org.osgi.util.function' that is declared in both visible 'global' and non-visible 'deprecated' APIs regions",
errors.iterator().next());
}