Serialization part 1: implement kotlinx-serializable, use serialized AddedViews

[mzorz]

First part in the path to close #393 and related.

This PR implements Kotlinx's serialization plugin so we can save / inflate current state of things for a given Story that is in memory in the StoryRepository, which is generally work in progress for the time being.

This will also set the base work for supporting Story projects in the future.

This PR provides a means for AddedViews to be serialized and then recreated from AddedViewInfo serialized class, which contains properties that are useful to re-place the same views on a given Story slide each time (scale, rotation, translation, and text information such as text, calculated fontSize, and color).

Example:

{
  "viewType": "TEXT",
  "viewInfo": {
    "rotation": 0,
    "translationX": 0,
    "translationY": 0,
    "scale": 1,
    "addedViewTextInfo": {
      "text": "hello",
      "fontSizePx": 60,
      "textColor": -490240
    }
  },
  "uri": null
}

Some notes on this:

• the textColor being serialized to a negative number, which may not look super elegant - we don't really care for now as long as it remains a two way operation; see the following section about normalization
• fontSizePx expressed in pixels: given we're only 'remembering' the value to re-create things on the same device, it's easier to keep it this way so we don't need to convert back and forth from scaled pixels (SP). Same notes about normalization applies.
• translation values are always relative to the device screen size

Why this is important

What this effectively means is, we can now keep the state for AddedViews (emoji, text) and be able to re-create them on the fly from this serialized format 🎉 👍

• we can now save (not part of this PR, but sets the base work for it) stuff, no longer should we lose a users's work when switching apps and OS decides to kill the process
• we could save project drafts and start them back later 💯
• we could further refine the format and normalize it to make it platform-agnostic (fontSize, textColor, and add some other information we're sourcing from elsewhere in the underlying platform such as the canvas size we're working on, etc) and eventually be able to share projects among platforms (iOS, Android, desktop).

Notes

• there's a new version of https://github.com/Kotlin/kotlinx.serialization that we might want to use, but it needs Kotlin Version 1.3.70 (we're still on 1.3.61 on WPAndroid) and it is incompatible with the previous version. FWIW, our stuff still works on version v.0.14.0 of the library so, sticking to that one for now.
  https://github.com/Kotlin/kotlinx.serialization/releases/tag/v0.14.0

To test

• apply this gist here https://gist.github.com/mzorz/7e3649c42277cbb369f9c613b4873a09
• this gist does the following: each time a switch between Story slides is performed, it will serialize the AddedViews (emoji, text), deserialize them and replace the original objects with the ones created from serialization.
• observe you can switch slides and objects will appear in the same place (translation) and respect rotation and size as you left them when placing them.

Here's a video of the app with this gist applied, showing the AddedViews get re-created correctly when tapping on different frames/slides on the frame selector, in https://cloudup.com/cIO-aA3b7ek

ToDo

• Proguard rules, see https://github.com/Kotlin/kotlinx.serialization (done in ef642bb)

[peril-automattic]

You can test the changes on this Pull Request by downloading the APK here.

[jd-alexander]

should this be isViewReady? Not sure. Just checking :)

Suggested change

	isViewReadd: Boolean = false
	isViewReady: Boolean = false

[mzorz]

it's intended - is this view being re-added.

[jd-alexander]

Okay understood. I get it now. Do you think it could be renamed to isViewBeingReadded? LMK. because at first glance I thought it was a typo 😅

[mzorz]

sure thing! done in 3713a9b :)

[mzorz]

@mzorz just to clarify this is now ready for review even though #435 is still a draft?

That's correct, this one can be reviewed. Will be lifting the draft status from #435 shortly.

[jd-alexander]

I left a few comments @mzorz but overall this is amazingly done! I have learned so much from reviewing this PR. I will be doing a second pass and running some tests soon!

[jd-alexander]

Thanks for these changes @mzorz I left a few minor comments but this is good to go LGTM 🚢

[mzorz]

Addressed your comments, thanks a lot for these suggestions @jd-alexander 🙇 - ready for another go 👍

[aforcier]

the textColor being serialized to a negative number, which may not look super elegant - we don't really care for now as long as it remains a two way operation; see the following section about normalization

I was a bit worried about this in case those int values worked like string resources, which IIRC shouldn't be relied on for something like serialization since they are not necessarily the same from build to build, and was going to suggest we store the color value in hexadecimal form.

However, after going down an unironically fun rabbithole, it looks like we're okay. It turns out that's just how android.graphics.Color represents colors, and they're resolvable back to hex (with alpha value). So in your example, -490240 is the int representation of #fff88500.

Integer.toHexString(-490240) // "fff88500"
// And to go back
"fff88500".toLong(16).toInt() // -490240

