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-analyser.git
The following commit(s) were added to refs/heads/master by this push: new 6fe22b6 SLING-8169 Bundle Import/Export Analyser needs to take API Regions into account 6fe22b6 is described below commit 6fe22b6f0e14408a54ef18cf820f72904215a7e6 Author: David Bosschaert <bossc...@adobe.com> AuthorDate: Thu Jan 17 14:27:46 2019 +0000 SLING-8169 Bundle Import/Export Analyser needs to take API Regions into account Make it possible to configure the analyzer to ignore API Regions, this is done with the following task configuration: <taskConfiguration> <bundle-packages> <ignoreAPIRegions>true</ignoreAPIRegions> </bundle-packages> </taskConfiguration> The default value is false. --- .../task/impl/CheckBundleExportsImports.java | 28 +++++++----- .../task/impl/CheckBundleExportsImportsTest.java | 50 ++++++++++++++++++++-- .../f_f_1/bundleOrigins.properties | 2 + .../f_f_1/regionOrigins.properties | 2 + 4 files changed, 69 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImports.java b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImports.java index 512cc10..1ea47db 100644 --- a/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImports.java +++ b/src/main/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImports.java @@ -46,12 +46,8 @@ import java.util.TreeMap; import java.util.stream.Collectors; public class CheckBundleExportsImports implements AnalyserTask { - /** - * Ignore API Region information files being absent. - */ - private static final String SKIP_API_REGION_FILE_ERRORS_SYSTEM_PROPERTY = "skipAPIRegionFileErrors"; - private static final String FILE_STORAGE_CONFIG_KEY = "fileStorage"; + private static final String IGNORE_API_REGIONS_CONFIG_KEY = "ignoreAPIRegions"; private static final String API_REGIONS = "api-regions"; private static final String GLOBAL_REGION = "global"; private static final String NO_REGION = " __NO_REGION__ "; @@ -123,6 +119,9 @@ public class CheckBundleExportsImports implements AnalyserTask { @Override public void execute(final AnalyserTaskContext ctx) throws IOException { + boolean ignoreAPIRegions = ctx.getConfiguration().getOrDefault( + IGNORE_API_REGIONS_CONFIG_KEY, "false").equalsIgnoreCase("true"); + // basic checks final Map<BundleDescriptor, Report> reports = new HashMap<>(); checkForVersionOnExportedPackages(ctx, reports); @@ -144,9 +143,18 @@ public class CheckBundleExportsImports implements AnalyserTask { exportingBundles.add(ctx.getFrameworkDescriptor()); } - ApiRegions apiRegions = readAPIRegionsFromFeature(ctx); - Map<String, Set<String>> bundleToOriginalFeatures = readBundleOrigins(ctx); - Map<String, Set<String>> featureToOriginalRegions = readRegionOrigins(ctx); + ApiRegions apiRegions; + Map<String, Set<String>> bundleToOriginalFeatures; + Map<String, Set<String>> featureToOriginalRegions; + if (ignoreAPIRegions) { + apiRegions = new ApiRegions(); // Empty API Regions + bundleToOriginalFeatures = Collections.emptyMap(); + featureToOriginalRegions = Collections.emptyMap(); + } else { + apiRegions = readAPIRegionsFromFeature(ctx); + bundleToOriginalFeatures = readBundleOrigins(ctx); + featureToOriginalRegions = readRegionOrigins(ctx); + } for(final Map.Entry<Integer, List<BundleDescriptor>> entry : bundlesMap.entrySet()) { // first add all exporting bundles @@ -377,7 +385,7 @@ public class CheckBundleExportsImports implements AnalyserTask { String fileStorage = ctx.getConfiguration().get(FILE_STORAGE_CONFIG_KEY); if (fileStorage == null) { - if (APIRegionExtension != null && System.getProperty(SKIP_API_REGION_FILE_ERRORS_SYSTEM_PROPERTY) == null) { + if (APIRegionExtension != null) { throw new IllegalStateException("Feature " + feature.getId() + " has API regions defined, but no storage is configured for origin information files. " + "Please configure the " + FILE_STORAGE_CONFIG_KEY + " configuration item."); @@ -388,7 +396,7 @@ public class CheckBundleExportsImports implements AnalyserTask { String featureName = feature.getId().toMvnId().replaceAll("[^a-zA-Z0-9\\.\\-]", "_"); File file = new File(fileStorage, featureName + File.separator + fileName); if (!file.exists()) { - if (APIRegionExtension != null && System.getProperty(SKIP_API_REGION_FILE_ERRORS_SYSTEM_PROPERTY) == null) + if (APIRegionExtension != null) throw new IllegalStateException("Feature " + feature.getId() + " has API regions defined but no file with origin information can be found " + file + " Configure the org.apache.sling.feature.extension.apiregions appropriately to write this information"); diff --git a/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImportsTest.java b/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImportsTest.java index 6e7967c..59f01b5 100644 --- a/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImportsTest.java +++ b/src/test/java/org/apache/sling/feature/analyser/task/impl/CheckBundleExportsImportsTest.java @@ -34,6 +34,8 @@ 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; @@ -165,12 +167,14 @@ public class CheckBundleExportsImportsTest { fdAddBundle(fd, "g:b1:1", "test-bundle1.jar"); fdAddBundle(fd, "g:b2:1", "test-bundle2.jar"); + Map<String, String> cfgMap = new HashMap<String, String>(); + cfgMap.put("fileStorage", resourceRoot + "/origins/testImportExportWithRegionMismatch"); + cfgMap.put("ignoreAPIRegions", "false"); + AnalyserTaskContext ctx = Mockito.mock(AnalyserTaskContext.class); Mockito.when(ctx.getFeature()).thenReturn(f); Mockito.when(ctx.getFeatureDescriptor()).thenReturn(fd); - Mockito.when(ctx.getConfiguration()).thenReturn( - Collections.singletonMap("fileStorage", - resourceRoot + "/origins/testImportExportWithRegionMismatch")); + Mockito.when(ctx.getConfiguration()).thenReturn(cfgMap); t.execute(ctx); Mockito.verify(ctx).reportError(Mockito.contains("org.foo.b")); @@ -182,6 +186,46 @@ public class CheckBundleExportsImportsTest { @Test /* + * Bundle 2 imports org.foo.b from bundle 1, but bundle 1 exports it in a different + * region, bundle 1 is in something region, and bundle 2 is in somethingelse region. + * However this should still pass as the analyzer is configured to ignore regions. + */ + public void testImportExportWithRegionMismatchIgnoreRegions() throws Exception { + String exJson = "[{\"name\": \"something\", \"exports\": [\"org.foo.b\"]}]"; + + CheckBundleExportsImports t = new CheckBundleExportsImports(); + + Feature f = new Feature(ArtifactId.fromMvnId("f:f:1")); + Extension ex = new Extension(ExtensionType.JSON, "api-regions", false); + ex.setJSON(exJson); + f.getExtensions().add(ex); + + FeatureDescriptor fd = new FeatureDescriptor() { + @Override + public Feature getFeature() { + return f; + } + }; + + fdAddBundle(fd, "g:b1:1", "test-bundle1.jar"); + fdAddBundle(fd, "g:b2:1", "test-bundle2.jar"); + + Map<String, String> cfgMap = new HashMap<String, String>(); + cfgMap.put("fileStorage", resourceRoot + "/origins/testImportExportWithRegionMismatch"); + cfgMap.put("ignoreAPIRegions", "true"); + + AnalyserTaskContext ctx = Mockito.mock(AnalyserTaskContext.class); + Mockito.when(ctx.getFeature()).thenReturn(f); + Mockito.when(ctx.getFeatureDescriptor()).thenReturn(fd); + Mockito.when(ctx.getConfiguration()).thenReturn(cfgMap); + t.execute(ctx); + + Mockito.verify(ctx, Mockito.never()).reportError(Mockito.anyString()); + Mockito.verify(ctx, Mockito.never()).reportWarning(Mockito.anyString()); + } + + @Test + /* * Bundle 3 imports org.foo.a from Bundle 1 and org.foo.e from Bundle 4. * The Feature is in a region called 'blah' which exports nothing, but because * all these bundles are in the same feature they can all see each other. diff --git a/src/test/resources/origins/testImportExportWithRegionMismatchIgnoreRegions/f_f_1/bundleOrigins.properties b/src/test/resources/origins/testImportExportWithRegionMismatchIgnoreRegions/f_f_1/bundleOrigins.properties new file mode 100644 index 0000000..788c4fa --- /dev/null +++ b/src/test/resources/origins/testImportExportWithRegionMismatchIgnoreRegions/f_f_1/bundleOrigins.properties @@ -0,0 +1,2 @@ +g\:b1\:1=f\:f1\:1 +g\:b2\:1=f\:f2\:1 diff --git a/src/test/resources/origins/testImportExportWithRegionMismatchIgnoreRegions/f_f_1/regionOrigins.properties b/src/test/resources/origins/testImportExportWithRegionMismatchIgnoreRegions/f_f_1/regionOrigins.properties new file mode 100644 index 0000000..3134d99 --- /dev/null +++ b/src/test/resources/origins/testImportExportWithRegionMismatchIgnoreRegions/f_f_1/regionOrigins.properties @@ -0,0 +1,2 @@ +f\:f1\:1=something,someotherthing +f\:f2\:1=somethingelse