You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@karaf.apache.org by gn...@apache.org on 2017/10/24 20:06:50 UTC

[karaf] 01/04: [KARAF-5023] The config:update command does not handle substituted properties correctly

This is an automated email from the ASF dual-hosted git repository.

gnodet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/karaf.git

commit b6494ae17709fff6c50df4fd219fec2a880ecfd1
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Tue Oct 24 17:08:01 2017 +0200

    [KARAF-5023] The config:update command does not handle substituted properties correctly
---
 .../karaf/config/command/ConfigCommandSupport.java |   6 +-
 .../command/ConfigPropertyCommandSupport.java      |  22 +--
 .../apache/karaf/config/command/EditCommand.java   |   7 +-
 .../karaf/config/command/PropAppendCommand.java    |   7 +-
 .../karaf/config/command/PropDelCommand.java       |   7 +-
 .../karaf/config/command/PropGetCommand.java       |  20 ++-
 .../karaf/config/command/PropListCommand.java      |  42 ++++-
 .../karaf/config/command/PropSetCommand.java       |   8 +-
 .../apache/karaf/config/command/UpdateCommand.java |   5 +-
 .../command/completers/ConfigurationCompleter.java |   6 +-
 .../apache/karaf/config/core/ConfigRepository.java |  28 +---
 .../karaf/config/core/impl/ConfigMBeanImpl.java    |  62 +++----
 .../config/core/impl/ConfigRepositoryImpl.java     | 185 +++++++--------------
 .../karaf/config/core/impl/MetaServiceCaller.java  |   2 -
 .../karaf/config/core/impl/osgi/Activator.java     |  17 +-
 .../karaf/config/command/EditCommandTest.java      |   5 +-
 .../karaf/config/command/UpdateCommandTest.java    |  14 +-
 .../itests/ssh/ConfigSshCommandSecurityTest.java   |   8 +-
 18 files changed, 209 insertions(+), 242 deletions(-)

diff --git a/config/src/main/java/org/apache/karaf/config/command/ConfigCommandSupport.java b/config/src/main/java/org/apache/karaf/config/command/ConfigCommandSupport.java
index 031f85d..2043158 100644
--- a/config/src/main/java/org/apache/karaf/config/command/ConfigCommandSupport.java
+++ b/config/src/main/java/org/apache/karaf/config/command/ConfigCommandSupport.java
@@ -17,8 +17,8 @@
 package org.apache.karaf.config.command;
 
 import java.util.Arrays;
-import java.util.Dictionary;
 
+import org.apache.felix.utils.properties.TypedProperties;
 import org.apache.karaf.config.core.ConfigRepository;
 import org.apache.karaf.shell.api.action.Action;
 import org.apache.karaf.shell.api.action.lifecycle.Reference;
