Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/746#discussion_r125920216
--- Diff:
core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java
---
@@ -511,30 +597,76 @@ private void collectCatalogItems(String sourceYaml,
Map<?,?> itemMetadata, List<
// brooklyn.libraries we treat specially, to append the list, with
the child's list preferred in classloading order
// `libraries` is supported in some places as a legacy syntax; it
should always be `brooklyn.libraries` for new apps
// TODO in 0.8.0 require brooklyn.libraries, don't allow
"libraries" on its own
- List<?> librariesNew =
MutableList.copyOf(getFirstAs(itemMetadataWithoutItemDef, List.class,
"brooklyn.libraries", "libraries").orNull());
- Collection<CatalogBundle> libraryBundlesNew =
CatalogItemDtoAbstract.parseLibraries(librariesNew);
-
- List<?> librariesCombined = MutableList.copyOf(librariesNew)
- .appendAll(getFirstAs(parentMetadata, List.class,
"brooklyn.libraries", "libraries").orNull());
- if (!librariesCombined.isEmpty())
- catalogMetadata.put("brooklyn.libraries", librariesCombined);
- Collection<CatalogBundle> libraryBundles =
CatalogItemDtoAbstract.parseLibraries(librariesCombined);
+ List<?> librariesAddedHereNames =
MutableList.copyOf(getFirstAs(itemMetadataWithoutItemDef, List.class,
"brooklyn.libraries", "libraries").orNull());
+ Collection<CatalogBundle> librariesAddedHereBundles =
CatalogItemDtoAbstract.parseLibraries(librariesAddedHereNames);
+
+ MutableSet<Object> librariesCombinedNames = MutableSet.of();
+ if (!isNoBundleOrSimpleWrappingBundle(mgmt, containingBundle)) {
+ // ensure containing bundle is declared, first, for search
purposes
+
librariesCombinedNames.add(containingBundle.getVersionedName().toOsgiString());
+ }
+ librariesCombinedNames.putAll(librariesAddedHereNames);
+ librariesCombinedNames.putAll(getFirstAs(parentMetadata,
Collection.class, "brooklyn.libraries", "libraries").orNull());
+ if (!librariesCombinedNames.isEmpty()) {
+ catalogMetadata.put("brooklyn.libraries",
librariesCombinedNames);
+ }
+ Collection<CatalogBundle> libraryBundles =
CatalogItemDtoAbstract.parseLibraries(librariesCombinedNames);
- // TODO as this may take a while if downloading, the REST call
should be async
- // (this load is required for the scan below and I think also for
yaml resolution)
- CatalogUtils.installLibraries(mgmt, libraryBundlesNew);
+ // TODO this may take a while if downloading; ideally the REST
call would be async
+ // but this load is required for resolving YAML in this BOM (and
if java-scanning);
+ // need to think through how we expect dependencies to be installed
+ CatalogUtils.installLibraries(mgmt, librariesAddedHereBundles);
+
+ // use resolved bundles
+ librariesAddedHereBundles = resolveWherePossible(mgmt,
librariesAddedHereBundles);
+ libraryBundles = resolveWherePossible(mgmt, libraryBundles);
Boolean scanJavaAnnotations =
getFirstAs(itemMetadataWithoutItemDef, Boolean.class, "scanJavaAnnotations",
"scan_java_annotations").orNull();
if (scanJavaAnnotations==null || !scanJavaAnnotations) {
// don't scan
} else {
- // scan for annotations: if libraries here, scan them; if
inherited libraries error; else scan classpath
- if (!libraryBundlesNew.isEmpty()) {
- result.addAll(scanAnnotationsFromBundles(mgmt,
libraryBundlesNew, catalogMetadata));
- } else if (libraryBundles.isEmpty()) {
- result.addAll(scanAnnotationsFromLocal(mgmt,
catalogMetadata));
+ if (isNoBundleOrSimpleWrappingBundle(mgmt, containingBundle)) {
+ Collection<CatalogItemDtoAbstract<?, ?>> scanResult;
+ // BOMs wrapped in JARs, or without JARs, have special
treatment
+ if
(isLibrariesMoreThanJustContainingBundle(librariesAddedHereBundles,
containingBundle)) {
+ // legacy mode, since 0.12.0, scan libraries
referenced in a legacy non-bundle BOM
+ log.warn("Deprecated use of scanJavaAnnotations to
scan other libraries ("+librariesAddedHereBundles+"); libraries should declare
they scan themselves");
+ scanResult =
scanAnnotationsLegacyInListOfLibraries(mgmt, librariesAddedHereBundles,
catalogMetadata, containingBundle);
+ } else if
(!isLibrariesMoreThanJustContainingBundle(libraryBundles, containingBundle)) {
+ // for default catalog, no libraries declared, we want
to scan local classpath
+ // bundle should be named "brooklyn-default-catalog"
+ if (containingBundle!=null &&
!containingBundle.getSymbolicName().contains("brooklyn-default-catalog")) {
+ // a user uplaoded a BOM trying to tell us to do a
local java scan; previously supported but becoming unsupported
+ log.warn("Deprecated use of scanJavaAnnotations in
non-Java BOM outwith the default catalog setup");
+ } else if (depth>0) {
+ // since 0.12.0, require this to be right next to
where libraries are defined, or at root
+ log.warn("Deprecated use of scanJavaAnnotations
declared in item; should be declared at the top level of the BOM");
+ }
+ scanResult =
scanAnnotationsFromLocalNonBundleClasspath(mgmt, catalogMetadata,
containingBundle);
+ } else {
+ throw new IllegalStateException("Cannot scan for Java
catalog items when libraries declared on an ancestor; scanJavaAnnotations
should be specified alongside brooklyn.libraries (or ideally those libraries
should specify to scan)");
+ }
+ if (scanResult!=null && !scanResult.isEmpty()) {
+ if (result!=null) {
+ result.addAll( scanResult );
+ } else {
+ // not returning a result; we need to add here
+ for (CatalogItem item: scanResult) {
+ mgmt.getCatalog().addItem(item);
+ }
+ }
+ }
} else {
- throw new IllegalStateException("Cannot scan catalog node
no local bundles, and with inherited bundles we will not scan the classpath");
+ throw new IllegalArgumentException("Scanning for Java
annotations is not supported in BOMs in bundles; "
+ + "entries should be listed explicitly in the
catalog.bom");
+ // see comments on scanAnnotationsInBundle
+// if (depth>0) {
--- End diff --
Best remove this commented code
---
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.
---