refactor: remove the package existing in java8#2792
Conversation
dosubot
bot
added
the
size:M
Tsukilc
changed the title
|
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
There was a problem hiding this comment.
Pull Request Overview
This PR removes Java 8 compatibility by eliminating sun.reflect-related reflection packages and hardcoding the use of jdk.internal.reflect.Reflection for Java 11+.
- Removes fallback support for Java 8's sun.reflect.Reflection package
- Updates error messages to explicitly require Java 11 or higher
- Cleans up conditional class loading logic that previously supported both Java 8 and 11+ reflection APIs
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Reflection.java | Simplifies reflection class loading by removing Java 8 fallback and hardcoding jdk.internal.reflect.Reflection |
| HugeSecurityManager.java | Removes sun.reflect.DelegatingClassLoader from accepted class loaders list |
| HugeFactoryAuthProxy.java | Removes Java 8-specific reflection filter registrations and updates logging |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| registerFieldsToFilterMethodTemp = | ||
| reflectionClazzTemp.getMethod("registerFieldsToFilter", | ||
| Class.class, String[].class); | ||
|
|
||
| registerMethodsToFilterMethodTemp = | ||
| reflectionClazzTemp.getMethod("registerMethodsToFilter", | ||
| Class.class, String[].class); | ||
| } catch (ClassNotFoundException e) { | ||
| try { | ||
| reflectionClazzTemp = Class.forName(SUN_REFLECT_REFLECTION); | ||
| } catch (ClassNotFoundException ex) { | ||
| LOG.error("Can't find Reflection class", ex); | ||
| } | ||
| LOG.error("Can't find jdk.internal.reflect.Reflection class, " + | ||
| "please ensure you are using Java 11", e); | ||
| } catch (NoSuchMethodException e) { | ||
| LOG.error("Can't find reflection filter methods", e); | ||
| } |
There was a problem hiding this comment.
The method lookup logic should be wrapped in a try-catch block to handle NoSuchMethodException separately from ClassNotFoundException, ensuring proper error handling for each failure scenario.
| LOG.debug("Internal class {} not found in this JDK implementation, skipping filter registration", | ||
| className, e); |
There was a problem hiding this comment.
The log message has a grammatical error. 'Internal class {} not found in this JDK implementation' should be 'Internal class {} not found in this JDK implementation,'.
Tsukilc
force-pushed
the
remove-jdk8-reflect
branch
from
26da84d to
8ccff76
Compare
| LOG.debug( | ||
| "Internal class {} not found in this JDK implementation, skipping filter " + | ||
| "registration", | ||
| className, e); |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2792 +/- ##
============================================
- Coverage 40.91% 0.08% -40.83%
+ Complexity 333 22 -311
============================================
Files 747 725 -22
Lines 60168 57643 -2525
Branches 7683 7234 -449
============================================
- Hits 24615 51 -24564
- Misses 32975 57590 +24615
+ Partials 2578 2 -2576 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| private static final Method REGISTER_METHODS_TO_FILTER_METHOD; | ||
|
|
||
| public static final String JDK_INTERNAL_REFLECT_REFLECTION = "jdk.internal.reflect.Reflection"; | ||
| public static final String SUN_REFLECT_REFLECTION = "sun.reflect.Reflection"; |
There was a problem hiding this comment.
why remove this constant variables?
imbajin
changed the title
* refactor: Delete the package existing in java8 * chore(format): remove custom line breaks
Purpose of the PR
sub task of [#2724](#2764)
No longer maintain compatibility with java8 and delete the sun.reflection-related reflection packages of java8
Main Changes
Verifying these changes
Does this PR potentially affect the following parts?
Documentation Status
Doc - TODODoc - DoneDoc - No Need