spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.5k stars 38.1k forks source link

Improve CollectionFactory to allow for single statement collection creation #28025

Closed snicoll closed 2 years ago

snicoll commented 2 years ago

Right now CollectionFactory allows to create a collection type, trying to find the most suitable implementation based on an concrete instance.

We have some use cases in AOT where we'd like to be able to streamline the creation of a collection in a single statement. We've tried something along those lines:

Stream.of(new RuntimeBeanReference("myCommandHandler")).collect(Collectors.toCollection(ManagedList::new))

Unfortunately the indirection with the collector can easily confuse the compiler. A more direct use where the collectionFactory would be provided with a bunch of elements could be nice.

sbrannen commented 2 years ago

Based on the requirements you've stated here and in https://github.com/spring-projects/spring-framework/pull/28094#issuecomment-1047754684, I've come up with the following simple prototype.

@SafeVarargs
public static <E, C extends Collection<E>> C collectionOf(Supplier<C> collectionFactory, E... elements) {
    Assert.notNull(collectionFactory, "'collectionFactory' must not be null");
    C collection = collectionFactory.get();
    Collections.addAll(collection, elements);
    return collection;
}

With the following test cases passing.

@Test
void collectionOfWorksCorrectly() {
    List<String> list = collectionOf(ArrayList::new, "foo", "bar");
    assertThat(list).isInstanceOf(ArrayList.class).containsExactly("foo", "bar");

    Set<Integer> set = collectionOf(HashSet::new, 4, 3, 2, 1);
    assertThat(set).isInstanceOf(HashSet.class).containsExactlyInAnyOrder(1, 2, 3, 4);

    Set<Integer> orderedSet = collectionOf(LinkedHashSet::new, 4, 2, 3, 1);
    assertThat(orderedSet).isInstanceOf(LinkedHashSet.class).containsExactlyInAnyOrder(4, 2, 3, 1);

    SortedSet<Integer> sortedSet = collectionOf(TreeSet::new, 4, 3, 2, 1);
    assertThat(sortedSet).isInstanceOf(TreeSet.class).containsExactly(1, 2, 3, 4);
}

@snicoll, is that what you had in mind?

Please note that the tests had to be written that way (with the created collection stored in a local variable), since there were compiler issues with AssertJ's assertThat() methods.

snicoll commented 2 years ago

Not quite. The single statement works obviously but not the type inference.

trying to find the most suitable implementation based on an concrete instance.

This is linked to AOT where we have a collection instance we'd like to write back. Taking the type of the collection is what I had in mind but I haven't dig enough. Phil also may have made that irrelevant with the latest update of code generation.

snicoll commented 2 years ago

We could call that inference algorithm based on the instance and get back the nearest type and use that for code generation. So I take it back, looks like that would work. Thanks.

sbrannen commented 2 years ago

So I take it back, looks like that would work. Thanks.

Are you saying that the proposed collectionOf() method looks good to go?

If so, I'd be happy to push it to main for 6.0 M4.

However, there's no rush on my side. So if it's not OK as-is, we can iterate over alternate solutions for 6.0 M5.

sbrannen commented 2 years ago

@snicoll, I've pushed a proof of concept to the following branch.

https://github.com/sbrannen/spring-framework/commits/issues/gh-28025-CollectionFactory-collectionOf

I think the new createCollection(Class<C>, Class<E>, E...) method may suit your needs, but I'd like to get your feedback before pushing anything to main.

Thanks!

snicoll commented 2 years ago

This is no longer needed as the AOT use cases became irrelevant.