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/11/14 15:15:17 UTC

[sling-org-apache-sling-feature] branch master updated: SLING-8109 Replace KeyValueMap with Map

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


The following commit(s) were added to refs/heads/master by this push:
     new 8da6f4c  SLING-8109 Replace KeyValueMap with Map<String,String>
8da6f4c is described below

commit 8da6f4ce8067451cb4fdaf0e1979bc9d3d630325
Author: David Bosschaert <bo...@adobe.com>
AuthorDate: Wed Nov 14 15:14:49 2018 +0000

    SLING-8109 Replace KeyValueMap with Map<String,String>
---
 .../java/org/apache/sling/feature/Artifact.java    |   9 +-
 .../java/org/apache/sling/feature/Feature.java     |  18 +--
 .../java/org/apache/sling/feature/KeyValueMap.java | 156 ---------------------
 .../sling/feature/builder/BuilderContext.java      |  22 ++-
 .../apache/sling/feature/builder/BuilderUtil.java  |  58 ++++----
 .../sling/feature/builder/FeatureBuilder.java      |  33 ++---
 .../sling/feature/builder/HandlerContext.java      |   6 +-
 .../sling/feature/builder/BuilderUtilTest.java     |  34 ++---
 .../sling/feature/builder/FeatureBuilderTest.java  |  47 +++----
 9 files changed, 117 insertions(+), 266 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/Artifact.java b/src/main/java/org/apache/sling/feature/Artifact.java
index 504f7fe..9ff92f9 100644
--- a/src/main/java/org/apache/sling/feature/Artifact.java
+++ b/src/main/java/org/apache/sling/feature/Artifact.java
@@ -16,6 +16,9 @@
  */
 package org.apache.sling.feature;
 
+import java.util.HashMap;
+import java.util.Map;
+
 /**
  * An artifact consists of
  * <ul>
@@ -35,7 +38,7 @@ public class Artifact implements Comparable<Artifact> {
     private final ArtifactId id;
 
     /** Artifact metadata. */
-    private final KeyValueMap metadata = new KeyValueMap();
+    private final Map<String,String> metadata = new HashMap<>();
 
     /**
      * Construct a new artifact
@@ -62,7 +65,7 @@ public class Artifact implements Comparable<Artifact> {
      * The metadata can be modified.
      * @return The metadata.
      */
