Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/868#discussion_r147090854
--- Diff:
core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
---
@@ -74,28 +92,40 @@ public BasicBrooklynTypeRegistry(ManagementContext
mgmt) {
}
private Iterable<RegisteredType> getAllWithoutCatalog(Predicate<?
super RegisteredType> filter) {
- // TODO thread safety
// TODO optimisation? make indexes and look up?
- return Iterables.filter(localRegisteredTypes.values(), filter);
+ return Locks.withLock(localRegistryLock.readLock(),
+ () ->
localRegisteredTypesAndContainingBundles.values().stream().
+ flatMap(m ->
m.values().stream()).filter(filter::apply).collect(Collectors.toList()) );
}
private Maybe<RegisteredType> getExactWithoutLegacyCatalog(String
symbolicName, String version, RegisteredTypeLoadingContext constraint) {
- // TODO look in any nested/private registries
- RegisteredType item =
localRegisteredTypes.get(symbolicName+":"+version);
+ RegisteredType item = Locks.withLock(localRegistryLock.readLock(),
+ ()->
getBestValue(localRegisteredTypesAndContainingBundles.get(symbolicName+":"+version))
);
return RegisteredTypes.tryValidate(item, constraint);
}
+ private RegisteredType getBestValue(Map<String, RegisteredType> m) {
+ if (m==null) return null;
+ if (m.isEmpty()) return null;
+ if (m.size()==1) return m.values().iterator().next();
+ // get the highest version of first alphabetical - to have a
canonical order
+ return m.get(
Ordering.from(VersionedNameStringComparator.INSTANCE).max(m.keySet()) );
--- End diff --
This probably feels ok for now, but it also feels like we can become a lot
stricter (and thus simpler and more predictable):
* forbid conflicting types of the same version from getting into the
catalog in the first place!
* prefer the type from our own bundle, or from our own dependencies
declared in `brooklyn.libraries` (i.e. pass in the context of which bundle is
looking it up).
I think you said you were deliberately supporting conflicting types from
different bundles, because multi-tenancy could then be implemented just be at
the bundle level. I think doing that makes our life more complicated and error
prone in the medium term. And long-term we might well not implement
multi-tenancy in that way.
---
