SLING-8783 : Create API for api regions
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 16560a0..feb37b9 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
@@ -74,9 +74,9 @@
final ApiRegion sourceRegion = srcRegions.getRegionByName(targetRegion.getName());
if (sourceRegion != null) {
srcRegions.remove(sourceRegion);
- for (final ApiExport srcExp : sourceRegion.getExports()) {
+ for (final ApiExport srcExp : sourceRegion.listExports()) {
if (targetRegion.getExportByName(srcExp.getName()) == null) {
- targetRegion.getExports().add(srcExp);
+ targetRegion.add(srcExp);
}
}
}
@@ -84,7 +84,7 @@
// If there are any remaining regions in the src extension, process them now
for (final ApiRegion r : srcRegions.listRegions()) {
- if (!targetRegions.addUniqueRegion(r)) {
+ if (!targetRegions.add(r)) {
throw new IllegalStateException("Duplicate region " + r.getName());
}
}
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegions.java b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegions.java
index 84d2ba7..088e105 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegions.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegions.java
@@ -51,7 +51,7 @@
for (final ApiRegion region : apiRegions.listRegions()) {
ApiExport export = region.getExportByName(exportedPackage);
if (export != null) {
- region.getExports().remove(export);
+ region.remove(export);
}
}
}
@@ -59,7 +59,7 @@
boolean isEmpty = true;
for (final ApiRegion region : apiRegions.listRegions()) {
- if (!region.getExports().isEmpty()) {
+ if (!region.listExports().isEmpty()) {
isEmpty = false;
break;
}
@@ -68,14 +68,14 @@
if (!isEmpty) {
// track a single error for each region
for (ApiRegion region : apiRegions.listRegions()) {
- if (!region.getExports().isEmpty()) {
+ if (!region.listExports().isEmpty()) {
Formatter formatter = new Formatter();
formatter.format("Region '%s' defined in feature '%s' declares %s package%s which %s not exported by any bundle:%n",
region.getName(),
ctx.getFeature().getId(),
- region.getExports().size(), getExtension(region.getExports(), "", "s"),
- getExtension(region.getExports(), "is", "are"));
- region.getExports().forEach(api -> formatter.format(" * %s%n", api.getName()));
+ region.listExports().size(), getExtension(region.listExports(), "", "s"),
+ getExtension(region.listExports(), "is", "are"));
+ region.listExports().forEach(api -> formatter.format(" * %s%n", api.getName()));
ctx.reportError(formatter.toString());
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsDuplicates.java b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsDuplicates.java
index 05cd8b4..25d12b5 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsDuplicates.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/analyser/CheckApiRegionsDuplicates.java
@@ -69,7 +69,7 @@
private static Set<String> calculateIntersection(ApiRegion source, ApiRegion target) {
final Set<String> intersection = new HashSet<>();
- for (ApiExport packageName : source.getExports()) {
+ for (ApiExport packageName : source.listExports()) {
if (target.getExportByName(packageName.getName()) != null) {
intersection.add(packageName.getName());
}
diff --git a/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiExport.java b/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiExport.java
index 2fcbd49..88ff28d 100644
--- a/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiExport.java
+++ b/src/main/java/org/apache/sling/feature/extension/apiregions/api/ApiExport.java
@@ -16,6 +16,9 @@
*/
package org.apache.sling.feature.extension.apiregions.api;
+import java.util.HashMap;
+import java.util.Map;
+
import org.apache.sling.feature.ArtifactId;
/**
@@ -23,47 +26,81 @@
*/
public class ApiExport {
- private volatile String name;
+ private final String name;
private volatile String toggle;
private volatile ArtifactId previous;
- public ApiExport() {
- // default
- }
+ private final Map<String, String> properties = new HashMap<>();
+ /**
+ * Create a new export
+ *
+ * @param name Package name for the export
+ */
public ApiExport(final String name) {
this.name = name;
}
+ /**
+ * Get the package name
+ *
+ * @return The package name
+ */
public String getName() {
return name;
}
- public void setName(String name) {
- this.name = name;
- }
-
+ /**
+ * Get the optional toggle information
+ *
+ * @return The toggle info or {@code null}
+ */
public String getToggle() {
return toggle;
}
+ /**
+ * Set the toggle info.
+ *
+ * @param toggle The toggle info
+ */
public void setToggle(String toggle) {
this.toggle = toggle;
}
+ /**
+ * Get the previous version of this api
+ *
+ * @return The previous version or {@code null}
+ */
public ArtifactId getPrevious() {
return previous;
}
+ /**
+ * Set the previous version
+ *
+ * @param previous Previus version
+ */
public void setPrevious(ArtifactId previous) {
this.previous = previous;
}
+ /**
+ * Get additional properties
+ *
+ * @return Modifiable map of properties
+ */
+ public Map<String, String> getProperties() {
+ return this.properties;
+ }
+
@Override
public String toString() {
- return "ApiExport [name=" + name + ", toggle=" + toggle + ", previous=" + previous + "]";
+ return "ApiExport [name=" + name + ", toggle=" + toggle + ", previous=" + previous + ", properties="
+ + properties + "]";
}
@Override
@@ -72,6 +109,7 @@
int result = 1;
result = prime * result + ((name == null) ? 0 : name.hashCode());
result = prime * result + ((previous == null) ? 0 : previous.hashCode());
+ result = prime * result + ((properties == null) ? 0 : properties.hashCode());
result = prime * result + ((toggle == null) ? 0 : toggle.hashCode());
return result;
}
@@ -95,6 +133,11 @@
return false;
} else if (!previous.equals(other.previous))
return false;
+ if (properties == null) {
+ if (other.properties != null)
+ return false;
+ } else if (!properties.equals(other.properties))
+ return false;
if (toggle == null) {
if (other.toggle != null)
return false;
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 a17af08..ba77277 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
@@ -57,12 +57,43 @@
}
/**
- * Modifiable collection of exports for this region
+ * Add the export. The export is only added if there isn't already a export with
+ * the same name
+ *
+ * @param export The export to add
+ * @return {@code true} if the export could be added, {@code false} otherwise
+ */
+ public boolean add(final ApiExport export) {
+ boolean found = false;
+ for (final ApiExport c : this.exports) {
+ if (c.getName().equals(export.getName())) {
+ found = true;
+ break;
+ }
+ }
+ if (!found) {
+ this.exports.add(export);
+ }
+ return !found;
+ }
+
+ /**
+ * Remove the export
+ *
+ * @param export export to remove
+ * @return {@code true} if the export got removed.
+ */
+ public boolean remove(final ApiExport export) {
+ return this.exports.remove(export);
+ }
+
+ /**
+ * Unmodifiable collection of exports for this region
*
* @return The collection of exports
*/
- public Collection<ApiExport> getExports() {
- return this.exports;
+ public Collection<ApiExport> listExports() {
+ return Collections.unmodifiableCollection(this.exports);
}
/**
@@ -70,10 +101,10 @@
*
* @return The collection of exports
*/
- public Collection<ApiExport> getAllExports() {
+ public Collection<ApiExport> listAllExports() {
final List<ApiExport> list = new ArrayList<>();
if (parent != null) {
- list.addAll(parent.getAllExports());
+ list.addAll(parent.listAllExports());
}
list.addAll(this.exports);
return Collections.unmodifiableCollection(list);
@@ -96,7 +127,7 @@
/**
* Get additional properties
- *
+ *
* @return Modifiable map of properties
*/
public Map<String, String> getProperties() {
@@ -105,7 +136,7 @@
/**
* Get the parent region
- *
+ *
* @return The parent region or {@code null}
*/
public ApiRegion getParent() {
@@ -114,7 +145,7 @@
/**
* Get the child region
- *
+ *
* @return The child region or {@code null}
*/
public ApiRegion getChild() {
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 d9966d1..86d700a 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
@@ -84,7 +84,7 @@
* @param region The region to add
* @return {@code true} if the region could be added, {@code false} otherwise
*/
- public boolean addUniqueRegion(final ApiRegion region) {
+ public boolean add(final ApiRegion region) {
boolean found = false;
for (final ApiRegion c : this.regions) {
if (c.getName().equals(region.getName())) {
@@ -172,18 +172,23 @@
final JsonObjectBuilder regionBuilder = Json.createObjectBuilder();
regionBuilder.add(NAME_KEY, region.getName());
- if (!region.getExports().isEmpty()) {
+ if (!region.listExports().isEmpty()) {
final JsonArrayBuilder expArrayBuilder = Json.createArrayBuilder();
- for (final ApiExport exp : region.getExports()) {
- if (exp.getToggle() == null) {
+ for (final ApiExport exp : region.listExports()) {
+ if (exp.getToggle() == null && exp.getPrevious() == null && exp.getProperties().isEmpty()) {
expArrayBuilder.add(exp.getName());
} else {
final JsonObjectBuilder expBuilder = Json.createObjectBuilder();
expBuilder.add(NAME_KEY, exp.getName());
- expBuilder.add(TOGGLE_KEY, exp.getToggle());
+ if (exp.getToggle() != null) {
+ expBuilder.add(TOGGLE_KEY, exp.getToggle());
+ }
if (exp.getPrevious() != null) {
expBuilder.add(PREVIOUS_KEY, exp.getPrevious().toMvnId());
}
+ for (final Map.Entry<String, String> entry : exp.getProperties().entrySet()) {
+ expBuilder.add(entry.getKey(), entry.getValue());
+ }
expArrayBuilder.add(expBuilder);
}
}
@@ -255,20 +260,30 @@
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);
+ final ApiExport export = new ApiExport(name);
+ if (!region.add(export)) {
+ throw new IOException("Export " + export.getName()
+ + " is defined twice in region " + region.getName());
+ }
}
} else if (e.getValueType() == ValueType.OBJECT) {
final JsonObject expObj = (JsonObject) e;
- final ApiExport export = new ApiExport();
- region.getExports().add(export);
+ final ApiExport export = new ApiExport(expObj.getString(NAME_KEY));
+ if (!region.add(export)) {
+ throw new IOException("Export " + export.getName() + " is defined twice in region "
+ + region.getName());
+ }
- 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)));
+ for (final String key : expObj.keySet()) {
+ if (NAME_KEY.equals(key)) {
+ continue; // already set
+ } else if (TOGGLE_KEY.equals(key)) {
+ export.setToggle(expObj.getString(key));
+ } else if (PREVIOUS_KEY.equals(key)) {
+ export.setPrevious(ArtifactId.parse(expObj.getString(key)));
+ } else {
+ export.getProperties().put(key, expObj.getString(key));
+ }
}
}
}
@@ -276,7 +291,7 @@
region.getProperties().put(entry.getKey(), ((JsonString) entry.getValue()).getString());
}
}
- if (!regions.addUniqueRegion(region)) {
+ if (!regions.add(region)) {
throw new IOException("Region " + region.getName() + " is defined twice");
}
}
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 2761e3a..04e967b 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
@@ -101,20 +101,20 @@
ApiRegions expected = new ApiRegions();
ApiRegion global = new ApiRegion("global");
- global.getExports().add(new ApiExport("a.b.c"));
- global.getExports().add(new ApiExport("d.e.f"));
- global.getExports().add(new ApiExport("test"));
- expected.addUniqueRegion(global);
+ global.add(new ApiExport("a.b.c"));
+ global.add(new ApiExport("d.e.f"));
+ global.add(new ApiExport("test"));
+ expected.add(global);
ApiRegion internal = new ApiRegion("internal");
- internal.getExports().add(new ApiExport("xyz"));
+ internal.add(new ApiExport("xyz"));
internal.getProperties().put("some-key", "some-val");
- expected.addUniqueRegion(internal);
+ expected.add(internal);
ApiRegion something = new ApiRegion("something");
- something.getExports().add(new ApiExport("a.ha"));
+ something.add(new ApiExport("a.ha"));
something.getProperties().put("my-key", "my-val");
- expected.addUniqueRegion(something);
+ expected.add(something);
ApiRegions created = ApiRegions.parse((JsonArray) tgEx.getJSONStructure());
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 e19893f..3db71ae 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
@@ -58,12 +58,12 @@
final ApiRegion global = regions.listRegions().get(0);
assertEquals("global", global.getName());
- assertEquals(2, global.getExports().size());
+ assertEquals(2, global.listExports().size());
final ApiRegion internal = regions.listRegions().get(1);
assertEquals("internal", internal.getName());
- assertEquals(1, internal.getExports().size());
+ assertEquals(1, internal.listExports().size());
}
@Test
@@ -76,11 +76,11 @@
final ApiRegion duplicate = new ApiRegion("two");
final ApiRegion other = new ApiRegion("other");
- assertTrue(regions.addUniqueRegion(one));
- assertTrue(regions.addUniqueRegion(two));
- assertTrue(regions.addUniqueRegion(three));
+ assertTrue(regions.add(one));
+ assertTrue(regions.add(two));
+ assertTrue(regions.add(three));
- assertFalse(regions.addUniqueRegion(duplicate));
+ assertFalse(regions.add(duplicate));
assertEquals(3, regions.listRegions().size());
@@ -106,34 +106,34 @@
final ApiRegions regions = new ApiRegions();
final ApiRegion one = new ApiRegion("one");
- one.getExports().add(new ApiExport("a"));
+ one.add(new ApiExport("a"));
final ApiRegion two = new ApiRegion("two");
- two.getExports().add(new ApiExport("b"));
+ two.add(new ApiExport("b"));
final ApiRegion three = new ApiRegion("three");
- three.getExports().add(new ApiExport("c"));
+ three.add(new ApiExport("c"));
- assertTrue(regions.addUniqueRegion(one));
- assertTrue(regions.addUniqueRegion(two));
- assertTrue(regions.addUniqueRegion(three));
+ assertTrue(regions.add(one));
+ assertTrue(regions.add(two));
+ assertTrue(regions.add(three));
- assertEquals(1, one.getAllExports().size());
- assertTrue(one.getAllExports().contains(new ApiExport("a")));
- assertEquals(1, one.getExports().size());
- assertTrue(one.getExports().contains(new ApiExport("a")));
+ assertEquals(1, one.listAllExports().size());
+ assertTrue(one.listAllExports().contains(new ApiExport("a")));
+ assertEquals(1, one.listExports().size());
+ assertTrue(one.listExports().contains(new ApiExport("a")));
- assertEquals(2, two.getAllExports().size());
- assertTrue(two.getAllExports().contains(new ApiExport("a")));
- assertTrue(two.getAllExports().contains(new ApiExport("b")));
- assertEquals(1, two.getExports().size());
- assertTrue(two.getExports().contains(new ApiExport("b")));
+ assertEquals(2, two.listAllExports().size());
+ assertTrue(two.listAllExports().contains(new ApiExport("a")));
+ assertTrue(two.listAllExports().contains(new ApiExport("b")));
+ assertEquals(1, two.listExports().size());
+ assertTrue(two.listExports().contains(new ApiExport("b")));
- assertEquals(3, three.getAllExports().size());
- assertTrue(three.getAllExports().contains(new ApiExport("a")));
- assertTrue(three.getAllExports().contains(new ApiExport("b")));
- assertTrue(three.getAllExports().contains(new ApiExport("c")));
- assertEquals(1, three.getExports().size());
- assertTrue(three.getExports().contains(new ApiExport("c")));
+ assertEquals(3, three.listAllExports().size());
+ assertTrue(three.listAllExports().contains(new ApiExport("a")));
+ assertTrue(three.listAllExports().contains(new ApiExport("b")));
+ assertTrue(three.listAllExports().contains(new ApiExport("c")));
+ assertEquals(1, three.listExports().size());
+ assertTrue(three.listExports().contains(new ApiExport("c")));
}
}
