Skip to content

Fixes decoding WriteResults for deletes (with no updateTime). #1622

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 2, 2018

Conversation

mikelehen
Copy link
Contributor

This should resolve #1591. We were inadvertantly decoding the WriteResult as
having an updateTime of 0 which caused it to be older than the version of the
document in the RemoteDocumentsCache. Therefore we kept the old version in
the cache instead of persisting the deleted document. This caused the document
to reappear in the query results until we resolved it via a limbo resolution.

FYI- I tried to write a spec test but:

  1. A writeAck step has only a single version number that's used for both the WriteResponse commitTime as well as the WriteResult updateTime (
    [self.driver receiveWriteAckWithVersion:version mutationResults:@[ mutationResult ]];
    ).
  2. The iOS spec tests test at the model layer instead of the proto layer (they construct a FSTMutationResult rather than GCFSWriteResult) and so I can't exercise the code with the bug anyway.

But I added serializer tests (which were completely missing for decodedMutationResult).

This should resolve #1591.  We were inadvertantly decoding the WriteResult as
having an updateTime of 0 which caused it to be older than the version of the
document in the RemoteDocumentsCache. Therefore we kept the old version in
the cache instead of persisting the deleted document. This caused the document
to reappear in the query results until we resolved it via a limbo resolution.
Copy link
Contributor

@wilhuff wilhuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mikelehen mikelehen merged commit d9f1162 into master Aug 2, 2018
@mikelehen mikelehen deleted the mikelehen/fix-delete-ackVersion branch August 2, 2018 02:16
mikelehen added a commit that referenced this pull request Aug 2, 2018
This should resolve #1591.  We were inadvertantly decoding the WriteResult as
having an updateTime of 0 which caused it to be older than the version of the
document in the RemoteDocumentsCache. Therefore we kept the old version in
the cache instead of persisting the deleted document. This caused the document
to reappear in the query results until we resolved it via a limbo resolution.
mikelehen added a commit that referenced this pull request Aug 2, 2018
This should resolve #1591.  We were inadvertantly decoding the WriteResult as
having an updateTime of 0 which caused it to be older than the version of the
document in the RemoteDocumentsCache. Therefore we kept the old version in
the cache instead of persisting the deleted document. This caused the document
to reappear in the query results until we resolved it via a limbo resolution.
wilhuff pushed a commit that referenced this pull request Aug 2, 2018
This should resolve #1591.  We were inadvertantly decoding the WriteResult as
having an updateTime of 0 which caused it to be older than the version of the
document in the RemoteDocumentsCache. Therefore we kept the old version in
the cache instead of persisting the deleted document. This caused the document
to reappear in the query results until we resolved it via a limbo resolution.
ryanwilson pushed a commit that referenced this pull request Aug 2, 2018
* Fixes decoding WriteResults for deletes (with no updateTime). (#1622)

This should resolve #1591.  We were inadvertantly decoding the WriteResult as
having an updateTime of 0 which caused it to be older than the version of the
document in the RemoteDocumentsCache. Therefore we kept the old version in
the cache instead of persisting the deleted document. This caused the document
to reappear in the query results until we resolved it via a limbo resolution.

* Add changelog entry for delete WriteResult fix (d9f116). (#1623)

* Actually this fix is going out with 0.13.0
@firebase firebase locked and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Firestore: deleted documents briefly reappear in cached snapshots
3 participants