Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/90#discussion_r15458083
--- Diff:
core/src/main/java/brooklyn/entity/proxying/InternalEntityFactory.java ---
@@ -139,15 +138,16 @@ public
InternalEntityFactory(ManagementContextInternal managementContext, Entity
}
builder.addAll(spec.getAdditionalInterfaces());
Set<Class<?>> interfaces = builder.build();
-
- // TODO OSGi strangeness! The classloader obtained from the type
should be enough.
- // If an OSGi class loader, it should delegate to find things like
Entity.class etc.
- // However, we get errors such as:
+
+ // When using the entity's classloader, we get errors such as:
// NoClassDefFoundError: brooklyn.event.AttributeSensor not
found by io.brooklyn.brooklyn-test-osgi-entities
// Building our own aggregating class loader gets around this.
- // But we really should not have to do this! What are the
consequences?
+ //
+ // The reason for the error is that the proxy tries to load all
classes
+ // referenced from the entity and its interfaces with the single
passed loader
+ // while a normal class loading would nest the class loaders
(loading interfaces'
+ // references with their own class loaders which in our case are
different).
AggregateClassLoader aggregateClassLoader =
AggregateClassLoader.newInstanceWithNoLoaders();
- aggregateClassLoader.addFirst(classloader);
--- End diff --
Looking at the code I didn't find a case where spec.getImplementation() !=
enttiy.getClass() - the entity is always instantiated from getImplementation()
if available.
Event if that's the case and the classes (and possibly the classloaders)
differ why would we want to use the classloader of a class which didn't
instantiate the entity instance?
Looking at this code again I think that it should be improved but in
another direction - adding the classloaders of all extended classes and
interfaces recursively, not only of the explicitly specified interfaces. WDYT?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
