Review collection/map converter optimizations that don't create new collection instances [SPR-6213]

[spring-projects-issues]

Lance Arlaus opened SPR-6213 and commented

See comments below
—

This bug was originally encountered while converting lists. Passing in a list for conversion to list, one would expect a new list containing the elements of the original list.
Instead, the converted object that's returned is the original list itself.

The test case below demonstrates this.

Note that this test succeeds in version 3.0.0.M4

public class DefaultConversionServiceBugTest {

	@Test
	@SuppressWarnings("unchecked")
	public void testUnmodifiableListConversion() {
		List<String> stringList = new ArrayList<String>();
		stringList.add("foo");
		stringList.add("bar");

		List<String> frozenList = Collections.unmodifiableList(stringList);
		ConversionService conversionService = new DefaultConversionService();
		
		List<String> converted = conversionService.convert(frozenList, List.class);

		// The converted list should contain all the elements in the original list
		Assert.assertEquals(frozenList, converted);
		Assert.assertNotSame(frozenList, converted);
	}
	
}

---

Affects: 3.0 RC1

Issue Links:

• Review Converter resolution algorithm [SPR-6314] #10980 Review Converter resolution algorithm

[spring-projects-issues]

Keith Donald commented

This issue should be fixed in trunk and for upcoming rc2. Can you confirm for your env?

[spring-projects-issues]

Lance Arlaus commented

This issue has not been resolved in RC2
I ran the unit test defined above which still fails

[spring-projects-issues]

Keith Donald commented

What is happening is the CollectionToCollection converter is deciding not to create a new List since the target List type is compatible with the source List type AND the element types of the source list is compatible with the target list. This is basically an optimization. We need to consider if this is desirable default behavior. I assume in your case you're concerned about the frozen list being subsequently modified after being bound to the target field?

[spring-projects-issues]

Keith Donald commented

Re-opening but also updating description and reclassifying as improvement.

[spring-projects-issues]

Lance Arlaus commented

After giving this one a little more thought, I would be inclined to stick with the current behavior of not copying the list for a noop conversion.

The responsibility of a conversion service, after all, is to convert, not to clone. The user of the conversion service can decide to copy the resulting collection. Otherwise, it's best not to incur the overhead of what could be an expensive (and undesired) operation.

[spring-projects-issues]

Keith Donald commented

We agree. Closing out.