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

Support decoding to Collection and Map subclasses #1019

Merged
merged 7 commits into from
Oct 27, 2022
Merged

Conversation

jyemin
Copy link
Contributor

@jyemin jyemin commented Oct 19, 2022

JAVA-4742

The main complication to adding this feature is that the existing IterableCodec is not able to extend a class with the parameterized types <T, C extends Collection<T>>. The same goes for MapCodec extending a class with the parameterized types <T, M extends Map<String, T>>. The type system prevents this in the face of the default behavior of using ArrayList and HashMap, respectively, for the existing public constructors. Also, since Iterable does not have an add method, it's problematic to add a decode method that supports parameterization with Iterable.

I tried a bunch of experiments to get these codecs to work with this new feature, failed, and went with the following approach:

  • Revert the recent changes to IterableCodec and MapCodec that added support for parameterization. Since they are part of the API they can't be removed, but they are deprecated.
  • Add new CollectionCodec and MapCodecV2 codecs that don't suffer from the same problems
  • Add CollectionCodecProvider and use it in all the default registries ahead of IterableCodecProvider (which has to remain to support case where it actually is an Iterable that is not a Collection. Without this we can't encode anything created by our own com.mongodb.internal.Iterables class, for example. )
  • Create instances of MapCodecV2 instead of MapCodec in MapCodecProvider

@jyemin jyemin self-assigned this Oct 19, 2022
…ent behavior of being able to provide a Codec for an Iterable that is not a Collection
@rozza rozza requested review from rozza and removed request for rozza October 21, 2022 08:28
@rozza
Copy link
Member

rozza commented Oct 21, 2022

I'm of the opinion that if we require the IterableCodec / IterableCodecProvider it should not be deprecated but the suggestion to use the CollectionCodecProvider instead works.

If we really want to deprecate it, it might be better to do so it in a separate ticket and refactor our code that requires it and remove it from the default registry (and accept it might break some niche usecases).

@jyemin jyemin requested a review from stIncMale October 26, 2022 19:52
@jyemin jyemin merged commit a999e15 into mongodb:master Oct 27, 2022
@jyemin jyemin deleted the j4742 branch October 27, 2022 03:02
ispringer pushed a commit to evergage/mongo-java-driver that referenced this pull request May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants