brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sjcorb...@apache.org
Subject [1/2] incubator-brooklyn git commit: Fail if user specified symbolicName and version don't match bundle
Date Wed, 14 Jan 2015 19:04:26 GMT
Repository: incubator-brooklyn
Updated Branches:
  refs/heads/master b67d58479 -> 869825804


Fail if user specified symbolicName and version don't match bundle

Bundles are always matched by their symbolicName and version, not by the user specified alternatives,
so allowing them to diverge will lead to unexpected behaviour. So far the logic was to fail
only if the bundle is already installed which was inconsistent with first-time install when
it would pass.


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

Branch: refs/heads/master
Commit: e6ec2e9c364d4ce56d5766204dd426b6e920d167
Parents: b2474e5
Author: Svetoslav Neykov <svetoslav.neykov@cloudsoftcorp.com>
Authored: Tue Jan 13 10:48:33 2015 +0200
Committer: Svetoslav Neykov <svetoslav.neykov@cloudsoftcorp.com>
Committed: Tue Jan 13 10:48:33 2015 +0200

----------------------------------------------------------------------
 .../brooklyn/management/ha/OsgiManager.java     | 44 +++++++-------------
 .../camp/lite/test-app-service-blueprint.yaml   |  2 +-
 .../brooklyn/catalog/CatalogYamlEntityTest.java | 24 ++++-------
 3 files changed, 24 insertions(+), 46 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e6ec2e9c/core/src/main/java/brooklyn/management/ha/OsgiManager.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/management/ha/OsgiManager.java b/core/src/main/java/brooklyn/management/ha/OsgiManager.java
index 2e79d43..a3cbba2 100644
--- a/core/src/main/java/brooklyn/management/ha/OsgiManager.java
+++ b/core/src/main/java/brooklyn/management/ha/OsgiManager.java
@@ -44,7 +44,6 @@ import brooklyn.util.os.Os;
 import brooklyn.util.os.Os.DeletionResult;
 import brooklyn.util.osgi.Osgis;
 import brooklyn.util.osgi.Osgis.BundleFinder;
-import brooklyn.util.osgi.Osgis.VersionedName;
 import brooklyn.util.repeat.Repeater;
 import brooklyn.util.time.Duration;
 
