Repository: karaf
Updated Branches:
  refs/heads/master fa00d0e84 -> 9de9f74b2


[KARAF-5123] Fix remove repo issue


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

Branch: refs/heads/master
Commit: 9de9f74b26497d76181b39e3a0cd5385d2c82267
Parents: efb8147
Author: Christian Schneider <[email protected]>
Authored: Mon May 29 15:40:11 2017 +0200
Committer: Christian Schneider <[email protected]>
Committed: Mon May 29 15:48:52 2017 +0200

----------------------------------------------------------------------
 .../internal/service/FeaturesServiceImpl.java   | 27 +++++++-
 .../internal/service/RepositoryCache.java       | 20 ++++++
 .../internal/service/RepositoryImpl.java        | 22 ++++++
 .../service/FeaturesServiceImplTest.java        | 71 ++++++++++++++------
 4 files changed, 117 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/9de9f74b/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 2cf14ce..5d65c03 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
@@ -379,7 +379,13 @@ public class FeaturesServiceImpl implements 
FeaturesService, Deployer.DeployCall
             return;
         }
 
-        Set<String> features = getRequiredFeatureIds(repo);
+        Set<Repository> repos = getReposToRemove(repo);
+
+        Set<String> features = new HashSet<>();
+        for (Repository tranRepo : repos) {
+            features.addAll(getRequiredFeatureIds(tranRepo));
+        }
+        
         if (!features.isEmpty()) {
             if (uninstall) {
                 uninstallFeatures(features, EnumSet.noneOf(Option.class));
@@ -401,6 +407,25 @@ public class FeaturesServiceImpl implements 
FeaturesService, Deployer.DeployCall
         callListeners(new RepositoryEvent(repo, 
RepositoryEvent.EventType.RepositoryRemoved, false));
     }
 
+    private Set<Repository> getReposToRemove(Repository repo) throws Exception 
{
+        Set<Repository> repos = repositories.tranGetRepositories(repo);
+        synchronized (lock) {
+            repos.removeAll(getRequiredRepos());
+        }
+        repos.add(repo);
+        return repos;
+    }
+    
+    private Set<Repository> getRequiredRepos() throws Exception {
+        synchronized (lock) {
+            Set<Repository> repos = new HashSet<>();
+            for (String  repoURI : state.repositories) {
+                repos.add(repositories.getRepository(URI.create(repoURI)));
+            }
+            return repos;
+        }
+    }
+
     private Set<String> getRequiredFeatureIds(Repository repo) throws 
Exception {
         synchronized (lock) {
             return Stream.of(repo.getFeatures())

http://git-wip-us.apache.org/repos/asf/karaf/blob/9de9f74b/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
index 31a39f8..73c2618 100644
--- 
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
@@ -131,4 +131,24 @@ public class RepositoryCache {
         }
     }
 
+    public Set<Repository> getRepositories(Repository repo) throws Exception {
+        HashSet<Repository> repos = new HashSet<>();
+        for (URI repoURI : repo.getRepositories()) {
+            repos.add(load(repoURI));
+        }
+        return repos;
+    }
+    
+    public Set<Repository> tranGetRepositories(Repository repo) throws 
Exception {
+        HashSet<Repository> repos = new HashSet<>();
+        repos.add(repo);
+        Set<Repository> deps = getRepositories(repo);
+        for (Repository depRepo : deps) {
+            if (!repos.contains(depRepo)) {
+                repos.addAll(tranGetRepositories(depRepo));
+            }
+        }
+        return repos;
+    }
+
 }

http://git-wip-us.apache.org/repos/asf/karaf/blob/9de9f74b/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryImpl.java
----------------------------------------------------------------------
diff --git 
a/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryImpl.java
 
b/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryImpl.java
index 1b3b561..d2032b0 100644
--- 
a/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryImpl.java
+++ 
b/features/core/src/main/java/org/apache/karaf/features/internal/service/RepositoryImpl.java
@@ -116,6 +116,28 @@ public class RepositoryImpl implements Repository {
             return super.read(b, off, len);
         }
     }
