You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@karaf.apache.org by gn...@apache.org on 2014/03/21 14:42:50 UTC

git commit: [KARAF-2839] Conditional features are not verified correctly after installing / uninstalling a feature

Repository: karaf
Updated Branches:
  refs/heads/karaf-3.0.x 8ca2f8274 -> bb934617d


[KARAF-2839] Conditional features are not verified correctly after installing / uninstalling a feature


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

Branch: refs/heads/karaf-3.0.x
Commit: bb934617de39c485f6dedb94f43617fc060a85cd
Parents: 8ca2f82
Author: Guillaume Nodet <gn...@gmail.com>
Authored: Fri Mar 21 13:53:46 2014 +0100
Committer: Guillaume Nodet <gn...@gmail.com>
Committed: Fri Mar 21 14:42:24 2014 +0100

----------------------------------------------------------------------
 .../features/internal/FeaturesServiceImpl.java  | 69 ++++++++++++++------
 .../karaf/itests/ConditionalFeaturesTest.java   |  4 ++
 2 files changed, 52 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/karaf/blob/bb934617/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java
----------------------------------------------------------------------
diff --git a/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java b/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java
index 70663d6..19fa01c 100644
--- a/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java
+++ b/features/core/src/main/java/org/apache/karaf/features/internal/FeaturesServiceImpl.java
@@ -391,20 +391,14 @@ public class FeaturesServiceImpl implements FeaturesService {
             	try {
                     doInstallFeature(s, f, verbose);
                     doInstallFeatureConditionals(s, f, verbose);
-                    state.bundleInfos.putAll(s.bundleInfos);
-                    state.bundles.addAll(s.bundles);
-                    state.features.putAll(s.features);
-                    state.installed.addAll(s.installed);
-                    state.bundleStartLevels.putAll(s.bundleStartLevels);
-
                     //Check if current feature satisfies the conditionals of existing features
                     for (Feature installedFeature : listInstalledFeatures()) {
-                        for (Conditional conditional : installedFeature.getConditional()) {
-                            if (dependenciesSatisfied(conditional.getCondition(), state)) {
-                                doInstallFeatureConditionals(s, installedFeature, verbose);
-                            }
-                        }
+                        doInstallFeatureConditionals(s, installedFeature, verbose);
+                    }
+                    for (Feature installedFeature : state.features.keySet()) {
+                        doInstallFeatureConditionals(s, installedFeature, verbose);
                     }
+
                     state.bundleInfos.putAll(s.bundleInfos);
                     state.bundles.addAll(s.bundles);
                     state.features.putAll(s.features);
@@ -558,16 +552,19 @@ public class FeaturesServiceImpl implements FeaturesService {
 
     protected void doInstallFeatureConditionals(InstallationState state, Feature feature,  boolean verbose) throws Exception {
         //Check conditions of the current feature.
-        for (Conditional conditional : feature.getConditional()) {
+        feature = getFeature(feature.getName(), feature.getVersion());
+        if (feature != null) {
+            for (Conditional conditional : feature.getConditional()) {
 
-            if (dependenciesSatisfied(conditional.getCondition(), state)) {
-                InstallationState s = new InstallationState();
-                doInstallFeature(s, conditional.asFeature(feature.getName(), feature.getVersion()), verbose);
-                state.bundleInfos.putAll(s.bundleInfos);
-                state.bundles.addAll(s.bundles);
-                state.features.putAll(s.features);
-                state.installed.addAll(s.installed);
-                state.bundleStartLevels.putAll(s.bundleStartLevels);
+                if (dependenciesSatisfied(conditional.getCondition(), state)) {
+                    InstallationState s = new InstallationState();
+                    doInstallFeature(s, conditional.asFeature(feature.getName(), feature.getVersion()), verbose);
+                    state.bundleInfos.putAll(s.bundleInfos);
+                    state.bundles.addAll(s.bundles);
+                    state.features.putAll(s.features);
+                    state.installed.addAll(s.installed);
+                    state.bundleStartLevels.putAll(s.bundleStartLevels);
+                }
             }
         }
     }
@@ -665,8 +662,10 @@ public class FeaturesServiceImpl implements FeaturesService {
         }
         boolean verbose = options != null && options.contains(Option.Verbose);
         boolean refresh = options == null || !options.contains(Option.NoAutoRefreshBundles);
+        String msg = "Uninstalling feature " + feature.getName() + " " + feature.getVersion();
+        LOGGER.info(msg);
         if (verbose) {
-            System.out.println("Uninstalling feature " + feature.getName() + " " + feature.getVersion());
+            System.out.println(msg);
         }
         // Grab all the bundles installed by this feature
         // and remove all those who will still be in use.
@@ -677,11 +676,39 @@ public class FeaturesServiceImpl implements FeaturesService {
         for (Conditional conditional : feature.getConditional()) {
             Feature conditionalFeature = conditional.asFeature(feature.getName(),feature.getVersion());
             if (installed.containsKey(conditionalFeature)) {
+                msg = "Uninstalling feature " + conditionalFeature.getName() + " " + conditionalFeature.getVersion();
+                LOGGER.info(msg);
+                if (verbose) {
+                    System.out.println(msg);
+                }
             	bundles.addAll(installed.remove(conditionalFeature));
             } else {
             	LOGGER.info("Conditional feature {}, hasn't been installed!");
             }
         }
+        for (Feature f : new HashSet<Feature>(installed.keySet())) {
+            f = getFeature(f.getName(), f.getVersion());
+            if (f != null) {
+                for (Conditional conditional : f.getConditional()) {
+                    boolean satisfied = true;
+                    for (Dependency dep : conditional.getCondition()) {
+                        Feature df = getFeatureForDependency(dep);
+                        satisfied &= installed.containsKey(df);
+                    }
+                    if (!satisfied) {
+                        Feature conditionalFeature = conditional.asFeature(f.getName(), f.getVersion());
+                        if (installed.containsKey(conditionalFeature)) {
+                            msg = "Uninstalling feature " + conditionalFeature.getName() + " " + conditionalFeature.getVersion();
+                            LOGGER.info(msg);
+                            if (verbose) {
+                                System.out.println(msg);
+                            }
+                            bundles.addAll(installed.remove(conditionalFeature));
+                        }
+                    }
+                }
+            }
+        }
 
         for (Set<Long> b : installed.values()) {
             bundles.removeAll(b);

http://git-wip-us.apache.org/repos/asf/karaf/blob/bb934617/itests/src/test/java/org/apache/karaf/itests/ConditionalFeaturesTest.java
----------------------------------------------------------------------
diff --git a/itests/src/test/java/org/apache/karaf/itests/ConditionalFeaturesTest.java b/itests/src/test/java/org/apache/karaf/itests/ConditionalFeaturesTest.java
index b747518..46908d1 100644
--- a/itests/src/test/java/org/apache/karaf/itests/ConditionalFeaturesTest.java
+++ b/itests/src/test/java/org/apache/karaf/itests/ConditionalFeaturesTest.java
@@ -89,5 +89,9 @@ public class ConditionalFeaturesTest extends KarafTestSupport {
           //ignore as the eventadmin activator might throw an error.
         }
         assertBundleInstalled("org.apache.felix.webconsole.plugins.event");
+
+        //Remove eventadmin
+        featureService.uninstallFeature("eventadmin");
+        assertBundleNotInstalled("org.apache.felix.webconsole.plugins.event");
     }
 }