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 2019/08/20 15:11:55 UTC

[sling-org-apache-sling-feature] 01/01: SLING-8644: Improve Startorder handling during merge

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

pauls pushed a commit to branch issues/SLING-8644
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature.git

commit 760480acf333e4625c5f1d5ed750827478708851
Author: Karl Pauls <ka...@gmail.com>
AuthorDate: Tue Aug 20 17:11:39 2019 +0200

    SLING-8644: Improve Startorder handling during merge
---
 .../apache/sling/feature/builder/BuilderUtil.java  | 128 ++++++++-------------
 .../sling/feature/builder/FeatureBuilder.java      |   4 +-
 .../sling/feature/builder/BuilderUtilTest.java     |  40 +++----
 .../sling/feature/builder/FeatureBuilderTest.java  |  89 +++++++++++++-
 4 files changed, 156 insertions(+), 105 deletions(-)

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 f58d2b9..4e747d7 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
@@ -19,11 +19,11 @@ package org.apache.sling.feature.builder;
 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.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -43,7 +43,6 @@ import javax.json.JsonWriter;
 import org.apache.sling.feature.Artifact;
 import org.apache.sling.feature.ArtifactId;
 import org.apache.sling.feature.Artifacts;
-import org.apache.sling.feature.Bundles;
 import org.apache.sling.feature.Configuration;
 import org.apache.sling.feature.Configurations;
 import org.apache.sling.feature.Extension;
