Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/683#discussion_r32011188
--- Diff: core/src/main/java/brooklyn/entity/rebind/RebindIteration.java ---
@@ -899,48 +914,64 @@ protected void setCatalogItemId(BrooklynObject item,
String catalogItemId) {
}
}
- protected <T extends BrooklynObject> Class<? extends T>
load(Class<T> bType, Memento memento) {
+ protected <T extends BrooklynObject> LoadedClass<? extends T>
load(Class<T> bType, Memento memento) {
return load(bType, memento.getType(),
memento.getCatalogItemId(), memento.getId());
}
+
@SuppressWarnings("unchecked")
- protected <T extends BrooklynObject> Class<? extends T>
load(Class<T> bType, String jType, String catalogItemId, String
contextSuchAsId) {
+ protected <T extends BrooklynObject> LoadedClass<? extends T>
load(Class<T> bType, String jType, String catalogItemId, String
contextSuchAsId) {
checkNotNull(jType, "Type of %s (%s) must not be null",
contextSuchAsId, bType.getSimpleName());
+
if (catalogItemId != null) {
- BrooklynClassLoadingContext loader =
getLoadingContextFromCatalogItemId(catalogItemId, classLoader, rebindContext);
- return loader.loadClass(jType, bType);
- } else {
- // we have previously used reflections; not sure if that's
needed?
- try {
- return (Class<T>)reflections.loadClass(jType);
- } catch (Exception e) {
- LOG.warn("Unable to load "+jType+" using reflections;
will try standard context");
- }
-
- if
(BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_INFER_CATALOG_ITEM_ON_REBIND))
{
- //Try loading from whichever catalog bundle succeeds.
- BrooklynCatalog catalog =
managementContext.getCatalog();
- for (CatalogItem<?, ?> item :
catalog.getCatalogItems()) {
- BrooklynClassLoadingContext catalogLoader =
CatalogUtils.newClassLoadingContext(managementContext, item);
- Maybe<Class<?>> catalogClass =
catalogLoader.tryLoadClass(jType);
- if (catalogClass.isPresent()) {
- return (Class<? extends T>) catalogClass.get();
+ CatalogItem<?, ?> catalogItem =
rebindContext.lookup().lookupCatalogItem(catalogItemId);
+ if (catalogItem == null) {
+ if
(BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_AUTO_FIX_CATALOG_REF_ON_REBIND))
{
+ // See
https://issues.apache.org/jira/browse/BROOKLYN-149
+ // This is a dangling reference to the catalog
item (which will have been logged by lookupCatalogItem).
+ // Try loading as any version.
+ if
(CatalogUtils.looksLikeVersionedId(catalogItemId)) {
+ String symbolicName =
CatalogUtils.getIdFromVersionedId(catalogItemId);
+ catalogItem =
rebindContext.lookup().lookupCatalogItem(symbolicName);
+
+ if (catalogItem != null) {
+ LOG.warn("Unable to load catalog item
"+catalogItemId+" for "+contextSuchAsId
+ +" ("+bType.getSimpleName()+");
will auto-upgrade to "+catalogItem.getCatalogItemId());
+ catalogItemId =
catalogItem.getCatalogItemId();
+ }
}
}
- throw new IllegalStateException("No catalogItemId
specified and can't load class from either classpath of catalog items");
+ }
+ if (catalogItem != null) {
+ BrooklynClassLoadingContext loader =
CatalogUtils.newClassLoadingContext(managementContext, catalogItem);
+ return new LoadedClass<T>(loader.loadClass(jType,
bType), catalogItemId);
} else {
- throw new IllegalStateException("No catalogItemId
specified and can't load class from classpath");
+ LOG.warn("Unable to load catalog item
"+catalogItemId+" for "+contextSuchAsId
+ +" ("+bType.getSimpleName()+"); will try
default class loader");
}
-
}
- }
+
+ try {
+ return new
LoadedClass<T>((Class<T>)reflections.loadClass(jType), catalogItemId);
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ LOG.warn("Unable to load "+jType+" using reflections; will
try standard context");
+ }
- protected BrooklynClassLoadingContext
getLoadingContextFromCatalogItemId(String catalogItemId, ClassLoader
classLoader, RebindContext rebindContext) {
- Preconditions.checkNotNull(catalogItemId, "catalogItemId
required (should not be null)");
- CatalogItem<?, ?> catalogItem =
rebindContext.lookup().lookupCatalogItem(catalogItemId);
- if (catalogItem != null) {
- return
CatalogUtils.newClassLoadingContext(managementContext, catalogItem);
+ if (catalogItemId != null) {
+ throw new IllegalStateException("Unable to load catalog
item "+catalogItemId+" for "+contextSuchAsId+", or load class from classpath");
+ } else if
(BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_BACKWARDS_COMPATIBILITY_INFER_CATALOG_ITEM_ON_REBIND))
{
--- End diff --
Looking at the code in details now it's not obvious why this (old) code
repeats the logic in `findCatalogItemId`. Actually the whole catalog item
inference could be encapsulated there. Just thinking out loud, no need to
change it in this PR.
---
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.
---