@@ -63,10 +62,6 @@ public class OsgiManager {
     protected Framework framework;
     protected File osgiCacheDir;
 
-    // we could manage without this map but it is useful to validate what is a user-supplied
url
-    protected Map<String,VersionedName> urlToBundleIdentifier = MutableMap.of();
-
-
     public OsgiManager(ManagementContext mgmt) {
         this.mgmt = mgmt;
     }
@@ -125,8 +120,6 @@ public class OsgiManager {
             Bundle b = Osgis.install(framework, bundle.getUrl());
 
             checkCorrectlyInstalled(bundle, b);
-
-            urlToBundleIdentifier.put(bundle.getUrl(), new VersionedName(b));
         } catch (BundleException e) {
             log.debug("Bundle from "+bundle+" failed to install (rethrowing): "+e);
             throw Throwables.propagate(e);
@@ -136,11 +129,8 @@ public class OsgiManager {
     private void checkCorrectlyInstalled(CatalogBundle bundle, Bundle b) {
         String nv = b.getSymbolicName()+":"+b.getVersion().toString();
 
-        if (bundle.isNamed() &&
-                (!bundle.getSymbolicName().equals(b.getSymbolicName()) ||
-                !bundle.getVersion().equals(b.getVersion().toString()))) {
-            log.warn("Bundle at " + bundle.getUrl() + " installed as " + nv +
-                    " but user explicitly requested " + bundle.getSymbolicName() + ":" +
bundle.getVersion());
+        if (!isBundleNameEqualOrAbsent(bundle, b)) {
+            throw new IllegalStateException("Bundle from "+bundle.getUrl()+" already installed
as "+nv+" but user explicitly requested "+bundle);
         }
 
         List<Bundle> matches = Osgis.bundleFinder(framework)
@@ -162,24 +152,16 @@ public class OsgiManager {
     private boolean checkBundleInstalledThrowIfInconsistent(CatalogBundle bundle) {
         String bundleUrl = bundle.getUrl();
         if (bundleUrl != null) {
-            VersionedName nv = urlToBundleIdentifier.get(bundleUrl);
-            if (nv!=null) {
-                if (bundle.isNamed() && !nv.equals(bundle.getSymbolicName(), bundle.getVersion()))
{
-                    throw new IllegalStateException("Bundle from "+bundleUrl+" already installed
as "+nv+" but user explicitly requested "+bundle);
-                }
-                Maybe<Bundle> installedBundle = Osgis.bundleFinder(framework).requiringFromUrl(bundleUrl).find();
-                if (installedBundle.isPresent()) {
-                    if (bundle.isNamed()) {
-                        Bundle b = installedBundle.get();
-                        if (!nv.equals(b.getSymbolicName(), b.getVersion().toString())) {
-                            log.error("Bundle from "+bundleUrl+" already installed as "+nv+"
but reports "+b.getSymbolicName()+":"+b.getVersion());
-                        }
-                    }
-                    log.trace("Bundle from "+bundleUrl+" already installed as "+nv+"; not
re-registering");
-                    return true;
+            Maybe<Bundle> installedBundle = Osgis.bundleFinder(framework).requiringFromUrl(bundleUrl).find();
+            if (installedBundle.isPresent()) {
+                Bundle b = installedBundle.get();
+                String nv = b.getSymbolicName()+":"+b.getVersion().toString();
+                if (!isBundleNameEqualOrAbsent(bundle, b)) {
+                    throw new IllegalStateException("Bundle from "+bundle.getUrl()+" already
installed as "+nv+" but user explicitly requested "+bundle);
                 } else {
-                    log.error("Bundle "+nv+" from "+bundleUrl+" is known in map but not installed;
perhaps in the process of installing?");
+                    log.trace("Bundle from "+bundleUrl+" already installed as "+nv+"; not
re-registering");
                 }
+                return true;
             }
         } else {
             Maybe<Bundle> installedBundle = Osgis.bundleFinder(framework).symbolicName(bundle.getSymbolicName()).version(bundle.getVersion()).find();
@@ -193,6 +175,12 @@ public class OsgiManager {
         return false;
     }
 
+    public static boolean isBundleNameEqualOrAbsent(CatalogBundle bundle, Bundle b) {
+        return !bundle.isNamed() ||
+                (bundle.getSymbolicName().equals(b.getSymbolicName()) &&
+                bundle.getVersion().equals(b.getVersion().toString()));
+    }
+
     public <T> Maybe<Class<T>> tryResolveClass(String type, CatalogBundle...
catalogBundles) {
         return tryResolveClass(type, Arrays.asList(catalogBundles));
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e6ec2e9c/core/src/test/resources/brooklyn/camp/lite/test-app-service-blueprint.yaml
----------------------------------------------------------------------
diff --git a/core/src/test/resources/brooklyn/camp/lite/test-app-service-blueprint.yaml b/core/src/test/resources/brooklyn/camp/lite/test-app-service-blueprint.yaml
index 846f299..c0bb607 100644
--- a/core/src/test/resources/brooklyn/camp/lite/test-app-service-blueprint.yaml
+++ b/core/src/test/resources/brooklyn/camp/lite/test-app-service-blueprint.yaml
@@ -33,6 +33,6 @@ brooklyn.catalog:
   type: io.camp.mock.MyApplication
   version: 0.9
   libraries:
-  - name: lib1
+  - name: org.apache.brooklyn.test.resources.osgi.brooklyn-test-osgi-entities
     version: 0.1.0
     url: classpath:/brooklyn/osgi/brooklyn-test-osgi-entities.jar
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/e6ec2e9c/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
----------------------------------------------------------------------
diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
index cb804a5..f569b19 100644
--- a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
+++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java
@@ -297,37 +297,27 @@ public class CatalogYamlEntityTest extends AbstractYamlTest {
         TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH);
 
         String firstItemId = "my.catalog.app.id.register_bundle";
-        String secondItemId = "my.catalog.app.id.reference_bundle";
         String nonExistentId = "non_existent_id";
         String nonExistentVersion = "9.9.9";
-        addCatalogItem(
-            "brooklyn.catalog:",
-            "  id: " + firstItemId,
-            "  version: " + TEST_VERSION,
-            "  libraries:",
-            "  - name: " + nonExistentId,
-            "    version: " + nonExistentVersion,
-            "    url: " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL,
-            "",
-            "services:",
-            "- type: " + SIMPLE_ENTITY_TYPE);
-        deleteCatalogEntity(firstItemId);
-
         try {
             addCatalogItem(
                 "brooklyn.catalog:",
-                "  id: " + secondItemId,
+                "  id: " + firstItemId,
                 "  version: " + TEST_VERSION,
                 "  libraries:",
                 "  - name: " + nonExistentId,
                 "    version: " + nonExistentVersion,
+                "    url: " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL,
                 "",
                 "services:",
                 "- type: " + SIMPLE_ENTITY_TYPE);
             fail();
         } catch (IllegalStateException e) {
-            assertEquals(e.getMessage(), "Bundle CatalogBundleDto{symbolicName=" + nonExistentId
+ ", version=" + nonExistentVersion + ", url=null} " +
-                    "not previously registered, but URL is empty.");
+            assertEquals(e.getMessage(), "Bundle from " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL
+ " already " +
+                    "installed as " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_NAME
+ ":" +
+                    OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_VERSION + " but user explicitly
requested " +
+                    "CatalogBundleDto{symbolicName=" + nonExistentId + ", version=" + nonExistentVersion
+ ", url=" +
+                    OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL + "}");
         }
     }
     


Mime
View raw message