You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2019/10/17 15:51:07 UTC

[sling-org-apache-sling-feature-extension-apiregions] branch master updated (d42176a -> e8b5b46)

This is an automated email from the ASF dual-hosted git repository.

cziegeler pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-extension-apiregions.git.


    from d42176a  SLING-8784 : Move api regions analysers to api regions extension
     new e054c29  SLING-8784 : Move api regions analysers to api regions extension
     new e8b5b46  SLING-8783 : Create API for api regions

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apiregions/APIRegionMergeHandler.java          |  47 +--
 .../analyser/AbstractApiRegionsAnalyserTask.java   |   5 +-
 .../CheckApiRegionsBundleExportsImports.java       |  29 +-
 .../apiregions/analyser/CheckApiRegionsOrder.java  |   4 +-
 .../extension/apiregions/api/ApiRegions.java       |  90 +++---
 .../apiregions/APIRegionMergeHandlerTest.java      |   2 +-
 .../CheckApiRegionsBundleExportsImportsTest.java   | 336 +++++++++++++++++++++
 .../f_f_1/bundleOrigins.properties                 |   2 +
 .../f_f_1/regionOrigins.properties                 |   2 +
 .../f_f_1/bundleOrigins.properties                 |   2 +
 .../f_f_1/regionOrigins.properties                 |   2 +
 .../f_f_1/bundleOrigins.properties                 |   2 +
 .../f_f_1/regionOrigins.properties                 |   2 +
 .../f_f_1/bundleOrigins.properties                 |   2 +
 .../f_f_1/regionOrigins.properties                 |   1 +
 .../f_f_1/bundleOrigins.properties                 |   2 +
 .../f_f_1/regionOrigins.properties                 |   2 +
 .../f_f_2/bundleOrigins.properties                 |   4 +
 .../f_f_2/regionOrigins.properties                 |   3 +
 src/test/resources/test-bundle1.jar                | Bin 0 -> 434 bytes
 src/test/resources/test-bundle2.jar                | Bin 0 -> 457 bytes
 src/test/resources/test-bundle3.jar                | Bin 0 -> 431 bytes
 src/test/resources/test-bundle4.jar                | Bin 0 -> 420 bytes
 src/test/resources/test-framework.jar              | Bin 0 -> 13288 bytes
 24 files changed, 467 insertions(+), 72 deletions(-)
 create mode 100644 src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleExportsImportsTest.java
 create mode 100644 src/test/resources/origins/testImportExportWithMatchingRegion/f_f_1/bundleOrigins.properties
 create mode 100644 src/test/resources/origins/testImportExportWithMatchingRegion/f_f_1/regionOrigins.properties
 create mode 100644 src/test/resources/origins/testImportExportWithRegionMismatch/f_f_1/bundleOrigins.properties
 create mode 100644 src/test/resources/origins/testImportExportWithRegionMismatch/f_f_1/regionOrigins.properties
 create mode 100644 src/test/resources/origins/testImportExportWithRegionMismatchIgnoreRegions/f_f_1/bundleOrigins.properties
 create mode 100644 src/test/resources/origins/testImportExportWithRegionMismatchIgnoreRegions/f_f_1/regionOrigins.properties
 create mode 100644 src/test/resources/origins/testImportExportWithRegionsMissing/f_f_1/bundleOrigins.properties
 create mode 100644 src/test/resources/origins/testImportExportWithRegionsMissing/f_f_1/regionOrigins.properties
 create mode 100644 src/test/resources/origins/testImportFromGlobalAlwaysSucceeds/f_f_1/bundleOrigins.properties
 create mode 100644 src/test/resources/origins/testImportFromGlobalAlwaysSucceeds/f_f_1/regionOrigins.properties
 create mode 100644 src/test/resources/origins/testImportFromOtherBundleInSameFeature/f_f_2/bundleOrigins.properties
 create mode 100644 src/test/resources/origins/testImportFromOtherBundleInSameFeature/f_f_2/regionOrigins.properties
 create mode 100644 src/test/resources/test-bundle1.jar
 create mode 100644 src/test/resources/test-bundle2.jar
 create mode 100644 src/test/resources/test-bundle3.jar
 create mode 100644 src/test/resources/test-bundle4.jar
 create mode 100644 src/test/resources/test-framework.jar


[sling-org-apache-sling-feature-extension-apiregions] 01/02: SLING-8784 : Move api regions analysers to api regions extension

Posted by cz...@apache.org.
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

commit e054c2940ca4fba7f3cd222c40d453efd52b1604
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Thu Oct 17 17:15:18 2019 +0200

    SLING-8784 : Move api regions analysers to api regions extension
---
 .../CheckApiRegionsBundleExportsImports.java       |  30 +-
 .../CheckApiRegionsBundleExportsImportsTest.java   | 336 +++++++++++++++++++++
 .../f_f_1/bundleOrigins.properties                 |   2 +
 .../f_f_1/regionOrigins.properties                 |   2 +
 .../f_f_1/bundleOrigins.properties                 |   2 +
 .../f_f_1/regionOrigins.properties                 |   2 +
 .../f_f_1/bundleOrigins.properties                 |   2 +
 .../f_f_1/regionOrigins.properties                 |   2 +
 .../f_f_1/bundleOrigins.properties                 |   2 +
 .../f_f_1/regionOrigins.properties                 |   1 +
 .../f_f_1/bundleOrigins.properties                 |   2 +
 .../f_f_1/regionOrigins.properties                 |   2 +
 .../f_f_2/bundleOrigins.properties                 |   4 +
 .../f_f_2/regionOrigins.properties                 |   3 +
 src/test/resources/test-bundle1.jar                | Bin 0 -> 434 bytes
 src/test/resources/test-bundle2.jar                | Bin 0 -> 457 bytes
 src/test/resources/test-bundle3.jar                | Bin 0 -> 431 bytes
 src/test/resources/test-bundle4.jar                | Bin 0 -> 420 bytes
 src/test/resources/test-framework.jar              | Bin 0 -> 13288 bytes
 19 files changed, 388 insertions(+), 4 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 42635da..cd2d34e 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
@@ -37,15 +37,20 @@ import java.util.SortedMap;
 import java.util.TreeMap;
 import java.util.stream.Collectors;
 
+import javax.json.JsonArray;
+import javax.json.stream.JsonParsingException;
+
 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.ApiRegions;
 import org.apache.sling.feature.scanner.BundleDescriptor;
 import org.apache.sling.feature.scanner.PackageInfo;
 import org.osgi.framework.Version;
 
-public class CheckApiRegionsBundleExportsImports extends AbstractApiRegionsAnalyserTask {
+public class CheckApiRegionsBundleExportsImports implements AnalyserTask {
 
     private static final String FILE_STORAGE_CONFIG_KEY = "fileStorage";
     private static final String IGNORE_API_REGIONS_CONFIG_KEY = "ignoreAPIRegions";
@@ -118,7 +123,7 @@ public class CheckApiRegionsBundleExportsImports extends AbstractApiRegionsAnaly
     }
 
     @Override
-    protected void execute(ApiRegions apiRegions, AnalyserTaskContext ctx) throws Exception {
+    public void execute(final AnalyserTaskContext ctx) throws Exception {
         boolean ignoreAPIRegions = ctx.getConfiguration().getOrDefault(
                 IGNORE_API_REGIONS_CONFIG_KEY, "false").equalsIgnoreCase("true");
 
@@ -145,13 +150,29 @@ public class CheckApiRegionsBundleExportsImports extends AbstractApiRegionsAnaly
 
         Map<String, Set<String>> bundleToOriginalFeatures;
         Map<String, Set<String>> featureToOriginalRegions;
+        ApiRegions apiRegions = new ApiRegions(); // Empty API Regions;
+
         if (ignoreAPIRegions) {
-            apiRegions = new ApiRegions(); // Empty API Regions
             bundleToOriginalFeatures = Collections.emptyMap();
             featureToOriginalRegions = Collections.emptyMap();
         } else {
             bundleToOriginalFeatures = readBundleOrigins(ctx);
             featureToOriginalRegions = readRegionOrigins(ctx);
+            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 (IllegalStateException | IllegalArgumentException | JsonParsingException e) {
+                    ctx.reportError("API Regions '" + apiRegionsExtension.getJSON()
+                            + "' does not represent a valid JSON 'api-regions': " + e.getMessage());
+                    return;
+                }
+            }
         }
 
         for(final Map.Entry<Integer, List<BundleDescriptor>> entry : bundlesMap.entrySet()) {
@@ -351,7 +372,8 @@ public class CheckApiRegionsBundleExportsImports extends AbstractApiRegionsAnaly
 
                 for (String region : getBundleRegions(info, bundleToOriginalFeatures, featureToOriginalRegions)) {
                     if (!NO_REGION.equals(region) &&
-                            apiRegions.getRegionByName(region).getExportByName(pck.getName()) == null)
+                            (apiRegions.getRegionByName(region) == null
+                                    || apiRegions.getRegionByName(region).getExportByName(pck.getName()) == null))
                         continue;
 
                     Set<String> regions = candidates.get(info);
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
new file mode 100644
index 0000000..4600dfc
--- /dev/null
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsBundleExportsImportsTest.java
@@ -0,0 +1,336 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.feature.extension.apiregions.analyser;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.sling.feature.Artifact;
+import org.apache.sling.feature.ArtifactId;
+import org.apache.sling.feature.Extension;
+import org.apache.sling.feature.ExtensionState;
+import org.apache.sling.feature.ExtensionType;
+import org.apache.sling.feature.Feature;
+import org.apache.sling.feature.analyser.task.AnalyserTaskContext;
+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.impl.BundleDescriptorImpl;
+import org.apache.sling.feature.scanner.impl.FeatureDescriptorImpl;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+public class CheckApiRegionsBundleExportsImportsTest {
+    private static File resourceRoot;
+
+    @BeforeClass
+    public static void setupClass() {
+        resourceRoot =
+                new File(CheckApiRegionsBundleExportsImportsTest.class.
+                        getResource("/test-framework.jar").getFile()).getParentFile();
+    }
+
+    @Test
+    public void testId() {
+        assertEquals(ApiRegions.EXTENSION_NAME + "-exportsimports", new CheckApiRegionsBundleExportsImports().getId());
+    }
+
+    @Test
+    /*
+     * Bundle b3 imports org.foo.e, but no bundle exports it. The feature is marked
+     * as complete which it isn't
+     */
+    public void testImportExportNoRegionsMarkedAsComplete() throws Exception {
+        CheckApiRegionsBundleExportsImports t = new CheckApiRegionsBundleExportsImports();
+
+        Feature f = new Feature(ArtifactId.fromMvnId("f:f:1"));
+        f.setComplete(true);
+        FeatureDescriptor fd = new FeatureDescriptorImpl(f);
+
+        fdAddBundle(fd, "g:b1:1", "test-bundle1.jar");
+        fdAddBundle(fd, "g:b3:1", "test-bundle3.jar");
+
+        AnalyserTaskContext ctx = Mockito.mock(AnalyserTaskContext.class);
+        Mockito.when(ctx.getFeature()).thenReturn(f);
+        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).reportError(Mockito.contains("marked as 'complete'"));
+        Mockito.verify(ctx, Mockito.never()).reportWarning(Mockito.anyString());
+    }
+
+    @Test
+    public void testImportExportNoRegionsAllOk() throws Exception {
+        CheckApiRegionsBundleExportsImports t = new CheckApiRegionsBundleExportsImports();
+
+        Feature f = new Feature(ArtifactId.fromMvnId("f:f:1"));
+        FeatureDescriptor fd = new FeatureDescriptorImpl(f);
+
+        fdAddBundle(fd, "g:b1:1", "test-bundle1.jar");
+        fdAddBundle(fd, "g:b2:1", "test-bundle2.jar");
+
+        AnalyserTaskContext ctx = Mockito.mock(AnalyserTaskContext.class);
+        Mockito.when(ctx.getFeature()).thenReturn(f);
+        Mockito.when(ctx.getFeatureDescriptor()).thenReturn(fd);
+        t.execute(ctx);
+
+        Mockito.verify(ctx, Mockito.never()).reportError(Mockito.anyString());
+        Mockito.verify(ctx, Mockito.never()).reportWarning(Mockito.anyString());
+    }
+
+    @Test
+    /*
+     * Bundle b3 imports org.foo.e, but no bundle exports it
+     */
+    public void testImportExportNoRegionsMissing() throws Exception {
+        CheckApiRegionsBundleExportsImports t = new CheckApiRegionsBundleExportsImports();
+
+        Feature f = new Feature(ArtifactId.fromMvnId("f:f:1"));
+        FeatureDescriptor fd = new FeatureDescriptorImpl(f);
+
+        fdAddBundle(fd, "g:b1:1", "test-bundle1.jar");
+        fdAddBundle(fd, "g:b3:1", "test-bundle3.jar");
+
+        AnalyserTaskContext ctx = Mockito.mock(AnalyserTaskContext.class);
+        Mockito.when(ctx.getFeature()).thenReturn(f);
+        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, Mockito.never()).reportWarning(Mockito.anyString());
+    }
+
+    @Test
+    /*
+     * Bundle 2 imports org.foo.b from bundle 1, but bundle 1 exports it in a
+     * different region, bundle 2 is in no region.
+     */
+    public void testImportExportWithRegionsMissing() throws Exception {
+        String exJson = "[{\"name\": \"something\", \"exports\": [\"org.foo.b\"]}]";
+
+        CheckApiRegionsBundleExportsImports t = new CheckApiRegionsBundleExportsImports();
+
+        Feature f = new Feature(ArtifactId.fromMvnId("f:f:1"));
+        Extension ex = new Extension(ExtensionType.JSON, "api-regions", ExtensionState.OPTIONAL);
+        ex.setJSON(exJson);
+        f.getExtensions().add(ex);
+
+        FeatureDescriptor fd = new FeatureDescriptorImpl(f);
+
+        fdAddBundle(fd, "g:b1:1", "test-bundle1.jar");
+        fdAddBundle(fd, "g:b2:1", "test-bundle2.jar");
+
+        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/testImportExportWithRegionsMissing"));
+        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());
+    }
+
+    @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.
+     */
+    public void testImportExportWithRegionMismatch() throws Exception {
+        String exJson = "[{\"name\": \"something\", \"exports\": [\"org.foo.b\"]}]";
+
+        CheckApiRegionsBundleExportsImports t = new CheckApiRegionsBundleExportsImports();
+
+        Feature f = new Feature(ArtifactId.fromMvnId("f:f:1"));
+        Extension ex = new Extension(ExtensionType.JSON, "api-regions", ExtensionState.OPTIONAL);
+        ex.setJSON(exJson);
+        f.getExtensions().add(ex);
+
+        FeatureDescriptor fd = new FeatureDescriptorImpl(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", "false");
+
+        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).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, Mockito.never()).reportWarning(Mockito.anyString());
+    }
+
+    @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\"]}]";
+
+        CheckApiRegionsBundleExportsImports t = new CheckApiRegionsBundleExportsImports();
+
+        Feature f = new Feature(ArtifactId.fromMvnId("f:f:1"));
+        Extension ex = new Extension(ExtensionType.JSON, "api-regions", ExtensionState.OPTIONAL);
+        ex.setJSON(exJson);
+        f.getExtensions().add(ex);
+
+        FeatureDescriptor fd = new FeatureDescriptorImpl(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.
+     */
+    public void testImportFromOtherBundleInSameFeature() throws Exception {
+        String exJson = "[{\"name\": \"blah\"}]"; // no exports
+
+        CheckApiRegionsBundleExportsImports t = new CheckApiRegionsBundleExportsImports();
+
+        Feature f = new Feature(ArtifactId.fromMvnId("f:f:2"));
+        Extension ex = new Extension(ExtensionType.JSON, "api-regions", ExtensionState.OPTIONAL);
+        ex.setJSON(exJson);
+        f.getExtensions().add(ex);
+
+        FeatureDescriptor fd = new FeatureDescriptorImpl(f);
+
+        fdAddBundle(fd, "g:b1:1", "test-bundle1.jar");
+        fdAddBundle(fd, "g:b3:1", "test-bundle3.jar");
+        fdAddBundle(fd, "g:b4:1", "test-bundle4.jar");
+
+        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/testImportFromOtherBundleInSameFeature"));
+        t.execute(ctx);
+
+        Mockito.verify(ctx, Mockito.never()).reportError(Mockito.anyString());
+        Mockito.verify(ctx, Mockito.never()).reportWarning(Mockito.anyString());
+    }
+
+    @Test
+    /*
+     * Bundle 2 imports org.foo.b from bundle 1. Bundle 1 exports it in the something region
+     * and bundle 2 imports it in the something region, so this succeeds.
+     */
+    public void testImportExportWithMatchingRegion() throws Exception {
+        String exJson = "[{\"name\": \"something\", \"exports\": [\"org.foo.b\"]}]";
+
+        CheckApiRegionsBundleExportsImports t = new CheckApiRegionsBundleExportsImports();
+
+        Feature f = new Feature(ArtifactId.fromMvnId("f:f:1"));
+        Extension ex = new Extension(ExtensionType.JSON, "api-regions", ExtensionState.OPTIONAL);
+        ex.setJSON(exJson);
+        f.getExtensions().add(ex);
+
+        FeatureDescriptor fd = new FeatureDescriptorImpl(f);
+
+        fdAddBundle(fd, "g:b1:1", "test-bundle1.jar");
+        fdAddBundle(fd, "g:b2:1", "test-bundle2.jar");
+
+        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/testImportExportWithMatchingRegion"));
+        t.execute(ctx);
+
+        Mockito.verify(ctx, Mockito.never()).reportError(Mockito.anyString());
+        Mockito.verify(ctx, Mockito.never()).reportWarning(Mockito.anyString());
+    }
+
+    @Test
+    /*
+     * Bundle 2 imports org.foo.b from bundle 1. Bundle 1 exports it in the global region.
+     * Bundle 2 is not explicitly part of the global region, but can still see it
+     */
+    public void testImportFromGlobalAlwaysSucceeds() throws Exception {
+        String exJson = "[{\"name\": \"global\", \"exports\": [\"org.foo.b\"]}]";
+
+        CheckApiRegionsBundleExportsImports t = new CheckApiRegionsBundleExportsImports();
+
+        Feature f = new Feature(ArtifactId.fromMvnId("f:f:1"));
+        Extension ex = new Extension(ExtensionType.JSON, "api-regions", ExtensionState.OPTIONAL);
+        ex.setJSON(exJson);
+        f.getExtensions().add(ex);
+
+        FeatureDescriptor fd = new FeatureDescriptorImpl(f);
+
+        fdAddBundle(fd, "g:b1:1", "test-bundle1.jar");
+        fdAddBundle(fd, "g:b2:1", "test-bundle2.jar");
+
+        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/testImportFromGlobalAlwaysSucceeds"));
+        t.execute(ctx);
+
+        Mockito.verify(ctx, Mockito.never()).reportError(Mockito.anyString());
+        Mockito.verify(ctx, Mockito.never()).reportWarning(Mockito.anyString());
+    }
+
+    private void fdAddBundle(FeatureDescriptor fd, String id, String file) throws IOException {
+        BundleDescriptor bd1 = new BundleDescriptorImpl(
+                new Artifact(ArtifactId.fromMvnId(id)), new File(resourceRoot, file).toURI().toURL(), 0);
+        fd.getBundleDescriptors().add(bd1);
+    }
+}
diff --git a/src/test/resources/origins/testImportExportWithMatchingRegion/f_f_1/bundleOrigins.properties b/src/test/resources/origins/testImportExportWithMatchingRegion/f_f_1/bundleOrigins.properties
new file mode 100644
index 0000000..788c4fa
--- /dev/null
+++ b/src/test/resources/origins/testImportExportWithMatchingRegion/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/testImportExportWithMatchingRegion/f_f_1/regionOrigins.properties b/src/test/resources/origins/testImportExportWithMatchingRegion/f_f_1/regionOrigins.properties
new file mode 100644
index 0000000..ceb9131
--- /dev/null
+++ b/src/test/resources/origins/testImportExportWithMatchingRegion/f_f_1/regionOrigins.properties
@@ -0,0 +1,2 @@
+f\:f1\:1=something,someotherthing
+f\:f2\:1=something
diff --git a/src/test/resources/origins/testImportExportWithRegionMismatch/f_f_1/bundleOrigins.properties b/src/test/resources/origins/testImportExportWithRegionMismatch/f_f_1/bundleOrigins.properties
new file mode 100644
index 0000000..788c4fa
--- /dev/null
+++ b/src/test/resources/origins/testImportExportWithRegionMismatch/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/testImportExportWithRegionMismatch/f_f_1/regionOrigins.properties b/src/test/resources/origins/testImportExportWithRegionMismatch/f_f_1/regionOrigins.properties
new file mode 100644
index 0000000..3134d99
--- /dev/null
+++ b/src/test/resources/origins/testImportExportWithRegionMismatch/f_f_1/regionOrigins.properties
@@ -0,0 +1,2 @@
+f\:f1\:1=something,someotherthing
+f\:f2\:1=somethingelse
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
diff --git a/src/test/resources/origins/testImportExportWithRegionsMissing/f_f_1/bundleOrigins.properties b/src/test/resources/origins/testImportExportWithRegionsMissing/f_f_1/bundleOrigins.properties
new file mode 100644
index 0000000..788c4fa
--- /dev/null
+++ b/src/test/resources/origins/testImportExportWithRegionsMissing/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/testImportExportWithRegionsMissing/f_f_1/regionOrigins.properties b/src/test/resources/origins/testImportExportWithRegionsMissing/f_f_1/regionOrigins.properties
new file mode 100644
index 0000000..019a972
--- /dev/null
+++ b/src/test/resources/origins/testImportExportWithRegionsMissing/f_f_1/regionOrigins.properties
@@ -0,0 +1 @@
+f\:f1\:1=something
diff --git a/src/test/resources/origins/testImportFromGlobalAlwaysSucceeds/f_f_1/bundleOrigins.properties b/src/test/resources/origins/testImportFromGlobalAlwaysSucceeds/f_f_1/bundleOrigins.properties
new file mode 100644
index 0000000..788c4fa
--- /dev/null
+++ b/src/test/resources/origins/testImportFromGlobalAlwaysSucceeds/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/testImportFromGlobalAlwaysSucceeds/f_f_1/regionOrigins.properties b/src/test/resources/origins/testImportFromGlobalAlwaysSucceeds/f_f_1/regionOrigins.properties
new file mode 100644
index 0000000..a94edd0
--- /dev/null
+++ b/src/test/resources/origins/testImportFromGlobalAlwaysSucceeds/f_f_1/regionOrigins.properties
@@ -0,0 +1,2 @@
+f\:f1\:1=someotherthing,global,lalala
+f\:f2\:1=something
diff --git a/src/test/resources/origins/testImportFromOtherBundleInSameFeature/f_f_2/bundleOrigins.properties b/src/test/resources/origins/testImportFromOtherBundleInSameFeature/f_f_2/bundleOrigins.properties
new file mode 100644
index 0000000..7910020
--- /dev/null
+++ b/src/test/resources/origins/testImportFromOtherBundleInSameFeature/f_f_2/bundleOrigins.properties
@@ -0,0 +1,4 @@
+g\:b1\:1=f\:f3\:1
+g\:b2\:1=f\:f3\:1
+g\:b3\:1=f\:f3\:1
+g\:b4\:1=f\:f3\:1
diff --git a/src/test/resources/origins/testImportFromOtherBundleInSameFeature/f_f_2/regionOrigins.properties b/src/test/resources/origins/testImportFromOtherBundleInSameFeature/f_f_2/regionOrigins.properties
new file mode 100644
index 0000000..3488a06
--- /dev/null
+++ b/src/test/resources/origins/testImportFromOtherBundleInSameFeature/f_f_2/regionOrigins.properties
@@ -0,0 +1,3 @@
+f\:f1\:1=something,someotherthing
+f\:f2\:1=abc,xyz
+f\:f3\:1=blah
diff --git a/src/test/resources/test-bundle1.jar b/src/test/resources/test-bundle1.jar
new file mode 100644
index 0000000..394c883
Binary files /dev/null and b/src/test/resources/test-bundle1.jar differ
diff --git a/src/test/resources/test-bundle2.jar b/src/test/resources/test-bundle2.jar
new file mode 100644
index 0000000..8fb2589
Binary files /dev/null and b/src/test/resources/test-bundle2.jar differ
diff --git a/src/test/resources/test-bundle3.jar b/src/test/resources/test-bundle3.jar
new file mode 100644
index 0000000..dcc0887
Binary files /dev/null and b/src/test/resources/test-bundle3.jar differ
diff --git a/src/test/resources/test-bundle4.jar b/src/test/resources/test-bundle4.jar
new file mode 100644
index 0000000..27d4f74
Binary files /dev/null and b/src/test/resources/test-bundle4.jar differ
diff --git a/src/test/resources/test-framework.jar b/src/test/resources/test-framework.jar
new file mode 100644
index 0000000..9ce022d
Binary files /dev/null and b/src/test/resources/test-framework.jar differ


