Merge pull request #2 from apache/variables
Change variable handling to not allow variables to be defined twice
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 @@
}
}
- @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 @@
}
}
- @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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
}
- /** 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 @@
}
- @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
