This is an automated email from the ASF dual-hosted git repository. pauls pushed a commit to branch variables in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-io.git
commit 3ac28989462dd981f073410b566b5d9fd484f1de Author: Karl Pauls <[email protected]> AuthorDate: Thu Sep 6 09:58:12 2018 +0200 Change variable handling to not allow variables to be defined twice --- .../feature/io/json/ConfigurationJSONReader.java | 5 - .../sling/feature/io/json/FeatureJSONReader.java | 19 ++-- .../sling/feature/io/json/JSONReaderBase.java | 12 +-- .../feature/io/json/FeatureJSONReaderTest.java | 49 --------- src/test/resources/features/test2.json | 107 ------------------- src/test/resources/features/test3.json | 116 --------------------- 6 files changed, 9 insertions(+), 299 deletions(-) diff --git a/src/main/java/org/apache/sling/feature/io/json/ConfigurationJSONReader.java b/src/main/java/org/apache/sling/feature/io/json/ConfigurationJSONReader.java index 7a9e05a..d330482 100644 --- a/src/main/java/org/apache/sling/feature/io/json/ConfigurationJSONReader.java +++ b/src/main/java/org/apache/sling/feature/io/json/ConfigurationJSONReader.java @@ -52,11 +52,6 @@ public class ConfigurationJSONReader extends JSONReaderBase { } } - @Override - protected Object handleResolveVars(final Object val) { - return val; - } - /** * Private constructor * @param location Optional location diff --git a/src/main/java/org/apache/sling/feature/io/json/FeatureJSONReader.java b/src/main/java/org/apache/sling/feature/io/json/FeatureJSONReader.java index 04fc850..0a9eb89 100644 --- a/src/main/java/org/apache/sling/feature/io/json/FeatureJSONReader.java +++ b/src/main/java/org/apache/sling/feature/io/json/FeatureJSONReader.java @@ -153,11 +153,6 @@ public class FeatureJSONReader extends JSONReaderBase { } } - @Override - protected Object handleResolveVars(final Object val) { - return FeatureBuilder.replaceVariables(val, null, this.feature); - } - private void readIncludes(final Map<String, Object> map) throws IOException { if ( map.containsKey(JSONConstants.FEATURE_INCLUDES)) { final Object includesObj = map.get(JSONConstants.FEATURE_INCLUDES); @@ -178,7 +173,7 @@ public class FeatureJSONReader extends JSONReaderBase { 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(handleResolveVars(obj.get(JSONConstants.ARTIFACT_ID)).toString()); + final ArtifactId id = ArtifactId.parse(obj.get(JSONConstants.ARTIFACT_ID).toString()); include = new Include(id); if ( obj.containsKey(JSONConstants.INCLUDE_REMOVALS) ) { @@ -282,7 +277,7 @@ public class FeatureJSONReader extends JSONReaderBase { 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) -> ManifestUtils.unmarshalAttribute(key, handleResolveVars(value), attrMap::put))); + attrs.forEach(rethrowBiConsumer((key, value) -> ManifestUtils.unmarshalAttribute(key, value, attrMap::put))); } Map<String, String> dirMap = new HashMap<>(); @@ -290,10 +285,10 @@ public class FeatureJSONReader extends JSONReaderBase { 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) -> ManifestUtils.unmarshalDirective(key, handleResolveVars(value), dirMap::put))); + dirs.forEach(rethrowBiConsumer((key, value) -> ManifestUtils.unmarshalDirective(key, value, dirMap::put))); } - final Requirement r = new RequirementImpl(null, handleResolveVars(obj.get(JSONConstants.REQCAP_NAMESPACE)).toString(), dirMap, attrMap); + final Requirement r = new RequirementImpl(null, obj.get(JSONConstants.REQCAP_NAMESPACE).toString(), dirMap, attrMap); feature.getRequirements().add(r); } } @@ -321,7 +316,7 @@ public class FeatureJSONReader extends JSONReaderBase { 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) -> ManifestUtils.unmarshalAttribute(key, handleResolveVars(value), attrMap::put))); + attrs.forEach(rethrowBiConsumer((key, value) -> ManifestUtils.unmarshalAttribute(key, value, attrMap::put))); } Map<String, String> dirMap = new HashMap<>(); @@ -329,10 +324,10 @@ public class FeatureJSONReader extends JSONReaderBase { 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) -> ManifestUtils.unmarshalDirective(key, handleResolveVars(value), dirMap::put))); + dirs.forEach(rethrowBiConsumer((key, value) -> ManifestUtils.unmarshalDirective(key, value, dirMap::put))); } - final Capability c = new CapabilityImpl(null, handleResolveVars(obj.get(JSONConstants.REQCAP_NAMESPACE)).toString(), dirMap, attrMap); + final Capability c = new CapabilityImpl(null, obj.get(JSONConstants.REQCAP_NAMESPACE).toString(), dirMap, attrMap); feature.getCapabilities().add(c); } } diff --git a/src/main/java/org/apache/sling/feature/io/json/JSONReaderBase.java b/src/main/java/org/apache/sling/feature/io/json/JSONReaderBase.java index 1e57427..5413070 100644 --- a/src/main/java/org/apache/sling/feature/io/json/JSONReaderBase.java +++ b/src/main/java/org/apache/sling/feature/io/json/JSONReaderBase.java @@ -220,7 +220,7 @@ abstract class JSONReaderBase { if ( entry.toString().startsWith("#") ) { continue; } - artifact = new Artifact(ArtifactId.parse(handleResolveVars(entry).toString())); + artifact = new Artifact(ArtifactId.parse((String) entry)); } else { @SuppressWarnings("unchecked") final Map<String, Object> bundleObj = (Map<String, Object>) entry; @@ -228,7 +228,7 @@ abstract class JSONReaderBase { 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(handleResolveVars(bundleObj.get(JSONConstants.ARTIFACT_ID)).toString()); + final ArtifactId id = ArtifactId.parse(bundleObj.get(JSONConstants.ARTIFACT_ID).toString()); artifact = new Artifact(id); for(final Map.Entry<String, Object> metadataEntry : bundleObj.entrySet()) { @@ -248,14 +248,6 @@ abstract class JSONReaderBase { } } - /** Substitutes variables that need to be specified before the resolver executes. - * These are variables in features, artifacts (such as bundles), requirements - * and capabilities. - * @param val The value that may contain a variable. - * @return The value with the variable substituted. - */ - abstract protected Object handleResolveVars(Object val); - protected void addConfigurations(final Map<String, Object> map, final Artifact artifact, final Configurations container) throws IOException { diff --git a/src/test/java/org/apache/sling/feature/io/json/FeatureJSONReaderTest.java b/src/test/java/org/apache/sling/feature/io/json/FeatureJSONReaderTest.java index b3a3ee0..f231711 100644 --- a/src/test/java/org/apache/sling/feature/io/json/FeatureJSONReaderTest.java +++ b/src/test/java/org/apache/sling/feature/io/json/FeatureJSONReaderTest.java @@ -65,55 +65,6 @@ public class FeatureJSONReaderTest { } - @Test public void testReadWithVariablesResolve() 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("Framework property substitution should not happen at resolve time", - "${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("Configuration substitution should not happen at resolve time", - "aa${ab_config}", props.get("a.value")); - assertEquals("${ab_config}bb", props.get("b.value")); - assertEquals("c${c_config}c", props.get("c.value")); - } - @Test public void testReadRepoInitExtension() throws Exception { Feature feature = U.readFeature("repoinit"); Extensions extensions = feature.getExtensions(); diff --git a/src/test/resources/features/test2.json b/src/test/resources/features/test2.json deleted file mode 100644 index 0e5c1c6..0000000 --- a/src/test/resources/features/test2.json +++ /dev/null @@ -1,107 +0,0 @@ -{ - "model-version": "1", - "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 diff --git a/src/test/resources/features/test3.json b/src/test/resources/features/test3.json deleted file mode 100644 index 630239d..0000000 --- a/src/test/resources/features/test3.json +++ /dev/null @@ -1,116 +0,0 @@ -{ - "#": "A comment", - "# array": ["array", "comment"], - "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", - "refvar": "${refvar}" - }, - - "includes" : [ - { - "#": "comment", - "id" : "${sling.gid}/sling/10", - "removals" : { - "configurations" : [ - ], - "bundles" : [ - ], - "#": "comment", - "framework-properties" : [ - ] - } - } - ], - "requirements" : [ - { - "namespace" : "osgi.${ns}", - "#": "comment", - "directives" : { - "#": "comment", - "filter" : "(&(osgi.contract=${contract.name})(&(version>=3.0)(!(version>=4.0))))" - } - } - ], - "capabilities" : [ - { - "#": "comment", - "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" : { - "#": "comment", - "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" : { - "# one": "comment", - "# two": "comment", - "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, - "#": "comment" - }, - { - "id" : "org.apache.sling/application-bundle/2.0.0", - "start-order" : 1 - }, - { - "id" : "org.apache.sling/another-bundle/2.1.0", - "start-order" : 1 - } - ], - "configurations" : { - "#": "comment", - "my.pid" : { - "#": "comment", - "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", - "refvar": "${refvar}" - } - } -} \ No newline at end of file
