This method directly calls the LinkedHashMap # get of the parent class, but after obtaining the value, it does not determine whether it is a null value
Comment From: pivotal-cla
@poying9464 Please sign the Contributor License Agreement!
Click here to manually synchronize the status of this Pull Request.
See the FAQ for frequently asked questions.
Comment From: pivotal-cla
@poying9464 Thank you for signing the Contributor License Agreement!
Comment From: ngocnhan-tran1996
@poying9464
Method getOrDefault
was described
Returns: the value to which the specified key is mapped, or defaultValue if this map contains no mapping for the key
LinkedCaseInsensitiveMap<V> implements Map<String, V>
, so the value return null belongs one of case
1. this map does not contains the input key
2. this input key has null value
I don't think this is a bug but I also wait for confirming from Spring team
Comment From: sdeleuze
The method getOrDefault(Object key, V defaultValue)
is annotated with @Nullable
, so the behavior currently implemented is likely the right one, so I will decline this PR.
Comment From: sbrannen
I took a look at this as well, and I think there's actually a bug in the implementation.
Specifically, I believe LinkedCaseInsensitiveMap.getOrDefault(Object, V)
should delegate to LinkedHashMap.getOrDefault(Object, V)
as follows.
public V getOrDefault(Object key, V defaultValue) {
if (key instanceof String string) {
String caseInsensitiveKey = this.caseInsensitiveKeys.get(convertKey(string));
if (caseInsensitiveKey != null) {
return this.targetMap.getOrDefault(caseInsensitiveKey, defaultValue);
}
}
return defaultValue;
}
@sdeleuze, what do you think?
Comment From: sbrannen
I took a look at this as well, and I think there's actually a bug in the implementation.
I took a closer look and realized that the if (caseInsensitiveKey != null)
effectively already performs the "contains key" logic in this.targetMap.getOrDefault(...)
.
Thus, there is no need to invoke this.targetMap.getOrDefault(...)
or perform any special handling as proposed in this PR.
@poying9464, please note that we have tests in org.springframework.util.LinkedCaseInsensitiveMapTests
which verify the expected behavior.
In light of the above, I am re-closing this issue as @sdeleuze had originally.
Comment From: poying9464
The method
getOrDefault(Object key, V defaultValue)
is annotated with@Nullable
, so the behavior currently implemented is likely the right one, so I will decline this PR.
Although this method has the @Nullable
annotation, I think that when the return result is null, the given defaultValue should be returned. It should be implemented according to the original design intention of Map#getOrDefault
Comment From: sbrannen
Hi @poying9464,
It should be implemented according to the original design intention of
Map#getOrDefault
As I mentioned above, it is already implemented according to the contract of Map#getOrDefault
.
See the following tests for details.
https://github.com/spring-projects/spring-framework/blob/1ea4eb147ad835769597309962dcceb72732b8d8/spring-core/src/test/java/org/springframework/util/LinkedCaseInsensitiveMapTests.java#L70-L92