Make more classes serializable

[GoogleCodeExporter]

Here are some classes that are not serializable that probably should be or at 
least might be worth making serializable.

com.google.common.base
  CharMatcher? (they're useful in predicates, but looks like more effort and apparently hasn't been needed thus far)
  Enums.valueOfFunction
  EquivalenceWrapper
  Optional

com.google.common.collect
  DiscreteDomain (if not the abstract class, the provided implementations)
  MinMaxPriorityQueue?
  Range

com.google.common.net
  HostSpecifier
  HostAndPort
  InternetDomainName

Original issue reported on code.google.com by cgdec...@gmail.com on 29 Apr 2011 at 4:05

[GoogleCodeExporter]

I agree that there seems to be room for more Serializable marking in Guava. off 
the top of my head I would add Joiner to that list.

Original comment by bj...@soldal.org on 6 May 2011 at 1:23

[GoogleCodeExporter]

Any chance of any of these being made serializable for r10? I'd particularly 
like to see Optional and Range serializable. I've written up changes to make 
most of these serializable, so if it would help I could submit a patch.

Original comment by cgdec...@gmail.com on 10 Jun 2011 at 2:19

[GoogleCodeExporter]

Suuuure, why not?  Most of the above seem uncontroversial enough, though I 
wouldn't bother with CharMatcher or MMPQ yet. Maybe not Host* at first if 
you're not in a rush to get them.  We need to have you sign a contributor 
agreement btw, tho I'm too lazy to look that up at this hour.

Original comment by kevin...@gmail.com on 10 Jun 2011 at 6:19

[GoogleCodeExporter]

Yeah, I hadn't bothered to do CharMatcher yet anyway. I included MMPQ and the 
net package classes mostly for consistency with PriorityQueue and stuff like 
InetAddress (and because the net package classes were so easy to make 
serializable) so I'm fine with leaving them out.

I signed a contributor agreement in April of last year when I contributed a 
patch to Guice... do I need to do that again? Anyway, I'll try to put the patch 
together tonight sometime when I'm at home.

Original comment by cgdec...@gmail.com on 10 Jun 2011 at 3:39

[GoogleCodeExporter]

Ok, patch is attached. It makes Enums.valueOfFunction, EquivalenceWrapper, 
Optional, Holder, Range (and Cut), the DiscreteDomain implementations and 
ContiguousSet serializable. I decided to make ContiguousSet require a 
serializable DiscreteDomain rather than allowing it to try to use normal 
ImmutableSortedSet serialization. I also made DiscreteDomain's constructor 
protected to allow other users to implement it... if you don't want that to be 
possible for now feel free to take that out. Also, I don't know much about GWT 
serialization so if there's anything special that needs to be done for any of 
these related to that, I didn't do it. =)

Original comment by cgdec...@gmail.com on 11 Jun 2011 at 5:52

Attachments:

• guava-615.patch

[GoogleCodeExporter]

Anyone had a chance to look at this?

Original comment by cgdec...@gmail.com on 24 Jun 2011 at 9:07

[GoogleCodeExporter]

We've started looking into *looking into* it!  Here, have a milestone tag.

Original comment by kevinb@google.com on 27 Jun 2011 at 2:29

• Changed state: Accepted
• Added labels: Milestone-Release10

[GoogleCodeExporter]

Original comment by kevinb@google.com on 13 Jul 2011 at 6:18

• Changed state: Triaged

[GoogleCodeExporter]

Original comment by kevinb@google.com on 13 Jul 2011 at 7:26

• Changed state: Accepted
• Added labels: Type-Enhancement

[GoogleCodeExporter]

Issue 599 has been merged into this issue.

Original comment by kevinb@google.com on 13 Jul 2011 at 7:35

[GoogleCodeExporter]

Colin, we've applied your patch (with the exception of Holder, which we're not 
really sure we want to be used widely), and it will be in release 10.

