If You create MultiValueMap via MultiValueMapAdapter, and value list is empty, call of method getFirst fail on IndexOutOfBounds exception:

private static class MultiValueMapAdapter<K, V> implements MultiValueMap<K, V>, Serializable {

        private final Map<K, List<V>> map;

        public MultiValueMapAdapter(Map<K, List<V>> map) {
            Assert.notNull(map, "'map' must not be null");
            this.map = map;
        }

        @Override
        @Nullable
        public V getFirst(K key) {
            List<V> values = this.map.get(key);
            return (values != null ? values.get(0) : null);
        }

Good solution is:

return (values != null && !values.isEmpty() ? values.get(0) : null);

Comment From: jhoeller

The common scenario is that such a list either contains one or more values or isn't set at all (in case of no value associated with a key). That said, point taken, we should align this with the existing LinkedMultiValueMap implementation where such a defensive emptiness check is present already.

Comment From: vlavasa

The common scenario is that such a list either contains one or more values or isn't set at all (in case of no value associated with a key). That said, point taken, we should align this with the existing LinkedMultiValueMap implementation where such a defensive emptiness check is present already.

OK, but the adapter is used in method: org.springframework.util.CollectionUtils#toMultiValueMap

and there is accepted any Map> map

Comment From: jhoeller

Good point, it can be constructed with any target map content, including empty list values there. Also, an addAll call with an empty list argument will also lead to such empty list state, just like in LinkedMultiValueMap. I was just pointing out why it is uncommon to encounter empty lists there, otherwise it would be surprising for this bug to remain unnoticed so many years.

Thanks for raising this, in any case! I've marked it for a fix in the upcoming 5.2.7 and 5.1.16 releases.

Comment From: midumitrescu

@jhoeller could I give it a go?

I have also prepared unit tests.

Comment From: jhoeller

@midumitrescu I got a local commit for 5.2.x staged along with a few others, using the same code that we have in LinkedMultiValueMap (with an && !values.isEmpty() check); I'll forward-merge this to master ASAP. So thanks for volunteering but I'm afraid it's effectively done already :-)

If you got unit tests for MultiValueMapAdapter, I'd appreciate if you could subsequently submit them as a separate PR. We got quite a few tests for LinkedMultiValueMap but no isolated ones for CollectionUtils.toMultiValueMap, as far as I can see, so there's definitely a gap to close there.

Comment From: jhoeller

@midumitrescu I see that your commit uses CollectionUtils.firstElement, another interesting solution. I nevertheless prefer alignment with the LinkedMultiValueMap implementation without any further method delegation involved, as in my locally staged commit.

To be clear about the unit tests, we don't have any at the moment. We'd benefit from complete unit tests for CollectionUtils.toMultiValueMap, to the extent that LinkedMultiValueMapTests covers the typical interaction patterns already. A contribution would be very welcome there!

Comment From: midumitrescu

@jhoeller Sorry, I did not see your message in time. I'll close my PR.

To not forget, 

toSingleValueMap

also calls 

list.get(0)

Producing basically the same ArrayIndexOutOfBoundsException.

By not wanting to duplicate the behavior I came to 

Collections.firstElement()

Just let me know when to add the unit tests. The thing is, I created only those necessary for fixing this bug.

However, I can gladly also complete them with some meaningful ones.

Comment From: jhoeller

That's a good catch indeed, my 5.2.x commit has this aligned with the LinkedMultiValueMap implementation as well where we are performing the same !values.isEmpty() check in toSingleValueMap. It's a matter of implementation style and therefore always debatable; we tend to keep our low-level utilities as decoupled from our own convenience methods as possible. From that perspective, LinkedMultiValueMap should not depend on CollectionUtils (which is meant to be working on top of collection implementations), and the same goes for the internal MultiValueMapAdapter... even if the latter is nested within CollectionUtils itself.

As for the unit tests, LinkedMultiValueMapTests is a good starting point. MultiValueMapAdapter should receive the same coverage there. So if you got some cycles left for volunteering, a PR for that part would be much appreciated :-) Otherwise we can also cover the unit tests at a later point.

Comment From: midumitrescu

@jhoeller I think I needed a bit of time to understand what you mean by aligning implementations.

Are you meaning 

    public static <K, V> MultiValueMap<K, V> toMultiValueMap(Map<K, List<V>> map) {
        return new LinkedMultiValueMap<>(map);
    }

Or is the MultiValueMapAdapter really needed?

Comment From: jhoeller

At this point it's just about coding the same methods the same way in both implementations. You got a point though, except for LinkedMultiValueMap always using an independent internal LinkedHashMap (and not a given external Map as-is), the implementations are almost the same. I'll see what I can do to unify them, maybe extracting a common base class... although we'd only do that in our recent branches, not in the backports.

Comment From: midumitrescu

Understood. I would be interested in giving a hand there, if needed.

If not, I will attend the unit tests now.

Comment From: jhoeller

I ended up moving the existing MultiValueMapAdapter class to top level (but package-visible), extending it in LinkedMultiValueMap and therefore reusing most of the implementation (and putting it into a single place for maintenance purposes).