Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow creation of immutable collections through CollectionFactory [SPR-16797] #21337

Open
spring-issuemaster opened this issue May 7, 2018 · 10 comments

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented May 7, 2018

Mark Paluch opened SPR-16797 and commented

Modelling application classes as immutable objects is a widely used pattern in Java and Kotlin which constantly gains adoption. We should allow creation of immutable collection types (Map, List, Set) through CollectionFactory or an adequate API that allows specification of the target type and the elements the result should contain.

It would be great to support:

  • Java 9 immutable collections
  • Google Guava collections
  • Vavr Collections (previously JavaSlang)

 


Issue Links:

2 votes, 7 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 9, 2018

John Butler commented

Juergen, I was the submitter for the DATAMONGO-1964 ticket that triggered this. I was hoping to be able to contribute. If you have not started work on it and I would have time (say a week or set as I need to set up my env for your coding standards) I would like to submit a branch. I was thinking that the simplest change to support immutables (specifically Guava's) would be to add methods in CollectionFactory that take the contents of the Collection / Map

public static <E> Collection<E> createCollectionOf(Class<?> collectionType, Iterable<? extends E> elements) {
... 
// Add support for ImmutableCollection, ImmutableList, ImmutableMultiset, ImmutableSet
} 

 

public static <K, V> Map<K, V> createMapOf(Class<?> mapType, Iteable<? extends Map.Entry<? extends K, ? extends V> elements) {... 
// Add support for ImmutableMap, ImmutableBiMap
// I don't think code can support ImmutableSortedMap because code would not know what Comparator to use
}

 

Couple questions, would it make sense for these methods to have a signature that returns the requested type instead of Collection and Map?

Something like:

 public static <C extends Collection<E>, E> C createCollectionOf(Class<C> collectionType, Iterable<? extends E> elements)

public static <M extends Map<K,V>, K, V> M createMapOf(ClassM?> mapType, Iteable<? extends Map.Entry<? extends K, ? extends V> elements)
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 9, 2018

John Butler commented

I am not sure that "Java 9 Immutable Collections" are pertinent because there is no distinct class or interface that denotes that the collection is immutable. In the case of List.of(...), it simply returns a List. Therefore the type passed to CollectionFactory would be just List with no way to determine if it was intended to be immutable. 

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 9, 2018

John Butler commented

@jhoeller I am having issues submitting / pushing my code to the spring-framework github repo. I have signed the CLA but am getting a 403. Can you provide some help? I have access to push to my own github repo just not spring-framework's.

remote: Permission to spring-projects/spring-framework.git denied to dancerjohn.
fatal: unable to access 'https://github.com/spring-projects/spring-framework.git                       /': The requested URL returned error: 403
 
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 9, 2018

Oliver Drotbohm commented

You usually create a fork of the repository into your own GitHub account, commit and push to that and then submit a pull request.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 10, 2018

John Butler commented

Pull request created:

Pull 1826

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 10, 2018

Stéphane Nicoll commented

I didn't see the conversation here before closing that PR. We have dropped our optional dependency for Guava in Spring Framework 5 and I don't think that is a good idea to reintroduce it.

 

WDYT Juergen Hoeller?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 10, 2018

John Butler commented

If you don't want the dependency, maybe consider a mechanism for injecting additional functionality similar to Jackson modules. Let me know if that would be more acceptable and I will give it a try. Note that this ticket was spawned by a request in Spring Data MongoDB which uses CollectionFactory for marshaling user domain objects. So even if spring-core does not wish to use Guava, this is preventing users of Spring Data MongoDB from using the immutable collections is their domain objects.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 10, 2018

Stéphane Nicoll commented

Thanks John Butler, let me sync with Juergen and we'll get back to you.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jun 4, 2018

John Butler commented

Hi All, any thoughts about a path forward on this?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jun 5, 2018

Juergen Hoeller commented

It looks like we won't be dealing with Vavr for 5.1 which is approaching its feature freeze soon; there are a couple of related tickets that we intend to pick up for 5.2 instead. Maybe Spring Data MongoDB can introduce Vavr and Guava support in a custom fashion for the time being, just delegating to the core CollectionFactory for regular collection types?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.