CollectionFactory API is not type-safe [SPR-12537]

[spring-projects-issues]

Sam Brannen opened SPR-12537 and commented

Status Quo

The changes made to CollectionFactory in conjunction with #15691 resulted in method signatures that are not type-safe, and the changes made in conjunction with #17089 further exacerbated the issue.

For example, the parameterized type E is not bound to the type of elements contained in the collection argument passed to createApproximateCollection(). Thus casting the value returned by EnumSet#copyOf(EnumSet) to (Collection<E>) can never guarantee that the returned collection actually contains elements of type E.

---

Example Tests

Each of the following tests (adapted from CollectionFactoryTests) compiles but throws a ClassCastException due to the lacking type safety of the method signatures.

@Test
public void createApproximateCollectionIsNotTypeSafeForEnumSet() {
	Collection<Integer> ints = createApproximateCollection(EnumSet.of(Color.BLUE), 3);
	ints.add(42);
}

@Test
public void createCollectionIsNotTypeSafeForEnumSet() {
	Collection<Integer> ints = createCollection(EnumSet.class, Color.class, 3);
	ints.add(42);
}

@Test
public void createApproximateMapIsNotTypeSafeForEnumMap() {
	EnumMap<Color, Integer> enumMap = new EnumMap<>(Color.class);
	enumMap.put(Color.RED, 1);
	enumMap.put(Color.BLUE, 2);
	Map<String, Integer> map = createApproximateMap(enumMap, 3);
	map.put("foo", 1);
}

@Test
public void createMapIsNotTypeSafeForEnumMap() {
	Map<String, Integer> map = createMap(EnumMap.class, Color.class, 3);
	map.put("foo", 1);
}

@Test
public void createMapIsNotTypeSafeForLinkedMultiValueMap() {
	Map<String, Integer> map = createMap(MultiValueMap.class, null, 3);
	map.put("foo", 1);
}

---

Deliverables

1. Ensure that all methods in CollectionFactory are type-safe.
   • Consider reverting to raw types, or alternatively to Collection<Object> and Map<Object, Object> as was done by the Spring Data Team.
2. Refactor CollectionFactoryTests accordingly.

---

Affects: 4.0 GA

Issue Links:

• Fix remaining compiler warnings and fail build if they return [SPR-11064] #15691 Fix remaining compiler warnings and fail build if they return ("depends on")
• Default conversion support for EnumSet / EnumMap [SPR-12483] #17089 Default conversion support for EnumSet / EnumMap ("depends on")
• CollectionFactory should create empty EnumSets and EnumMaps [SPR-12533] #17138 CollectionFactory should create empty EnumSets and EnumMaps
• Document type safety limitations of the CollectionFactory API [SPR-12596] #17197 Document type safety limitations of the CollectionFactory API ("is superseded by")

[spring-projects-issues]

Juergen Hoeller commented

While I do see the point, I don't think that this is a bug per se but rather just a tradeoff: The caller is responsible for specifying corresponding arguments and element types. Being able to assign the return value to a typed collection is a convenience which brings CollectionFactory use syntactically closer to direct collection instantiation; otherwise, the caller would have to explicitly cast to a collection of a specific element type. The caller is free to treat the return value as a raw collection or collection of Object if he chooses to avoid this convenience.

Juergen

[spring-projects-issues]

Sam Brannen commented

Sounds reasonable, and I've updated the documentation to reflect that (see #17197).