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

Built-in support for Guava collections #2742

Open
eamonnmcmanus opened this issue Sep 17, 2024 · 3 comments
Open

Built-in support for Guava collections #2742

eamonnmcmanus opened this issue Sep 17, 2024 · 3 comments

Comments

@eamonnmcmanus
Copy link
Member

eamonnmcmanus commented Sep 17, 2024

Problem solved by the feature

Although Gson and Guava are both Google open-source projects, Gson does not have any understanding of Guava collection-related types such as ImmutableList and ImmutableSortedMap. There is at least one open-source project that provides this functionality, but it does not currently seem to be released anywhere accessible. Internally, Google seems to have at least 12 implementations of ImmutableListTypeAdapterFactory.

Feature description

Make it so that a default Gson instance knows how to deserialize into ImmutableList<String> and the like, if Guava is on the classpath. We can use reflection and an <optional> dependency to ensure that Gson does not pull in Guava if it is not already used elsewhere in a project.

Alternatives / workarounds

Build guava-gson-serializers manually, or write your own TypeAdapterFactory.

@Marcono1234
Copy link
Collaborator

Marcono1234 commented Sep 17, 2024

Will that require java8 Issues related to making Java 8 the minimum supported version since Guava has Java 8 as minimum version? Or do you want to implement this fully using reflection (which seems rather error-prone / verbose)?

@eamonnmcmanus
Copy link
Member Author

I was planning to log a separate issue about requiring Java 8 by default. I think we can reasonably drop support for Java 7 at this point. Having Java 8 would mean that we might be able to have a better solution for classes like java.time.Instant, for example.

@eamonnmcmanus
Copy link
Member Author

I'm logging this issue now because when I tried running #2068 against Google's internal tests, nearly all of the failures were due to trying to deserialize ImmutableList<Something> without actually having a TypeAdapter for that. That happens to work at the moment in some circumstances, but it won't with #2068. I was able to fix the failures by registering a TypeAdapter in each case, but it was tedious. It would be much simpler to land that change if ImmutableList just worked. (And of course it will be beneficial to others in the same situation.)

Migrating to Java 8 also makes it much easier to write the TypeAdapter, since we can do this sort of thing:

  private static final ImmutableMap<Class<?>, Function<List<?>, Collection<?>>> COLLECTION_TYPES =
      ImmutableMap.of(
          ImmutableList.class,
          ImmutableList::copyOf,
          ImmutableSet.class,
          ImmutableSet::copyOf,
          ImmutableSortedSet.class,
          ImmutableSortedSet::copyOf);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants