bosschaert closed pull request #10: Initial variable support on the feature level URL: https://github.com/apache/sling-whiteboard/pull/10
This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/FeatureJSONReader.java b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/FeatureJSONReader.java index 9c50a41..401cc09 100644 --- a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/FeatureJSONReader.java +++ b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/FeatureJSONReader.java @@ -16,9 +16,14 @@ */ package org.apache.sling.feature.support.json; -import static org.apache.sling.feature.support.util.LambdaUtil.rethrowBiConsumer; -import static org.apache.sling.feature.support.util.ManifestUtil.unmarshalAttribute; -import static org.apache.sling.feature.support.util.ManifestUtil.unmarshalDirective; +import org.apache.felix.configurator.impl.json.JSONUtil; +import org.apache.sling.feature.ArtifactId; +import org.apache.sling.feature.Feature; +import org.apache.sling.feature.Include; +import org.apache.sling.feature.OSGiCapability; +import org.apache.sling.feature.OSGiRequirement; +import org.osgi.resource.Capability; +import org.osgi.resource.Requirement; import java.io.IOException; import java.io.Reader; @@ -27,23 +32,22 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.json.Json; import javax.json.JsonObject; -import org.apache.felix.configurator.impl.json.JSONUtil; -import org.apache.sling.feature.ArtifactId; -import org.apache.sling.feature.Feature; -import org.apache.sling.feature.Include; -import org.apache.sling.feature.OSGiCapability; -import org.apache.sling.feature.OSGiRequirement; -import org.osgi.resource.Capability; -import org.osgi.resource.Requirement; +import static org.apache.sling.feature.support.util.LambdaUtil.rethrowBiConsumer; +import static org.apache.sling.feature.support.util.ManifestUtil.unmarshalAttribute; +import static org.apache.sling.feature.support.util.ManifestUtil.unmarshalDirective; /** * This class offers a method to read a {@code Feature} using a {@code Reader} instance. */ public class FeatureJSONReader extends JSONReaderBase { + // The pattern that variables in Feature JSON follow + private static final Pattern VARIABLE_PATTERN = Pattern.compile("\\$\\{[a-zA-Z0-9.-_]+\\}"); /** * Read a new feature from the reader @@ -87,12 +91,15 @@ public static Feature read(final Reader reader, /** The provided id. */ private final ArtifactId providedId; + /** The variables from the JSON. */ + private Map<String, String> variables; + /** * Private constructor * @param pId Optional id * @param location Optional location */ - private FeatureJSONReader(final ArtifactId pId, final String location) { + FeatureJSONReader(final ArtifactId pId, final String location) { super(location); this.providedId = pId; } @@ -130,6 +137,8 @@ private Feature readFeature(final Reader reader) this.feature.setVendor(getProperty(map, JSONConstants.FEATURE_VENDOR)); this.feature.setLicense(getProperty(map, JSONConstants.FEATURE_LICENSE)); + this.variables = this.readVariables(map); + this.readBundles(map, feature.getBundles(), feature.getConfigurations()); this.readFrameworkProperties(map, feature.getFrameworkProperties()); this.readConfigurations(map, feature.getConfigurations()); @@ -145,6 +154,53 @@ private Feature readFeature(final Reader reader) return feature; } + @Override + protected Object handleVars(Object value) { + if (!(value instanceof String)) { + return value; + } + + String textWithVars = (String) value; + + Matcher m = VARIABLE_PATTERN.matcher(textWithVars.toString()); + StringBuffer sb = new StringBuffer(); + while (m.find()) { + String var = m.group(); + + int len = var.length(); + String name = var.substring(2, len - 1); + String val = variables.get(name); + if (val != null) { + m.appendReplacement(sb, Matcher.quoteReplacement(val)); + } else { + throw new IllegalStateException("Undefined variable: " + name); + } + } + m.appendTail(sb); + + return sb.toString(); + } + + private Map<String, String> readVariables(Map<String, Object> map) throws IOException { + Map<String, String> varMap = new HashMap<>(); + + if (map.containsKey(JSONConstants.FEATURE_VARIABLES)) { + final Object variablesObj = map.get(JSONConstants.FEATURE_VARIABLES); + checkType(JSONConstants.FEATURE_VARIABLES, variablesObj, Map.class); + + @SuppressWarnings("unchecked") + final Map<String, Object> vars = (Map<String, Object>) variablesObj; + for (final Map.Entry<String, Object> entry : vars.entrySet()) { + checkType("variable value", entry.getValue(), String.class, Boolean.class, Number.class); + if (varMap.get(entry.getKey()) != null) { + throw new IOException(this.exceptionPrefix + "Duplicate variable " + entry.getKey()); + } + varMap.put(entry.getKey(), "" + entry.getValue()); + } + } + return varMap; + } + private void readIncludes(final Map<String, Object> map) throws IOException { if ( map.containsKey(JSONConstants.FEATURE_INCLUDES)) { final Object includesObj = map.get(JSONConstants.FEATURE_INCLUDES); @@ -165,7 +221,7 @@ private void readIncludes(final Map<String, Object> map) throws IOException { throw new IOException(exceptionPrefix + " include is missing required artifact id"); } checkType("Include " + JSONConstants.ARTIFACT_ID, obj.get(JSONConstants.ARTIFACT_ID), String.class); - final ArtifactId id = ArtifactId.parse(obj.get(JSONConstants.ARTIFACT_ID).toString()); + final ArtifactId id = ArtifactId.parse(handleVars(obj.get(JSONConstants.ARTIFACT_ID)).toString()); include = new Include(id); if ( obj.containsKey(JSONConstants.INCLUDE_REMOVALS) ) { @@ -263,7 +319,7 @@ private void readRequirements(Map<String, Object> map) throws IOException { checkType("Requirement attributes", obj.get(JSONConstants.REQCAP_ATTRIBUTES), Map.class); @SuppressWarnings("unchecked") final Map<String, Object> attrs = (Map<String, Object>)obj.get(JSONConstants.REQCAP_ATTRIBUTES); - attrs.forEach(rethrowBiConsumer((key, value) -> unmarshalAttribute(key, value, attrMap::put))); + attrs.forEach(rethrowBiConsumer((key, value) -> unmarshalAttribute(key, handleVars(value), attrMap::put))); } Map<String, String> dirMap = new HashMap<>(); @@ -271,10 +327,10 @@ private void readRequirements(Map<String, Object> map) throws IOException { checkType("Requirement directives", obj.get(JSONConstants.REQCAP_DIRECTIVES), Map.class); @SuppressWarnings("unchecked") final Map<String, Object> dirs = (Map<String, Object>)obj.get(JSONConstants.REQCAP_DIRECTIVES); - dirs.forEach(rethrowBiConsumer((key, value) -> unmarshalDirective(key, value, dirMap::put))); + dirs.forEach(rethrowBiConsumer((key, value) -> unmarshalDirective(key, handleVars(value), dirMap::put))); } - final Requirement r = new OSGiRequirement(obj.get(JSONConstants.REQCAP_NAMESPACE).toString(), attrMap, dirMap); + final Requirement r = new OSGiRequirement(handleVars(obj.get(JSONConstants.REQCAP_NAMESPACE)).toString(), attrMap, dirMap); feature.getRequirements().add(r); } } @@ -302,7 +358,7 @@ private void readCapabilities(Map<String, Object> map) throws IOException { checkType("Capability attributes", obj.get(JSONConstants.REQCAP_ATTRIBUTES), Map.class); @SuppressWarnings("unchecked") final Map<String, Object> attrs = (Map<String, Object>)obj.get(JSONConstants.REQCAP_ATTRIBUTES); - attrs.forEach(rethrowBiConsumer((key, value) -> unmarshalAttribute(key, value, attrMap::put))); + attrs.forEach(rethrowBiConsumer((key, value) -> unmarshalAttribute(key, handleVars(value), attrMap::put))); } Map<String, String> dirMap = new HashMap<>(); @@ -310,10 +366,10 @@ private void readCapabilities(Map<String, Object> map) throws IOException { checkType("Capability directives", obj.get(JSONConstants.REQCAP_DIRECTIVES), Map.class); @SuppressWarnings("unchecked") final Map<String, Object> dirs = (Map<String, Object>) obj.get(JSONConstants.REQCAP_DIRECTIVES); - dirs.forEach(rethrowBiConsumer((key, value) -> unmarshalDirective(key, value, dirMap::put))); + dirs.forEach(rethrowBiConsumer((key, value) -> unmarshalDirective(key, handleVars(value), dirMap::put))); } - final Capability c = new OSGiCapability(obj.get(JSONConstants.REQCAP_NAMESPACE).toString(), attrMap, dirMap); + final Capability c = new OSGiCapability(handleVars(obj.get(JSONConstants.REQCAP_NAMESPACE)).toString(), attrMap, dirMap); feature.getCapabilities().add(c); } } diff --git a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/JSONConstants.java b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/JSONConstants.java index 8f1fce5..1b52d73 100644 --- a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/JSONConstants.java +++ b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/JSONConstants.java @@ -16,15 +16,17 @@ */ package org.apache.sling.feature.support.json; +import org.apache.sling.feature.Configuration; + import java.util.Arrays; import java.util.List; -import org.apache.sling.feature.Configuration; - public abstract class JSONConstants { public static final String FEATURE_ID = "id"; + public static final String FEATURE_VARIABLES = "variables"; + public static final String FEATURE_BUNDLES = "bundles"; public static final String FEATURE_FRAMEWORK_PROPERTIES = "framework-properties"; @@ -46,6 +48,7 @@ public static final String FEATURE_LICENSE = "license"; public static final List<String> FEATURE_KNOWN_PROPERTIES = Arrays.asList(FEATURE_ID, + FEATURE_VARIABLES, FEATURE_BUNDLES, FEATURE_FRAMEWORK_PROPERTIES, FEATURE_CONFIGURATIONS, diff --git a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/JSONReaderBase.java b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/JSONReaderBase.java index f3182d4..ee66e68 100644 --- a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/JSONReaderBase.java +++ b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/JSONReaderBase.java @@ -16,6 +16,20 @@ */ package org.apache.sling.feature.support.json; +import org.apache.felix.configurator.impl.json.JSMin; +import org.apache.felix.configurator.impl.json.JSONUtil; +import org.apache.felix.configurator.impl.json.TypeConverter; +import org.apache.felix.configurator.impl.model.Config; +import org.apache.sling.feature.Artifact; +import org.apache.sling.feature.ArtifactId; +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.ExtensionType; +import org.apache.sling.feature.Extensions; +import org.apache.sling.feature.KeyValueMap; + import java.io.IOException; import java.io.Reader; import java.io.StringWriter; @@ -34,20 +48,6 @@ import javax.json.JsonStructure; import javax.json.JsonWriter; -import org.apache.felix.configurator.impl.json.JSMin; -import org.apache.felix.configurator.impl.json.JSONUtil; -import org.apache.felix.configurator.impl.json.TypeConverter; -import org.apache.felix.configurator.impl.model.Config; -import org.apache.sling.feature.Artifact; -import org.apache.sling.feature.ArtifactId; -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.ExtensionType; -import org.apache.sling.feature.Extensions; -import org.apache.sling.feature.KeyValueMap; - /** * Common methods for JSON reading. */ @@ -138,7 +138,7 @@ protected void readArtifacts(final String section, final Artifact artifact; checkType(artifactType, entry, Map.class, String.class); if ( entry instanceof String ) { - artifact = new Artifact(ArtifactId.parse(entry.toString())); + artifact = new Artifact(ArtifactId.parse(handleVars(entry).toString())); } else { @SuppressWarnings("unchecked") final Map<String, Object> bundleObj = (Map<String, Object>) entry; @@ -146,7 +146,7 @@ protected void readArtifacts(final String section, throw new IOException(exceptionPrefix + " " + artifactType + " is missing required artifact id"); } checkType(artifactType + " " + JSONConstants.ARTIFACT_ID, bundleObj.get(JSONConstants.ARTIFACT_ID), String.class); - final ArtifactId id = ArtifactId.parse(bundleObj.get(JSONConstants.ARTIFACT_ID).toString()); + final ArtifactId id = ArtifactId.parse(handleVars(bundleObj.get(JSONConstants.ARTIFACT_ID)).toString()); artifact = new Artifact(id); for(final Map.Entry<String, Object> metadataEntry : bundleObj.entrySet()) { @@ -166,6 +166,11 @@ protected void readArtifacts(final String section, } } + protected Object handleVars(Object val) { + // No variable substitution at this level, but subclasses can add this in + return val; + } + protected void addConfigurations(final Map<String, Object> map, final Artifact artifact, final Configurations container) throws IOException { @@ -201,7 +206,7 @@ protected void addConfigurations(final Map<String, Object> map, throw new IOException(exceptionPrefix + "Configuration must not define configurator property " + key); } final Object val = c.getProperties().get(key); - config.getProperties().put(key, val); + config.getProperties().put(key, handleVars(val)); } if ( config.getProperties().get(Configuration.PROP_ARTIFACT) != null ) { throw new IOException(exceptionPrefix + "Configuration must not define property " + Configuration.PROP_ARTIFACT); @@ -241,7 +246,7 @@ protected void readFrameworkProperties(final Map<String, Object> map, if ( container.get(entry.getKey()) != null ) { throw new IOException(this.exceptionPrefix + "Duplicate framework property " + entry.getKey()); } - container.put(entry.getKey(), entry.getValue().toString()); + container.put(entry.getKey(), handleVars(entry.getValue()).toString()); } } diff --git a/featuremodel/feature-support/src/test/java/org/apache/sling/feature/support/json/FeatureJSONReaderTest.java b/featuremodel/feature-support/src/test/java/org/apache/sling/feature/support/json/FeatureJSONReaderTest.java index 28674a0..d82e906 100644 --- a/featuremodel/feature-support/src/test/java/org/apache/sling/feature/support/json/FeatureJSONReaderTest.java +++ b/featuremodel/feature-support/src/test/java/org/apache/sling/feature/support/json/FeatureJSONReaderTest.java @@ -16,18 +16,32 @@ */ package org.apache.sling.feature.support.json; +import org.apache.sling.feature.ArtifactId; +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.Extensions; import org.apache.sling.feature.Feature; +import org.apache.sling.feature.Include; +import org.apache.sling.feature.KeyValueMap; import org.junit.Test; import org.osgi.resource.Capability; +import org.osgi.resource.Requirement; +import java.lang.reflect.Field; import java.util.Arrays; +import java.util.Collections; +import java.util.Dictionary; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class FeatureJSONReaderTest { @@ -55,6 +69,53 @@ } + @Test public void testReadWithVariables() throws Exception { + final Feature feature = U.readFeature("test2"); + + List<Include> includes = feature.getIncludes(); + assertEquals(1, includes.size()); + Include include = includes.get(0); + assertEquals("org.apache.sling:sling:9", include.getId().toMvnId()); + + List<Requirement> reqs = feature.getRequirements(); + Requirement req = reqs.get(0); + assertEquals("osgi.contract", req.getNamespace()); + assertEquals("(&(osgi.contract=JavaServlet)(&(version>=3.0)(!(version>=4.0))))", + req.getDirectives().get("filter")); + + List<Capability> caps = feature.getCapabilities(); + Capability cap = null; + for (Capability c : caps) { + if ("osgi.service".equals(c.getNamespace())) { + cap = c; + break; + } + } + assertEquals(Collections.singletonList("org.osgi.service.http.runtime.HttpServiceRuntime"), + cap.getAttributes().get("objectClass")); + assertEquals("org.osgi.service.http.runtime", + cap.getDirectives().get("uses")); + // TODO this seems quite broken: fix! + // assertEquals("org.osgi.service.http.runtime,org.osgi.service.http.runtime.dto", + // cap.getDirectives().get("uses")); + + KeyValueMap fwProps = feature.getFrameworkProperties(); + assertEquals("something", fwProps.get("brave")); + + Bundles bundles = feature.getBundles(); + ArtifactId id = new ArtifactId("org.apache.sling", "foo-xyz", "1.2.3", null, null); + assertTrue(bundles.containsExact(id)); + ArtifactId id2 = new ArtifactId("org.apache.sling", "bar-xyz", "1.2.3", null, null); + assertTrue(bundles.containsExact(id2)); + + Configurations configurations = feature.getConfigurations(); + Configuration config = configurations.getConfiguration("my.pid2"); + Dictionary<String, Object> props = config.getProperties(); + assertEquals("aaright!", props.get("a.value")); + assertEquals("right!bb", props.get("b.value")); + assertEquals("creally?c", props.get("c.value")); + } + @Test public void testReadRepoInitExtension() throws Exception { Feature feature = U.readFeature("repoinit"); Extensions extensions = feature.getExtensions(); @@ -70,4 +131,29 @@ Extension ext = extensions.iterator().next(); assertEquals("some repo init\ntext\n", ext.getText()); } + + @Test public void testHandleVars() throws Exception { + FeatureJSONReader reader = new FeatureJSONReader(null, null); + Map<String, Object> vars = new HashMap<>(); + vars.put("var1", "bar"); + vars.put("varvariable", "${myvar}"); + vars.put("var.2", "2"); + setPrivateField(reader, "variables", vars); + + assertEquals("foobarfoo", reader.handleVars("foo${var1}foo")); + assertEquals("barbarbar", reader.handleVars("${var1}${var1}${var1}")); + assertEquals("${}test${myvar}2", reader.handleVars("${}test${varvariable}${var.2}")); + try { + reader.handleVars("${undefined}"); + fail("Should throw an exception on the undefined variable"); + } catch (IllegalStateException ise) { + // good + } + } + + private void setPrivateField(Object obj, String name, Object value) throws Exception { + Field field = obj.getClass().getDeclaredField(name); + field.setAccessible(true); + field.set(obj, value); + } } diff --git a/featuremodel/feature-support/src/test/resources/features/test2.json b/featuremodel/feature-support/src/test/resources/features/test2.json new file mode 100644 index 0000000..acae2a2 --- /dev/null +++ b/featuremodel/feature-support/src/test/resources/features/test2.json @@ -0,0 +1,106 @@ +{ + "id" : "org.apache.sling/test2/1.1", + + "variables": { + "common.version": "1.2.3", + "contract.name": "JavaServlet", + "ab_config": "right!", + "c_config": "really?", + "includever": "9", + "ns": "contract", + "sling.gid": "org.apache.sling", + "something": "something", + "svc": "service" + }, + + "includes" : [ + { + "id" : "${sling.gid}/sling/${includever}", + "removals" : { + "configurations" : [ + ], + "bundles" : [ + ], + "framework-properties" : [ + ] + } + } + ], + "requirements" : [ + { + "namespace" : "osgi.${ns}", + "directives" : { + "filter" : "(&(osgi.contract=${contract.name})(&(version>=3.0)(!(version>=4.0))))" + } + } + ], + "capabilities" : [ + { + "namespace" : "osgi.implementation", + "attributes" : { + "osgi.implementation" : "osgi.http", + "version:Version" : "1.1" + }, + "directives" : { + "uses" : "javax.servlet,javax.servlet.http,org.osgi.service.http.context,org.osgi.service.http.whiteboard" + } + }, + { + "namespace" : "osgi.${svc}", + "attributes" : { + "objectClass:List<String>" : "org.osgi.${svc}.http.runtime.HttpServiceRuntime" + }, + "directives" : { + "uses" : "org.osgi.${svc}.http.runtime,org.osgi.${svc}.http.runtime.dto" + } + }, + { + "namespace" : "osgi.contract", + "attributes" : { + "osgi.contract" : "JavaServlet", + "osgi.implementation" : "osgi.http", + "version:Version" : "3.1" + }, + "directives" : { + "uses" : "org.osgi.service.http.runtime,org.osgi.service.http.runtime.dto" + } + } + ], + "framework-properties" : { + "foo" : 1, + "brave" : "${something}", + "org.apache.felix.scr.directory" : "launchpad/scr" + }, + "bundles" :[ + { + "id" : "org.apache.sling/oak-server/1.0.0", + "hash" : "4632463464363646436", + "start-order" : 1 + }, + { + "id" : "org.apache.sling/application-bundle/2.0.0", + "start-order" : 1 + }, + { + "id" : "org.apache.sling/another-bundle/2.1.0", + "start-order" : 1 + }, + { + "id" : "org.apache.sling/foo-xyz/${common.version}", + "start-order" : 2 + }, + "org.apache.sling/bar-xyz/${common.version}" + ], + "configurations" : { + "my.pid" : { + "foo" : 5, + "bar" : "test", + "number:Integer" : 7 + }, + "my.pid2" : { + "a.value" : "aa${ab_config}", + "b.value" : "${ab_config}bb", + "c.value" : "c${c_config}c" + } + } +} \ No newline at end of file ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services