+    
+    @Override
+    public int hashCode() {
+        return uri.hashCode();
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj)
+            return true;
+        if (obj == null)
+            return false;
+        if (getClass() != obj.getClass())
+            return false;
+        RepositoryImpl other = (RepositoryImpl)obj;
+        if (uri == null) {
+            if (other.uri != null)
+                return false;
+        } else if (!uri.equals(other.uri))
+            return false;
+        return true;
+    }
 
     @Override
     public String toString() {

http://git-wip-us.apache.org/repos/asf/karaf/blob/9de9f74b/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 20b79f5..082fc21 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
@@ -27,12 +27,14 @@ import java.util.Map;
 
 import org.apache.felix.resolver.ResolverImpl;
 import org.apache.karaf.features.Feature;
+import org.apache.karaf.features.FeaturesService;
 import org.apache.karaf.features.TestBase;
 import org.apache.karaf.features.FeaturesService.Option;
 import org.apache.karaf.features.internal.resolver.Slf4jResolverLog;
 import 
org.apache.karaf.features.internal.service.BundleInstallSupport.FrameworkInfo;
 import org.easymock.EasyMock;
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.osgi.service.resolver.Resolver;
@@ -145,35 +147,60 @@ public class FeaturesServiceImplTest extends TestBase {
     }
 
     @Test
-    public void testRemoveRepo() throws Exception {
+    public void testRemoveRepo1() throws Exception {
+        final FeaturesService featureService = createTestFeatureService();
+        URI repoA = URI.create("custom:remove/a.xml");
+        featureService.addRepository(repoA);
+        Feature a1Feature = featureService.getFeature("a1");
+        installFeature(featureService, a1Feature);
+        Feature b1Feature = featureService.getFeature("b1");
+        installFeature(featureService, b1Feature);
+        featureService.removeRepository(repoA);
+        assertNotInstalled(featureService, a1Feature);
+        assertNotInstalled(featureService, b1Feature);
+    }
+    
+    @Test
+    public void testRemoveRepo2() throws Exception {
+        final FeaturesService featureService = createTestFeatureService();
+        URI repoA = URI.create("custom:remove/a.xml");
+        URI repoB = URI.create("custom:remove/b.xml");
+        featureService.addRepository(repoA);
+        featureService.addRepository(repoB);
+        Feature a1Feature = featureService.getFeature("a1");
+        installFeature(featureService, a1Feature);
+        Feature b1Feature = featureService.getFeature("b1");
+        installFeature(featureService, b1Feature);
+        featureService.removeRepository(repoA);
+        assertNotInstalled(featureService, a1Feature);
+        assertInstalled(featureService, b1Feature);
+    }
+
+    private FeaturesServiceImpl createTestFeatureService() {
         FeaturesServiceConfig cfg = new FeaturesServiceConfig();
         BundleInstallSupport installSupport = 
EasyMock.niceMock(BundleInstallSupport.class);
         FrameworkInfo dummyInfo = new FrameworkInfo();
         expect(installSupport.getInfo()).andReturn(dummyInfo).atLeastOnce();
         EasyMock.replay(installSupport);
-        final FeaturesServiceImpl impl = new FeaturesServiceImpl(new 
Storage(), null, null, this.resolver,
+        final FeaturesServiceImpl featureService = new FeaturesServiceImpl(new 
Storage(), null, null, this.resolver,
                                                                  
installSupport, null, cfg);
-        URI activemqRepo = URI.create("custom:remove/a.xml");
-        impl.addRepository(activemqRepo);
-        Feature a1Feature = impl.getFeature("a1");
-        installFeature(impl, a1Feature);
-        Feature b1Feature = impl.getFeature("b1");
-        installFeature(impl, b1Feature);
-        impl.removeRepository(activemqRepo);
-        assertFalse("a1 feature should not be installed anymore after removal 
of repo",
-                    impl.isInstalled(a1Feature));
-        /*
-        assertFalse("b1 feature should not be installed anymore after removal 
of repo",
-                    impl.isInstalled(b1Feature));
-        */
-        assertTrue("b1 feature still present -> issue KARAF-5123 is not yet 
fixed",
-                    impl.isInstalled(b1Feature));
-    }
-
-    private void installFeature(final FeaturesServiceImpl impl, Feature 
a1Feature)
+        return featureService;
+    }
+
+    private void assertNotInstalled(FeaturesService featureService, Feature 
feature) {
+        assertFalse("Feature " + feature.getName() + " should not be installed 
anymore after removal of repo",
+                    featureService.isInstalled(feature));
+    }
+    
+    private void assertInstalled(FeaturesService featureService, Feature 
feature) {
+        assertTrue("Feature " + feature.getName() + " should still be 
installed after removal of repo",
+                    featureService.isInstalled(feature));
+    }
+
+    private void installFeature(final FeaturesService featureService, Feature 
a1Feature)
         throws Exception, InterruptedException {
-        impl.installFeature(a1Feature, EnumSet.noneOf(Option.class));
-        while (!impl.isInstalled(a1Feature)) {
+        featureService.installFeature(a1Feature, EnumSet.noneOf(Option.class));
+        while (!featureService.isInstalled(a1Feature)) {
             Thread.sleep(100);
         }
     }

Reply via email to