You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@karaf.apache.org by cs...@apache.org on 2017/08/12 07:12:23 UTC

[01/14] karaf git commit: Cleanup test [Forced Update!]

Repository: karaf
Updated Branches:
  refs/heads/model_features cd938d501 -> bdffc0c74 (forced update)


Cleanup test


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/38c728df
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/38c728df
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/38c728df

Branch: refs/heads/model_features
Commit: 38c728dfe31c8056859767692b0d8848469a33e7
Parents: b9ae8bf
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Thu Aug 10 18:39:28 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Thu Aug 10 18:39:28 2017 +0200

----------------------------------------------------------------------
 .../internal/service/FeatureValidationUtil.java | 41 -------------
 .../service/FeaturesValidationTest.java         | 61 ++++++--------------
 2 files changed, 18 insertions(+), 84 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/38c728df/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureValidationUtil.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureValidationUtil.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureValidationUtil.java
deleted file mode 100644
index f0b24a6..0000000
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureValidationUtil.java
+++ /dev/null
@@ -1,41 +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.karaf.features.internal.service;
-
-import java.net.URI;
-
-import org.apache.karaf.features.internal.model.JaxbUtil;
-
-/**
- * Utility class which fires XML Schema validation.
- */
-public final class FeatureValidationUtil {
-
-    private FeatureValidationUtil() {
-    }
-
-    /**
-     * Runs schema validation.
-     *
-     * @param uri Uri to validate.
-     * @throws Exception When validation fails.
-     */
-    public static void validate(URI uri) throws Exception {
-        JaxbUtil.unmarshal(uri.toASCIIString(), true);
-    }
-
-}

http://git-wip-us.apache.org/repos/asf/karaf/blob/38c728df/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesValidationTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesValidationTest.java b/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesValidationTest.java
index af3c2e1..09badc5 100644
--- a/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesValidationTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesValidationTest.java
@@ -16,61 +16,43 @@
  */
 package org.apache.karaf.features.internal.service;
 