@@ -131,8 +130,8 @@ class BuilderUtil {
      * @throws IllegalStateException If bundles can't be merged, for example if no
      *                               override is specified for a clash.
      */
-    static void mergeBundles(final Bundles target,
-        final Bundles source,
+    static void mergeArtifacts(final Artifacts target,
+        final Artifacts source,
         final Feature sourceFeature,
             final List<ArtifactId> artifactOverrides,
         final String originKey) {
@@ -140,12 +139,13 @@ class BuilderUtil {
         for (final Artifact artifactFromSource : source) {
 
             // set of artifacts in target, matching the artifact from source
-            final Set<Artifact> allExistingInTarget = new HashSet<>();
+            final Set<Artifact> allExistingInTarget = new LinkedHashSet<>();
             for (final ArtifactId id : artifactFromSource.getAliases(true)) {
-                final Artifact targetArtifact = target.getSame(id);
-                // Find aliased bundles in target
-                if (targetArtifact != null) {
-                    allExistingInTarget.add(targetArtifact);
+                for (Artifact targetArtifact : target) {
+                    // Find aliased bundles in target
+                    if (id.isSame(targetArtifact.getId())) {
+                        allExistingInTarget.add(targetArtifact);
+                    }
                 }
 
                 findAliasedArtifacts(id, target, allExistingInTarget);
@@ -157,12 +157,18 @@ class BuilderUtil {
             }
 
             int insertPos = target.size();
+            int count = 0;
             for (final Artifact existing : allExistingInTarget) {
                 if (sourceFeature.getId().toMvnId().equals(existing.getMetadata().get(originKey))) {
                     // If the source artifact came from the same feature, keep them side-by-side
-                    selectedArtifacts.addAll(Arrays.asList(existing, artifactFromSource));
+                    selectedArtifacts.add(count++, existing);
+                    selectedArtifacts.add(artifactFromSource);
                 } else {
-                    selectedArtifacts.addAll(selectArtifactOverride(existing, artifactFromSource, artifactOverrides));
+                    List<Artifact> artifacts = selectArtifactOverride(existing, artifactFromSource, artifactOverrides);
+                    if (artifacts.size() > 1) {
+                        selectedArtifacts.add(count++, artifacts.remove(0));
+                    }
+                    selectedArtifacts.addAll(artifacts);
                     Artifact same = null;
                     while ((same = target.getSame(existing.getId())) != null) {
                         // Keep executing removeSame() which ignores the version until last one was
@@ -176,7 +182,7 @@ class BuilderUtil {
                 }
             }
 
-            for (final Artifact sa : selectedArtifacts) {
+            for (final Artifact sa : new LinkedHashSet<>(selectedArtifacts)) {
                 // create a copy to detach artifact from source
                 final Artifact cp = sa.copy(sa.getId());
                 // Record the original feature of the bundle, if needed
@@ -196,11 +202,11 @@ class BuilderUtil {
         }
     }
 
-    static Set<Artifact> selectArtifactOverride(Artifact fromTarget, Artifact fromSource,
+    static List<Artifact> selectArtifactOverride(Artifact fromTarget, Artifact fromSource,
             List<ArtifactId> artifactOverrides) {
         if (fromTarget.getId().equals(fromSource.getId())) {
             // They're the same so return the source (latest)
-            return Collections.singleton(fromSource);
+            return Collections.singletonList(selectStartOder(fromTarget, fromSource, fromSource));
         }
 
         final Set<ArtifactId> commonPrefixes = getCommonArtifactIds(fromTarget, fromSource);
@@ -209,7 +215,7 @@ class BuilderUtil {
                     "Internal error selecting override. No common prefix between " + fromTarget + " and " + fromSource);
         }
 
-        final Set<Artifact> result = new HashSet<>();
+        final List<Artifact> result = new ArrayList<>();
         for (ArtifactId prefix : commonPrefixes) {
             for (final ArtifactId override : artifactOverrides) {
                 if (match(prefix, override)) {
@@ -221,21 +227,21 @@ class BuilderUtil {
                     } else if (BuilderContext.VERSION_OVERRIDE_HIGHEST.equalsIgnoreCase(rule)) {
                         Version a1v = fromTarget.getId().getOSGiVersion();
                         Version a2v = fromSource.getId().getOSGiVersion();
-                        result.add(a1v.compareTo(a2v) > 0 ? fromTarget : fromSource);
+                        result.add(selectStartOder(fromTarget, fromSource, a1v.compareTo(a2v) > 0 ? fromTarget : fromSource));
                     } else if (BuilderContext.VERSION_OVERRIDE_LATEST.equalsIgnoreCase(rule)) {
-                        result.add(fromSource);
+                        result.add(selectStartOder(fromTarget, fromSource, fromSource));
                     } else {
 
                         // The rule must represent a version
                         // See if its one of the existing artifact. If so use those, as they may have
                         // additional metadata
                         if (fromTarget.getId().getVersion().equals(rule)) {
-                            result.add(fromTarget);
+                            result.add(selectStartOder(fromTarget, fromSource, fromTarget));
                         } else if (fromSource.getId().getVersion().equals(rule)) {
-                            result.add(fromSource);
+                            result.add(selectStartOder(fromTarget, fromSource, fromSource));
                         } else {
                             // It's a completely new artifact
-                            result.add(new Artifact(override));
+                            result.add(selectStartOder(fromTarget, fromSource, new Artifact(override)));
                         }
                     }
                 }
@@ -249,6 +255,29 @@ class BuilderUtil {
                 fromTarget + " and " + fromSource + ". The rule must be specified for " + commonPrefixes);
     }
 
+    private static Artifact selectStartOder(Artifact a, Artifact b, Artifact target) {
+        int startOrderA = a.getStartOrder();
+        int startOrderB = b.getStartOrder();
+        int startOrderNew;
+        if (startOrderA == 0) {
+            startOrderNew = startOrderB;
+        } else if (startOrderB == 0) {
+            startOrderNew = startOrderA;
+        } else if (startOrderA < startOrderB) {
+            startOrderNew = startOrderA;
+        } else {
+            startOrderNew = startOrderB;
+        }
+        if (startOrderNew != target.getStartOrder()) {
+            Artifact result = target.copy(target.getId());
+            result.setStartOrder(startOrderNew);
+            return result;
+        }
+        else {
+            return target;
+        }
+    }
+
     private static boolean match(final ArtifactId id, final ArtifactId override) {
         int matchCount = 0;
         // check group id
@@ -401,64 +430,7 @@ class BuilderUtil {
                 break;
 
         case ARTIFACTS:
-            for(final Artifact artifactFromSource : source.getArtifacts()) {
-                // set of artifacts in target, matching the artifact from source
-                final Set<Artifact> allExistingInTarget = new HashSet<>();
-                for (final ArtifactId id : artifactFromSource.getAliases(true)) {
-                    final Artifact targetArtifact = target.getArtifacts().getSame(id);
-                    // Find aliased artifact in target
-                    if (targetArtifact != null) {
-                        allExistingInTarget.add(targetArtifact);
-                    }
-
-                    findAliasedArtifacts(id, target.getArtifacts(), allExistingInTarget);
-                }
-
-                final List<Artifact> selectedArtifacts = new ArrayList<>();
-                if (allExistingInTarget.isEmpty()) {
-                    selectedArtifacts.add(artifactFromSource);
-                }
-
-                int insertPos = target.getArtifacts().size();
-                for (final Artifact existing : allExistingInTarget) {
-                    if (sourceFeature.getId().toMvnId().equals(existing.getMetadata().get(originKey))) {
-                        // If the source artifact came from the same feature, keep them side-by-side
-                        selectedArtifacts.addAll(Arrays.asList(existing, artifactFromSource));
-                    } else {
-                        selectedArtifacts.addAll(selectArtifactOverride(existing, artifactFromSource, artifactOverrides));
-                        Artifact same = null;
-                        while ((same = target.getArtifacts().getSame(existing.getId())) != null) {
-                            // Keep executing removeSame() which ignores the version until last one was
-                            // removed
-                            final int p = target.getArtifacts().indexOf(same);
-                            if (p < insertPos) {
-                                insertPos = p;
-                            }
-                            target.getArtifacts().remove(p);
-                        }
-                    }
-                }
-
-                for (final Artifact sa : selectedArtifacts) {
-                    // create a copy to detach artifact from source
-                    final Artifact cp = sa.copy(sa.getId());
-                    // Record the original feature of the artifact, if needed
-                    if (originKey != null) {
-                        if (sourceFeature != null && source.getArtifacts().contains(sa)
-                                && sa.getMetadata().get(originKey) == null) {
-                            cp.getMetadata().put(originKey, sourceFeature.getId().toMvnId());
-                        }
-                    }
-                    if (insertPos == target.getArtifacts().size()) {
-                        target.getArtifacts().add(cp);
-                        insertPos = target.getArtifacts().size();
-                    } else {
-                        target.getArtifacts().add(insertPos, cp);
-                        insertPos++;
-                    }
-                }
-
-            }
+            mergeArtifacts(target.getArtifacts(), source.getArtifacts(), sourceFeature, artifactOverrides, originKey);
             break;
         }
     }
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 2fef8e7..7dc6a42 100644
--- a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
+++ b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
@@ -328,7 +328,7 @@ public abstract class FeatureBuilder {
 
             // and merge the current feature over the prototype feature into the result
             merge(result, feature, context, Collections.singletonList(
-                    ArtifactId.parse(BuilderUtil.CATCHALL_OVERRIDE + BuilderContext.VERSION_OVERRIDE_LATEST)),
+                    ArtifactId.parse(BuilderUtil.CATCHALL_OVERRIDE + BuilderContext.VERSION_OVERRIDE_ALL)),
                     TRACKING_KEY);
 
             for (Artifact a : result.getBundles()) {
@@ -356,7 +356,7 @@ public abstract class FeatureBuilder {
             final List<ArtifactId> artifactOverrides,
             final String originKey) {
         BuilderUtil.mergeVariables(target.getVariables(), source.getVariables(), context);
-        BuilderUtil.mergeBundles(target.getBundles(), source.getBundles(), source, artifactOverrides, originKey);
+        BuilderUtil.mergeArtifacts(target.getBundles(), source.getBundles(), source, artifactOverrides, originKey);
         BuilderUtil.mergeConfigurations(target.getConfigurations(), source.getConfigurations());
         BuilderUtil.mergeFrameworkProperties(target.getFrameworkProperties(), source.getFrameworkProperties(), context);
         BuilderUtil.mergeRequirements(target.getRequirements(), source.getRequirements());
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 419cd3c..d8721b5 100644
--- a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
@@ -109,7 +109,7 @@ public class BuilderUtilTest {
         final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
 
         List<ArtifactId> overrides = Arrays.asList(ArtifactId.parse("g:a:HIGHEST"), ArtifactId.parse("g:b:HIGHEST"));
-        BuilderUtil.mergeBundles(target, source, orgFeat, overrides, null);
+        BuilderUtil.mergeArtifacts(target, source, orgFeat, overrides, null);
 
         final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
         assertEquals(3, result.size());
@@ -133,7 +133,7 @@ public class BuilderUtilTest {
         final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
 
         List<ArtifactId> overrides = Arrays.asList(ArtifactId.parse("g:a:LATEST"), ArtifactId.parse("g:b:LATEST"));
-        BuilderUtil.mergeBundles(target, source, orgFeat, overrides, null);
+        BuilderUtil.mergeArtifacts(target, source, orgFeat, overrides, null);
 
         final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
         assertEquals(3, result.size());
@@ -153,7 +153,7 @@ public class BuilderUtilTest {
 
         final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
         try {
-            BuilderUtil.mergeBundles(target, source, orgFeat, Collections.emptyList(), null);
+            BuilderUtil.mergeArtifacts(target, source, orgFeat, Collections.emptyList(), null);
             fail("Expected exception ");
         } catch (Exception e) {
             String msg = e.getMessage();
@@ -175,7 +175,7 @@ public class BuilderUtilTest {
         final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
         List<ArtifactId> overrides = new ArrayList<>();
         overrides.add(ArtifactId.parse("x:z:HIGHEST"));
-        BuilderUtil.mergeBundles(target, source, orgFeat, overrides, null);
+        BuilderUtil.mergeArtifacts(target, source, orgFeat, overrides, null);
 
         final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
         assertEquals(1, result.size());
@@ -192,11 +192,11 @@ public class BuilderUtilTest {
 
         final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
         List<ArtifactId> overrides = Arrays.asList(ArtifactId.parse("g:a:LATEST"), ArtifactId.parse("g:b:LATEST"));
-        BuilderUtil.mergeBundles(target, source, orgFeat, overrides, null);
+        BuilderUtil.mergeArtifacts(target, source, orgFeat, overrides, null);
 
         final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
         assertEquals(1, result.size());
-        assertContains(result, 2, ArtifactId.parse("g/a/1.1"));
+        assertContains(result, 1, ArtifactId.parse("g/a/1.1"));
     }
 
     @Test public void testMergeBundles() {
@@ -212,7 +212,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, new ArrayList<>(), null);
+        BuilderUtil.mergeArtifacts(target, source, orgFeat, new ArrayList<>(), null);
 
         final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
         assertEquals(6, result.size());
@@ -232,11 +232,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, new ArrayList<>(), null);
+        BuilderUtil.mergeArtifacts(target, source, orgFeat, new ArrayList<>(), null);
 
         final Bundles target2 = new Bundles();
         final Feature orgFeat2 = new Feature(new ArtifactId("g", "a", "1", null, null));
-        BuilderUtil.mergeBundles(target2, target, orgFeat2, new ArrayList<>(), null);
+        BuilderUtil.mergeArtifacts(target2, target, orgFeat2, new ArrayList<>(), null);
 
         List<Entry<Integer, Artifact>> result = getBundles(target2);
         assertEquals(2, result.size());
@@ -436,27 +436,27 @@ public class BuilderUtilTest {
         Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
         Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2"));
         List<ArtifactId> overrides = Arrays.asList(ArtifactId.parse("gid:aid2:1"), ArtifactId.parse("gid:aid:ALL"));
-        assertEquals(Sets.newSet(a1, a2), BuilderUtil.selectArtifactOverride(a1, a2, overrides));
+        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.singleton(a1), BuilderUtil.selectArtifactOverride(a1, a2, Collections.emptyList()));
+        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<ArtifactId> overrides = Collections.singletonList(ArtifactId.parse("gid:aid:1"));
-        assertEquals(Collections.singleton(a1), BuilderUtil.selectArtifactOverride(a1, a2, overrides));
+        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<ArtifactId> overrides = Collections.singletonList(ArtifactId.parse("gid:aid:2"));
-        assertEquals(Collections.singleton(a2), BuilderUtil.selectArtifactOverride(a1, a2, overrides));
+        assertEquals(Collections.singletonList(a2), BuilderUtil.selectArtifactOverride(a1, a2, overrides));
     }
 
     @Test
@@ -464,7 +464,7 @@ public class BuilderUtilTest {
         Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
         Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2"));
         List<ArtifactId> overrides = Collections.singletonList(ArtifactId.parse("gid:aid:3"));
-        assertEquals(Collections.singleton(new Artifact(ArtifactId.fromMvnId("gid:aid:3"))),
+        assertEquals(Collections.singletonList(new Artifact(ArtifactId.fromMvnId("gid:aid:3"))),
                 BuilderUtil.selectArtifactOverride(a1, a2, overrides));
     }
 
@@ -473,7 +473,7 @@ public class BuilderUtilTest {
         Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
         Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
         List<ArtifactId> overrides = Collections.singletonList(ArtifactId.parse("gid:aid:3"));
-        assertEquals(Collections.singleton(new Artifact(ArtifactId.fromMvnId("gid:aid:1"))),
+        assertEquals(Collections.singletonList(new Artifact(ArtifactId.fromMvnId("gid:aid:1"))),
                 BuilderUtil.selectArtifactOverride(a1, a2, overrides));
     }
 
@@ -482,7 +482,7 @@ public class BuilderUtilTest {
         Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
         Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2"));
         List<ArtifactId> overrides = Arrays.asList(ArtifactId.parse("gid:aid:2"), ArtifactId.parse("gid:aid:3"));
-        assertEquals(Sets.newSet(a2, new Artifact(ArtifactId.fromMvnId("gid:aid:3"))),
+        assertEquals(Arrays.asList(a2, new Artifact(ArtifactId.fromMvnId("gid:aid:3"))),
                 BuilderUtil.selectArtifactOverride(a1, a2, overrides));
     }
 
@@ -513,16 +513,16 @@ public class BuilderUtilTest {
         Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1.1"));
         Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2.0.1"));
         List<ArtifactId> overrides = Collections.singletonList(ArtifactId.parse("gid:aid:HIGHEST"));
-        assertEquals(Collections.singleton(a2), BuilderUtil.selectArtifactOverride(a1, a2, overrides));
-        assertEquals(Collections.singleton(a2), BuilderUtil.selectArtifactOverride(a2, a1, overrides));
+        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<ArtifactId> overrides = Collections.singletonList(ArtifactId.parse("gid:aid:LATEST"));
-        assertEquals(Collections.singleton(a2), BuilderUtil.selectArtifactOverride(a1, a2, overrides));
-        assertEquals(Collections.singleton(a1), BuilderUtil.selectArtifactOverride(a2, a1, overrides));
+        assertEquals(Collections.singletonList(a2), BuilderUtil.selectArtifactOverride(a1, a2, overrides));
+        assertEquals(Collections.singletonList(a1), BuilderUtil.selectArtifactOverride(a2, a1, overrides));
     }
 
     @Test public void testHandlerConfiguration() {
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 883a7d7..4ba78ba 100644
--- a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
@@ -197,6 +197,63 @@ public class FeatureBuilderTest {
         assertNull(actuals.getPrototype());
     }
 
+    @Test public void testMergeMultipleVersions() {
+        Feature a = new Feature(ArtifactId.fromMvnId("g:a:1"));
+        Feature b = new Feature(ArtifactId.fromMvnId("g:b:1"));
+
+
+        a.getBundles().add(BuilderUtilTest.createBundle("o/a/1.0.0", 10));
+        a.getBundles().add(BuilderUtilTest.createBundle("o/a/2.0.0", 9));
+        a.getBundles().add(BuilderUtilTest.createBundle("o/a/3.0.0", 11));
+
+        b.getBundles().add(BuilderUtilTest.createBundle("o/a/4.0.0", 8));
+        b.getBundles().add(BuilderUtilTest.createBundle("o/a/5.0.0", 12));
+        b.getBundles().add(BuilderUtilTest.createBundle("o/a/6.0.0", 10));
+
+        Feature ab = new Feature(ArtifactId.fromMvnId("g:ab:1"));
+        ab.getBundles().add(BuilderUtilTest.createBundle("o/a/6.0.0", 8));
+
+        Feature assembled = FeatureBuilder.assemble(ArtifactId.fromMvnId("g:ab:1"), new BuilderContext(provider)
+                .addArtifactsOverride(ArtifactId.fromMvnId("o:a:HIGHEST")), a, b);
+        assembled.getExtensions().clear();
+
+        equals(ab, assembled );
+
+        ab = new Feature(ArtifactId.fromMvnId("g:ab:1"));
+        ab.getBundles().add(BuilderUtilTest.createBundle("o/a/6.0.0", 8));
+
+        assembled = FeatureBuilder.assemble(ArtifactId.fromMvnId("g:ab:1"), new BuilderContext(provider)
+            .addArtifactsOverride(ArtifactId.fromMvnId("o:a:LATEST")), a, b);
+        assembled.getExtensions().clear();
+
+        equals(ab, assembled );
+
+        ab = new Feature(ArtifactId.fromMvnId("g:ab:1"));
+        ab.getBundles().addAll(a.getBundles());
+        ab.getBundles().addAll(b.getBundles());
+
+        assembled = FeatureBuilder.assemble(ArtifactId.fromMvnId("g:ab:1"), new BuilderContext(provider)
+            .addArtifactsOverride(ArtifactId.fromMvnId("o:a:ALL")), a, b);
+        assembled.getExtensions().clear();
+
+
+        equals(ab, assembled );
+
+        a.getBundles().get(1).setStartOrder(1);
+
+        ab = new Feature(ArtifactId.fromMvnId("g:ab:1"));
+        ab.getBundles().add(BuilderUtilTest.createBundle("o/a/6.0.0", 1));
+        ab.getBundles().get(0).setStartOrder(1);
+
+        assembled = FeatureBuilder.assemble(ArtifactId.fromMvnId("g:ab:1"), new BuilderContext(provider)
+            .addArtifactsOverride(ArtifactId.fromMvnId("o:a:LATEST")), a, b);
+        assembled.getExtensions().clear();
+
+
+        equals(ab, assembled);
+    }
+
+
     @Test public void testNoIncludesNoUpgrade() throws Exception {
         final Feature base = new Feature(ArtifactId.parse("org.apache.sling/test-feature/1.1"));
 
@@ -295,10 +352,14 @@ public class FeatureBuilderTest {
         final Feature result = base.copy();
         result.setPrototype(null);
         result.getBundles().clear();
+
         result.getBundles().add(BuilderUtilTest.createBundle("org.apache.sling/foo-bar/4.5.6", 3));
+        result.getBundles().add(BuilderUtilTest.createBundle("group/testnewversion_low/2", 5));
         result.getBundles().add(BuilderUtilTest.createBundle("group/testnewversion_low/1", 5));
+        result.getBundles().add(BuilderUtilTest.createBundle("group/testnewversion_high/2", 5));
         result.getBundles().add(BuilderUtilTest.createBundle("group/testnewversion_high/5", 5));
-        result.getBundles().add(BuilderUtilTest.createBundle("group/testnewstartlevel/1", 10));
+        result.getBundles().add(BuilderUtilTest.createBundle("group/testnewstartlevel/1", 5));
+        result.getBundles().add(BuilderUtilTest.createBundle("group/testnewstartlevelandversion/1", 5));
         result.getBundles().add(BuilderUtilTest.createBundle("group/testnewstartlevelandversion/2", 10));
         result.getBundles().add(a1.copy(a1.getId()));
         result.getBundles().add(BuilderUtilTest.createBundle("org.apache.sling/application-bundle/2.0.0", 1));
@@ -330,15 +391,23 @@ public class FeatureBuilderTest {
         Feature assembled = FeatureBuilder.assemble(base, builderContext);
 
         Feature result = new Feature(ArtifactId.parse("g:tgtart:1"));
+
+        result.getBundles().add(BuilderUtilTest.createBundle("group/testmulti/2", 8));
+
+
         Artifact b1 = new Artifact(ArtifactId.fromMvnId("group:testmulti:1"));
         result.getBundles().add(b1);
+
+
+        Artifact b2 = new Artifact(ArtifactId.fromMvnId("group:testmulti:3"));
+        result.getBundles().add(b2);
+
         Artifact b3 = new Artifact(ArtifactId.fromMvnId("group:someart:1.2.3"));
         b3.setStartOrder(4);
         result.getBundles().add(b3);
         Artifact b0 = new Artifact(ArtifactId.fromMvnId("g:myart:1"));
         result.getBundles().add(b0);
-        Artifact b2 = new Artifact(ArtifactId.fromMvnId("group:testmulti:3"));
-        result.getBundles().add(b2);
+
 
         equals(result, assembled);
     }
@@ -379,7 +448,11 @@ public class FeatureBuilderTest {
         Feature assembled = FeatureBuilder.assemble(base, builderContext);
 
         Feature result = new Feature(ArtifactId.parse("g:tgtart:1"));
+
+        result.getBundles().add(BuilderUtilTest.createBundle("group/testmulti/2", 8));
+
         Artifact b1 = new Artifact(ArtifactId.fromMvnId("group:testmulti:1"));
+        b1.setStartOrder(4);
         result.getBundles().add(b1);
         Artifact b3 = new Artifact(ArtifactId.fromMvnId("group:someart:1.2.3"));
         b3.setStartOrder(4);
@@ -402,15 +475,21 @@ public class FeatureBuilderTest {
         Feature assembled = FeatureBuilder.assemble(base, builderContext);
 
         Feature result = new Feature(ArtifactId.parse("g:tgtart:1"));
+
+        result.getBundles().add(BuilderUtilTest.createBundle("group/testmulti/2", 8));
+
         Artifact b1 = new Artifact(ArtifactId.fromMvnId("group:testmulti:1"));
+        b1.setStartOrder(4);
         result.getBundles().add(b1);
+
+        Artifact b2 = new Artifact(ArtifactId.fromMvnId("group:testmulti:3"));
+        result.getBundles().add(b2);
+
         Artifact b3 = new Artifact(ArtifactId.fromMvnId("group:someart:1.2.3"));
         b3.setStartOrder(4);
         result.getBundles().add(b3);
         Artifact b0 = new Artifact(ArtifactId.fromMvnId("g:myart:1"));
         result.getBundles().add(b0);
-        Artifact b2 = new Artifact(ArtifactId.fromMvnId("group:testmulti:3"));
-        result.getBundles().add(b2);
 
         equals(result, assembled);
     }