[ISSUE#13249] Fix configuration change plugin return incompatibility. #13255

KiteSoar · 2025-04-16T03:54:22Z

What is the purpose of the change

its for #13249

Brief changelog

XX

Verifying this change

XXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
Run mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.

wuyfee · 2025-04-16T04:40:42Z

$F A I L U R E$
DETAILS
✅ - docker: success
✅ - deploy (standalone & cluster & standalone_auth): success
✅ - e2e-java-test (standalone & cluster & standalone_auth): success
❌ - e2e-go-test (standalone & cluster): failure
✅ - e2e-cpp-test (standalone & cluster): success
✅ - e2e-csharp-test (standalone & cluster): success
✅ - e2e-nodejs-test (standalone & cluster): success
✅ - e2e-python-test (standalone & cluster): success
✅ - clean (standalone & cluster & standalone_auth): success

wuyfee · 2025-04-16T05:17:43Z

$F A I L U R E$
DETAILS
✅ - docker: success
✅ - deploy (standalone & cluster & standalone_auth): success
✅ - e2e-java-test (standalone & cluster & standalone_auth): success
❌ - e2e-go-test (standalone & cluster): failure
✅ - e2e-cpp-test (standalone & cluster): success
✅ - e2e-csharp-test (standalone & cluster): success
✅ - e2e-nodejs-test (standalone & cluster): success
✅ - e2e-python-test (standalone & cluster): success
✅ - clean (standalone & cluster & standalone_auth): success

…bility

codecov-commenter · 2025-04-17T00:46:53Z

Codecov Report

Attention: Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.

Project coverage is 72.32%. Comparing base (6daea18) to head (7c50fb5).

Files with missing lines	Patch %	Lines
...nacos/plugin/config/ConfigChangePluginManager.java	0.00%	3 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #13255      +/-   ##
=============================================
+ Coverage      72.27%   72.32%   +0.04%     
- Complexity     10064    10067       +3     
=============================================
  Files           1313     1313              
  Lines          42275    42268       -7     
  Branches        4443     4442       -1     
=============================================
+ Hits           30556    30571      +15     
+ Misses          9606     9584      -22     
  Partials        2113     2113

Files with missing lines	Coverage Δ
...nacos/config/server/aspect/ConfigChangeAspect.java	`86.66% <100.00%> (+12.87%)`	⬆️
...nacos/plugin/config/ConfigChangePluginManager.java	`66.00% <0.00%> (-4.22%)`	⬇️

... and 1 file with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6daea18...7c50fb5. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

wuyfee · 2025-04-17T01:02:14Z

$F A I L U R E$
DETAILS
✅ - docker: success
✅ - deploy (standalone & cluster & standalone_auth): success
✅ - e2e-java-test (standalone & cluster & standalone_auth): success
❌ - e2e-go-test (standalone & cluster): failure
✅ - e2e-cpp-test (standalone & cluster): success
✅ - e2e-csharp-test (standalone & cluster): success
✅ - e2e-nodejs-test (standalone & cluster): success
✅ - e2e-python-test (standalone & cluster): success
✅ - clean (standalone & cluster & standalone_auth): success

KomachiSion

感觉这个修复方式太粗暴了，只返回boolean感觉不适配的范围会更大。

KiteSoar · 2025-04-23T01:21:43Z

感觉这个修复方式太粗暴了，只返回boolean感觉不适配的范围会更大。

感觉不会扩大不适配范围哇

原有代码不兼容的原因是因为切面切的这两个方法返回值都是 Boolean

    private static final String PUBLISH_CONFIG =
            "execution(* com.alibaba.nacos.config.server.service.ConfigOperationService.publishConfig(..))";
    
    private static final String DELETE_CONFIG =
            "execution(* com.alibaba.nacos.config.server.service.ConfigOperationService.deleteConfig(..))";

原来前置插件执行失败时返回了RestResult，所以导致了返回值类型不兼容，如果不改成返回boolean，还有其他比较好的修复方式嘛🤔

KomachiSion · 2025-04-25T07:23:49Z

感觉这个修复方式太粗暴了，只返回boolean感觉不适配的范围会更大。

感觉不会扩大不适配范围哇

原有代码不兼容的原因是因为切面切的这两个方法返回值都是 Boolean
    private static final String PUBLISH_CONFIG =
            "execution(* com.alibaba.nacos.config.server.service.ConfigOperationService.publishConfig(..))";
    
    private static final String DELETE_CONFIG =
            "execution(* com.alibaba.nacos.config.server.service.ConfigOperationService.deleteConfig(..))";
原来前置插件执行失败时返回了RestResult，所以导致了返回值类型不兼容，如果不改成返回boolean，还有其他比较好的修复方式嘛🤔

没有太理解，插件返回RestResult，和ConfigOperationService.deleteConfig返回boolean有什么关系？

KiteSoar · 2025-04-27T14:36:08Z

感觉这个修复方式太粗暴了，只返回boolean感觉不适配的范围会更大。

感觉不会扩大不适配范围哇
原有代码不兼容的原因是因为切面切的这两个方法返回值都是 Boolean
    private static final String PUBLISH_CONFIG =
            "execution(* com.alibaba.nacos.config.server.service.ConfigOperationService.publishConfig(..))";
    
    private static final String DELETE_CONFIG =
            "execution(* com.alibaba.nacos.config.server.service.ConfigOperationService.deleteConfig(..))";
原来前置插件执行失败时返回了RestResult，所以导致了返回值类型不兼容，如果不改成返回boolean，还有其他比较好的修复方式嘛🤔
没有太理解，插件返回RestResult，和ConfigOperationService.deleteConfig返回boolean有什么关系？

比如说我在页面上新建一个配置
调用的是这个接口

    @PostMapping
    @TpsControl(pointName = "ConfigPublish")
    @Secured(action = ActionTypes.WRITE, signType = SignType.CONFIG)
    public Boolean publishConfig(HttpServletRequest request, HttpServletResponse response,
            @RequestParam(value = "dataId") String dataId, @RequestParam(value = "group") String group,
            @RequestParam(value = "tenant", required = false, defaultValue = StringUtils.EMPTY) String tenant,
            @RequestParam(value = "content") String content, @RequestParam(value = "tag", required = false) String tag,
            @RequestParam(value = "appName", required = false) String appName,
            @RequestParam(value = "src_user", required = false) String srcUser,
            @RequestParam(value = "config_tags", required = false) String configTags,
            @RequestParam(value = "desc", required = false) String desc,
            @RequestParam(value = "use", required = false) String use,
            @RequestParam(value = "effect", required = false) String effect,
            @RequestParam(value = "type", required = false) String type,
            @RequestParam(value = "schema", required = false) String schema,
            @RequestParam(required = false) String encryptedDataKey) throws NacosException {
        
        String encryptedDataKeyFinal = null;
        if (StringUtils.isNotBlank(encryptedDataKey)) {
            encryptedDataKeyFinal = encryptedDataKey;
        } else {
            Pair<String, String> pair = EncryptionHandler.encryptHandler(dataId, content);
            content = pair.getSecond();
            encryptedDataKeyFinal = pair.getFirst();
        }
        
        // check tenant
        tenant = NamespaceUtil.processNamespaceParameter(tenant);
        ParamUtils.checkTenant(tenant);
        ParamUtils.checkParam(dataId, group, "datumId", content);
        ParamUtils.checkParam(tag);
        
        ConfigForm configForm = new ConfigForm();
        configForm.setDataId(dataId);
        configForm.setGroup(group);
        configForm.setNamespaceId(tenant);
        configForm.setContent(content);
        configForm.setTag(tag);
        configForm.setAppName(appName);
        configForm.setSrcUser(srcUser);
        configForm.setConfigTags(configTags);
        configForm.setDesc(desc);
        configForm.setUse(use);
        configForm.setEffect(effect);
        configForm.setType(type);
        configForm.setSchema(schema);
        if (StringUtils.isBlank(srcUser)) {
            configForm.setSrcUser(RequestUtil.getSrcUserName(request));
        }
        if (!ConfigType.isValidType(type)) {
            configForm.setType(ConfigType.getDefaultType().getType());
        }
        
        ConfigRequestInfo configRequestInfo = new ConfigRequestInfo();
        configRequestInfo.setSrcIp(RequestUtil.getRemoteIp(request));
        configRequestInfo.setSrcType(Constants.HTTP);
        configRequestInfo.setRequestIpApp(RequestUtil.getAppName(request));
        configRequestInfo.setBetaIps(request.getHeader("betaIps"));
        configRequestInfo.setCasMd5(request.getHeader("casMd5"));
        
        return configOperationService.publishConfig(configForm, configRequestInfo, encryptedDataKeyFinal);
    }

这个接口里会调用configOperationService#publishConfig方法，而在ConfigChangeAspect的切面类里切的就是这个方法，所以会在执行configOperationService#publishConfig方法前，会先执行切面的ConfigChangeAspect#publishOrUpdateConfigAround方法

    private static final String PUBLISH_CONFIG =
            "execution(* com.alibaba.nacos.config.server.service.ConfigOperationService.publishConfig(..))";
    
    /**
     * Publish or update config.
     */
    @Around(PUBLISH_CONFIG)
    Object publishOrUpdateConfigAround(ProceedingJoinPoint pjp) throws Throwable {
        return RestResultUtils.failed("错误");

假设我让publishOrUpdateConfigAround方法里返回一个RestResult类型，会出现返回值类型不匹配，如下所示

KiteSoar · 2025-04-27T14:41:33Z

感觉这个修复方式太粗暴了，只返回boolean感觉不适配的范围会更大。

感觉不会扩大不适配范围哇
原有代码不兼容的原因是因为切面切的这两个方法返回值都是 Boolean
    private static final String PUBLISH_CONFIG =
            "execution(* com.alibaba.nacos.config.server.service.ConfigOperationService.publishConfig(..))";
    
    private static final String DELETE_CONFIG =
            "execution(* com.alibaba.nacos.config.server.service.ConfigOperationService.deleteConfig(..))";
原来前置插件执行失败时返回了RestResult，所以导致了返回值类型不兼容，如果不改成返回boolean，还有其他比较好的修复方式嘛🤔
没有太理解，插件返回RestResult，和ConfigOperationService.deleteConfig返回boolean有什么关系？

比如说我在页面上新建一个配置调用的是这个接口

    @PostMapping
    @TpsControl(pointName = "ConfigPublish")
    @Secured(action = ActionTypes.WRITE, signType = SignType.CONFIG)
    public Boolean publishConfig(HttpServletRequest request, HttpServletResponse response,
            @RequestParam(value = "dataId") String dataId, @RequestParam(value = "group") String group,
            @RequestParam(value = "tenant", required = false, defaultValue = StringUtils.EMPTY) String tenant,
            @RequestParam(value = "content") String content, @RequestParam(value = "tag", required = false) String tag,
            @RequestParam(value = "appName", required = false) String appName,
            @RequestParam(value = "src_user", required = false) String srcUser,
            @RequestParam(value = "config_tags", required = false) String configTags,
            @RequestParam(value = "desc", required = false) String desc,
            @RequestParam(value = "use", required = false) String use,
            @RequestParam(value = "effect", required = false) String effect,
            @RequestParam(value = "type", required = false) String type,
            @RequestParam(value = "schema", required = false) String schema,
            @RequestParam(required = false) String encryptedDataKey) throws NacosException {
        
        String encryptedDataKeyFinal = null;
        if (StringUtils.isNotBlank(encryptedDataKey)) {
            encryptedDataKeyFinal = encryptedDataKey;
        } else {
            Pair<String, String> pair = EncryptionHandler.encryptHandler(dataId, content);
            content = pair.getSecond();
            encryptedDataKeyFinal = pair.getFirst();
        }
        
        // check tenant
        tenant = NamespaceUtil.processNamespaceParameter(tenant);
        ParamUtils.checkTenant(tenant);
        ParamUtils.checkParam(dataId, group, "datumId", content);
        ParamUtils.checkParam(tag);
        
        ConfigForm configForm = new ConfigForm();
        configForm.setDataId(dataId);
        configForm.setGroup(group);
        configForm.setNamespaceId(tenant);
        configForm.setContent(content);
        configForm.setTag(tag);
        configForm.setAppName(appName);
        configForm.setSrcUser(srcUser);
        configForm.setConfigTags(configTags);
        configForm.setDesc(desc);
        configForm.setUse(use);
        configForm.setEffect(effect);
        configForm.setType(type);
        configForm.setSchema(schema);
        if (StringUtils.isBlank(srcUser)) {
            configForm.setSrcUser(RequestUtil.getSrcUserName(request));
        }
        if (!ConfigType.isValidType(type)) {
            configForm.setType(ConfigType.getDefaultType().getType());
        }
        
        ConfigRequestInfo configRequestInfo = new ConfigRequestInfo();
        configRequestInfo.setSrcIp(RequestUtil.getRemoteIp(request));
        configRequestInfo.setSrcType(Constants.HTTP);
        configRequestInfo.setRequestIpApp(RequestUtil.getAppName(request));
        configRequestInfo.setBetaIps(request.getHeader("betaIps"));
        configRequestInfo.setCasMd5(request.getHeader("casMd5"));
        
        return configOperationService.publishConfig(configForm, configRequestInfo, encryptedDataKeyFinal);
    }

这个接口里会调用configOperationService#publishConfig方法，而在ConfigChangeAspect的切面类里切的就是这个方法，所以会在执行configOperationService#publishConfig方法前，会先执行切面的ConfigChangeAspect#publishOrUpdateConfigAround方法

    private static final String PUBLISH_CONFIG =
            "execution(* com.alibaba.nacos.config.server.service.ConfigOperationService.publishConfig(..))";
    
    /**
     * Publish or update config.
     */
    @Around(PUBLISH_CONFIG)
    Object publishOrUpdateConfigAround(ProceedingJoinPoint pjp) throws Throwable {
        return RestResultUtils.failed("错误");

假设我让publishOrUpdateConfigAround方法里返回一个RestResult类型，会出现返回值类型不匹配，如下所示

关联的这个issue问题同理

KiteSoar force-pushed the fix-aspect branch from dd5bbac to f268aef Compare April 16, 2025 04:40

[ISSUE#13249] Fix configuration change plugin return object incompati…

7c50fb5

…bility

KiteSoar force-pushed the fix-aspect branch from f268aef to 7c50fb5 Compare April 17, 2025 00:33

KomachiSion requested changes Apr 21, 2025

View reviewed changes

KomachiSion approved these changes May 19, 2025

View reviewed changes

KomachiSion merged commit acb967b into alibaba:develop May 19, 2025
7 checks passed

KomachiSion added the kind/bug label May 19, 2025

KomachiSion modified the milestones: 2.5.2, 3.0.1 May 19, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ISSUE#13249] Fix configuration change plugin return incompatibility. #13255

[ISSUE#13249] Fix configuration change plugin return incompatibility. #13255

KiteSoar commented Apr 16, 2025

wuyfee commented Apr 16, 2025

wuyfee commented Apr 16, 2025

codecov-commenter commented Apr 17, 2025 •

edited

Loading

wuyfee commented Apr 17, 2025

KomachiSion left a comment

KiteSoar commented Apr 23, 2025 •

edited

Loading

KomachiSion commented Apr 25, 2025

KiteSoar commented Apr 27, 2025

KiteSoar commented Apr 27, 2025

[ISSUE#13249] Fix configuration change plugin return incompatibility. #13255

[ISSUE#13249] Fix configuration change plugin return incompatibility. #13255

Conversation

KiteSoar commented Apr 16, 2025

What is the purpose of the change

Brief changelog

Verifying this change

wuyfee commented Apr 16, 2025

wuyfee commented Apr 16, 2025

codecov-commenter commented Apr 17, 2025 • edited Loading

Codecov Report

wuyfee commented Apr 17, 2025

KomachiSion left a comment

Choose a reason for hiding this comment

KiteSoar commented Apr 23, 2025 • edited Loading

KomachiSion commented Apr 25, 2025

KiteSoar commented Apr 27, 2025

KiteSoar commented Apr 27, 2025

codecov-commenter commented Apr 17, 2025 •

edited

Loading

KiteSoar commented Apr 23, 2025 •

edited

Loading