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 2021/06/28 14:27:03 UTC
[sling-org-apache-sling-feature] branch master updated: SLING-10566
: Keep track of feature originas per configuration property
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 8ce2d5d SLING-10566 : Keep track of feature originas per configuration property
8ce2d5d is described below
commit 8ce2d5dc45f5d180ce16bf202ed4a1aaaa11f7d9
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Mon Jun 28 16:26:38 2021 +0200
SLING-10566 : Keep track of feature originas per configuration property
---
.../org/apache/sling/feature/Configuration.java | 63 +++++++++++++++++++++-
.../apache/sling/feature/builder/BuilderUtil.java | 14 +++++
.../org/apache/sling/feature/package-info.java | 2 +-
.../apache/sling/feature/ConfigurationTest.java | 48 +++++++++++++++++
.../sling/feature/builder/BuilderUtilTest.java | 50 +++++++++++++++++
5 files changed, 175 insertions(+), 2 deletions(-)
diff --git a/src/main/java/org/apache/sling/feature/Configuration.java b/src/main/java/org/apache/sling/feature/Configuration.java
index f8fbe80..6b004f7 100644
--- a/src/main/java/org/apache/sling/feature/Configuration.java
+++ b/src/main/java/org/apache/sling/feature/Configuration.java
@@ -236,7 +236,68 @@ public class Configuration
this.properties.put(PROP_FEATURE_ORIGINS, values);
}
}
-
+
+ /**
+ * Get the feature origins for a property - if recorded
+ *
+ * @param propertyName The name of the property
+ * @return A immutable list of feature artifact ids - list might be empty
+ * @since 1.8
+ * @throws IllegalArgumentException If the stored values are not valid artifact ids
+ */
+ public List<ArtifactId> getFeatureOrigins(final String propertyName) {
+ final List<ArtifactId> list = new ArrayList<>();
+ final Object origins = this.properties.get(PROP_FEATURE_ORIGINS.concat("-").concat(propertyName));
+ if ( origins != null ) {
+ final String[] values = Converters.standardConverter().convert(origins).to(String[].class);
+ for(final String v : values) {
+ list.add(ArtifactId.parse(v));
+ }
+ }
+ return Collections.unmodifiableList(list);
+ }
+
+ /**
+ * Get the feature origins for a property.
+ * If no origins are recorded, the provided id is returned.
+ *
+ * @param propertyName The name of the property
+ * @param self The id of the current feature
+ * @return A immutable list of feature artifact ids
+ * @since 1.8
+ * @throws IllegalArgumentException If the stored values are not valid artifact ids
+ */
+ public List<ArtifactId> getFeatureOrigins(final String propertyName, final ArtifactId self) {
+ final List<ArtifactId> list = new ArrayList<>();
+ final Object origins = this.properties.get(PROP_FEATURE_ORIGINS.concat("-").concat(propertyName));
+ if ( origins != null ) {
+ final String[] values = Converters.standardConverter().convert(origins).to(String[].class);
+ for(final String v : values) {
+ list.add(ArtifactId.parse(v));
+ }
+ }
+ if ( list.isEmpty() ) {
+ list.add(self);
+ }
+ return Collections.unmodifiableList(list);
+ }
+
+ /**
+ * Set the feature origins for a property
+ * @param propertyName The name of the property
+ * @param featureOrigins the list of artifact ids or null to remove the info from this object
+ * @since 1.8
+ */
+ public void setFeatureOrigins(final String propertyName, final List<ArtifactId> featureOrigins) {
+ if ( featureOrigins == null || featureOrigins.isEmpty() ) {
+ this.properties.remove(PROP_FEATURE_ORIGINS.concat("-").concat(propertyName));
+ } else {
+ final List<String> list = featureOrigins.stream().map(ArtifactId::toMvnId).collect(Collectors.toList());
+ final String[] values = Converters.standardConverter().convert(list).to(String[].class);
+ this.properties.put(PROP_FEATURE_ORIGINS.concat("-").concat(propertyName), values);
+ }
+ }
+
/**
* Get the configuration properties of the configuration. This configuration
* properties are all properties minus properties used to manage the
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 a6f617d..b981dbb 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
@@ -450,6 +450,7 @@ class BuilderUtil {
target.remove(found);
found = cfg.copy(cfg.getPid());
target.add(idx, found);
+ setPropertyFeatureOrigins(found, sourceFeatureId);
handled = true;
} else if (BuilderContext.CONFIG_FAIL_ON_PROPERTY_CLASH.equals(override.getValue())){
final List<ArtifactId> origins = found.getFeatureOrigins();
@@ -459,6 +460,7 @@ class BuilderUtil {
break outer;
} else {
found.getProperties().put(key, cfg.getProperties().get(key));
+ cfg.setFeatureOrigins(key, cfg.getFeatureOrigins(key, sourceFeatureId));
}
}
// restore origin
@@ -469,11 +471,15 @@ class BuilderUtil {
for (Enumeration<String> i = cfg.getProperties().keys(); i.hasMoreElements(); ) {
final String key = i.nextElement();
found.getProperties().put(key, cfg.getProperties().get(key));
+ final List<ArtifactId> propOrigins = new ArrayList<>(found.getFeatureOrigins(key));
+ propOrigins.addAll(cfg.getFeatureOrigins(key, sourceFeatureId));
+ found.setFeatureOrigins(key, propOrigins);
}
// restore origin
found.setFeatureOrigins(origins);
handled = true;
} else if (BuilderContext.CONFIG_USE_FIRST.equals(override.getValue())) {
+ // no need to update property origins
handled = true;
found = null;
} else if (BuilderContext.CONFIG_MERGE_FIRST.equals(override.getValue())) {
@@ -482,6 +488,7 @@ class BuilderUtil {
final String key = i.nextElement();
if (found.getProperties().get(key) == null) {
found.getProperties().put(key, cfg.getProperties().get(key));
+ cfg.setFeatureOrigins(key, cfg.getFeatureOrigins(key, sourceFeatureId));
}
}
// restore origin
@@ -499,6 +506,7 @@ class BuilderUtil {
// create new configuration
found = cfg.copy(cfg.getPid());
target.add(found);
+ setPropertyFeatureOrigins(found, sourceFeatureId);
if ( !found.getFeatureOrigins().isEmpty() ) {
found = null;
}
@@ -512,6 +520,12 @@ class BuilderUtil {
}
}
+ private static void setPropertyFeatureOrigins(final Configuration cfg, final ArtifactId sourceFeatureId) {
+ for(final String propName : Collections.list(cfg.getConfigurationProperties().keys())) {
+ cfg.setFeatureOrigins(propName, cfg.getFeatureOrigins(propName, sourceFeatureId));
+ }
+ }
+
/**
* Merge the framework properties
* @param targetFeature The target feature
diff --git a/src/main/java/org/apache/sling/feature/package-info.java b/src/main/java/org/apache/sling/feature/package-info.java
index 2eb78b4..a9a3f0f 100644
--- a/src/main/java/org/apache/sling/feature/package-info.java
+++ b/src/main/java/org/apache/sling/feature/package-info.java
@@ -17,7 +17,7 @@
* under the License.
*/
-@org.osgi.annotation.versioning.Version("1.7.1")
+@org.osgi.annotation.versioning.Version("1.8.0")
package org.apache.sling.feature;
diff --git a/src/test/java/org/apache/sling/feature/ConfigurationTest.java b/src/test/java/org/apache/sling/feature/ConfigurationTest.java
index 8fb4e30..cb403c5 100644
--- a/src/test/java/org/apache/sling/feature/ConfigurationTest.java
+++ b/src/test/java/org/apache/sling/feature/ConfigurationTest.java
@@ -122,4 +122,52 @@ public class ConfigurationTest {
assertEquals("As keys are case insensitive, there should just be 1 key",
1, c1.getProperties().size());
}
+
+ @Test
+ public void testPropertyFeatureOrigins() {
+ final ArtifactId self = ArtifactId.parse("self:self:1");
+
+ final Configuration cfg = new Configuration("foo");
+ cfg.getProperties().put("a", true);
+ assertTrue(cfg.getFeatureOrigins("a").isEmpty());
+ assertNull(cfg.getConfigurationProperties().get(Configuration.PROP_FEATURE_ORIGINS.concat("-").concat("a")));
+ assertNull(cfg.getProperties().get(Configuration.PROP_FEATURE_ORIGINS.concat("-").concat("a")));
+ assertEquals(1, cfg.getFeatureOrigins("a", self).size());
+ assertEquals(self, cfg.getFeatureOrigins("a", self).get(0));
+
+ // single id
+ final ArtifactId id = ArtifactId.parse("g:a:1");
+ cfg.setFeatureOrigins("a", Collections.singletonList(id));
+ assertEquals(1, cfg.getFeatureOrigins("a").size());
+ assertEquals(id, cfg.getFeatureOrigins("a").get(0));
+ assertEquals(1, cfg.getFeatureOrigins("a", self).size());
+ assertEquals(id, cfg.getFeatureOrigins("a", self).get(0));
+
+ assertNull(cfg.getConfigurationProperties().get(Configuration.PROP_FEATURE_ORIGINS.concat("-").concat("a")));
+ assertNotNull(cfg.getProperties().get(Configuration.PROP_FEATURE_ORIGINS.concat("-").concat("a")));
+ final String[] array = (String[]) cfg.getProperties().get(Configuration.PROP_FEATURE_ORIGINS.concat("-").concat("a"));
+ assertArrayEquals(new String[] {id.toMvnId()}, array);
+
+ // add another id
+ final ArtifactId id2 = ArtifactId.parse("g:b:2");
+ cfg.setFeatureOrigins("a", Arrays.asList(id, id2));
+ assertEquals(2, cfg.getFeatureOrigins("a").size());
+ assertEquals(id, cfg.getFeatureOrigins("a").get(0));
+ assertEquals(id2, cfg.getFeatureOrigins("a").get(1));
+ assertEquals(2, cfg.getFeatureOrigins("a", self).size());
+ assertEquals(id, cfg.getFeatureOrigins("a", self).get(0));
+ assertEquals(id2, cfg.getFeatureOrigins("a", self).get(1));
+
+ assertNull(cfg.getConfigurationProperties().get(Configuration.PROP_FEATURE_ORIGINS.concat("-").concat("a")));
+ assertNotNull(cfg.getProperties().get(Configuration.PROP_FEATURE_ORIGINS.concat("-").concat("a")));
+ final String[] array2 = (String[]) cfg.getProperties().get(Configuration.PROP_FEATURE_ORIGINS.concat("-").concat("a"));
+ assertArrayEquals(new String[] {id.toMvnId(), id2.toMvnId()}, array2);
+
+ // remove
+ cfg.getProperties().remove("a");
+ cfg.setFeatureOrigins("a", null);
+ assertTrue(cfg.getFeatureOrigins("a").isEmpty());
+ assertNull(cfg.getConfigurationProperties().get(Configuration.PROP_FEATURE_ORIGINS.concat("-").concat("a")));
+ assertNull(cfg.getProperties().get(Configuration.PROP_FEATURE_ORIGINS.concat("-").concat("a")));
+ }
}
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 9c5e8f0..095f94a 100644
--- a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
@@ -906,6 +906,11 @@ public class BuilderUtilTest {
assertEquals(target.getConfiguration("bar").getConfigurationProperties(), bar.getConfigurationProperties());
assertEquals(1, target.getConfiguration("bar").getFeatureOrigins().size());
assertEquals(SOURCE_ID, target.getConfiguration("bar").getFeatureOrigins().get(0));
+
+ // property origins
+ assertEquals(1, target.getConfiguration("bar").getFeatureOrigins("barKey").size());
+ assertEquals(SOURCE_ID, target.getConfiguration("bar").getFeatureOrigins("barKey").get(0));
+ assertEquals(0, target.getConfiguration("bar").getFeatureOrigins("fookey").size());
}
@Test public void testMergeConfigurationsCLASH() {
@@ -959,6 +964,10 @@ public class BuilderUtilTest {
assertEquals("valueFOO", target.getConfiguration("foo").getConfigurationProperties().get("fooKey"));
assertNull(target.getConfiguration("foo").getConfigurationProperties().get("barKey"));
assertTrue(target.getConfiguration("foo").getFeatureOrigins().isEmpty());
+
+ // property origins
+ assertEquals(0, target.getConfiguration("foo").getFeatureOrigins("fooKey").size());
+ assertEquals(0, target.getConfiguration("foo").getFeatureOrigins("barKey").size());
}
@Test public void testMergeConfigurationsUSELATEST() {
@@ -976,6 +985,11 @@ public class BuilderUtilTest {
assertNull(target.getConfiguration("foo").getConfigurationProperties().get("fooKey"));
assertEquals(1, target.getConfiguration("foo").getFeatureOrigins().size());
assertEquals(SOURCE_ID, target.getConfiguration("foo").getFeatureOrigins().get(0));
+
+ // property origins
+ assertEquals(1, target.getConfiguration("foo").getFeatureOrigins("barKey").size());
+ assertEquals(SOURCE_ID, target.getConfiguration("foo").getFeatureOrigins("barKey").get(0));
+ assertEquals(0, target.getConfiguration("foo").getFeatureOrigins("fooKey").size());
}
@Test public void testMergeConfigurationsMERGELATEST() {
@@ -992,8 +1006,41 @@ public class BuilderUtilTest {
assertEquals("valueBAR", target.getConfiguration("foo").getConfigurationProperties().get("fooKey"));
assertEquals(1, target.getConfiguration("foo").getFeatureOrigins().size());
assertEquals(SOURCE_ID, target.getConfiguration("foo").getFeatureOrigins().get(0));
+
+ // property origins
+ assertEquals(1, target.getConfiguration("foo").getFeatureOrigins("fooKey").size());
+ assertEquals(SOURCE_ID, target.getConfiguration("foo").getFeatureOrigins("fooKey").get(0));
}
+ @Test public void testMergeConfigurationsUsingThreeExtensions() {
+ final ArtifactId SOURCE2_ID = ArtifactId.parse("source:source:2");
+
+ Configurations target = new Configurations();
+ Configurations source = new Configurations();
+ Configurations source2 = new Configurations();
+ Configuration foo = new Configuration("foo");
+ foo.getProperties().put("fooKey", "valueFOO");
+ target.add(foo);
+ Configuration foo2 = new Configuration("foo");
+ foo2.getProperties().put("fooKey", "valueBAR");
+ source.add(foo2);
+ Configuration foo3 = new Configuration("foo");
+ foo3.getProperties().put("fooKey", "final");
+ source2.add(foo3);
+ BuilderUtil.mergeConfigurations(target, source, Collections.singletonMap("fo*", BuilderContext.CONFIG_MERGE_LATEST), SOURCE_ID);
+ BuilderUtil.mergeConfigurations(target, source2, Collections.singletonMap("fo*", BuilderContext.CONFIG_MERGE_LATEST), SOURCE2_ID);
+
+ assertEquals("final", target.getConfiguration("foo").getConfigurationProperties().get("fooKey"));
+ assertEquals(2, target.getConfiguration("foo").getFeatureOrigins().size());
+ assertEquals(SOURCE_ID, target.getConfiguration("foo").getFeatureOrigins().get(0));
+ assertEquals(SOURCE2_ID, target.getConfiguration("foo").getFeatureOrigins().get(1));
+
+ // property origins
+ assertEquals(2, target.getConfiguration("foo").getFeatureOrigins("fooKey").size());
+ assertEquals(SOURCE_ID, target.getConfiguration("foo").getFeatureOrigins("fooKey").get(0));
+ assertEquals(SOURCE2_ID, target.getConfiguration("foo").getFeatureOrigins("fooKey").get(1));
+ }
+
@Test public void testMergeConfigurationsMERGEFIRST() {
Configurations target = new Configurations();
Configurations source = new Configurations();
@@ -1010,6 +1057,9 @@ public class BuilderUtilTest {
assertEquals("valueBAR", target.getConfiguration("foo").getConfigurationProperties().get("barKey"));
assertEquals(1, target.getConfiguration("foo").getFeatureOrigins().size());
assertEquals(SOURCE_ID, target.getConfiguration("foo").getFeatureOrigins().get(0));
+
+ // property origins
+ assertEquals(0, target.getConfiguration("foo").getFeatureOrigins("fooKey").size());
}
@Test public void testMergeConfigurationsFactory() {