dazey3 commented on a change in pull request #65:
URL: https://github.com/apache/openjpa/pull/65#discussion_r454610970
##########
File path:
openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCClassFileTransformer.java
##########
@@ -118,17 +120,44 @@ public PCClassFileTransformer(MetaDataRepository repos,
if (className == null) {
return null;
}
- // prevent re-entrant calls, which can occur if the enhancing
- // loader is used to also load OpenJPA libraries; this is to prevent
- // recursive enhancement attempts for internal openjpa libraries
- if (_transforming)
- return null;
-
- _transforming = true;
return transform0(className, redef, bytes);
}
+ // very simplified flavor of
+ // @apache/tomee >
+ // container/openejb-core >
+ // org/apache/openejb/util/classloader/URLClassLoaderFirst.java#L207
+ private boolean isExcluded(final String className) {
+ if ( // api
+ className.startsWith("javax/") ||
+ className.startsWith("jakarta/") ||
+ // openjpa dependencies
+ className.startsWith("serp/") ||
+ // jvm
+ className.startsWith("java/") ||
+ className.startsWith("sun/") ||
+ className.startsWith("jdk/")) {
+ return true;
+ }
+ // it is faster to use substring when multiple tests are needed
+ if (className.startsWith("org/apache/")) {
+ final String sub = className.substring("org/apache/".length());
+ if (sub.startsWith("openjpa/") ||
Review comment:
I personally don't like code changes like this. Referencing hardcoded
strings and doing numerous string comparisons/manipulation is kinda
gross/hacky. I can maybe see the argument that perhaps a ThreadLocal is too
small/simple of a change, but complex string comparisons like this can't be the
solution either.
----------------------------------------------------------------
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:
us...@infra.apache.org