[sling-org-apache-sling-feature-extension-apiregions] 02/02: SLING-8783 : Create API for api regions

Posted by cz...@apache.org.
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

commit e8b5b46e7ec205919a73b7fba515a226d7137dee
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Thu Oct 17 17:49:29 2019 +0200

    SLING-8783 : Create API for api regions
---
 .../apiregions/APIRegionMergeHandler.java          | 47 ++++++-----
 .../analyser/AbstractApiRegionsAnalyserTask.java   |  5 +-
 .../CheckApiRegionsBundleExportsImports.java       |  3 +-
 .../apiregions/analyser/CheckApiRegionsOrder.java  |  4 +-
 .../extension/apiregions/api/ApiRegions.java       | 90 ++++++++++++----------
 .../apiregions/APIRegionMergeHandlerTest.java      |  2 +-
 6 files changed, 81 insertions(+), 70 deletions(-)

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 ec28400..4b6141a 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
@@ -55,36 +55,41 @@ public class APIRegionMergeHandler implements MergeHandler {
 
         storeBundleOrigins(context, source, target);
 
-        final ApiRegions srcRegions = ApiRegions.parse((JsonArray) sourceEx.getJSONStructure());
+        try {
+            final ApiRegions srcRegions = ApiRegions.parse((JsonArray) sourceEx.getJSONStructure());
 
-        storeRegionOrigins(context, source, target, srcRegions);
+            storeRegionOrigins(context, source, target, srcRegions);
 
-        final ApiRegions targetRegions;
-        if (targetEx != null) {
-            targetRegions = ApiRegions.parse((JsonArray) targetEx.getJSONStructure());
-        } else {
-            targetEx = new Extension(sourceEx.getType(), sourceEx.getName(), sourceEx.getState());
-            target.getExtensions().add(targetEx);
+            final ApiRegions targetRegions;
+            if (targetEx != null) {
+                targetRegions = ApiRegions.parse((JsonArray) targetEx.getJSONStructure());
+            } else {
+                targetEx = new Extension(sourceEx.getType(), sourceEx.getName(), sourceEx.getState());
+                target.getExtensions().add(targetEx);
 
-            targetRegions = new ApiRegions();
-        }
+                targetRegions = new ApiRegions();
+            }
 
-        for (final ApiRegion targetRegion : targetRegions.getRegions()) {
-            final ApiRegion sourceRegion = srcRegions.getRegionByName(targetRegion.getName());
-            if (sourceRegion != null) {
-                srcRegions.getRegions().remove(sourceRegion);
-                for (final ApiExport srcExp : sourceRegion.getExports()) {
-                    if (targetRegion.getExportByName(srcExp.getName()) == null) {
-                        targetRegion.getExports().add(srcExp);
+            for (final ApiRegion targetRegion : targetRegions.getRegions()) {
+                final ApiRegion sourceRegion = srcRegions.getRegionByName(targetRegion.getName());
+                if (sourceRegion != null) {
+                    srcRegions.getRegions().remove(sourceRegion);
+                    for (final ApiExport srcExp : sourceRegion.getExports()) {
+                        if (targetRegion.getExportByName(srcExp.getName()) == null) {
+                            targetRegion.getExports().add(srcExp);
+                        }
                     }
                 }
             }
-        }
 
-        // If there are any remaining regions in the src extension, process them now
-        targetRegions.getRegions().addAll(srcRegions.getRegions());
+            // If there are any remaining regions in the src extension, process them now
+            targetRegions.getRegions().addAll(srcRegions.getRegions());
+
+            targetEx.setJSONStructure(targetRegions.toJSONArray());
 
-        targetEx.setJSONStructure(targetRegions.toJSONArray());
+        } catch (final IOException e) {
+            throw new RuntimeException(e);
+        }
     }
 
     private void storeRegionOrigins(HandlerContext context, Feature source, Feature target, ApiRegions regions) {
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTask.java b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTask.java
index 4e8e594..916e865 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTask.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/AbstractApiRegionsAnalyserTask.java
@@ -16,8 +16,9 @@
  */
 package org.apache.sling.feature.extension.apiregions.analyser;
 
+import java.io.IOException;
+
 import javax.json.JsonArray;
-import javax.json.stream.JsonParsingException;
 
 import org.apache.sling.feature.Extension;
 import org.apache.sling.feature.Extensions;
@@ -57,7 +58,7 @@ public abstract class AbstractApiRegionsAnalyserTask implements AnalyserTask {
         ApiRegions apiRegions;
         try {
             apiRegions = ApiRegions.parse((JsonArray) apiRegionsExtension.getJSONStructure());
-        } catch (IllegalStateException | IllegalArgumentException | JsonParsingException e) {
+        } catch (IOException e) {
             ctx.reportError("API Regions '"
                     + apiRegionsExtension.getJSON()
                     + "' does not represent a valid JSON 'api-regions': "
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 cd2d34e..dc65ee5 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
@@ -38,7 +38,6 @@ import java.util.TreeMap;
 import java.util.stream.Collectors;
 
 import javax.json.JsonArray;
-import javax.json.stream.JsonParsingException;
 
 import org.apache.sling.feature.Extension;
 import org.apache.sling.feature.Extensions;
@@ -167,7 +166,7 @@ public class CheckApiRegionsBundleExportsImports implements AnalyserTask {
             if (apiRegionsExtension != null && apiRegionsExtension.getJSONStructure() != null) {
                 try {
                     apiRegions = ApiRegions.parse((JsonArray) apiRegionsExtension.getJSONStructure());
-                } catch (IllegalStateException | IllegalArgumentException | JsonParsingException e) {
+                } catch (IOException e) {
                     ctx.reportError("API Regions '" + apiRegionsExtension.getJSON()
                             + "' does not represent a valid JSON 'api-regions': " + e.getMessage());
                     return;
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsOrder.java b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsOrder.java
index d9aa9f3..a922933 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsOrder.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsOrder.java
@@ -16,11 +16,11 @@
  */
 package org.apache.sling.feature.extension.apiregions.analyser;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
 import javax.json.JsonArray;
-import javax.json.stream.JsonParsingException;
 
 import org.apache.sling.feature.Extension;
 import org.apache.sling.feature.Extensions;
@@ -100,7 +100,7 @@ public class CheckApiRegionsOrder implements AnalyserTask {
                     return;
                 }
             }
-        } catch (final IllegalStateException | IllegalArgumentException | JsonParsingException e) {
+        } catch (final IOException e) {
             ctx.reportError("Invalid api regions");
         }
     }
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegions.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegions.java
index 00d939e..2dcf0d7 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegions.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegions.java
@@ -27,6 +27,7 @@ import java.util.Map;
 import javax.json.Json;
 import javax.json.JsonArray;
 import javax.json.JsonArrayBuilder;
+import javax.json.JsonException;
 import javax.json.JsonObject;
 import javax.json.JsonObjectBuilder;
 import javax.json.JsonReader;
@@ -116,8 +117,9 @@ public class ApiRegions {
      * Convert regions into json
      *
      * @return The json array
+     * @throws IOException If generating the JSON fails
      */
-    public JsonArray toJSONArray() {
+    public JsonArray toJSONArray() throws IOException {
         final JsonArrayBuilder arrayBuilder = Json.createArrayBuilder();
 
         for (final ApiRegion region : this.getRegions()) {
@@ -156,15 +158,14 @@ public class ApiRegions {
      * Convert regions into json
      *
      * @return The json array as a string
+     * @throws IOException If generating the JSON fails
      */
-    public String toJSON() {
+    public String toJSON() throws IOException {
         final JsonArray array = this.toJSONArray();
         try (final StringWriter stringWriter = new StringWriter();
                 final JsonWriter writer = Json.createWriter(stringWriter)) {
             writer.writeArray(array);
             return stringWriter.toString();
-        } catch (final IOException e) {
-            throw new IllegalStateException(e);
         }
     }
 
@@ -173,8 +174,9 @@ public class ApiRegions {
      *
      * @param json The json as a string
      * @return The api regions
+     * @throws IOException If the json could not be parsed
      */
-    public static ApiRegions parse(final String json) {
+    public static ApiRegions parse(final String json) throws IOException {
         try (final JsonReader reader = Json.createReader(new StringReader(json))) {
             return parse(reader.readArray());
         }
@@ -185,54 +187,58 @@ public class ApiRegions {
      *
      * @param json The json
      * @return The api regions
+     * @throws IOException If the json could not be parsed
      */
-    public static ApiRegions parse(final JsonArray json) {
-        final ApiRegions regions = new ApiRegions();
+    public static ApiRegions parse(final JsonArray json) throws IOException {
+        try {
+            final ApiRegions regions = new ApiRegions();
 
-        for (final JsonValue value : json) {
-            if (value.getValueType() != ValueType.OBJECT) {
-                throw new IllegalArgumentException("Illegal api regions json " + json);
-            }
-            final ApiRegion region = new ApiRegion();
-
-            final JsonObject obj = (JsonObject) value;
-            region.setName(obj.getString(NAME_KEY));
-
-            for(final Map.Entry<String, JsonValue> entry : obj.entrySet()) {
-                if ( NAME_KEY.equals(entry.getKey()) ) {
-                    region.setName(obj.getString(NAME_KEY));
-                } else if (entry.getKey().equals(EXPORTS_KEY)) {
-                    for (final JsonValue e : (JsonArray)entry.getValue()) {
-                        if (e.getValueType() == ValueType.STRING) {
-                            final String name = ((JsonString) e).getString();
-                            if (!name.startsWith("#")) {
+            for (final JsonValue value : json) {
+                if (value.getValueType() != ValueType.OBJECT) {
+                    throw new IOException("Illegal api regions json " + json);
+                }
+                final ApiRegion region = new ApiRegion();
+
+                final JsonObject obj = (JsonObject) value;
+                region.setName(obj.getString(NAME_KEY));
+
+                for (final Map.Entry<String, JsonValue> entry : obj.entrySet()) {
+                    if (NAME_KEY.equals(entry.getKey())) {
+                        region.setName(obj.getString(NAME_KEY));
+                    } else if (entry.getKey().equals(EXPORTS_KEY)) {
+                        for (final JsonValue e : (JsonArray) entry.getValue()) {
+                            if (e.getValueType() == ValueType.STRING) {
+                                final String name = ((JsonString) e).getString();
+                                if (!name.startsWith("#")) {
+                                    final ApiExport export = new ApiExport();
+                                    region.getExports().add(export);
+
+                                    export.setName(name);
+                                }
+                            } else if (e.getValueType() == ValueType.OBJECT) {
+                                final JsonObject expObj = (JsonObject) e;
                                 final ApiExport export = new ApiExport();
                                 region.getExports().add(export);
 
-                                export.setName(name);
-                            }
-                        } else if (e.getValueType() == ValueType.OBJECT) {
-                            final JsonObject expObj = (JsonObject) e;
-                            final ApiExport export = new ApiExport();
-                            region.getExports().add(export);
-
-                            export.setName(expObj.getString(NAME_KEY));
-                            export.setToggle(expObj.getString(TOGGLE_KEY, null));
-                            if (expObj.containsKey(PREVIOUS_KEY)) {
-                                export.setPrevious(ArtifactId.parse(expObj.getString(PREVIOUS_KEY)));
+                                export.setName(expObj.getString(NAME_KEY));
+                                export.setToggle(expObj.getString(TOGGLE_KEY, null));
+                                if (expObj.containsKey(PREVIOUS_KEY)) {
+                                    export.setPrevious(ArtifactId.parse(expObj.getString(PREVIOUS_KEY)));
+                                }
                             }
                         }
+                    } else {
+                        region.getProperties().put(entry.getKey(), ((JsonString) entry.getValue()).getString());
                     }
-                } else {
-                    region.getProperties().put(entry.getKey(), ((JsonString) entry.getValue()).getString());
+                }
+                if (!regions.addUniqueRegion(region)) {
+                    throw new IOException("Region " + region.getName() + " is defined twice");
                 }
             }
-            if (!regions.addUniqueRegion(region)) {
-                throw new IllegalArgumentException("Region " + region.getName() + " is defined twice");
-            }
-
+            return regions;
+        } catch (final JsonException | IllegalArgumentException e) {
+            throw new IOException(e);
         }
-        return regions;
     }
 
     @Override
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 aa15510..3eafb39 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
@@ -76,7 +76,7 @@ public class APIRegionMergeHandlerTest {
     }
 
     @Test
-    public void testAPIRegionMerging() {
+    public void testAPIRegionMerging() throws Exception {
         APIRegionMergeHandler armh = new APIRegionMergeHandler();
 
         Feature tf = new Feature(ArtifactId.fromMvnId("x:t:1"));