-    public KeyValueMap getMetadata() {
+    public Map<String,String> getMetadata() {
         return this.metadata;
     }
 
@@ -130,7 +133,7 @@ public class Artifact implements Comparable<Artifact> {
 
     /**
      * Create a copy of the artifact with a different id
-     * 
+     *
      * @param id The new id
      * @return The copy of the feature with the new id
      */
diff --git a/src/main/java/org/apache/sling/feature/Feature.java b/src/main/java/org/apache/sling/feature/Feature.java
index b761260..9b0b9c6 100644
--- a/src/main/java/org/apache/sling/feature/Feature.java
+++ b/src/main/java/org/apache/sling/feature/Feature.java
@@ -16,15 +16,17 @@
  */
 package org.apache.sling.feature;
 
-import java.util.ArrayList;
-import java.util.Enumeration;
-import java.util.List;
-
 import org.apache.felix.utils.resource.CapabilityImpl;
 import org.apache.felix.utils.resource.RequirementImpl;
 import org.osgi.resource.Capability;
 import org.osgi.resource.Requirement;
 
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 /**
  * A feature consists of
  * <ul>
@@ -47,7 +49,7 @@ public class Feature implements Comparable<Feature> {
 
     private final Configurations configurations = new Configurations();
 
-    private final KeyValueMap frameworkProperties = new KeyValueMap();
+    private final Map<String,String> frameworkProperties = new HashMap<>();
 
     private final List<Requirement> requirements = new ArrayList<>();
 
@@ -55,7 +57,7 @@ public class Feature implements Comparable<Feature> {
 
     private final Extensions extensions = new Extensions();
 
-    private final KeyValueMap variables = new KeyValueMap();
+    private final Map<String,String> variables = new HashMap<>();
 
     /** The optional location. */
     private volatile String location;
@@ -144,7 +146,7 @@ public class Feature implements Comparable<Feature> {
      * The returned object is modifiable.
      * @return The framework properties
      */
-    public KeyValueMap getFrameworkProperties() {
+    public Map<String,String> getFrameworkProperties() {
         return this.frameworkProperties;
     }
 
@@ -227,7 +229,7 @@ public class Feature implements Comparable<Feature> {
      * Obtain the variables of the feature
      * @return The variables
      */
-    public KeyValueMap getVariables() {
+    public Map<String,String> getVariables() {
         return this.variables;
     }
 
diff --git a/src/main/java/org/apache/sling/feature/KeyValueMap.java b/src/main/java/org/apache/sling/feature/KeyValueMap.java
deleted file mode 100644
index 60b49e5..0000000
--- a/src/main/java/org/apache/sling/feature/KeyValueMap.java
+++ /dev/null
@@ -1,156 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with this
- * work for additional information regarding copyright ownership. The ASF
- * licenses this file to You under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
- * License for the specific language governing permissions and limitations under
- * the License.
- */
-package org.apache.sling.feature;
-
-import java.util.Iterator;
-import java.util.Map;
-import java.util.Map.Entry;
-import java.util.TreeMap;
-
-/**
- * Helper class to hold key value pairs.
- *
- * This class is not thread-safe.
- */
-public class KeyValueMap
-    implements Iterable<Map.Entry<String, String>> {
-
-    /** The map holding the actual key value pairs. */
-    private final Map<String, String> properties = new TreeMap<>();
-
-    /**
-     * Get an item from the map.
-     * @param key The key of the item.
-     * @return The item or {@code null}.
-     */
-    public String get(final String key) {
-       return this.properties.get(key);
-    }
-
-    /**
-     * Get an item from the map or the default.
-     *
-     * @param key The key of the item.
-     * @param def The default value
-     * @return The item or the default.
-     */
-    public String getOrDefault(final String key, final String def) {
-        String result = this.properties.get(key);
-        if (result == null) {
-            result = def;
-        }
-        return result;
-    }
-
-    /**
-     * Put an item in the map
-     *
-     * @param key   The key of the item.
-     * @param value The value
-     */
-    public void put(final String key, final String value) {
-        this.properties.put(key, value);
-    }
-
-    /**
-     * Remove an item from the map
-     * @param key The key of the item.
-     * @return The previously stored value for the key or {@code null}.
-     */
-    public String remove(final String key) {
-        return this.properties.remove(key);
-    }
-
-    /**
-     * Put all items from the other map in this map
-     * @param map The other map
-     */
-    public void putAll(final KeyValueMap map) {
-        if (map != null) {
-            this.properties.putAll(map.properties);
-        }
-    }
-
-    /**
-     * Put all items from the other map in this map
-     * 
-     * @param map The other map
-     */
-    public void putAll(final Map<String, String> map) {
-        if (map != null) {
-            this.properties.putAll(map);
-        }
-    }
-
-    @Override
-    public Iterator<Entry<String, String>> iterator() {
-        return this.properties.entrySet().iterator();
-    }
-
-    /**
-     * Check whether this map is empty.
-     * @return {@code true} if the map is empty.
-     */
-    public boolean isEmpty() {
-        return this.properties.isEmpty();
-    }
-
-    @Override
-    public String toString() {
-        return properties.toString();
-    }
-
-    /**
-     * Get the size of the map.
-     * @return The size of the map.
-     */
-    public int size() {
-        return this.properties.size();
-    }
-
-    /**
-     * Clear the map
-     */
-    public void clear() {
-        this.properties.clear();
-    }
-
-    @Override
-    public int hashCode() {
-        final int prime = 31;
-        int result = 1;
-        result = prime * result + ((properties == null) ? 0 : properties.hashCode());
-        return result;
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-        if (this == obj)
-            return true;
-        if (obj == null)
-            return false;
-        if (getClass() != obj.getClass())
-            return false;
-        KeyValueMap other = (KeyValueMap) obj;
-        if (properties == null) {
-            if (other.properties != null)
-                return false;
-        } else if (!properties.equals(other.properties))
-            return false;
-        return true;
-    }
-}
diff --git a/src/main/java/org/apache/sling/feature/builder/BuilderContext.java b/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
index d10f81e..1dd100c 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderContext.java
@@ -22,8 +22,6 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import org.apache.sling.feature.KeyValueMap;
-
 /**
  * Builder context holds services used by {@link FeatureBuilder}.
  *
@@ -41,11 +39,11 @@ public class BuilderContext {
     /** The optional artifact provider. */
     private ArtifactProvider artifactProvider;
 
-    private final Map<String, KeyValueMap> extensionConfiguration = new HashMap<>();
+    private final Map<String, Map<String,String>> extensionConfiguration = new HashMap<>();
     private final List<MergeHandler> mergeExtensions = new ArrayList<>();
     private final List<PostProcessHandler> postProcessExtensions = new ArrayList<>();
-    private final KeyValueMap variables = new KeyValueMap();
-    private final KeyValueMap frameworkProperties = new KeyValueMap();
+    private final Map<String,String> variables = new HashMap<>();
+    private final Map<String,String> frameworkProperties = new HashMap<>();
 
     private ArtifactMergeAlgorithm merge = ArtifactMergeAlgorithm.HIGHEST;
 
@@ -79,7 +77,7 @@ public class BuilderContext {
      * @param vars The overwrites
      * @return The builder context
      */
-    public BuilderContext addVariablesOverwrites(final KeyValueMap vars) {
+    public BuilderContext addVariablesOverwrites(final Map<String,String> vars) {
         this.variables.putAll(vars);
         return this;
     }
@@ -90,7 +88,7 @@ public class BuilderContext {
      * @param props The overwrites
      * @return The builder context
      */
-    public BuilderContext addFrameworkPropertiesOverwrites(final KeyValueMap props) {
+    public BuilderContext addFrameworkPropertiesOverwrites(final Map<String,String> props) {
         this.frameworkProperties.putAll(props);
         return this;
     }
@@ -135,18 +133,18 @@ public class BuilderContext {
      * @param The configuration for the handler
      * @return The builder context
      */
-    public BuilderContext setHandlerConfiguration(final String name, final KeyValueMap cfg) {
+    public BuilderContext setHandlerConfiguration(final String name, final Map<String,String> cfg) {
         this.extensionConfiguration.put(name, cfg);
         return this;
     }
 
     /**
      * Obtain the handler configuration.
-     * 
+     *
      * @return The current handler configuration object. The key is the handler name
      *         and the value is a map of configuration values.
      */
-    Map<String, KeyValueMap> getHandlerConfigurations() {
+    Map<String, Map<String,String>> getHandlerConfigurations() {
         return this.extensionConfiguration;
     }
 
@@ -154,11 +152,11 @@ public class BuilderContext {
         return this.artifactProvider;
     }
 
-    KeyValueMap getVariablesOverwrites() {
+    Map<String,String> getVariablesOverwrites() {
         return  this.variables;
     }
 
-    KeyValueMap getFrameworkPropertiesOverwrites() {
+    Map<String,String> getFrameworkPropertiesOverwrites() {
         return this.frameworkProperties;
     }
 
diff --git a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
index b007f4e..ad804d9 100644
--- a/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
+++ b/src/main/java/org/apache/sling/feature/builder/BuilderUtil.java
@@ -16,10 +16,22 @@
  */
 package org.apache.sling.feature.builder;
 
+import org.apache.sling.feature.Artifact;
+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.Feature;
+import org.apache.sling.feature.FeatureConstants;
+import org.apache.sling.feature.builder.BuilderContext.ArtifactMergeAlgorithm;
+import org.osgi.resource.Capability;
+import org.osgi.resource.Requirement;
+
 import java.io.StringReader;
 import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.Enumeration;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Stream;
@@ -34,18 +46,6 @@ import javax.json.JsonValue;
 import javax.json.JsonValue.ValueType;
 import javax.json.JsonWriter;
 
-import org.apache.sling.feature.Artifact;
-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.Feature;
-import org.apache.sling.feature.FeatureConstants;
-import org.apache.sling.feature.KeyValueMap;
-import org.apache.sling.feature.builder.BuilderContext.ArtifactMergeAlgorithm;
-import org.osgi.resource.Capability;
-import org.osgi.resource.Requirement;
-
 /**
  * Utility methods for the builders
  */
@@ -73,12 +73,12 @@ class BuilderUtil {
         return null;
     }
 
-    private static void mergeWithContextOverwrite(String type, KeyValueMap target, KeyValueMap source, Iterable<Map.Entry<String,String>> context) {
-        KeyValueMap result = new KeyValueMap();
-        for (Map.Entry<String, String> entry : target) {
+    private static void mergeWithContextOverwrite(String type, Map<String,String> target, Map<String,String> source, Iterable<Map.Entry<String,String>> context) {
+        Map<String,String> result = new HashMap<>();
+        for (Map.Entry<String, String> entry : target.entrySet()) {
             result.put(entry.getKey(), contains(entry.getKey(), context) ? get(entry.getKey(), context) : entry.getValue());
         }
-        for (Map.Entry<String, String> entry : source) {
+        for (Map.Entry<String, String> entry : source.entrySet()) {
             if (contains(entry.getKey(), context)) {
                 result.put(entry.getKey(), get(entry.getKey(), context));
             }
@@ -95,7 +95,7 @@ class BuilderUtil {
                         result.put(entry.getKey(), value);
                     }
                 }
-                else if (!contains(entry.getKey(), target)) {
+                else if (!contains(entry.getKey(), target.entrySet())) {
                     result.put(entry.getKey(), value);
                 }
             }
@@ -105,9 +105,9 @@ class BuilderUtil {
     }
 
     // variables
-    static void mergeVariables(KeyValueMap target, KeyValueMap source, BuilderContext context) {
+    static void mergeVariables(Map<String,String> target, Map<String,String> source, BuilderContext context) {
         mergeWithContextOverwrite("Variable", target, source,
-                (null != context) ? context.getVariablesOverwrites() : null);
+                (null != context) ? context.getVariablesOverwrites().entrySet() : null);
     }
 
     /**
@@ -171,9 +171,9 @@ class BuilderUtil {
     }
 
     // framework properties (add/merge)
-    static void mergeFrameworkProperties(final KeyValueMap target, final KeyValueMap source, BuilderContext context) {
+    static void mergeFrameworkProperties(final Map<String,String> target, final Map<String,String> source, BuilderContext context) {
         mergeWithContextOverwrite("Property", target, source,
-                context != null ? context.getFrameworkPropertiesOverwrites() : null);
+                context != null ? context.getFrameworkPropertiesOverwrites().entrySet() : null);
     }
 
     // requirements (add)
@@ -360,7 +360,7 @@ class BuilderUtil {
 
     private static class HandlerContextImpl implements HandlerContext {
         private final ArtifactProvider artifactProvider;
-        private final KeyValueMap configuration;
+        private final Map<String,String> configuration;
 
         public HandlerContextImpl(BuilderContext bc, MergeHandler handler) {
             artifactProvider = bc.getArtifactProvider();
@@ -372,13 +372,17 @@ class BuilderUtil {
             configuration = getHandlerConfiguration(bc, handler);
         }
 
-        private KeyValueMap getHandlerConfiguration(BuilderContext bc, Object handler) {
-            final KeyValueMap result = new KeyValueMap();
+        private Map<String,String> getHandlerConfiguration(BuilderContext bc, Object handler) {
+            final Map<String,String> result = new HashMap<>();
 
-            result.putAll(bc.getHandlerConfigurations().get("*"));
+            Map<String, String> overall = bc.getHandlerConfigurations().get("*");
+            if (overall != null)
+                result.putAll(overall);
             final String name = getHandlerName(handler);
             if (name != null) {
-                result.putAll(bc.getHandlerConfigurations().get(name));
+                Map<String, String> handlerSpecific = bc.getHandlerConfigurations().get(name);
+                if (handlerSpecific != null)
+                    result.putAll(handlerSpecific);
             }
             return result;
         }
@@ -393,7 +397,7 @@ class BuilderUtil {
         }
 
         @Override
-        public KeyValueMap getConfiguration() {
+        public Map<String,String> getConfiguration() {
             return configuration;
         }
     }
diff --git a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
index a7709fe..4668fa0 100644
--- a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
+++ b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
@@ -16,16 +16,6 @@
  */
 package org.apache.sling.feature.builder;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
 import org.apache.sling.feature.Artifact;
 import org.apache.sling.feature.ArtifactId;
 import org.apache.sling.feature.Configuration;
@@ -34,9 +24,18 @@ import org.apache.sling.feature.ExtensionType;
 import org.apache.sling.feature.Feature;
 import org.apache.sling.feature.FeatureConstants;
 import org.apache.sling.feature.Include;
-import org.apache.sling.feature.KeyValueMap;
 import org.osgi.framework.Version;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 public abstract class FeatureBuilder {
 
     /** Pattern for using variables. */
@@ -221,7 +220,7 @@ public abstract class FeatureBuilder {
      * @param feature The feature
      * @param additionalVariables Optional additional variables
      */
-    public static void resolveVariables(final Feature feature, final KeyValueMap additionalVariables) {
+    public static void resolveVariables(final Feature feature, final Map<String,String> additionalVariables) {
         for(final Configuration cfg : feature.getConfigurations()) {
         	final Set<String> keys = new HashSet<>(Collections.list(cfg.getProperties().keys()));
         	for(final String key : keys) {
@@ -229,7 +228,7 @@ public abstract class FeatureBuilder {
                 cfg.getProperties().put(key, replaceVariables(value, additionalVariables, feature));
             }
         }
-        for(final Map.Entry<String, String> entry : feature.getFrameworkProperties()) {
+        for(final Map.Entry<String, String> entry : feature.getFrameworkProperties().entrySet()) {
             // the  value is always a string
             entry.setValue((String)replaceVariables(entry.getValue(), additionalVariables, feature));
         }
@@ -247,7 +246,7 @@ public abstract class FeatureBuilder {
      * @param feature The feature containing variables
      * @return The value with the variables substituted.
      */
-    static Object replaceVariables(final Object value, final KeyValueMap additionalVariables, final Feature feature) {
+    static Object replaceVariables(final Object value, final Map<String,String> additionalVariables, final Feature feature) {
         if (!(value instanceof String)) {
             return value;
         }
@@ -261,8 +260,10 @@ public abstract class FeatureBuilder {
 
             final int len = var.length();
             final String name = var.substring(2, len - 1);
-            if (BuilderUtil.contains(name, feature.getVariables())) {
-                String val = BuilderUtil.get(name, additionalVariables);
+            if (BuilderUtil.contains(name, feature.getVariables().entrySet())) {
+                String val = null;
+                if (additionalVariables != null)
+                    val = BuilderUtil.get(name, additionalVariables.entrySet());
                 if (val == null) {
                     val = feature.getVariables().get(name);
                 }
diff --git a/src/main/java/org/apache/sling/feature/builder/HandlerContext.java b/src/main/java/org/apache/sling/feature/builder/HandlerContext.java
index c9637a6..0fd9f2c 100644
--- a/src/main/java/org/apache/sling/feature/builder/HandlerContext.java
+++ b/src/main/java/org/apache/sling/feature/builder/HandlerContext.java
@@ -1,6 +1,6 @@
 /*
 
- * Licensed to the Apache Software Foundation (ASF) under one or more
+Map<String,String> * Licensed to the Apache Software Foundation (ASF) under one or more
  * contributor license agreements.  See the NOTICE file distributed with
  * this work for additional information regarding copyright ownership.
  * The ASF licenses this file to You under the Apache License, Version 2.0
@@ -17,7 +17,7 @@
  */
 package org.apache.sling.feature.builder;
 
-import org.apache.sling.feature.KeyValueMap;
+import java.util.Map;
 
 /**
  * Context for an extension handler.
@@ -34,5 +34,5 @@ public interface HandlerContext {
      * @return Map of provided configuration, or an empty map if there is no configuration.
      * Never {@code null}.
      */
-    KeyValueMap getConfiguration();
+    Map<String,String> getConfiguration();
 }
diff --git a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
index ce3621f..5fd187d 100644
--- a/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/BuilderUtilTest.java
@@ -16,14 +16,21 @@
  */
 package org.apache.sling.feature.builder;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.fail;
+import org.apache.sling.feature.Artifact;
+import org.apache.sling.feature.ArtifactId;
+import org.apache.sling.feature.Bundles;
+import org.apache.sling.feature.Extension;
+import org.apache.sling.feature.ExtensionType;
+import org.apache.sling.feature.Feature;
+import org.apache.sling.feature.builder.BuilderContext.ArtifactMergeAlgorithm;
+import org.junit.Test;
+import org.mockito.Mockito;
 
 import java.io.StringReader;
 import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -38,16 +45,9 @@ import javax.json.JsonString;
 import javax.json.JsonValue;
 import javax.json.stream.JsonGenerator;
 
-import org.apache.sling.feature.Artifact;
-import org.apache.sling.feature.ArtifactId;
-import org.apache.sling.feature.Bundles;
-import org.apache.sling.feature.Extension;
-import org.apache.sling.feature.ExtensionType;
-import org.apache.sling.feature.Feature;
-import org.apache.sling.feature.KeyValueMap;
-import org.apache.sling.feature.builder.BuilderContext.ArtifactMergeAlgorithm;
-import org.junit.Test;
-import org.mockito.Mockito;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
 
 public class BuilderUtilTest {
 
@@ -221,10 +221,10 @@ public class BuilderUtilTest {
     }
 
     @Test public void testMergeVariables() {
-        KeyValueMap target = new KeyValueMap();
+        Map<String,String> target = new HashMap<>();
         target.put("x", "327");
 
-        KeyValueMap source = new KeyValueMap();
+        Map<String,String> source = new HashMap<>();
         source.put("a", "b");
 
         BuilderUtil.mergeVariables(target, source, null);
@@ -269,7 +269,7 @@ public class BuilderUtilTest {
 
             JsonObjectBuilder jo = Json.createObjectBuilder();
             boolean hasCfg = false;
-            for (Map.Entry<String, String> entry : context.getConfiguration()) {
+            for (Map.Entry<String, String> entry : context.getConfiguration().entrySet()) {
                 jo.add(entry.getKey(), entry.getValue());
                 hasCfg = true;
             }
@@ -342,7 +342,7 @@ public class BuilderUtilTest {
     }
 
     @Test public void testMergeCustomExtensionsFirst() {
-        KeyValueMap m = new KeyValueMap();
+        Map<String,String> m = new HashMap<>();
         m.put("abc", "def");
         m.put("hij", "klm");
 
diff --git a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
index ddcd43d..dd22ac6 100644
--- a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
@@ -16,23 +16,6 @@
  */
 package org.apache.sling.feature.builder;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
-import java.util.AbstractMap;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 import org.apache.felix.utils.resource.CapabilityImpl;
 import org.apache.felix.utils.resource.RequirementImpl;
 import org.apache.sling.feature.Artifact;
@@ -43,11 +26,27 @@ import org.apache.sling.feature.ExtensionType;
 import org.apache.sling.feature.Feature;
 import org.apache.sling.feature.FeatureConstants;
 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.util.AbstractMap;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+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 FeatureBuilderTest {
 
     private static final Map<String, Feature> FEATURES = new HashMap<>();
@@ -586,7 +585,7 @@ public class FeatureBuilderTest {
     @Test public void testHandleVars() throws Exception {
         ArtifactId aid = new ArtifactId("gid", "aid", "1.2.3", null, null);
         Feature feature = new Feature(aid);
-        KeyValueMap kvMap = feature.getVariables();
+        Map<String,String> kvMap = feature.getVariables();
         kvMap.put("var1", "bar");
         kvMap.put("varvariable", "${myvar}");
         kvMap.put("var.2", "2");
@@ -605,16 +604,16 @@ public class FeatureBuilderTest {
         Feature aFeature = new Feature(aid);
         Feature bFeature = new Feature(bid);
 
-        KeyValueMap kvMapA = aFeature.getVariables();
+        Map<String,String> kvMapA = aFeature.getVariables();
         kvMapA.put("var1", "val1");
         kvMapA.put("var2", "val2");
 
-        KeyValueMap kvMapB = bFeature.getVariables();
+        Map<String,String> kvMapB = bFeature.getVariables();
         kvMapB.put("var1", "val1");
         kvMapB.put("var2", "val2");
         kvMapB.put("var3", "val3");
 
-        KeyValueMap override = new KeyValueMap();
+        Map<String,String> override = new HashMap<>();
         override.put("var3", "valo");
         override.put("val4", "notused");
 
@@ -627,7 +626,7 @@ public class FeatureBuilderTest {
             }
                 }).addVariablesOverwrites(override), aFeature, bFeature);
 
-        KeyValueMap vars = new KeyValueMap();
+        Map<String,String> vars = new HashMap<>();
         vars.putAll(kvMapA);
         vars.putAll(kvMapB);
 
@@ -661,7 +660,7 @@ public class FeatureBuilderTest {
             }
                 }).addVariablesOverwrites(override), aFeature, bFeature);
 
-        vars = new KeyValueMap();
+        vars = new HashMap<>();
         vars.putAll(kvMapA);
         vars.putAll(kvMapB);
         vars.put("var2", "valo");