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/07/06 10:29:58 UTC

[sling-org-apache-sling-feature-io] branch master updated: SLING-7765 Application creation missing variable declarations

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-org-apache-sling-feature-io.git


The following commit(s) were added to refs/heads/master by this push:
     new 6583073  SLING-7765 Application creation missing variable declarations
6583073 is described below

commit 6583073070781bda4c338a50ffe4e4bb9ea5c972
Author: David Bosschaert <bo...@adobe.com>
AuthorDate: Fri Jul 6 11:29:18 2018 +0100

    SLING-7765 Application creation missing variable declarations
    
    Generalize the variable processing for both Feature and Application and
    apply to application reading and writing too.
---
 pom.xml                                            |  2 +
 .../feature/io/json/ApplicationJSONReader.java     | 19 ++++--
 .../feature/io/json/ApplicationJSONWriter.java     |  3 +
 .../sling/feature/io/json/FeatureJSONReader.java   | 62 +-----------------
 .../sling/feature/io/json/JSONConstants.java       |  1 +
 .../sling/feature/io/json/JSONReaderBase.java      | 73 ++++++++++++++++++++++
 .../feature/io/json/FeatureJSONReaderTest.java     | 15 ++---
 7 files changed, 104 insertions(+), 71 deletions(-)

diff --git a/pom.xml b/pom.xml
index f2d1a8b..2454d05 100644
--- a/pom.xml
+++ b/pom.xml
@@ -53,6 +53,8 @@
                             org.apache.felix.configurator.impl.model                            
                         </Conditional-Package>
                     </instructions>
+                    <!--  Skip baselining for 0.x version -->
+                    <skip>true</skip>
                 </configuration>
             </plugin>
             <plugin>
diff --git a/src/main/java/org/apache/sling/feature/io/json/ApplicationJSONReader.java b/src/main/java/org/apache/sling/feature/io/json/ApplicationJSONReader.java
index 88d3bc5..91b4048 100644
--- a/src/main/java/org/apache/sling/feature/io/json/ApplicationJSONReader.java
+++ b/src/main/java/org/apache/sling/feature/io/json/ApplicationJSONReader.java
@@ -16,6 +16,10 @@
  */
 package org.apache.sling.feature.io.json;
 
+import org.apache.felix.configurator.impl.json.JSONUtil;
+import org.apache.sling.feature.Application;
+import org.apache.sling.feature.ArtifactId;
+
 import java.io.IOException;
 import java.io.Reader;
 import java.io.StringReader;
@@ -24,10 +28,6 @@ import java.util.Map;
 import javax.json.Json;
 import javax.json.JsonObject;
 
-import org.apache.felix.configurator.impl.json.JSONUtil;
-import org.apache.sling.feature.Application;
-import org.apache.sling.feature.ArtifactId;
-
 /**
  * This class offers a method to read an {@code Application} using a {@code Reader} instance.
  */
@@ -79,6 +79,7 @@ public class ApplicationJSONReader extends JSONReaderBase {
         if ( frameworkId != null ) {
             app.setFramework(ArtifactId.parse(frameworkId));
         }
+        this.readVariables(map, app.getVariables());
         this.readBundles(map, app.getBundles(), app.getConfigurations());
         this.readFrameworkProperties(map, app.getFrameworkProperties());
         this.readConfigurations(map, app.getConfigurations());
@@ -87,6 +88,16 @@ public class ApplicationJSONReader extends JSONReaderBase {
                 JSONConstants.APP_KNOWN_PROPERTIES,
                 this.app.getExtensions(), this.app.getConfigurations());
     }
+
+    @Override
+    protected Object handleResolveVars(Object val) {
+        return handleVars(val, app.getVariables());
+    }
+
+    @Override
+    protected Object handleLaunchVars(Object val) {
+        return handleVars(val, app.getVariables());
+    }
 }
 
 
diff --git a/src/main/java/org/apache/sling/feature/io/json/ApplicationJSONWriter.java b/src/main/java/org/apache/sling/feature/io/json/ApplicationJSONWriter.java
index bdf4902..89a8366 100644
--- a/src/main/java/org/apache/sling/feature/io/json/ApplicationJSONWriter.java
+++ b/src/main/java/org/apache/sling/feature/io/json/ApplicationJSONWriter.java
@@ -59,6 +59,9 @@ public class ApplicationJSONWriter extends JSONWriterBase {
             ob.add(JSONConstants.APP_FRAMEWORK, app.getFramework().toMvnId());
         }
 
