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 92bc78f SLING-8107 : Correct setting the feature origin
92bc78f is described below
commit 92bc78f3bfd414ceae6cba2e89d513e24b6853eb
Author: Carsten Ziegeler <[email protected]>
AuthorDate: Wed Nov 14 08:25:32 2018 +0100
SLING-8107 : Correct setting the feature origin
---
.../java/org/apache/sling/feature/Artifact.java | 14 +++++
.../apache/sling/feature/builder/BuilderUtil.java | 73 +++++++++++++++-------
.../sling/feature/builder/FeatureBuilder.java | 72 ++++++++++++---------
.../sling/feature/builder/BuilderUtilTest.java | 10 +--
4 files changed, 110 insertions(+), 59 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/Artifact.java
b/src/main/java/org/apache/sling/feature/Artifact.java
index c4767c0..504f7fe 100644
--- a/src/main/java/org/apache/sling/feature/Artifact.java
+++ b/src/main/java/org/apache/sling/feature/Artifact.java
@@ -128,6 +128,20 @@ public class Artifact implements Comparable<Artifact> {
return this.id.equals(((Artifact)obj).id);
}
+ /**
+ * Create a copy of the artifact with a different id
+ *
+ * @param id The new id
+ * @return The copy of the feature with the new id
+ */
+ public Artifact copy(final ArtifactId id) {
+ final Artifact result = new Artifact(id);
+
+ result.getMetadata().putAll(this.getMetadata());
+
+ return result;
+ }
+
@Override
public String toString() {
return "Artifact [id=" + id.toMvnId()
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 ab80ec2..f8864b9 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
@@ -110,18 +110,20 @@ class BuilderUtil {
mergeWithContextOverwrite("Variable", target, source, (null !=
context) ? context.getVariables() : null);
}
- // bundles
+ /**
+ * Merge bundles from source into target
+ *
+ * @param target The target bundles
+ * @param source The source bundles
+ * @param originatingFeature 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) {
for(final Map.Entry<Integer, List<Artifact>> entry :
source.getBundlesByStartOrder().entrySet()) {
for(final Artifact a : entry.getValue()) {
- // Record the original feature of the bundle
- if
(a.getMetadata().get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE) == null) {
-
a.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
originatingFeature.getId().toMvnId());
- }
-
// version handling - use provided algorithm
boolean replace = true;
if (artifactMergeAlg == ArtifactMergeAlgorithm.HIGHEST) {
@@ -132,7 +134,15 @@ class BuilderUtil {
}
if ( replace ) {
target.removeSame(a.getId());
- target.add(a);
+ // create a copy to detach artifact from source
+ final Artifact cp = a.copy(a.getId());
+ // Record the original feature of the bundle
+ if (originatingFeature != null
+ &&
a.getMetadata().get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE) == null) {
+
cp.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
+ originatingFeature.getId().toMvnId());
+ }
+ target.add(cp);
}
}
}
@@ -183,9 +193,17 @@ class BuilderUtil {
}
}
- // default merge for extensions
+ /**
+ * Merge an extension from source into target
+ *
+ * @param target The target extension
+ * @param source The source extension
+ * @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 Extension source, final Feature originatingFeature,
final ArtifactMergeAlgorithm artifactMergeAlg) {
switch ( target.getType() ) {
case TEXT : // simply append
@@ -225,29 +243,38 @@ class BuilderUtil {
target.setJSON(buffer.toString());
break;
- case ARTIFACTS : for(final Artifact a : source.getArtifacts()) {
- // use artifactMergeAlg
- boolean replace = true;
+ 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;
- }
+ final Artifact existing =
target.getArtifacts().getSame(a.getId());
+ if (existing != null
+ &&
existing.getId().getOSGiVersion().compareTo(a.getId().getOSGiVersion()) > 0) {
+ replace = false;
}
- if ( replace ) {
- target.getArtifacts().removeSame(a.getId());
- target.getArtifacts().add(a);
}
+ if (replace) {
+ target.getArtifacts().removeSame(a.getId());
+ // 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) {
+
cp.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
+ originatingFeature.getId().toMvnId());
+ }
+ target.getArtifacts().add(cp);
}
- break;
+ }
+ break;
}
}
// extensions (add/merge)
static void mergeExtensions(final Feature target,
final Feature source,
- final ArtifactMergeAlgorithm artifactMergeAlg,
- final BuilderContext context) {
+ final ArtifactMergeAlgorithm artifactMergeAlg, final
BuilderContext context, final boolean recordOrigin) {
for(final Extension ext : source.getExtensions()) {
boolean found = false;
@@ -269,7 +296,7 @@ class BuilderUtil {
}
if ( !handled ) {
// default merge
- mergeExtensions(current, ext, artifactMergeAlg);
+ mergeExtensions(current, ext, recordOrigin ? source :
null, artifactMergeAlg);
}
}
}
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 0fd9dbb..a7709fe 100644
--- a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
+++ b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
@@ -196,7 +196,7 @@ public abstract class FeatureBuilder {
}
usedFeatures.add(assembled.getId());
- merge(target, assembled, context, context.getMergeAlgorithm());
+ merge(target, assembled, context, context.getMergeAlgorithm(),
true);
}
// append feature list in extension
@@ -315,15 +315,15 @@ public abstract class FeatureBuilder {
throw new IllegalStateException(
"Included feature " + i.getId() + " is marked as final
and can't be used in an include.");
}
- final Feature af = internalAssemble(processedFeatures, f, context);
+ final Feature includedFeature =
internalAssemble(processedFeatures, f, context);
// process include instructions
- include(af, i);
+ processInclude(includedFeature, i);
// and now merge
- merge(result, af, context,
BuilderContext.ArtifactMergeAlgorithm.LATEST);
+ merge(result, includedFeature, context,
BuilderContext.ArtifactMergeAlgorithm.LATEST, true);
- merge(result, feature, context,
BuilderContext.ArtifactMergeAlgorithm.LATEST);
+ merge(result, feature, context,
BuilderContext.ArtifactMergeAlgorithm.LATEST, false);
}
processedFeatures.remove(feature.getId().toMvnId());
@@ -334,9 +334,9 @@ public abstract class FeatureBuilder {
private static void merge(final Feature target,
final Feature source,
final BuilderContext context,
- final BuilderContext.ArtifactMergeAlgorithm mergeAlg) {
+ final BuilderContext.ArtifactMergeAlgorithm mergeAlg, final
boolean recordOrigin) {
BuilderUtil.mergeVariables(target.getVariables(),
source.getVariables(), context);
- BuilderUtil.mergeBundles(target.getBundles(), source.getBundles(),
source, mergeAlg);
+ BuilderUtil.mergeBundles(target.getBundles(), source.getBundles(),
recordOrigin ? source : null, mergeAlg);
BuilderUtil.mergeConfigurations(target.getConfigurations(),
source.getConfigurations());
BuilderUtil.mergeFrameworkProperties(target.getFrameworkProperties(),
source.getFrameworkProperties(), context);
BuilderUtil.mergeRequirements(target.getRequirements(),
source.getRequirements());
@@ -344,29 +344,36 @@ public abstract class FeatureBuilder {
BuilderUtil.mergeExtensions(target,
source,
mergeAlg,
- context);
+ context, recordOrigin);
}
- private static void include(final Feature base, final Include i) {
- // process removals
- // bundles
- for(final ArtifactId a : i.getBundleRemovals()) {
+ /**
+ * Process the include statement Process all the removals contained in the
+ * include
+ *
+ * @param feature The feature
+ * @param include The include
+ */
+ private static void processInclude(final Feature feature, final Include
include) {
+ // process bundles removals
+ for (final ArtifactId a : include.getBundleRemovals()) {
boolean removed = false;
final boolean ignoreVersion =
a.getOSGiVersion().equals(Version.emptyVersion);
if ( ignoreVersion ) {
// remove any version of that bundle
- while (base.getBundles().removeSame(a)) {
+ while (feature.getBundles().removeSame(a)) {
// continue to remove
removed = true;
}
} else {
// remove exact version
- removed = base.getBundles().removeExact(a);
+ removed = feature.getBundles().removeExact(a);
}
if ( !removed ) {
- throw new IllegalStateException("Bundle " + a + " can't be
removed from feature " + base.getId() + " as it is not part of that feature.");
+ throw new IllegalStateException("Bundle " + a + " can't be
removed from feature " + feature.getId()
+ + " as it is not part of that feature.");
}
- final Iterator<Configuration> iter =
base.getConfigurations().iterator();
+ final Iterator<Configuration> iter =
feature.getConfigurations().iterator();
while ( iter.hasNext() ) {
final Configuration cfg = iter.next();
final String bundleId =
(String)cfg.getProperties().get(Configuration.PROP_ARTIFACT_ID);
@@ -382,8 +389,9 @@ public abstract class FeatureBuilder {
}
}
}
- // configurations
- for(final String c : i.getConfigurationRemovals()) {
+
+ // process configuration removals
+ for (final String c : include.getConfigurationRemovals()) {
final int attrPos = c.indexOf('@');
final String val = (attrPos == -1 ? c : c.substring(0, attrPos));
final String attr = (attrPos == -1 ? null : c.substring(attrPos +
1));
@@ -391,39 +399,40 @@ public abstract class FeatureBuilder {
final int sepPos = val.indexOf('~');
Configuration found = null;
if ( sepPos == -1 ) {
- found = base.getConfigurations().getConfiguration(val);
+ found = feature.getConfigurations().getConfiguration(val);
} else {
final String factoryPid = val.substring(0, sepPos);
final String name = val.substring(sepPos + 1);
- found =
base.getConfigurations().getFactoryConfiguration(factoryPid, name);
+ found =
feature.getConfigurations().getFactoryConfiguration(factoryPid, name);
}
if ( found != null ) {
if ( attr == null ) {
- base.getConfigurations().remove(found);
+ feature.getConfigurations().remove(found);
} else {
found.getProperties().remove(attr);
}
}
}
- // framework properties
- for(final String p : i.getFrameworkPropertiesRemovals()) {
- base.getFrameworkProperties().remove(p);
+ // process framework properties removals
+ for (final String p : include.getFrameworkPropertiesRemovals()) {
+ feature.getFrameworkProperties().remove(p);
}
- // extensions
- for(final String name : i.getExtensionRemovals()) {
- for(final Extension ext : base.getExtensions()) {
+ // process extensions removals
+ for (final String name : include.getExtensionRemovals()) {
+ for (final Extension ext : feature.getExtensions()) {
if ( ext.getName().equals(name) ) {
- base.getExtensions().remove(ext);
+ feature.getExtensions().remove(ext);
break;
}
}
}
- for(final Map.Entry<String, List<ArtifactId>> entry :
i.getArtifactExtensionRemovals().entrySet()) {
- for(final Extension ext : base.getExtensions()) {
+ // process artifact extensions removals
+ for (final Map.Entry<String, List<ArtifactId>> entry :
include.getArtifactExtensionRemovals().entrySet()) {
+ for (final Extension ext : feature.getExtensions()) {
if ( ext.getName().equals(entry.getKey()) ) {
for(final ArtifactId toRemove : entry.getValue() ) {
boolean removed = false;
@@ -452,7 +461,8 @@ public abstract class FeatureBuilder {
}
}
if ( !removed ) {
- throw new IllegalStateException("Artifact " +
toRemove + " can't be removed from feature " + base.getId() + " as it is not
part of that feature.");
+ throw new IllegalStateException("Artifact " +
toRemove + " can't be removed from feature "
+ + feature.getId() + " as it is not part of
that feature.");
}
}
break;
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 f49527c..798578a 100644
--- a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
@@ -215,7 +215,7 @@ public class BuilderUtilTest {
source.setJSON("[\"source1\",\"source2\"]");
- BuilderUtil.mergeExtensions(target, source,
ArtifactMergeAlgorithm.HIGHEST);
+ BuilderUtil.mergeExtensions(target, source, null,
ArtifactMergeAlgorithm.HIGHEST);
assertEquals(target.getJSON(),
"[\"target1\",\"target2\",\"source1\",\"source2\"]");
@@ -307,7 +307,7 @@ public class BuilderUtilTest {
Feature ft = new Feature(ArtifactId.fromMvnId("g:t:1"));
assertEquals("Precondition", 0, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST,
ctx);
+ BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST,
ctx, true);
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
@@ -331,7 +331,7 @@ public class BuilderUtilTest {
ft.getExtensions().add(et);
assertEquals("Precondition", 1, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST,
ctx);
+ BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST,
ctx, true);
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
@@ -358,7 +358,7 @@ public class BuilderUtilTest {
Feature ft = new Feature(ArtifactId.fromMvnId("g:t:1"));
assertEquals("Precondition", 0, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST,
ctx);
+ BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST,
ctx, true);
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
@@ -383,7 +383,7 @@ public class BuilderUtilTest {
ft.getExtensions().add(et);
assertEquals("Precondition", 1, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST,
ctx);
+ BuilderUtil.mergeExtensions(ft, fs, ArtifactMergeAlgorithm.LATEST,
ctx, true);
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
