AbstractMultimap.replaceValues(Iterable) throws Exception when trying to replace a collection with itself.

[GoogleCodeExporter]

When calling replaceValues on a Multimap using the same collection being 
replaced as a parameter, the method throws a ConcurrentModificationException. 
This is because AbstractMultimap.replaceValues takes the collection being 
replaced, clears it, and then iterates over the parameter to fill it. It should 
check if the parameter and the old collection are the same instance, and if 
that happens, it shouldn't do anything.
Here's a test to reproduce the problem:

    public void testMultimap() {
        SortedSetMultimap<Long, Long> multimap = TreeMultimap.create();
        multimap.put(1L, 2L);

        multimap.replaceValues(1L, multimap.get(1L));
    }

In case this use isn't allowed, it should throw a more declarative Exception 
and explain this on the method's javadoc.

Original issue reported on code.google.com by cau...@gmail.com on 19 Nov 2013 at 7:48

[GoogleCodeExporter]

Throwing CME seems like a reasonable behavior to me. (What does 
aHashSet.addAll(aHashSet) do?)

Original comment by kevinb@google.com on 20 Nov 2013 at 8:11

• Added labels: Package-Collect, Type-Defect

[GoogleCodeExporter]

That was my first reaction, too. This case is slightly different, though: It's 
more like aHashSet.setMyContentsToExactly(aHashSet). That's still arguably 
broken, and of course it's a fictitious method, so there's no real precedent 
here. And I haven't heard of this problem before in the long time that Multimap 
has been around. I just say all this to point out that the addAll precedent is 
inexact.

Original comment by cpov...@google.com on 20 Nov 2013 at 8:16

[GoogleCodeExporter]

I disagree with both aHashSet.addAll(aHashSet) and 
aHashSet.setMyContentsToExactly(aHashSet) examples.
One possible interpretation of a multimap is a Map<Key, Collection<Value>>. 
When you have an implementation such as SortedSetMultimap, you see that the 
underlying collections associated with each key have an importance, and the 
multimap is not just a map that allows repetitions.
If CME is a reasonable behavior, then it should also be thrown in this scenario:
    public void testMap() {
        Map<Long, Long> map = Maps.newHashMap();
        map.put(1L, 2L);

        map.put(1L, map.get(1L));
    }

You can emulate this using
        multimap.asMap().put(1L, multimap.get(1L));
And this throws an UnsupportedOperationException

Original comment by cau...@gmail.com on 21 Nov 2013 at 11:50

[GoogleCodeExporter]

This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:10

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:17

• Changed state: Migrated

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08

• Added labels: Restrict-AddIssueComment-Commit