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 2018/11/13 18:31:47 UTC
[sling-org-apache-sling-feature] branch master updated: SLING-8104
: Avoid magic when merging features. Make algorithm configurable through
context
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.git
The following commit(s) were added to refs/heads/master by this push:
new d485ee4 SLING-8104 : Avoid magic when merging features. Make algorithm configurable through context
d485ee4 is described below
commit d485ee4d8217ce3c44bb0fcdff885fa117d12d2b
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Tue Nov 13 19:31:38 2018 +0100
SLING-8104 : Avoid magic when merging features. Make algorithm configurable through context
---
.../sling/feature/builder/BuilderContext.java | 20 ++++++++-
.../apache/sling/feature/builder/BuilderUtil.java | 38 ++++++++---------
.../sling/feature/builder/FeatureBuilder.java | 8 ++--
.../sling/feature/builder/BuilderUtilTest.java | 48 +++++++++++-----------
4 files changed, 63 insertions(+), 51 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/builder/BuilderContext.java b/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
index a907c0f..991a242 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
@@ -16,8 +16,6 @@
*/
package org.apache.sling.feature.builder;
-import org.apache.sling.feature.KeyValueMap;
-
import java.util.Arrays;
import java.util.LinkedHashMap;
import java.util.List;
@@ -25,11 +23,17 @@ import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
+import org.apache.sling.feature.KeyValueMap;
+
/**
* Builder context holds services used by {@link FeatureBuilder}.
*/
public class BuilderContext {
+ enum ArtifactMergeAlgorithm {
+ LATEST, HIGHEST
+ }
+
private final ArtifactProvider artifactProvider;
private final FeatureProvider provider;
private final Map<String, Map<String, String>> extensionConfiguration = new ConcurrentHashMap<>();
@@ -38,6 +42,8 @@ public class BuilderContext {
private final KeyValueMap variables = new KeyValueMap();
private final Map<String, String> properties = new LinkedHashMap<>();
+ private ArtifactMergeAlgorithm merge = ArtifactMergeAlgorithm.HIGHEST;
+
/**
* Create a new context
*
@@ -82,6 +88,11 @@ public class BuilderContext {
return this;
}
+ public BuilderContext setMergeAlgorithm(final ArtifactMergeAlgorithm alg) {
+ this.merge = alg;
+ return this;
+ }
+
/**
* Obtain the handler configuration. The object returned can be modified to provide
* additional handler configurations.
@@ -112,6 +123,10 @@ public class BuilderContext {
return this.provider;
}
+ ArtifactMergeAlgorithm getMergeAlgorithm() {
+ return this.merge;
+ }
+
/**
* Get the list of merge extensions
* @return The list of merge extensions
@@ -137,6 +152,7 @@ public class BuilderContext {
final BuilderContext ctx = new BuilderContext(featureProvider, this.artifactProvider, this.variables, this.properties);
ctx.mergeExtensions.addAll(mergeExtensions);
ctx.postProcessExtensions.addAll(postProcessExtensions);
+ ctx.merge = this.merge;
return ctx;
}
}
diff --git a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
index 7cc6748..ab80ec2 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
@@ -16,17 +16,6 @@
*/
package org.apache.sling.feature.builder;
-import org.apache.sling.feature.Artifact;
-import org.apache.sling.feature.Bundles;
-import org.apache.sling.feature.Configuration;
-import org.apache.sling.feature.Configurations;
-import org.apache.sling.feature.Extension;
-import org.apache.sling.feature.Feature;
-import org.apache.sling.feature.FeatureConstants;
-import org.apache.sling.feature.KeyValueMap;
-import org.osgi.resource.Capability;
-import org.osgi.resource.Requirement;
-
import java.io.StringReader;
import java.io.StringWriter;
import java.util.ArrayList;
@@ -46,16 +35,23 @@ import javax.json.JsonValue;
import javax.json.JsonValue.ValueType;
import javax.json.JsonWriter;
+import org.apache.sling.feature.Artifact;
+import org.apache.sling.feature.Bundles;
+import org.apache.sling.feature.Configuration;
+import org.apache.sling.feature.Configurations;
+import org.apache.sling.feature.Extension;
+import org.apache.sling.feature.Feature;
+import org.apache.sling.feature.FeatureConstants;
+import org.apache.sling.feature.KeyValueMap;
+import org.apache.sling.feature.builder.BuilderContext.ArtifactMergeAlgorithm;
+import org.osgi.resource.Capability;
+import org.osgi.resource.Requirement;
+
/**
* Utility methods for the builders
*/
class BuilderUtil {
- enum ArtifactMerge {
- LATEST,
- HIGHEST
- }
-
static boolean contains(String key, Iterable<Map.Entry<String, String>> iterable) {
if (iterable != null) {
for (Map.Entry<String, String> entry : iterable) {
@@ -118,7 +114,7 @@ class BuilderUtil {
static void mergeBundles(final Bundles target,
final Bundles source,
final Feature originatingFeature,
- final ArtifactMerge artifactMergeAlg) {
+ final ArtifactMergeAlgorithm artifactMergeAlg) {
for(final Map.Entry<Integer, List<Artifact>> entry : source.getBundlesByStartOrder().entrySet()) {
for(final Artifact a : entry.getValue()) {
// Record the original feature of the bundle
@@ -128,7 +124,7 @@ class BuilderUtil {
// version handling - use provided algorithm
boolean replace = true;
- if ( artifactMergeAlg == ArtifactMerge.HIGHEST ) {
+ if (artifactMergeAlg == ArtifactMergeAlgorithm.HIGHEST) {
final Artifact existing = target.getSame(a.getId());
if ( existing != null && existing.getId().getOSGiVersion().compareTo(a.getId().getOSGiVersion()) > 0 ) {
replace = false;
@@ -190,7 +186,7 @@ class BuilderUtil {
// default merge for extensions
static void mergeExtensions(final Extension target,
final Extension source,
- final ArtifactMerge artifactMergeAlg) {
+ final ArtifactMergeAlgorithm artifactMergeAlg) {
switch ( target.getType() ) {
case TEXT : // simply append
target.setText(target.getText() + "\n" + source.getText());
@@ -232,7 +228,7 @@ class BuilderUtil {
case ARTIFACTS : for(final Artifact a : source.getArtifacts()) {
// use artifactMergeAlg
boolean replace = true;
- if ( artifactMergeAlg == ArtifactMerge.HIGHEST ) {
+ if (artifactMergeAlg == ArtifactMergeAlgorithm.HIGHEST) {
final Artifact existing = target.getArtifacts().getSame(a.getId());
if ( existing != null && existing.getId().getOSGiVersion().compareTo(a.getId().getOSGiVersion()) > 0 ) {
replace = false;
@@ -250,7 +246,7 @@ class BuilderUtil {
// extensions (add/merge)
static void mergeExtensions(final Feature target,
final Feature source,
- final ArtifactMerge artifactMergeAlg,
+ final ArtifactMergeAlgorithm artifactMergeAlg,
final BuilderContext context) {
for(final Extension ext : source.getExtensions()) {
boolean found = false;
diff --git a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
index 7038cb3..0fd9dbb 100644
--- a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
+++ b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
@@ -196,7 +196,7 @@ public abstract class FeatureBuilder {
}
usedFeatures.add(assembled.getId());
- merge(target, assembled, context, BuilderUtil.ArtifactMerge.HIGHEST);
+ merge(target, assembled, context, context.getMergeAlgorithm());
}
// append feature list in extension
@@ -321,9 +321,9 @@ public abstract class FeatureBuilder {
include(af, i);
// and now merge
- merge(result, af, context, BuilderUtil.ArtifactMerge.LATEST);
+ merge(result, af, context, BuilderContext.ArtifactMergeAlgorithm.LATEST);
- merge(result, feature, context, BuilderUtil.ArtifactMerge.LATEST);
+ merge(result, feature, context, BuilderContext.ArtifactMergeAlgorithm.LATEST);
}
processedFeatures.remove(feature.getId().toMvnId());
@@ -334,7 +334,7 @@ public abstract class FeatureBuilder {
private static void merge(final Feature target,
final Feature source,
final BuilderContext context,
- final BuilderUtil.ArtifactMerge mergeAlg) {
+ final BuilderContext.ArtifactMergeAlgorithm mergeAlg) {
BuilderUtil.mergeVariables(target.getVariables(), source.getVariables(), context);
BuilderUtil.mergeBundles(target.getBundles(), source.getBundles(), source, mergeAlg);
BuilderUtil.mergeConfigurations(target.getConfigurations(), source.getConfigurations());
diff --git a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
index ed02f1f..f49527c 100644
--- a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
@@ -16,16 +16,9 @@
*/
package org.apache.sling.feature.builder;
-import org.apache.sling.feature.Artifact;
-import org.apache.sling.feature.ArtifactId;
-import org.apache.sling.feature.Bundles;
-import org.apache.sling.feature.Extension;
-import org.apache.sling.feature.ExtensionType;
-import org.apache.sling.feature.Feature;
-import org.apache.sling.feature.KeyValueMap;
-import org.apache.sling.feature.builder.BuilderUtil.ArtifactMerge;
-import org.junit.Test;
-import org.mockito.Mockito;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
import java.io.StringReader;
import java.io.StringWriter;
@@ -46,9 +39,16 @@ import javax.json.JsonString;
import javax.json.JsonValue;
import javax.json.stream.JsonGenerator;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.fail;
+import org.apache.sling.feature.Artifact;
+import org.apache.sling.feature.ArtifactId;
+import org.apache.sling.feature.Bundles;
+import org.apache.sling.feature.Extension;
+import org.apache.sling.feature.ExtensionType;
+import org.apache.sling.feature.Feature;
+import org.apache.sling.feature.KeyValueMap;
+import org.apache.sling.feature.builder.BuilderContext.ArtifactMergeAlgorithm;
+import org.junit.Test;
+import org.mockito.Mockito;
public class BuilderUtilTest {
@@ -106,7 +106,7 @@ public class BuilderUtilTest {
final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
- BuilderUtil.mergeBundles(target, source, orgFeat, ArtifactMerge.HIGHEST);
+ BuilderUtil.mergeBundles(target, source, orgFeat, ArtifactMergeAlgorithm.HIGHEST);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
assertEquals(3, result.size());
@@ -134,7 +134,7 @@ public class BuilderUtilTest {
final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
- BuilderUtil.mergeBundles(target, source, orgFeat, ArtifactMerge.LATEST);
+ BuilderUtil.mergeBundles(target, source, orgFeat, ArtifactMergeAlgorithm.LATEST);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
assertEquals(3, result.size());
@@ -152,7 +152,7 @@ public class BuilderUtilTest {
source.add(createBundle("g/a/1.1", 2));
final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
- BuilderUtil.mergeBundles(target, source, orgFeat, ArtifactMerge.LATEST);
+ BuilderUtil.mergeBundles(target, source, orgFeat, ArtifactMergeAlgorithm.LATEST);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
assertEquals(1, result.size());
@@ -172,7 +172,7 @@ public class BuilderUtilTest {
source.add(createBundle("g/f/2.5", 3));
final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
- BuilderUtil.mergeBundles(target, source, orgFeat, ArtifactMerge.LATEST);
+ BuilderUtil.mergeBundles(target, source, orgFeat, ArtifactMergeAlgorithm.LATEST);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
assertEquals(6, result.size());
@@ -192,11 +192,11 @@ public class BuilderUtilTest {
source.add(createBundle("g/b/1.0", 1));
final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", "c1", "slingfeature"));
- BuilderUtil.mergeBundles(target, source, orgFeat, ArtifactMerge.LATEST);
+ BuilderUtil.mergeBundles(target, source, orgFeat, ArtifactMergeAlgorithm.LATEST);
final Bundles target2 = new Bundles();
final Feature orgFeat2 = new Feature(new ArtifactId("g", "a", "1", null, null));
- BuilderUtil.mergeBundles(target2, target, orgFeat2, ArtifactMerge.LATEST);
+ BuilderUtil.mergeBundles(target2, target, orgFeat2, ArtifactMergeAlgorithm.LATEST);
List<Entry<Integer, Artifact>> result = getBundles(target2);
assertEquals(2, result.size());
@@ -215,7 +215,7 @@ public class BuilderUtilTest {
source.setJSON("[\"source1\",\"source2\"]");
- BuilderUtil.mergeExtensions(target, source, ArtifactMerge.HIGHEST);
+ BuilderUtil.mergeExtensions(target, source, ArtifactMergeAlgorithm.HIGHEST);
assertEquals(target.getJSON(), "[\"target1\",\"target2\",\"source1\",\"source2\"]");
@@ -307,7 +307,7 @@ public class BuilderUtilTest {
Feature ft = new Feature(ArtifactId.fromMvnId("g:t:1"));
assertEquals("Precondition", 0, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ArtifactMerge.LATEST, ctx);
+ BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST, ctx);
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
@@ -331,7 +331,7 @@ public class BuilderUtilTest {
ft.getExtensions().add(et);
assertEquals("Precondition", 1, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ArtifactMerge.LATEST, ctx);
+ BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST, ctx);
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
@@ -358,7 +358,7 @@ public class BuilderUtilTest {
Feature ft = new Feature(ArtifactId.fromMvnId("g:t:1"));
assertEquals("Precondition", 0, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ArtifactMerge.LATEST, ctx);
+ BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST, ctx);
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
@@ -383,7 +383,7 @@ public class BuilderUtilTest {
ft.getExtensions().add(et);
assertEquals("Precondition", 1, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ArtifactMerge.LATEST, ctx);
+ BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST, ctx);
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);