he1l0world commented on code in PR #11048:
URL: https://github.com/apache/cloudstack/pull/11048#discussion_r3939030341
##########
framework/spring/module/src/main/java/org/apache/cloudstack/spring/module/model/impl/DefaultModuleDefinitionSet.java:
##########
@@ -310,24 +314,36 @@ public Map<String, ApplicationContext> getContextMap() {
@Override
public Resource[] getConfigResources(String name) {
- Set<Resource> resources = new LinkedHashSet<Resource>();
-
- ModuleDefinition original = null;
- ModuleDefinition def = original = modules.get(name);
-
- if (def == null)
+ ModuleDefinition def = modules.get(name);
+ if (def == null) {
return new Resource[] {};
+ }
+
+ Set<Resource> resources = new LinkedHashSet<>();
resources.addAll(def.getContextLocations());
- while (def != null) {
- resources.addAll(def.getInheritableContextLocations());
- def = modules.get(def.getParentName());
+ resources.addAll(collectInheritedResources(def));
+
+ resources.addAll(def.getOverrideContextLocations());
+
+ return resources.toArray(Resource[]::new);
+ }
+
+ private Set<Resource> collectInheritedResources(final ModuleDefinition
def) {
+ if (def == null) {
+ return Collections.emptySet();
}
- resources.addAll(original.getOverrideContextLocations());
+ final Set<Resource> cachedResources =
inheritedConfigResourcesMap.get(def.getName());
+ if (cachedResources != null) {
+ return cachedResources;
+ }
Review Comment:
I don’t think concurrency applies here since the context lookup should
always be single-threaded. Please correct me if I’m missing anything.
Also, changing this to a thread-safe data type alone would not fully solve
the issue if `getConfigResources()` is called concurrently. In that case, we
would probably need to synchronize the whole `collectInheritedResources() `
method.
So my preference would be to keep the current implementation unless we have
a concurrent call use case for it.
Happy to change it if I’m missing anything :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
