You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by pa...@apache.org on 2020/01/20 16:00:21 UTC
[sling-org-apache-sling-feature-extension-apiregions] branch master updated: SLING-8970 - adding test & fixing logic to consider inherited exports… (#5)
This is an automated email from the ASF dual-hosted git repository.
pauls 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 1ba6cef SLING-8970 - adding test & fixing logic to consider inherited exports… (#5)
1ba6cef is described below
commit 1ba6cefbcabbb4d14bb06a7fbf13efb6e24fa168
Author: Karl Pauls <pa...@apache.org>
AuthorDate: Mon Jan 20 17:00:14 2020 +0100
SLING-8970 - adding test & fixing logic to consider inherited exports… (#5)
* SLING-8970 - adding test & fixing logic to consider inherited exports in region checks
* SLING-8970 - adding test & fixing logic to consider inherited exports in region checks
---
.../apiregions/APIRegionMergeHandler.java | 19 ++++---
.../CheckApiRegionsBundleExportsImports.java | 2 +-
.../extension/apiregions/api/ApiRegion.java | 31 +++++------
.../extension/apiregions/api/ApiRegions.java | 47 ++++------------
.../apiregions/launcher/LauncherProperties.java | 3 ++
.../CheckApiRegionsBundleExportsImportsTest.java | 35 ++++++++++++
.../extension/apiregions/api/TestApiRegions.java | 21 ++++----
.../launcher/LauncherPropertiesTest.java | 63 ++++++++++++++++++++++
8 files changed, 149 insertions(+), 72 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 a544954..b9975d4 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
@@ -63,7 +63,6 @@ public class APIRegionMergeHandler implements MergeHandler {
for (final ApiRegion targetRegion : targetRegions.listRegions()) {
final ApiRegion sourceRegion = srcRegions.getRegionByName(targetRegion.getName());
if (sourceRegion != null) {
- srcRegions.remove(sourceRegion);
for (final ApiExport srcExp : sourceRegion.listExports()) {
if (targetRegion.getExportByName(srcExp.getName()) == null) {
targetRegion.add(srcExp);
@@ -81,13 +80,17 @@ public class APIRegionMergeHandler implements MergeHandler {
// If there are any remaining regions in the src extension, process them now
for (final ApiRegion r : srcRegions.listRegions()) {
- LinkedHashSet<ArtifactId> origins = new LinkedHashSet<>(Arrays.asList(r.getFeatureOrigins()));
- if (origins.isEmpty()) {
- origins.add(source.getId());
- r.setFeatureOrigins(origins.toArray(new ArtifactId[0]));
- }
- if (!targetRegions.add(r)) {
- throw new IllegalStateException("Duplicate region " + r.getName());
+ if (targetRegions.getRegionByName(r.getName()) == null) {
+ LinkedHashSet<ArtifactId> origins = new LinkedHashSet<>(Arrays.asList(r.getFeatureOrigins()));
+ if (origins.isEmpty())
+ {
+ origins.add(source.getId());
+ r.setFeatureOrigins(origins.toArray(new ArtifactId[0]));
+ }
+ if (!targetRegions.add(r))
+ {
+ throw new IllegalStateException("Duplicate region " + r.getName());
+ }
}
}
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 d11e2a5..c25c5a6 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
@@ -340,7 +340,7 @@ public class CheckApiRegionsBundleExportsImports implements AnalyserTask {
for (String region : getBundleRegions(info, apiRegions, ignoreAPIRegions)) {
if (!NO_REGION.equals(region) &&
(apiRegions.getRegionByName(region) == null
- || apiRegions.getRegionByName(region).getExportByName(pck.getName()) == null))
+ || apiRegions.getRegionByName(region).getAllExportByName(pck.getName()) == null))
continue;
Set<String> regions = candidates.get(info);
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegion.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegion.java
index e7da0b2..dd9f4b2 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegion.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiRegion.java
@@ -47,8 +47,6 @@ public class ApiRegion {
private volatile ApiRegion parent;
- private volatile ApiRegion child;
-
/**
* Create a new named region
*
@@ -157,6 +155,21 @@ public class ApiRegion {
}
/**
+ * Get an export by name
+ *
+ * @param name package name
+ * @return The export or {@code null}
+ */
+ public ApiExport getAllExportByName(final String name) {
+ for (final ApiExport e : listAllExports()) {
+ if (e.getName().equals(name)) {
+ return e;
+ }
+ }
+ return null;
+ }
+
+ /**
* Get additional properties
*
* @return Modifiable map of properties
@@ -174,26 +187,14 @@ public class ApiRegion {
return this.parent;
}
- /**
- * Get the child region
- *
- * @return The child region or {@code null}
- */
- public ApiRegion getChild() {
- return this.child;
- }
-
void setParent(final ApiRegion region) {
this.parent = region;
}
- void setChild(final ApiRegion region) {
- this.child = region;
- }
@Override
public String toString() {
- return "ApiRegion [exports=" + exports + ", properties=" + properties + ", name=" + name + "]";
+ return "ApiRegion [exports=" + exports + ", properties=" + properties + ", name=" + name + ", feature-origins=" + origins + "]";
}
@Override
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 238b116..790d84c 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
@@ -20,6 +20,7 @@ import java.io.IOException;
import java.io.StringReader;
import java.io.StringWriter;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
@@ -97,46 +98,20 @@ public class ApiRegions {
* @return {@code true} if the region could be added, {@code false} otherwise
*/
public boolean add(final ApiRegion region) {
- boolean found = false;
for (final ApiRegion c : this.regions) {
if (c.getName().equals(region.getName())) {
- found = true;
- break;
- }
- }
- if (!found) {
- final ApiRegion parent = this.regions.isEmpty() ? null : this.regions.get(this.regions.size() - 1);
- this.regions.add(region);
- region.setParent(parent);
- region.setChild(null);
- if (parent != null) {
- parent.setChild(region);
- }
- }
- return !found;
- }
-
- /**
- * Remove the region
- *
- * @param region Region to remove
- * @return {@code true} if the region got removed.
- */
- public boolean remove(final ApiRegion region) {
- boolean result = this.regions.remove(region);
- if (result) {
- final ApiRegion parent = region.getParent();
- final ApiRegion child = region.getChild();
- if (parent != null) {
- parent.setChild(child);
- region.setParent(null);
- }
- if (child != null) {
- child.setParent(parent);
- region.setChild(null);
+ return false;
}
}
- return result;
+ this.regions.stream()
+ .filter(
+ existingRegion ->
+ Stream.of(existingRegion.getFeatureOrigins())
+ .anyMatch(Arrays.asList(region.getFeatureOrigins())::contains)
+ ).reduce((a,b) -> b).ifPresent(region::setParent);
+
+ this.regions.add(region);
+ return true;
}
/**
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/launcher/LauncherProperties.java b/src/main/java/org/apache/sling/feature/extension/apiregions/launcher/LauncherProperties.java
index 516ed09..955db85 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/launcher/LauncherProperties.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/launcher/LauncherProperties.java
@@ -111,6 +111,9 @@ public class LauncherProperties
regionNames = new HashSet<>();
}
regionNames.add(region.getName());
+ for (ApiRegion parent = region.getParent(); parent != null; parent = parent.getParent()) {
+ regionNames.add(parent.getName());
+ }
return regionNames;
});
}
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 a0492fa..3801cc7 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
@@ -336,6 +336,41 @@ public class CheckApiRegionsBundleExportsImportsTest {
Mockito.verify(ctx, Mockito.never()).reportWarning(Mockito.anyString());
}
+ @Test
+ /*
+ * Bundle 2 imports org.foo.b from bundle 1. Bundle 1 exports it in region1.
+ * Regions
+ */
+ public void testImportFromInheritedRegionSucceeds() throws Exception {
+ String exJson = "[" +
+ "{\"name\": \"region1\", \"exports\": [\"org.foo.b\"],\"feature-origins\":[\"f:f1:1\"]}," +
+ "{\"name\": \"region2\", \"exports\": [\"org.foo.c\"],\"feature-origins\":[\"f:f1:1\",\"f:f2:1\"]}," +
+ "{\"name\": \"region3\", \"exports\": [],\"feature-origins\":[\"f:f2:1\"]}" +
+ "]";
+
+ 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", f.getId());
+ fdAddBundle(fd, "g:b2:1", "test-bundle2.jar", ArtifactId.fromMvnId("f:f2:1"));
+
+ 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());
+
+ }
+
private void fdAddBundle(FeatureDescriptor fd, String id, String file, ArtifactId... origins) throws IOException {
Artifact artifact = new Artifact(ArtifactId.fromMvnId(id));
artifact.setFeatureOrigins(origins);
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/api/TestApiRegions.java b/src/test/java/org/apache/sling/feature/extension/apiregions/api/TestApiRegions.java
index ee152a2..0ad697c 100644
--- a/src/test/java/org/apache/sling/feature/extension/apiregions/api/TestApiRegions.java
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/api/TestApiRegions.java
@@ -22,6 +22,7 @@ import java.io.Reader;
import java.io.StringWriter;
import java.io.Writer;
+import org.apache.sling.feature.ArtifactId;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -69,11 +70,16 @@ public class TestApiRegions {
public void testOrdering() throws Exception {
final ApiRegions regions = new ApiRegions();
final ApiRegion one = new ApiRegion("one");
+ one.setFeatureOrigins(ArtifactId.fromMvnId("f:f1:1"));
final ApiRegion two = new ApiRegion("two");
+ two.setFeatureOrigins(ArtifactId.fromMvnId("f:f1:1"));
final ApiRegion three = new ApiRegion("three");
+ three.setFeatureOrigins(ArtifactId.fromMvnId("f:f1:1"));
final ApiRegion duplicate = new ApiRegion("two");
+ duplicate.setFeatureOrigins(ArtifactId.fromMvnId("f:f1:1"));
final ApiRegion other = new ApiRegion("other");
+ other.setFeatureOrigins(ArtifactId.fromMvnId("f:f1:1"));
assertTrue(regions.add(one));
assertTrue(regions.add(two));
@@ -84,20 +90,8 @@ public class TestApiRegions {
assertEquals(3, regions.listRegions().size());
assertNull(one.getParent());
- assertEquals(two, one.getChild());
assertEquals(one, two.getParent());
- assertEquals(three, two.getChild());
assertEquals(two, three.getParent());
- assertNull(three.getChild());
-
- assertFalse(regions.remove(other));
- assertTrue(regions.remove(two));
-
- assertEquals(2, regions.listRegions().size());
- assertNull(one.getParent());
- assertEquals(three, one.getChild());
- assertEquals(one, three.getParent());
- assertNull(three.getChild());
}
@Test
@@ -105,12 +99,15 @@ public class TestApiRegions {
final ApiRegions regions = new ApiRegions();
final ApiRegion one = new ApiRegion("one");
+ one.setFeatureOrigins(ArtifactId.fromMvnId("f:f1:1"));
one.add(new ApiExport("a"));
final ApiRegion two = new ApiRegion("two");
+ two.setFeatureOrigins(ArtifactId.fromMvnId("f:f1:1"));
two.add(new ApiExport("b"));
final ApiRegion three = new ApiRegion("three");
+ three.setFeatureOrigins(ArtifactId.fromMvnId("f:f1:1"));
three.add(new ApiExport("c"));
assertTrue(regions.add(one));
diff --git a/src/test/java/org/apache/sling/feature/extension/apiregions/launcher/LauncherPropertiesTest.java b/src/test/java/org/apache/sling/feature/extension/apiregions/launcher/LauncherPropertiesTest.java
new file mode 100644
index 0000000..6fe8b28
--- /dev/null
+++ b/src/test/java/org/apache/sling/feature/extension/apiregions/launcher/LauncherPropertiesTest.java
@@ -0,0 +1,63 @@
+/*
+ * 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.launcher;
+
+import java.io.IOException;
+import java.util.Properties;
+
+import org.apache.sling.feature.extension.apiregions.api.ApiRegions;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class LauncherPropertiesTest
+{
+ @Test
+ public void testInheritedFeature() throws IOException
+ {
+ ApiRegions apiRegions = ApiRegions.parse("[" +
+ "{\"name\": \"region1\", \"exports\": [\"org.foo.b\"],\"feature-origins\":[\"f:f1:1\"]}," +
+ "{\"name\": \"region2\", \"exports\": [\"org.foo.c\"],\"feature-origins\":[\"f:f1:1\",\"f:f2:1\"]}," +
+ "{\"name\": \"region3\", \"exports\": [],\"feature-origins\":[\"f:f2:1\"]}," +
+ "{\"name\": \"region4\", \"exports\": [\"org.foo.b\"],\"feature-origins\":[\"f:f1:1\"]}" +
+ "]");
+
+ Properties properties = LauncherProperties.getFeatureIDtoRegionsMap(apiRegions);
+
+ Assert.assertNotNull(properties);
+ Assert.assertEquals("region1,region2,region4", properties.getProperty("f:f1:1"));
+
+ Assert.assertEquals("region1,region2,region3", properties.getProperty("f:f2:1"));
+ }
+
+ @Test
+ public void testNotInheritedFeature() throws IOException
+ {
+ ApiRegions apiRegions = ApiRegions.parse("[" +
+ "{\"name\": \"region1\", \"exports\": [\"org.foo.b\"],\"feature-origins\":[\"f:f1:1\"]}," +
+ "{\"name\": \"region2\", \"exports\": [\"org.foo.c\"],\"feature-origins\":[\"f:f1:1\"]}," +
+ "{\"name\": \"region3\", \"exports\": [],\"feature-origins\":[\"f:f2:1\"]}," +
+ "{\"name\": \"region4\", \"exports\": [],\"feature-origins\":[\"f:f2:1\"]}" +
+ "]");
+
+ Properties properties = LauncherProperties.getFeatureIDtoRegionsMap(apiRegions);
+
+ Assert.assertNotNull(properties);
+ Assert.assertEquals("region1,region2", properties.getProperty("f:f1:1"));
+
+ Assert.assertEquals("region3,region4", properties.getProperty("f:f2:1"));
+ }
+}