@@ -51,8 +51,8 @@ public abstract class ConfigCommandSupport implements Action {
     protected abstract Object doExecute() throws Exception;
 
     @SuppressWarnings("rawtypes")
-    protected Dictionary<String, Object> getEditedProps() throws Exception {
-        return (Dictionary) this.session.get(PROPERTY_CONFIG_PROPS);
+    protected TypedProperties getEditedProps() throws Exception {
+        return (TypedProperties) this.session.get(PROPERTY_CONFIG_PROPS);
     }
 
     public void setConfigRepository(ConfigRepository configRepository) {
diff --git a/config/src/main/java/org/apache/karaf/config/command/ConfigPropertyCommandSupport.java b/config/src/main/java/org/apache/karaf/config/command/ConfigPropertyCommandSupport.java
index 0caa5f4..533ea93 100644
--- a/config/src/main/java/org/apache/karaf/config/command/ConfigPropertyCommandSupport.java
+++ b/config/src/main/java/org/apache/karaf/config/command/ConfigPropertyCommandSupport.java
@@ -16,9 +16,7 @@
  */
 package org.apache.karaf.config.command;
 
-import java.util.Dictionary;
-import java.util.Hashtable;
-
+import org.apache.felix.utils.properties.TypedProperties;
 import org.apache.karaf.config.command.completers.ConfigurationCompleter;
 import org.apache.karaf.shell.api.action.Completion;
 import org.apache.karaf.shell.api.action.Option;
@@ -33,12 +31,12 @@ public abstract class ConfigPropertyCommandSupport extends ConfigCommandSupport
     protected String pid;
 
     protected Object doExecute() throws Exception {
-        Dictionary<String, Object> props = getEditedProps();
+        TypedProperties props = getEditedProps();
         if (props == null && pid == null) {
             System.err.println("No configuration is being edited--run the edit command first");
         } else {
             if (props == null) {
-                props = new Hashtable<>();
+                props = new TypedProperties();
             }
             propertyAction(props);
             if(requiresUpdate(pid)) {
@@ -53,8 +51,7 @@ public abstract class ConfigPropertyCommandSupport extends ConfigCommandSupport
      *
      * @param props the dictionary where to apply the action.
      */
-    @SuppressWarnings("rawtypes")
-    protected abstract void propertyAction(Dictionary props);
+    protected abstract void propertyAction(TypedProperties props);
 
     /**
      * Check if the configuration requires to be updated.
@@ -74,8 +71,13 @@ public abstract class ConfigPropertyCommandSupport extends ConfigCommandSupport
      * @throws Exception in case of configuration failure.
      */
     @Override
-    protected Dictionary<String, Object> getEditedProps() throws Exception {
-        Dictionary<String, Object> props = this.configRepository.getConfigProperties(pid);
-        return (props != null) ? props : super.getEditedProps();
+    protected TypedProperties getEditedProps() throws Exception {
+        if (pid != null) {
+            return this.configRepository.getConfig(pid);
+        }
+        else {
+            return super.getEditedProps();
+        }
     }
+
 }
diff --git a/config/src/main/java/org/apache/karaf/config/command/EditCommand.java b/config/src/main/java/org/apache/karaf/config/command/EditCommand.java
index 8b03fc5..d3493b4 100644
--- a/config/src/main/java/org/apache/karaf/config/command/EditCommand.java
+++ b/config/src/main/java/org/apache/karaf/config/command/EditCommand.java
@@ -16,8 +16,7 @@
  */
 package org.apache.karaf.config.command;
 
-import java.util.Dictionary;
-
+import org.apache.felix.utils.properties.TypedProperties;
 import org.apache.karaf.config.command.completers.ConfigurationCompleter;
 import org.apache.karaf.shell.api.action.Argument;
 import org.apache.karaf.shell.api.action.Command;
@@ -37,7 +36,7 @@ public class EditCommand extends ConfigCommandSupport {
     @Option(name = "--force", aliases = {}, description = "Force the edition of this config, even if another one was under edition", required = false, multiValued = false)
     boolean force;
 
-    @Option(name = "--factory", aliases = {}, description = "Define this config as a factory config. Will be crearted on calling update", required = false, multiValued = false)
+    @Option(name = "--factory", aliases = {}, description = "Define this config as a factory config. Will be created on calling update", required = false, multiValued = false)
     boolean factory;
 
     @Option(name = "--alias", aliases = {}, description = "Specifies the alias used for this factory config.", required = false, multiValued = false)
@@ -71,7 +70,7 @@ public class EditCommand extends ConfigCommandSupport {
             System.err.println("The --alias only works in case of a factory configuration. Add the --factory option.");
         }
 
-        Dictionary props = this.configRepository.getConfigProperties(pid);
+        TypedProperties props = this.configRepository.getConfig(pid);
         this.session.put(PROPERTY_CONFIG_PID, pid);
         this.session.put(PROPERTY_FACTORY, factory);
         this.session.put(PROPERTY_CONFIG_PROPS, props);
diff --git a/config/src/main/java/org/apache/karaf/config/command/PropAppendCommand.java b/config/src/main/java/org/apache/karaf/config/command/PropAppendCommand.java
index f021146..6b00431 100644
--- a/config/src/main/java/org/apache/karaf/config/command/PropAppendCommand.java
+++ b/config/src/main/java/org/apache/karaf/config/command/PropAppendCommand.java
@@ -16,8 +16,9 @@
  */
 package org.apache.karaf.config.command;
 
-import java.util.Dictionary;
+import java.util.Collection;
 
+import org.apache.felix.utils.properties.TypedProperties;
 import org.apache.karaf.config.command.completers.ConfigurationPropertyCompleter;
 import org.apache.karaf.shell.api.action.Argument;
 import org.apache.karaf.shell.api.action.Command;
@@ -37,12 +38,14 @@ public class PropAppendCommand extends ConfigPropertyCommandSupport {
 
     @SuppressWarnings({"unchecked", "rawtypes"})
     @Override
-    public void propertyAction(Dictionary props) {
+    public void propertyAction(TypedProperties props) {
         final Object currentValue = props.get(prop);
         if (currentValue == null) {
             props.put(prop, value);
         } else if (currentValue instanceof String) {
             props.put(prop, currentValue + value);
+        } else if (currentValue instanceof Collection) {
+            ((Collection) currentValue).add(value);
         } else {
             System.err.println("Append Failed: current value is not a String.");
         }
diff --git a/config/src/main/java/org/apache/karaf/config/command/PropDelCommand.java b/config/src/main/java/org/apache/karaf/config/command/PropDelCommand.java
index e416cf6..528c972 100644
--- a/config/src/main/java/org/apache/karaf/config/command/PropDelCommand.java
+++ b/config/src/main/java/org/apache/karaf/config/command/PropDelCommand.java
@@ -16,13 +16,11 @@
  */
 package org.apache.karaf.config.command;
 
-import java.util.Dictionary;
-
+import org.apache.felix.utils.properties.TypedProperties;
 import org.apache.karaf.shell.api.action.Argument;
 import org.apache.karaf.shell.api.action.Command;
 import org.apache.karaf.shell.api.action.lifecycle.Service;
 
-
 @Command(scope = "config", name = "property-delete", description = "Deletes a property from the configuration being edited.")
 @Service
 public class PropDelCommand extends ConfigPropertyCommandSupport {
@@ -30,9 +28,8 @@ public class PropDelCommand extends ConfigPropertyCommandSupport {
     @Argument(index = 0, name = "property", description = "The name of the property to delete", required = true, multiValued = false)
     String prop;
 
-    @SuppressWarnings("rawtypes")
     @Override
-    public void propertyAction(Dictionary props) {
+    public void propertyAction(TypedProperties props) {
         props.remove(prop);
     }
 }
diff --git a/config/src/main/java/org/apache/karaf/config/command/PropGetCommand.java b/config/src/main/java/org/apache/karaf/config/command/PropGetCommand.java
index 4656e57..48d453e 100644
--- a/config/src/main/java/org/apache/karaf/config/command/PropGetCommand.java
+++ b/config/src/main/java/org/apache/karaf/config/command/PropGetCommand.java
@@ -16,27 +16,37 @@
  */
 package org.apache.karaf.config.command;
 
-
+import org.apache.felix.utils.properties.TypedProperties;
 import org.apache.karaf.config.command.completers.ConfigurationPropertyCompleter;
 import org.apache.karaf.shell.api.action.Argument;
 import org.apache.karaf.shell.api.action.Command;
 import org.apache.karaf.shell.api.action.Completion;
+import org.apache.karaf.shell.api.action.Option;
 import org.apache.karaf.shell.api.action.lifecycle.Service;
 
-import java.util.Dictionary;
+import java.util.List;
 
 @Command(scope = "config", name = "property-get", description = "Gets the value of a property in the currently edited configuration.")
 @Service
 public class PropGetCommand extends ConfigPropertyCommandSupport {
 
+    @Option(name = "--raw")
+    boolean raw;
+
     @Argument(index = 0, name = "property", description = "The name of the property to get value for", required = true, multiValued = false)
     @Completion(ConfigurationPropertyCompleter.class)
     String prop;
 
-    @SuppressWarnings("rawtypes")
     @Override
-    public void propertyAction(Dictionary props) {
-        System.out.println(props.get(prop));
+    public void propertyAction(TypedProperties props) {
+        if (raw) {
+            List<String> strings = props.getRaw(prop);
+            for (String s : strings) {
+                System.out.println(s);
+            }
+        } else {
+            System.out.println(displayValue(props.get(prop)));
+        }
     }
 
 
diff --git a/config/src/main/java/org/apache/karaf/config/command/PropListCommand.java b/config/src/main/java/org/apache/karaf/config/command/PropListCommand.java
index 015e118..60b2209 100644
--- a/config/src/main/java/org/apache/karaf/config/command/PropListCommand.java
+++ b/config/src/main/java/org/apache/karaf/config/command/PropListCommand.java
@@ -16,22 +16,50 @@
  */
 package org.apache.karaf.config.command;
 
-import java.util.Dictionary;
-import java.util.Enumeration;
+import java.io.IOException;
+import java.io.StringReader;
+import java.io.StringWriter;
+import java.util.Map;
+import java.util.TreeMap;
 
+import org.apache.felix.utils.properties.TypedProperties;
 import org.apache.karaf.shell.api.action.Command;
+import org.apache.karaf.shell.api.action.Option;
 import org.apache.karaf.shell.api.action.lifecycle.Service;
 
 @Command(scope = "config", name = "property-list", description = "Lists properties from the currently edited configuration.")
 @Service
 public class PropListCommand extends ConfigPropertyCommandSupport {
 
-    @SuppressWarnings("rawtypes")
+    @Option(name = "--raw")
+    boolean raw;
+
     @Override
-    public void propertyAction(Dictionary props) {
-        for (Enumeration e = props.keys(); e.hasMoreElements(); ) {
-            Object key = e.nextElement();
-            System.out.println("   " + key + " = " + props.get(key));
+    public void propertyAction(TypedProperties props) {
+        if (raw) {
+            try {
+                StringWriter sw = new StringWriter();
+                props.save(sw);
+                System.out.print(sw.toString());
+            } catch (IOException e) {
+                throw new RuntimeException(e);
+            }
+        }
+        else
+        {
+            try {
+                StringWriter sw = new StringWriter();
+                props.save(sw);
+                TypedProperties p = new TypedProperties();
+                p.load(new StringReader(sw.toString()));
+                props = p;
+            } catch (IOException e) {
+                // Ignore
+            }
+            Map<String, Object> sortedProps = new TreeMap<>(props);
+            for (Map.Entry<String, Object> entry : sortedProps.entrySet()) {
+                System.out.println("   " + entry.getKey() + " = " + displayValue(entry.getValue()));
+            }
         }
     }
 
diff --git a/config/src/main/java/org/apache/karaf/config/command/PropSetCommand.java b/config/src/main/java/org/apache/karaf/config/command/PropSetCommand.java
index b4c3f85..74eb2e8 100644
--- a/config/src/main/java/org/apache/karaf/config/command/PropSetCommand.java
+++ b/config/src/main/java/org/apache/karaf/config/command/PropSetCommand.java
@@ -16,8 +16,12 @@
  */
 package org.apache.karaf.config.command;
 
+import java.io.IOException;
+import java.io.StringReader;
+import java.io.StringWriter;
 import java.util.Dictionary;
 
+import org.apache.felix.utils.properties.TypedProperties;
 import org.apache.karaf.config.command.completers.ConfigurationPropertyCompleter;
 import org.apache.karaf.shell.api.action.Argument;
 import org.apache.karaf.shell.api.action.Command;
@@ -35,9 +39,9 @@ public class PropSetCommand extends ConfigPropertyCommandSupport {
     @Argument(index = 1, name = "value", description = "The value of the property", required = true, multiValued = false)
     Object value;
 
-    @SuppressWarnings({"unchecked", "rawtypes"})
     @Override
-    public void propertyAction(Dictionary props) {
+    public void propertyAction(TypedProperties props) {
         props.put(prop, value);
     }
+
 }
diff --git a/config/src/main/java/org/apache/karaf/config/command/UpdateCommand.java b/config/src/main/java/org/apache/karaf/config/command/UpdateCommand.java
index 3e36694..a908c8c 100644
--- a/config/src/main/java/org/apache/karaf/config/command/UpdateCommand.java
+++ b/config/src/main/java/org/apache/karaf/config/command/UpdateCommand.java
@@ -16,8 +16,7 @@
  */
 package org.apache.karaf.config.command;
 
-import java.util.Dictionary;
-
+import org.apache.felix.utils.properties.TypedProperties;
 import org.apache.karaf.shell.api.action.Command;
 import org.apache.karaf.shell.api.action.lifecycle.Service;
 
@@ -28,7 +27,7 @@ public class UpdateCommand extends ConfigCommandSupport {
     @Override
     @SuppressWarnings({ "rawtypes", "unchecked" })
     protected Object doExecute() throws Exception {
-        Dictionary props = getEditedProps();
+        TypedProperties props = getEditedProps();
         if (props == null) {
             System.err.println("No configuration is being edited--run the edit command first");
             return null;
diff --git a/config/src/main/java/org/apache/karaf/config/command/completers/ConfigurationCompleter.java b/config/src/main/java/org/apache/karaf/config/command/completers/ConfigurationCompleter.java
index b0cef13..19cc0df 100644
--- a/config/src/main/java/org/apache/karaf/config/command/completers/ConfigurationCompleter.java
+++ b/config/src/main/java/org/apache/karaf/config/command/completers/ConfigurationCompleter.java
@@ -77,7 +77,7 @@ public class ConfigurationCompleter implements Completer, ConfigurationListener
 
         Collection<String> pids = new ArrayList<>();
         for (Configuration config : configs) {
-            pids.add(config.getPid());
+            pids.add(config.getPid() + " ");
         }
 
         delegate.getStrings().addAll(pids);
@@ -95,9 +95,9 @@ public class ConfigurationCompleter implements Completer, ConfigurationListener
     public void configurationEvent(ConfigurationEvent configurationEvent) {
         String pid = configurationEvent.getPid();
         if (configurationEvent.getType() == ConfigurationEvent.CM_DELETED) {
-            delegate.getStrings().remove(pid);
+            delegate.getStrings().remove(pid + " ");
         } else if (configurationEvent.getType() == ConfigurationEvent.CM_UPDATED) {
-            delegate.getStrings().add(pid);
+            delegate.getStrings().add(pid + " ");
         }
     }
 }
diff --git a/config/src/main/java/org/apache/karaf/config/core/ConfigRepository.java b/config/src/main/java/org/apache/karaf/config/core/ConfigRepository.java
index 55cb1b0..556cc17 100644
--- a/config/src/main/java/org/apache/karaf/config/core/ConfigRepository.java
+++ b/config/src/main/java/org/apache/karaf/config/core/ConfigRepository.java
@@ -17,36 +17,24 @@
 package org.apache.karaf.config.core;
 
 import java.io.IOException;
-import java.util.Dictionary;
+import java.util.Map;
 
+import org.apache.felix.utils.properties.TypedProperties;
 import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.service.cm.ConfigurationAdmin;
 
 public interface ConfigRepository {
 
-    /**
-     * Save config to storage or ConfigurationAdmin.
-     *
-     * @param pid the configuration PID.
-     * @param props the dictionary used to update the configuration.
-     * @throws IOException in case of update failure.
-     */
-    void update(String pid, Dictionary<String, Object> props) throws IOException;
+    ConfigurationAdmin getConfigAdmin();
+
+    TypedProperties getConfig(String pid) throws IOException, InvalidSyntaxException;
 
     void delete(String pid) throws Exception;
 
-    Dictionary<String, Object> getConfigProperties(String pid) throws IOException, InvalidSyntaxException;
+    void update(String pid, Map<String, Object> properties) throws IOException;
 
-    ConfigurationAdmin getConfigAdmin();
+    String createFactoryConfiguration(String factoryPid, Map<String, Object> properties) throws IOException;
 
-    /**
-     * Create a factory based configuration.
-     *
-     * @param factoryPid the configuration factory PID.
-     * @param properties the new properties to set in the configuration.
-     * @return the created configuration PID.
-     */
-    String createFactoryConfiguration(String factoryPid, Dictionary<String, Object> properties);
+    String createFactoryConfiguration(String factoryPid, String alias, Map<String, Object> properties) throws IOException;
 
-    String createFactoryConfiguration(String factoryPid, String alias, Dictionary<String, Object> properties);
 }
diff --git a/config/src/main/java/org/apache/karaf/config/core/impl/ConfigMBeanImpl.java b/config/src/main/java/org/apache/karaf/config/core/impl/ConfigMBeanImpl.java
index 3a93f9f..de142c8 100644
--- a/config/src/main/java/org/apache/karaf/config/core/impl/ConfigMBeanImpl.java
+++ b/config/src/main/java/org/apache/karaf/config/core/impl/ConfigMBeanImpl.java
@@ -18,7 +18,6 @@ import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.Arrays;
 import java.util.Dictionary;
-import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.List;
@@ -29,9 +28,11 @@ import javax.management.MBeanException;
 import javax.management.NotCompliantMBeanException;
 import javax.management.StandardMBean;
 
+import org.apache.felix.utils.properties.TypedProperties;
 import org.apache.karaf.config.core.ConfigMBean;
 import org.apache.karaf.config.core.ConfigRepository;
 import org.apache.karaf.util.StreamUtils;
+import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.service.cm.Configuration;
 
 /**
@@ -54,14 +55,8 @@ public class ConfigMBeanImpl extends StandardMBean implements ConfigMBean {
     }
 
     @SuppressWarnings({ "unchecked", "rawtypes" })
-    private Dictionary<String, Object> getConfigProperties(String pid) throws IOException {
-        Configuration configuration = getConfiguration(pid);
-
-        Dictionary<String, Object> dictionary = configuration.getProperties();
-        if (dictionary == null) {
-            dictionary = new Hashtable(new java.util.Properties());
-        }
-        return dictionary;
+    private TypedProperties getConfigProperties(String pid) throws IOException, InvalidSyntaxException {
+        return configRepo.getConfig(pid);
     }
 
     /**
@@ -81,7 +76,7 @@ public class ConfigMBeanImpl extends StandardMBean implements ConfigMBean {
     @Override
     public void create(String pid) throws MBeanException {
         try {
-            configRepo.update(pid, new Hashtable<>());
+            configRepo.update(pid, new TypedProperties());
         } catch (Exception e) {
             throw new MBeanException(null, e.toString());
         }
@@ -130,13 +125,10 @@ public class ConfigMBeanImpl extends StandardMBean implements ConfigMBean {
     @SuppressWarnings("rawtypes")
     public Map<String, String> listProperties(String pid) throws MBeanException {
         try {
-            Dictionary dictionary = getConfigProperties(pid);
-
+            TypedProperties dictionary = getConfigProperties(pid);
             Map<String, String> propertiesMap = new HashMap<>();
-            for (Enumeration e = dictionary.keys(); e.hasMoreElements(); ) {
-                Object key = e.nextElement();
-                Object value = dictionary.get(key);
-                propertiesMap.put(key.toString(), value.toString());
+            for (Map.Entry<String, Object> e : dictionary.entrySet()) {
+                propertiesMap.put(e.getKey(), displayValue(e.getValue().toString()));
             }
             return propertiesMap;
         } catch (Exception e) {
@@ -144,10 +136,20 @@ public class ConfigMBeanImpl extends StandardMBean implements ConfigMBean {
         }
     }
 
+    protected String displayValue(Object value) {
+        if (value == null) {
+            return "<null>";
+        }
+        if (value.getClass().isArray()) {
+            return Arrays.toString((Object[]) value);
+        }
+        return value.toString();
+    }
+
     @Override
     public void deleteProperty(String pid, String key) throws MBeanException {
         try {
-            Dictionary<String, Object> dictionary = getConfigProperties(pid);
+            TypedProperties dictionary = getConfigProperties(pid);
             dictionary.remove(key);
             configRepo.update(pid, dictionary);
         } catch (Exception e) {
@@ -158,7 +160,7 @@ public class ConfigMBeanImpl extends StandardMBean implements ConfigMBean {
     @Override
     public void appendProperty(String pid, String key, String value) throws MBeanException {
         try {
-            Dictionary<String, Object> dictionary = getConfigProperties(pid);
+            TypedProperties dictionary = getConfigProperties(pid);
             Object currentValue = dictionary.get(key);
             if (currentValue == null) {
                 dictionary.put(key, value);
@@ -176,7 +178,7 @@ public class ConfigMBeanImpl extends StandardMBean implements ConfigMBean {
     @Override
     public void setProperty(String pid, String key, String value) throws MBeanException {
         try {
-            Dictionary<String, Object> dictionary = getConfigProperties(pid);
+            TypedProperties dictionary = getConfigProperties(pid);
             dictionary.put(key, value);
             configRepo.update(pid, dictionary);
         } catch (Exception e) {
@@ -187,7 +189,7 @@ public class ConfigMBeanImpl extends StandardMBean implements ConfigMBean {
     @Override
     public String getProperty(String pid, String key) throws MBeanException {
         try {
-            Dictionary<String, Object> dictionary = getConfigProperties(pid);
+            TypedProperties dictionary = getConfigProperties(pid);
             Object value = dictionary.get(key);
             if (value != null) {
                 return value.toString();
@@ -201,11 +203,9 @@ public class ConfigMBeanImpl extends StandardMBean implements ConfigMBean {
     @Override
     public void update(String pid, Map<String, String> properties) throws MBeanException {
         try {
-            if (properties == null) {
-                properties = new HashMap<>();
-            }
-            Dictionary<String, Object> dictionary = toDictionary(properties);
-            configRepo.update(pid, dictionary);
+            TypedProperties props = configRepo.getConfig(pid);
+            props.putAll(properties);
+            configRepo.update(pid, props);
         } catch (Exception e) {
             throw new MBeanException(null, e.toString());
         }
@@ -226,10 +226,14 @@ public class ConfigMBeanImpl extends StandardMBean implements ConfigMBean {
     }
 
 	@Override
-	public String createFactoryConfiguration(String factoryPid,
-			Map<String, String> properties) throws MBeanException {
-		Dictionary<String, Object> dict = toDictionary(properties);
-		return configRepo.createFactoryConfiguration(factoryPid, dict);
+	public String createFactoryConfiguration(String factoryPid, Map<String, String> properties) throws MBeanException {
+        try {
+            TypedProperties props = new TypedProperties();
+            props.putAll(properties);
+            return configRepo.createFactoryConfiguration(factoryPid, props);
+        } catch (Exception e) {
+            throw new MBeanException(null, e.toString());
+        }
 	}
 
 }
diff --git a/config/src/main/java/org/apache/karaf/config/core/impl/ConfigRepositoryImpl.java b/config/src/main/java/org/apache/karaf/config/core/impl/ConfigRepositoryImpl.java
index 5d3d93f..b2d05c8 100644
--- a/config/src/main/java/org/apache/karaf/config/core/impl/ConfigRepositoryImpl.java
+++ b/config/src/main/java/org/apache/karaf/config/core/impl/ConfigRepositoryImpl.java
@@ -18,18 +18,16 @@ package org.apache.karaf.config.core.impl;
 
 import java.io.File;
 import java.io.IOException;
-import java.io.UncheckedIOException;
 import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
-import java.util.ArrayList;
 import java.util.Dictionary;
 import java.util.Enumeration;
 import java.util.Hashtable;
-import java.util.UUID;
+import java.util.Map;
 
-import org.apache.felix.utils.properties.Properties;
+import org.apache.felix.utils.properties.TypedProperties;
 import org.apache.karaf.config.core.ConfigRepository;
 import org.osgi.framework.Constants;
 import org.osgi.framework.InvalidSyntaxException;
@@ -45,29 +43,35 @@ public class ConfigRepositoryImpl implements ConfigRepository {
     private static final String FILEINSTALL_FILE_NAME = "felix.fileinstall.filename";
 
     private ConfigurationAdmin configAdmin;
-    private File storage;
 
     public ConfigRepositoryImpl(ConfigurationAdmin configAdmin) {
-        this(configAdmin, null);
-    }
-
-    public ConfigRepositoryImpl(ConfigurationAdmin configAdmin, File storage) {
         this.configAdmin = configAdmin;
-        this.storage = storage;
     }
 
     /* (non-Javadoc)
      * @see org.apache.karaf.shell.config.impl.ConfigRepository#update(java.lang.String, java.util.Dictionary, boolean)
      */
     @Override
-    public void update(String pid, Dictionary<String, Object> properties) throws IOException {
-        LOGGER.trace("Update configuration {}", pid);
-        Configuration cfg = configAdmin.getConfiguration(pid, null);
-        cfg.update(properties);
+    public void update(String pid, Map<String, Object> properties) throws IOException {
         try {
-            updateStorage(pid, properties);
-        } catch (Exception e) {
-            LOGGER.warn("Can't update cfg file", e);
+            LOGGER.trace("Updating configuration {}", pid);
+            Configuration cfg = configAdmin.getConfiguration(pid, null);
+            Dictionary<String, Object> dict = cfg.getProperties();
+            TypedProperties props = new TypedProperties();
+            File file = getCfgFileFromProperties(dict);
+            if (file != null) {
+                props.load(file);
+                props.putAll(properties);
+                props.save(file);
+                props.clear();
+                props.load(file);
+                props.put(FILEINSTALL_FILE_NAME, file.toURI().toString());
+            } else {
+                props.putAll(properties);
+            }
+            cfg.update(new Hashtable<>(props));
+        } catch (URISyntaxException e) {
+            throw new IOException("Error updating config", e);
         }
     }
 
@@ -76,140 +80,79 @@ public class ConfigRepositoryImpl implements ConfigRepository {
      */
     @Override
     public void delete(String pid) throws Exception {
-        LOGGER.trace("Delete configuration {}", pid);
+        LOGGER.trace("Deleting configuration {}", pid);
         Configuration configuration = configAdmin.getConfiguration(pid, null);
-        File cfgFile = getCfgFileFromProperties(configuration.getProperties());
         configuration.delete();
-        try {
-            if (cfgFile != null) {
-                LOGGER.trace("Delete {}", cfgFile.getName());
-                cfgFile.delete();
-            }
-        } catch (Exception e) {
-            LOGGER.warn("Can't delete cfg file", e);
-        }
     }
 
     private File getCfgFileFromProperties(Dictionary<String, Object> properties) throws URISyntaxException, MalformedURLException {
-        File cfgFile = null;
         if (properties != null) {
             Object val = properties.get(FILEINSTALL_FILE_NAME);
-            if (val instanceof URL) {
-                cfgFile = new File(((URL) val).toURI());
-            }
-            if (val instanceof URI) {
-                cfgFile = new File((URI) val);
-            }
-            if (val instanceof String) {
-                cfgFile = new File(new URL((String) val).toURI());
-            }
+            return getCfgFileFromProperty(val);
         }
-        return cfgFile;
+        return null;
     }
 
-    protected void updateStorage(String pid, Dictionary<String, Object> props) throws IOException {
-        if (storage != null) {
-            Configuration cfg = configAdmin.getConfiguration(pid, null);
-            // Initialize cfgFile with default location. Value gets overwritten when the existing configuration references a correct location.
-            File cfgFile = new File(storage, pid + ".cfg");
-            if (cfg != null) {
-                Dictionary<String, Object> oldProps = cfg.getProperties();
-                if (oldProps != null && oldProps.get(FILEINSTALL_FILE_NAME) != null) {
-                    try {
-                        cfgFile = getCfgFileFromProperties(oldProps);
-                        if (cfgFile == null) {
-                            throw new IOException("The configuration value '" + oldProps.get(FILEINSTALL_FILE_NAME)
-                                    + "' for '" + FILEINSTALL_FILE_NAME + "' does not represent a valid file location.");
-                        }
-                    } catch (URISyntaxException | MalformedURLException e) {
-                        throw new IOException(e);
-                    }
-                }
-            }
-            LOGGER.trace("Update {}", cfgFile.getName());
-            // update the cfg file
-            Properties properties = new Properties(cfgFile);
-            for (Enumeration<String> keys = props.keys(); keys.hasMoreElements(); ) {
-                String key = keys.nextElement();
-                if (!Constants.SERVICE_PID.equals(key)
-                        && !ConfigurationAdmin.SERVICE_FACTORYPID.equals(key)
-                        && !FILEINSTALL_FILE_NAME.equals(key)) {
-                    if (props.get(key) != null) {
-                        properties.put(key, props.get(key).toString());
-                    }
-                }
-            }
-            // remove "removed" properties from the cfg file
-            ArrayList<String> propertiesToRemove = new ArrayList<>();
-            for (String key : properties.keySet()) {
-                if (props.get(key) == null
-                        && !Constants.SERVICE_PID.equals(key)
-                        && !ConfigurationAdmin.SERVICE_FACTORYPID.equals(key)
-                        && !FILEINSTALL_FILE_NAME.equals(key)) {
-                    propertiesToRemove.add(key);
-                }
-            }
-            for (String key : propertiesToRemove) {
-                properties.remove(key);
-            }
-            // save the cfg file
-            storage.mkdirs();
-            properties.save();
+    private File getCfgFileFromProperty(Object val) throws URISyntaxException, MalformedURLException {
+        if (val instanceof URL) {
+            return new File(((URL) val).toURI());
+        }
+        if (val instanceof URI) {
+            return new File((URI) val);
+        }
+        if (val instanceof String) {
+            return new File(new URL((String) val).toURI());
         }
+        return null;
     }
 
-    /* (non-Javadoc)
-     * @see org.apache.karaf.shell.config.impl.ConfigRepository#getConfigProperties(java.lang.String)
-     */
     @Override
-    public Dictionary<String, Object> getConfigProperties(String pid) throws IOException, InvalidSyntaxException {
+    public TypedProperties getConfig(String pid) throws IOException, InvalidSyntaxException {
         if (pid != null && configAdmin != null) {
             Configuration configuration = configAdmin.getConfiguration(pid, null);
             if (configuration != null) {
+                TypedProperties tp = new TypedProperties();
                 Dictionary<String, Object> props = configuration.getProperties();
-                return (props != null) ? props : new Hashtable<>();
+                if (props != null) {
+                    File file;
+                    try {
+                        file = getCfgFileFromProperties(props);
+                    } catch (URISyntaxException e) {
+                        throw new IOException(e);
+                    }
+                    if (file != null) {
+                        tp.load(file);
+                    } else {
+                        for (Enumeration<String> e = props.keys(); e.hasMoreElements();) {
+                            String key = e.nextElement();
+                            Object val = props.get(key);
+                            tp.put(key, val);
+                        }
+                        tp.remove( Constants.SERVICE_PID );
+                        tp.remove( ConfigurationAdmin.SERVICE_FACTORYPID );
+                    }
+                }
+                return tp;
             }
         }
         return null;
     }
 
     @Override
-    public ConfigurationAdmin getConfigAdmin() {
-        return configAdmin;
+    public String createFactoryConfiguration(String factoryPid, Map<String, Object> properties) throws IOException {
+        return createFactoryConfiguration(factoryPid, null, properties);
     }
 
     @Override
-    public String createFactoryConfiguration(String factoryPid, Dictionary<String, Object> properties) {
-        return createFactoryConfiguration(factoryPid, null, properties);
+    public String createFactoryConfiguration(String factoryPid, String alias, Map<String, Object> properties) throws IOException {
+        Configuration config = configAdmin.createFactoryConfiguration(factoryPid, "?");
+        config.update(new Hashtable<>(properties));
+        return config.getPid();
     }
 
     @Override
-    public String createFactoryConfiguration(String factoryPid, String alias, Dictionary<String, Object> properties) {
-        try {
-            Configuration config = configAdmin.createFactoryConfiguration(factoryPid, null);
-            if (storage != null) {
-                // Check, whether a file location is already provided.
-                if (properties.get(FILEINSTALL_FILE_NAME) == null) {
-                    // Create a synthetic unique alias for the factory
-                    // configuration when it is unspecified.
-                    if (alias == null) {
-                        // Felix Fileinstall uses the hyphen as separator
-                        // between factoryPid and alias. For safety reasons, all
-                        // hyphens are removed from the generated UUID.
-                        alias = UUID.randomUUID().toString().replaceAll("-", "");
-                    }
-                    String cfgFileName = factoryPid + "-" + alias + ".cfg";
-                    File cfgFile = new File(storage, cfgFileName);
-                    properties.put(FILEINSTALL_FILE_NAME, cfgFile.getCanonicalFile().toURI().toString());
-                }
-            }
-            config.update(properties);
-            String pid = config.getPid();
-            updateStorage(pid, properties);
-            return pid;
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
-        }
+    public ConfigurationAdmin getConfigAdmin() {
+        return configAdmin;
     }
 
 }
diff --git a/config/src/main/java/org/apache/karaf/config/core/impl/MetaServiceCaller.java b/config/src/main/java/org/apache/karaf/config/core/impl/MetaServiceCaller.java
index 5401df0..0db2a79 100644
--- a/config/src/main/java/org/apache/karaf/config/core/impl/MetaServiceCaller.java
+++ b/config/src/main/java/org/apache/karaf/config/core/impl/MetaServiceCaller.java
@@ -21,8 +21,6 @@ import org.osgi.framework.BundleContext;
 import org.osgi.framework.ServiceReference;
 import org.osgi.service.metatype.MetaTypeInformation;
 import org.osgi.service.metatype.MetaTypeService;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
 import java.util.Arrays;
diff --git a/config/src/main/java/org/apache/karaf/config/core/impl/osgi/Activator.java b/config/src/main/java/org/apache/karaf/config/core/impl/osgi/Activator.java
index cef9527..2138dad 100644
--- a/config/src/main/java/org/apache/karaf/config/core/impl/osgi/Activator.java
+++ b/config/src/main/java/org/apache/karaf/config/core/impl/osgi/Activator.java
@@ -23,12 +23,8 @@ import org.apache.karaf.util.tracker.BaseActivator;
 import org.apache.karaf.util.tracker.annotation.ProvideService;
 import org.apache.karaf.util.tracker.annotation.RequireService;
 import org.apache.karaf.util.tracker.annotation.Services;
-import org.osgi.service.cm.Configuration;
 import org.osgi.service.cm.ConfigurationAdmin;
 
-import java.io.File;
-import java.io.IOException;
-
 @Services(
         requires = @RequireService(ConfigurationAdmin.class),
         provides = @ProvideService(ConfigRepository.class)
@@ -41,9 +37,7 @@ public class Activator extends BaseActivator {
             return;
         }
 
-        File persistenceStorage = getPersistenceStorage(configurationAdmin);
-
-        ConfigRepository configRepository = new ConfigRepositoryImpl(configurationAdmin, persistenceStorage);
+        ConfigRepository configRepository = new ConfigRepositoryImpl(configurationAdmin);
         register(ConfigRepository.class, configRepository);
 
         ConfigMBeanImpl configMBean = new ConfigMBeanImpl();
@@ -51,13 +45,4 @@ public class Activator extends BaseActivator {
         registerMBean(configMBean, "type=config");
     }
 
-    private File getPersistenceStorage(ConfigurationAdmin configurationAdmin) throws IOException {
-        Configuration configuration = configurationAdmin.getConfiguration("org.apache.karaf.config");
-        if(configuration.getProperties() == null){
-            return new File(System.getProperty("karaf.etc"));
-        }
-        String storage = (String) configuration.getProperties().get("storage");
-        return storage == null || storage.trim().isEmpty() ? null : new File(storage);
-    }
-
 }
diff --git a/config/src/test/java/org/apache/karaf/config/command/EditCommandTest.java b/config/src/test/java/org/apache/karaf/config/command/EditCommandTest.java
index 70c457f..5082b02 100644
--- a/config/src/test/java/org/apache/karaf/config/command/EditCommandTest.java
+++ b/config/src/test/java/org/apache/karaf/config/command/EditCommandTest.java
@@ -20,6 +20,7 @@ import java.util.Dictionary;
 import java.util.Hashtable;
 
 import junit.framework.TestCase;
+import org.apache.felix.utils.properties.TypedProperties;
 import org.apache.karaf.config.core.impl.ConfigRepositoryImpl;
 import org.apache.karaf.shell.api.console.Session;
 import org.osgi.service.cm.Configuration;
@@ -67,7 +68,7 @@ public class EditCommandTest extends TestCase {
         // the PID and Dictionary should have been set on the session
         assertEquals("The PID should be set on the session",
                      PID, session.get(ConfigCommandSupport.PROPERTY_CONFIG_PID));
-        assertSame("The Dictionary returned by the ConfigAdmin service should be set on the session",
+        assertEquals("The Dictionary returned by the ConfigAdmin service should be set on the session",
                    props, session.get(ConfigCommandSupport.PROPERTY_CONFIG_PROPS));
     }
     
@@ -87,7 +88,7 @@ public class EditCommandTest extends TestCase {
         // the PID and an empty Dictionary should have been set on the session        
         assertEquals("The PID should be set on the session",
                      PID, session.get(ConfigCommandSupport.PROPERTY_CONFIG_PID));
-        Dictionary props = (Dictionary) session.get(ConfigCommandSupport.PROPERTY_CONFIG_PROPS);
+        TypedProperties props = (TypedProperties) session.get(ConfigCommandSupport.PROPERTY_CONFIG_PROPS);
         assertNotNull("Should have a Dictionary on the session", props);
         assertTrue("Should have an empty Dictionary on the session", props.isEmpty());
     }
diff --git a/config/src/test/java/org/apache/karaf/config/command/UpdateCommandTest.java b/config/src/test/java/org/apache/karaf/config/command/UpdateCommandTest.java
index 3de1a62..0d7dfc0 100644
--- a/config/src/test/java/org/apache/karaf/config/command/UpdateCommandTest.java
+++ b/config/src/test/java/org/apache/karaf/config/command/UpdateCommandTest.java
@@ -16,13 +16,14 @@
  */
 package org.apache.karaf.config.command;
 
-
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.replay;
 
 import java.util.Dictionary;
+import java.util.Enumeration;
 import java.util.Hashtable;
 
+import org.apache.felix.utils.properties.TypedProperties;
 import org.apache.karaf.config.core.ConfigRepository;
 import org.easymock.EasyMock;
 
@@ -37,7 +38,7 @@ public class UpdateCommandTest extends TestCase {
     private static final String PID = "myPid";
 
     public void testupdateRegularConfig() throws Exception {
-		Dictionary<String, Object> props = new Hashtable<>();
+        Hashtable<String, Object> props = new Hashtable<>();
 
         UpdateCommand command = new UpdateCommand();
         ConfigRepository configRepo = EasyMock.createMock(ConfigRepository.class);
@@ -54,7 +55,7 @@ public class UpdateCommandTest extends TestCase {
     }
 
     public void testupdateOnNewFactoryPid() throws Exception {
-		Dictionary<String, Object> props = new Hashtable<>();
+		Hashtable<String, Object> props = new Hashtable<>();
 
         UpdateCommand command = new UpdateCommand();
         ConfigRepository configRepo = EasyMock.createMock(ConfigRepository.class);
@@ -75,7 +76,12 @@ public class UpdateCommandTest extends TestCase {
 		MockCommandSession session = new MockCommandSession();
         session.put(ConfigCommandSupport.PROPERTY_CONFIG_PID, pid);
         session.put(ConfigCommandSupport.PROPERTY_FACTORY, isFactory);
-        session.put(ConfigCommandSupport.PROPERTY_CONFIG_PROPS, props);
+        TypedProperties tp = new TypedProperties();
+        for (Enumeration<String> e = props.keys(); e.hasMoreElements();) {
+            String key = e.nextElement();
+            tp.put(key, props.get(key));
+        }
+        session.put(ConfigCommandSupport.PROPERTY_CONFIG_PROPS, tp);
 		return session;
 	}
 
diff --git a/itests/src/test/java/org/apache/karaf/itests/ssh/ConfigSshCommandSecurityTest.java b/itests/src/test/java/org/apache/karaf/itests/ssh/ConfigSshCommandSecurityTest.java
index f7b65b7..2e012c9 100644
--- a/itests/src/test/java/org/apache/karaf/itests/ssh/ConfigSshCommandSecurityTest.java
+++ b/itests/src/test/java/org/apache/karaf/itests/ssh/ConfigSshCommandSecurityTest.java
@@ -66,14 +66,14 @@ public class ConfigSshCommandSecurityTest extends SshCommandTestBase {
         String result = assertCommand(user, "config:edit " + pid + "\n" +
                 "config:property-list\n" +
                 "config:cancel", Result.OK);
-        Assert.assertTrue(result.contains("x = yz"));
-        Assert.assertTrue(result.contains("a = b"));
+        Assert.assertTrue("Result should contain 'x = yz': " + result, result.contains("x = yz"));
+        Assert.assertTrue("Result should contain 'a = b': " + result, result.contains("a = b"));
         String result2 = assertCommand(user, "config:edit " + pid + "\n" +
                 "config:property-delete a\n" +
                 "config:property-list\n" +
                 "config:update", Result.OK);
-        Assert.assertTrue(result2.contains("x = yz"));
-        Assert.assertFalse(result2.contains("a = b"));
+        Assert.assertTrue("Result should contain 'x = yz': " + result2, result2.contains("x = yz"));
+        Assert.assertFalse("Result should contain 'a = b': " + result2, result2.contains("a = b"));
 
         if (isAdmin) {
             assertCommand(user, "config:delete " + pid, Result.OK);

-- 
To stop receiving notification emails like this one, please contact
"commits@karaf.apache.org" <co...@karaf.apache.org>.