[
https://issues.apache.org/jira/browse/SLING-8104?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16692278#comment-16692278
]
ASF GitHub Bot commented on SLING-8104:
---------------------------------------
bosschaert closed pull request #8: SLING-8104 Avoid magic when merging features
URL: https://github.com/apache/sling-org-apache-sling-feature/pull/8
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
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 1dd100c..f97d43d 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
@@ -29,10 +29,6 @@
*/
public class BuilderContext {
- enum ArtifactMergeAlgorithm {
- LATEST, HIGHEST
- }
-
/** The required feature provider */
private final FeatureProvider provider;
@@ -42,10 +38,10 @@
private final Map<String, Map<String,String>> extensionConfiguration = new
HashMap<>();
private final List<MergeHandler> mergeExtensions = new ArrayList<>();
private final List<PostProcessHandler> postProcessExtensions = new
ArrayList<>();
+ private final List<String> artifactsOverrides = new ArrayList<>();
private final Map<String,String> variables = new HashMap<>();
private final Map<String,String> frameworkProperties = new HashMap<>();
- private ArtifactMergeAlgorithm merge = ArtifactMergeAlgorithm.HIGHEST;
/**
* Create a new context
@@ -93,6 +89,17 @@ public BuilderContext addFrameworkPropertiesOverwrites(final
Map<String,String>
return this;
}
+ /**
+ * Add overrides for artifact clashes
+ *
+ * @param overrides The overwrites
+ * @return The builder context
+ */
+ public BuilderContext addArtifactsOverrides(final List<String> overrides) {
+ this.artifactsOverrides.addAll(overrides);
+ return this;
+ }
+
/**
* Add merge extensions
*
@@ -115,17 +122,6 @@ public BuilderContext addPostProcessExtensions(final
PostProcessHandler... exten
return this;
}
- /**
- * Set the merge algorithm
- *
- * @param alg The algorithm
- * @return The builder context
- */
- public BuilderContext setMergeAlgorithm(final ArtifactMergeAlgorithm alg) {
- this.merge = alg;
- return this;
- }
-
/**
* Set a handler configuration
*
@@ -152,8 +148,12 @@ ArtifactProvider getArtifactProvider() {
return this.artifactProvider;
}
+ List<String> getArtifactOverrides() {
+ return this.artifactsOverrides;
+ }
+
Map<String,String> getVariablesOverwrites() {
- return this.variables;
+ return this.variables;
}
Map<String,String> getFrameworkPropertiesOverwrites() {
@@ -168,10 +168,6 @@ FeatureProvider getFeatureProvider() {
return this.provider;
}
- ArtifactMergeAlgorithm getMergeAlgorithm() {
- return this.merge;
- }
-
/**
* Get the list of merge extensions
* @return The list of merge extensions
@@ -196,11 +192,11 @@ ArtifactMergeAlgorithm getMergeAlgorithm() {
BuilderContext clone(final FeatureProvider featureProvider) {
final BuilderContext ctx = new BuilderContext(featureProvider);
ctx.setArtifactProvider(this.artifactProvider);
+ ctx.artifactsOverrides.addAll(this.artifactsOverrides);
ctx.variables.putAll(this.variables);
ctx.frameworkProperties.putAll(this.frameworkProperties);
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 ad804d9..4af8580 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
@@ -23,13 +23,15 @@
import org.apache.sling.feature.Extension;
import org.apache.sling.feature.Feature;
import org.apache.sling.feature.FeatureConstants;
-import org.apache.sling.feature.builder.BuilderContext.ArtifactMergeAlgorithm;
+import org.osgi.framework.Version;
import org.osgi.resource.Capability;
import org.osgi.resource.Requirement;
import java.io.StringReader;
import java.io.StringWriter;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
@@ -50,6 +52,7 @@
* Utility methods for the builders
*/
class BuilderUtil {
+ static final String CATCHALL_OVERRIDE = "*:*:";
static boolean contains(String key, Iterable<Map.Entry<String, String>>
iterable) {
if (iterable != null) {
@@ -115,32 +118,40 @@ static void mergeVariables(Map<String,String> target,
Map<String,String> source,
*
* @param target The target bundles
* @param source The source bundles
- * @param originatingFeature Optional, if set origin will be recorded
+ * @param sourceFeature Optional, if set origin will be recorded
* @param artifactMergeAlg Algorithm used to merge the artifacts
*/
static void mergeBundles(final Bundles target,
final Bundles source,
- final Feature originatingFeature,
- final ArtifactMergeAlgorithm artifactMergeAlg) {
+ final Feature sourceFeature,
+ final List<String> artifactOverrides) {
for(final Map.Entry<Integer, List<Artifact>> entry :
source.getBundlesByStartOrder().entrySet()) {
for(final Artifact a : entry.getValue()) {
- // version handling - use provided algorithm
- boolean replace = true;
- if (artifactMergeAlg == ArtifactMergeAlgorithm.HIGHEST) {
- final Artifact existing = target.getSame(a.getId());
- if ( existing != null &&
existing.getId().getOSGiVersion().compareTo(a.getId().getOSGiVersion()) > 0 ) {
- replace = false;
+ Artifact existing = target.getSame(a.getId());
+ List<Artifact> selectedArtifacts = null;
+ if (existing != null) {
+ if (sourceFeature.getId().toMvnId().equals(
+
existing.getMetadata().get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE))) {
+ // If the source artifact came from the same feature,
keep them side-by-side
+ selectedArtifacts = Arrays.asList(existing, a);
+ } else {
+ selectedArtifacts = selectArtifactOverride(existing,
a, artifactOverrides);
+ while(target.removeSame(existing.getId())) {
+ // Keep executing removeSame() which ignores the
version until last one was removed
+ }
}
+ } else {
+ selectedArtifacts = Collections.singletonList(a);
}
- if ( replace ) {
- target.removeSame(a.getId());
+
+ for (Artifact sa : selectedArtifacts) {
// create a copy to detach artifact from source
- final Artifact cp = a.copy(a.getId());
+ final Artifact cp = sa.copy(sa.getId());
// Record the original feature of the bundle
- if (originatingFeature != null
- &&
a.getMetadata().get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE) == null) {
+ if (sourceFeature != null && source.contains(sa)
+ &&
sa.getMetadata().get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE) == null) {
cp.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
- originatingFeature.getId().toMvnId());
+ sourceFeature.getId().toMvnId());
}
target.add(cp);
}
@@ -148,6 +159,51 @@ static void mergeBundles(final Bundles target,
}
}
+ static List<Artifact> selectArtifactOverride(Artifact a1, Artifact a2,
List<String> artifactOverrides) {
+ if (a1.getId().equals(a2.getId())) {
+ // They're the same so return one of them
+ return Collections.singletonList(a2);
+ }
+
+ String a1gid = a1.getId().getGroupId();
+ String a1aid = a1.getId().getArtifactId();
+ String a2gid = a2.getId().getGroupId();
+ String a2aid = a2.getId().getArtifactId();
+
+ if (!a1gid.equals(a2gid))
+ throw new IllegalStateException("Artifacts must have the same
group ID: " + a1 + " and " + a2);
+ if (!a1aid.equals(a2aid))
+ throw new IllegalStateException("Artifacts must have the same
artifact ID: " + a1 + " and " + a2);
+
+ String prefix = a1gid + ":" + a1aid + ":";
+ for (String o : artifactOverrides) {
+ if (o.startsWith(prefix) || o.startsWith(CATCHALL_OVERRIDE)) {
+ int idx = o.lastIndexOf(':');
+ if (idx <= 0 || o.length() <= idx)
+ continue;
+
+ String rule = o.substring(idx+1).trim();
+
+ if ("ALL".equals(rule)) {
+ return Arrays.asList(a1, a2);
+ } else if ("HIGHEST".equals(rule)) {
+ Version a1v = a1.getId().getOSGiVersion();
+ Version a2v = a2.getId().getOSGiVersion();
+ return a1v.compareTo(a2v) > 0 ?
Collections.singletonList(a1) : Collections.singletonList(a2);
+ } else if ("LATEST".equals(rule)) {
+ return Collections.singletonList(a2);
+ } else if (a1.getId().getVersion().equals(rule)) {
+ return Collections.singletonList(a1);
+ } else if (a2.getId().getVersion().equals(rule)) {
+ return Collections.singletonList(a2);
+ }
+ throw new IllegalStateException("Override rule " + o + " not
applicable to artifacts " + a1 + " and " + a2);
+ }
+ }
+ throw new IllegalStateException("Artifact override rule required to
select between these two artifacts " +
+ a1 + " and " + a2);
+ }
+
// configurations - merge / override
static void mergeConfigurations(final Configurations target, final
Configurations source) {
for(final Configuration cfg : source) {
@@ -199,13 +255,13 @@ static void mergeCapabilities(final List<Capability>
target, final List<Capabili
*
* @param target The target extension
* @param source The source extension
- * @param originatingFeature Optional, if set origin will be recorded for
- * artifacts
+ * @param originatingFeature Optional, if set origin will be recorded for
artifacts
* @param artifactMergeAlg The merge algorithm for artifacts
*/
static void mergeExtensions(final Extension target,
- final Extension source, final Feature originatingFeature,
- final ArtifactMergeAlgorithm artifactMergeAlg) {
+ final Extension source,
+ final Feature sourceFeature,
+ final List<String> artifactOverrides) {
switch ( target.getType() ) {
case TEXT : // simply append
target.setText(target.getText() + "\n" + source.getText());
@@ -245,25 +301,32 @@ static void mergeExtensions(final Extension target,
break;
case ARTIFACTS:
- for (final Artifact a : source.getArtifacts()) {
- // use artifactMergeAlg
- boolean replace = true;
- 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;
+ for(final Artifact a : source.getArtifacts()) {
+ Artifact existing = target.getArtifacts().getSame(a.getId());
+ List<Artifact> selectedArtifacts = null;
+ if (existing != null) {
+ if (sourceFeature.getId().toMvnId().equals(
+
existing.getMetadata().get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE))) {
+ // If the source artifact came from the same feature,
keep them side-by-side
+ selectedArtifacts = Arrays.asList(existing, a);
+ } else {
+ selectedArtifacts = selectArtifactOverride(existing,
a, artifactOverrides);
+
while(target.getArtifacts().removeSame(existing.getId())) {
+ // Keep executing removeSame() which ignores the
version until last one was removed
+ }
}
+ } else {
+ selectedArtifacts = Collections.singletonList(a);
}
- if (replace) {
- target.getArtifacts().removeSame(a.getId());
+
+ for (Artifact sa : selectedArtifacts) {
// create a copy to detach artifact from source
- final Artifact cp = a.copy(a.getId());
- // Record the original feature of the artifact
- if (originatingFeature != null
- &&
a.getMetadata().get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE) == null) {
+ final Artifact cp = sa.copy(sa.getId());
+ // Record the original feature of the bundle
+ if (sourceFeature != null &&
source.getArtifacts().contains(sa)
+ &&
sa.getMetadata().get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE) == null) {
cp.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
- originatingFeature.getId().toMvnId());
+ sourceFeature.getId().toMvnId());
}
target.getArtifacts().add(cp);
}
@@ -275,7 +338,8 @@ static void mergeExtensions(final Extension target,
// extensions (add/merge)
static void mergeExtensions(final Feature target,
final Feature source,
- final ArtifactMergeAlgorithm artifactMergeAlg, final
BuilderContext context, final boolean recordOrigin) {
+ final BuilderContext context,
+ final List<String> artifactOverrides) {
for(final Extension ext : source.getExtensions()) {
boolean found = false;
@@ -297,7 +361,7 @@ static void mergeExtensions(final Feature target,
}
if ( !handled ) {
// default merge
- mergeExtensions(current, ext, recordOrigin ? source :
null, artifactMergeAlg);
+ mergeExtensions(current, ext, source,
artifactOverrides);
}
}
}
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 4668fa0..c23b02a 100644
--- a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
+++ b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
@@ -195,7 +195,7 @@ public static Feature assemble(
}
usedFeatures.add(assembled.getId());
- merge(target, assembled, context, context.getMergeAlgorithm(),
true);
+ merge(target, assembled, context, context.getArtifactOverrides());
}
// append feature list in extension
@@ -321,10 +321,10 @@ private static Feature internalAssemble(final
List<String> processedFeatures,
// process include instructions
processInclude(includedFeature, i);
- // and now merge
- merge(result, includedFeature, context,
BuilderContext.ArtifactMergeAlgorithm.LATEST, true);
-
- merge(result, feature, context,
BuilderContext.ArtifactMergeAlgorithm.LATEST, false);
+ // and now merge the included feature into the result. No
overrides should be needed since the result is empty before
+ merge(result, includedFeature, context, Collections.emptyList());
+ // and merge the current feature over the included feature into
the result
+ merge(result, feature, context,
Collections.singletonList("*:*:LATEST"));
}
processedFeatures.remove(feature.getId().toMvnId());
@@ -335,17 +335,14 @@ private static Feature internalAssemble(final
List<String> processedFeatures,
private static void merge(final Feature target,
final Feature source,
final BuilderContext context,
- final BuilderContext.ArtifactMergeAlgorithm mergeAlg, final
boolean recordOrigin) {
+ final List<String> artifactOverrides) {
BuilderUtil.mergeVariables(target.getVariables(),
source.getVariables(), context);
- BuilderUtil.mergeBundles(target.getBundles(), source.getBundles(),
recordOrigin ? source : null, mergeAlg);
+ BuilderUtil.mergeBundles(target.getBundles(), source.getBundles(),
source, artifactOverrides);
BuilderUtil.mergeConfigurations(target.getConfigurations(),
source.getConfigurations());
BuilderUtil.mergeFrameworkProperties(target.getFrameworkProperties(),
source.getFrameworkProperties(), context);
BuilderUtil.mergeRequirements(target.getRequirements(),
source.getRequirements());
BuilderUtil.mergeCapabilities(target.getCapabilities(),
source.getCapabilities());
- BuilderUtil.mergeExtensions(target,
- source,
- mergeAlg,
- context, recordOrigin);
+ BuilderUtil.mergeExtensions(target, source, context,
artifactOverrides);
}
/**
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 5fd187d..23f3338 100644
--- a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
@@ -22,7 +22,6 @@
import org.apache.sling.feature.Extension;
import org.apache.sling.feature.ExtensionType;
import org.apache.sling.feature.Feature;
-import org.apache.sling.feature.builder.BuilderContext.ArtifactMergeAlgorithm;
import org.junit.Test;
import org.mockito.Mockito;
@@ -30,6 +29,7 @@
import java.io.StringWriter;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -105,7 +105,8 @@ private int assertContains(final List<Map.Entry<Integer,
Artifact>> bundles,
final Feature orgFeat = new Feature(new ArtifactId("gid", "aid",
"123", null, null));
- BuilderUtil.mergeBundles(target, source, orgFeat,
ArtifactMergeAlgorithm.HIGHEST);
+ List<String> overrides = Arrays.asList("g:a:HIGHEST", "g:b:HIGHEST");
+ BuilderUtil.mergeBundles(target, source, orgFeat, overrides);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
assertEquals(3, result.size());
@@ -133,7 +134,8 @@ private int assertContains(final List<Map.Entry<Integer,
Artifact>> bundles,
final Feature orgFeat = new Feature(new ArtifactId("gid", "aid",
"123", null, null));
- BuilderUtil.mergeBundles(target, source, orgFeat,
ArtifactMergeAlgorithm.LATEST);
+ List<String> overrides = Arrays.asList("g:a:LATEST", "g:b:LATEST");
+ BuilderUtil.mergeBundles(target, source, orgFeat, overrides);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
assertEquals(3, result.size());
@@ -151,7 +153,8 @@ private int assertContains(final List<Map.Entry<Integer,
Artifact>> bundles,
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,
ArtifactMergeAlgorithm.LATEST);
+ List<String> overrides = Arrays.asList("g:a:LATEST", "g:b:LATEST");
+ BuilderUtil.mergeBundles(target, source, orgFeat, overrides);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
assertEquals(1, result.size());
@@ -171,7 +174,7 @@ private int assertContains(final List<Map.Entry<Integer,
Artifact>> bundles,
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,
ArtifactMergeAlgorithm.LATEST);
+ BuilderUtil.mergeBundles(target, source, orgFeat, new ArrayList<>());
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
assertEquals(6, result.size());
@@ -191,11 +194,11 @@ private int assertContains(final List<Map.Entry<Integer,
Artifact>> bundles,
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,
ArtifactMergeAlgorithm.LATEST);
+ BuilderUtil.mergeBundles(target, source, orgFeat, new ArrayList<>());
final Bundles target2 = new Bundles();
final Feature orgFeat2 = new Feature(new ArtifactId("g", "a", "1",
null, null));
- BuilderUtil.mergeBundles(target2, target, orgFeat2,
ArtifactMergeAlgorithm.LATEST);
+ BuilderUtil.mergeBundles(target2, target, orgFeat2, new ArrayList<>());
List<Entry<Integer, Artifact>> result = getBundles(target2);
assertEquals(2, result.size());
@@ -214,7 +217,7 @@ private int assertContains(final List<Map.Entry<Integer,
Artifact>> bundles,
source.setJSON("[\"source1\",\"source2\"]");
- BuilderUtil.mergeExtensions(target, source, null,
ArtifactMergeAlgorithm.HIGHEST);
+ BuilderUtil.mergeExtensions(target, source, null, new ArrayList<>());
assertEquals(target.getJSON(),
"[\"target1\",\"target2\",\"source1\",\"source2\"]");
@@ -306,7 +309,7 @@ static void copyJsonObject(JsonObject obj, JsonGenerator
gen, String ... exclusi
Feature ft = new Feature(ArtifactId.fromMvnId("g:t:1"));
assertEquals("Precondition", 0, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST,
ctx, true);
+ BuilderUtil.mergeExtensions(ft, fs, ctx, new ArrayList<>());
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
@@ -330,7 +333,7 @@ static void copyJsonObject(JsonObject obj, JsonGenerator
gen, String ... exclusi
ft.getExtensions().add(et);
assertEquals("Precondition", 1, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST,
ctx, true);
+ BuilderUtil.mergeExtensions(ft, fs, ctx, new ArrayList<>());
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
@@ -357,7 +360,7 @@ static void copyJsonObject(JsonObject obj, JsonGenerator
gen, String ... exclusi
Feature ft = new Feature(ArtifactId.fromMvnId("g:t:1"));
assertEquals("Precondition", 0, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST,
ctx, true);
+ BuilderUtil.mergeExtensions(ft, fs, ctx, new ArrayList<>());
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
@@ -382,7 +385,7 @@ static void copyJsonObject(JsonObject obj, JsonGenerator
gen, String ... exclusi
ft.getExtensions().add(et);
assertEquals("Precondition", 1, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST,
ctx, true);
+ BuilderUtil.mergeExtensions(ft, fs, ctx, new ArrayList<>());
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
@@ -393,6 +396,80 @@ static void copyJsonObject(JsonObject obj, JsonGenerator
gen, String ... exclusi
assertEquals(er.readObject(), ar.readObject());
}
+ @Test public void testSelectArtifactOverrideAll() {
+ Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
+ Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2"));
+ List<String> overrides = Arrays.asList("gid:aid2:1", "gid:aid:ALL ");
+ assertEquals(Arrays.asList(a1, a2),
BuilderUtil.selectArtifactOverride(a1, a2, overrides));
+ }
+
+ @Test public void testSelectArtifactOverrideIdenticalNeedsNoRule() {
+ Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
+ Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
+ assertEquals(Collections.singletonList(a1),
BuilderUtil.selectArtifactOverride(a1, a2, Collections.emptyList()));
+ }
+
+ @Test public void testSelectArtifactOverride1() {
+ Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
+ Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2"));
+ List<String> overrides = Collections.singletonList("gid:aid:1");
+ assertEquals(Collections.singletonList(a1),
BuilderUtil.selectArtifactOverride(a1, a2, overrides));
+ }
+
+ @Test public void testSelectArtifactOverride2() {
+ Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
+ Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2"));
+ List<String> overrides = Collections.singletonList("gid:aid:2");
+ assertEquals(Collections.singletonList(a2),
BuilderUtil.selectArtifactOverride(a1, a2, overrides));
+ }
+
+ @Test(expected=IllegalStateException.class)
+ public void testSelectArtifactOverride3() {
+ Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
+ Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2"));
+ List<String> overrides = Collections.singletonList("gid:aid:3");
+ BuilderUtil.selectArtifactOverride(a1, a2, overrides);
+ }
+
+ @Test(expected=IllegalStateException.class)
+ public void testSelectArtifactOverrideDifferentGroupID() {
+ Artifact a1 = new Artifact(ArtifactId.fromMvnId("aid:aid:1"));
+ Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2"));
+ List<String> overrides = Collections.singletonList("gid:aid:2");
+ BuilderUtil.selectArtifactOverride(a1, a2, overrides);
+ }
+
+ @Test(expected=IllegalStateException.class)
+ public void testSelectArtifactOverrideDifferentArtifactID() {
+ Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:gid:1"));
+ Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2"));
+ List<String> overrides = Collections.singletonList("gid:aid:2");
+ BuilderUtil.selectArtifactOverride(a1, a2, overrides);
+ }
+
+ @Test(expected=IllegalStateException.class)
+ public void testSelectArtifactOverrideDifferentNoRule() {
+ Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
+ Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2"));
+ BuilderUtil.selectArtifactOverride(a1, a2, Collections.emptyList());
+ }
+
+ @Test public void testSelectArtifactOverrideHigest() {
+ Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1.1"));
+ Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2.0.1"));
+ List<String> overrides = Collections.singletonList("gid:aid:HIGHEST");
+ assertEquals(Collections.singletonList(a2),
BuilderUtil.selectArtifactOverride(a1, a2, overrides));
+ assertEquals(Collections.singletonList(a2),
BuilderUtil.selectArtifactOverride(a2, a1, overrides));
+ }
+
+ @Test public void testSelectArtifactOverrideLatest() {
+ Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1.1"));
+ Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2.0.1"));
+ List<String> overrides = Collections.singletonList("gid:aid:LATEST");
+ assertEquals(Collections.singletonList(a2),
BuilderUtil.selectArtifactOverride(a1, a2, overrides));
+ assertEquals(Collections.singletonList(a1),
BuilderUtil.selectArtifactOverride(a2, a1, overrides));
+ }
+
@SafeVarargs
public static Artifact createBundle(final String id, final int startOrder,
Map.Entry<String, String> ... metadata) {
final Artifact a = new Artifact(ArtifactId.parse(id));
diff --git
a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
index dd22ac6..786a175 100644
--- a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
@@ -71,6 +71,25 @@
FEATURES.put(f1.getId().toMvnId(), f1);
}
+ static {
+ final Feature f2 = new Feature(ArtifactId.parse("g/a/2"));
+
+ f2.getBundles().add(BuilderUtilTest.createBundle("group/testmulti/1",
4));
+ f2.getBundles().add(BuilderUtilTest.createBundle("group/testmulti/2",
8));
+
f2.getBundles().add(BuilderUtilTest.createBundle("group/someart/1.2.3", 4));
+
+ FEATURES.put(f2.getId().toMvnId(), f2);
+ }
+
+ static {
+ final Feature f2 = new Feature(ArtifactId.parse("g/a/3"));
+
+ f2.getBundles().add(BuilderUtilTest.createBundle("group/testmulti/2",
8));
+
f2.getBundles().add(BuilderUtilTest.createBundle("group/someart/1.2.3", 4));
+
+ FEATURES.put(f2.getId().toMvnId(), f2);
+ }
+
private final FeatureProvider provider = new FeatureProvider() {
@Override
@@ -311,13 +330,127 @@ private void equals(final Feature expected, final
Feature actuals) {
co3.getProperties().put("prop", "value");
result.getConfigurations().add(co3);
+ BuilderContext builderContext = new BuilderContext(provider);
+
// assemble
- final Feature assembled = FeatureBuilder.assemble(base, new
BuilderContext(provider));
+ final Feature assembled = FeatureBuilder.assemble(base,
builderContext);
// and test
equals(result, assembled);
}
+ @Test public void testSingleIncludeMultiVersion() {
+ Feature base = new Feature(ArtifactId.fromMvnId("g:tgtart:1"));
+ Include i1 = new Include(ArtifactId.fromMvnId("g:a:3"));
+ base.setInclude(i1);
+ base.getBundles().add(new Artifact(ArtifactId.fromMvnId("g:myart:1")));
+ base.getBundles().add(new
Artifact(ArtifactId.fromMvnId("group:testmulti:1")));
+ base.getBundles().add(new
Artifact(ArtifactId.fromMvnId("group:testmulti:3")));
+
+ BuilderContext builderContext = new BuilderContext(provider);
+ Feature assembled = FeatureBuilder.assemble(base, builderContext);
+
+ Feature result = new Feature(ArtifactId.parse("g:tgtart:1"));
+ Artifact b0 = new Artifact(ArtifactId.fromMvnId("g:myart:1"));
+ b0.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:tgtart:1");
+ result.getBundles().add(b0);
+ Artifact b1 = new Artifact(ArtifactId.fromMvnId("group:testmulti:1"));
+ b1.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:tgtart:1");
+ result.getBundles().add(b1);
+ Artifact b2 = new Artifact(ArtifactId.fromMvnId("group:testmulti:3"));
+ b2.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:tgtart:1");
+ result.getBundles().add(b2);
+ Artifact b3 = new
Artifact(ArtifactId.fromMvnId("group:someart:1.2.3"));
+ b3.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:a:3");
+ b3.setStartOrder(4);
+ result.getBundles().add(b3);
+
+ equals(result, assembled);
+ }
+
+ @Test public void testSingleIncludeMultiVersion2() {
+ Feature base = new Feature(ArtifactId.fromMvnId("g:tgtart:1"));
+ Include i1 = new Include(ArtifactId.fromMvnId("g:a:2"));
+ base.setInclude(i1);
+ base.getBundles().add(new Artifact(ArtifactId.fromMvnId("g:myart:1")));
+
+ BuilderContext builderContext = new BuilderContext(provider);
+ Feature assembled = FeatureBuilder.assemble(base, builderContext);
+
+ Feature result = new Feature(ArtifactId.parse("g:tgtart:1"));
+ Artifact b0 = new Artifact(ArtifactId.fromMvnId("g:myart:1"));
+ b0.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:tgtart:1");
+ result.getBundles().add(b0);
+ Artifact b1 = new Artifact(ArtifactId.fromMvnId("group:testmulti:1"));
+ b1.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:a:2");
+ b1.setStartOrder(4);
+ result.getBundles().add(b1);
+ Artifact b2 = new Artifact(ArtifactId.fromMvnId("group:testmulti:2"));
+ b2.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:a:2");
+ b2.setStartOrder(8);
+ result.getBundles().add(b2);
+ Artifact b3 = new
Artifact(ArtifactId.fromMvnId("group:someart:1.2.3"));
+ b3.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:a:2");
+ b3.setStartOrder(4);
+ result.getBundles().add(b3);
+
+ equals(result, assembled);
+ }
+
+ @Test public void testSingleIncludeMultiVersion3() {
+ Feature base = new Feature(ArtifactId.fromMvnId("g:tgtart:1"));
+ Include i1 = new Include(ArtifactId.fromMvnId("g:a:2"));
+ base.setInclude(i1);
+ base.getBundles().add(new Artifact(ArtifactId.fromMvnId("g:myart:1")));
+ base.getBundles().add(new
Artifact(ArtifactId.fromMvnId("group:testmulti:1")));
+
+ BuilderContext builderContext = new BuilderContext(provider);
+ Feature assembled = FeatureBuilder.assemble(base, builderContext);
+
+ Feature result = new Feature(ArtifactId.parse("g:tgtart:1"));
+ Artifact b0 = new Artifact(ArtifactId.fromMvnId("g:myart:1"));
+ b0.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:tgtart:1");
+ result.getBundles().add(b0);
+ Artifact b1 = new Artifact(ArtifactId.fromMvnId("group:testmulti:1"));
+ b1.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:tgtart:1");
+ result.getBundles().add(b1);
+ Artifact b3 = new
Artifact(ArtifactId.fromMvnId("group:someart:1.2.3"));
+ b3.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:a:2");
+ b3.setStartOrder(4);
+ result.getBundles().add(b3);
+
+ equals(result, assembled);
+ }
+
+ @Test public void testSingleIncludeMultiVersion4() {
+ Feature base = new Feature(ArtifactId.fromMvnId("g:tgtart:1"));
+ Include i1 = new Include(ArtifactId.fromMvnId("g:a:2"));
+ base.setInclude(i1);
+ base.getBundles().add(new Artifact(ArtifactId.fromMvnId("g:myart:1")));
+ base.getBundles().add(new
Artifact(ArtifactId.fromMvnId("group:testmulti:1")));
+ base.getBundles().add(new
Artifact(ArtifactId.fromMvnId("group:testmulti:3")));
+
+ BuilderContext builderContext = new BuilderContext(provider);
+ Feature assembled = FeatureBuilder.assemble(base, builderContext);
+
+ Feature result = new Feature(ArtifactId.parse("g:tgtart:1"));
+ Artifact b0 = new Artifact(ArtifactId.fromMvnId("g:myart:1"));
+ b0.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:tgtart:1");
+ result.getBundles().add(b0);
+ Artifact b1 = new Artifact(ArtifactId.fromMvnId("group:testmulti:1"));
+ b1.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:tgtart:1");
+ result.getBundles().add(b1);
+ Artifact b2 = new Artifact(ArtifactId.fromMvnId("group:testmulti:3"));
+ b2.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:tgtart:1");
+ result.getBundles().add(b2);
+ Artifact b3 = new
Artifact(ArtifactId.fromMvnId("group:someart:1.2.3"));
+ b3.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
"g:a:2");
+ b3.setStartOrder(4);
+ result.getBundles().add(b3);
+
+ equals(result, assembled);
+ }
+
@Test public void testDeduplicationInclude() throws Exception {
final ArtifactId idA = ArtifactId.fromMvnId("g:a:1.0.0");
final ArtifactId idB = ArtifactId.fromMvnId("g:b:1.0.0");
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Avoid magic when merging features
> ---------------------------------
>
> Key: SLING-8104
> URL: https://issues.apache.org/jira/browse/SLING-8104
> Project: Sling
> Issue Type: Improvement
> Components: Feature Model
> Reporter: Carsten Ziegeler
> Assignee: David Bosschaert
> Priority: Blocker
> Fix For: slingfeature-maven-plugin 1.0.0, Feature Model 0.2.2
>
>
> Currently when features are merged a simple algorithm is applied which just
> picks the highest version based on the artifact version. However this version
> might not have no meaning at all and might not really reflect what has
> changed inside the bundle.
> Especially when there is a major version change, this approach seems to be
> clearly wrong
> But in the end, picking a single version is magic.
> While the problem could probably be solved by using something like a resolver
> and figure out if just one version is enough or if both versions are needed,
> without a resolver there is no way to figure this out.
> Therefore we should provide a similar way as we do for variables at the
> moment: if there is a clash the caller needs to provide context on what to
> choose.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
