Hi Sergey, Please see my comment inline ------------- Freeman(Yue) Fang
Red Hat, Inc. FuseSource is now part of Red Hat On 2014-12-3, at 下午7:03, Sergey Beryozkin wrote: > Hi Freeman > > Thanks for this effort, I did a minor update to avoid updating all of > ClasspathScanner client code (JAX-RS server & client for now - but possibly > JAXWS Spring parsers in the future too). > Dan may likely have more comments but for now I have a couple of questions, > - is FrameworkUtil.getBundle useful ? That returns a CXF bundle, right ? It is useful as a fallback to get a Bundle, it will return cxf-core bundle so that we can create an OsgiBundleResourcePatternResolver instance to try to find the resources > > - should this Spring DM code be pushed to a dedicated SpringOsgiUtil to > minimize the risk of some JVMs failing with some eager resolution class > loading exceptions in non-OSGI cases ? +1 for this proposal, I can put the spring dm code into a separated Util class to avoid such concern. Cheers > > Cheers, Sergey > > > On 03/12/14 05:06, [email protected] wrote: >> Repository: cxf >> Updated Branches: >> refs/heads/master 76c4df1bc -> 0de0309f2 >> >> >> [CXF-6131]JAX-RS resource spring auto-discovery doesn't work in OSGi >> >> >> Project: http://git-wip-us.apache.org/repos/asf/cxf/repo >> Commit: http://git-wip-us.apache.org/repos/asf/cxf/commit/0de0309f >> Tree: http://git-wip-us.apache.org/repos/asf/cxf/tree/0de0309f >> Diff: http://git-wip-us.apache.org/repos/asf/cxf/diff/0de0309f >> >> Branch: refs/heads/master >> Commit: 0de0309f28e1543660c763c158f4353d6d2189be >> Parents: 76c4df1 >> Author: Freeman Fang <[email protected]> >> Authored: Wed Dec 3 13:06:42 2014 +0800 >> Committer: Freeman Fang <[email protected]> >> Committed: Wed Dec 3 13:06:42 2014 +0800 >> >> ---------------------------------------------------------------------- >> core/pom.xml | 20 ++++++++++++++++++- >> .../org/apache/cxf/bus/osgi/CXFActivator.java | 13 ++++++++++++ >> .../cxf/common/util/SpringClasspathScanner.java | 21 ++++++++++++++++++-- >> parent/pom.xml | 11 ++++++++++ >> .../JAXRSServerFactoryBeanDefinitionParser.java | 12 +++++++++-- >> 5 files changed, 72 insertions(+), 5 deletions(-) >> ---------------------------------------------------------------------- >> >> >> http://git-wip-us.apache.org/repos/asf/cxf/blob/0de0309f/core/pom.xml >> ---------------------------------------------------------------------- >> diff --git a/core/pom.xml b/core/pom.xml >> index 83ec6f8..d7e8f7a 100644 >> --- a/core/pom.xml >> +++ b/core/pom.xml >> @@ -44,7 +44,13 @@ >> >> org.slf4j*;resolution:=optional;version="${cxf.osgi.slf4j.version}", >> org.apache.log4j*;resolution:=optional, >> >> net.sf.cglib*;resolution:=optional;version="${cxf.cglib.osgi.version}", >> - >> org.springframework*;resolution:=optional;version="${cxf.osgi.spring.version}", >> + >> org.springframework.osgi.io;resolution:=optional;version="${cxf.osgi.spring.osgi.version}", >> + >> org.springframework.osgi.util;resolution:=optional;version="${cxf.osgi.spring.osgi.version}", >> + >> org.springframework.aop*;resolution:=optional;version="${cxf.osgi.spring.version}", >> + >> org.springframework.beans*;resolution:=optional;version="${cxf.osgi.spring.version}", >> + >> org.springframework.context*;resolution:=optional;version="${cxf.osgi.spring.version}", >> + >> org.springframework.core*;resolution:=optional;version="${cxf.osgi.spring.version}", >> + >> org.springframework.util*;resolution:=optional;version="${cxf.osgi.spring.version}", >> >> org.objectweb.asm*;resolution:=optional;version="${cxf.osgi.asm.version}", >> javax.activation;version="${cxf.osgi.javax.activation.version}", >> javax.annotation;version="${cxf.osgi.javax.annotation.version}", >> @@ -205,6 +211,18 @@ >> <optional>true</optional> >> <scope>provided</scope> >> </dependency> >> + <dependency> >> + <groupId>org.springframework.osgi</groupId> >> + <artifactId>spring-osgi-io</artifactId> >> + <optional>true</optional> >> + <scope>provided</scope> >> + </dependency> >> + <dependency> >> + <groupId>org.springframework.osgi</groupId> >> + <artifactId>spring-osgi-core</artifactId> >> + <optional>true</optional> >> + <scope>provided</scope> >> + </dependency> >> </dependencies> >> <build> >> <plugins> >> >> http://git-wip-us.apache.org/repos/asf/cxf/blob/0de0309f/core/src/main/java/org/apache/cxf/bus/osgi/CXFActivator.java >> ---------------------------------------------------------------------- >> diff --git a/core/src/main/java/org/apache/cxf/bus/osgi/CXFActivator.java >> b/core/src/main/java/org/apache/cxf/bus/osgi/CXFActivator.java >> index 3fed8a8..cb9cce1 100644 >> --- a/core/src/main/java/org/apache/cxf/bus/osgi/CXFActivator.java >> +++ b/core/src/main/java/org/apache/cxf/bus/osgi/CXFActivator.java >> @@ -45,14 +45,23 @@ import org.osgi.util.tracker.ServiceTracker; >> * - Blueprint namespaces >> */ >> public class CXFActivator implements BundleActivator { >> + >> + /* >> + * a flag to tell if run the CXF in OSGi cont >> + */ >> + private static boolean inOSGi; >> + >> private List<Extension> extensions; >> private ManagedWorkQueueList workQueues = new ManagedWorkQueueList(); >> private ServiceTracker configAdminTracker; >> private CXFExtensionBundleListener cxfBundleListener; >> private ServiceRegistration workQueueServiceRegistration; >> + >> + >> >> /** {@inheritDoc}*/ >> public void start(BundleContext context) throws Exception { >> + inOSGi = true; >> cxfBundleListener = new >> CXFExtensionBundleListener(context.getBundle().getBundleId()); >> context.addBundleListener(cxfBundleListener); >> cxfBundleListener.registerExistingBundles(context); >> @@ -123,5 +132,9 @@ public class CXFActivator implements BundleActivator { >> configAdminTracker.close(); >> ExtensionRegistry.removeExtensions(extensions); >> } >> + >> + public static boolean isInOSGi() { >> + return inOSGi; >> + } >> >> } >> >> http://git-wip-us.apache.org/repos/asf/cxf/blob/0de0309f/core/src/main/java/org/apache/cxf/common/util/SpringClasspathScanner.java >> ---------------------------------------------------------------------- >> diff --git >> a/core/src/main/java/org/apache/cxf/common/util/SpringClasspathScanner.java >> b/core/src/main/java/org/apache/cxf/common/util/SpringClasspathScanner.java >> index 769d1f0..ce4dd7f 100644 >> --- >> a/core/src/main/java/org/apache/cxf/common/util/SpringClasspathScanner.java >> +++ >> b/core/src/main/java/org/apache/cxf/common/util/SpringClasspathScanner.java >> @@ -28,7 +28,10 @@ import java.util.HashSet; >> import java.util.List; >> import java.util.Map; >> >> +import org.apache.cxf.bus.osgi.CXFActivator; >> import org.apache.cxf.common.classloader.ClassLoaderUtils; >> +import org.osgi.framework.Bundle; >> +import org.osgi.framework.FrameworkUtil; >> import org.springframework.core.io.Resource; >> import >> org.springframework.core.io.support.PathMatchingResourcePatternResolver; >> import org.springframework.core.io.support.ResourcePatternResolver; >> @@ -36,6 +39,8 @@ import org.springframework.core.type.AnnotationMetadata; >> import >> org.springframework.core.type.classreading.CachingMetadataReaderFactory; >> import org.springframework.core.type.classreading.MetadataReader; >> import org.springframework.core.type.classreading.MetadataReaderFactory; >> +import org.springframework.osgi.io.OsgiBundleResourcePatternResolver; >> +import org.springframework.osgi.util.BundleDelegatingClassLoader; >> import org.springframework.util.ClassUtils; >> >> class SpringClasspathScanner extends ClasspathScanner { >> @@ -156,8 +161,20 @@ class SpringClasspathScanner extends ClasspathScanner { >> } >> >> private ResourcePatternResolver getResolver(ClassLoader loader) { >> - return loader != null >> - ? new PathMatchingResourcePatternResolver(loader) : new >> PathMatchingResourcePatternResolver(); >> + if (CXFActivator.isInOSGi()) { >> + //in OSGi should use spring-dm OsgiBundleResourcePatternResolver >> + // which can handle bundle url >> + Bundle bundle = null; >> + if (loader instanceof BundleDelegatingClassLoader) { >> + bundle = ((BundleDelegatingClassLoader)loader).getBundle(); >> + } else { >> + bundle = >> FrameworkUtil.getBundle(SpringClasspathScanner.class); >> + } >> + return new OsgiBundleResourcePatternResolver(bundle); >> + } else { >> + return loader != null >> + ? new PathMatchingResourcePatternResolver(loader) : new >> PathMatchingResourcePatternResolver(); >> + } >> } >> >> private boolean shouldSkip(final String classname) { >> >> http://git-wip-us.apache.org/repos/asf/cxf/blob/0de0309f/parent/pom.xml >> ---------------------------------------------------------------------- >> diff --git a/parent/pom.xml b/parent/pom.xml >> index cc4cbaa..7122127 100644 >> --- a/parent/pom.xml >> +++ b/parent/pom.xml >> @@ -56,6 +56,7 @@ >> >> <cxf.osgi.symbolic.name>${project.groupId}.${project.artifactId}</cxf.osgi.symbolic.name> >> <cxf.osgi.dynamic.import /> >> <cxf.osgi.spring.version>[3.1,5)</cxf.osgi.spring.version> >> + >> <cxf.osgi.spring.osgi.version>[1.2.0,2)</cxf.osgi.spring.osgi.version> >> >> <cxf.osgi.commons.lang.version>[2.4,3)</cxf.osgi.commons.lang.version> >> <cxf.osgi.slf4j.version>[1.5,2)</cxf.osgi.slf4j.version> >> >> <cxf.osgi.javax.annotation.version>[0.0,2)</cxf.osgi.javax.annotation.version> >> @@ -1296,6 +1297,16 @@ >> </exclusions> >> </dependency> >> <dependency> >> + <groupId>org.springframework.osgi</groupId> >> + <artifactId>spring-osgi-io</artifactId> >> + <version>${cxf.spring.osgi.version}</version> >> + </dependency> >> + <dependency> >> + <groupId>org.springframework.osgi</groupId> >> + <artifactId>spring-osgi-core</artifactId> >> + <version>${cxf.spring.osgi.version}</version> >> + </dependency> >> + <dependency> >> <groupId>commons-codec</groupId> >> <artifactId>commons-codec</artifactId> >> <version>${cxf.commons-codec.version}</version> >> >> http://git-wip-us.apache.org/repos/asf/cxf/blob/0de0309f/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/spring/JAXRSServerFactoryBeanDefinitionParser.java >> ---------------------------------------------------------------------- >> diff --git >> a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/spring/JAXRSServerFactoryBeanDefinitionParser.java >> >> b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/spring/JAXRSServerFactoryBeanDefinitionParser.java >> index 40b6286..e17bd1a 100644 >> --- >> a/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/spring/JAXRSServerFactoryBeanDefinitionParser.java >> +++ >> b/rt/frontend/jaxrs/src/main/java/org/apache/cxf/jaxrs/spring/JAXRSServerFactoryBeanDefinitionParser.java >> @@ -21,6 +21,7 @@ package org.apache.cxf.jaxrs.spring; >> import java.io.IOException; >> import java.lang.annotation.Annotation; >> import java.util.ArrayList; >> +import java.util.Arrays; >> import java.util.Collection; >> import java.util.List; >> import java.util.Map; >> @@ -31,6 +32,7 @@ import javax.xml.namespace.QName; >> >> import org.w3c.dom.Element; >> >> +import org.apache.cxf.bus.osgi.CXFActivator; >> import org.apache.cxf.bus.spring.BusWiringBeanFactoryPostProcessor; >> import org.apache.cxf.common.util.ClasspathScanner; >> import org.apache.cxf.common.util.StringUtils; >> @@ -192,8 +194,14 @@ public class JAXRSServerFactoryBeanDefinitionParser >> extends AbstractBeanDefiniti >> >> try { >> if (basePackages != null) { >> - final Map< Class< ? extends Annotation >, Collection< >> Class< ? > > > classes = >> - ClasspathScanner.findClasses(basePackages, >> Provider.class, Path.class); >> + //if run CXF in OSGi, we should pass in the classloader >> associated with >> + //the bundle which has JAXRS resources under a certain >> basePackages >> + ClassLoader loader = >> Thread.currentThread().getContextClassLoader(); >> + final Map< Class< ? extends Annotation >, Collection< >> Class< ? > > > classes = >> + CXFActivator.isInOSGi() >> + ? ClasspathScanner.findClasses( >> + basePackages, >> Arrays.asList(Provider.class, Path.class), loader) >> + : ClasspathScanner.findClasses(basePackages, >> Provider.class, Path.class); >> >> >> this.setProviders(createBeans(classes.get(Provider.class))); >> >> this.setServiceBeans(createBeans(classes.get(Path.class))); >> > > > --
