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.
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 @@
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 @@
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 @@
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 @@
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 @@
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.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 @@
+ "\"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 @@
@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 @@
+ "{\"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 @@
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"));
+ }
}