We also found that we could make ContiguousSet serializable simply with 
"implements Serializable" without needed the SerializedForm class.  (Actually, 
it's now a new RegularContiguousSet class that the code goes in.)  Did you find 
this not to be the case, or is there some other reason to handle ContiguousSet 
but not the other classes in this way?  Thanks.

Original comment by cpov...@google.com on 1 Aug 2011 at 4:25

• Removed labels: Milestone-Release10

[GoogleCodeExporter]

I used SerializedForm because otherwise ImmutableSortedSet's serialization code 
gets used, resulting in the ContiguousSet deserializing to something that isn't 
a ContiguousSet (assuming an array of all the elements fits in memory at all). 
Looking at it again, I suppose I could have made readObject in 
ImmutableSortedSet package-private instead of private and then overridden it 
and writeReplace in (Regular)ContiguousSet to do default things. Or shifted 
around the serialization code in ImmutableSortedSet's hierarchy even more, 
perhaps. Or maybe I'm missing something here?

Original comment by cgdec...@gmail.com on 1 Aug 2011 at 4:59

[GoogleCodeExporter]

To clarify, serializing a ContiguousSet like Ranges.closed(1, 10).asSet() 
results in a RegularImmutableSortedSet on deserialization because 
ImmutableSortedSet does its own writeReplace (which will cause an OOME for a 
large enough range when it calls toArray on the set).

Original comment by cgdec...@gmail.com on 1 Aug 2011 at 7:58

[GoogleCodeExporter]

Whoa, thanks.  I just submitted a CL to use your original code and test for the 
problem you mentioned.

Original comment by cpov...@google.com on 1 Aug 2011 at 8:01

[GoogleCodeExporter]

I think EmptyContiguousSet has the same issue (it would deserialize to an 
EmptyImmutableSortedSet). I've attached another patch that gives it a 
serialized form as well. It also removes "implements Serializable" on 
RegularContiguousSet since ImmutableCollection is already declared 
Serializable. For some reason my compiler is crashing when I try to build Guava 
so I haven't been able to try the changes out, but they're pretty 
straightforward so I think they should work.

Original comment by cgdec...@gmail.com on 2 Aug 2011 at 3:04

Attachments:

• contiguous-set-serialization.patch

[GoogleCodeExporter]

Original comment by cpov...@google.com on 2 Aug 2011 at 4:17

• Added labels: Milestone-Release10

[GoogleCodeExporter]

Original comment by cpov...@google.com on 2 Aug 2011 at 4:17

[GoogleCodeExporter]

EmptyContiguousSet fixed with your patch, thanks.  We also added some tests to 
catch future problems of this nature.

Original comment by cpov...@google.com on 4 Aug 2011 at 6:38

• Removed labels: Milestone-Release10

[GoogleCodeExporter]

I would enjoy having a serializable WrappedSet in HashMultimap, so that the 
HashMultimap may become serializable. I started to customize this by myself but 
all final/private make the thing very difficult.

http://stackoverflow.com/questions/8488394/guava-setmultimap-not-serializable-du
e-to-not-serializable-wrappedset

Original comment by martin.p...@gmail.com on 13 Dec 2011 at 2:02

[GoogleCodeExporter]

[off topic]

While Guava does discourage extensibility through inheritance (for good 
reasons), you *could* still do it using the decorator pattern, which Guava 
encourages.

To do this, you'd need to extend ForwardingMultimap or ForwardingSetMultimap, 
delegate to the HashMultimap, and override the readResolve() and writeReplace() 
methods to serialize the Multimap as you see fit (e.g. using a serialization 
proxy).

http://docs.guava-libraries.googlecode.com/git-history/v10.0.1/javadoc/com/googl
e/common/collect/ForwardingSetMultimap.html

Original comment by nev...@gmail.com on 13 Dec 2011 at 9:40

[GoogleCodeExporter]

Since the majority of these classes have been made serializable and the rest I 
don't particularly feel the need to make serializable, I'm going close this 
issue. I think it makes sense for future requests for particular classes to be 
made serializable to be added as separate issues. (If this was being kept open 
for any particular reason, feel free to reopen it.)

Original comment by cgdec...@gmail.com on 13 Dec 2011 at 9:59

• Changed state: Fixed
• Added labels: Milestone-Release10

[GoogleCodeExporter]

This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:15

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:09

• Added labels: Restrict-AddIssueComment-Commit