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 63e8239 SLING-8169 Bundle Import/Export Analyser needs to take API Regions into account 63e8239 is described below commit 63e8239c7941399a3e80d97e550d543c4fba0df5 Author: David Bosschaert <bossc...@adobe.com> AuthorDate: Wed Jan 16 16:54:38 2019 +0000 SLING-8169 Bundle Import/Export Analyser needs to take API Regions into account Store bundle origins in a file called bundleOrigins.properties and the region origins in a file called regionOrigins.properties. The Analyser can use this input to check imports/exports wrt API regions. --- pom.xml | 6 + .../apiregions/APIRegionMergeHandler.java | 78 +++++++++++++ .../extension/apiregions/AbstractHandler.java | 28 +++-- .../apiregions/BundleArtifactFeatureHandler.java | 8 +- .../apiregions/APIRegionMergeHandlerTest.java | 125 ++++++++++++++++++++- 5 files changed, 231 insertions(+), 14 deletions(-) diff --git a/pom.xml b/pom.xml index 7cb13e0..84e398b 100644 --- a/pom.xml +++ b/pom.xml @@ -73,6 +73,12 @@ <scope>test</scope> </dependency> <dependency> + <groupId>org.mockito</groupId> + <artifactId>mockito-core</artifactId> + <version>2.23.4</version> + <scope>test</scope> + </dependency> + <dependency> <groupId>org.apache.johnzon</groupId> <artifactId>johnzon-core</artifactId> <version>1.0.0</version> diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandler.java b/src/main/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandler.java index 17f032f..87c4289 100644 --- a/src/main/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandler.java +++ b/src/main/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandler.java @@ -16,18 +16,27 @@ */ package org.apache.sling.feature.extension.apiregions; +import org.apache.sling.feature.Artifact; import org.apache.sling.feature.Extension; import org.apache.sling.feature.Feature; import org.apache.sling.feature.builder.HandlerContext; import org.apache.sling.feature.builder.MergeHandler; +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; import java.io.StringReader; import java.io.StringWriter; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Properties; +import java.util.Set; +import java.util.stream.Collectors; import javax.json.Json; import javax.json.JsonArray; @@ -54,6 +63,8 @@ public class APIRegionMergeHandler implements MergeHandler { if (targetEx != null && !targetEx.getName().equals(API_REGIONS_NAME)) return; + storeBundleOrigins(context, source, target); + JsonReader srcJR = Json.createReader(new StringReader(sourceEx.getJSON())); JsonArray srcJA = srcJR.readArray(); @@ -83,6 +94,8 @@ public class APIRegionMergeHandler implements MergeHandler { srcRegions.put(regionName, region); } + storeRegionOrigins(context, source, target, srcRegions.keySet()); + JsonArray tgtJA; if (targetEx != null) { JsonReader tgtJR = Json.createReader(new StringReader(targetEx.getJSON())); @@ -153,6 +166,71 @@ public class APIRegionMergeHandler implements MergeHandler { targetEx.setJSON(sw.toString()); } + private void storeRegionOrigins(HandlerContext context, Feature source, Feature target, Set<String> regions) { + try { + File f = getFeatureDataFile(context, target, "regionOrigins.properties"); + + Properties p = new Properties(); + if (f.isFile()) { + try (FileInputStream fis = new FileInputStream(f)) { + p.load(fis); + } + } + + String fid = source.getId().toMvnId(); + p.put(fid, regions.stream().collect(Collectors.joining(","))); + + try (FileOutputStream fos = new FileOutputStream(f)) { + p.store(fos, "Mapping from feature ID to regions that the feature is a member of"); + } + } catch (IOException e) { + throw new IllegalStateException("Problem storing region origin information", e); + } + } + + private void storeBundleOrigins(HandlerContext context, Feature source, Feature target) { + try { + File f = getFeatureDataFile(context, target, "bundleOrigins.properties"); + + String featureId = source.getId().toMvnId(); + Properties p = new Properties(); + if (f.isFile()) { + try (FileInputStream fis = new FileInputStream(f)) { + p.load(fis); + } + } + + for (Artifact b : source.getBundles()) { + String bundleId = b.getId().toMvnId(); + String org = p.getProperty(bundleId); + String newVal; + if (org != null) { + List<String> l = Arrays.asList(org.split(",")); + if (!l.contains(featureId)) + newVal = org + "," + featureId; + else + newVal = org; + } else { + newVal = featureId; + } + p.setProperty(bundleId, newVal); + } + + try (FileOutputStream fos = new FileOutputStream(f)) { + p.store(fos, "Mapping from bundle artifact IDs to features that contained the bundle."); + } + } catch (IOException e) { + throw new IllegalStateException("Problem storing bundle origin information", e); + } + } + + private File getFeatureDataFile(HandlerContext context, Feature target, String fileName) throws IOException { + String featureName = target.getId().toMvnId().replaceAll("[^a-zA-Z0-9\\.\\-]", "_"); + File f = AbstractHandler.getDataFile(context, featureName, fileName); + f.getParentFile().mkdirs(); + return f; + } + private static List<String> readJsonArray(JsonArray jsonArray) { List<String> l = new ArrayList<>(); for (JsonValue jv : jsonArray) { diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/AbstractHandler.java b/src/main/java/org/apache/sling/feature/extension/apiregions/AbstractHandler.java index a67ceb6..356e48a 100644 --- a/src/main/java/org/apache/sling/feature/extension/apiregions/AbstractHandler.java +++ b/src/main/java/org/apache/sling/feature/extension/apiregions/AbstractHandler.java @@ -37,24 +37,36 @@ class AbstractHandler { static final String NAME_KEY = "name"; static final String EXPORTS_KEY = "exports"; - private static final String FILE_PREFIX = "apiregions."; - private static final String FILE_STORAGE_DIR_KEY = "fileStorage"; + static final String FILE_PREFIX = "apiregions."; + static final String FILE_STORAGE_DIR_KEY = "fileStorage"; - protected File getDataFile(HandlerContext context, String name) throws IOException { + protected static File getDataFile(HandlerContext context, String directory, String name) throws IOException { String stg = context.getConfiguration().get(FILE_STORAGE_DIR_KEY); - Path p; + File f; if (stg != null) { - p = new File(stg, name).toPath(); + File dir; + if (directory != null) { + dir = new File(stg, directory); + dir.mkdirs(); + } else { + dir = new File(stg); + } + f = new File(dir, name); } else { - p = Files.createTempFile(FILE_PREFIX, name); + // If we store in the temp space we don't use the directory + Path p = Files.createTempFile(FILE_PREFIX, name); + f = p.toFile(); + f.deleteOnExit(); } - File f = p.toFile(); - f.deleteOnExit(); System.setProperty(FILE_PREFIX + name, f.getCanonicalPath()); return f; } + protected static File getDataFile(HandlerContext context, String name) throws IOException { + return getDataFile(context, null, name); + } + protected Properties loadProperties(File file) throws IOException, FileNotFoundException { Properties map = new Properties(); if (file.exists()) { diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandler.java b/src/main/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandler.java index a8b9052..9bd4182 100644 --- a/src/main/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandler.java +++ b/src/main/java/org/apache/sling/feature/extension/apiregions/BundleArtifactFeatureHandler.java @@ -103,9 +103,11 @@ public class BundleArtifactFeatureHandler extends AbstractHandler implements Pos if (packages != null) { packageSet.addAll(Arrays.asList(packages.split(","))); } - JsonArray eja = jo.getJsonArray(EXPORTS_KEY); - for (int i=0; i < eja.size(); i++) { - packageSet.add(eja.getString(i)); + if (jo.containsKey(EXPORTS_KEY)) { + JsonArray eja = jo.getJsonArray(EXPORTS_KEY); + for (int i=0; i < eja.size(); i++) { + packageSet.add(eja.getString(i)); + } } rpMap.put(region, packageSet.stream().collect(Collectors.joining(","))); } diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandlerTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandlerTest.java index 488df88..fa965e8 100644 --- a/src/test/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandlerTest.java +++ b/src/test/java/org/apache/sling/feature/extension/apiregions/APIRegionMergeHandlerTest.java @@ -16,13 +16,26 @@ */ package org.apache.sling.feature.extension.apiregions; +import org.apache.sling.feature.Artifact; import org.apache.sling.feature.ArtifactId; import org.apache.sling.feature.Extension; import org.apache.sling.feature.ExtensionType; import org.apache.sling.feature.Feature; +import org.apache.sling.feature.builder.HandlerContext; +import org.junit.After; +import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; import java.io.StringReader; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; +import java.util.Comparator; +import java.util.Properties; import javax.json.Json; import javax.json.JsonArray; @@ -33,6 +46,22 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class APIRegionMergeHandlerTest { + private Path tempDir; + + @Before + public void setUp() throws IOException { + tempDir = Files.createTempDirectory(getClass().getSimpleName()); + } + + @After + public void tearDown() throws IOException { + // Delete the temp dir again + Files.walk(tempDir) + .sorted(Comparator.reverseOrder()) + .map(Path::toFile) + .forEach(File::delete); + } + @Test public void testCanMerge() { APIRegionMergeHandler armh = new APIRegionMergeHandler(); @@ -63,7 +92,8 @@ public class APIRegionMergeHandlerTest { + "\"exports\": [\"a.ha\"]," + "\"my-key\": \"my-val\"}]"); - armh.merge(null, tf, sf, tgEx, srEx); + HandlerContext hc = Mockito.mock(HandlerContext.class); + armh.merge(hc, tf, sf, tgEx, srEx); String expectedJSON = "[{\"name\":\"global\"," + "\"exports\": [\"a.b.c\",\"d.e.f\", \"test\"]}," @@ -83,7 +113,7 @@ public class APIRegionMergeHandlerTest { @Test - public void testRegionExportsInheritance() throws Exception { + public void testRegionExportsNoInheritance() throws Exception { APIRegionMergeHandler armh = new APIRegionMergeHandler(); Feature tf = new Feature(ArtifactId.fromMvnId("x:t:1")); @@ -99,7 +129,8 @@ public class APIRegionMergeHandlerTest { + "{\"name\":\"forbidden\"," + "\"exports\":[\"abc\",\"klm\"]}]"); - armh.merge(null, tf, sf, null, srEx); + HandlerContext hc = Mockito.mock(HandlerContext.class); + armh.merge(hc, tf, sf, null, srEx); Extension tgEx = tf.getExtensions().iterator().next(); @@ -114,4 +145,92 @@ public class APIRegionMergeHandlerTest { assertEquals(ea, aa); } + + @Test + public void testStoreBundleOrigins() throws Exception { + HandlerContext hc = Mockito.mock(HandlerContext.class); + Mockito.when(hc.getConfiguration()).thenReturn( + Collections.singletonMap(AbstractHandler.FILE_STORAGE_DIR_KEY, + tempDir.toString())); + + APIRegionMergeHandler armh = new APIRegionMergeHandler(); + + Feature tf = new Feature(ArtifactId.fromMvnId("g:t:1")); + Feature sf1 = new Feature(ArtifactId.fromMvnId("g:s1:1")); + Extension sf1Ex = new Extension(ExtensionType.JSON, "api-regions", false); + sf1Ex.setJSON("[]"); + + sf1.getBundles().add(new Artifact(ArtifactId.fromMvnId("a:b1:1"))); + sf1.getBundles().add(new Artifact(ArtifactId.fromMvnId("a:b2:1"))); + + armh.merge(hc, tf, sf1, null, sf1Ex); + + Feature sf2 = new Feature(ArtifactId.fromMvnId("g:s2:1")); + Extension sf2Ex = new Extension(ExtensionType.JSON, "api-regions", false); + sf2Ex.setJSON("[]"); + + sf2.getBundles().add(new Artifact(ArtifactId.fromMvnId("a:b2:1"))); + sf2.getBundles().add(new Artifact(ArtifactId.fromMvnId("a:b3:1"))); + sf2.getBundles().add(new Artifact(ArtifactId.fromMvnId("a:b2:1"))); + + armh.merge(hc, tf, sf2, tf.getExtensions().getByName("api-regions"), sf2Ex); + + Feature sf3 = new Feature(ArtifactId.fromMvnId("g:s3:1")); + Extension sf3Ex = new Extension(ExtensionType.JSON, "api-regions", false); + sf3Ex.setJSON("[]"); + + sf3.getBundles().add(new Artifact(ArtifactId.fromMvnId("a:b2:1"))); + + armh.merge(hc, tf, sf3, tf.getExtensions().getByName("api-regions"), sf3Ex); + + Properties bo = new Properties(); + bo.load(new FileInputStream(new File(tempDir.toFile(), "g_t_1/bundleOrigins.properties"))); + assertEquals(3, bo.size()); + + assertEquals("g:s1:1", bo.get("a:b1:1")); + assertEquals("g:s1:1,g:s2:1,g:s3:1", bo.get("a:b2:1")); + assertEquals("g:s2:1", bo.get("a:b3:1")); + } + + @Test + public void testStoreRegionOrigins() throws Exception { + HandlerContext hc = Mockito.mock(HandlerContext.class); + Mockito.when(hc.getConfiguration()).thenReturn( + Collections.singletonMap(AbstractHandler.FILE_STORAGE_DIR_KEY, + tempDir.toString())); + + APIRegionMergeHandler armh = new APIRegionMergeHandler(); + + Feature tf = new Feature(ArtifactId.fromMvnId("x:t:1")); + Feature sf1 = new Feature(ArtifactId.fromMvnId("y:s:2")); + + Extension sr1Ex = new Extension(ExtensionType.JSON, "api-regions", false); + sr1Ex.setJSON("[{\"name\":\"global\"," + + "\"exports\": [\"a.b.c\",\"d.e.f\"]}," + + "{\"name\":\"deprecated\"," + + "\"exports\":[\"klm\",\"#ignored\",\"qrs\"]}," + + "{\"name\":\"internal\"," + + "\"exports\":[\"xyz\"]}," + + "{\"name\":\"forbidden\"," + + "\"exports\":[\"abc\",\"klm\"]}]"); + + armh.merge(hc, tf, sf1, null, sr1Ex); + + Feature sf2 = new Feature(ArtifactId.fromMvnId("z:s:1")); + + Extension sr2Ex = new Extension(ExtensionType.JSON, "api-regions", false); + sr2Ex.setJSON("[{\"name\":\"global\"," + + "\"exports\": [\"g.h.i\"]}," + + "{\"name\":\"internal\"," + + "\"exports\":[]}," + + "{\"name\":\"somethingelse\"," + + "\"exports\":[\"qqq\"]}]"); + armh.merge(hc, tf, sf2, tf.getExtensions().getByName("api-regions"), sr2Ex); + + Properties ro = new Properties(); + ro.load(new FileInputStream(new File(tempDir.toFile(), "x_t_1/regionOrigins.properties"))); + assertEquals(2, ro.size()); + assertEquals("global,deprecated,internal,forbidden", ro.get("y:s:2")); + assertEquals("global,internal,somethingelse", ro.get("z:s:1")); + } }