Conflict resolution when collecting into a Map

[indalaterre]

Hi all
this PR is just to add the mergeFunction supported by Java Map collector for streams.
This happen in case a collect to a non multimap is requested.

@jponge If you agree to this it would be possible to have this in the 2.8CR of quarkus?

[cescoffier]

LGTM, but let's wait for @jponge's review.

[codecov]

Codecov Report

Merging #880 (1a734c2) into main (0674f47) will increase coverage by 0.26%.
The diff coverage is 100.00%.

❗ Current head 1a734c2 differs from pull request most recent head 174824a. Consider uploading reports for the commit 174824a to get more accurate results

@@             Coverage Diff              @@
##               main     #880      +/-   ##
============================================
+ Coverage     89.08%   89.35%   +0.26%     
- Complexity     3086     3098      +12     
============================================
  Files           391      391              
  Lines         12475    12479       +4     
  Branches       1589     1591       +2     
============================================
+ Hits          11113    11150      +37     
+ Misses          714      692      -22     
+ Partials        648      637      -11     

Impacted Files	Coverage Δ
...n/java/io/smallrye/mutiny/groups/MultiCollect.java	100.00% <100.00%> (ø)
...perators/multi/processors/SerializedProcessor.java	92.07% <0.00%> (-2.98%)	⬇️
...tiny/operators/multi/MultiBufferWithTimeoutOp.java	73.10% <0.00%> (-1.69%)	⬇️
...mallrye/mutiny/operators/multi/MultiFlatMapOp.java	85.10% <0.00%> (ø)
...lrye/mutiny/operators/uni/builders/UniJoinAll.java	95.83% <0.00%> (+1.38%)	⬆️
...subscription/SwitchableSubscriptionSubscriber.java	97.60% <0.00%> (+1.59%)	⬆️
...mallrye/mutiny/helpers/queues/MpscLinkedQueue.java	98.38% <0.00%> (+1.61%)	⬆️
...java/io/smallrye/mutiny/helpers/Subscriptions.java	80.66% <0.00%> (+1.65%)	⬆️
...erators/multi/builders/SerializedMultiEmitter.java	80.48% <0.00%> (+2.43%)	⬆️
...lrye/mutiny/subscription/SerializedSubscriber.java	88.07% <0.00%> (+2.75%)	⬆️
... and 7 more

[jponge]

Great idea, see my comments for improvement requests in docs + tests.

[jponge]

💡

BTW I see that your commits get reported as "unverified" on GitHub ("No user is associated with the committer email.")

Note that for authenticity purposes we require signed commits https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits

[jponge]

You can’t check that it returns null but you can check that it is not null. You can however have a test like https://github.com/smallrye/smallrye-mutiny/blob/main/implementation/src/test/java/io/smallrye/mutiny/groups/MultiDemandPacingTest.java#L137 <https://github.com/smallrye/smallrye-mutiny/blob/main/implementation/src/test/java/io/smallrye/mutiny/groups/MultiDemandPacingTest.java#L137> to ensure that when the function returns null then the Multi fails nicely rather than having a NPE pop up from somewhere else and leaving the subscription in an inconsistent state.

…

On 21 Mar 2022, at 10:15, Mauro de Palma ***@***.***> wrote: @indalaterre commented on this pull request. In implementation/src/main/java/io/smallrye/mutiny/groups/MultiCollect.java <#880 (comment)>: > @@ -164,6 +165,28 @@ public MultiCollect(Multi<T> upstream) { return collector(upstream, Collectors.toMap(actualKM, actualVM), false); } + /** + * + * @param keyMapper a ***@***.*** Function} to map item to a key for the ***@***.*** Map}. Must not be ***@***.*** null}, + * must not produce ***@***.*** null} + * @param valueMapper a ***@***.*** Function} to map item to a value for the ***@***.*** Map}. Must not be ***@***.*** null}, + * must not produce ***@***.*** null} + * @param mergeFunction a ***@***.*** BinaryOperator} used to resolve collisions between values associated with the same key Ok the mergefunction must not be null. But how I can check that it doesn't return null. We can just put a comment in the documentation which says that the key will be removed in this case — Reply to this email directly, view it on GitHub <#880 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAGK2PYN47QKZVOVYMADXLVBA44RANCNFSM5RFIQQNA>. You are receiving this because you were mentioned.

[indalaterre]

Ok just pushed the code and the commits are signed with a GPG key

[jponge]

Thanks but the commits still show as unverified (email is missing, etc) :-)

Also, could you add a test for the case when the function returns null?

[indalaterre]

Added a test that checks the removal of the key in case of mergeFunction returns null
And I finally got the "verified" :D

[jponge]

I'll have to do a manual merge because it looks a weird on my side

[jponge]

Merged manually in main, thanks @indalaterre