Skip to content

refactor: modify plugin class loading order to follow parent delegation mechanism #7258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 6, 2025

Conversation

guqing
Copy link
Member

@guqing guqing commented Mar 3, 2025

What type of PR is this?

/kind improvement
/area core
/milestone 2.20.x

What this PR does / why we need it:

修改插件类加载顺序遵循双亲委派机制,以避免插件需要手动排除冲突类的问题

此 PR 的动力是:

  1. 插件排除依赖复杂而麻烦
  2. 尝试多次无法很好的通过工具实现这一点
  3. 对于一些依赖如 kotlin 何 spring security oauth 等同一 JVM 只能加载一次(即不能再次从插件加载)且插件可能无法排除依赖或排除依赖后功能不正确如遇到链接错误等
  4. 签名文件冲突等问题

resources 下的资源文件加载顺序还是插件优先,避免与 halo 同名文件不加载的问题

进过测试,插件依赖功能以及其他插件的功能不受影响,建议 Reviewer 再测试一遍

Does this PR introduce a user-facing change?

调整插件类的加载顺序使其遵循双亲委派机制,替代原先的 Plugin -> Dependent Plugin -> Halo 加载顺序

@f2c-ci-robot f2c-ci-robot bot added kind/improvement Categorizes issue or PR as related to a improvement. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Mar 3, 2025
@f2c-ci-robot f2c-ci-robot bot added this to the 2.20.x milestone Mar 3, 2025
@f2c-ci-robot f2c-ci-robot bot added the area/core Issues or PRs related to the Halo Core label Mar 3, 2025
@f2c-ci-robot f2c-ci-robot bot requested review from LIlGG and ruibaby March 3, 2025 09:22
Copy link

codecov bot commented Mar 4, 2025

Codecov Report

Attention: Patch coverage is 2.85714% with 34 lines in your changes missing coverage. Please review.

Project coverage is 57.15%. Comparing base (eff73dc) to head (f5b9bec).
Report is 102 commits behind head on main.

Files with missing lines Patch % Lines
.../halo/app/plugin/loader/HaloPluginClassLoader.java 0.00% 28 Missing ⚠️
...in/java/run/halo/app/plugin/HaloPluginManager.java 20.00% 4 Missing ⚠️
...va/run/halo/app/plugin/loader/DevPluginLoader.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7258      +/-   ##
============================================
+ Coverage     56.99%   57.15%   +0.16%     
- Complexity     3999     4049      +50     
============================================
  Files           714      720       +6     
  Lines         24110    24429     +319     
  Branches       1585     1605      +20     
============================================
+ Hits          13742    13963     +221     
- Misses         9756     9846      +90     
- Partials        612      620       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

sonarqubecloud bot commented Mar 5, 2025

Copy link
Member

@ruibaby ruibaby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@f2c-ci-robot f2c-ci-robot bot added the lgtm Indicates that a PR is ready to be merged. label Mar 5, 2025
Copy link
Member

@JohnNiang JohnNiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

尽管插件在没有排除已存在的依赖的情况下也能正常运行,不过仍然建议插件主动排除已存在的依赖,这样做可以有效兼容之前的 Halo 版本和减少产物体积。

Copy link

f2c-ci-robot bot commented Mar 6, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JohnNiang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@f2c-ci-robot f2c-ci-robot bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 6, 2025
@f2c-ci-robot f2c-ci-robot bot merged commit 1d8a25c into halo-dev:main Mar 6, 2025
9 checks passed
@guqing guqing deleted the refactor/plugin-loader-order branch March 6, 2025 03:05
guqing added a commit to guqing/halo that referenced this pull request Mar 13, 2025
f2c-ci-robot bot pushed a commit that referenced this pull request Mar 13, 2025
…delegation mechanism (#7258)" (#7290)

#### What type of PR is this?
/kind cleanup

#### What this PR does / why we need it:
撤回对插件类加载顺序的改动这可能导致破坏性更新

同时,不在考虑修改加载顺序问题,由于社区版和专业版引入的依赖不同插件无法以社区版为依赖基准保证功能在专业版也可用,举个例子:
1. 插件引入了 okhttp4 作为依赖,这可能是插件引入的依赖所附带的
2. 在社区版没有问题,插件开发者也是这么测试的
3. 但是在专业版中引入了 okhttp3 作为依赖,此时插件在专业版就不可用了因为插件依赖了 okhttp4 的功能

通过上述问题就导致了不可预知的问题

#### Does this PR introduce a user-facing change?

```release-note
撤回对插件类加载顺序的改动这可能导致破坏性更新
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/core Issues or PRs related to the Halo Core kind/improvement Categorizes issue or PR as related to a improvement. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants