Framework: build redux store from scratch if persisted data is too old

[gwwar]

If persisted data for our redux store is too old ( > 7 days ) we build the redux store from scratch. This also adds missing tests for initial-state.js which also covers some initial server state behavior

Testing Instructions

In development.json set persist-redux to true
In the console set localStorage debug to 'calypso:state'
Start up Calypso and navigate. The persisted store should load correctly without issues.
Tests in client/state pass

cc @rralian @aduth @mtias @dmsnell @ockham @seear

[seear]

The addTimestamp/removeTimestamp calls could maybe be moved inside the serialize/deserialize functions, making this line a little simpler?

[gwwar]

👍 added in e57ad3f

[seear]

Interested in how the 7 day figure was arrived at?

[seear]

Bootstrapping is still working fine on this branch 👍

[aduth]

I'm all for leveraging dependencies, but this seems a bit... overkill? 😄 Can't we just calculate this in advance and leave a comment // 7 days ?

[gwwar]

Perhaps something like this is clear enough?

const MAX_AGE = 7 * 24 * 60 * 60 * 1000;

[aduth]

Perhaps something like this is clear enough?

Not very clear to me 😄

I'd suggest:

const MAX_AGE = 604800000; // 7 days

Or if you think it's important to explain the conversion:

const MAX_AGE = 604800000; // 7 days = 7 * 24 * 60 * 60 * 1000

[dmsnell]

It's nice on the PHP side of things because we use constants that are easily searchable

const MAX_AGE = 7 * DAY_IN_HOURS * HOUR_IN_MS

or something like that. different here because of the s vs ms thing

[gwwar]

Thanks for the suggestions! I'll update with some constants. The problem with simply adding a comment is that they can get out of sync with the value, which is why I added in the micro lib. I've used it in the past where folks were tired of redefining constants in separate files.

[gwwar]

Updated in e57ad3f

[aduth]

I think an ideal mechanism would be for the expiration to apply to individual segments of the state tree, not the entire tree. It'll be a bit abrupt that if my next session happens to fall on the lapse date, everything known in the persisted store is forgotten. Especially concerning if we're relying on this behavior in the desktop app to provide an offline experience if a user launches it without network connectivity (correct me if this is not a goal of persistence).

Another preference might be towards the store initialization not managing the expiration of items, but rather the reducer perform its own clean-up in receiving new data. So rather than risk objects ever being deleted during startup, they'd only be cleaned up as new items are received.

That said, I think this would be a fine direction for a short-term solution to the problem.

[gwwar]

Thanks for the reviews!

Interested in how the 7 day figure was arrived at?

Completely arbitrary. This is more of an interim solution to make sure we have some mechanism to avoid completely eating all available browser storage. The chosen time seems long enough that devs would be able to sniff out any issues. We can certainly tune this as needed or replace this mechanism as our needs change.

I think an ideal mechanism would be for the expiration to apply to individual segments of the state tree, not the entire tree.

No arguments here, we'll iterate on this. I'm also interested to see if it make sense to store subtrees in different keys as well. As our tree grows, I'd be concerned with serialization costs if we continue to use our current method.

Another preference might be towards the store initialization not managing the expiration of items, but rather the reducer perform its own clean-up in receiving new data.

Completely reasonable, though it does put additional burden on anyone implementing reducers. Initialization is a very convenient place to change our data handling strategies.

[aduth]

Definitely not necessary to change, though as an FYI, you don't need to assign the spy to a variable. Instead:

sinon.spy( console, 'error' );
expect( console.error.called ).to.equal( false );
console.error.restore();

[gwwar]

The bizarre thing was that at my last place we would sometimes see restore() fail 1 out of xx times when using this approach. It was probably due to some leaky test cleanup with a huge test suite, but I've gotten into the habit of storing it in a variable.

[aduth]

Future iterations notwithstanding, I think this is a good first step to address the immediate concern of stale / large data persistence 👍

[gwwar]

Thanks for the reviews! I'm squishing commits.