Merge pull request #927 from iocanel/jclouds-osgi-fixes

Jclouds osgi fixes

core/pom.xml

@@ -39,7 +39,10 @@
     </scm>
 
     <properties>
-        <jclouds.osgi.import>*</jclouds.osgi.import>
+        <jclouds.osgi.import>
+          org.nnsoft.guice.rocoto*;version="[6.1,7)",
+          *
+        </jclouds.osgi.import>
         <jclouds.osgi.export>org.jclouds*;version=${project.version};-noimport:=true</jclouds.osgi.export>
         <jclouds.osgi.activator>org.jclouds.osgi.Activator</jclouds.osgi.activator>
     </properties>

core/src/main/java/org/jclouds/osgi/MetadataBundleListener.java

@@ -19,7 +19,10 @@
 package org.jclouds.osgi;
 
 import com.google.common.base.Charsets;
-
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Multimap;
+import com.google.common.io.Closeables;
 import org.jclouds.apis.ApiMetadata;
 import org.jclouds.apis.ApiRegistry;
 import org.jclouds.providers.ProviderMetadata;
@@ -33,8 +36,8 @@ import java.io.BufferedReader;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.net.URL;
-import java.util.HashMap;
+import java.util.Collection;
-import java.util.Map;
+import java.util.List;
 
 /**
  * A {@link BundleListener} that listens for {@link BundleEvent} and searches for {@link org.jclouds.providers.ProviderMetadata} and {@link org.jclouds.apis.ApiMetadata} in newly
@@ -43,21 +46,25 @@ import java.util.Map;
  */
 public class MetadataBundleListener implements BundleListener {
 
-  private Map<Long, ProviderMetadata> providerMetadataMap = new HashMap<Long, ProviderMetadata>();
+   private Multimap<Long, ProviderMetadata> providerMetadataMap = ArrayListMultimap.create();
-  private Map<Long, ApiMetadata> apiMetadataMap = new HashMap<Long, ApiMetadata>();
+   private Multimap<Long, ApiMetadata> apiMetadataMap = ArrayListMultimap.create();
 
 
    public void start(BundleContext bundleContext) {
       bundleContext.addBundleListener(this);
       for (Bundle bundle : bundleContext.getBundles()) {
          if (bundle.getState() == Bundle.ACTIVE) {
-        ProviderMetadata providerMetadata = getProviderMetadata(bundle);
+            List<ProviderMetadata> providerMetadataList = getProviderMetadata(bundle);
-        ApiMetadata apiMetadata = getApiMetadata(bundle);
+            List<ApiMetadata> apiMetadataList = getApiMetadata(bundle);
 
+            for (ProviderMetadata providerMetadata : providerMetadataList) {
                if (providerMetadata != null) {
                   ProviderRegistry.registerProvider(providerMetadata);
                   providerMetadataMap.put(bundle.getBundleId(), providerMetadata);
                }
+            }
+
+            for (ApiMetadata apiMetadata : apiMetadataList) {
                if (apiMetadata != null) {
                   ApiRegistry.registerApi(apiMetadata);
                   apiMetadataMap.put(bundle.getBundleId(), apiMetadata);
@@ -65,6 +72,7 @@ public class MetadataBundleListener implements BundleListener {
             }
          }
       }
+   }
 
    public void stop(BundleContext bundleContext) {
       providerMetadataMap.clear();
@@ -73,31 +81,41 @@ public class MetadataBundleListener implements BundleListener {
 
    @Override
    public void bundleChanged(BundleEvent event) {
-    ProviderMetadata providerMetadata;
+      Collection<ProviderMetadata> providerMetadataList = null;
-    ApiMetadata apiMetadata;
+      Collection<ApiMetadata> apiMetadataList = null;
       switch (event.getType()) {
          case BundleEvent.STARTED:
-        providerMetadata = getProviderMetadata(event.getBundle());
+            providerMetadataList = getProviderMetadata(event.getBundle());
-        apiMetadata = getApiMetadata(event.getBundle());
+            apiMetadataList = getApiMetadata(event.getBundle());
+            for (ProviderMetadata providerMetadata : providerMetadataList) {
                if (providerMetadata != null) {
                   ProviderRegistry.registerProvider(providerMetadata);
                   providerMetadataMap.put(event.getBundle().getBundleId(), providerMetadata);
                }
+            }
+
+            for (ApiMetadata apiMetadata : apiMetadataList) {
                if (apiMetadata != null) {
                   ApiRegistry.registerApi(apiMetadata);
                   apiMetadataMap.put(event.getBundle().getBundleId(), apiMetadata);
                }
+            }
             break;
          case BundleEvent.STOPPING:
          case BundleEvent.STOPPED:
-        providerMetadata = providerMetadataMap.get(event.getBundle().getBundleId());
+            providerMetadataList = providerMetadataMap.get(event.getBundle().getBundleId());
-        apiMetadata = apiMetadataMap.get(event.getBundle().getBundleId());
+            apiMetadataList = apiMetadataMap.get(event.getBundle().getBundleId());
-        if (providerMetadata != null) {
+
+            if (providerMetadataList != null) {
+               for (ProviderMetadata providerMetadata : providerMetadataList) {
                   ProviderRegistry.unregisterProvider(providerMetadata);
                }
-        if (apiMetadata != null) {
+            }
+            if (apiMetadataList != null) {
+               for (ApiMetadata apiMetadata : apiMetadataList) {
                   ApiRegistry.unRegisterApi(apiMetadata);
                }
+            }
             break;
       }
    }
@@ -108,13 +126,18 @@ public class MetadataBundleListener implements BundleListener {
     * @param bundle
     * @return
     */
-  public ProviderMetadata getProviderMetadata(Bundle bundle) {
+   public List<ProviderMetadata> getProviderMetadata(Bundle bundle) {
-    ProviderMetadata metadata = null;
+      List<ProviderMetadata> metadataList = Lists.newArrayList();
-    String className = getProviderMetadataClassName(bundle);
+      String classNames = getProviderMetadataClassNames(bundle);
-    if (className != null && !className.isEmpty()) {
+      if (classNames != null && !classNames.isEmpty()) {
+         for (String className : classNames.split("\n")) {
             try {
                Class<? extends ProviderMetadata> providerMetadataClass = bundle.loadClass(className);
-        metadata = providerMetadataClass.newInstance();
+               //Classes loaded by other class loaders are not assignable.
+               if (ProviderMetadata.class.isAssignableFrom(providerMetadataClass)) {
+                  ProviderMetadata metadata = providerMetadataClass.newInstance();
+                  metadataList.add(metadata);
+               }
             } catch (ClassNotFoundException e) {
                // ignore
             } catch (InstantiationException e) {
@@ -123,7 +146,8 @@ public class MetadataBundleListener implements BundleListener {
                // ignore
             }
          }
-    return metadata;
+      }
+      return metadataList;
    }
 
    /**
@@ -132,13 +156,18 @@ public class MetadataBundleListener implements BundleListener {
     * @param bundle
     * @return
     */
-  public ApiMetadata getApiMetadata(Bundle bundle) {
+   public List<ApiMetadata> getApiMetadata(Bundle bundle) {
-    ApiMetadata metadata = null;
+      List<ApiMetadata> metadataList = Lists.newArrayList();
-    String className = getApiMetadataClassName(bundle);
+      String classNames = getApiMetadataClassNames(bundle);
-    if (className != null && !className.isEmpty()) {
+      if (classNames != null && !classNames.isEmpty()) {
+         for (String className : classNames.split("\n")) {
             try {
                Class<? extends ApiMetadata> apiMetadataClass = bundle.loadClass(className);
-        metadata = apiMetadataClass.newInstance();
+               //Classes loaded by other class loaders are not assignable.
+               if (ApiMetadata.class.isAssignableFrom(apiMetadataClass)) {
+                  ApiMetadata metadata = apiMetadataClass.newInstance();
+                  metadataList.add(metadata);
+               }
             } catch (ClassNotFoundException e) {
                // ignore
             } catch (InstantiationException e) {
@@ -147,11 +176,12 @@ public class MetadataBundleListener implements BundleListener {
                // ignore
             }
          }
-    return metadata;
+      }
+      return metadataList;
    }
 
 
-  public String getMetadataClassName(Bundle bundle, String pathToMetadata) {
+   public String getMetadataClassNames(Bundle bundle, String pathToMetadata) {
       URL resource = bundle.getEntry(pathToMetadata);
       InputStream is = null;
       InputStreamReader reader = null;
@@ -168,21 +198,9 @@ public class MetadataBundleListener implements BundleListener {
          }
       } catch (Throwable e) {
       } finally {
-      try {
+         Closeables.closeQuietly(reader);
-        if (reader != null)
+         Closeables.closeQuietly(bufferedReader);
-          reader.close();
+         Closeables.closeQuietly(is);
-      } catch (Throwable e) {
-      }
-      try {
-        if (bufferedReader != null)
-          bufferedReader.close();
-      } catch (Throwable e) {
-      }
-      try {
-        is.close();
-      } catch (Throwable e) {
-      }
-
       }
       return sb.toString().trim();
    }
@@ -193,8 +211,8 @@ public class MetadataBundleListener implements BundleListener {
     * @param bundle
     * @return
     */
-  public String getProviderMetadataClassName(Bundle bundle) {
+   public String getProviderMetadataClassNames(Bundle bundle) {
-    return getMetadataClassName(bundle, "/META-INF/services/org.jclouds.providers.ProviderMetadata");
+      return getMetadataClassNames(bundle, "/META-INF/services/org.jclouds.providers.ProviderMetadata");
    }
 
    /**
@@ -203,8 +221,8 @@ public class MetadataBundleListener implements BundleListener {
     * @param bundle
     * @return
     */
-  public String getApiMetadataClassName(Bundle bundle) {
+   public String getApiMetadataClassNames(Bundle bundle) {
-    return getMetadataClassName(bundle, "/META-INF/services/org.jclouds.apis.ApiMetadata");
+      return getMetadataClassNames(bundle, "/META-INF/services/org.jclouds.apis.ApiMetadata");
    }
 
 }

core/src/test/java/org/jclouds/osgi/MetadataBundleListenerTest.java

@@ -0,0 +1,147 @@
+/**
+ * Licensed to jclouds, Inc. (jclouds) under one or more
+ * contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  jclouds 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.jclouds.osgi;
+
+import org.jclouds.apis.ApiMetadata;
+import org.jclouds.apis.JcloudsTestBlobStoreApiMetadata;
+import org.jclouds.apis.JcloudsTestComputeApiMetadata;
+import org.jclouds.apis.JcloudsTestYetAnotherComputeApiMetadata;
+import org.jclouds.providers.JcloudsTestBlobStoreProviderMetadata;
+import org.jclouds.providers.JcloudsTestComputeProviderMetadata;
+import org.jclouds.providers.JcloudsTestYetAnotherComputeProviderMetadata;
+import org.jclouds.providers.ProviderMetadata;
+import org.osgi.framework.Bundle;
+import org.testng.annotations.Test;
+
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.List;
+
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.assertNotNull;
+
+
+public class MetadataBundleListenerTest {
+
+   @Test
+   public void testSanity() throws MalformedURLException, ClassNotFoundException {
+      //We are checking here that the class loader we create and use in this test series is indeed different and isolated from our tests classloader.
+      ClassLoader loader = createIsolatedClassLoader();
+      assertFalse(ProviderMetadata.class.isAssignableFrom(loader.loadClass("org.jclouds.providers.JcloudsTestComputeProviderMetadata")));
+   }
+
+
+   @Test
+   public void testGetProviderMetadata() throws Exception {
+      MetadataBundleListener listener = new MetadataBundleListener();
+      Bundle bundle = createMock(Bundle.class);
+      expect(bundle.getEntry("/META-INF/services/org.jclouds.providers.ProviderMetadata")).andReturn(getClass().getResource("/META-INF/services/org.jclouds.providers.ProviderMetadata")).anyTimes();
+      expect(bundle.loadClass("org.jclouds.providers.JcloudsTestBlobStoreProviderMetadata")).andReturn(JcloudsTestBlobStoreProviderMetadata.class).anyTimes();
+      expect(bundle.loadClass("org.jclouds.providers.JcloudsTestComputeProviderMetadata")).andReturn(JcloudsTestComputeProviderMetadata.class).anyTimes();
+      expect(bundle.loadClass("org.jclouds.providers.JcloudsTestYetAnotherComputeProviderMetadata")).andReturn(JcloudsTestYetAnotherComputeProviderMetadata.class).anyTimes();
+
+      replay(bundle);
+      List<ProviderMetadata> providerMetadataList = listener.getProviderMetadata(bundle);
+      assertNotNull(providerMetadataList);
+      assertEquals(3, providerMetadataList.size());
+      assertTrue(providerMetadataList.contains(new JcloudsTestBlobStoreProviderMetadata()));
+      assertTrue(providerMetadataList.contains(new JcloudsTestComputeProviderMetadata()));
+      assertTrue(providerMetadataList.contains(new JcloudsTestYetAnotherComputeProviderMetadata()));
+      verify(bundle);
+   }
+
+   @Test
+   public void testGetProviderMetadataFromMultipleClassLoaders() throws Exception {
+      ClassLoader isolatedClassLoader = createIsolatedClassLoader();
+      MetadataBundleListener listener = new MetadataBundleListener();
+      Bundle bundle = createMock(Bundle.class);
+      expect(bundle.getEntry("/META-INF/services/org.jclouds.providers.ProviderMetadata")).andReturn(getClass().getResource("/META-INF/services/org.jclouds.providers.ProviderMetadata")).anyTimes();
+      expect(bundle.loadClass("org.jclouds.providers.JcloudsTestBlobStoreProviderMetadata")).andReturn(isolatedClassLoader.loadClass(JcloudsTestBlobStoreProviderMetadata.class.getName())).anyTimes();
+      expect(bundle.loadClass("org.jclouds.providers.JcloudsTestComputeProviderMetadata")).andReturn(JcloudsTestComputeProviderMetadata.class).anyTimes();
+      expect(bundle.loadClass("org.jclouds.providers.JcloudsTestYetAnotherComputeProviderMetadata")).andReturn(JcloudsTestYetAnotherComputeProviderMetadata.class).anyTimes();
+
+      replay(bundle);
+      List<ProviderMetadata> providerMetadataList = listener.getProviderMetadata(bundle);
+      assertNotNull(providerMetadataList);
+      assertEquals(2, providerMetadataList.size());
+      assertFalse(providerMetadataList.contains(new JcloudsTestBlobStoreProviderMetadata()));
+      assertTrue(providerMetadataList.contains(new JcloudsTestComputeProviderMetadata()));
+      assertTrue(providerMetadataList.contains(new JcloudsTestYetAnotherComputeProviderMetadata()));
+      verify(bundle);
+   }
+
+   @Test
+   public void testGetApiMetadata() throws Exception {
+      MetadataBundleListener listener = new MetadataBundleListener();
+      Bundle bundle = createMock(Bundle.class);
+      expect(bundle.getEntry("/META-INF/services/org.jclouds.apis.ApiMetadata")).andReturn(getClass().getResource("/META-INF/services/org.jclouds.apis.ApiMetadata")).anyTimes();
+      expect(bundle.loadClass("org.jclouds.apis.JcloudsTestBlobStoreApiMetadata")).andReturn(JcloudsTestBlobStoreApiMetadata.class).anyTimes();
+      expect(bundle.loadClass("org.jclouds.apis.JcloudsTestComputeApiMetadata")).andReturn(JcloudsTestComputeApiMetadata.class).anyTimes();
+      expect(bundle.loadClass("org.jclouds.apis.JcloudsTestYetAnotherComputeApiMetadata")).andReturn(JcloudsTestYetAnotherComputeApiMetadata.class).anyTimes();
+
+      replay(bundle);
+      List<ApiMetadata> apiMetadataList = listener.getApiMetadata(bundle);
+      assertNotNull(apiMetadataList);
+      assertEquals(3, apiMetadataList.size());
+      assertTrue(apiMetadataList.contains(new JcloudsTestBlobStoreApiMetadata()));
+      assertTrue(apiMetadataList.contains(new JcloudsTestComputeApiMetadata()));
+      assertTrue(apiMetadataList.contains(new JcloudsTestYetAnotherComputeApiMetadata()));
+      verify(bundle);
+   }
+
+   @Test
+   public void testGetApiMetadataFromMultipleClassLoaders() throws Exception {
+      ClassLoader isolatedClassLoader = createIsolatedClassLoader();
+      MetadataBundleListener listener = new MetadataBundleListener();
+      Bundle bundle = createMock(Bundle.class);
+      expect(bundle.getEntry("/META-INF/services/org.jclouds.apis.ApiMetadata")).andReturn(getClass().getResource("/META-INF/services/org.jclouds.apis.ApiMetadata")).anyTimes();
+      expect(bundle.loadClass("org.jclouds.apis.JcloudsTestBlobStoreApiMetadata")).andReturn(isolatedClassLoader.loadClass(JcloudsTestBlobStoreApiMetadata.class.getName())).anyTimes();
+      expect(bundle.loadClass("org.jclouds.apis.JcloudsTestComputeApiMetadata")).andReturn(JcloudsTestComputeApiMetadata.class).anyTimes();
+      expect(bundle.loadClass("org.jclouds.apis.JcloudsTestYetAnotherComputeApiMetadata")).andReturn(JcloudsTestYetAnotherComputeApiMetadata.class).anyTimes();
+
+      replay(bundle);
+      List<ApiMetadata> apiMetadataList = listener.getApiMetadata(bundle);
+      assertNotNull(apiMetadataList);
+      assertEquals(2, apiMetadataList.size());
+      assertFalse(apiMetadataList.contains(new JcloudsTestBlobStoreApiMetadata()));
+      assertTrue(apiMetadataList.contains(new JcloudsTestComputeApiMetadata()));
+      assertTrue(apiMetadataList.contains(new JcloudsTestYetAnotherComputeApiMetadata()));
+      verify(bundle);
+   }
+
+
+   /**
+    * Creates a different {@link ClassLoader}.
+    *
+    * @return
+    */
+   private ClassLoader createIsolatedClassLoader() throws MalformedURLException {
+      URLClassLoader testClassLoader = (URLClassLoader) getClass().getClassLoader();
+      URL[] urls = testClassLoader.getURLs();
+      URLClassLoader loader = new URLClassLoader(urls, null);
+      return loader;
+   }
+}

drivers/netty/pom.xml

@@ -35,7 +35,11 @@
 
     <properties>
         <jclouds.osgi.export>org.jclouds.netty*;version="${project.version}"</jclouds.osgi.export>
-        <jclouds.osgi.import>org.jclouds*;version="${project.version}",*</jclouds.osgi.import>
+        <jclouds.osgi.import>
+          org.jclouds*;version="${project.version}",
+          org.jboss.netty*;version="[3.3,4)",
+          *
+        </jclouds.osgi.import>
     </properties>
 
     <dependencies>

drivers/slf4j/pom.xml

@@ -35,7 +35,11 @@
 
     <properties>
         <jclouds.osgi.export>org.jclouds.logging.slf4j*;version="${project.version}"</jclouds.osgi.export>
-        <jclouds.osgi.import>org.jclouds*;version="${project.version}",*</jclouds.osgi.import>
+        <jclouds.osgi.import>
+          org.jclouds*;version="${project.version}",
+          org.slf4j*;version="[1.5,2)",
+          *
+        </jclouds.osgi.import>
     </properties>
 
     <!-- bootstrapping: need to fetch the project POM -->