Modify local cards on edit instead of waiting for server

[jkomoros]

A lot of performance problems happen because cards are updated by the server one at time (e.g. when the timestamp finalizes, or after updateInboundLinks fires).

One thing we could do is have it so on every edit we make local edits to the cards collection in state mirroring what we expect the server to ultimately do. Then when we receive new cards in updateCards, we compare each one to the ones we have locally, and if they're equivalent (skipping any timestamp fields) then we drop that update on the floor.

One of the reasons that updateInboundLinks is done on the server is because we can't guarantee that the editor of the card has access to all of the cards pointed to, although realistically that's very very rare (the only time it would happen is if a card already had a reference to a card the editor can't see, and then it's removed it).

Related to #538 and #517, because those problems would be obviated by this

[jkomoros]

applyCardFirebaseUpdate should be pretty useful

[jkomoros]

modifyCardWithBatch is the one callsite that needs to change-- should take another object (instead of or in addition to batch) to make the updates on

One oddity is if sections/tags change, then filters might not update until later when those changes come back, leading to out of sync behavior (which is technically possible now, too). Maybe we should move to having section/tag membership be driven entirely based on what the card itself says (as of when it was seen by updateCards), and have the section/tag firestore get be used entirely for prettyString/color/permission information, not filter membership?

... Or just live with the fact that it's possible those two are slightly different, which is true today. And plus, those should come back from firestore natively very quickly (the thing that is slow is updateInboundLinks and timestamps)

[jkomoros]

(Just use Firestore.Timestamp.now() to generate faux firestore fields)

• [x] An isFirestoreTimestamp that given an object returns true if it looks like a FirestoreTimestamp (real or faux, if there is a difference) (verify that the ones that come in via updateCards and the ones built with new Firestore.Timestamp() work the same
• [x] applyCardFirebaseUpdate grows a replaceFirestoreTimestamps, that, if true, when it sees a firestoreTimestampSentinel, replaces it with a Firestore.Timestamp. We'll use that in modifyCardWithBatch
• [x] modifyCardWithBatch takes an optional updatedCardsOverlay, and currentCards. If both are provided, then updatedCardsOverlay is DIRECTLY modified with a copy of currentCards, combined with the diff, into updatedCardsOverlay. All serverTimestamps are replaced with firestoreTimestamp (audit if those are generated anywhere else in the diff that we have to look for or just updated and updated_substantive fields)
• [x] modifyCards calls updateCards with the updatedCardOverlay
• [x] deepRoughlyEqual checks for equality, ignoring timestamp fields and pretending they are all the same
• [x] updateCards, when provided with cards, checks to see if they already exist in the existing set; if they do and are deepRoughlyEqual then it skips that one
• [x] Verify that updateCards, in the common case, doesn't do many deepEqualsIgnoringTimestamps
• [x] modifyCards calls updateCards directly with the overlay (verify that second update is indeed dropped, and see which one happens first...)
• [x] modifyCardWithBatch does the inboundLinks updates of referenced cards; copying them into updatedCardsOverlay if not yet there and modifying in place (so that edits to multiple build up). Note this might cause a thrashing problem if we get the updated/final timestamp back from server and THEN we get the updated inboundLinks back in a second server callback. (But most cards are either directly affected (serverTimestamp), or indirectly affected (no server timestamps), so it shouldn't be an edge case that triggers very often, and would only be a small amount of thrash anyway)
• [ ] Decide if it's a problem taht updateCards is called with the immediate firestore snapshot preliminary version immediately before updateCardsWithOverlay is (since we wait for the batch to commit and by that time the firestore callbacks, wih the preliminary/cached results are already called). It's probably not; it's in the same microtask so none of the expensive pipeline should run... One fix would be to disable persistence/offline which doesn't save any load time anyway (verify that's true in production)
• [x] Think through an edge case of a card has an updateInboundLinks pending, and before that fires from the server, is directly edited and commited. Does that cause a problem now? Did it cause a problem in the past? (As long as the problem is "a bit of thrash in the caching pipeline" then that's fine)
• [ ] Figure out how to make it so the case of "multi-edit a group of cards to all add a reference to ANOTHER_CARD, and then now ANOTHER_CARD slowly incrementally updates one-by-one to get inbound links, in a way that thrashes the commit overlay)
• [x] Consider whether reorderCard should also use the updatedCardsOverlay / ... updateCards workflow (it currently doesn't pass updatedCardsOverlay). Nah, it's fine because the sort_order update wil ahve already been fired by firestore before the overlay is applied, and the later update will just be timestamps clearing and those are already ignored.
• [ ] Test performance with and without the updateCards skipping and verify it's faster
• [ ] Test updateInboundLinks on serverside and verify it works (we refactored the impl on MBP but MBP can't deploy)

[jkomoros]

134 gets much more important now that the finicky inboundLinksUpdates is duplicated on server and client

[jkomoros]

Actually turning on the local inboundLinks updating can't be done yet because of a common problem.

Say you have a collection of 3 cards, and you're adding a see_also reference from each of them in a batch edit to ANOTHER_CARD.

The machinery correctly identifies that each of the three cards will update, as well as the ANOTHER_CARD, which will receive three new inbound updates. The firestore layer immediately tells updateCards that the 3 outward cards will update, and then right after that the local overlay is applied and also says, "and ANOTHER_CARD will also get three new inbound references"

... But then as each of the three outboudn cards has its updateInboundLinks run on the server, updateCards is called on ANOTHER_CARD three separate times as its inbound links incrementally extends. And the first time, the machinery sees that it's not equivalent to the one that was set via overlay, so it swaps out with the new one (which only has one of the inbound liknks set yet). And then the next two stream in, and each doesn't match.

The more common case of "edit one card to add/drop multiple other references in one edit" should work. But this multi-edit case is pretty common too, and ideally we should be able to have it not thrash, too.

This is not an easy one to fix; it's not that we can say "ignore all future updates to this card, this version we're giving you now is the canonical" (because at some point the incremental inboudn links updates will finish and at that point we should listen to them), or "ignore the next 5 updates" (because there could be an interleaved real edit from elsewhere that needs to be incorporated).

I guess there will have to be a solution of "look, I recognize these incremental updates will stream in; if you see any of these references being added, just drop that update", and as each one streams in, remove it from the "expected changes" list.

So maybe we store a diff somewhere in state and chip away at it as the updates stream in until no diff is left? So the logic is "either the existing card already fully incorporates the edit OR the expected diff has parts, matching the diff of the change, that has not been consumed."

[jkomoros]

A generalized system would be something like "here is the diff from previous state to expected prior state" and every incremental change that comes in would either be a no-op (discarded) or a incremental or full part of that diff that eats away at it until it's gone. (Note this wouldn't naively work right now because the initial main update for cards happens BEFORE the overlay is applied, since firestore does the local optimistic updates immediately).

But that's quite a bit of machinery, diffing, etc. Another thing is just to observe that this only ever happens for inbound references, and handle that specially. Basically it treats updates thare are only inbound links to be no-ops unless updateCards is called with a forceUpdateInboundLinks, which is only called from updateCardOverlay. There's still a small problem with this: what if another clients edits lead to inboundLinks updates that were not initiated by this client? That system would erroneously ignore them.

Anything that special-cases inbound links behaviors in the diffing machinery will need to be updated (perhaps in non-obvious ways) if updateInboundLinks ever is updated to do anything else in addition.

[jkomoros]

The remainder of the work on this will be obviated when we fix #585

[jkomoros]

Now that #585 is fixed, this issue can be fully closed