Spec for serialization format

[RogerRiggs]

The serialization format of all JSR 310 classes. Ser needs to be documented as part of the spec.

[RichardWarburton]

Email core libs when there's a format proposal.

[RogerRiggs]

Broaden the description to apply to serialization of all persistent classes; Previously only referred to the missing spec for TZ serialization.

[RichardWarburton]

Using a serialisation delegate, with a package scoped Ser class.

[RichardWarburton]

Implement the main package on a branch and then ask @RogerRiggs to get an internal Oracle reviewer

[RichardWarburton]

I have pushed a branch to https://github.com/RichardWarburton/threeten/tree/serialisation for @RogerRiggs to get an internal reviewer to look at. Overall process is documented in javadoc of javax.time.Ser.

[jodastephen]

Some comments.

• I'll be interested in the response to the enums!
• LocalDate serialization should use bytes for month and day (just not the class LocalDate) - serialization is not padded to boundaries).
• Some methods have some weird indentation (I suspect that your IDE has 2 spaces rather than 4, and you've got a mixture in the result)
• Period has extra imports, but no extra code
• ZoneOffset already has a special form in the zone package Ser class. It should probably be copied

Overall, I think this will produce effective and well-defined serialization forms.

[RichardWarburton]

Thanks for your comments Stephen, its pretty late my end so I'll fix those issues up tomorrow.

The enums approach runs under the assumption that the TCK will define an ordering for such classes.

[RogerRiggs]

I'll ask for more comments from other Oracle folks. Comments:

• The use of methods that look like those of Externalizable (but are not) may be confusing to the casual reader.
• The symmetry of writeExternal with readExternal should be continued even the Enums; it seems to be an unnecessary shortcut.
• It says versioning is supported but not how; do I assume there will be a Ser2 class at some point?
• Does Ser need to be mutable? There are no accessors for type or object.

More later,

[RogerRiggs]

If the type byte is used for versioning then something (the type perhaps) will need to be passed to the read/writeExternal method so each class has enough information to deal with the data in the stream properly.
If new read/writeExternal methods would be needed then perhaps they should be named to include the version number so its clear what version of data they handle. Thanks to Brian G for his comments.

[jodastephen]

I'd note that the read/write external methods could all be moved to the Ser class. However, when I first came up with the pattern I found that using the current approach allowed better access to the internals of each class (less package scoped code).

Personally, I think that a version 2 of any of these specs is highly unlikely, as the state of the class pretty much defines the type, so any change to the state would need a different type.

[RogerRiggs]

While unlikely, it is prudent to make sure the versioning mechanism works before v2. The built-in serialization handles version skew in both directions since it must deal with older versions being able to interoperation with newer versions. It tolerates both adding fields and 'removing' fields. The stream protocol is robust in that it can handle missing and extra information without getting a stream corrupted error.

[RichardWarburton]

I've pushed updates to the branch in response to the feedback - see commit names for specifics - I think this fixes most raised issues.

• I think I've fixed spacing issues, somehow tabs had managed to creep in somewhere. If you spot any others poke me with specifics.
• I don't see any extra imports in Period, and nothing appears when I run git diff serialisation:src/main/java/javax/time/Period.java master:src/main/java/javax/time/Period.java locally
• Ser does need to be mutable. When it is deserialised an empty constructor is used, and its fields populated by readExternal, so its inherently mutable.
• I prefer leaving the read/write methods on the specific classes since it seems a more appropriate separation of concerns to delegate rather use getters/setters to pull data out of the class in order to serialise it. They can be moved if you wish.
• I don't think the type byte needs to be passed over. If you wish to create a version two method then it can be named something like: readExternalVersionTwo(DataInput in) and the Ser class will know to call that method from the different type byte that it has read.
• I have not renamed these methods away from readExternal/writeExternal. Its a trivial change to make, but I'm not entirely sure what's sensible - readExternalViaSer or writeExternalViaSer were cumbersome possibilities. Basically if you want a rename, take your pick!

[RichardWarburton]

I've pushed the necessary tests for the serialization format into this branch.

[RogerRiggs]

The handling of enum's is asymmetric. For each readExternal() method there should be a corresponding writeExternal(); adjecent in the code so it is easy to see they are complementary. Ser.writeInternal should call the Enum's writeExternal method and not shortcut it.

The usage of ZONE_ID_TYPE and ZONE_ID_FIXED_TYPE is matching implementation types not public types. That might be worthy of a comment. The names imply different types of ZoneId but the type is only used to reflect a customization serialization format that is more related to the implementation than the ZoneId.

[jodastephen]

Agreed to integrate Richard's code.

[RogerRiggs]

Is this integration still pending?

[jodastephen]

Yes. Not happened yet.

[jodastephen]

Merged branch (lots of git pain). Removed special enum handling. Simplified ZoneId handling. Regenerated bin files.