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/09 15:39:29 UTC

[1/2] karaf git commit: Extract common code in test

Repository: karaf
Updated Branches:
  refs/heads/model_features [created] 54dac91a8


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/728dcbee
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/728dcbee
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/728dcbee

Branch: refs/heads/model_features
Commit: 728dcbeeaddf16d10c1d8a53599a364250239b7f
Parents: b65801b
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Wed Aug 9 15:51:45 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Wed Aug 9 15:51:45 2017 +0200

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


http://git-wip-us.apache.org/repos/asf/karaf/blob/728dcbee/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);


[2/2] 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/54dac91a
Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/54dac91a
Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/54dac91a

Branch: refs/heads/model_features
Commit: 54dac91a87e1f1f076bddc9556146d3bd42f0d5f
Parents: 728dcbe
Author: Christian Schneider <ch...@die-schneider.net>
Authored: Wed Aug 9 17:39:06 2017 +0200
Committer: Christian Schneider <ch...@die-schneider.net>
Committed: Wed Aug 9 17:39:06 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/54dac91a/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/54dac91a/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/54dac91a/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/54dac91a/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>
+