From 7ce4e6242659ed786827570cada2a93400038f36 Mon Sep 17 00:00:00 2001 From: Christian Schneider Date: Wed, 17 May 2017 14:28:18 +0200 Subject: [PATCH] Refactoring of RepositoryCache --- .../internal/service/FeaturesServiceImpl.java | 145 ++++++------------ .../internal/service/RepositoryCache.java | 134 ++++++++++++++++ 2 files changed, 178 insertions(+), 101 deletions(-) create mode 100644 features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryCache.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 32e0bffa608..65e19ece9c7 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 @@ -46,8 +46,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import org.apache.felix.utils.manifest.Clause; -import org.apache.felix.utils.manifest.Parser; import org.apache.felix.utils.version.VersionCleaner; import org.apache.felix.utils.version.VersionRange; import org.apache.felix.utils.version.VersionTable; @@ -101,6 +99,7 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall private final Resolver resolver; private final BundleInstallSupport installSupport; private final FeaturesServiceConfig cfg; + private final RepositoryCache repositories; private final ThreadLocal outputFile = new ThreadLocal<>(); @@ -116,10 +115,13 @@ public class FeaturesServiceImpl implements FeaturesService, Deployer.DeployCall // Synchronized on lock private final Object lock = new Object(); private final State state = new State(); - private final Map repositoryCache = new HashMap<>(); + private final ExecutorService executor; + + //the outer map's key is feature name, the inner map's key is feature version private Map> featureCache; + public FeaturesServiceImpl(StateStorage storage, FeatureRepoFinder featureFinder, ConfigurationAdmin configurationAdmin, @@ -133,6 +135,7 @@ public FeaturesServiceImpl(StateStorage storage, this.resolver = resolver; this.installSupport = installSupport; this.globalRepository = globalRepository; + this.repositories = new RepositoryCache(cfg.blacklisted); this.cfg = cfg; this.executor = Executors.newSingleThreadExecutor(); loadState(); @@ -331,12 +334,6 @@ public String[] getRepositoryNames() { // Repositories support // - public Repository loadRepository(URI uri) throws Exception { - RepositoryImpl repo = new RepositoryImpl(uri, cfg.blacklisted); - repo.load(true); - return repo; - } - @Override public void validateRepository(URI uri) throws Exception { throw new UnsupportedOperationException(); @@ -349,10 +346,9 @@ public void addRepository(URI uri) throws Exception { @Override public void addRepository(URI uri, boolean install) throws Exception { - Repository repository = loadRepository(uri); + Repository repository = repositories.loadAndValidate(uri); synchronized (lock) { - // Clean cache - repositoryCache.put(uri.toString(), repository); + repositories.addRepository(repository); featureCache = null; // Add repo if (!state.repositories.add(uri.toString())) { @@ -383,13 +379,7 @@ public void removeRepository(URI uri, boolean uninstall) throws Exception { return; } - Set features; - synchronized (lock) { - features = Stream.of(repo.getFeatures()) - .filter(this::isRequired) - .map(Feature::getId) - .collect(Collectors.toSet()); - } + Set features = getRequiredFeatureIds(repo); if (!features.isEmpty()) { if (uninstall) { uninstallFeatures(features, EnumSet.noneOf(Option.class)); @@ -405,22 +395,21 @@ public void removeRepository(URI uri, boolean uninstall) throws Exception { } // Clean cache featureCache = null; - repo = repositoryCache.get(uri.toString()); - List toRemove = new ArrayList<>(); - toRemove.add(uri.toString()); - while (!toRemove.isEmpty()) { - Repository rep = repositoryCache.remove(toRemove.remove(0)); - if (rep != null) { - for (URI u : rep.getRepositories()) { - toRemove.add(u.toString()); - } - } - } + repositories.removeRepository(uri); saveState(); } callListeners(new RepositoryEvent(repo, RepositoryEvent.EventType.RepositoryRemoved, false)); } + private Set getRequiredFeatureIds(Repository repo) throws Exception { + synchronized (lock) { + return Stream.of(repo.getFeatures()) + .filter(this::isRequired) + .map(Feature::getId) + .collect(Collectors.toSet()); + } + } + @Override public void restoreRepository(URI uri) throws Exception { throw new UnsupportedOperationException(); @@ -434,60 +423,39 @@ public void refreshRepository(URI uri) throws Exception { @Override public Repository[] listRepositories() throws Exception { - // Make sure the cache is loaded - getFeatures(); + makeSureFeatureCacheLoaded(); synchronized (lock) { - return repositoryCache.values().toArray(new Repository[repositoryCache.size()]); + return repositories.listRepositories(); } } @Override public Repository[] listRequiredRepositories() throws Exception { - // Make sure the cache is loaded - getFeatures(); + makeSureFeatureCacheLoaded(); synchronized (lock) { - List repos = new ArrayList<>(); - for (Map.Entry entry : repositoryCache.entrySet()) { - if (state.repositories.contains(entry.getKey())) { - repos.add(entry.getValue()); - } - } - return repos.toArray(new Repository[repos.size()]); + return repositories.listRequiredRepositories(state.repositories); } } @Override public Repository getRepository(String name) throws Exception { - // Make sure the cache is loaded - getFeatures(); + makeSureFeatureCacheLoaded(); synchronized (lock) { - for (Repository repo : this.repositoryCache.values()) { - if (name.equals(repo.getName())) { - return repo; - } - } - return null; + return repositories.getRepository(name); } } @Override public Repository getRepository(URI uri) throws Exception { - // Make sure the cache is loaded - getFeatures(); + makeSureFeatureCacheLoaded(); synchronized (lock) { - for (Repository repo : this.repositoryCache.values()) { - if (repo.getURI().equals(uri)) { - return repo; - } - } - return null; + return repositories.getRepository(uri); } } @Override public String getRepositoryName(URI uri) throws Exception { - Repository repo = getRepository(uri); - return (repo != null) ? repo.getName() : null; + return repositories.getRepositoryName(uri); } // @@ -576,6 +544,10 @@ public Feature[] listFeatures() throws Exception { Map> allFeatures = getFeatures(); return flattenFeatures(allFeatures); } + + private void makeSureFeatureCacheLoaded() throws Exception { + getFeatures(); + } /** * Should not be called while holding a lock. @@ -588,44 +560,20 @@ protected Map> getFeatures() throws Exception { } uris = new TreeSet<>(state.repositories); } - //the outer map's key is feature name, the inner map's key is feature version - Map> map = new HashMap<>(); - // Load blacklist - Set blacklistStrings = Blacklist.loadBlacklist(cfg.blacklisted); - Clause[] blacklist = Parser.parseClauses(blacklistStrings.toArray(new String[blacklistStrings.size()])); - // Two phase load: - // * first load dependent repositories - Set loaded = new HashSet<>(); - List toLoad = new ArrayList<>(uris); - while (!toLoad.isEmpty()) { - String uri = toLoad.remove(0); - Repository repo; - synchronized (lock) { - repo = repositoryCache.get(uri); - } - try { - if (repo == null) { - RepositoryImpl rep = new RepositoryImpl(URI.create(uri), blacklist); - rep.load(); - repo = rep; - synchronized (lock) { - repositoryCache.put(uri, repo); - } - } - if (loaded.add(uri)) { - for (URI u : repo.getRepositories()) { - toLoad.add(u.toString()); - } - } - } catch (Exception e) { - LOGGER.warn("Can't load features repository {}", uri, e); - } - } - List repos; + repositories.loadDependent(uris); + Map> map = createFeatureCacheInternal(); synchronized (lock) { - repos = new ArrayList<>(repositoryCache.values()); + if (uris.equals(state.repositories)) { + // Only update cache if list of repositories was not changed in the mean time + featureCache = map; + } } - // * then load all features + return map; + } + + private Map> createFeatureCacheInternal() throws Exception { + Map> map = new HashMap<>(); + Repository[] repos= repositories.listRepositories(); for (Repository repo : repos) { for (Feature f : repo.getFeatures()) { if (map.get(f.getName()) == null) { @@ -637,11 +585,6 @@ protected Map> getFeatures() throws Exception { } } } - synchronized (lock) { - if (uris.equals(state.repositories)) { - featureCache = map; - } - } return map; } diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryCache.java b/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryCache.java new file mode 100644 index 00000000000..31a39f8639f --- /dev/null +++ b/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryCache.java @@ -0,0 +1,134 @@ +/* + * 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 java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.apache.felix.utils.manifest.Clause; +import org.apache.felix.utils.manifest.Parser; +import org.apache.karaf.features.Repository; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class RepositoryCache { + private static final Logger LOGGER = LoggerFactory.getLogger(RepositoryCache.class); + private final Map repositoryCache = new HashMap<>(); + private final Clause[] blacklisted; + + public RepositoryCache(String blacklisted) { + this.blacklisted = loadBlacklist(blacklisted); + } + + private Clause[] loadBlacklist(String blacklisted) { + Set blacklistStrings = Blacklist.loadBlacklist(blacklisted); + return Parser.parseClauses(blacklistStrings.toArray(new String[blacklistStrings.size()])); + } + + public Repository load(URI uri) throws Exception { + return new RepositoryImpl(uri, blacklisted); + } + + public Repository loadAndValidate(URI uri) throws Exception { + RepositoryImpl repo = new RepositoryImpl(uri, blacklisted); + repo.load(true); + return repo; + } + + public synchronized void addRepository(Repository repository) throws Exception { + String repoUriSt = repository.getURI().toString(); + repositoryCache.put(repoUriSt, repository); + } + + public synchronized void removeRepository(URI repositoryUri) throws Exception { + List toRemove = new ArrayList<>(); + toRemove.add(repositoryUri.toString()); + while (!toRemove.isEmpty()) { + Repository rep = repositoryCache.remove(toRemove.remove(0)); + if (rep != null) { + for (URI u : rep.getRepositories()) { + toRemove.add(u.toString()); + } + } + } + } + + public synchronized Repository[] listRepositories() { + return repositoryCache.values().toArray(new Repository[repositoryCache.size()]); + } + + public synchronized Repository[] listRequiredRepositories(Set topLevelRepoUris) throws Exception { + List repos = new ArrayList<>(); + for (Map.Entry entry : repositoryCache.entrySet()) { + if (topLevelRepoUris.contains(entry.getKey())) { + repos.add(entry.getValue()); + } + } + return repos.toArray(new Repository[repos.size()]); + } + + public synchronized Repository getRepository(String name) throws Exception { + for (Repository repo : this.repositoryCache.values()) { + if (name.equals(repo.getName())) { + return repo; + } + } + return null; + } + + public synchronized Repository getRepository(URI uri) throws Exception { + for (Repository repo : this.repositoryCache.values()) { + if (repo.getURI().equals(uri)) { + return repo; + } + } + return null; + } + + public String getRepositoryName(URI uri) throws Exception { + Repository repo = getRepository(uri); + return (repo != null) ? repo.getName() : null; + } + + public synchronized void loadDependent(Set topLevelRepoUris) { + Set loaded = new HashSet<>(); + List toLoad = new ArrayList<>(topLevelRepoUris); + while (!toLoad.isEmpty()) { + String uri = toLoad.remove(0); + Repository repo = repositoryCache.get(uri); + try { + if (repo == null) { + repo = load(URI.create(uri)); + repositoryCache.put(uri, repo); + } + if (loaded.add(uri)) { + for (URI u : repo.getRepositories()) { + toLoad.add(u.toString()); + } + } + } catch (Exception e) { + LOGGER.warn("Can't load features repository {}", uri, e); + } + } + } + +}