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 2018/11/21 10:07:54 UTC
[sling-org-apache-sling-feature] branch master updated: SLING-8123
Remove org-feature tracking from feature model
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 28c7434 SLING-8123 Remove org-feature tracking from feature model
28c7434 is described below
commit 28c743415ea1d236b3a22425370b1002554b26c6
Author: David Bosschaert <bo...@adobe.com>
AuthorDate: Wed Nov 21 10:07:17 2018 +0000
SLING-8123 Remove org-feature tracking from feature model
---
.../org/apache/sling/feature/Configuration.java | 6 --
.../org/apache/sling/feature/FeatureConstants.java | 6 --
.../apache/sling/feature/builder/BuilderUtil.java | 65 +++++++++---------
.../sling/feature/builder/FeatureBuilder.java | 80 ++++++++--------------
.../sling/feature/builder/BuilderUtilTest.java | 38 ++++------
5 files changed, 77 insertions(+), 118 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/Configuration.java b/src/main/java/org/apache/sling/feature/Configuration.java
index b203625..417713b 100644
--- a/src/main/java/org/apache/sling/feature/Configuration.java
+++ b/src/main/java/org/apache/sling/feature/Configuration.java
@@ -55,12 +55,6 @@ public class Configuration
*/
public static final String PROP_ARTIFACT_ID = PROP_PREFIX + "service.bundleLocation";
- /**
- * This optional configuration property stores the artifact id (mvn id) of the
- * feature this configuration actually originated from.
- */
- public static final String PROP_ORIGINAL__FEATURE = PROP_PREFIX + "org-feature";
-
/** The pid or name for factory pids. */
private final String pid;
diff --git a/src/main/java/org/apache/sling/feature/FeatureConstants.java b/src/main/java/org/apache/sling/feature/FeatureConstants.java
index 83821c6..3072711 100644
--- a/src/main/java/org/apache/sling/feature/FeatureConstants.java
+++ b/src/main/java/org/apache/sling/feature/FeatureConstants.java
@@ -41,10 +41,4 @@ public abstract class FeatureConstants {
* optional.
*/
public static final String EXTENSION_NAME_ASSEMBLED_FEATURES = "assembled-features";
-
- /**
- * When an artifact is merged, the original feature that contained the artifact
- * is recorded in this attribute.
- */
- public static final String ARTIFACT_ATTR_ORIGINAL_FEATURE = "org-feature";
}
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 be56b0b..77da15c 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
@@ -16,6 +16,16 @@
*/
package org.apache.sling.feature.builder;
+import org.apache.sling.feature.Artifact;
+import org.apache.sling.feature.Bundles;
+import org.apache.sling.feature.Configuration;
+import org.apache.sling.feature.Configurations;
+import org.apache.sling.feature.Extension;
+import org.apache.sling.feature.Feature;
+import org.osgi.framework.Version;
+import org.osgi.resource.Capability;
+import org.osgi.resource.Requirement;
+
import java.io.StringReader;
import java.io.StringWriter;
import java.util.ArrayList;
@@ -37,17 +47,6 @@ import javax.json.JsonValue;
import javax.json.JsonValue.ValueType;
import javax.json.JsonWriter;
-import org.apache.sling.feature.Artifact;
-import org.apache.sling.feature.Bundles;
-import org.apache.sling.feature.Configuration;
-import org.apache.sling.feature.Configurations;
-import org.apache.sling.feature.Extension;
-import org.apache.sling.feature.Feature;
-import org.apache.sling.feature.FeatureConstants;
-import org.osgi.framework.Version;
-import org.osgi.resource.Capability;
-import org.osgi.resource.Requirement;
-
/**
* Utility methods for the builders
*/
@@ -130,18 +129,19 @@ class BuilderUtil {
* @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
*/
static void mergeBundles(final Bundles target,
final Bundles source,
final Feature sourceFeature,
- final List<String> artifactOverrides) {
+ final List<String> artifactOverrides,
+ 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) {
- if (sourceFeature.getId().toMvnId().equals(
- existing.getMetadata().get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE))) {
+ 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);
} else {
@@ -157,11 +157,11 @@ class BuilderUtil {
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 (sourceFeature != null && source.contains(sa)
- && sa.getMetadata().get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE) == null) {
- cp.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
- sourceFeature.getId().toMvnId());
+ // 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());
+ }
}
target.add(cp);
}
@@ -215,7 +215,7 @@ class BuilderUtil {
}
// configurations - merge / override
- static void mergeConfigurations(final Configurations target, final Configurations source, final Feature origin) {
+ static void mergeConfigurations(final Configurations target, final Configurations source) {
for(final Configuration cfg : source) {
boolean found = false;
for(final Configuration current : target) {
@@ -232,9 +232,6 @@ class BuilderUtil {
}
if ( !found ) {
final Configuration newCfg = cfg.copy(cfg.getPid());
- if (origin != null) {
- newCfg.getProperties().put(Configuration.PROP_ORIGINAL__FEATURE, origin.getId().toMvnId());
- }
target.add(newCfg);
}
}
@@ -275,7 +272,8 @@ class BuilderUtil {
static void mergeExtensions(final Extension target,
final Extension source,
final Feature sourceFeature,
- final List<String> artifactOverrides) {
+ final List<String> artifactOverrides,
+ final String originKey) {
switch ( target.getType() ) {
case TEXT : // simply append
target.setText(target.getText() + "\n" + source.getText());
@@ -319,8 +317,7 @@ class BuilderUtil {
Artifact existing = target.getArtifacts().getSame(a.getId());
List<Artifact> selectedArtifacts = null;
if (existing != null) {
- if (sourceFeature.getId().toMvnId().equals(
- existing.getMetadata().get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE))) {
+ if (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);
} else {
@@ -336,11 +333,12 @@ class BuilderUtil {
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 (sourceFeature != null && source.getArtifacts().contains(sa)
- && sa.getMetadata().get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE) == null) {
- cp.getMetadata().put(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE,
- sourceFeature.getId().toMvnId());
+ // Record the original feature of the bundle 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);
}
@@ -353,7 +351,8 @@ class BuilderUtil {
static void mergeExtensions(final Feature target,
final Feature source,
final BuilderContext context,
- final List<String> artifactOverrides) {
+ final List<String> artifactOverrides,
+ final String originKey) {
for(final Extension ext : source.getExtensions()) {
boolean found = false;
@@ -375,7 +374,7 @@ class BuilderUtil {
}
if ( !handled ) {
// default merge
- mergeExtensions(current, ext, source, artifactOverrides);
+ mergeExtensions(current, ext, source, artifactOverrides, originKey);
}
}
}
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 b9f8e94..0cf02ca 100644
--- a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
+++ b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
@@ -16,17 +16,6 @@
*/
package org.apache.sling.feature.builder;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Dictionary;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
import org.apache.sling.feature.Artifact;
import org.apache.sling.feature.ArtifactId;
import org.apache.sling.feature.Configuration;
@@ -37,7 +26,19 @@ import org.apache.sling.feature.FeatureConstants;
import org.apache.sling.feature.Include;
import org.osgi.framework.Version;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
public abstract class FeatureBuilder {
+ /** This key is used to track origins while includes are merged in */
+ private static final String TRACKING_KEY = "tracking-key";
/** Pattern for using variables. */
private static final Pattern VARIABLE_PATTERN = Pattern.compile("\\$\\{[a-zA-Z0-9.-_]+\\}");
@@ -196,7 +197,7 @@ public abstract class FeatureBuilder {
}
usedFeatures.add(assembled.getId());
- merge(target, assembled, context, context.getArtifactOverrides());
+ merge(target, assembled, context, context.getArtifactOverrides(), null);
}
// append feature list in extension
@@ -325,14 +326,22 @@ public abstract class FeatureBuilder {
processInclude(includedFeature, i);
// and now merge the included feature into the result. No overrides should be needed since the result is empty before
- merge(result, includedFeature, context, Collections.emptyList());
+ merge(result, includedFeature, context, Collections.emptyList(), TRACKING_KEY);
// and merge the current feature over the included feature into the result
merge(result, feature, context, Collections.singletonList(
- BuilderUtil.CATCHALL_OVERRIDE + BuilderUtil.OVERRIDE_SELECT_LATEST));
+ BuilderUtil.CATCHALL_OVERRIDE + BuilderUtil.OVERRIDE_SELECT_LATEST), TRACKING_KEY);
- // set the origin of the artifacts and configurations included and part of the resulting feature back to null
- resetOrigins(result, includedFeature);
+ for (Artifact a : result.getBundles()) {
+ a.getMetadata().remove(TRACKING_KEY);
+ }
+ for (Extension e : result.getExtensions()) {
+ if (ExtensionType.ARTIFACTS == e.getType()) {
+ for (Artifact a : e.getArtifacts()) {
+ a.getMetadata().remove(TRACKING_KEY);
+ }
+ }
+ }
}
result.setAssembled(true);
@@ -345,14 +354,15 @@ public abstract class FeatureBuilder {
private static void merge(final Feature target,
final Feature source,
final BuilderContext context,
- final List<String> artifactOverrides) {
+ final List<String> artifactOverrides,
+ final String originKey) {
BuilderUtil.mergeVariables(target.getVariables(), source.getVariables(), context);
- BuilderUtil.mergeBundles(target.getBundles(), source.getBundles(), source, artifactOverrides);
- BuilderUtil.mergeConfigurations(target.getConfigurations(), source.getConfigurations(), source);
+ BuilderUtil.mergeBundles(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());
BuilderUtil.mergeCapabilities(target.getCapabilities(), source.getCapabilities());
- BuilderUtil.mergeExtensions(target, source, context, artifactOverrides);
+ BuilderUtil.mergeExtensions(target, source, context, artifactOverrides, originKey);
}
/**
@@ -468,34 +478,4 @@ public abstract class FeatureBuilder {
}
}
}
-
- // Set the origin of elements coming from an included feature to the target feature
- private static void resetOrigins(Feature feature, Feature includedFeature) {
- String currentID = feature.getId().toMvnId();
- String includedID = includedFeature.getId().toMvnId();
-
- // As the feature is a prototype, set the origins to the target where it's going to
- for (Artifact a : feature.getBundles()) {
- Map<String, String> md = a.getMetadata();
- if (currentID.equals(md.get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE))
- || includedID.equals(md.get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE)))
- md.remove(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE);
- }
- for (Configuration c : feature.getConfigurations()) {
- Dictionary<String, Object> props = c.getProperties();
- if (currentID.equals(props.get(Configuration.PROP_ORIGINAL__FEATURE))
- || includedID.equals(props.get(Configuration.PROP_ORIGINAL__FEATURE)))
- props.remove(Configuration.PROP_ORIGINAL__FEATURE);
- }
- for (Extension e : feature.getExtensions()) {
- if (ExtensionType.ARTIFACTS == e.getType()) {
- for (Artifact a : e.getArtifacts()) {
- Map<String, String> md = a.getMetadata();
- if (currentID.equals(md.get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE))
- || includedID.equals(md.get(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE)))
- md.remove(FeatureConstants.ARTIFACT_ATTR_ORIGINAL_FEATURE);
- }
- }
- }
- }
}
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 23f3338..90c2e39 100644
--- a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
@@ -46,7 +46,6 @@ import javax.json.JsonValue;
import javax.json.stream.JsonGenerator;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
import static org.junit.Assert.fail;
public class BuilderUtilTest {
@@ -106,17 +105,12 @@ 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");
- BuilderUtil.mergeBundles(target, source, orgFeat, overrides);
+ BuilderUtil.mergeBundles(target, source, orgFeat, overrides, null);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
assertEquals(3, result.size());
- int i = assertContains(result, 1, ArtifactId.parse("g/a/1.1"));
- Map.Entry<Integer, Artifact> a1 = result.get(i);
- assertEquals("gid:aid:123", a1.getValue().getMetadata().get("org-feature"));
-
- int i2 = assertContains(result, 2, ArtifactId.parse("g/b/2.0"));
- Map.Entry<Integer, Artifact> a2 = result.get(i2);
- assertNull(a2.getValue().getMetadata().get("org-feature"));
+ assertContains(result, 1, ArtifactId.parse("g/a/1.1"));
+ assertContains(result, 2, ArtifactId.parse("g/b/2.0"));
assertContains(result, 3, ArtifactId.parse("g/c/2.5"));
}
@@ -135,7 +129,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");
- BuilderUtil.mergeBundles(target, source, orgFeat, overrides);
+ BuilderUtil.mergeBundles(target, source, orgFeat, overrides, null);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
assertEquals(3, result.size());
@@ -154,7 +148,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");
- BuilderUtil.mergeBundles(target, source, orgFeat, overrides);
+ BuilderUtil.mergeBundles(target, source, orgFeat, overrides, null);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
assertEquals(1, result.size());
@@ -174,7 +168,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<>());
+ BuilderUtil.mergeBundles(target, source, orgFeat, new ArrayList<>(), null);
final List<Map.Entry<Integer, Artifact>> result = getBundles(target);
assertEquals(6, result.size());
@@ -194,18 +188,16 @@ 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<>());
+ BuilderUtil.mergeBundles(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<>());
+ BuilderUtil.mergeBundles(target2, target, orgFeat2, new ArrayList<>(), null);
List<Entry<Integer, Artifact>> result = getBundles(target2);
assertEquals(2, result.size());
- int i1 = assertContains(result, 1, ArtifactId.parse("g/a/1.0"));
- assertEquals("g:a:1", result.get(i1).getValue().getMetadata().get("org-feature"));
- int i2 = assertContains(result, 1, ArtifactId.parse("g/b/1.0"));
- assertEquals("gid:aid:slingfeature:c1:123", result.get(i2).getValue().getMetadata().get("org-feature"));
+ assertContains(result, 1, ArtifactId.parse("g/a/1.0"));
+ assertContains(result, 1, ArtifactId.parse("g/b/1.0"));
}
@Test public void testMergeExtensions() {
@@ -217,7 +209,7 @@ public class BuilderUtilTest {
source.setJSON("[\"source1\",\"source2\"]");
- BuilderUtil.mergeExtensions(target, source, null, new ArrayList<>());
+ BuilderUtil.mergeExtensions(target, source, null, new ArrayList<>(), null);
assertEquals(target.getJSON(), "[\"target1\",\"target2\",\"source1\",\"source2\"]");
@@ -309,7 +301,7 @@ public class BuilderUtilTest {
Feature ft = new Feature(ArtifactId.fromMvnId("g:t:1"));
assertEquals("Precondition", 0, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ctx, new ArrayList<>());
+ BuilderUtil.mergeExtensions(ft, fs, ctx, new ArrayList<>(), null);
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
@@ -333,7 +325,7 @@ public class BuilderUtilTest {
ft.getExtensions().add(et);
assertEquals("Precondition", 1, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ctx, new ArrayList<>());
+ BuilderUtil.mergeExtensions(ft, fs, ctx, new ArrayList<>(), null);
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
@@ -360,7 +352,7 @@ public class BuilderUtilTest {
Feature ft = new Feature(ArtifactId.fromMvnId("g:t:1"));
assertEquals("Precondition", 0, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ctx, new ArrayList<>());
+ BuilderUtil.mergeExtensions(ft, fs, ctx, new ArrayList<>(), null);
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);
@@ -385,7 +377,7 @@ public class BuilderUtilTest {
ft.getExtensions().add(et);
assertEquals("Precondition", 1, ft.getExtensions().size());
- BuilderUtil.mergeExtensions(ft, fs, ctx, new ArrayList<>());
+ BuilderUtil.mergeExtensions(ft, fs, ctx, new ArrayList<>(), null);
assertEquals(1, ft.getExtensions().size());
Extension actual = ft.getExtensions().get(0);