Skip to content

Conversation

@YAGAMIL
Copy link
Contributor

@YAGAMIL YAGAMIL commented May 19, 2023

  1. org.apache.dubbo.config.spring.util.DubboAnnotationUtils.convertParameters
    When the put target is an empty String array, the empty HashMap instead of java.util.Collections.EmptyMap supports subsequent scalable put operations org.apache.dubbo.config.spring.util.DubboAnnotationUtils#convertParameters中当参数列表未空时,直接返回了不可修改的Collections.emptyMap(),我认为不合适 #12350

What is the purpose of the change

So that @DubbboService#parameter, even if empty, does not affect the extra put operation causing the program to throw an exception

  if (ArrayUtils.isEmpty(parameters)) {
       return Collections.emptyMap();
   }

---->

  if (ArrayUtils.isEmpty(parameters)) {
       return new HashMap<>();
  }

Collections.emptyMap()is a special implementation of EmptyMap inside Collections that doesn't support change, so I don't think it makes any sense to set a Map of that type here

    public V put(K key, V value) {
        throw new UnsupportedOperationException();
    }
  1. org.apache.dubbo.config.AbstractMethodConfig#getParameters

Prevents this function from returning null

    public Map<String, String> getParameters() {
        return parameters;
    }

---->

    public Map<String, String> getParameters() {
        this.parameters = Optional.ofNullable(this.parameters).orElseGet(HashMap::new);
        return this.parameters;
    }

Verifying this change

......

Copy link
Member

@AlbumenJ AlbumenJ left a comment

Choose a reason for hiding this comment

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

BTW, pls replace the PR subject as a considerable one and comment in English.

import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.*;
Copy link
Member

Choose a reason for hiding this comment

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

Please do not use comma import

public static Map<String, String> convertParameters(String[] parameters) {
if (ArrayUtils.isEmpty(parameters)) {
return Collections.emptyMap();
return new HashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

Null return in org.apache.dubbo.config.AbstractMethodConfig#getParameters is also need to be considered

…eters中put对象为空时,返回空的HashMap而不是EmptyMap以支持后续put
@YAGAMIL YAGAMIL force-pushed the 3.2-convertParamtersFix branch from 797ed4b to 8270187 Compare May 19, 2023 08:03
@YAGAMIL YAGAMIL changed the title org.apache.dubbo.config.spring.util.DubboAnnotationUtils.convertParam… When PropertyValue "parameters" is [null] or [EMPTY], return to HashMap that can be modified May 19, 2023

public Map<String, String> getParameters() {
return parameters;
this.parameters = Optional.ofNullable(parameters).orElseGet(HashMap::new);
Copy link
Member

Choose a reason for hiding this comment

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

Set the newly created map to this.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@YAGAMIL YAGAMIL requested a review from AlbumenJ May 19, 2023 12:09
@AlbumenJ AlbumenJ merged commit 9b9688c into apache:3.2 May 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants