-
Notifications
You must be signed in to change notification settings - Fork 343
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OSGi class loader bridge #294
OSGi class loader bridge #294
Conversation
I would love to hear some feedback from OSGi specialists here as well as from @jhalterman Please also note that I wasn't able to implement any jUnits here since I'd need a test OSGi container and some heavy setup for that. |
04e06c6
to
a1e659f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR, this PR looks good to me, just had some comments about the coding style.
I will merge this PR after the comments addressed.
* @author m.dzhigarov | ||
*/ | ||
public class BridgeClassLoaderFactory | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
coding style: public class BridgeClassLoaderFactory {
try { | ||
return additionalClassLoader.loadClass(name); | ||
} | ||
catch (ClassNotFoundException e) {} // Don't mind... Attempt next class loader |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
coding style:
try {
...
} catch (ClassNotFoundException e) {
// Don't mind... Attempt next class loader
|
} | ||
} | ||
|
||
private static Set<ClassLoader> getAllClassLoadersInTheTypeHierarchy(Set<Class< ? >> allExtendedOrImplementedTypesRecursively) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the space Set<Class< ? >>
} | ||
|
||
private static Set<ClassLoader> getAllClassLoadersInTheTypeHierarchy(Set<Class< ? >> allExtendedOrImplementedTypesRecursively) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coding style: move the curly bracket to previous line
catch (ClassNotFoundException e) {} // Don't mind... Attempt next class loader | ||
} | ||
|
||
throw new ClassNotFoundException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to have the class name of this exception!
} | ||
} | ||
|
||
private static final Map<ClassLoader, WeakReference<BridgeClassLoader>> cache = new WeakHashMap<ClassLoader, WeakReference<BridgeClassLoader>>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be UPPER case for the name of a static final variable.
Enhancer enhancer = new Enhancer(); | ||
if (useOSGiClassLoaderBridging) | ||
enhancer.setClassLoader(BridgeClassLoaderFactory.getClassLoader(type)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
two spaces for indent
a1e659f
to
2ad2b67
Compare
Just release with version v1.1.2, please take a look, thanks. |
Creating a new PR on top of my first one:
#286
The idea here is to apply the OSGi Class Loader Bridge approach that is described here:
https://www.infoq.com/articles/code-generation-with-osgi
This eliminates the need to define explicit OSGi imports to the cglib classes in user's bundles.
In addition to that, it attempts to solve the issue described here:
https://stackoverflow.com/questions/47854086/cglib-creating-class-proxy-in-osgi-results-in-noclassdeffounderror
The idea is to keep track of class loaders of class types that are found in the parent type hierarchy of the user's type. We can later use these class loaders as fallback for loading types that are otherwise unknown to the Bundle's class loader of the user's type.