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 2019/06/18 06:07:00 UTC
[sling-org-apache-sling-feature] branch master updated: SLING-8520
: Improve artifact/bundle merging
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 b67c8bb SLING-8520 : Improve artifact/bundle merging
b67c8bb is described below
commit b67c8bb8239a29dcae90d0eb1fa2270c89114e6b
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Tue Jun 18 08:06:52 2019 +0200
SLING-8520 : Improve artifact/bundle merging
---
.../java/org/apache/sling/feature/ArtifactId.java | 13 +-
.../sling/feature/builder/BuilderContext.java | 59 +++-
.../apache/sling/feature/builder/BuilderUtil.java | 303 ++++++++++++---------
.../sling/feature/builder/FeatureBuilder.java | 5 +-
.../apache/sling/feature/builder/package-info.java | 2 +-
.../sling/feature/builder/BuilderUtilTest.java | 77 +++---
.../sling/feature/builder/FeatureBuilderTest.java | 95 +++----
7 files changed, 316 insertions(+), 238 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/ArtifactId.java b/src/main/java/org/apache/sling/feature/ArtifactId.java
index 348a8c4..44f07a8 100644
--- a/src/main/java/org/apache/sling/feature/ArtifactId.java
+++ b/src/main/java/org/apache/sling/feature/ArtifactId.java
@@ -165,11 +165,11 @@ public class ArtifactId implements Comparable<ArtifactId> {
if ( parts.length < 3 || parts.length > 5) {
throw new IllegalArgumentException("Invalid mvn coordinates: " + coordinates);
}
- final String gId = parts[0];
- final String aId = parts[1];
- final String version = parts[parts.length - 1];
- final String type = parts.length > 3 ? parts[2] : null;
- final String classifier = parts.length > 4 ? parts[3] : null;
+ final String gId = parts[0].trim();
+ final String aId = parts[1].trim();
+ final String version = parts[parts.length - 1].trim();
+ final String type = parts.length > 3 ? parts[2].trim() : null;
+ final String classifier = parts.length > 4 ? parts[3].trim() : null;
return new ArtifactId(gId, aId, version, classifier, type);
}
@@ -281,7 +281,10 @@ public class ArtifactId implements Comparable<ArtifactId> {
/**
* Return the OSGi version
+ *
* @return The OSGi version
+ * @throws IllegalArgumentException If the numerical components are negative or
+ * the qualifier string is invalid.
*/
public Version getOSGiVersion() {
String parts[] = version.split("\\.");
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 0fe344d..8fb53a7 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
@@ -21,14 +21,49 @@ import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.stream.Collectors;
+
+import org.apache.sling.feature.ArtifactId;
/**
- * Builder context holds services used by {@link FeatureBuilder}.
+ * Builder context holds services used by {@link FeatureBuilder} and controls
+ * how features are assembled and aggregated.
*
+ * <p>
+ * When two features are merged, being it a prototype with the feature using the
+ * prototype or two features, there might be a clash with bundles or artifacts.
+ * A clash occurs when there is an artifact in the source and the target with
+ * different versions. If the version is the same, the source artifact will
+ * override the target artifact. However, all other cases need instructions on
+ * how to proceed.
+ * <p>
+ * An override rule is an artifact id. As the version for the rule, one of
+ * {@link BuilderContext#VERSON_OVERRIDE_ALL},
+ * {@link BuilderContext#VERSION_OVERRIDE_LATEST} or
+ * {@link BuilderContext#VERSION_OVERRIDE_HIGHEST} as well as any version can be
+ * specified. If the artifact id should match more than a single artifact
+ * {@link BuilderContext#COORDINATE_MATCH_ALL} can be specified as group id,
+ * artifact id, type and/or classifier.
+ * <p>
* This class is not thread-safe.
*/
public class BuilderContext {
+ /** Used in override rule to select all candidates. */
+ public static final String VERSION_OVERRIDE_ALL = "ALL";
+
+ /**
+ * Used in override rule to select the candidate with the highest version (OSGi
+ * version comparison rules).
+ */
+ public static final String VERSION_OVERRIDE_HIGHEST = "HIGHEST";
+
+ /** Used in override rule to select the last candidate applied. */
+ public static final String VERSION_OVERRIDE_LATEST = "LATEST";
+
+ /** Used in override rule to match all coordinates */
+ public static final String COORDINATE_MATCH_ALL = "*";
+
/** The required feature provider */
private final FeatureProvider provider;
@@ -38,7 +73,7 @@ public class BuilderContext {
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 List<ArtifactId> artifactsOverrides = new ArrayList<>();
private final Map<String,String> variables = new HashMap<>();
private final Map<String,String> frameworkProperties = new HashMap<>();
@@ -94,9 +129,24 @@ public class BuilderContext {
*
* @param overrides The overrides
* @return The builder context
+ * @throws IllegalArgumentException If the provided overrides are not following
+ * the artifact id syntax
+ * @deprecated Use {@link #addArtifactsOverride(ArtifactId)} instead.
*/
+ @Deprecated
public BuilderContext addArtifactsOverrides(final List<String> overrides) {
- this.artifactsOverrides.addAll(overrides);
+ this.artifactsOverrides.addAll(overrides.stream().map(f -> ArtifactId.parse(f)).collect(Collectors.toList()));
+ return this;
+ }
+
+ /**
+ * Add an override for artifact clashes
+ *
+ * @param override The override
+ * @return The builder context
+ */
+ public BuilderContext addArtifactsOverride(final ArtifactId override) {
+ this.artifactsOverrides.add(override);
return this;
}
@@ -148,7 +198,7 @@ public class BuilderContext {
return this.artifactProvider;
}
- List<String> getArtifactOverrides() {
+ List<ArtifactId> getArtifactOverrides() {
return this.artifactsOverrides;
}
@@ -195,6 +245,7 @@ public class BuilderContext {
ctx.artifactsOverrides.addAll(this.artifactsOverrides);
ctx.variables.putAll(this.variables);
ctx.frameworkProperties.putAll(this.frameworkProperties);
+ ctx.extensionConfiguration.putAll(this.extensionConfiguration);
ctx.mergeExtensions.addAll(mergeExtensions);
ctx.postProcessExtensions.addAll(postProcessExtensions);
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 173c87b..72a7fb6 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
@@ -24,9 +24,9 @@ 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;
import java.util.Set;
import java.util.stream.Stream;
@@ -56,14 +56,6 @@ import org.osgi.resource.Capability;
* Utility methods for the builders
*/
class BuilderUtil {
- /** Used in override rule to select all candidates. */
- static final String OVERRIDE_SELECT_ALL = "ALL";
-
- /** Used in override rule to select the candidate with the highest version (OSGi version comparison rules). */
- static final String OVERRIDE_SELECT_HIGHEST = "HIGHEST";
-
- /** Used in override rule to select the last candidate applied. */
- static final String OVERRIDE_SELECT_LATEST = "LATEST";
/** Used in override rule to have it apply to all artifacts. */
static final String CATCHALL_OVERRIDE = "*:*:";
@@ -133,145 +125,184 @@ class BuilderUtil {
/**
* Merge bundles from source into target
*
- * @param target The target bundles
- * @param source The source bundles
- * @param sourceFeature Optional, if set origin will be recorded
- * @param artifactMergeAlg Algorithm used to merge the artifacts
- * @param originKey An optional key used to track origins of merged bundles
+ * @param target The target bundles
+ * @param source The source bundles
+ * @param sourceFeature Optional, if set origin will be recorded
+ * @param artifactOverrides Artifact override instructions
+ * @param originKey An optional key used to track origins of merged
+ * bundles
+ * @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,
final Feature sourceFeature,
- final List<String> artifactOverrides,
+ final List<ArtifactId> artifactOverrides,
final String originKey) {
- for(final Map.Entry<Integer, List<Artifact>> entry : source.getBundlesByStartOrder().entrySet()) {
- for(final Artifact a : entry.getValue()) {
- Set<ArtifactId> artifactIds = a.getAliases(true);
-
- List<Artifact> allExisting = new ArrayList<>();
- for (final ArtifactId id : artifactIds) {
- Artifact s = target.getSame(id);
- // Find aliased bundles in target
- if (s != null) {
- allExisting.add(s);
- }
- allExisting.addAll(findAliasedArtifacts(id, target));
+ for (final Artifact artifactFromSource : source) {
+
+ // 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.getSame(id);
+ // Find aliased bundles in target
+ if (targetArtifact != null) {
+ allExistingInTarget.add(targetArtifact);
}
- final List<Artifact> selectedArtifacts = new ArrayList<>();
- for (final Artifact existing : allExisting) {
- 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, a));
- } else {
- selectedArtifacts.addAll(selectArtifactOverride(existing, a, artifactOverrides));
- while(target.removeSame(existing.getId())) {
- // Keep executing removeSame() which ignores the version until last one was removed
+ findAliasedArtifacts(id, target, allExistingInTarget);
+ }
+
+ final List<Artifact> selectedArtifacts = new ArrayList<>();
+ if (allExistingInTarget.isEmpty()) {
+ selectedArtifacts.add(artifactFromSource);
+ }
+
+ int insertPos = target.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.getSame(existing.getId())) != null) {
+ // Keep executing removeSame() which ignores the version until last one was
+ // removed
+ final int p = target.indexOf(same);
+ if (p < insertPos) {
+ insertPos = p;
}
+ target.remove(p);
}
}
+ }
- if (selectedArtifacts.isEmpty()) {
- selectedArtifacts.add(a);
- }
-
- for (Artifact sa : 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
- if (originKey != null) {
- if (sourceFeature != null && source.contains(sa) && sa.getMetadata().get(originKey) == null) {
- cp.getMetadata().put(originKey, sourceFeature.getId().toMvnId());
- }
+ 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 bundle, if needed
+ if (originKey != null) {
+ if (sourceFeature != null && source.contains(sa) && sa.getMetadata().get(originKey) == null) {
+ cp.getMetadata().put(originKey, sourceFeature.getId().toMvnId());
}
+ }
+ if (insertPos == target.size()) {
target.add(cp);
+ insertPos = target.size();
+ } else {
+ target.add(insertPos, cp);
+ insertPos++;
}
}
}
}
- 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);
+ static Set<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);
}
- Set<String> commonPrefixes = getCommonPrefixes(a1, a2);
+ final Set<ArtifactId> commonPrefixes = getCommonArtifactIds(fromTarget, fromSource);
if (commonPrefixes.isEmpty()) {
- throw new IllegalStateException("Internal error selecting override. No common prefix between " + a1 + " and " + a2);
+ throw new IllegalStateException(
+ "Internal error selecting override. No common prefix between " + fromTarget + " and " + fromSource);
}
- Set<Artifact> result = new LinkedHashSet<>();
- for (String prefix : commonPrefixes) {
- 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 (OVERRIDE_SELECT_ALL.equals(rule)) {
- return Arrays.asList(a1, a2);
- } else if (OVERRIDE_SELECT_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 (OVERRIDE_SELECT_LATEST.equals(rule)) {
- return Collections.singletonList(a2);
- }
-
- // 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 (a1.getId().getVersion().equals(rule)) {
- result.add(a1);
- } else if (a2.getId().getVersion().equals(rule)) {
- result.add(a2);
+ final Set<Artifact> result = new HashSet<>();
+ for (ArtifactId prefix : commonPrefixes) {
+ for (final ArtifactId override : artifactOverrides) {
+ if (match(prefix, override)) {
+ String rule = override.getVersion();
+
+ if (BuilderContext.VERSION_OVERRIDE_ALL.equalsIgnoreCase(rule)) {
+ result.add(fromTarget);
+ result.add(fromSource);
+ } 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);
+ } else if (BuilderContext.VERSION_OVERRIDE_LATEST.equalsIgnoreCase(rule)) {
+ result.add(fromSource);
} else {
- // It's a completely new artifact
- result.add(new Artifact(ArtifactId.fromMvnId(o)));
+
+ // 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);
+ } else if (fromSource.getId().getVersion().equals(rule)) {
+ result.add(fromSource);
+ } else {
+ // It's a completely new artifact
+ result.add(new Artifact(override));
+ }
}
}
}
}
- if (result.size() > 0) {
- return new ArrayList<>(result);
+ if (!result.isEmpty()) {
+ return result;
}
throw new IllegalStateException("Artifact override rule required to select between these two artifacts " +
- a1 + " and " + a2 + ". The rule must be specified for " + commonPrefixes);
+ fromTarget + " and " + fromSource + ". The rule must be specified for " + commonPrefixes);
}
- private static Set<String> getCommonPrefixes(Artifact a1, Artifact a2) {
- Set<String> a1Prefixes = getPrefixesIncludingAliases(a1);
- Set<String> a2Prefixes = getPrefixesIncludingAliases(a2);
-
- a1Prefixes.retainAll(a2Prefixes);
- return a1Prefixes;
+ private static boolean match(final ArtifactId id, final ArtifactId override) {
+ int matchCount = 0;
+ // check group id
+ if (BuilderContext.COORDINATE_MATCH_ALL.equals(override.getGroupId())) {
+ matchCount++;
+ } else if (id.getGroupId().equals(override.getGroupId())) {
+ matchCount++;
+ }
+ // check artifact id
+ if (BuilderContext.COORDINATE_MATCH_ALL.equals(override.getArtifactId())) {
+ matchCount++;
+ } else if (id.getArtifactId().equals(override.getArtifactId())) {
+ matchCount++;
+ }
+ // check type
+ if (BuilderContext.COORDINATE_MATCH_ALL.equals(override.getType())) {
+ matchCount++;
+ } else if (id.getType().equals(override.getType())) {
+ matchCount++;
+ }
+ // check classifier
+ if (BuilderContext.COORDINATE_MATCH_ALL.equals(override.getClassifier())) {
+ matchCount++;
+ } else if (Objects.equals(id.getClassifier(), override.getClassifier())) {
+ matchCount++;
+ }
+ return matchCount == 4;
}
- private static Set<String> getPrefixesIncludingAliases(Artifact a) {
- Set<String> prefixes = new HashSet<>();
- for (ArtifactId aid : a.getAliases(true)) {
- String id = aid.toMvnId();
- prefixes.add(id.substring(0, id.lastIndexOf(':') + 1));
+ private static Set<ArtifactId> getCommonArtifactIds(Artifact a1, Artifact a2) {
+ final Set<ArtifactId> result = new HashSet<>();
+ for (final ArtifactId id : a1.getAliases(true)) {
+ for (final ArtifactId c : a2.getAliases(true)) {
+ if (id.isSame(c)) {
+ result.add(id);
+ }
+ }
}
- return prefixes;
- }
- private static List<Artifact> findAliasedArtifacts(ArtifactId id, Artifacts bundles) {
- List<Artifact> result = new ArrayList<>();
+ return result;
+ }
- String prefix = id.getGroupId() + ":" + id.getArtifactId() + ":";
- for (Artifact a : bundles) {
- for (ArtifactId aid : a.getAliases(false)) {
- if (aid.toMvnId().startsWith(prefix)) {
+ private static void findAliasedArtifacts(final ArtifactId id, final Artifacts targetBundles,
+ final Set<Artifact> result) {
+ for (final Artifact a : targetBundles) {
+ for (final ArtifactId aid : a.getAliases(false)) {
+ if (aid.isSame(id)) {
result.add(a);
}
}
}
- return result;
}
// configurations - merge / override
@@ -332,7 +363,7 @@ class BuilderUtil {
static void mergeExtensions(final Extension target,
final Extension source,
final Feature sourceFeature,
- final List<String> artifactOverrides,
+ final List<ArtifactId> artifactOverrides,
final String originKey) {
switch ( target.getType() ) {
case TEXT : // simply append
@@ -373,49 +404,63 @@ class BuilderUtil {
break;
case ARTIFACTS:
- for(final Artifact a : source.getArtifacts()) {
- Set<ArtifactId> artifactIds = a.getAliases(true);
-
- List<Artifact> allExisting = new ArrayList<>();
- for (final ArtifactId id : artifactIds) {
- Artifact s = target.getArtifacts().getSame(id);
- // Find aliased bundles in target
- if (s != null) {
- allExisting.add(s);
+ 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);
}
- allExisting.addAll(findAliasedArtifacts(id, target.getArtifacts()));
+ findAliasedArtifacts(id, target.getArtifacts(), allExistingInTarget);
}
final List<Artifact> selectedArtifacts = new ArrayList<>();
- for (final Artifact existing : allExisting) {
+ 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, a));
+ selectedArtifacts.addAll(Arrays.asList(existing, artifactFromSource));
} else {
- selectedArtifacts.addAll(selectArtifactOverride(existing, a, artifactOverrides));
- while(target.getArtifacts().removeSame(existing.getId())) {
- // Keep executing removeSame() which ignores the version until last one was removed
+ 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);
}
}
}
- if (selectedArtifacts.isEmpty()) {
- selectedArtifacts.add(a);
- }
-
- for (Artifact sa : selectedArtifacts) {
+ 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 bundle if needed
+ // 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());
}
}
- target.getArtifacts().add(cp);
+ if (insertPos == target.getArtifacts().size()) {
+ target.getArtifacts().add(cp);
+ insertPos = target.getArtifacts().size();
+ } else {
+ target.getArtifacts().add(insertPos, cp);
+ insertPos++;
+ }
}
+
}
break;
}
@@ -425,7 +470,7 @@ class BuilderUtil {
static void mergeExtensions(final Feature target,
final Feature source,
final BuilderContext context,
- final List<String> artifactOverrides,
+ final List<ArtifactId> artifactOverrides,
final String originKey) {
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 2cbf959..b114f9f 100644
--- a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
+++ b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
@@ -327,7 +327,8 @@ public abstract class FeatureBuilder {
// and merge the current feature over the prototype feature into the result
merge(result, feature, context, Collections.singletonList(
- BuilderUtil.CATCHALL_OVERRIDE + BuilderUtil.OVERRIDE_SELECT_LATEST), TRACKING_KEY);
+ ArtifactId.parse(BuilderUtil.CATCHALL_OVERRIDE + BuilderContext.VERSION_OVERRIDE_LATEST)),
+ TRACKING_KEY);
for (Artifact a : result.getBundles()) {
a.getMetadata().remove(TRACKING_KEY);
@@ -351,7 +352,7 @@ public abstract class FeatureBuilder {
private static void merge(final Feature target,
final Feature source,
final BuilderContext context,
- final List<String> artifactOverrides,
+ final List<ArtifactId> artifactOverrides,
final String originKey) {
BuilderUtil.mergeVariables(target.getVariables(), source.getVariables(), context);
BuilderUtil.mergeBundles(target.getBundles(), source.getBundles(), source, artifactOverrides, originKey);
diff --git a/src/main/java/org/apache/sling/feature/builder/package-info.java b/src/main/java/org/apache/sling/feature/builder/package-info.java
index 4ecbe60..d8c51dc 100644
--- a/src/main/java/org/apache/sling/feature/builder/package-info.java
+++ b/src/main/java/org/apache/sling/feature/builder/package-info.java
@@ -17,7 +17,7 @@
* under the License.
*/
-@org.osgi.annotation.versioning.Version("1.0.0")
+@org.osgi.annotation.versioning.Version("1.1.0")
package org.apache.sling.feature.builder;
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 75c4531..51919b5 100644
--- a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
@@ -16,15 +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.builder.BuilderUtil.HandlerContextImpl;
-import org.junit.Test;
-import org.mockito.Mockito;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import java.io.StringReader;
import java.io.StringWriter;
@@ -46,9 +40,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.assertTrue;
-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.builder.BuilderUtil.HandlerContextImpl;
+import org.junit.Test;
+import org.mockito.Mockito;
+import org.mockito.internal.util.collections.Sets;
public class BuilderUtilTest {
@@ -106,7 +107,7 @@ public class BuilderUtilTest {
final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
- List<String> overrides = Arrays.asList("g:a:HIGHEST", "g:b:HIGHEST");
+ List<ArtifactId> overrides = Arrays.asList(ArtifactId.parse("g:a:HIGHEST"), ArtifactId.parse("g:b:HIGHEST"));
BuilderUtil.mergeBundles(target, source, orgFeat, overrides, null);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
@@ -130,7 +131,7 @@ public class BuilderUtilTest {
final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
- List<String> overrides = Arrays.asList("g:a:LATEST", "g:b:LATEST");
+ List<ArtifactId> overrides = Arrays.asList(ArtifactId.parse("g:a:LATEST"), ArtifactId.parse("g:b:LATEST"));
BuilderUtil.mergeBundles(target, source, orgFeat, overrides, null);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
@@ -171,8 +172,8 @@ public class BuilderUtilTest {
source.add(createBundle("x/z/1.9", 2));
final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
- List<String> overrides = new ArrayList<>();
- overrides.add("x:z:HIGHEST");
+ List<ArtifactId> overrides = new ArrayList<>();
+ overrides.add(ArtifactId.parse("x:z:HIGHEST"));
BuilderUtil.mergeBundles(target, source, orgFeat, overrides, null);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
@@ -189,7 +190,7 @@ public class BuilderUtilTest {
source.add(createBundle("g/a/1.1", 2));
final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
- List<String> overrides = Arrays.asList("g:a:LATEST", "g:b:LATEST");
+ List<ArtifactId> overrides = Arrays.asList(ArtifactId.parse("g:a:LATEST"), ArtifactId.parse("g:b:LATEST"));
BuilderUtil.mergeBundles(target, source, orgFeat, overrides, null);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
@@ -433,36 +434,36 @@ public class BuilderUtilTest {
@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));
+ 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));
}
@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()));
+ assertEquals(Collections.singleton(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));
+ List<ArtifactId> overrides = Collections.singletonList(ArtifactId.parse("gid:aid:1"));
+ assertEquals(Collections.singleton(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));
+ List<ArtifactId> overrides = Collections.singletonList(ArtifactId.parse("gid:aid:2"));
+ assertEquals(Collections.singleton(a2), BuilderUtil.selectArtifactOverride(a1, a2, overrides));
}
@Test
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");
- assertEquals(Collections.singletonList(new Artifact(ArtifactId.fromMvnId("gid:aid:3"))),
+ List<ArtifactId> overrides = Collections.singletonList(ArtifactId.parse("gid:aid:3"));
+ assertEquals(Collections.singleton(new Artifact(ArtifactId.fromMvnId("gid:aid:3"))),
BuilderUtil.selectArtifactOverride(a1, a2, overrides));
}
@@ -470,8 +471,8 @@ public class BuilderUtilTest {
public void testSelectArtifactOverrideWithoutClash() {
Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
- List<String> overrides = Collections.singletonList("gid:aid:3");
- assertEquals(Collections.singletonList(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"))),
BuilderUtil.selectArtifactOverride(a1, a2, overrides));
}
@@ -479,8 +480,8 @@ public class BuilderUtilTest {
public void testSelectArtifactOverrideMulti() {
Artifact a1 = new Artifact(ArtifactId.fromMvnId("gid:aid:1"));
Artifact a2 = new Artifact(ArtifactId.fromMvnId("gid:aid:2"));
- List<String> overrides = Arrays.asList("gid:aid:2", "gid:aid:3");
- assertEquals(Arrays.asList(a2, new Artifact(ArtifactId.fromMvnId("gid:aid:3"))),
+ 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"))),
BuilderUtil.selectArtifactOverride(a1, a2, overrides));
}
@@ -488,7 +489,7 @@ public class BuilderUtilTest {
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");
+ List<ArtifactId> overrides = Collections.singletonList(ArtifactId.parse("gid:aid:2"));
BuilderUtil.selectArtifactOverride(a1, a2, overrides);
}
@@ -496,7 +497,7 @@ public class BuilderUtilTest {
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");
+ List<ArtifactId> overrides = Collections.singletonList(ArtifactId.parse("gid:aid:2"));
BuilderUtil.selectArtifactOverride(a1, a2, overrides);
}
@@ -510,17 +511,17 @@ public class BuilderUtilTest {
@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));
+ 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));
}
@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));
+ 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));
}
@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 7b7f374..3d529ca 100644
--- a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
@@ -23,7 +23,6 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
@@ -83,12 +82,12 @@ public class FeatureBuilderTest {
}
static {
- final Feature f2 = new Feature(ArtifactId.parse("g/a/3"));
+ final Feature f3 = 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));
+ f3.getBundles().add(BuilderUtilTest.createBundle("group/testmulti/2", 8));
+ f3.getBundles().add(BuilderUtilTest.createBundle("group/someart/1.2.3", 4));
- FEATURES.put(f2.getId().toMvnId(), f2);
+ FEATURES.put(f3.getId().toMvnId(), f3);
}
private final FeatureProvider provider = new FeatureProvider() {
@@ -99,33 +98,6 @@ public class FeatureBuilderTest {
}
};
- private List<Map.Entry<Integer, Artifact>> getBundles(final Feature f) {
- final List<Map.Entry<Integer, Artifact>> result = new ArrayList<>();
- for(final Map.Entry<Integer, List<Artifact>> entry : f.getBundles().getBundlesByStartOrder().entrySet()) {
- for(final Artifact artifact : entry.getValue()) {
- result.add(new Map.Entry<Integer, Artifact>() {
-
- @Override
- public Integer getKey() {
- return entry.getKey();
- }
-
- @Override
- public Artifact getValue() {
- return artifact;
- }
-
- @Override
- public Artifact setValue(Artifact value) {
- return null;
- }
- });
- }
- }
-
- return result;
- }
-
private void equals(final Feature expected, final Feature actuals) {
assertFalse(expected.isAssembled());
assertTrue(actuals.isAssembled());
@@ -140,20 +112,15 @@ public class FeatureBuilderTest {
assertEquals(expected.getVariables(), actuals.getVariables());
// bundles
- final List<Map.Entry<Integer, Artifact>> expectedBundles = getBundles(expected);
- final List<Map.Entry<Integer, Artifact>> actualsBundles = getBundles(actuals);
+ final List<Artifact> expectedBundles = expected.getBundles();
+ final List<Artifact> actualsBundles = actuals.getBundles();
assertEquals(expectedBundles.size(), actualsBundles.size());
- for(final Map.Entry<Integer, Artifact> entry : expectedBundles) {
- boolean found = false;
- for(final Map.Entry<Integer, Artifact> inner : actualsBundles) {
- if ( inner.getValue().getId().equals(entry.getValue().getId()) ) {
- found = true;
- assertEquals("Startlevel of bundle " + entry.getValue(), entry.getKey(), inner.getKey());
- assertEquals("Metadata of bundle " + entry.getValue(), entry.getValue().getMetadata(), inner.getValue().getMetadata());
- break;
- }
- }
- assertTrue("Bundle " + entry.getValue() + " in level " + entry.getKey(), found);
+ for (int i = 0; i < expectedBundles.size(); i++) {
+ final Artifact eb = expectedBundles.get(i);
+ final Artifact ab = actualsBundles.get(i);
+ assertEquals(eb.getId(), ab.getId());
+ assertEquals("Start order of bundle " + eb, eb.getStartOrder(), ab.getStartOrder());
+ assertEquals("Metadata of bundle " + eb, eb.getMetadata(), ab.getMetadata());
}
// configurations
@@ -325,10 +292,20 @@ public class FeatureBuilderTest {
// create the expected result
final Feature result = base.copy();
- result.getVariables().put("varx", "myvalx");
result.setPrototype(null);
- result.getFrameworkProperties().put("bar", "X");
+ 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/1", 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/testnewstartlevelandversion/2", 10));
+ result.getBundles().add(a1.copy(a1.getId()));
+ result.getBundles().add(BuilderUtilTest.createBundle("org.apache.sling/application-bundle/2.0.0", 1));
+ result.getBundles().add(BuilderUtilTest.createBundle("org.apache.sling/another-bundle/2.1.0", 1));
+ result.getBundles().add(BuilderUtilTest.createBundle("org.apache.sling/foo-xyz/1.2.3", 2));
+
+ result.getVariables().put("varx", "myvalx");
+ result.getFrameworkProperties().put("bar", "X");
final Configuration co3 = new Configuration("org.apache.sling.foo");
co3.getProperties().put("prop", "value");
result.getConfigurations().add(co3);
@@ -352,15 +329,15 @@ public class FeatureBuilderTest {
Feature assembled = FeatureBuilder.assemble(base, builderContext);
Feature result = new Feature(ArtifactId.parse("g:tgtart:1"));
- Artifact b0 = new Artifact(ArtifactId.fromMvnId("g:myart:1"));
- result.getBundles().add(b0);
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);
}
@@ -375,8 +352,6 @@ public class FeatureBuilderTest {
Feature assembled = FeatureBuilder.assemble(base, builderContext);
Feature result = new Feature(ArtifactId.parse("g:tgtart:1"));
- Artifact b0 = new Artifact(ArtifactId.fromMvnId("g:myart:1"));
- result.getBundles().add(b0);
Artifact b1 = new Artifact(ArtifactId.fromMvnId("group:testmulti:1"));
b1.setStartOrder(4);
result.getBundles().add(b1);
@@ -386,6 +361,8 @@ public class FeatureBuilderTest {
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);
equals(result, assembled);
}
@@ -401,13 +378,13 @@ public class FeatureBuilderTest {
Feature assembled = FeatureBuilder.assemble(base, builderContext);
Feature result = new Feature(ArtifactId.parse("g:tgtart:1"));
- Artifact b0 = new Artifact(ArtifactId.fromMvnId("g:myart:1"));
- result.getBundles().add(b0);
Artifact b1 = new Artifact(ArtifactId.fromMvnId("group:testmulti:1"));
result.getBundles().add(b1);
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);
equals(result, assembled);
}
@@ -424,15 +401,15 @@ public class FeatureBuilderTest {
Feature assembled = FeatureBuilder.assemble(base, builderContext);
Feature result = new Feature(ArtifactId.parse("g:tgtart:1"));
- Artifact b0 = new Artifact(ArtifactId.fromMvnId("g:myart:1"));
- result.getBundles().add(b0);
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);
}