grkvlt commented on a change in pull request #1150:
URL: https://github.com/apache/brooklyn-server/pull/1150#discussion_r582905757
##########
File path:
core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java
##########
@@ -367,7 +375,7 @@ public URL getResource(String name) {
}
if (bundle == null) {
throw new IllegalStateException("Bundle " +
toBundleString(symbolicName, version)
- + " not found to load " + name);
+ + " not found to load " + name +
(originalSymbolicName.equals(symbolicName)?"":". Original symbolic name:
"+originalSymbolicName));
Review comment:
Suggest moving this code out into the `toBundleString` helper method,
where it can append "was originally _originalName_" if it finds `symbolicName`
in the rename map
##########
File path:
core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java
##########
@@ -329,9 +332,14 @@ public URL getResource(String name) {
return Maybe.absentNull();
}
- protected <T> Maybe<T> tryLoadFromBundle(LoaderDispatcher<T> dispatcher,
String symbolicName, String version,
+ protected <T> Maybe<T> tryLoadFromBundle(LoaderDispatcher<T> dispatcher,
String originalSymbolicName, String version,
String name) {
Framework framework = getFramework();
+ String symbolicName = originalSymbolicName;
+ if(UPDATED_SYMBOLICS_NAMES.containsKey(symbolicName)){
+ log.debug("Using {} as symbolicName instead of {} as it is in
UPDATED_SYMBOLICS_NAMES list", symbolicName, originalSymbolicName);
+ symbolicName = UPDATED_SYMBOLICS_NAMES.get(symbolicName);
Review comment:
Instead of setting this here, look for where we use `symbolicName` and
do:
```
String nameToSearch = findUpdatedSymbolicName(symbolicName);
```
Reason for this is we may need multiple passes through the map since A
becomes B, then B becomes C later, and we may have both A and B, as legacy
names, so now need to resolve them both to C.
##########
File path:
core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java
##########
@@ -329,9 +332,14 @@ public URL getResource(String name) {
return Maybe.absentNull();
}
- protected <T> Maybe<T> tryLoadFromBundle(LoaderDispatcher<T> dispatcher,
String symbolicName, String version,
+ protected <T> Maybe<T> tryLoadFromBundle(LoaderDispatcher<T> dispatcher,
String originalSymbolicName, String version,
String name) {
Framework framework = getFramework();
+ String symbolicName = originalSymbolicName;
+ if(UPDATED_SYMBOLICS_NAMES.containsKey(symbolicName)){
Review comment:
would prefer a helper method `isSymbolicNameChanged`
##########
File path:
core/src/main/java/org/apache/brooklyn/util/core/ClassLoaderUtils.java
##########
@@ -76,6 +76,9 @@
private static final String WHITE_LIST_DEFAULT =
"org\\.apache\\.brooklyn\\..*:" + BrooklynVersion.getOsgiVersion();
+ // Contains the bundle symbolic names replaced in new versions mapped to
the new name
+ private Map<String,String> UPDATED_SYMBOLICS_NAMES =
ImmutableMap.of("org.apache.commons.codec", "org.apache.commons.commons-codec");
Review comment:
Change name of map to `SYMBOLIC_NAME_UPDATES` and use the
`ImmutableMap.builder()` syntax to allow for commented entries and future
updates.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