Apparently there are a few different ways Android might represent the number with some color space changes as of Android O, but I tried pre-Android O and the representation seems to be the same. The risk then would be that someone updates their version of Android and the representation changes (so the serialized version we stored doesn't match what is expected), but that seems a bit far-fetched since everything seems to work in recent Android versions. And if we did run into a weird issue here we can add some extra logic to handle that exceptional representation.

So none of this is important, just sharing 😀 It's also something for me to think about when I add typeface support, since that may be a case where we need to store our own identifier instead of the integer resource value Android will probably be using to represent a typeface.

[aforcier]

I'm still planning to give the code a functional test as well, but something on my mind from looking things over: what support does kotlin-serialization have for versioning? Put another way, if after release we add or change the name of one of the keys, how is the attempt to deserialize from an older version handled?

[aforcier]

Applied the gist and gave this a try, working well!

I'll try out part 2 and the WPAndroid changes next week.

I did notice that sometimes added views jump around slightly if the last action on them is a resize before they're reloaded:

https://cloudup.com/cE-45g0j-zB

But it's not a big deal.

[jd-alexander]

Thanks @mzorz I did the second pass. Good work. LGTM 🚢

[mzorz]

Thanks for your reviews @aforcier @jd-alexander 🙇

@aforcier loved your investigation re: representation of some values there :), I think you have a point that these constants may change if you update the OS, and even more we'll have to come up with a more robust representation that can remain objective throughout devices / OS versions / platforms in the future for sure if we want to share the serialized data.

Agreed on the typeface support, we'd certainly want to have that under control so better come up with our own way to identify the one being used 👍

Also saw the video with the small glitch with added view positioning, thanks for noting it 👍

[mzorz]

Re: this #414 (comment)

I'm still planning to give the code a functional test as well, but something on my mind from looking things over: what support does kotlin-serialization have for versioning? Put another way, if after release we add or change the name of one of the keys, how is the attempt to deserialize from an older version handled?

Good question @aforcier 💯

I haven't seen "native" support for versioning in kotlinx-serialization at the plugin level, a first guess that comes to mind is that we can always rely on a mechanism similar to what Java has (declaring a serialVersionUID in a companion object for each class we are declaring as @Serializable here)
In the case we have to rollout our own versioning, we can also try and go along the lines of what we do in WPAndroid with DB_VERSION for databases (since this is data that is going to be persisted, sounds like a possibility).

[...]

Looking a bit further it seems we can choose the format in which things are stored, these are already supported:

• JSON
• CBOR
• Protobuf

Also some others can be installed (status for some is experimental):

• HOCON
• Properties
• community-supported: Avro, Bson, XML, YAML

Did some digging over these few formats, seems to throw the notion that versioning handling is (unsurprisingly) controversial:

• https://cbor.io/
  "The Concise Binary Object Representation (CBOR) is a data format whose design goals include the possibility of extremely small code size, fairly small message size, and extensibility without the need for version negotiation.”
  This approach sounds pretty much like JSON in the sense JSON is pretty much free-form and it's left to the application level to decide what to do with things. In our case, the serializer would construct an object of the given type (this depends on the serializer used / expected) and then fill in the fields it finds (if some fields are missing, it'll just provide an object with its fields uninitialized).
  Digging a bit more, found in fact the RFC says their data model is based on the JSON data model - read the Introduction section.

• Protobuf seems more rigid in this aspect - there's some notes I found on how to handle legacy fields (fields we'd want to remove on a later version of a serialized object) to keep them but mark them reserved so these can't be re-used:
  Also checking above there's an interesting section, look at this:
  "
  Required Is Forever You should be very careful about marking fields as required. If at some point you wish to stop writing or sending a required field, it will be problematic to change the field to an optional field – old readers will consider messages without this field to be incomplete and may reject or drop them unintentionally. You should consider writing application-specific custom validation routines for your buffers instead. Some engineers at Google have come to the conclusion that using required does more harm than good; they prefer to use only optional and repeated. However, this view is not universal.
  "

• HOCON is a superset of JSON aimed at making it more readable (that's their aim - so no special support for versioning there)

The only one that we know supports versioning by the format itself is, well, XML, but it kind of defeats the purpose as we'd need to deal with the other complexities it brings.

So basically I think we'll need to rollout our own versioning scheme, the simplest path forward may be at least to have a flavor of serialVersionID, maybe some other data such as platform (when we go multiplatform) and then let the application level make interpretations based on this information.

Let me know your thoughts @aforcier 👍

[aforcier]

Really nice exploration @mzorz!

I agree with your conclusion, let's rollout our own scheme, starting the the bare minimum. A serialVersionID equivalent sounds good, plus the basics of checking against it (only one logic branch for now, but let's have it prepared for future use I'd say?). And you're right, we may as well add a platform=android identifier as well for when we eventually have a need for it. 👍

[mzorz]

I agree with your conclusion, let's rollout our own scheme, starting the the bare minimum.

Good! tracked here #452