-import java.net.URL;
-
-import org.apache.karaf.features.Library;
-import org.apache.karaf.features.internal.model.Features;
-import org.apache.karaf.features.internal.model.JaxbUtil;
-import org.junit.Test;
-
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import java.net.URI;
+
+import org.apache.karaf.features.Library;
+import org.apache.karaf.features.internal.model.Features;
+import org.apache.karaf.features.internal.model.JaxbUtil;
+import org.junit.Test;
+
 public class FeaturesValidationTest {
 
     @Test
     public void testNs10() throws Exception {
-        FeatureValidationUtil.validate(getClass().getResource("f02.xml").toURI());
-    }
-
-    @Test
-    public void testNs10Unmarshall() throws Exception {
-        URL url = getClass().getResource("f02.xml");
-        Features features = JaxbUtil.unmarshal(url.toExternalForm(), true);
+        Features features = unmarshalAndValidate("f02.xml");
         assertNotNull(features);
     }
 
     @Test
     public void testNs10NoName() throws Exception {
-        FeatureValidationUtil.validate(getClass().getResource("f03.xml").toURI());
-    }
-
-    @Test
-    public void testNs10NoNameUnmarshall() throws Exception {
-        URL url = getClass().getResource("f03.xml");
-        Features features = JaxbUtil.unmarshal(url.toExternalForm(), true);
+        Features features = unmarshalAndValidate("f03.xml");
         assertNotNull(features);
     }
 
     @Test
     public void testNs11() throws Exception {
-        FeatureValidationUtil.validate(getClass().getResource("f04.xml").toURI());
-    }
-
-    @Test
-    public void testNs11Unmarshall() throws Exception {
-        URL url = getClass().getResource("f04.xml");
-        Features features = JaxbUtil.unmarshal(url.toExternalForm(), true);
+        Features features = unmarshalAndValidate("f04.xml");;
         assertNotNull(features);
     }
 
     @Test
     public void testNs11NoName() throws Exception {
         try {
-            FeatureValidationUtil.validate(getClass().getResource("f05.xml").toURI());
+            unmarshalAndValidate("f05.xml");
             fail("Validation should have failed");
         } catch (Exception e) {
             // ok
@@ -78,26 +60,14 @@ public class FeaturesValidationTest {
     }
 
     @Test
-    public void testNs12() throws Exception {
-        FeatureValidationUtil.validate(getClass().getResource("f06.xml").toURI());
-    }
-
-    @Test
     public void testNs12Unmarshall() throws Exception {
-        URL url = getClass().getResource("f06.xml");
-        Features features = JaxbUtil.unmarshal(url.toExternalForm(), true);
+        Features features = unmarshalAndValidate("f06.xml");
         assertNotNull(features);
     }
 
     @Test
     public void testNs13() throws Exception {
-        FeatureValidationUtil.validate(getClass().getResource("f07.xml").toURI());
-    }
-
-    @Test
-    public void testNs13Unmarshall() throws Exception {
-        URL url = getClass().getResource("f07.xml");
-        Features features = JaxbUtil.unmarshal(url.toExternalForm(), true);
+        Features features = unmarshalAndValidate("f07.xml");
         assertNotNull(features);
         assertEquals("2.5.6.SEC02", features.getFeature().get(0).getVersion());
         assertTrue(features.getFeature().get(1).isHidden());
@@ -109,4 +79,9 @@ public class FeaturesValidationTest {
         assertTrue(features.getFeature().get(0).getLibraries().get(0).isDelegate());
     }
 
+    private Features unmarshalAndValidate(String path) throws Exception {
+        URI uri = getClass().getResource(path).toURI();
+        return JaxbUtil.unmarshal(uri.toASCIIString(), true);
+    }
+
 }


[10/14] karaf git commit: [KARAF-5300] Add documentation

Posted by cs...@apache.org.
[KARAF-5300] Add documentation


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/fe2f0fbc
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/fe2f0fbc
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/fe2f0fbc

Branch: refs/heads/model_features
Commit: fe2f0fbcc2dcb58439e148a11342f0d417f4d8d6
Parents: 63f4240
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Thu Aug 10 10:33:22 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Sat Aug 12 09:12:13 2017 +0200

----------------------------------------------------------------------
 .../karaf/features/internal/service/FeatureReq.java   | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/fe2f0fbc/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureReq.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureReq.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureReq.java
index 2e1f652..1bac816 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureReq.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureReq.java
@@ -22,6 +22,15 @@ import org.osgi.framework.VersionRange;
 
 /**
  * Requirement for a feature
+ * 
+ * <p>The syntax of a requirement as a String is name[/versionRange].
+ * If no versionRange is given then a range of [0,) is assumeed which matches all versions.
+ * 
+ * <p>
+ * - name: Can be a feature name or a glob like myfeat*
+ * - versionRange: version or range
+ * - version: Will specify a specific version. Like [version,version]. An exemption is 0.0.0 which matches all versions.
+ * - range: Like defined in OSGi VersionRange. Example: [1.0.0, 1.1.0)  
  */
 public class FeatureReq {
     public static final String VERSION_SEPARATOR = "/";
@@ -57,7 +66,10 @@ public class FeatureReq {
     }
 
     private static VersionRange exactVersion(String versionRange) {
-        return new VersionRange(VersionRange.LEFT_CLOSED, new Version(versionRange), new Version(versionRange), VersionRange.RIGHT_CLOSED);
+        return new VersionRange(VersionRange.LEFT_CLOSED, 
+                                new Version(versionRange), 
+                                new Version(versionRange), 
+                                VersionRange.RIGHT_CLOSED);
     }
 
     public FeatureReq(String name, VersionRange versionRange) {


[09/14] karaf git commit: [KARAF-5304] Update karaf script to be compliant with AIX

Posted by cs...@apache.org.
[KARAF-5304] Update karaf script to be compliant with AIX


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/b7bdb359
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/b7bdb359
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/b7bdb359

Branch: refs/heads/model_features
Commit: b7bdb359499d7fd9db09847e078d20cb461c2ace
Parents: 244fafd
Author: Jean-Baptiste Onofré <jb...@apache.org>
Authored: Fri Aug 11 15:00:57 2017 +0200
Committer: Jean-Baptiste Onofré <jb...@apache.org>
Committed: Fri Aug 11 15:01:34 2017 +0200

----------------------------------------------------------------------
 .../base/src/main/filtered-resources/resources/bin/karaf         | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/b7bdb359/assemblies/features/base/src/main/filtered-resources/resources/bin/karaf
----------------------------------------------------------------------
diff --git a/assemblies/features/base/src/main/filtered-resources/resources/bin/karaf b/assemblies/features/base/src/main/filtered-resources/resources/bin/karaf
index dc022bc..048b344 100644
--- a/assemblies/features/base/src/main/filtered-resources/resources/bin/karaf
+++ b/assemblies/features/base/src/main/filtered-resources/resources/bin/karaf
@@ -89,10 +89,10 @@ checkRootInstance() {
       ROOT_INSTANCE_PID=$(sed -n -e '/item.0.pid/ s/.*\= *//p' "${KARAF_HOME}/instances/instance.properties")
       ROOT_INSTANCE_NAME=$(sed -n -e '/item.0.name/ s/.*\= *//p' "${KARAF_HOME}/instances/instance.properties")
       if [ "${ROOT_INSTANCE_PID}" -ne "0" ]; then
-          if ps p "${ROOT_INSTANCE_PID}" > /dev/null
+          if ps -p "${ROOT_INSTANCE_PID}" > /dev/null
           then
               MAIN=org.apache.karaf.main.Main
-              PID_COMMAND=$(ps p "${ROOT_INSTANCE_PID}" o args | sed 1d)
+              PID_COMMAND=$(ps -p "${ROOT_INSTANCE_PID}" o args | sed 1d)
               if [ "${PID_COMMAND#*$MAIN}" != "$PID_COMMAND" ]; then
                 ROOT_INSTANCE_RUNNING=true
               fi


[08/14] karaf git commit: Refactor FeatureConfigInstaller

Posted by cs...@apache.org.
Refactor FeatureConfigInstaller


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/244fafdc
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/244fafdc
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/244fafdc

Branch: refs/heads/model_features
Commit: 244fafdc40a6b8210c8f5b2c65a3f8f5d7f7f07e
Parents: 5cc9c81
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Fri Aug 11 12:01:28 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Fri Aug 11 12:01:28 2017 +0200

----------------------------------------------------------------------
 .../service/FeatureConfigInstaller.java         | 182 +++++++++----------
 1 file changed, 89 insertions(+), 93 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/244fafdc/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java
index 5a86e53..694427e 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java
@@ -62,15 +62,17 @@ public class FeatureConfigInstaller {
         this.configCfgStore = configCfgStore;
     }
 
-    private String[] parsePid(String pid) {
+    private ConfigId parsePid(String pid) {
         int n = pid.indexOf('-');
+        ConfigId cid = new ConfigId();
+        cid.fullPid = pid;
         if (n > 0) {
-            String factoryPid = pid.substring(n + 1);
-            pid = pid.substring(0, n);
-            return new String[]{pid, factoryPid};
+            cid.factoryPid = pid.substring(n + 1);
+            cid.pid = pid.substring(0, n);
         } else {
-            return new String[]{pid, null};
+            cid.pid = pid;
         }
+        return cid;
     }
 
     private Configuration createConfiguration(ConfigurationAdmin configurationAdmin, String pid,
@@ -83,21 +85,16 @@ public class FeatureConfigInstaller {
         }
     }
 
-    private Configuration findExistingConfiguration(ConfigurationAdmin configurationAdmin, String pid,
-                                                    String factoryPid)
+    private Configuration findExistingConfiguration(ConfigurationAdmin configurationAdmin, ConfigId cid)
         throws IOException, InvalidSyntaxException {
         String filter;
-        if (factoryPid == null) {
-            filter = "(" + Constants.SERVICE_PID + "=" + pid + ")";
+        if (cid.factoryPid == null) {
+            filter = "(" + Constants.SERVICE_PID + "=" + cid.pid + ")";
         } else {
-            String key = createConfigurationKey(pid, factoryPid);
-            filter = "(" + CONFIG_KEY + "=" + key + ")";
+            filter = "(" + CONFIG_KEY + "=" + cid.fullPid + ")";
         }
         Configuration[] configurations = configurationAdmin.listConfigurations(filter);
-        if (configurations != null && configurations.length > 0) {
-            return configurations[0];
-        }
-        return null;
+        return (configurations != null && configurations.length > 0) ? configurations[0] : null;
     }
 
     public void installFeatureConfigs(Feature feature) throws IOException, InvalidSyntaxException {
@@ -110,26 +107,20 @@ public class FeatureConfigInstaller {
             } else {
                 props.load(new StringReader(val));
             }
-            String[] pid = parsePid(config.getName());
-            Configuration cfg = findExistingConfiguration(configAdmin, pid[0], pid[1]);
+            ConfigId cid = parsePid(config.getName());
+            Configuration cfg = findExistingConfiguration(configAdmin, cid);
             if (cfg == null) {
                 Dictionary<String, Object> cfgProps = convertToDict(props);
-                cfg = createConfiguration(configAdmin, pid[0], pid[1]);
-                String key = createConfigurationKey(pid[0], pid[1]);
-                cfgProps.put(CONFIG_KEY, key);
-                props.put(CONFIG_KEY, key);
+                cfg = createConfiguration(configAdmin, cid.pid, cid.factoryPid);
+                cfgProps.put(CONFIG_KEY, cid.fullPid);
+                props.put(CONFIG_KEY, cid.fullPid);
                 if (storage != null && configCfgStore) {
-                    File cfgFile;
-                    if (pid[1] != null) {
-                        cfgFile = new File(storage, pid[0] + "-" + pid[1] + ".cfg");
-                    } else {
-                        cfgFile = new File(storage, pid[0] + ".cfg");
-                    }
+                    File cfgFile = new File(storage, cid.fullPid + ".cfg");
                     cfgProps.put(FILEINSTALL_FILE_NAME, cfgFile.getAbsoluteFile().toURI().toString());
                 }
                 cfg.update(cfgProps);
                 try {
-                    updateStorage(pid[0], pid[1], props, false);
+                    updateStorage(cid, props, false);
                 } catch (Exception e) {
                     LOGGER.warn("Can't update cfg file", e);
                 }
@@ -145,7 +136,7 @@ public class FeatureConfigInstaller {
                 if (update) {
                     cfg.update(properties);
                     try {
-                        updateStorage(pid[0], pid[1], props, true);
+                        updateStorage(cid, props, true);
                     } catch (Exception e) {
                         LOGGER.warn("Can't update cfg file", e);
                     }
@@ -166,10 +157,6 @@ public class FeatureConfigInstaller {
         return cfgProps;
     }
 
-    private String createConfigurationKey(String pid, String factoryPid) {
-        return factoryPid == null ? pid : pid + "-" + factoryPid;
-    }
-
     /**
      * Substitute variables in the final name and append prefix if necessary.
      *
@@ -260,77 +247,86 @@ public class FeatureConfigInstaller {
         }
     }
 
-    protected void updateStorage(String pid, String factoryPid, TypedProperties props, boolean append)
+    protected void updateStorage(ConfigId cid, TypedProperties props, boolean append)
         throws Exception {
         if (storage != null && configCfgStore) {
-            // get the cfg file
-            File cfgFile;
-            if (factoryPid != null) {
-                cfgFile = new File(storage, pid + "-" + factoryPid + ".cfg");
+            File cfgFile = getConfigFile(cid);
+            if (!cfgFile.exists()) {
+                props.save(cfgFile);
             } else {
-                cfgFile = new File(storage, pid + ".cfg");
+                updateExistingConfig(props, append, cfgFile);
             }
-            Configuration cfg = findExistingConfiguration(configAdmin, pid, factoryPid);
-            // update the cfg file depending of the configuration
-            if (cfg != null && cfg.getProperties() != null) {
-                Object val = cfg.getProperties().get(FILEINSTALL_FILE_NAME);
-                try {
-                    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());
-                    }
-                } catch (Exception e) {
-                    throw new IOException(e.getMessage(), e);
+        }
+    }
+
+    private File getConfigFile(ConfigId cid) throws IOException, InvalidSyntaxException {
+        Configuration cfg = findExistingConfiguration(configAdmin, cid);
+        // update the cfg file depending of the configuration
+        File cfgFile = new File(storage, cid.fullPid + ".cfg");
+        if (cfg != null && cfg.getProperties() != null) {
+            Object val = cfg.getProperties().get(FILEINSTALL_FILE_NAME);
+            try {
+                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());
                 }
+            } catch (Exception e) {
+                throw new IOException(e.getMessage(), e);
             }
-            LOGGER.trace("Update {}", cfgFile.getName());
-            // update the cfg file
-            if (!cfgFile.exists()) {
-                props.save(cfgFile);
-            } else {
-                TypedProperties properties = new TypedProperties();
-                properties.load(cfgFile);
-                for (String key : props.keySet()) {
-                    if (!Constants.SERVICE_PID.equals(key)
-                        && !ConfigurationAdmin.SERVICE_FACTORYPID.equals(key)
-                        && !FILEINSTALL_FILE_NAME.equals(key)) {
-                        List<String> comments = props.getComments(key);
-                        List<String> value = props.getRaw(key);
-                        Object writeValue = (value.size() == 1) ? value.get(0) : value;
-                        if (!properties.containsKey(key)) {
-                            properties.put(key, comments, writeValue);
-                        } else if (!append) {
-                            if (comments.isEmpty()) {
-                                comments = properties.getComments(key);
-                            }
-                            properties.put(key, comments, writeValue);
-                        }
+        }
+        LOGGER.trace("Update {}", cfgFile.getName());
+        return cfgFile;
+    }
+
+    private void updateExistingConfig(TypedProperties props, boolean append, File cfgFile)
+        throws IOException {
+        TypedProperties properties = new TypedProperties();
+        properties.load(cfgFile);
+        for (String key : props.keySet()) {
+            if (!isInternalKey(key)) {
+                List<String> comments = props.getComments(key);
+                List<String> value = props.getRaw(key);
+                Object writeValue = (value.size() == 1) ? value.get(0) : value;
+                if (!properties.containsKey(key)) {
+                    properties.put(key, comments, writeValue);
+                } else if (!append) {
+                    if (comments.isEmpty()) {
+                        comments = properties.getComments(key);
                     }
+                    properties.put(key, comments, writeValue);
                 }
-                if (!append) {
-                    // remove "removed" properties from the cfg file
-                    ArrayList<String> propertiesToRemove = new ArrayList<>();
-                    for (String key : properties.keySet()) {
-                        if (!props.containsKey(key) && !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);
-                    }
+            }
+        }
+        if (!append) {
+            // remove "removed" properties from the cfg file
+            ArrayList<String> propertiesToRemove = new ArrayList<>();
+            for (String key : properties.keySet()) {
+                if (!props.containsKey(key) && !isInternalKey(key)) {
+                    propertiesToRemove.add(key);
                 }
-                // save the cfg file
-                storage.mkdirs();
-                properties.save(cfgFile);
+            }
+            for (String key : propertiesToRemove) {
+                properties.remove(key);
             }
         }
+        storage.mkdirs();
+        properties.save(cfgFile);
+    }
+
+    private boolean isInternalKey(String key) {
+        return Constants.SERVICE_PID.equals(key)
+            || ConfigurationAdmin.SERVICE_FACTORYPID.equals(key)
+            || FILEINSTALL_FILE_NAME.equals(key);
     }
 
+    class ConfigId {
+        String fullPid;
+        String pid;
+        String factoryPid;
+    }
 }


[13/14] karaf git commit: Extract common code in test

Posted by cs...@apache.org.
Extract common code in test


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/0e1e325f
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/0e1e325f
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/0e1e325f

Branch: refs/heads/model_features
Commit: 0e1e325f2f208276ef6405977cddacd6a5262837
Parents: b7bdb35
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Wed Aug 9 15:51:45 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Sat Aug 12 09:12:13 2017 +0200

----------------------------------------------------------------------
 .../service/FeaturesServiceImplTest.java        | 62 +++++++++-----------
 1 file changed, 28 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/0e1e325f/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java b/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
index 01ad34b..a5ec75a 100644
--- a/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
@@ -73,32 +73,27 @@ public class FeaturesServiceImplTest extends TestBase {
         field.setAccessible(true);
         field.set(null, null);
     }
+    
+    @Test
+    public void testListFeatureWithoutVersion() throws Exception {
+        Feature transactionFeature = feature("transaction", "1.0.0");
+        FeaturesServiceImpl impl = featuresServiceWithFeatures(transactionFeature);
+        assertNotNull(impl.getFeatures("transaction", null));
+        assertSame(transactionFeature, impl.getFeatures("transaction", org.apache.karaf.features.internal.model.Feature.DEFAULT_VERSION)[0]);
+    }
 
     @Test
     public void testGetFeature() throws Exception {
         Feature transactionFeature = feature("transaction", "1.0.0");
-        final Map<String, Map<String, Feature>> features = features(transactionFeature);
-        FeaturesServiceConfig cfg = new FeaturesServiceConfig();
-        BundleInstallSupport installSupport = EasyMock.niceMock(BundleInstallSupport.class);
-        EasyMock.replay(installSupport);
-        final FeaturesServiceImpl impl = new FeaturesServiceImpl(new Storage(), null, null, this.resolver, installSupport, null, cfg ) {
-            protected Map<String,Map<String,Feature>> getFeatureCache() throws Exception {
-                return features;
-            }
-        };
+        FeaturesServiceImpl impl = featuresServiceWithFeatures(transactionFeature);
         assertNotNull(impl.getFeatures("transaction", org.apache.karaf.features.internal.model.Feature.DEFAULT_VERSION));
         assertSame(transactionFeature, impl.getFeatures("transaction", org.apache.karaf.features.internal.model.Feature.DEFAULT_VERSION)[0]);
     }
     
     @Test
     public void testGetFeatureStripVersion() throws Exception {
-        FeaturesServiceConfig cfg = new FeaturesServiceConfig();
-        BundleInstallSupport installSupport = EasyMock.mock(BundleInstallSupport.class);
-        final FeaturesServiceImpl impl = new FeaturesServiceImpl(new Storage(), null, null, this.resolver, installSupport, null, cfg) {
-            protected Map<String,Map<String,Feature>> getFeatureCache() throws Exception {
-                return features(feature("transaction", "1.0.0"));
-            }
-        };
+        Feature transactionFeature = feature("transaction", "1.0.0");
+        FeaturesServiceImpl impl = featuresServiceWithFeatures(transactionFeature);
         Feature[] features = impl.getFeatures("transaction", "  1.0.0  ");
         assertEquals(1, features.length);
         Feature feature = features[0];
@@ -108,29 +103,15 @@ public class FeaturesServiceImplTest extends TestBase {
     
     @Test
     public void testGetFeatureNotAvailable() throws Exception {
-        FeaturesServiceConfig cfg = new FeaturesServiceConfig();
-        BundleInstallSupport installSupport = EasyMock.mock(BundleInstallSupport.class);
-        final FeaturesServiceImpl impl = new FeaturesServiceImpl(new Storage(), null, null, this.resolver, installSupport, null, cfg) {
-            protected Map<String,Map<String,Feature>> getFeatureCache() throws Exception {
-                return features(feature("transaction", "1.0.0"));
-            }
-        };
+        Feature transactionFeature = feature("transaction", "1.0.0");
+        FeaturesServiceImpl impl = featuresServiceWithFeatures(transactionFeature);
         assertEquals(0, impl.getFeatures("activemq", org.apache.karaf.features.internal.model.Feature.DEFAULT_VERSION).length);
     }
     
     @Test
     public void testGetFeatureHighestAvailable() throws Exception {
-        final Map<String, Map<String, Feature>> features = features(
-                feature("transaction", "1.0.0"),
-                feature("transaction", "2.0.0")
-        );
-        FeaturesServiceConfig cfg = new FeaturesServiceConfig();
-        BundleInstallSupport installSupport = EasyMock.mock(BundleInstallSupport.class);
-        final FeaturesServiceImpl impl = new FeaturesServiceImpl(new Storage(), null, null, this.resolver, installSupport, null, cfg) {
-            protected Map<String,Map<String,Feature>> getFeatureCache() throws Exception {
-                return features;
-            }
-        };
+        FeaturesServiceImpl impl = featuresServiceWithFeatures(feature("transaction", "1.0.0"),
+                                                               feature("transaction", "2.0.0"));
         assertNotNull(impl.getFeatures("transaction", org.apache.karaf.features.internal.model.Feature.DEFAULT_VERSION));
         assertEquals("2.0.0", impl.getFeatures("transaction", org.apache.karaf.features.internal.model.Feature.DEFAULT_VERSION)[0].getVersion());
     }
@@ -175,6 +156,19 @@ public class FeaturesServiceImplTest extends TestBase {
         assertInstalled(featureService, b1Feature);
     }
 
+    private FeaturesServiceImpl featuresServiceWithFeatures(Feature... staticFeatures) {
+        final Map<String, Map<String, Feature>> features = features(staticFeatures);
+        FeaturesServiceConfig cfg = new FeaturesServiceConfig();
+        BundleInstallSupport installSupport = EasyMock.niceMock(BundleInstallSupport.class);
+        EasyMock.replay(installSupport);
+        final FeaturesServiceImpl impl = new FeaturesServiceImpl(new Storage(), null, null, this.resolver, installSupport, null, cfg ) {
+            protected Map<String,Map<String,Feature>> getFeatureCache() throws Exception {
+                return features;
+            }
+        };
+        return impl;
+    }
+
     private FeaturesServiceImpl createTestFeatureService() {
         FeaturesServiceConfig cfg = new FeaturesServiceConfig();
         BundleInstallSupport installSupport = EasyMock.niceMock(BundleInstallSupport.class);


[06/14] karaf git commit: Test config write back

Posted by cs...@apache.org.
Test config write back

Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/0cbb2685
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/0cbb2685
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/0cbb2685

Branch: refs/heads/model_features
Commit: 0cbb2685817e70a45e44cb423dd2bd5cdfa37351
Parents: 562019c
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Fri Aug 11 11:15:59 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Fri Aug 11 11:15:59 2017 +0200

----------------------------------------------------------------------
 .../src/test/java/org/apache/karaf/features/AppendTest.java | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/0cbb2685/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/AppendTest.java b/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
index 662ad2d..d64da81 100644
--- a/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
@@ -22,6 +22,8 @@ import static org.easymock.EasyMock.expectLastCall;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+import java.io.File;
+import java.io.FileInputStream;
 import java.io.IOException;
 import java.util.Dictionary;
 import java.util.Hashtable;
@@ -48,7 +50,7 @@ public class AppendTest {
     @Before
     public void before() throws Exception {
         System.setProperty("karaf.data", "data");
-        System.setProperty("karaf.etc", "etc");
+        System.setProperty("karaf.etc", "target");
         RepositoryImpl r = new RepositoryImpl(getClass().getResource("internal/service/f08.xml").toURI());
         Feature[] features = r.getFeatures();
         feature = features[0];
@@ -71,6 +73,8 @@ public class AppendTest {
 
     @Test
     public void testAppend() throws Exception {
+        File cfgFile = new File("target/org.ops4j.pax.web.cfg");
+        cfgFile.delete();
         Hashtable<String, Object> original = new Hashtable<>();
         original.put("foo", "bar");
         Configuration config = expectConfig(admin, original);
@@ -81,6 +85,9 @@ public class AppendTest {
         installer.installFeatureConfigs(feature);
         c.verify();
         assertEquals("data/pax-web-jsp", captured.getValue().get("javax.servlet.context.tempdir"));
+        Properties props = new Properties();
+        props.load(new FileInputStream(cfgFile));
+        assertEquals("data/pax-web-jsp", props.getProperty("javax.servlet.context.tempdir"));
     }
 
     private Configuration expectConfig(ConfigurationAdmin admin, Hashtable<String, Object> original)


[11/14] karaf git commit: [KARAF-5300] Reuse and extract feature matching code

Posted by cs...@apache.org.
[KARAF-5300] Reuse and extract feature matching code


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/bdffc0c7
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/bdffc0c7
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/bdffc0c7

Branch: refs/heads/model_features
Commit: bdffc0c7413e40d4fc1461b27a9f792bfcaadb8c
Parents: 89b2a44
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Thu Aug 10 15:30:59 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Sat Aug 12 09:12:13 2017 +0200

----------------------------------------------------------------------
 .../internal/service/FeaturesServiceImpl.java   | 79 +++++++++++---------
 1 file changed, 43 insertions(+), 36 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/bdffc0c7/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
index 2518a09..7b61ea8 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
@@ -519,15 +519,15 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
 
     @Override
     public Feature[] getFeatures(String nameOrId) throws Exception {
-        return getFeatures(new FeatureReq(nameOrId));
+        return toArray(getFeatures(new FeatureReq(nameOrId)));
     }
 
     @Override
     public Feature[] getFeatures(String name, String version) throws Exception {
-        return getFeatures(new FeatureReq(name, version));
+        return toArray(getFeatures(new FeatureReq(name, version)));
     }
     
-    private Feature[] getFeatures(FeatureReq featureReq) throws Exception {
+    private Collection<Feature> getFeatures(FeatureReq featureReq) throws Exception {
         List<Feature> features = new ArrayList<>();
         Pattern pattern = Pattern.compile(featureReq.getName());
         Map<String, Map<String, Feature>> allFeatures = getFeatureCache();
@@ -540,6 +540,10 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
                 }
             }
         }
+        return features;
+    }
+    
+    private Feature[] toArray(Collection<Feature> features) {
         return features.toArray(new Feature[features.size()]);
     }
 
@@ -818,33 +822,33 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
     private Set<FeatureReq> computeFeaturesToAdd(EnumSet<Option> options, 
                                                  Set<FeatureReq> toInstall) throws Exception {
         Feature[] installedFeatures = listInstalledFeatures();
-        Map<String, Map<String, Feature>> allFeatures = getFeatureCache();
         Set<FeatureReq> toAdd = new HashSet<>();
-        for (FeatureReq feature : toInstall) {
-            Pattern pattern = Pattern.compile(feature.getName());
-            boolean matched = false;
-            for (String fKey : allFeatures.keySet()) {
-                Matcher matcher = pattern.matcher(fKey);
-                if (matcher.matches()) {
-                    Feature f = getFeatureMatching(fKey, feature.getVersionRange());
-                    if (f != null) {
-                        toAdd.add(new FeatureReq(f));
-                        for (Feature installedFeature : installedFeatures) {
-                            if (installedFeature.getName().equals(f.getName()) && installedFeature.getVersion().equals(f.getVersion())) {
-                                LOGGER.info("The specified feature: '{}' version '{}' {}",f.getName(),f.getVersion(),f.getVersion().endsWith("SNAPSHOT") ? "has been upgraded": "is already installed");
-                            }
-                        }
-                        matched = true;
+        for (FeatureReq featureReq : toInstall) {
+            Collection<Feature> matching = getFeatures(featureReq);
+            for (Feature f: matching) {
+                toAdd.add(new FeatureReq(f));
+                for (Feature installedFeature : installedFeatures) {
+                    if (isSameFeature(f, installedFeature)) {
+                        logInstalledOrUpdated(f);
                     }
                 }
             }
-            if (!matched && !options.contains(Option.NoFailOnFeatureNotFound)) {
-                throw new IllegalArgumentException("No matching features for " + feature);
+            if (matching.isEmpty() && !options.contains(Option.NoFailOnFeatureNotFound)) {
+                throw new IllegalArgumentException("No matching features for " + featureReq);
             }
         }
         return toAdd;
     }
 
+    private void logInstalledOrUpdated(Feature f) {
+        String msg = f.getVersion().endsWith("SNAPSHOT") ? "has been upgraded": "is already installed";
+        LOGGER.info("The specified feature: '{}' version '{}' {}", f.getName(), f.getVersion(), msg);
+    }
+
+    private boolean isSameFeature(Feature a, Feature b) {
+        return b.getName().equals(a.getName()) && b.getVersion().equals(a.getVersion());
+    }
+
     private Set<FeatureReq> computeFeaturesToRemoveOnUpdate(Set<FeatureReq> featuresToAdd,
                                              Set<FeatureReq> existingFeatures) throws Exception {
         Set<String> namesToAdd = featuresToAdd.stream().map(f -> f.getName()).collect(toSet());
@@ -868,34 +872,37 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         if (region == null || region.isEmpty()) {
             region = ROOT_REGION;
         }
-        Set<String> requiredForRegion = required.computeIfAbsent(region, k -> new HashSet<>());
+        Set<String> requirements = required.computeIfAbsent(region, k -> new HashSet<>());
+        Set<FeatureReq> existingFeatures = requirements.stream().map(r -> toFeatureReq(r)).collect(toSet());
         Set<FeatureReq> featuresToRemove = new HashSet<>();
         for (FeatureReq feature : featureReqs) {
-            Pattern pattern = Pattern.compile(feature.getName());
-            List<FeatureReq> toRemove = new ArrayList<>();
-            for (String existingFeature : requiredForRegion) {
-               FeatureReq existingFeatureReq = toFeatureReq(existingFeature);
-               if (existingFeatureReq != null) {
-                   Matcher matcher = pattern.matcher(existingFeatureReq.getName());
-                   if (matcher.matches() && feature.getVersionRange().includes(existingFeatureReq.getVersionRange().getLeft())) {
-                       toRemove.add(existingFeatureReq);
-                   }
-               }
-            }
-
+            List<FeatureReq> toRemove = getMatching(existingFeatures, feature);
             if (toRemove.isEmpty()) {
                 throw new IllegalArgumentException("Feature named '" + feature + "' is not installed");
             }
             featuresToRemove.addAll(toRemove);
         }
         print("Removing features: " + join(featuresToRemove), options.contains(Option.Verbose));
-        featuresToRemove.stream().forEach(f->requiredForRegion.remove(toRequirement(f)));
-        if (requiredForRegion.isEmpty()) {
+        featuresToRemove.stream().forEach(f -> requirements.remove(toRequirement(f)));
+        if (requirements.isEmpty()) {
             required.remove(region);
         }
         doProvisionInThread(required, emptyMap(), state, getFeaturesById(), options);
     }
 
+    private List<FeatureReq> getMatching(Set<FeatureReq> existingFeatures, FeatureReq feature) {
+        Pattern pattern = Pattern.compile(feature.getName());
+        List<FeatureReq> matching = new ArrayList<>();
+        for (FeatureReq existingFeatureReq : existingFeatures) {
+            Matcher matcher = pattern.matcher(existingFeatureReq.getName());
+            Version existingVersion = existingFeatureReq.getVersionRange().getLeft();  
+            if (matcher.matches() && feature.getVersionRange().includes(existingVersion)) {
+                matching.add(existingFeatureReq);
+            }
+        }
+        return matching;
+    }
+
     private FeatureReq toFeatureReq(String featureReq) {
         if (!featureReq.startsWith(FEATURE_OSGI_REQUIREMENT_PREFIX)) {
             return null;


[07/14] karaf git commit: [KARAF-5305] Fix issue when cfg file exists

Posted by cs...@apache.org.
[KARAF-5305] Fix issue when cfg file exists


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/5cc9c81b
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/5cc9c81b
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/5cc9c81b

Branch: refs/heads/model_features
Commit: 5cc9c81b3aadacab6c33614f22680b2acadde20d
Parents: 0cbb268
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Fri Aug 11 11:42:10 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Fri Aug 11 11:42:10 2017 +0200

----------------------------------------------------------------------
 .../internal/service/FeatureConfigInstaller.java     |  5 +++--
 .../java/org/apache/karaf/features/AppendTest.java   | 15 ++++++++++++++-
 2 files changed, 17 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/5cc9c81b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java
index 77522c5..5a86e53 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java
@@ -301,13 +301,14 @@ public class FeatureConfigInstaller {
                         && !FILEINSTALL_FILE_NAME.equals(key)) {
                         List<String> comments = props.getComments(key);
                         List<String> value = props.getRaw(key);
+                        Object writeValue = (value.size() == 1) ? value.get(0) : value;
                         if (!properties.containsKey(key)) {
-                            properties.put(key, comments, value);
+                            properties.put(key, comments, writeValue);
                         } else if (!append) {
                             if (comments.isEmpty()) {
                                 comments = properties.getComments(key);
                             }
-                            properties.put(key, comments, value);
+                            properties.put(key, comments, writeValue);
                         }
                     }
                 }

http://git-wip-us.apache.org/repos/asf/karaf/blob/5cc9c81b/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/AppendTest.java b/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
index d64da81..434cbc9 100644
--- a/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
@@ -24,6 +24,7 @@ import static org.junit.Assert.assertTrue;
 
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.util.Dictionary;
 import java.util.Hashtable;
@@ -72,9 +73,21 @@ public class AppendTest {
     }
 
     @Test
-    public void testAppend() throws Exception {
+    public void testAppendWhenFileExists() throws Exception {
+        testAppendInternal(true);
+    }
+    
+    @Test
+    public void testAppendWhenNoFileExists() throws Exception {
+        testAppendInternal(false);
+    }
+
+    private void testAppendInternal(boolean cfgFileExists) throws IOException, InvalidSyntaxException, FileNotFoundException {
         File cfgFile = new File("target/org.ops4j.pax.web.cfg");
         cfgFile.delete();
+        if (cfgFileExists) {
+            cfgFile.createNewFile();
+        }
         Hashtable<String, Object> original = new Hashtable<>();
         original.put("foo", "bar");
         Configuration config = expectConfig(admin, original);


[03/14] karaf git commit: Cleanup test

Posted by cs...@apache.org.
Cleanup test


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/3b42fb5a
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/3b42fb5a
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/3b42fb5a

Branch: refs/heads/model_features
Commit: 3b42fb5a5b52ce20a7843521d5f41ff157086672
Parents: 05806fd
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Thu Aug 10 19:15:26 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Thu Aug 10 19:15:26 2017 +0200

----------------------------------------------------------------------
 features/core/pom.xml                           |  7 ++++
 .../apache/karaf/features/ConditionalTest.java  | 36 ++++++++------------
 2 files changed, 21 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/3b42fb5a/features/core/pom.xml
----------------------------------------------------------------------
diff --git a/features/core/pom.xml b/features/core/pom.xml
index d4f5944..9e5a272 100644
--- a/features/core/pom.xml
+++ b/features/core/pom.xml
@@ -98,6 +98,13 @@
             <artifactId>tinybundles</artifactId>
             <scope>test</scope>
         </dependency>
+        
+        <dependency>
+            <groupId>org.hamcrest</groupId>
+            <artifactId>hamcrest-all</artifactId>
+            <version>1.3</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

http://git-wip-us.apache.org/repos/asf/karaf/blob/3b42fb5a/features/core/src/test/java/org/apache/karaf/features/ConditionalTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/ConditionalTest.java b/features/core/src/test/java/org/apache/karaf/features/ConditionalTest.java
index b103683..aed4b3d 100644
--- a/features/core/src/test/java/org/apache/karaf/features/ConditionalTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/ConditionalTest.java
@@ -16,39 +16,31 @@
  */
 package org.apache.karaf.features;
 
-import junit.framework.TestCase;
+import static org.hamcrest.Matchers.contains;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
 import org.apache.karaf.features.internal.service.RepositoryImpl;
+import org.junit.Test;
 
 
-public class ConditionalTest extends TestCase {
+public class ConditionalTest {
 
+    @Test
     public void testLoad() throws Exception {
         RepositoryImpl r = new RepositoryImpl(getClass().getResource("internal/service/f06.xml").toURI());
-        // Check repo
         Feature[] features = r.getFeatures();
-        assertNotNull(features);
         assertEquals(1, features.length);
         Feature feature = features[0];
 
-        assertNotNull(feature.getConditional());
         assertEquals(2,feature.getConditional().size());
 
-        Conditional conditional = feature.getConditional().get(0);
-        assertNotNull(conditional.getCondition());
-        assertEquals(1,conditional.getCondition().size());
-        String dependency = conditional.getCondition().get(0);
-        assertNotNull(dependency);
-        assertEquals("http", dependency);
-        assertNotNull(conditional.getBundles());
-        assertEquals(1, feature.getConditional().get(0).getBundles().size());
-
-        conditional = feature.getConditional().get(1);
-        assertNotNull(conditional.getCondition());
-        assertEquals(1,conditional.getCondition().size());
-        dependency = conditional.getCondition().get(0);
-        assertNotNull(dependency);
-        assertEquals("req:osgi.ee;filter:=\"(&(osgi.ee=JavaSE)(!(version>=1.7)))\"", dependency);
-
-        String wrapperName = "my6/1.5.3-beta-3".replaceAll("[^A-Za-z0-9 ]", "_");
+        Conditional conditional1 = feature.getConditional().get(0);
+        assertThat(conditional1.getCondition(), contains("http"));
+        assertEquals(1, conditional1.getBundles().size());
+
+        Conditional conditional2 = feature.getConditional().get(1);
+        assertThat(conditional2.getCondition(), contains("req:osgi.ee;filter:=\"(&(osgi.ee=JavaSE)(!(version>=1.7)))\""));
     }
+    
 }


[12/14] karaf git commit: [KARAF-5300] Split install into add and upgrade

Posted by cs...@apache.org.
[KARAF-5300] Split install into add and upgrade


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/89b2a44d
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/89b2a44d
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/89b2a44d

Branch: refs/heads/model_features
Commit: 89b2a44d65d267cba2568e8712603232b7ec642f
Parents: fe2f0fb
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Thu Aug 10 11:23:10 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Sat Aug 12 09:12:13 2017 +0200

----------------------------------------------------------------------
 .../internal/service/FeaturesServiceImpl.java   | 114 +++++++++----------
 1 file changed, 55 insertions(+), 59 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/89b2a44d/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
index e44db6e..2518a09 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
@@ -83,6 +83,8 @@ import org.osgi.service.resolver.Resolver;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static java.util.Collections.emptyMap;
+import static java.util.stream.Collectors.toSet;
 import static org.apache.karaf.features.internal.service.StateStorage.toStringStringSetMap;
 import static org.apache.karaf.features.internal.util.MapUtils.add;
 import static org.apache.karaf.features.internal.util.MapUtils.copy;
@@ -786,28 +788,39 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
 
     @Override
     public void installFeatures(Set<String> featuresIn, String region, EnumSet<Option> options) throws Exception {
-        Set<FeatureReq> featureReqs = new HashSet<>();
+        Set<FeatureReq> toInstall = new HashSet<>();
         for (String feature : featuresIn) {
-            featureReqs.add(new FeatureReq(feature));
+            toInstall.add(new FeatureReq(feature));
         }
         State state = copyState();
         Map<String, Set<String>> requires = copy(state.requirements);
         if (region == null || region.isEmpty()) {
             region = ROOT_REGION;
         }
-        Set<String> requiredForRegion = requires.computeIfAbsent(region, k -> new HashSet<>());
-        computeRequirements(options, featureReqs, requiredForRegion);
-        Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
-        doProvisionInThread(requires, stateChanges, state, getFeaturesById(), options);
-    }
+        Set<String> requirements = requires.computeIfAbsent(region, k -> new HashSet<>());
+        Set<FeatureReq> existingFeatures = requirements.stream().map(r -> toFeatureReq(r)).collect(toSet());
+
+        Set<FeatureReq> toAdd = computeFeaturesToAdd(options, toInstall);
+        toAdd.stream().forEach(f -> requirements.add(toRequirement(f)));
+        print("Adding features: " + join(toAdd), options.contains(Option.Verbose));
+        
+        if (options.contains(Option.Upgrade)) {
+            Set<FeatureReq> toRemove = computeFeaturesToRemoveOnUpdate(toAdd, existingFeatures);
+            toRemove.stream().forEach(f -> requirements.remove(toRequirement(f)));
+            if (!toRemove.isEmpty()) {
+                print("Removing features: " + join(toRemove), options.contains(Option.Verbose));
+            }
+        }
 
-    void computeRequirements(EnumSet<Option> options, Set<FeatureReq> featureReqs,
-                                 Set<String> requirements)
-        throws Exception {
+        doProvisionInThread(requires, emptyMap(), state, getFeaturesById(), options);
+    }
+    
+    private Set<FeatureReq> computeFeaturesToAdd(EnumSet<Option> options, 
+                                                 Set<FeatureReq> toInstall) throws Exception {
+        Feature[] installedFeatures = listInstalledFeatures();
         Map<String, Map<String, Feature>> allFeatures = getFeatureCache();
-        List<FeatureReq> featuresToAdd = new ArrayList<>();
-        List<String> featuresToRemove = new ArrayList<>();
-        for (FeatureReq feature : featureReqs) {
+        Set<FeatureReq> toAdd = new HashSet<>();
+        for (FeatureReq feature : toInstall) {
             Pattern pattern = Pattern.compile(feature.getName());
             boolean matched = false;
             for (String fKey : allFeatures.keySet()) {
@@ -815,8 +828,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
                 if (matcher.matches()) {
                     Feature f = getFeatureMatching(fKey, feature.getVersionRange());
                     if (f != null) {
-                        featuresToAdd.add(new FeatureReq(f));
-                        Feature[] installedFeatures = listInstalledFeatures();
+                        toAdd.add(new FeatureReq(f));
                         for (Feature installedFeature : installedFeatures) {
                             if (installedFeature.getName().equals(f.getName()) && installedFeature.getVersion().equals(f.getVersion())) {
                                 LOGGER.info("The specified feature: '{}' version '{}' {}",f.getName(),f.getVersion(),f.getVersion().endsWith("SNAPSHOT") ? "has been upgraded": "is already installed");
@@ -829,29 +841,20 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
             if (!matched && !options.contains(Option.NoFailOnFeatureNotFound)) {
                 throw new IllegalArgumentException("No matching features for " + feature);
             }
-            if (options.contains(Option.Upgrade)) {
-                for (String existentFeatureReq : requirements) {
-                    FeatureReq existentFeature = getFeatureRefFromRequired(existentFeatureReq);
-                    if (existentFeature.getName().equals(feature.getName())
-                            && !featuresToAdd.contains(existentFeature)) {
-                        featuresToRemove.add(existentFeature.toString());
-                        //do not break cycle to remove all old versions of feature
-                    }
-                }
-            }
-        }
-        if (!featuresToRemove.isEmpty()) {
-            print("Removing features: " + join(featuresToRemove), options.contains(Option.Verbose));
-            for (String featureReq : featuresToRemove) {
-                requirements.remove(FEATURE_OSGI_REQUIREMENT_PREFIX + featureReq);
-            }
         }
-        List<String> featuresToDisplay = new ArrayList<>();
-        for (FeatureReq feature : featuresToAdd) {
-            requirements.add(FEATURE_OSGI_REQUIREMENT_PREFIX + feature.toString());
-            featuresToDisplay.add(feature.toString());
-        }
-        print("Adding features: " + join(featuresToDisplay), options.contains(Option.Verbose));
+        return toAdd;
+    }
+
+    private Set<FeatureReq> computeFeaturesToRemoveOnUpdate(Set<FeatureReq> featuresToAdd,
+                                             Set<FeatureReq> existingFeatures) throws Exception {
+        Set<String> namesToAdd = featuresToAdd.stream().map(f -> f.getName()).collect(toSet());
+        return existingFeatures.stream()
+            .filter(f -> namesToAdd.contains(f.getName()) && !featuresToAdd.contains(f))
+            .collect(toSet());
+    }
+
+    private String toRequirement(FeatureReq feature) {
+        return FEATURE_OSGI_REQUIREMENT_PREFIX + feature.toString();
     }
 
     @Override
@@ -865,21 +868,20 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         if (region == null || region.isEmpty()) {
             region = ROOT_REGION;
         }
-        Set<String> existingFeatures = required.computeIfAbsent(region, k -> new HashSet<>());
-        Set<String> featuresToRemove = new HashSet<>();
+        Set<String> requiredForRegion = required.computeIfAbsent(region, k -> new HashSet<>());
+        Set<FeatureReq> featuresToRemove = new HashSet<>();
         for (FeatureReq feature : featureReqs) {
             Pattern pattern = Pattern.compile(feature.getName());
-            List<String> toRemove = new ArrayList<>();
-            for (String existingFeature : existingFeatures) {
-               FeatureReq existingFeatureReq = getFeatureRefFromRequired(existingFeature);
+            List<FeatureReq> toRemove = new ArrayList<>();
+            for (String existingFeature : requiredForRegion) {
+               FeatureReq existingFeatureReq = toFeatureReq(existingFeature);
                if (existingFeatureReq != null) {
                    Matcher matcher = pattern.matcher(existingFeatureReq.getName());
                    if (matcher.matches() && feature.getVersionRange().includes(existingFeatureReq.getVersionRange().getLeft())) {
-                       toRemove.add(existingFeature);
+                       toRemove.add(existingFeatureReq);
                    }
                }
             }
-            toRemove.retainAll(existingFeatures);
 
             if (toRemove.isEmpty()) {
                 throw new IllegalArgumentException("Feature named '" + feature + "' is not installed");
@@ -887,15 +889,14 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
             featuresToRemove.addAll(toRemove);
         }
         print("Removing features: " + join(featuresToRemove), options.contains(Option.Verbose));
-        existingFeatures.removeAll(featuresToRemove);
-        if (existingFeatures.isEmpty()) {
+        featuresToRemove.stream().forEach(f->requiredForRegion.remove(toRequirement(f)));
+        if (requiredForRegion.isEmpty()) {
             required.remove(region);
         }
-        Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
-        doProvisionInThread(required, stateChanges, state, getFeaturesById(), options);
+        doProvisionInThread(required, emptyMap(), state, getFeaturesById(), options);
     }
 
-    private FeatureReq getFeatureRefFromRequired(String featureReq) {
+    private FeatureReq toFeatureReq(String featureReq) {
         if (!featureReq.startsWith(FEATURE_OSGI_REQUIREMENT_PREFIX)) {
             return null;
         }
@@ -914,8 +915,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         State state = copyState();
         Map<String, Set<String>> required = copy(state.requirements);
         add(required, requirements);
-        Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
-        doProvisionInThread(required, stateChanges, state, getFeaturesById(), options);
+        doProvisionInThread(required, emptyMap(), state, getFeaturesById(), options);
     }
 
     @Override
@@ -923,8 +923,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         State state = copyState();
         Map<String, Set<String>> required = copy(state.requirements);
         remove(required, requirements);
-        Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
-        doProvisionInThread(required, stateChanges, state, getFeaturesById(), options);
+        doProvisionInThread(required, emptyMap(), state, getFeaturesById(), options);
     }
 
     @Override
@@ -947,8 +946,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
             saveState();
             stateCopy = state.copy();
         }
-        Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
-        doProvisionInThread(requirements, stateChanges, stateCopy, getFeaturesById(), options);
+        doProvisionInThread(requirements, emptyMap(), stateCopy, getFeaturesById(), options);
     }
 
     private <T> Set<T> diff(Set<T> s1, Set<T> s2) {
@@ -1187,9 +1185,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         installSupport.installLibraries(feature);
     }
 
-
-
-    private String join(Collection<String> list) {
-        return String.join(", ", list);
+    private String join(Collection<FeatureReq> reqs) {
+        return reqs.stream().map(f->f.toString()).collect(Collectors.joining(","));
     }
 }


[04/14] karaf git commit: Replace tabs in files

Posted by cs...@apache.org.
Replace tabs in files

Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/935006e6
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/935006e6
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/935006e6

Branch: refs/heads/model_features
Commit: 935006e6d1ca68a5f84b2ad1584e7db84f939eb5
Parents: 3b42fb5
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Fri Aug 11 10:07:52 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Fri Aug 11 10:07:52 2017 +0200

----------------------------------------------------------------------
 .../service/FeatureConfigInstaller.java         | 80 ++++++++---------
 .../org/apache/karaf/features/AppendTest.java   | 90 ++++++++++----------
 .../org/apache/karaf/features/FeatureTest.java  | 20 ++---
 .../apache/karaf/features/RepositoryTest.java   | 12 ++-
 4 files changed, 104 insertions(+), 98 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/935006e6/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java
index d1fc525..77522c5 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureConfigInstaller.java
@@ -73,8 +73,9 @@ public class FeatureConfigInstaller {
         }
     }
 
-    private Configuration createConfiguration(ConfigurationAdmin configurationAdmin,
-                                              String pid, String factoryPid) throws IOException, InvalidSyntaxException {
+    private Configuration createConfiguration(ConfigurationAdmin configurationAdmin, String pid,
+                                              String factoryPid)
+        throws IOException, InvalidSyntaxException {
         if (factoryPid != null) {
             return configurationAdmin.createFactoryConfiguration(pid, null);
         } else {
@@ -82,8 +83,9 @@ public class FeatureConfigInstaller {
         }
     }
 
-    private Configuration findExistingConfiguration(ConfigurationAdmin configurationAdmin,
-                                                    String pid, String factoryPid) throws IOException, InvalidSyntaxException {
+    private Configuration findExistingConfiguration(ConfigurationAdmin configurationAdmin, String pid,
+                                                    String factoryPid)
+        throws IOException, InvalidSyntaxException {
         String filter;
         if (factoryPid == null) {
             filter = "(" + Constants.SERVICE_PID + "=" + pid + ")";
@@ -99,7 +101,7 @@ public class FeatureConfigInstaller {
     }
 
     public void installFeatureConfigs(Feature feature) throws IOException, InvalidSyntaxException {
-    	for (ConfigInfo config : feature.getConfigurations()) {
+        for (ConfigInfo config : feature.getConfigurations()) {
             TypedProperties props = new TypedProperties();
             // trim lines
             String val = config.getValue();
@@ -108,14 +110,14 @@ public class FeatureConfigInstaller {
             } else {
                 props.load(new StringReader(val));
             }
-			String[] pid = parsePid(config.getName());
-			Configuration cfg = findExistingConfiguration(configAdmin, pid[0], pid[1]);
-			if (cfg == null) {
-				Dictionary<String, Object> cfgProps = convertToDict(props);
-				cfg = createConfiguration(configAdmin, pid[0], pid[1]);
-				String key = createConfigurationKey(pid[0], pid[1]);
-				cfgProps.put(CONFIG_KEY, key);
-				props.put(CONFIG_KEY, key);
+            String[] pid = parsePid(config.getName());
+            Configuration cfg = findExistingConfiguration(configAdmin, pid[0], pid[1]);
+            if (cfg == null) {
+                Dictionary<String, Object> cfgProps = convertToDict(props);
+                cfg = createConfiguration(configAdmin, pid[0], pid[1]);
+                String key = createConfigurationKey(pid[0], pid[1]);
+                cfgProps.put(CONFIG_KEY, key);
+                props.put(CONFIG_KEY, key);
                 if (storage != null && configCfgStore) {
                     File cfgFile;
                     if (pid[1] != null) {
@@ -125,15 +127,15 @@ public class FeatureConfigInstaller {
                     }
                     cfgProps.put(FILEINSTALL_FILE_NAME, cfgFile.getAbsoluteFile().toURI().toString());
                 }
-				cfg.update(cfgProps);
+                cfg.update(cfgProps);
                 try {
                     updateStorage(pid[0], pid[1], props, false);
                 } catch (Exception e) {
                     LOGGER.warn("Can't update cfg file", e);
                 }
-			} else if (config.isAppend()) {
+            } else if (config.isAppend()) {
                 boolean update = false;
-				Dictionary<String,Object> properties = cfg.getProperties();
+                Dictionary<String, Object> properties = cfg.getProperties();
                 for (String key : props.keySet()) {
                     if (properties.get(key) == null) {
                         properties.put(key, props.get(key));
@@ -148,20 +150,21 @@ public class FeatureConfigInstaller {
                         LOGGER.warn("Can't update cfg file", e);
                     }
                 }
-			}
-		}
+            }
+        }
         for (ConfigFileInfo configFile : feature.getConfigurationFiles()) {
-            installConfigurationFile(configFile.getLocation(), configFile.getFinalname(), configFile.isOverride());
+            installConfigurationFile(configFile.getLocation(), configFile.getFinalname(),
+                                     configFile.isOverride());
         }
     }
 
-	private Dictionary<String, Object> convertToDict(Map<String, Object> props) {
-		Dictionary<String, Object> cfgProps = new Hashtable<>();
+    private Dictionary<String, Object> convertToDict(Map<String, Object> props) {
+        Dictionary<String, Object> cfgProps = new Hashtable<>();
         for (Map.Entry<String, Object> e : props.entrySet()) {
             cfgProps.put(e.getKey(), e.getValue());
         }
-		return cfgProps;
-	}
+        return cfgProps;
+    }
 
     private String createConfigurationKey(String pid, String factoryPid) {
         return factoryPid == null ? pid : pid + "-" + factoryPid;
@@ -194,8 +197,8 @@ public class FeatureConfigInstaller {
         // Substitute all variables, but keep unknown ones.
         final String dummyKey = "";
         try {
-            finalname = InterpolationHelper.substVars(finalname, dummyKey, null, null, null,
-                    true, true, false);
+            finalname = InterpolationHelper.substVars(finalname, dummyKey, null, null, null, true, true,
+                                                      false);
         } catch (final IllegalArgumentException ex) {
             LOGGER.info("Substitution failed. Skip substitution of variables of configuration final name ({}).",
                     finalname);
@@ -219,7 +222,8 @@ public class FeatureConfigInstaller {
         return finalname;
     }
 
-    private void installConfigurationFile(String fileLocation, String finalname, boolean override) throws IOException {
+    private void installConfigurationFile(String fileLocation, String finalname, boolean override)
+        throws IOException {
         finalname = substFinalName(finalname);
 
         File file = new File(finalname);
@@ -256,7 +260,8 @@ public class FeatureConfigInstaller {
         }
     }
 
-    protected void updateStorage(String pid, String factoryPid, TypedProperties props, boolean append) throws Exception {
+    protected void updateStorage(String pid, String factoryPid, TypedProperties props, boolean append)
+        throws Exception {
         if (storage != null && configCfgStore) {
             // get the cfg file
             File cfgFile;
@@ -271,13 +276,13 @@ public class FeatureConfigInstaller {
                 Object val = cfg.getProperties().get(FILEINSTALL_FILE_NAME);
                 try {
                     if (val instanceof URL) {
-                        cfgFile = new File(((URL) val).toURI());
+                        cfgFile = new File(((URL)val).toURI());
                     }
                     if (val instanceof URI) {
-                        cfgFile = new File((URI) val);
+                        cfgFile = new File((URI)val);
                     }
                     if (val instanceof String) {
-                        cfgFile = new File(new URL((String) val).toURI());
+                        cfgFile = new File(new URL((String)val).toURI());
                     }
                 } catch (Exception e) {
                     throw new IOException(e.getMessage(), e);
@@ -289,11 +294,11 @@ public class FeatureConfigInstaller {
                 props.save(cfgFile);
             } else {
                 TypedProperties properties = new TypedProperties();
-                properties.load( cfgFile );
+                properties.load(cfgFile);
                 for (String key : props.keySet()) {
                     if (!Constants.SERVICE_PID.equals(key)
-                            && !ConfigurationAdmin.SERVICE_FACTORYPID.equals(key)
-                            && !FILEINSTALL_FILE_NAME.equals(key)) {
+                        && !ConfigurationAdmin.SERVICE_FACTORYPID.equals(key)
+                        && !FILEINSTALL_FILE_NAME.equals(key)) {
                         List<String> comments = props.getComments(key);
                         List<String> value = props.getRaw(key);
                         if (!properties.containsKey(key)) {
@@ -310,10 +315,9 @@ public class FeatureConfigInstaller {
                     // remove "removed" properties from the cfg file
                     ArrayList<String> propertiesToRemove = new ArrayList<>();
                     for (String key : properties.keySet()) {
-                        if (!props.containsKey(key)
-                                && !Constants.SERVICE_PID.equals(key)
-                                && !ConfigurationAdmin.SERVICE_FACTORYPID.equals(key)
-                                && !FILEINSTALL_FILE_NAME.equals(key)) {
+                        if (!props.containsKey(key) && !Constants.SERVICE_PID.equals(key)
+                            && !ConfigurationAdmin.SERVICE_FACTORYPID.equals(key)
+                            && !FILEINSTALL_FILE_NAME.equals(key)) {
                             propertiesToRemove.add(key);
                         }
                     }
@@ -323,7 +327,7 @@ public class FeatureConfigInstaller {
                 }
                 // save the cfg file
                 storage.mkdirs();
-                properties.save( cfgFile );
+                properties.save(cfgFile);
             }
         }
     }

http://git-wip-us.apache.org/repos/asf/karaf/blob/935006e6/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/AppendTest.java b/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
index 49daded..d6d5d20 100644
--- a/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
@@ -28,58 +28,62 @@ import org.osgi.service.cm.ConfigurationAdmin;
 
 public class AppendTest extends TestCase {
 
-	public void testLoad() throws Exception {
+    public void testLoad() throws Exception {
 
-		System.setProperty("karaf.data", "data");
-		System.setProperty("karaf.etc", "etc");
+        System.setProperty("karaf.data", "data");
+        System.setProperty("karaf.etc", "etc");
 
-		RepositoryImpl r = new RepositoryImpl(getClass().getResource("internal/service/f08.xml").toURI());
-		// Check repo
-		Feature[] features = r.getFeatures();
-		assertNotNull(features);
-		assertEquals(1, features.length);
-		Feature feature = features[0];
+        RepositoryImpl r = new RepositoryImpl(getClass().getResource("internal/service/f08.xml").toURI());
+        // Check repo
+        Feature[] features = r.getFeatures();
+        assertNotNull(features);
+        assertEquals(1, features.length);
+        Feature feature = features[0];
 
-		ConfigInfo configInfo = feature.getConfigurations().get(0);
-		assertNotNull(configInfo);
-		assertTrue(configInfo.isAppend());
+        ConfigInfo configInfo = feature.getConfigurations().get(0);
+        assertNotNull(configInfo);
+        assertTrue(configInfo.isAppend());
 
-		Properties properties = configInfo.getProperties();
-		assertNotNull(properties);
-		String property = properties.getProperty("javax.servlet.context.tempdir");
-		assertNotNull(property);
-		assertFalse(property.contains("${"));
+        Properties properties = configInfo.getProperties();
+        assertNotNull(properties);
+        String property = properties.getProperty("javax.servlet.context.tempdir");
+        assertNotNull(property);
+        assertFalse(property.contains("${"));
         assertEquals(property, "data/pax-web-jsp");
 
-		ConfigurationAdmin admin = EasyMock.createMock(ConfigurationAdmin.class);
-		Configuration config = EasyMock.createMock(Configuration.class);
-		EasyMock.expect(admin.listConfigurations(EasyMock.eq("(service.pid=org.ops4j.pax.web)")))
-				.andReturn(new Configuration[] { config });
-		Hashtable<String, Object> original = new Hashtable<>();
+        ConfigurationAdmin admin = EasyMock.createMock(ConfigurationAdmin.class);
+        Configuration config = EasyMock.createMock(Configuration.class);
+        EasyMock.expect(admin.listConfigurations(EasyMock.eq("(service.pid=org.ops4j.pax.web)")))
+            .andReturn(new Configuration[] {
+                                            config
+        });
+        Hashtable<String, Object> original = new Hashtable<>();
         original.put("javax.servlet.context.tempdir", "data/pax-web-jsp");
-		EasyMock.expect(config.getProperties()).andReturn(original);
+        EasyMock.expect(config.getProperties()).andReturn(original);
 
-		Hashtable<String, Object> expected = new Hashtable<>();
+        Hashtable<String, Object> expected = new Hashtable<>();
         expected.put("org.ops4j.pax.web", "data/pax-web-jsp");
-		expected.put("org.apache.karaf.features.configKey", "org.ops4j.pax.web");
-		expected.put("foo", "bar");
-		EasyMock.expectLastCall();
-		EasyMock.replay(admin, config);
+        expected.put("org.apache.karaf.features.configKey", "org.ops4j.pax.web");
+        expected.put("foo", "bar");
+        EasyMock.expectLastCall();
+        EasyMock.replay(admin, config);
 
-		FeatureConfigInstaller installer = new FeatureConfigInstaller(admin);
-		installer.installFeatureConfigs(feature);
-		EasyMock.verify(admin, config);
+        FeatureConfigInstaller installer = new FeatureConfigInstaller(admin);
+        installer.installFeatureConfigs(feature);
+        EasyMock.verify(admin, config);
 
-		EasyMock.reset(admin, config);
-		EasyMock.expect(admin.listConfigurations(EasyMock.eq("(service.pid=org.ops4j.pax.web)")))
-				.andReturn(new Configuration[]{config});
-		original = new Hashtable<>();
-		original.put("org.apache.karaf.features.configKey", "org.ops4j.pax.web");
-		original.put("javax.servlet.context.tempdir", "value");
-		original.put("foo", "bar");
-		EasyMock.expect(config.getProperties()).andReturn(original);
-		EasyMock.replay(admin, config);
-		installer.installFeatureConfigs(feature);
-		EasyMock.verify(admin, config);
-	}
+        EasyMock.reset(admin, config);
+        EasyMock.expect(admin.listConfigurations(EasyMock.eq("(service.pid=org.ops4j.pax.web)")))
+            .andReturn(new Configuration[] {
+                                            config
+        });
+        original = new Hashtable<>();
+        original.put("org.apache.karaf.features.configKey", "org.ops4j.pax.web");
+        original.put("javax.servlet.context.tempdir", "value");
+        original.put("foo", "bar");
+        EasyMock.expect(config.getProperties()).andReturn(original);
+        EasyMock.replay(admin, config);
+        installer.installFeatureConfigs(feature);
+        EasyMock.verify(admin, config);
+    }
 }

http://git-wip-us.apache.org/repos/asf/karaf/blob/935006e6/features/core/src/test/java/org/apache/karaf/features/FeatureTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/FeatureTest.java b/features/core/src/test/java/org/apache/karaf/features/FeatureTest.java
index 33debdf..0cd66bb 100644
--- a/features/core/src/test/java/org/apache/karaf/features/FeatureTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/FeatureTest.java
@@ -19,14 +19,14 @@ package org.apache.karaf.features;
 import junit.framework.TestCase;
 
 public class FeatureTest extends TestCase {
-	
-	public void testValueOf() {
-		Feature feature = org.apache.karaf.features.internal.model.Feature.valueOf("name/1.0.0");
-		assertEquals(feature.getName(), "name");
-		assertEquals(feature.getVersion(), "1.0.0");
-		feature = org.apache.karaf.features.internal.model.Feature.valueOf("name");
-		assertEquals(feature.getName(), "name");
-		assertEquals(feature.getVersion(), org.apache.karaf.features.internal.model.Feature.DEFAULT_VERSION);
-	}
-	
+
+    public void testValueOf() {
+        Feature feature = org.apache.karaf.features.internal.model.Feature.valueOf("name/1.0.0");
+        assertEquals(feature.getName(), "name");
+        assertEquals(feature.getVersion(), "1.0.0");
+        feature = org.apache.karaf.features.internal.model.Feature.valueOf("name");
+        assertEquals(feature.getName(), "name");
+        assertEquals(feature.getVersion(), org.apache.karaf.features.internal.model.Feature.DEFAULT_VERSION);
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/karaf/blob/935006e6/features/core/src/test/java/org/apache/karaf/features/RepositoryTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/RepositoryTest.java b/features/core/src/test/java/org/apache/karaf/features/RepositoryTest.java
index de543cd..c630c6f 100644
--- a/features/core/src/test/java/org/apache/karaf/features/RepositoryTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/RepositoryTest.java
@@ -43,7 +43,7 @@ public class RepositoryTest extends TestCase {
         assertNotNull(features[0].getConfigurations());
         assertEquals(1, features[0].getConfigurations().size());
         assertNotNull(features[0].getConfigurations().get(0).getName());
-		assertEquals("c1", features[0].getConfigurations().get(0).getName());
+        assertEquals("c1", features[0].getConfigurations().get(0).getName());
         assertEquals(1, features[0].getConfigurations().get(0).getProperties().size());
         assertEquals("v", features[0].getConfigurations().get(0).getProperties().get("k"));
         assertNotNull(features[0].getDependencies());
@@ -85,12 +85,10 @@ public class RepositoryTest extends TestCase {
         assertEquals("f1", features[0].getName());
         assertNotNull(features[0].getConfigurations());
         assertEquals(1, features[0].getConfigurations().size());
-		assertNotNull(features[0].getConfigurations().get(0).getName());
-		assertEquals("c1", features[0].getConfigurations().get(0).getName());
-		assertEquals(1, features[0].getConfigurations().get(0).getProperties()
-				.size());
-		assertEquals("v", features[0].getConfigurations().get(0)
-				.getProperties().get("k"));
+        assertNotNull(features[0].getConfigurations().get(0).getName());
+        assertEquals("c1", features[0].getConfigurations().get(0).getName());
+        assertEquals(1, features[0].getConfigurations().get(0).getProperties().size());
+        assertEquals("v", features[0].getConfigurations().get(0).getProperties().get("k"));
         assertNotNull(features[0].getDependencies());
         assertEquals(0, features[0].getDependencies().size());
         assertNotNull(features[0].getBundles());


[02/14] karaf git commit: Fix warnings

Posted by cs...@apache.org.
Fix warnings


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/05806fdc
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/05806fdc
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/05806fdc

Branch: refs/heads/model_features
Commit: 05806fdce8294163aca5cb4b20a651230ce21516
Parents: 38c728d
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Thu Aug 10 19:15:11 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Thu Aug 10 19:15:11 2017 +0200

----------------------------------------------------------------------
 .../org/apache/karaf/features/internal/service/DeployerTest.java | 1 -
 .../karaf/features/internal/support/TestDownloadManager.java     | 4 ++--
 2 files changed, 2 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/05806fdc/features/core/src/test/java/org/apache/karaf/features/internal/service/DeployerTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/internal/service/DeployerTest.java b/features/core/src/test/java/org/apache/karaf/features/internal/service/DeployerTest.java
index 651f88e..c9cc4ae 100644
--- a/features/core/src/test/java/org/apache/karaf/features/internal/service/DeployerTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/internal/service/DeployerTest.java
@@ -471,7 +471,6 @@ public class DeployerTest {
         doTestPrereqOnPrereq(4);
     }
 
-    @SuppressWarnings("unchecked")
     private void doTestPrereqOnPrereq(int scenario) throws Exception {
         IMocksControl c = EasyMock.createControl();
 

http://git-wip-us.apache.org/repos/asf/karaf/blob/05806fdc/features/core/src/test/java/org/apache/karaf/features/internal/support/TestDownloadManager.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/internal/support/TestDownloadManager.java b/features/core/src/test/java/org/apache/karaf/features/internal/support/TestDownloadManager.java
index c78f854..550519f 100644
--- a/features/core/src/test/java/org/apache/karaf/features/internal/support/TestDownloadManager.java
+++ b/features/core/src/test/java/org/apache/karaf/features/internal/support/TestDownloadManager.java
@@ -38,10 +38,10 @@ public class TestDownloadManager implements DownloadManager, Downloader {
 
     private final MultiException exception = new MultiException("Error");
     private final ConcurrentMap<String, StreamProvider> providers = new ConcurrentHashMap<>();
-    private final Class loader;
+    private final Class<?> loader;
     private final String dir;
 
-    public TestDownloadManager(Class loader, String dir) {
+    public TestDownloadManager(Class<?> loader, String dir) {
         this.loader = loader;
         this.dir = dir;
     }


[05/14] karaf git commit: Improve test to actually test the append scenario

Posted by cs...@apache.org.
Improve test to actually test the append scenario

Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/562019cb
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/562019cb
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/562019cb

Branch: refs/heads/model_features
Commit: 562019cbd2ffea35508d2cb30c29028130bd56de
Parents: 935006e
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Fri Aug 11 11:00:11 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Fri Aug 11 11:00:11 2017 +0200

----------------------------------------------------------------------
 .../org/apache/karaf/features/AppendTest.java   | 110 +++++++++++--------
 1 file changed, 63 insertions(+), 47 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/562019cb/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/AppendTest.java b/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
index d6d5d20..662ad2d 100644
--- a/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/AppendTest.java
@@ -16,74 +16,90 @@
  */
 package org.apache.karaf.features;
 
+import static org.easymock.EasyMock.eq;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.util.Dictionary;
 import java.util.Hashtable;
 import java.util.Properties;
 
-import junit.framework.TestCase;
 import org.apache.karaf.features.internal.service.FeatureConfigInstaller;
 import org.apache.karaf.features.internal.service.RepositoryImpl;
+import org.easymock.Capture;
 import org.easymock.EasyMock;
+import org.easymock.IMocksControl;
+import org.junit.Before;
+import org.junit.Test;
+import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.service.cm.Configuration;
 import org.osgi.service.cm.ConfigurationAdmin;
 
-public class AppendTest extends TestCase {
-
-    public void testLoad() throws Exception {
+public class AppendTest {
 
+    private IMocksControl c;
+    private Feature feature;
+    private ConfigurationAdmin admin;
+    private FeatureConfigInstaller installer;
+    
+    @Before
+    public void before() throws Exception {
         System.setProperty("karaf.data", "data");
         System.setProperty("karaf.etc", "etc");
-
         RepositoryImpl r = new RepositoryImpl(getClass().getResource("internal/service/f08.xml").toURI());
-        // Check repo
         Feature[] features = r.getFeatures();
-        assertNotNull(features);
-        assertEquals(1, features.length);
-        Feature feature = features[0];
-
-        ConfigInfo configInfo = feature.getConfigurations().get(0);
-        assertNotNull(configInfo);
-        assertTrue(configInfo.isAppend());
-
-        Properties properties = configInfo.getProperties();
-        assertNotNull(properties);
-        String property = properties.getProperty("javax.servlet.context.tempdir");
-        assertNotNull(property);
-        assertFalse(property.contains("${"));
-        assertEquals(property, "data/pax-web-jsp");
+        feature = features[0];
+        checkFeature(feature);
+        c = EasyMock.createControl();
+        admin = c.createMock(ConfigurationAdmin.class);
+        installer = new FeatureConfigInstaller(admin);
+    }
 
-        ConfigurationAdmin admin = EasyMock.createMock(ConfigurationAdmin.class);
-        Configuration config = EasyMock.createMock(Configuration.class);
-        EasyMock.expect(admin.listConfigurations(EasyMock.eq("(service.pid=org.ops4j.pax.web)")))
-            .andReturn(new Configuration[] {
-                                            config
-        });
+    @Test
+    public void testNoChange() throws Exception {
         Hashtable<String, Object> original = new Hashtable<>();
-        original.put("javax.servlet.context.tempdir", "data/pax-web-jsp");
-        EasyMock.expect(config.getProperties()).andReturn(original);
+        original.put("javax.servlet.context.tempdir", "bar");
+        expectConfig(admin, original);
 
-        Hashtable<String, Object> expected = new Hashtable<>();
-        expected.put("org.ops4j.pax.web", "data/pax-web-jsp");
-        expected.put("org.apache.karaf.features.configKey", "org.ops4j.pax.web");
-        expected.put("foo", "bar");
-        EasyMock.expectLastCall();
-        EasyMock.replay(admin, config);
+        c.replay();
+        installer.installFeatureConfigs(feature);
+        c.verify();
+    }
 
-        FeatureConfigInstaller installer = new FeatureConfigInstaller(admin);
+    @Test
+    public void testAppend() throws Exception {
+        Hashtable<String, Object> original = new Hashtable<>();
+        original.put("foo", "bar");
+        Configuration config = expectConfig(admin, original);
+        Capture<Dictionary<String, ?>> captured = EasyMock.newCapture();
+        config.update(EasyMock.capture(captured));
+        expectLastCall();
+        c.replay();
         installer.installFeatureConfigs(feature);
-        EasyMock.verify(admin, config);
+        c.verify();
+        assertEquals("data/pax-web-jsp", captured.getValue().get("javax.servlet.context.tempdir"));
+    }
 
-        EasyMock.reset(admin, config);
-        EasyMock.expect(admin.listConfigurations(EasyMock.eq("(service.pid=org.ops4j.pax.web)")))
+    private Configuration expectConfig(ConfigurationAdmin admin, Hashtable<String, Object> original)
+        throws IOException, InvalidSyntaxException {
+        Configuration config = c.createMock(Configuration.class);
+        expect(admin.listConfigurations(eq("(service.pid=org.ops4j.pax.web)")))
             .andReturn(new Configuration[] {
                                             config
-        });
-        original = new Hashtable<>();
-        original.put("org.apache.karaf.features.configKey", "org.ops4j.pax.web");
-        original.put("javax.servlet.context.tempdir", "value");
-        original.put("foo", "bar");
-        EasyMock.expect(config.getProperties()).andReturn(original);
-        EasyMock.replay(admin, config);
-        installer.installFeatureConfigs(feature);
-        EasyMock.verify(admin, config);
+        }).atLeastOnce();
+        expect(config.getProperties()).andReturn(original).atLeastOnce();
+        return config;
+    }
+
+    private void checkFeature(Feature feature) {
+        ConfigInfo configInfo = feature.getConfigurations().get(0);
+        assertTrue(configInfo.isAppend());
+
+        Properties properties = configInfo.getProperties();
+        String tempDir = properties.getProperty("javax.servlet.context.tempdir");
+        assertEquals("data/pax-web-jsp", tempDir);
     }
 }


[14/14] karaf git commit: [KARAF-5300] Use FeatureReq instead of string for FeaturesService

Posted by cs...@apache.org.
[KARAF-5300] Use FeatureReq instead of string for FeaturesService


Project: http://git-wip-us.apache.org/repos/asf/karaf/repo
Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/63f42402
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/63f42402
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/63f42402

Branch: refs/heads/model_features
Commit: 63f42402392eba5d11f02afe60d375d9804f50c3
Parents: 0e1e325
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Wed Aug 9 17:39:06 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Sat Aug 12 09:12:13 2017 +0200

----------------------------------------------------------------------
 .../features/internal/service/FeatureReq.java   |  84 +++++++
 .../internal/service/FeaturesServiceImpl.java   | 226 +++++++------------
 .../service/FeaturesServiceImplTest.java        |  46 +++-
 .../karaf/features/internal/service/f09.xml     |  24 ++
 4 files changed, 229 insertions(+), 151 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/63f42402/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureReq.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureReq.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureReq.java
new file mode 100644
index 0000000..2e1f652
--- /dev/null
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeatureReq.java
@@ -0,0 +1,84 @@
+/*
+ * 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.karaf.features.internal.service;
+
+import org.apache.karaf.features.Feature;
+import org.osgi.framework.Version;
+import org.osgi.framework.VersionRange;
+
+/**
+ * Requirement for a feature
+ */
+public class FeatureReq {
+    public static final String VERSION_SEPARATOR = "/";
+    private static Version HIGHEST = new Version(Integer.MAX_VALUE, Integer.MAX_VALUE, Integer.MAX_VALUE);
+    private static final VersionRange RANGE_ALL = new VersionRange(VersionRange.LEFT_CLOSED, Version.emptyVersion, HIGHEST, VersionRange.RIGHT_CLOSED);
+    private String name;
+    private VersionRange versionRange;
+    
+    public FeatureReq(String nameAndRange) {
+        String[] parts = nameAndRange.trim().split(VERSION_SEPARATOR);
+        this.name = parts[0];
+        this.versionRange = (parts.length == 1) ? RANGE_ALL : range(parts[1]);
+    }
+    
+    public FeatureReq(String name, String versionRange) {
+        this.name = name;
+        this.versionRange = range(versionRange);
+    }
+    
+    private VersionRange range(String versionRange) {
+        if (versionRange == null) {
+            return RANGE_ALL;
+        }
+        versionRange = versionRange.trim();
+        if ("0.0.0".equals(versionRange)) {
+            return RANGE_ALL;
+        }
+        if (versionRange.contains(",")) {
+            return new VersionRange(versionRange);
+        } else {
+            return exactVersion(versionRange);
+        }
+    }
+
+    private static VersionRange exactVersion(String versionRange) {
+        return new VersionRange(VersionRange.LEFT_CLOSED, new Version(versionRange), new Version(versionRange), VersionRange.RIGHT_CLOSED);
+    }
+
+    public FeatureReq(String name, VersionRange versionRange) {
+        this.name = name;
+        this.versionRange = versionRange;
+    }
+    
+    public FeatureReq(Feature feature) {
+        this(feature.getName(), exactVersion(feature.getVersion()));
+    }
+    
+    public String getName() {
+        return name;
+    }
+    
+    public VersionRange getVersionRange() {
+        return versionRange;
+    }
+    
+    @Override
+    public String toString() {
+        return this.name + "/" + this.getVersionRange().toString();
+    }
+}

http://git-wip-us.apache.org/repos/asf/karaf/blob/63f42402/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
index 6a000cd..e44db6e 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/FeaturesServiceImpl.java
@@ -32,7 +32,6 @@ import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Hashtable;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -49,9 +48,7 @@ import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import org.apache.felix.utils.manifest.Clause;
 import org.apache.felix.utils.version.VersionCleaner;
-import org.apache.felix.utils.version.VersionRange;
 import org.apache.felix.utils.version.VersionTable;
 import org.apache.karaf.features.DeploymentEvent;
 import org.apache.karaf.features.DeploymentListener;
@@ -77,6 +74,7 @@ import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleException;
 import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.framework.Version;
+import org.osgi.framework.VersionRange;
 import org.osgi.resource.Resource;
 import org.osgi.resource.Wire;
 import org.osgi.service.cm.Configuration;
@@ -519,22 +517,22 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
 
     @Override
     public Feature[] getFeatures(String nameOrId) throws Exception {
-        String[] parts = nameOrId.split(VERSION_SEPARATOR);
-        String name = parts.length > 0 ? parts[0] : nameOrId;
-        String version = parts.length > 1 ? parts[1] : null;
-        return getFeatures(name, version);
+        return getFeatures(new FeatureReq(nameOrId));
     }
 
     @Override
     public Feature[] getFeatures(String name, String version) throws Exception {
+        return getFeatures(new FeatureReq(name, version));
+    }
+    
+    private Feature[] getFeatures(FeatureReq featureReq) throws Exception {
         List<Feature> features = new ArrayList<>();
-        Pattern pattern = Pattern.compile(name);
+        Pattern pattern = Pattern.compile(featureReq.getName());
         Map<String, Map<String, Feature>> allFeatures = getFeatureCache();
         for (String featureName : allFeatures.keySet()) {
             Matcher matcher = pattern.matcher(featureName);
             if (matcher.matches()) {
-                Map<String, Feature> versions = allFeatures.get(featureName);
-                Feature matchingFeature = getFeatureMatching(versions, version);
+                Feature matchingFeature = getFeatureMatching(featureName, featureReq.getVersionRange());
                 if (matchingFeature != null) {
                     features.add(matchingFeature);
                 }
@@ -543,35 +541,26 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         return features.toArray(new Feature[features.size()]);
     }
 
-    private Feature getFeatureMatching(Map<String, Feature> versions, String version) {
-        if (version != null) {
-            version = version.trim();
-            if (version.equals(org.apache.karaf.features.internal.model.Feature.DEFAULT_VERSION)) {
-                version = "";
-            }
-        } else {
-            version = "";
-        }
+    private Feature getFeatureMatching(String featureName, VersionRange version) throws Exception {
+        Map<String, Map<String, Feature>> allFeatures = getFeatureCache();
+        Map<String, Feature> versions = allFeatures.get(featureName);
         if (versions == null || versions.isEmpty()) {
             return null;
-        } else {
-            Feature feature = version.isEmpty() ? null : versions.get(version);
-            if (feature == null) {
-                // Compute version range. If an version has been given, assume exact range
-                VersionRange versionRange = version.isEmpty()
-                        ? new VersionRange(Version.emptyVersion)
-                        : new VersionRange(version, true, true);
-                Version latest = Version.emptyVersion;
-                for (String available : versions.keySet()) {
-                    Version availableVersion = VersionTable.getVersion(available);
-                    if (availableVersion.compareTo(latest) >= 0 && versionRange.contains(availableVersion)) {
-                        feature = versions.get(available);
-                        latest = availableVersion;
-                    }
-                }
+        }
+        return getLatestFeature(versions, version);
+    }
+
+    private Feature getLatestFeature(Map<String, Feature> versions, VersionRange versionRange) {
+        Version latest = Version.emptyVersion;
+        Feature feature = null;
+        for (String available : versions.keySet()) {
+            Version availableVersion = VersionTable.getVersion(available);
+            if (availableVersion.compareTo(latest) >= 0 && versionRange.includes(availableVersion)) {
+                feature = versions.get(available);
+                latest = availableVersion;
             }
-            return feature;
         }
+        return feature;
     }
 
     @Override
@@ -586,6 +575,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
 
     /**
      * Should not be called while holding a lock.
+     * @return map from feature name to map from feature version to Feature
      */
     protected Map<String, Map<String, Feature>> getFeatureCache() throws Exception {
         Set<String> uris;
@@ -709,7 +699,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
 
     @Override
     public boolean isRequired(Feature f) {
-        String id = FEATURE_OSGI_REQUIREMENT_PREFIX + getFeatureRequirement(f);
+        String id = FEATURE_OSGI_REQUIREMENT_PREFIX + new FeatureReq(f).toString();
         synchronized (lock) {
             Set<String> features = state.requirements.get(ROOT_REGION);
             return features != null && features.contains(id);
@@ -795,31 +785,37 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
     }
 
     @Override
-    public void installFeatures(Set<String> features, String region, EnumSet<Option> options) throws Exception {
+    public void installFeatures(Set<String> featuresIn, String region, EnumSet<Option> options) throws Exception {
+        Set<FeatureReq> featureReqs = new HashSet<>();
+        for (String feature : featuresIn) {
+            featureReqs.add(new FeatureReq(feature));
+        }
         State state = copyState();
-        Map<String, Set<String>> required = copy(state.requirements);
+        Map<String, Set<String>> requires = copy(state.requirements);
         if (region == null || region.isEmpty()) {
             region = ROOT_REGION;
         }
-        Set<String> fl = required.computeIfAbsent(region, k -> new HashSet<>());
+        Set<String> requiredForRegion = requires.computeIfAbsent(region, k -> new HashSet<>());
+        computeRequirements(options, featureReqs, requiredForRegion);
+        Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
+        doProvisionInThread(requires, stateChanges, state, getFeaturesById(), options);
+    }
+
+    void computeRequirements(EnumSet<Option> options, Set<FeatureReq> featureReqs,
+                                 Set<String> requirements)
+        throws Exception {
         Map<String, Map<String, Feature>> allFeatures = getFeatureCache();
-        List<String> featuresToAdd = new ArrayList<>();
+        List<FeatureReq> featuresToAdd = new ArrayList<>();
         List<String> featuresToRemove = new ArrayList<>();
-        for (String feature : features) {
-            if (!feature.contains(VERSION_SEPARATOR)) {
-                feature += "/0.0.0";
-            }
-            String name = feature.substring(0, feature.indexOf(VERSION_SEPARATOR));
-            String version = feature.substring(feature.indexOf(VERSION_SEPARATOR) + 1);
-            Pattern pattern = Pattern.compile(name);
+        for (FeatureReq feature : featureReqs) {
+            Pattern pattern = Pattern.compile(feature.getName());
             boolean matched = false;
             for (String fKey : allFeatures.keySet()) {
                 Matcher matcher = pattern.matcher(fKey);
                 if (matcher.matches()) {
-                    Feature f = getFeatureMatching(allFeatures.get(fKey), version);
+                    Feature f = getFeatureMatching(fKey, feature.getVersionRange());
                     if (f != null) {
-                        String req = getFeatureRequirement(f);
-                        featuresToAdd.add(req);
+                        featuresToAdd.add(new FeatureReq(f));
                         Feature[] installedFeatures = listInstalledFeatures();
                         for (Feature installedFeature : installedFeatures) {
                             if (installedFeature.getName().equals(f.getName()) && installedFeature.getVersion().equals(f.getVersion())) {
@@ -834,12 +830,11 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
                 throw new IllegalArgumentException("No matching features for " + feature);
             }
             if (options.contains(Option.Upgrade)) {
-                for (String existentFeatureReq : fl) {
-                    //remove requirement prefix feature:
-                    String existentFeature = existentFeatureReq.substring(FEATURE_OSGI_REQUIREMENT_PREFIX.length());
-                    if (existentFeature.startsWith(name + VERSION_SEPARATOR)
+                for (String existentFeatureReq : requirements) {
+                    FeatureReq existentFeature = getFeatureRefFromRequired(existentFeatureReq);
+                    if (existentFeature.getName().equals(feature.getName())
                             && !featuresToAdd.contains(existentFeature)) {
-                        featuresToRemove.add(existentFeature);
+                        featuresToRemove.add(existentFeature.toString());
                         //do not break cycle to remove all old versions of feature
                     }
                 }
@@ -848,78 +843,64 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         if (!featuresToRemove.isEmpty()) {
             print("Removing features: " + join(featuresToRemove), options.contains(Option.Verbose));
             for (String featureReq : featuresToRemove) {
-                fl.remove(FEATURE_OSGI_REQUIREMENT_PREFIX + featureReq);
+                requirements.remove(FEATURE_OSGI_REQUIREMENT_PREFIX + featureReq);
             }
         }
-        featuresToAdd = new ArrayList<>(new LinkedHashSet<>(featuresToAdd));
         List<String> featuresToDisplay = new ArrayList<>();
-        for (String feature : featuresToAdd) {
-            fl.add(FEATURE_OSGI_REQUIREMENT_PREFIX + feature);
-            String v = feature.substring(feature.indexOf(VERSION_SEPARATOR) + VERSION_SEPARATOR.length());
-            VersionRange vr = new VersionRange(v, true);
-            if (vr.isPointVersion()) {
-                v = feature.substring(0, feature.indexOf(VERSION_SEPARATOR) + VERSION_SEPARATOR.length())
-                        + vr.getCeiling().toString();
-            }
-            featuresToDisplay.add(v);
+        for (FeatureReq feature : featuresToAdd) {
+            requirements.add(FEATURE_OSGI_REQUIREMENT_PREFIX + feature.toString());
+            featuresToDisplay.add(feature.toString());
         }
         print("Adding features: " + join(featuresToDisplay), options.contains(Option.Verbose));
-        Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
-        doProvisionInThread(required, stateChanges, state, getFeaturesById(), options);
     }
 
     @Override
-    public void uninstallFeatures(Set<String> features, String region, EnumSet<Option> options) throws Exception {
+    public void uninstallFeatures(Set<String> featuresIn, String region, EnumSet<Option> options) throws Exception {
+        Set<FeatureReq> featureReqs = new HashSet<>();
+        for (String feature : featuresIn) {
+            featureReqs.add(new FeatureReq(feature));
+        }
         State state = copyState();
         Map<String, Set<String>> required = copy(state.requirements);
         if (region == null || region.isEmpty()) {
             region = ROOT_REGION;
         }
-        Set<String> fl = required.computeIfAbsent(region, k -> new HashSet<>());
-        List<String> featuresToRemove = new ArrayList<>();
-        for (String feature : new HashSet<>(features)) {
+        Set<String> existingFeatures = required.computeIfAbsent(region, k -> new HashSet<>());
+        Set<String> featuresToRemove = new HashSet<>();
+        for (FeatureReq feature : featureReqs) {
+            Pattern pattern = Pattern.compile(feature.getName());
             List<String> toRemove = new ArrayList<>();
-            feature = normalize(feature);
-            if (feature.endsWith("/0.0.0")) {
-                // Match only on name
-                String nameSep = FEATURE_OSGI_REQUIREMENT_PREFIX + feature.substring(0, feature.indexOf(VERSION_SEPARATOR) + 1);
-                for (String f : fl) {
-                    Pattern pattern = Pattern.compile(nameSep.substring(0, nameSep.length() - 1));
-                    Matcher matcher = pattern.matcher(f);
-                    if (matcher.matches() || normalize(f).startsWith(nameSep)) {
-                        toRemove.add(f);
-                    }
-                }
-            } else {
-                Pattern pattern = getNameAndVersionPattern(feature);
-                for (String f : fl) {
-                    Matcher matcher = pattern.matcher(f);
-                    if (matcher.matches()) {
-                        toRemove.add(f);
-                    }
-                }
+            for (String existingFeature : existingFeatures) {
+               FeatureReq existingFeatureReq = getFeatureRefFromRequired(existingFeature);
+               if (existingFeatureReq != null) {
+                   Matcher matcher = pattern.matcher(existingFeatureReq.getName());
+                   if (matcher.matches() && feature.getVersionRange().includes(existingFeatureReq.getVersionRange().getLeft())) {
+                       toRemove.add(existingFeature);
+                   }
+               }
             }
-            toRemove.retainAll(fl);
+            toRemove.retainAll(existingFeatures);
 
             if (toRemove.isEmpty()) {
                 throw new IllegalArgumentException("Feature named '" + feature + "' is not installed");
             }
             featuresToRemove.addAll(toRemove);
         }
-        featuresToRemove = new ArrayList<>(new LinkedHashSet<>(featuresToRemove));
         print("Removing features: " + join(featuresToRemove), options.contains(Option.Verbose));
-        fl.removeAll(featuresToRemove);
-        if (fl.isEmpty()) {
+        existingFeatures.removeAll(featuresToRemove);
+        if (existingFeatures.isEmpty()) {
             required.remove(region);
         }
         Map<String, Map<String, FeatureState>> stateChanges = Collections.emptyMap();
         doProvisionInThread(required, stateChanges, state, getFeaturesById(), options);
     }
 
-    private Pattern getNameAndVersionPattern(String feature) {
-        String name = feature.substring(0, feature.indexOf(VERSION_SEPARATOR));
-        String version = feature.substring(feature.indexOf(VERSION_SEPARATOR) + 1);
-        return getFeaturePattern(name, version);
+    private FeatureReq getFeatureRefFromRequired(String featureReq) {
+        if (!featureReq.startsWith(FEATURE_OSGI_REQUIREMENT_PREFIX)) {
+            return null;
+        }
+        String featureReq1 = featureReq.substring(FEATURE_OSGI_REQUIREMENT_PREFIX.length());
+        return new FeatureReq(featureReq1);
     }
 
     @Override
@@ -981,27 +962,6 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         return repositories.create(uri, true, true);
     }
 
-    private Map<String, Feature> loadAllFeatures(Set<URI> uris) throws Exception {
-        //the outer map's key is feature name, the inner map's key is feature version
-        Map<String, Feature> map = new HashMap<>();
-        // Two phase load:
-        // * first load dependent repositories
-        Set<URI> loaded = new HashSet<>();
-        List<URI> toLoad = new ArrayList<>(uris);
-        Clause[] blacklisted = repositories.getBlacklisted();
-        while (!toLoad.isEmpty()) {
-            URI uri = toLoad.remove(0);
-            if (loaded.add(uri)) {
-                Repository repo = new RepositoryImpl(uri, blacklisted);
-                Collections.addAll(toLoad, repo.getRepositories());
-                for (Feature f : repo.getFeatures()) {
-                    map.put(f.getId(), f);
-                }
-            }
-        }
-        return map;
-    }
-
     @Override
     public Map<String, Set<String>> listRequirements() {
         synchronized (lock) {
@@ -1227,31 +1187,9 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall
         installSupport.installLibraries(feature);
     }
 
-    private Pattern getFeaturePattern(String name, String version) {
-        String req = FEATURE_OSGI_REQUIREMENT_PREFIX + getFeatureRequirement(name, version);
-        req = req.replace("[", "\\[");
-        req = req.replace("(", "\\(");
-        req = req.replace("]", "\\]");
-        req = req.replace(")", "\\)");
-        return Pattern.compile(req);
-    }
 
-    private String getFeatureRequirement(Feature feature) {
-        return getFeatureRequirement(feature.getName(), feature.getVersion());
-    }
-
-    private String getFeatureRequirement(String name, String version) {
-        return name + VERSION_SEPARATOR + new VersionRange(version, true);
-    }
 
-    private String join(List<String> list) {
-        StringBuilder sb = new StringBuilder();
-        for (int i = 0; i < list.size(); i++) {
-            if (i > 0) {
-                sb.append(", ");
-            }
-            sb.append(list.get(i));
-        }
-        return sb.toString();
+    private String join(Collection<String> list) {
+        return String.join(", ", list);
     }
 }

http://git-wip-us.apache.org/repos/asf/karaf/blob/63f42402/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
----------------------------------------------------------------------
diff --git a/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java b/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
index a5ec75a..f5363d4 100644
--- a/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
+++ b/features/core/src/test/java/org/apache/karaf/features/internal/service/FeaturesServiceImplTest.java
@@ -125,7 +125,7 @@ public class FeaturesServiceImplTest extends TestBase {
         impl.addRepository(URI.create("custom:cycle/a-references-b.xml"));
         impl.getFeatureCache();
     }
-
+    
     @Test
     public void testRemoveRepo1() throws Exception {
         final FeaturesService featureService = createTestFeatureService();
@@ -141,6 +141,29 @@ public class FeaturesServiceImplTest extends TestBase {
     }
     
     @Test
+    public void testGetFeatureWithVersion() throws Exception {
+        final FeaturesService featureService = createTestFeatureService();
+        URI repoA = URI.create("custom:f09.xml");
+        featureService.addRepository(repoA);
+        Feature feature = featureService.getFeature("test", "1.0.0");
+        assertEquals("1.0.0", feature.getVersion());
+    }
+
+    @Test
+    public void testInstallAndUpdate() throws Exception {
+        final FeaturesService featureService = createTestFeatureService();
+        URI repoA = URI.create("custom:f09.xml");
+        featureService.addRepository(repoA);
+        Feature test100 = featureService.getFeature("test", "1.0.0");
+        installFeature(featureService, test100);
+        assertInstalled(featureService, test100);
+        Feature test110 = featureService.getFeature("test", "1.1.0");
+        featureService.installFeature(test110, EnumSet.of(Option.Upgrade));
+        waitInstalled(featureService, test110);
+        assertNotInstalled(featureService, test100);
+    }
+    
+    @Test
     public void testRemoveRepo2() throws Exception {
         final FeaturesService featureService = createTestFeatureService();
         URI repoA = URI.create("custom:remove/a.xml");
@@ -175,9 +198,8 @@ public class FeaturesServiceImplTest extends TestBase {
         FrameworkInfo dummyInfo = new FrameworkInfo();
         expect(installSupport.getInfo()).andReturn(dummyInfo).atLeastOnce();
         EasyMock.replay(installSupport);
-        final FeaturesServiceImpl featureService = new FeaturesServiceImpl(new Storage(), null, null, this.resolver,
-                                                                 installSupport, null, cfg);
-        return featureService;
+        return new FeaturesServiceImpl(new Storage(), null, null, this.resolver,
+                                       installSupport, null, cfg);
     }
 
     private void assertNotInstalled(FeaturesService featureService, Feature feature) {
@@ -190,11 +212,21 @@ public class FeaturesServiceImplTest extends TestBase {
                     featureService.isInstalled(feature));
     }
 
-    private void installFeature(final FeaturesService featureService, Feature a1Feature)
+    private void installFeature(final FeaturesService featureService, Feature feature)
         throws Exception {
-        featureService.installFeature(a1Feature, EnumSet.noneOf(Option.class));
-        while (!featureService.isInstalled(a1Feature)) {
+        featureService.installFeature(feature, EnumSet.noneOf(Option.class));
+        waitInstalled(featureService, feature);
+    }
+
+    private void waitInstalled(final FeaturesService featureService, Feature feature)
+        throws InterruptedException {
+        int count = 40;
+        while (!featureService.isInstalled(feature) && count > 0) {
             Thread.sleep(100);
+            count --;
+        }
+        if (count == 0) {
+            throw new RuntimeException("Feature " + feature + " not installed.");
         }
     }
     

http://git-wip-us.apache.org/repos/asf/karaf/blob/63f42402/features/core/src/test/resources/org/apache/karaf/features/internal/service/f09.xml
----------------------------------------------------------------------
diff --git a/features/core/src/test/resources/org/apache/karaf/features/internal/service/f09.xml b/features/core/src/test/resources/org/apache/karaf/features/internal/service/f09.xml
new file mode 100644
index 0000000..8a6c26a
--- /dev/null
+++ b/features/core/src/test/resources/org/apache/karaf/features/internal/service/f09.xml
@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+    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.
+-->
+<features name="karaf" xmlns="http://karaf.apache.org/xmlns/features/v1.2.1">
+    <feature name="test" version="1.0.0" >
+    </feature>
+    <feature name="test" version="1.1.0" >
+    </feature>
+</features>
+