Repository: karaf
Updated Branches:
  refs/heads/master 4174fb78a -> 1e0631669


Refactoring of RepositoryCache


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

Branch: refs/heads/master
Commit: 1e0631669294d00d5c9a8e4fecaacb61e07544c0
Parents: e80bfa7
Author: Christian Schneider <[email protected]>
Authored: Wed May 17 14:28:18 2017 +0200
Committer: Christian Schneider <[email protected]>
Committed: Thu May 18 11:37:09 2017 +0200

----------------------------------------------------------------------
 .../internal/service/FeaturesServiceImpl.java   | 145 ++++++-------------
 .../internal/service/RepositoryCache.java       | 134 +++++++++++++++++
 2 files changed, 178 insertions(+), 101 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/1e063166/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 32e0bff..65e19ec 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.regex.Pattern;
 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<String> 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<String, Repository> 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<String, Map<String, Feature>> featureCache;
 
+
     public FeaturesServiceImpl(StateStorage storage,
                                FeatureRepoFinder featureFinder,
                                ConfigurationAdmin configurationAdmin,
@@ -133,6 +135,7 @@ public class FeaturesServiceImpl implements 
FeaturesService, Deployer.DeployCall
         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 class FeaturesServiceImpl implements 
FeaturesService, Deployer.DeployCall
     // 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 class FeaturesServiceImpl implements 
FeaturesService, Deployer.DeployCall
 
     @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 class FeaturesServiceImpl implements 
FeaturesService, Deployer.DeployCall
             return;
         }
 
-        Set<String> features;
-        synchronized (lock) {
-            features = Stream.of(repo.getFeatures())
-                    .filter(this::isRequired)
-                    .map(Feature::getId)
-                    .collect(Collectors.toSet());
-        }
+        Set<String> features = getRequiredFeatureIds(repo);
         if (!features.isEmpty()) {
             if (uninstall) {
                 uninstallFeatures(features, EnumSet.noneOf(Option.class));
@@ -405,22 +395,21 @@ public class FeaturesServiceImpl implements 
FeaturesService, Deployer.DeployCall
             }
             // Clean cache
             featureCache = null;
-            repo = repositoryCache.get(uri.toString());
-            List<String> 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<String> 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 class FeaturesServiceImpl implements 
FeaturesService, Deployer.DeployCall
 
     @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<Repository> repos = new ArrayList<>();
-            for (Map.Entry<String, Repository> 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 class FeaturesServiceImpl implements 
FeaturesService, Deployer.DeployCall
         Map<String, Map<String, Feature>> allFeatures = getFeatures();
         return flattenFeatures(allFeatures);
     }
+    
+    private void makeSureFeatureCacheLoaded() throws Exception {
+        getFeatures();
+    }
 
     /**
      * Should not be called while holding a lock.
@@ -588,44 +560,20 @@ public class FeaturesServiceImpl implements 
FeaturesService, Deployer.DeployCall
             }
             uris = new TreeSet<>(state.repositories);
         }
-        //the outer map's key is feature name, the inner map's key is feature 
version
-        Map<String, Map<String, Feature>> map = new HashMap<>();
-        // Load blacklist
-        Set<String> blacklistStrings = 
Blacklist.loadBlacklist(cfg.blacklisted);
-        Clause[] blacklist = Parser.parseClauses(blacklistStrings.toArray(new 
String[blacklistStrings.size()]));
-        // Two phase load:
-        // * first load dependent repositories
-        Set<String> loaded = new HashSet<>();
-        List<String> 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<Repository> repos;
+        repositories.loadDependent(uris);
+        Map<String, Map<String, Feature>> 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<String, Map<String, Feature>> createFeatureCacheInternal() 
throws Exception {
+        Map<String, Map<String, Feature>> 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 @@ public class FeaturesServiceImpl implements 
FeaturesService, Deployer.DeployCall
                 }
             }
         }
-        synchronized (lock) {
-            if (uris.equals(state.repositories)) {
-                featureCache = map;
-            }
-        }
         return map;
     }
 

http://git-wip-us.apache.org/repos/asf/karaf/blob/1e063166/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/RepositoryCache.java
 
b/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryCache.java
new file mode 100644
index 0000000..31a39f8
--- /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<String, Repository> repositoryCache = new HashMap<>();
+    private final Clause[] blacklisted;
+    
+    public RepositoryCache(String blacklisted) {
+        this.blacklisted = loadBlacklist(blacklisted);
+    }
+    
+    private Clause[] loadBlacklist(String blacklisted) {
+        Set<String> 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<String> 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<String> 
topLevelRepoUris) throws Exception {
+        List<Repository> repos = new ArrayList<>();
+        for (Map.Entry<String, Repository> 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<String> topLevelRepoUris) {
+        Set<String> loaded = new HashSet<>();
+        List<String> 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);
+            }
+        }
+    }
+
+}

Reply via email to