Adding new API to register backward compatible class names#17772
Adding new API to register backward compatible class names#17772
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an API on PluginManager to register mappings from deprecated plugin class names to their modern replacements, enabling backward compatibility during plugin/library migrations.
Changes:
- Introduced
registerBackwardCompatibleClassName(oldClassName, newClassName)to store deprecated→new class name mappings. - Added an inline comment describing the method’s intent.
| } | ||
| } | ||
|
|
||
| // Register plugin class names when users try to deprecate old plugins/libraries. |
There was a problem hiding this comment.
This introduces a new public SPI method but only documents it with an inline comment. Please convert this to a proper Javadoc that clarifies expected inputs (e.g., fully-qualified class names), when it should be called (startup vs runtime), and what behavior consumers should expect if a mapping already exists.
| // Register plugin class names when users try to deprecate old plugins/libraries. | |
| /** | |
| * Registers a backward-compatible mapping between a deprecated plugin implementation class and its replacement. | |
| * | |
| * <p>Both {@code oldClassName} and {@code newClassName} must be fully-qualified class names that can be | |
| * resolved in the plugin classpath.</p> | |
| * | |
| * <p>This method is intended to be invoked during system startup or plugin initialization, before any plugins | |
| * are loaded or instantiated. Calling it at runtime after plugins have already been created will only affect | |
| * subsequent lookups.</p> | |
| * | |
| * <p>If a mapping for the given {@code oldClassName} already exists, the previous mapping will be silently | |
| * replaced with the new value.</p> | |
| * | |
| * @param oldClassName the fully-qualified name of the deprecated plugin class | |
| * @param newClassName the fully-qualified name of the replacement plugin class | |
| */ |
|
|
||
| // Register plugin class names when users try to deprecate old plugins/libraries. | ||
| public void registerBackwardCompatibleClassName(String oldClassName, String newClassName) { | ||
| PLUGINS_BACKWARD_COMPATIBLE_CLASS_NAME_MAP.put(oldClassName, newClassName); |
There was a problem hiding this comment.
This silently overwrites any existing mapping for the same oldClassName, which can make misconfiguration hard to detect. Consider using a non-overwriting registration (e.g., put-if-absent) and either throw an exception or log/report when an existing mapping differs from the new value.
| PLUGINS_BACKWARD_COMPATIBLE_CLASS_NAME_MAP.put(oldClassName, newClassName); | |
| String previous = PLUGINS_BACKWARD_COMPATIBLE_CLASS_NAME_MAP.putIfAbsent(oldClassName, newClassName); | |
| if (previous != null && !previous.equals(newClassName)) { | |
| LOGGER.warn("Backward-compatible class name mapping for '{}' already exists: '{}' -> '{}'. " | |
| + "Ignoring new mapping to '{}'.", oldClassName, oldClassName, previous, newClassName); | |
| } |
xiangfu0
force-pushed
the
allow-register-backward-compatible-class-mapping
branch
from
f5c009b to
4d0e628
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #17772 +/- ##
============================================
+ Coverage 63.22% 63.23% +0.01%
Complexity 1454 1454
============================================
Files 3179 3179
Lines 191310 191315 +5
Branches 29263 29264 +1
============================================
+ Hits 120950 120976 +26
+ Misses 60918 60900 -18
+ Partials 9442 9439 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| // Register plugin class names when users try to deprecate old plugins/libraries. | ||
| public void registerBackwardCompatibleClassName(String oldClassName, String newClassName) { | ||
| String existingNewClassName = PLUGINS_BACKWARD_COMPATIBLE_CLASS_NAME_MAP.get(oldClassName); |
There was a problem hiding this comment.
You can directly call put() and check the return value.
| LOGGER.warn("There is already a mapping for backward compatible class from old [{}] to [{}], override it to [{}]", | ||
| oldClassName, existingNewClassName, newClassName); | ||
| } | ||
| PLUGINS_BACKWARD_COMPATIBLE_CLASS_NAME_MAP.put(oldClassName, newClassName); |
There was a problem hiding this comment.
Suggest logging an INFO for new registered class
| LOGGER.warn("There is already a mapping for backward compatible class from old [{}] to [{}], override it to [{}]", | ||
| oldClassName, existingNewClassName, newClassName); | ||
| } | ||
| PLUGINS_BACKWARD_COMPATIBLE_CLASS_NAME_MAP.put(oldClassName, newClassName); |
This pull request introduces a new method to the
PluginManagerclass to support backward compatibility for plugin class names. This allows users to register mappings from deprecated class names to their new equivalents, helping maintain compatibility when plugins or libraries are updated.Backward compatibility support:
registerBackwardCompatibleClassNamemethod toPluginManagerthat lets users map old plugin class names to new ones for smoother deprecation and migration processes.