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/03/16 12:06:11 UTC

[sling-whiteboard] branch master updated: [Feature Model] Support separate phases for reading variables

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-whiteboard.git


The following commit(s) were added to refs/heads/master by this push:
     new 122d5c8  [Feature Model] Support separate phases for reading variables
122d5c8 is described below

commit 122d5c8b4945fbdb4d69e89d2fe6e609ed3f0c81
Author: David Bosschaert <da...@gmail.com>
AuthorDate: Fri Mar 16 12:01:14 2018 +0000

    [Feature Model] Support separate phases for reading variables
    
    Variables in the feature model must be substituted at different points in
    time, depending on where they reside. Variables in the includes, bundle,
    requirements and capabilities section must be substituted before the
    resolver runs, as they influence the resolver result. They should not be
    substituted at launch time. Other variables, such as configuration and
    framework properties variables must be substituted at launch time.
---
 .../sling/feature/analyser/AnalyserTest.java       |   5 +-
 .../impl/ApplicationBuilderTest.java               |   3 +-
 .../feature/resolver/FrameworkResolverTest.java    |   3 +-
 .../apache/sling/feature/support/FeatureUtil.java  |   5 +-
 .../feature/support/json/FeatureJSONReader.java    |  50 +++++++---
 .../sling/feature/support/json/JSONReaderBase.java |  26 +++++-
 .../support/json/FeatureJSONReaderTest.java        |  59 ++++++++++--
 .../support/json/FeatureJSONWriterTest.java        |   3 +-
 .../org/apache/sling/feature/support/json/U.java   |  10 +-
 .../src/test/resources/features/test3.json         | 103 +++++++++++++++++++++
 .../apache/sling/feature/maven/Preprocessor.java   |  27 +++---
 .../apache/sling/feature/maven/ProjectHelper.java  |   5 +-
 12 files changed, 250 insertions(+), 49 deletions(-)

diff --git a/featuremodel/feature-analyser/src/test/java/org/apache/sling/feature/analyser/AnalyserTest.java b/featuremodel/feature-analyser/src/test/java/org/apache/sling/feature/analyser/AnalyserTest.java
index bff0af9..f2b4b95 100644
--- a/featuremodel/feature-analyser/src/test/java/org/apache/sling/feature/analyser/AnalyserTest.java
+++ b/featuremodel/feature-analyser/src/test/java/org/apache/sling/feature/analyser/AnalyserTest.java
@@ -28,6 +28,7 @@ import org.apache.sling.feature.support.ArtifactManager;
 import org.apache.sling.feature.support.ArtifactManagerConfig;
 import org.apache.sling.feature.support.FeatureUtil;
 import org.apache.sling.feature.support.json.FeatureJSONReader;
+import org.apache.sling.feature.support.json.FeatureJSONReader.Phase;
 import org.junit.Test;
 
 import java.io.File;
@@ -46,7 +47,7 @@ public class AnalyserTest {
         final Analyser analyser = new Analyser(scanner);
         try ( final Reader reader = new InputStreamReader(AnalyserTest.class.getResourceAsStream("/feature_complete.json"),
                 "UTF-8") ) {
-            Feature feature = FeatureJSONReader.read(reader, "feature");
+            Feature feature = FeatureJSONReader.read(reader, "feature", Phase.RESOLVE);
 
             Application app = FeatureUtil.assembleApplication(null, ArtifactManager.getArtifactManager(new ArtifactManagerConfig()),
                     getTestResolver(), feature);
@@ -61,7 +62,7 @@ public class AnalyserTest {
         final Analyser analyser = new Analyser(scanner);
         try ( final Reader reader = new InputStreamReader(AnalyserTest.class.getResourceAsStream("/feature_incomplete.json"),
                 "UTF-8") ) {
-            Feature feature = FeatureJSONReader.read(reader, "feature");
+            Feature feature = FeatureJSONReader.read(reader, "feature", Phase.RESOLVE);
 
             Application app = FeatureUtil.assembleApplication(null, ArtifactManager.getArtifactManager(new ArtifactManagerConfig()),
                     getTestResolver(), feature);
diff --git a/featuremodel/feature-applicationbuilder/src/test/java/org/apache/sling/feature/applicationbuilder/impl/ApplicationBuilderTest.java b/featuremodel/feature-applicationbuilder/src/test/java/org/apache/sling/feature/applicationbuilder/impl/ApplicationBuilderTest.java
index 0c88957..9d84329 100644
--- a/featuremodel/feature-applicationbuilder/src/test/java/org/apache/sling/feature/applicationbuilder/impl/ApplicationBuilderTest.java
+++ b/featuremodel/feature-applicationbuilder/src/test/java/org/apache/sling/feature/applicationbuilder/impl/ApplicationBuilderTest.java
@@ -29,6 +29,7 @@ import org.apache.sling.feature.support.ArtifactManager;
 import org.apache.sling.feature.support.ArtifactManagerConfig;
 import org.apache.sling.feature.support.json.ApplicationJSONWriter;
 import org.apache.sling.feature.support.json.FeatureJSONReader;
+import org.apache.sling.feature.support.json.FeatureJSONReader.Phase;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -155,7 +156,7 @@ public class ApplicationBuilderTest {
         final ArtifactHandler featureArtifact = artifactManager.getArtifactHandler(file);
 
         try (final FileReader r = new FileReader(featureArtifact.getFile())) {
-            final Feature f = FeatureJSONReader.read(r, featureArtifact.getUrl());
+            final Feature f = FeatureJSONReader.read(r, featureArtifact.getUrl(), Phase.RESOLVE);
             return f;
         }
     }
diff --git a/featuremodel/feature-resolver/src/test/java/org/apache/sling/feature/resolver/FrameworkResolverTest.java b/featuremodel/feature-resolver/src/test/java/org/apache/sling/feature/resolver/FrameworkResolverTest.java
index 290263e..4d3cb59 100644
--- a/featuremodel/feature-resolver/src/test/java/org/apache/sling/feature/resolver/FrameworkResolverTest.java
+++ b/featuremodel/feature-resolver/src/test/java/org/apache/sling/feature/resolver/FrameworkResolverTest.java
@@ -23,6 +23,7 @@ import org.apache.sling.feature.support.ArtifactHandler;
 import org.apache.sling.feature.support.ArtifactManager;
 import org.apache.sling.feature.support.ArtifactManagerConfig;
 import org.apache.sling.feature.support.json.FeatureJSONReader;
+import org.apache.sling.feature.support.json.FeatureJSONReader.Phase;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -140,7 +141,7 @@ public class FrameworkResolverTest {
         final ArtifactHandler featureArtifact = artifactManager.getArtifactHandler(file);
 
         try (final FileReader r = new FileReader(featureArtifact.getFile())) {
-            final Feature f = FeatureJSONReader.read(r, featureArtifact.getUrl());
+            final Feature f = FeatureJSONReader.read(r, featureArtifact.getUrl(), Phase.RESOLVE);
             return f;
         }
     }
diff --git a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/FeatureUtil.java b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/FeatureUtil.java
index 70a7b50..3a39086 100644
--- a/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/FeatureUtil.java
+++ b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/FeatureUtil.java
@@ -24,6 +24,7 @@ import org.apache.sling.feature.process.BuilderContext;
 import org.apache.sling.feature.process.FeatureProvider;
 import org.apache.sling.feature.process.FeatureResolver;
 import org.apache.sling.feature.support.json.FeatureJSONReader;
+import org.apache.sling.feature.support.json.FeatureJSONReader.Phase;
 
 import java.io.File;
 import java.io.FileReader;
@@ -244,7 +245,7 @@ public class FeatureUtil {
                 try {
                     final ArtifactHandler handler = artifactManager.getArtifactHandler("mvn:" + id.toMvnPath());
                     try (final FileReader r = new FileReader(handler.getFile())) {
-                        final Feature f = FeatureJSONReader.read(r, handler.getUrl());
+                        final Feature f = FeatureJSONReader.read(r, handler.getUrl(), Phase.RESOLVE);
                         return f;
                     }
 
@@ -278,7 +279,7 @@ public class FeatureUtil {
         final ArtifactHandler featureArtifact = artifactManager.getArtifactHandler(file);
 
         try (final FileReader r = new FileReader(featureArtifact.getFile())) {
-            final Feature f = FeatureJSONReader.read(r, featureArtifact.getUrl());
+            final Feature f = FeatureJSONReader.read(r, featureArtifact.getUrl(), Phase.RESOLVE);
             return f;
         }
     }
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 401cc09..50d5a0b 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
@@ -46,6 +46,8 @@ import static org.apache.sling.feature.support.util.ManifestUtil.unmarshalDirect
  * This class offers a method to read a {@code Feature} using a {@code Reader} instance.
  */
 public class FeatureJSONReader extends JSONReaderBase {
+    public enum Phase { RESOLVE, LAUNCH }
+
     // The pattern that variables in Feature JSON follow
     private static final Pattern VARIABLE_PATTERN = Pattern.compile("\\$\\{[a-zA-Z0-9.-_]+\\}");
 
@@ -58,9 +60,9 @@ public class FeatureJSONReader extends JSONReaderBase {
      * @return The read feature
      * @throws IOException If an IO errors occurs or the JSON is invalid.
      */
-    public static Feature read(final Reader reader, final String location)
+    public static Feature read(final Reader reader, final String location, final Phase phase)
     throws IOException {
-        return read(reader, null, location);
+        return read(reader, null, location, phase);
     }
 
     /**
@@ -75,10 +77,11 @@ public class FeatureJSONReader extends JSONReaderBase {
      */
     public static Feature read(final Reader reader,
             final ArtifactId providedId,
-            final String location)
+            final String location,
+            final Phase phase)
     throws IOException {
         try {
-            final FeatureJSONReader mr = new FeatureJSONReader(providedId, location);
+            final FeatureJSONReader mr = new FeatureJSONReader(providedId, location, phase);
             return mr.readFeature(reader);
         } catch (final IllegalStateException | IllegalArgumentException e) {
             throw new IOException(e);
@@ -94,14 +97,17 @@ public class FeatureJSONReader extends JSONReaderBase {
     /** The variables from the JSON. */
     private Map<String, String> variables;
 
+    private final Phase phase;
+
     /**
      * Private constructor
      * @param pId Optional id
      * @param location Optional location
      */
-    FeatureJSONReader(final ArtifactId pId, final String location) {
+    FeatureJSONReader(final ArtifactId pId, final String location, final Phase phase) {
         super(location);
         this.providedId = pId;
+        this.phase = phase;
     }
 
     /**
@@ -154,8 +160,26 @@ public class FeatureJSONReader extends JSONReaderBase {
         return feature;
     }
 
+
+
+    @Override
+    protected Object handleResolveVars(Object val) {
+        if (phase == Phase.RESOLVE) {
+            return handleVars(val);
+        } else {
+            return val;
+        }
+    }
+
     @Override
-    protected Object handleVars(Object value) {
+    protected Object handleLaunchVars(Object val) {
+        if (phase == Phase.LAUNCH) {
+            return handleVars(val);
+        }
+        return val;
+    }
+
+    private Object handleVars(Object value) {
         if (!(value instanceof String)) {
             return value;
         }
@@ -221,7 +245,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(handleVars(obj.get(JSONConstants.ARTIFACT_ID)).toString());
+                    final ArtifactId id = ArtifactId.parse(handleResolveVars(obj.get(JSONConstants.ARTIFACT_ID)).toString());
                     include = new Include(id);
 
                     if ( obj.containsKey(JSONConstants.INCLUDE_REMOVALS) ) {
@@ -319,7 +343,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) -> unmarshalAttribute(key, handleVars(value), attrMap::put)));
+                    attrs.forEach(rethrowBiConsumer((key, value) -> unmarshalAttribute(key, handleResolveVars(value), attrMap::put)));
                 }
 
                 Map<String, String> dirMap = new HashMap<>();
@@ -327,10 +351,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) -> unmarshalDirective(key, handleVars(value), dirMap::put)));
+                    dirs.forEach(rethrowBiConsumer((key, value) -> unmarshalDirective(key, handleResolveVars(value), dirMap::put)));
                 }
 
-                final Requirement r = new OSGiRequirement(handleVars(obj.get(JSONConstants.REQCAP_NAMESPACE)).toString(), attrMap, dirMap);
+                final Requirement r = new OSGiRequirement(handleResolveVars(obj.get(JSONConstants.REQCAP_NAMESPACE)).toString(), attrMap, dirMap);
                 feature.getRequirements().add(r);
             }
         }
@@ -358,7 +382,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) -> unmarshalAttribute(key, handleVars(value), attrMap::put)));
+                    attrs.forEach(rethrowBiConsumer((key, value) -> unmarshalAttribute(key, handleResolveVars(value), attrMap::put)));
                 }
 
                 Map<String, String> dirMap = new HashMap<>();
@@ -366,10 +390,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) -> unmarshalDirective(key, handleVars(value), dirMap::put)));
+                    dirs.forEach(rethrowBiConsumer((key, value) -> unmarshalDirective(key, handleResolveVars(value), dirMap::put)));
                 }
 
-                final Capability c = new OSGiCapability(handleVars(obj.get(JSONConstants.REQCAP_NAMESPACE)).toString(), attrMap, dirMap);
+                final Capability c = new OSGiCapability(handleResolveVars(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/JSONReaderBase.java b/featuremodel/feature-support/src/main/java/org/apache/sling/feature/support/json/JSONReaderBase.java
index 3f02731..a014ff1 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
@@ -138,7 +138,7 @@ abstract class JSONReaderBase {
             final Artifact artifact;
             checkType(artifactType, entry, Map.class, String.class);
             if ( entry instanceof String ) {
-                artifact = new Artifact(ArtifactId.parse(handleVars(entry).toString()));
+                artifact = new Artifact(ArtifactId.parse(handleResolveVars(entry).toString()));
             } else {
                 @SuppressWarnings("unchecked")
                 final Map<String, Object> bundleObj = (Map<String, Object>) entry;
@@ -146,7 +146,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(handleVars(bundleObj.get(JSONConstants.ARTIFACT_ID)).toString());
+                final ArtifactId id = ArtifactId.parse(handleResolveVars(bundleObj.get(JSONConstants.ARTIFACT_ID)).toString());
 
                 artifact = new Artifact(id);
                 for(final Map.Entry<String, Object> metadataEntry : bundleObj.entrySet()) {
@@ -167,7 +167,23 @@ abstract class JSONReaderBase {
         }
     }
 
-    protected Object handleVars(Object val) {
+    /** 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 substitiuted.
+     */
+    protected Object handleResolveVars(Object val) {
+        // No variable substitution at this level, but subclasses can add this in
+        return val;
+    }
+
+    /** Substitutes variables that need to be substituted at launch time.
+     * These are all variables that are not needed by the resolver.
+     * @param val The value that may contain a variable.
+     * @return The value with the variable substitiuted.
+     */
+    protected Object handleLaunchVars(Object val) {
         // No variable substitution at this level, but subclasses can add this in
         return val;
     }
@@ -209,7 +225,7 @@ abstract class JSONReaderBase {
                     throw new IOException(exceptionPrefix + "Configuration must not define configurator property " + key);
                 }
                 final Object val = c.getProperties().get(key);
-                config.getProperties().put(key, handleVars(val));
+                config.getProperties().put(key, handleLaunchVars(val));
             }
             if ( config.getProperties().get(Configuration.PROP_ARTIFACT) != null ) {
                 throw new IOException(exceptionPrefix + "Configuration must not define property " + Configuration.PROP_ARTIFACT);
@@ -250,7 +266,7 @@ abstract class JSONReaderBase {
                 if ( container.get(entry.getKey()) != null ) {
                     throw new IOException(this.exceptionPrefix + "Duplicate framework property " + entry.getKey());
                 }
-                container.put(entry.getKey(), handleVars(entry.getValue()).toString());
+                container.put(entry.getKey(), handleLaunchVars(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 d82e906..e464f48 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
@@ -25,6 +25,7 @@ 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.apache.sling.feature.support.json.FeatureJSONReader.Phase;
 import org.junit.Test;
 import org.osgi.resource.Capability;
 import org.osgi.resource.Requirement;
@@ -69,7 +70,7 @@ public class FeatureJSONReaderTest {
 
     }
 
-    @Test public void testReadWithVariables() throws Exception {
+    @Test public void testReadWithVariablesResolve() throws Exception {
         final Feature feature = U.readFeature("test2");
 
         List<Include> includes = feature.getIncludes();
@@ -100,7 +101,8 @@ public class FeatureJSONReaderTest {
         //        cap.getDirectives().get("uses"));
 
         KeyValueMap fwProps = feature.getFrameworkProperties();
-        assertEquals("something", fwProps.get("brave"));
+        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);
@@ -111,9 +113,52 @@ public class FeatureJSONReaderTest {
         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 testReadWithVariablesLaunch() throws Exception {
+        final Feature feature = U.readFeature("test3", Phase.LAUNCH);
+
+        List<Include> includes = feature.getIncludes();
+        assertEquals(1, includes.size());
+        Include include = includes.get(0);
+        assertEquals("Include substitution should not happen at launch time",
+                "${sling.gid}:sling:10", include.getId().toMvnId());
+
+        List<Requirement> reqs = feature.getRequirements();
+        Requirement req = reqs.get(0);
+        assertEquals("Requirement substitution should not happen at launch time",
+                "osgi.${ns}", req.getNamespace());
+        assertEquals("Requirement substitution should not happen at launch time",
+                "(&(osgi.contract=${contract.name})(&(version>=3.0)(!(version>=4.0))))",
+                req.getDirectives().get("filter"));
+
+        List<Capability> caps = feature.getCapabilities();
+        Capability cap = null;
+        for (Capability c : caps) {
+            if ("osgi.${svc}".equals(c.getNamespace())) {
+                cap = c;
+                break;
+            }
+        }
+        assertEquals("Capability substitution should not happen at launch time",
+                Collections.singletonList("org.osgi.${svc}.http.runtime.HttpServiceRuntime"),
+                cap.getAttributes().get("objectClass"));
+
+        KeyValueMap fwProps = feature.getFrameworkProperties();
+        assertEquals("something", fwProps.get("brave"));
+
+        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"));
+        assertEquals("The variable definition looks like a variable definition, this escaping mechanism should work",
+                "${refvar}", props.get("refvar"));
     }
 
     @Test public void testReadRepoInitExtension() throws Exception {
@@ -133,18 +178,18 @@ public class FeatureJSONReaderTest {
     }
 
     @Test public void testHandleVars() throws Exception {
-        FeatureJSONReader reader = new FeatureJSONReader(null, null);
+        FeatureJSONReader reader = new FeatureJSONReader(null, null, Phase.LAUNCH);
         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}"));
+        assertEquals("foobarfoo", reader.handleLaunchVars("foo${var1}foo"));
+        assertEquals("barbarbar", reader.handleLaunchVars("${var1}${var1}${var1}"));
+        assertEquals("${}test${myvar}2", reader.handleLaunchVars("${}test${varvariable}${var.2}"));
         try {
-            reader.handleVars("${undefined}");
+            reader.handleLaunchVars("${undefined}");
             fail("Should throw an exception on the undefined variable");
         } catch (IllegalStateException ise) {
             // good
diff --git a/featuremodel/feature-support/src/test/java/org/apache/sling/feature/support/json/FeatureJSONWriterTest.java b/featuremodel/feature-support/src/test/java/org/apache/sling/feature/support/json/FeatureJSONWriterTest.java
index d1b4c72..a373558 100644
--- a/featuremodel/feature-support/src/test/java/org/apache/sling/feature/support/json/FeatureJSONWriterTest.java
+++ b/featuremodel/feature-support/src/test/java/org/apache/sling/feature/support/json/FeatureJSONWriterTest.java
@@ -17,6 +17,7 @@
 package org.apache.sling.feature.support.json;
 
 import org.apache.sling.feature.Feature;
+import org.apache.sling.feature.support.json.FeatureJSONReader.Phase;
 import org.junit.Test;
 
 import java.io.StringReader;
@@ -33,7 +34,7 @@ public class FeatureJSONWriterTest {
         try ( final StringWriter writer = new StringWriter() ) {
             FeatureJSONWriter.write(writer, f);
             try ( final StringReader reader = new StringReader(writer.toString()) ) {
-                rf = FeatureJSONReader.read(reader, null);
+                rf = FeatureJSONReader.read(reader, null, Phase.RESOLVE);
             }
         }
         assertEquals(f.getId(), rf.getId());
diff --git a/featuremodel/feature-support/src/test/java/org/apache/sling/feature/support/json/U.java b/featuremodel/feature-support/src/test/java/org/apache/sling/feature/support/json/U.java
index 6122b01..eee0e8e 100644
--- a/featuremodel/feature-support/src/test/java/org/apache/sling/feature/support/json/U.java
+++ b/featuremodel/feature-support/src/test/java/org/apache/sling/feature/support/json/U.java
@@ -18,6 +18,7 @@ package org.apache.sling.feature.support.json;
 
 import org.apache.sling.feature.Configuration;
 import org.apache.sling.feature.Feature;
+import org.apache.sling.feature.support.json.FeatureJSONReader.Phase;
 import org.osgi.resource.Capability;
 import org.osgi.resource.Requirement;
 
@@ -30,11 +31,16 @@ import static org.junit.Assert.fail;
 /** Test utilities */
 public class U {
 
-    /** Read the feature from the provided resource */
+    /** Read the feature from the provided resource
+     */
     public static Feature readFeature(final String name) throws Exception {
+        return readFeature(name, Phase.RESOLVE);
+    }
+
+    public static Feature readFeature(final String name, final Phase phase) throws Exception {
         try ( final Reader reader = new InputStreamReader(U.class.getResourceAsStream("/features/" + name + ".json"),
                 "UTF-8") ) {
-            return FeatureJSONReader.read(reader, name);
+            return FeatureJSONReader.read(reader, name, phase);
         }
     }
 
diff --git a/featuremodel/feature-support/src/test/resources/features/test3.json b/featuremodel/feature-support/src/test/resources/features/test3.json
new file mode 100644
index 0000000..eb44d5c
--- /dev/null
+++ b/featuremodel/feature-support/src/test/resources/features/test3.json
@@ -0,0 +1,103 @@
+{
+    "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" : [
+         {
+             "id" : "${sling.gid}/sling/10",
+             "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
+            }
+    ],
+    "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",
+           "refvar": "${refvar}"
+        }
+    }
+}
\ No newline at end of file
diff --git a/featuremodel/osgifeature-maven-plugin/src/main/java/org/apache/sling/feature/maven/Preprocessor.java b/featuremodel/osgifeature-maven-plugin/src/main/java/org/apache/sling/feature/maven/Preprocessor.java
index 1435e4b..3799712 100644
--- a/featuremodel/osgifeature-maven-plugin/src/main/java/org/apache/sling/feature/maven/Preprocessor.java
+++ b/featuremodel/osgifeature-maven-plugin/src/main/java/org/apache/sling/feature/maven/Preprocessor.java
@@ -16,15 +16,6 @@
  */
 package org.apache.sling.feature.maven;
 
-import java.io.File;
-import java.io.FileReader;
-import java.io.IOException;
-import java.io.Reader;
-import java.io.StringReader;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.stream.Collectors;
-
 import org.apache.maven.model.Dependency;
 import org.apache.maven.project.MavenProject;
 import org.apache.sling.feature.Artifact;
@@ -37,8 +28,18 @@ import org.apache.sling.feature.process.FeatureBuilder;
 import org.apache.sling.feature.process.FeatureProvider;
 import org.apache.sling.feature.support.FeatureUtil;
 import org.apache.sling.feature.support.json.FeatureJSONReader;
+import org.apache.sling.feature.support.json.FeatureJSONReader.Phase;
 import org.codehaus.plexus.logging.Logger;
 
+import java.io.File;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.Reader;
+import java.io.StringReader;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
+
 /**
  * The processor processes all feature projects.
  */
@@ -193,7 +194,7 @@ public class Preprocessor {
 
                 // We should pass in an "id" to FeatureJSONReader.read and later on check the id (again, need to handle ref files)
                 try (final FileReader reader = new FileReader(file)) {
-                    final Feature feature = FeatureJSONReader.read(reader, id, file.getAbsolutePath());
+                    final Feature feature = FeatureJSONReader.read(reader, id, file.getAbsolutePath(), Phase.RESOLVE);
 
                     this.checkFeatureId(id, feature);
 
@@ -368,7 +369,7 @@ public class Preprocessor {
         if ( config.getInlinedFeature() != null ) {
             logger.debug("Reading inlined model from project " + project.getId());
             try (final Reader reader = new StringReader(config.getInlinedFeature())) {
-                feature = FeatureJSONReader.read(reader, id, null);
+                feature = FeatureJSONReader.read(reader, id, null, Phase.RESOLVE);
             } catch ( final IOException io) {
                 throw new RuntimeException("Unable to read inlined feature", io);
             }
@@ -379,7 +380,7 @@ public class Preprocessor {
             }
             logger.debug("Reading feature " + featureFile + " in project " + project.getId());
             try (final FileReader reader = new FileReader(featureFile)) {
-                feature = FeatureJSONReader.read(reader, id, featureFile.getAbsolutePath());
+                feature = FeatureJSONReader.read(reader, id, featureFile.getAbsolutePath(), Phase.RESOLVE);
             } catch ( final IOException io) {
                 throw new RuntimeException("Unable to read feature " + featureFile, io);
             }
@@ -502,7 +503,7 @@ public class Preprocessor {
                     // "external" dependency, we can already resolve it
                     final File featureFile = ProjectHelper.getOrResolveArtifact(info.project, env.session, env.artifactHandlerManager, env.resolver, id).getFile();
                     try (final FileReader r = new FileReader(featureFile)) {
-                        return FeatureJSONReader.read(r, featureFile.getAbsolutePath());
+                        return FeatureJSONReader.read(r, featureFile.getAbsolutePath(), Phase.RESOLVE);
                     } catch ( final IOException ioe) {
                         env.logger.error("Unable to read feature file from " + featureFile, ioe);
                     }
diff --git a/featuremodel/osgifeature-maven-plugin/src/main/java/org/apache/sling/feature/maven/ProjectHelper.java b/featuremodel/osgifeature-maven-plugin/src/main/java/org/apache/sling/feature/maven/ProjectHelper.java
index a314113..2efb69c 100644
--- a/featuremodel/osgifeature-maven-plugin/src/main/java/org/apache/sling/feature/maven/ProjectHelper.java
+++ b/featuremodel/osgifeature-maven-plugin/src/main/java/org/apache/sling/feature/maven/ProjectHelper.java
@@ -31,6 +31,7 @@ import org.apache.maven.project.MavenProject;
 import org.apache.sling.feature.ArtifactId;
 import org.apache.sling.feature.Feature;
 import org.apache.sling.feature.support.json.FeatureJSONReader;
+import org.apache.sling.feature.support.json.FeatureJSONReader.Phase;
 import org.apache.sling.feature.support.json.FeatureJSONWriter;
 import org.codehaus.plexus.util.xml.Xpp3Dom;
 
@@ -95,7 +96,7 @@ public abstract class ProjectHelper {
                 result = null;
             } else {
                 try ( final StringReader r = new StringReader(text) ) {
-                    result = FeatureJSONReader.read(r, project.getId());
+                    result = FeatureJSONReader.read(r, project.getId(), Phase.RESOLVE);
                     project.setContextValue(cacheKey, result);
                 } catch ( final IOException ioe) {
                     throw new RuntimeException(ioe.getMessage(), ioe);
@@ -124,7 +125,7 @@ public abstract class ProjectHelper {
                         throw new RuntimeException("Unable to get feature from internal store.");
                     }
                     try ( final StringReader r = new StringReader(text) ) {
-                        final Feature feature = FeatureJSONReader.read(r, project.getId());
+                        final Feature feature = FeatureJSONReader.read(r, project.getId(), Phase.RESOLVE);
                         result.add(feature);
                     } catch ( final IOException ioe) {
                         throw new RuntimeException(ioe.getMessage(), ioe);

-- 
To stop receiving notification emails like this one, please contact
davidb@apache.org.