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 <[email protected]> Authored: Tue Jan 13 10:48:33 2015 +0200 Committer: Svetoslav Neykov <[email protected]> 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 + "}"); } }
