Github user neykov commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/585#discussion_r28645426
--- Diff:
core/src/main/java/brooklyn/catalog/internal/BasicBrooklynCatalog.java ---
@@ -529,145 +641,315 @@ public void load() {
log.warn("Name property will be ignored due to the existence
of displayName and at least one of id, symbolicName");
}
- final String catalogSymbolicName;
- if (Strings.isNonBlank(symbolicName)) {
- catalogSymbolicName = symbolicName;
- } else if (Strings.isNonBlank(id)) {
- if (Strings.isNonBlank(id) &&
CatalogUtils.looksLikeVersionedId(id)) {
- catalogSymbolicName =
CatalogUtils.getIdFromVersionedId(id);
+ // if symname not set, infer from: id, then name, then item id,
then item name
+ if (Strings.isBlank(symbolicName)) {
+ if (Strings.isNonBlank(id)) {
+ if (CatalogUtils.looksLikeVersionedId(id)) {
+ symbolicName = CatalogUtils.getIdFromVersionedId(id);
+ } else {
+ symbolicName = id;
+ }
+ } else if (Strings.isNonBlank(name)) {
+ if (CatalogUtils.looksLikeVersionedId(name)) {
+ symbolicName = CatalogUtils.getIdFromVersionedId(name);
+ } else {
+ symbolicName = name;
+ }
} else {
- catalogSymbolicName = id;
- }
- } else if (Strings.isNonBlank(name)) {
- catalogSymbolicName = name;
- } else if (Strings.isNonBlank(plan.getName())) {
- catalogSymbolicName = plan.getName();
- } else if (plan.getServices().size()==1) {
- Service svc = Iterables.getOnlyElement(plan.getServices());
- if (Strings.isBlank(svc.getServiceType())) {
- throw new IllegalStateException("CAMP service type not
expected to be missing for " + svc);
+ symbolicName = setFromItemIfUnset(symbolicName, itemAsMap,
"id");
+ symbolicName = setFromItemIfUnset(symbolicName, itemAsMap,
"name");
+ if (Strings.isBlank(symbolicName)) {
+ log.error("Can't infer catalog item symbolicName from
the following plan:\n" + sourceYaml);
+ throw new IllegalStateException("Can't infer catalog
item symbolicName from catalog item metadata");
+ }
}
- catalogSymbolicName = svc.getServiceType();
- } else {
- log.error("Can't infer catalog item symbolicName from the
following plan:\n" + yaml);
- throw new IllegalStateException("Can't infer catalog item
symbolicName from catalog item description");
}
- final String catalogVersion;
+ // if version not set, infer from: id, then from name, then item
version
if (CatalogUtils.looksLikeVersionedId(id)) {
- catalogVersion = CatalogUtils.getVersionFromVersionedId(id);
- if (version != null && !catalogVersion.equals(version)) {
- throw new IllegalArgumentException("Discrepency between
version set in id " + catalogVersion + " and version property " + version);
+ String versionFromId =
CatalogUtils.getVersionFromVersionedId(id);
+ if (versionFromId != null && Strings.isNonBlank(version) &&
!versionFromId.equals(version)) {
+ throw new IllegalArgumentException("Discrepency between
version set in id " + versionFromId + " and version property " + version);
}
- } else if (Strings.isNonBlank(version)) {
- catalogVersion = version;
- } else {
- log.warn("No version specified for catalog item " +
catalogSymbolicName + ". Using default value.");
- catalogVersion = null;
+ version = versionFromId;
}
-
- final String catalogDisplayName;
- if (Strings.isNonBlank(displayName)) {
- catalogDisplayName = displayName;
- } else if (Strings.isNonBlank(name)) {
- catalogDisplayName = name;
- } else if (Strings.isNonBlank(plan.getName())) {
- catalogDisplayName = plan.getName();
- } else {
- catalogDisplayName = null;
+ if (Strings.isBlank(version)) {
+ if (CatalogUtils.looksLikeVersionedId(name)) {
+ version = CatalogUtils.getVersionFromVersionedId(name);
+ } else if (Strings.isBlank(version)) {
+ version = setFromItemIfUnset(version, itemAsMap,
"version");
+ if (version==null) {
+ log.warn("No version specified for catalog item " +
symbolicName + ". Using default value.");
+ version = null;
+ }
+ }
}
-
- final String catalogDescription;
- if (Strings.isNonBlank(description)) {
- catalogDescription = description;
- } else if (Strings.isNonBlank(plan.getDescription())) {
- catalogDescription = plan.getDescription();
- } else {
- catalogDescription = null;
+
+ // if not set, ID can come from symname:version, failing that,
from the plan.id, failing that from the sym name
+ if (Strings.isBlank(id)) {
+ // let ID be inferred, especially from name, to support style
where only "name" is specified, with inline version
+ if (Strings.isNonBlank(symbolicName) &&
Strings.isNonBlank(version)) {
+ id = symbolicName + ":" + version;
+ }
+ id = setFromItemIfUnset(id, itemAsMap, "id");
+ if (Strings.isBlank(id)) {
+ if (Strings.isNonBlank(symbolicName)) {
+ id = symbolicName;
+ } else {
+ log.error("Can't infer catalog item id from the
following plan:\n" + sourceYaml);
+ throw new IllegalStateException("Can't infer catalog
item id from catalog item metadata");
+ }
+ }
}
- final String catalogIconUrl;
- if (Strings.isNonBlank(iconUrl)) {
- catalogIconUrl = iconUrl;
- } else if (Strings.isNonBlank(iconUrlUnderscore)) {
- catalogIconUrl = iconUrlUnderscore;
- } else {
- catalogIconUrl = null;
+ if (Strings.isBlank(displayName)) {
+ if (Strings.isNonBlank(name)) displayName = name;
+ displayName = setFromItemIfUnset(displayName, itemAsMap,
"name");
}
- final Boolean catalogDeprecated = Boolean.valueOf(deprecated);
+ String description = getFirstAs(catalogMetadata, String.class,
"description").orNull();
+ description = setFromItemIfUnset(description, itemAsMap,
"description");
- CatalogUtils.installLibraries(mgmt, libraries);
+ // icon.url is discouraged, but kept for legacy compatibility;
should deprecate this
+ final String catalogIconUrl = getFirstAs(catalogMetadata,
String.class, "iconUrl", "icon_url", "icon.url").orNull();
- String versionedId =
CatalogUtils.getVersionedId(catalogSymbolicName, catalogVersion);
- BrooklynClassLoadingContext loader =
CatalogUtils.newClassLoadingContext(mgmt, versionedId, libraries);
- AbstractBrooklynObjectSpec<?, ?> spec =
createSpec(catalogSymbolicName, plan, loader);
+ final String deprecated = getFirstAs(catalogMetadata,
String.class, "deprecated").orNull();
+ final Boolean catalogDeprecated = Boolean.valueOf(deprecated);
- CatalogItemDtoAbstract<?, ?> dto = createItemBuilder(spec,
catalogSymbolicName, catalogVersion)
- .libraries(libraries)
- .displayName(catalogDisplayName)
- .description(catalogDescription)
+ // run again now that we know the ID
+ planInterpreter = new PlanInterpreterGuessingType(id, item,
sourceYaml, itemType, loader, result).reconstruct();
+ if (!planInterpreter.isResolved()) {
+ throw new IllegalStateException("Could not resolve plan once
id and itemType are known (recursive reference?): "+sourceYaml);
+ }
+ String sourcePlanYaml = planInterpreter.getPlanYaml();
+
+ CatalogItemDtoAbstract<?, ?> dto = createItemBuilder(itemType,
symbolicName, version)
+ .libraries(libraryBundles)
+ .displayName(displayName)
+ .description(description)
+ .deprecated(catalogDeprecated)
.iconUrl(catalogIconUrl)
- .plan(yaml)
+ .plan(sourcePlanYaml)
.build();
dto.setManagementContext((ManagementContextInternal) mgmt);
- return dto;
+ result.add(dto);
}
- private AbstractBrooklynObjectSpec<?,?> createSpec(String
symbolicName, DeploymentPlan plan, BrooklynClassLoadingContext loader) {
- if (isPolicyPlan(plan)) {
- return createPolicySpec(plan, loader);
- } else if (isLocationPlan(plan)) {
- return createLocationSpec(plan, loader);
- } else {
- return createEntitySpec(symbolicName, plan, loader);
+ private String setFromItemIfUnset(String oldValue, Map<?,?> item,
String fieldAttr) {
+ if (Strings.isNonBlank(oldValue)) return oldValue;
+ if (item!=null) {
+ Object newValue = item.get(fieldAttr);
+ if (newValue instanceof String &&
Strings.isNonBlank((String)newValue))
+ return (String)newValue;
}
+ return oldValue;
}
- private CatalogItemBuilder<?>
createItemBuilder(AbstractBrooklynObjectSpec<?, ?> spec, String itemId, String
version) {
- if (spec instanceof EntitySpec) {
- if (isApplicationSpec((EntitySpec<?>)spec)) {
- return CatalogItemBuilder.newTemplate(itemId, version);
+
+ private class PlanInterpreterGuessingType {
+
+ final String id;
+ final Map<?,?> item;
+ final String itemYaml;
+ final BrooklynClassLoadingContext loader;
+ final List<CatalogItemDtoAbstract<?, ?>> itemsDefinedSoFar;
+
+ CatalogItemType catalogItemType;
+ String planYaml;
+ @SuppressWarnings("unused")
+ DeploymentPlan plan;
+ AbstractBrooklynObjectSpec<?,?> spec;
+ boolean resolved = false;
+
+ public PlanInterpreterGuessingType(@Nullable String id, Object
item, String itemYaml, @Nullable CatalogItemType optionalCiType,
+ BrooklynClassLoadingContext loader,
List<CatalogItemDtoAbstract<?,?>> itemsDefinedSoFar) {
+ // ID is useful to prevent recursive references (currently for
entities only)
+ this.id = id;
+
+ if (item instanceof String) {
+ // if just a string supplied, wrap as map
+ this.item = MutableMap.of("type", item);
+ this.itemYaml = "type:\n"+makeAsIndentedObject(itemYaml);
} else {
- return CatalogItemBuilder.newEntity(itemId, version);
+ this.item = (Map<?,?>)item;
+ this.itemYaml = itemYaml;
}
- } else if (spec instanceof PolicySpec) {
- return CatalogItemBuilder.newPolicy(itemId, version);
- } else if (spec instanceof LocationSpec) {
- return CatalogItemBuilder.newLocation(itemId, version);
- } else {
- throw new IllegalStateException("Unknown spec type " +
spec.getClass().getName() + " (" + spec + ")");
+ this.catalogItemType = optionalCiType;
+ this.loader = loader;
+ this.itemsDefinedSoFar = itemsDefinedSoFar;
}
- }
- private boolean isApplicationSpec(EntitySpec<?> spec) {
- return
!Boolean.TRUE.equals(spec.getConfig().get(EntityManagementUtils.WRAPPER_APP_MARKER));
+ public PlanInterpreterGuessingType reconstruct() {
+ attemptType(null, CatalogItemType.ENTITY);
+ attemptType(null, CatalogItemType.TEMPLATE);
--- End diff --
This will never succeed, either the ENTITY above will resolve or this will
fail as well, for the same reason.
---
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.
---
