Remove null empty headers#724
Conversation
| * @see Request#headers() | ||
| */ | ||
| public Map<String, Collection<String>> headers() { | ||
| headers.forEach((key, value) -> value.removeIf(e -> e == null)); |
There was a problem hiding this comment.
I don't like the idea of modifying the existing map...
may be just filter and create new one?!
…he current map. Supporting with tests as it needs to be LinkedHashMap not to lost the sorting
|
@kdavisk6 LGTM, what do you think? Do you foresee any issues on removing empty headers? |
|
|
||
| return Collections.unmodifiableMap( | ||
| headers.entrySet().stream().filter(h -> h.getValue() != null && !h.getValue().isEmpty()) | ||
| .collect(toMap( |
There was a problem hiding this comment.
just use default toMap... toMap(Entry::getKey, Entry::getValue) no need to handle duplicated keys or map creation
There was a problem hiding this comment.
if I do so, it returns the map in any order, with LinkedHashMap it collects them in same order. In fact that was my first approach and it broke tests
There was a problem hiding this comment.
The order of the Map really should not matter, but I don't have a strong opinion on the use of LinkedHashMap. I do agree that there is no need to handle duplicates, that is, unless you want to support that. In that case, I would merge the duplicate key values, since it is acceptable to have more than one value for a header.
|
@velo I've added some feedback, I think this is ok to merge. |
* Creating headers from Request removing those with null or empty values * Moving back to let the empty strings as valid header * Returning headers filtering null and empty rather removing them for the current map. Supporting with tests as it needs to be LinkedHashMap not to lost the sorting
* Creating headers from Request removing those with null or empty values * Moving back to let the empty strings as valid header * Returning headers filtering null and empty rather removing them for the current map. Supporting with tests as it needs to be LinkedHashMap not to lost the sorting
#676