+        // variables
+        writeVariables(ob, app.getVariables());
+
         // features
         if ( !app.getFeatureIds().isEmpty() ) {
             JsonArrayBuilder featuresArr = Json.createArrayBuilder();
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 035089e..bff858a 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
@@ -21,7 +21,6 @@ import org.apache.felix.utils.resource.RequirementImpl;
 import org.apache.sling.feature.ArtifactId;
 import org.apache.sling.feature.Feature;
 import org.apache.sling.feature.Include;
-import org.apache.sling.feature.KeyValueMap;
 import org.osgi.resource.Capability;
 import org.osgi.resource.Requirement;
 
@@ -33,8 +32,6 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.function.BiConsumer;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 import javax.json.Json;
 import javax.json.JsonObject;
@@ -45,9 +42,6 @@ import javax.json.JsonObject;
 public class FeatureJSONReader extends JSONReaderBase {
     public enum SubstituteVariables { NONE, RESOLVE, LAUNCH }
 
-    // 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
      * The reader is not closed. It is up to the caller to close the reader.
@@ -93,9 +87,6 @@ public class FeatureJSONReader extends JSONReaderBase {
     /** The provided id. */
     private final ArtifactId providedId;
 
-    /** The variables from the JSON. */
-    private Map<String, String> variables;
-
     /** The current reading phase. */
     private final SubstituteVariables phase;
 
@@ -172,7 +163,7 @@ public class FeatureJSONReader extends JSONReaderBase {
     @Override
     protected Object handleResolveVars(Object val) {
         if (phase == SubstituteVariables.RESOLVE) {
-            return handleVars(val);
+            return handleVars(val, feature.getVariables());
         } else {
             return val;
         }
@@ -181,60 +172,11 @@ public class FeatureJSONReader extends JSONReaderBase {
     @Override
     protected Object handleLaunchVars(Object val) {
         if (phase == SubstituteVariables.LAUNCH) {
-            return handleVars(val);
+            return handleVars(val, feature.getVariables());
         }
         return val;
     }
 
-    private 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 void readVariables(Map<String, Object> map, KeyValueMap kvMap) throws IOException {
-        variables = 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);
-
-                String key = entry.getKey();
-                if (kvMap.get(key) != null) {
-                    throw new IOException(this.exceptionPrefix + "Duplicate variable " + key);
-                }
-                String value = "" + entry.getValue();
-                kvMap.put(key, value);
-                variables.put(key, value);
-            }
-        }
-    }
-
     private void readIncludes(final Map<String, Object> map) throws IOException {
         if ( map.containsKey(JSONConstants.FEATURE_INCLUDES)) {
             final Object includesObj = map.get(JSONConstants.FEATURE_INCLUDES);
diff --git a/src/main/java/org/apache/sling/feature/io/json/JSONConstants.java b/src/main/java/org/apache/sling/feature/io/json/JSONConstants.java
index 3b6f707..5fd9a5a 100644
--- a/src/main/java/org/apache/sling/feature/io/json/JSONConstants.java
+++ b/src/main/java/org/apache/sling/feature/io/json/JSONConstants.java
@@ -81,6 +81,7 @@ public abstract class JSONConstants {
     public static final String APP_FEATURES = "features";
 
     public static final List<String> APP_KNOWN_PROPERTIES = Arrays.asList(APP_FRAMEWORK,
+            FEATURE_VARIABLES,
             FEATURE_BUNDLES,
             FEATURE_FRAMEWORK_PROPERTIES,
             FEATURE_CONFIGURATIONS,
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 cefad18..35738bc 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
@@ -38,12 +38,15 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Enumeration;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import javax.json.Json;
 import javax.json.JsonArrayBuilder;
@@ -56,6 +59,8 @@ import javax.json.JsonWriter;
  * Common methods for JSON reading.
  */
 abstract class JSONReaderBase {
+    // The pattern that variables in Feature/Application JSON follow
+    private static final Pattern VARIABLE_PATTERN = Pattern.compile("\\$\\{[a-zA-Z0-9.-_]+\\}");
 
     /** The optional location. */
     protected final String location;
@@ -136,6 +141,38 @@ abstract class JSONReaderBase {
     }
 
     /**
+     * Read the variables section
+     * @param map The map describing the feature or application
+     * @param kvMap The variables will be written to this Key Value Map
+     * @return The same variables as a normal map
+     * @throws IOException If the json is invalid.
+     */
+    protected Map<String, String> readVariables(Map<String, Object> map, KeyValueMap kvMap) throws IOException {
+        HashMap<String, String> variables = 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);
+
+                String key = entry.getKey();
+                if (kvMap.get(key) != null) {
+                    throw new IOException(this.exceptionPrefix + "Duplicate variable " + key);
+                }
+                String value = "" + entry.getValue();
+                kvMap.put(key, value);
+                variables.put(key, value);
+            }
+        }
+        return variables;
+    }
+
+
+    /**
      * Read the bundles / start levels section
      * @param map The map describing the feature
      * @param container The bundles container
@@ -476,6 +513,42 @@ abstract class JSONReaderBase {
             throw new IOException(this.exceptionPrefix + "Key " + key + " is not one of the allowed types " + Arrays.toString(types) + " : " + val.getClass());
         }
     }
+
+    /**
+     * Substitute variables in the provided value. The variables must follow the
+     * syntax ${variable_name} and are looked up in the provided variables.
+     * If the provided value contains no variables, it will be returned as-is.
+     * @param value The value that can contain variables
+     * @param variables The variables that can be substituted.
+     * @return The value with the variables substituted.
+     * @throws IllegalStateException when a variable in the value is not present
+     * in the variables map.
+     */
+    protected Object handleVars(Object value, KeyValueMap variables) {
+        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();
+    }
 }
 
 
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 aa7559c..6ee87e3 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
@@ -35,9 +35,7 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.Dictionary;
 import java.util.Enumeration;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -196,11 +194,14 @@ public class FeatureJSONReaderTest {
 
     @Test public void testHandleVars() throws Exception {
         FeatureJSONReader reader = new FeatureJSONReader(null, null, SubstituteVariables.LAUNCH);
-        Map<String, Object> vars = new HashMap<>();
-        vars.put("var1", "bar");
-        vars.put("varvariable", "${myvar}");
-        vars.put("var.2", "2");
-        setPrivateField(reader, "variables", vars);
+
+        ArtifactId aid = new ArtifactId("gid", "aid", "1.2.3", null, null);
+        Feature feature = new Feature(aid);
+        KeyValueMap kvMap = feature.getVariables();
+        kvMap.put("var1", "bar");
+        kvMap.put("varvariable", "${myvar}");
+        kvMap.put("var.2", "2");
+        setPrivateField(reader, "feature", feature);
 
         assertEquals("foobarfoo", reader.handleLaunchVars("foo${var1}foo"));
         assertEquals("barbarbar", reader.handleLaunchVars("${var1}${var1}${var1}"));