[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/3b900267 Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/3b900267 Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/3b900267 Branch: refs/heads/model_features Commit: 3b900267c4710302198cd66c02a869e1b74ff0f0 Parents: 8f94e29 Author: Christian Schneider <ch...@die-schneider.net> Authored: Wed Aug 9 17:39:06 2017 +0200 Committer: Christian Schneider <ch...@die-schneider.net> Committed: Thu Aug 17 16:29:30 2017 +0200 ---------------------------------------------------------------------- .../features/internal/service/FeatureReq.java | 84 ++++++++ .../internal/service/FeaturesServiceImpl.java | 204 ++++++++----------- .../service/FeaturesServiceImplTest.java | 46 ++++- .../karaf/features/internal/service/f09.xml | 24 +++ 4 files changed, 229 insertions(+), 129 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/karaf/blob/3b900267/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/3b900267/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 27ccf97..5562390 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; @@ -50,7 +49,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; 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; @@ -76,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 +518,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 +542,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 +576,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 +700,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 +786,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 +831,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 +844,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 @@ -1206,31 +1188,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/3b900267/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/3b900267/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> +