brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From neykov <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request #338: Support nested catalog item definitions -...
Date Thu, 05 Jan 2017 13:11:03 GMT
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/338#discussion_r94751531
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java
---
    @@ -910,70 +911,55 @@ protected Entity newEntity(EntityMementoManifest entityManifest)
{
                     ((AbstractEntity)entity).setManagementContext(managementContext);
                     managementContext.prePreManage(entity);
                 }
    -            
    -            setCatalogItemId(entity, transformedCatalogItemId);
    +
    +            setCatalogItemIds(entity, loaded.catalogItemIds);
    +
                 return entity;
             }
     
    -        protected void setCatalogItemId(BrooklynObject item, String catalogItemId) {
    -            if (catalogItemId!=null) {
    -                ((BrooklynObjectInternal)item).setCatalogItemId(catalogItemId);
    -            }
    +        protected void setCatalogItemIds(BrooklynObject object, List<String> superIds)
{
    +            ((BrooklynObjectInternal)object).setCatalogItemIds(superIds);
             }
     
    +
             protected <T extends BrooklynObject> LoadedClass<? extends T> load(Class<T>
bType, Memento memento) {
    -            return load(bType, memento.getType(), memento.getCatalogItemId(), memento.getId());
    +            return load(bType, memento.getType(), memento.getCatalogItemSuperIds(), memento.getId());
             }
             
             @SuppressWarnings("unchecked")
    -        protected <T extends BrooklynObject> LoadedClass<? 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, List<String> catalogItemIds, String contextSuchAsId) {
                 checkNotNull(jType, "Type of %s (%s) must not be null", contextSuchAsId,
bType.getSimpleName());
    -            
    -            if (catalogItemId != null) {
    -                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.getSymbolicNameFromVersionedId(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();
    -                            }
    -                        }
    -                    }
    -                }
    -                if (catalogItem != null) {
    -                    BrooklynClassLoadingContext loader = CatalogUtils.newClassLoadingContext(managementContext,
catalogItem);
    -                    return new LoadedClass<T>(loader.loadClass(jType, bType), catalogItemId);
    +
    +            List<String> idsFromReboundCatalog = MutableList.of();
    +            if (catalogItemIds != null && !catalogItemIds.isEmpty()) {
    +                findCatalogIdsInReboundCatalog(bType, catalogItemIds, contextSuchAsId,
idsFromReboundCatalog);
    +                if (idsFromReboundCatalog.size() == catalogItemIds.size()) {
    +                    BrooklynClassLoadingContext loader = CatalogUtils.newClassLoadingContextForCatalogItems(managementContext,
idsFromReboundCatalog);
    +                    return new LoadedClass<T>(loader.loadClass(jType, bType), idsFromReboundCatalog);
                     } else {
    -                    LOG.warn("Unable to load catalog item "+catalogItemId+" for "+contextSuchAsId
    +                    LOG.warn("Unable to load all catalog items "+ Iterables.toString(catalogItemIds)
+" for "+contextSuchAsId
    --- End diff --
    
    Fall through only if `loader.loadClass(jType, bType)` fails with the available catalog
items. Any catalog items being able to load the class here is better than any of the fall
backs.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message