You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by da...@apache.org on 2019/01/22 13:22:58 UTC
[sling-org-apache-sling-feature] branch master updated: SLING-8214
Provide a way to handle bundles with same symbolic name but different mvn
group/artifact ids
This is an automated email from the ASF dual-hosted git repository.
davidb 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 eb68a49 SLING-8214 Provide a way to handle bundles with same symbolic name but different mvn group/artifact ids
eb68a49 is described below
commit eb68a49f23a5fc4d5f00066d07d855b0d85ea956
Author: David Bosschaert <bo...@adobe.com>
AuthorDate: Tue Jan 22 13:22:37 2019 +0000
SLING-8214 Provide a way to handle bundles with same symbolic name but different mvn group/artifact ids
---
.../java/org/apache/sling/feature/Artifact.java | 31 +++-
.../apache/sling/feature/builder/BuilderUtil.java | 159 ++++++++++++++-------
.../sling/feature/builder/BuilderUtilTest.java | 41 ++++++
3 files changed, 179 insertions(+), 52 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/Artifact.java b/src/main/java/org/apache/sling/feature/Artifact.java
index 9ff92f9..c1d5523 100644
--- a/src/main/java/org/apache/sling/feature/Artifact.java
+++ b/src/main/java/org/apache/sling/feature/Artifact.java
@@ -17,19 +17,24 @@
package org.apache.sling.feature;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Map;
+import java.util.Set;
/**
* An artifact consists of
* <ul>
* <li>An id
* <li>metadata
- * <li>an optional start order property (which is part of the metadata)
+ * <li>optional alias and start order properties (which are part of the metadata)
* </ul>
*
* This class is not thread-safe.
*/
public class Artifact implements Comparable<Artifact> {
+ /** Can be used in artifact metadata to specify an alias. Multiple aliases can be comma-separated. */
+ public static final String KEY_ALIAS = "alias";
+
/** This key might be used by bundles to define the start order. */
public static final String KEY_START_ORDER = "start-order";
@@ -70,6 +75,30 @@ public class Artifact implements Comparable<Artifact> {
}
/**
+ * Obtain the alias or aliases for the artifact.
+ * @param includeMain Whether to include the main ID in the result.
+ * @return The aliases or an empty set if there are none.
+ */
+ public Set<ArtifactId> getAliases(boolean includeMain) {
+ Set<ArtifactId> artifactIds = new HashSet<>();
+ if (includeMain)
+ artifactIds.add(getId());
+
+ String aliases = getMetadata().get(KEY_ALIAS);
+ if (aliases != null) {
+ for (String alias : aliases.split(",")) {
+ alias = alias.trim();
+ if (alias.indexOf(':') == alias.lastIndexOf(':')) {
+ // No version provided, set to version zero
+ alias += ":0.0.0";
+ }
+ artifactIds.add(ArtifactId.fromMvnId(alias));
+ }
+ }
+ return artifactIds;
+ }
+
+ /**
* Get the start order of the artifact.
* This is a convenience method which gets the value for the property named
* {@code #KEY_START_ORDER} from the metadata.
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 e45bbf2..3c911a3 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
@@ -18,6 +18,7 @@ package org.apache.sling.feature.builder;
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;
@@ -34,6 +35,7 @@ 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;
@@ -141,20 +143,34 @@ class BuilderUtil {
final String originKey) {
for(final Map.Entry<Integer, List<Artifact>> entry : source.getBundlesByStartOrder().entrySet()) {
for(final Artifact a : entry.getValue()) {
- Artifact existing = target.getSame(a.getId());
- List<Artifact> selectedArtifacts = null;
- if (existing != null) {
+ 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));
+ }
+
+ 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 = Arrays.asList(existing, a);
+ selectedArtifacts.addAll(Arrays.asList(existing, a));
} else {
- selectedArtifacts = selectArtifactOverride(existing, a, artifactOverrides);
+ selectedArtifacts.addAll(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 (selectedArtifacts.isEmpty()) {
+ selectedArtifacts.add(a);
}
for (Artifact sa : selectedArtifacts) {
@@ -178,45 +194,41 @@ class BuilderUtil {
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);
+ Set<String> commonPrefixes = getCommonPrefixes(a1, a2);
+ if (commonPrefixes.isEmpty()) {
+ throw new IllegalStateException("Internal error selecting override. No common prefix between " + a1 + " and " + a2);
+ }
- String prefix = a1gid + ":" + a1aid + ":";
Set<Artifact> result = new LinkedHashSet<>();
- 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);
- }
+ 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);
- } 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 (a1.getId().getVersion().equals(rule)) {
+ result.add(a1);
+ } else if (a2.getId().getVersion().equals(rule)) {
+ result.add(a2);
+ } else {
+ // It's a completely new artifact
+ result.add(new Artifact(ArtifactId.fromMvnId(o)));
+ }
}
}
}
@@ -225,7 +237,38 @@ class BuilderUtil {
}
throw new IllegalStateException("Artifact override rule required to select between these two artifacts " +
- a1 + " and " + a2);
+ a1 + " and " + a2 + ". 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 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));
+ }
+ return prefixes;
+ }
+
+ private static List<Artifact> findAliasedArtifacts(ArtifactId id, Artifacts bundles) {
+ List<Artifact> result = new ArrayList<>();
+
+ String prefix = id.getGroupId() + ":" + id.getArtifactId() + ":";
+ for (Artifact a : bundles) {
+ for (ArtifactId aid : a.getAliases(false)) {
+ if (aid.toMvnId().startsWith(prefix)) {
+ result.add(a);
+ }
+ }
+ }
+ return result;
}
// configurations - merge / override
@@ -328,20 +371,34 @@ class BuilderUtil {
case ARTIFACTS:
for(final Artifact a : source.getArtifacts()) {
- Artifact existing = target.getArtifacts().getSame(a.getId());
- List<Artifact> selectedArtifacts = null;
- if (existing != null) {
+ 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);
+ }
+
+ allExisting.addAll(findAliasedArtifacts(id, target.getArtifacts()));
+ }
+
+ 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 = Arrays.asList(existing, a);
+ selectedArtifacts.addAll(Arrays.asList(existing, a));
} else {
- selectedArtifacts = selectArtifactOverride(existing, a, artifactOverrides);
+ selectedArtifacts.addAll(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 (selectedArtifacts.isEmpty()) {
+ selectedArtifacts.add(a);
}
for (Artifact sa : selectedArtifacts) {
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 75c6252..f4b6439 100644
--- a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
@@ -46,6 +46,7 @@ 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;
public class BuilderUtilTest {
@@ -138,6 +139,46 @@ public class BuilderUtilTest {
assertContains(result, 3, ArtifactId.parse("g/c/2.5"));
}
+ @Test public void testMergeBundlesWithAliasNoRule() {
+ final Bundles target = new Bundles();
+ Artifact b = createBundle("g/b/2.0", 2);
+ b.getMetadata().put("alias", "x:z:1,a:a");
+ target.add(b);
+
+ final Bundles source = new Bundles();
+ source.add(createBundle("x/z/1.9", 2));
+
+ final Feature orgFeat = new Feature(new ArtifactId("gid", "aid", "123", null, null));
+ try {
+ BuilderUtil.mergeBundles(target, source, orgFeat, Collections.emptyList(), null);
+ fail("Expected exception ");
+ } catch (Exception e) {
+ String msg = e.getMessage();
+ assertTrue(msg.contains("override rule required"));
+ assertTrue(msg.contains("g:b:"));
+ assertTrue(msg.contains("x:z:"));
+ }
+ }
+
+ @Test public void testMergeBundlesWithAlias() {
+ final Bundles target = new Bundles();
+ Artifact b = createBundle("g/b/2.0", 2);
+ b.getMetadata().put("alias", "x:z:1,a:a");
+ target.add(b);
+
+ final Bundles source = new Bundles();
+ 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");
+ BuilderUtil.mergeBundles(target, source, orgFeat, overrides, null);
+
+ final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
+ assertEquals(1, result.size());
+ assertContains(result, 2, ArtifactId.parse("g/b/2.0"));
+ }
+
@Test public void testMergeBundlesDifferentStartlevel() {
final Bundles target = new Bundles();