Skip to content

Adding UnknownDocument and hasCommittedMutations flag #29

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
Sep 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,28 @@ public void testCanUpdateAnExistingDocument() {
assertEquals(finalData, doc.getData());
}

@Test
public void testCanUpdateAnUnknownDocument() {
DocumentReference writerRef = testFirestore().collection("collection").document();
DocumentReference readerRef =
testFirestore().collection("collection").document(writerRef.getId());
waitFor(writerRef.set(map("a", "a")));
waitFor(readerRef.update(map("b", "b")));
DocumentSnapshot writerSnap = waitFor(writerRef.get(Source.CACHE));
assertTrue(writerSnap.exists());
try {
waitFor(readerRef.get(Source.CACHE));
fail("Should have thrown exception");
} catch (RuntimeException e) {
assertEquals(
((FirebaseFirestoreException) e.getCause().getCause()).getCode(), Code.UNAVAILABLE);
}
writerSnap = waitFor(writerRef.get());
assertEquals(map("a", "a", "b", "b"), writerSnap.getData());
DocumentSnapshot readerSnap = waitFor(readerRef.get());
assertEquals(map("a", "a", "b", "b"), readerSnap.getData());
}

@Test
public void testCanMergeDataWithAnExistingDocumentUsingSet() {
DocumentReference documentReference = testCollection("rooms").document("eros");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.firebase.firestore;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.firebase.firestore.util.Assert.fail;

import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
Expand All @@ -26,7 +27,6 @@
import com.google.firebase.firestore.model.Document;
import com.google.firebase.firestore.model.MaybeDocument;
import com.google.firebase.firestore.model.NoDocument;
import com.google.firebase.firestore.util.Assert;
import com.google.firebase.firestore.util.Executors;
import com.google.firebase.firestore.util.Util;
import java.util.Collections;
Expand Down Expand Up @@ -237,15 +237,19 @@ private Task<DocumentSnapshot> getAsync(DocumentReference documentRef) {
}
List<MaybeDocument> docs = task.getResult();
if (docs.size() != 1) {
throw Assert.fail("Mismatch in docs returned from document lookup.");
throw fail("Mismatch in docs returned from document lookup.");
}
MaybeDocument doc = docs.get(0);
if (doc instanceof NoDocument) {
if (doc instanceof Document) {
return DocumentSnapshot.fromDocument(
firestore, (Document) doc, /*fromCache=*/ false);
} else if (doc instanceof NoDocument) {
return DocumentSnapshot.fromNoDocument(
firestore, doc.getKey(), /*fromCache=*/ false);
} else {
return DocumentSnapshot.fromDocument(
firestore, (Document) doc, /*fromCache=*/ false);
throw fail(
"BatchGetDocumentsRequest returned unexpected document type: "
+ doc.getClass().getCanonicalName());
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.firebase.firestore.FirebaseFirestoreException.Code;
import com.google.firebase.firestore.core.UserData.ParsedSetData;
import com.google.firebase.firestore.core.UserData.ParsedUpdateData;
import com.google.firebase.firestore.model.Document;
import com.google.firebase.firestore.model.DocumentKey;
import com.google.firebase.firestore.model.MaybeDocument;
import com.google.firebase.firestore.model.NoDocument;
Expand All @@ -40,6 +41,8 @@
import java.util.concurrent.TimeUnit;
import javax.annotation.Nullable;

import static com.google.firebase.firestore.util.Assert.fail;

/**
* Internal transaction object responsible for accumulating the mutations to perform and the base
* versions for any documents read.
Expand All @@ -55,10 +58,14 @@ public Transaction(Datastore d) {
}

private void recordVersion(MaybeDocument doc) throws FirebaseFirestoreException {
SnapshotVersion docVersion = doc.getVersion();
if (doc instanceof NoDocument) {
SnapshotVersion docVersion;
if (doc instanceof Document) {
docVersion = doc.getVersion();
} else if (doc instanceof NoDocument) {
// For nonexistent docs, we must use precondition with version 0 when we overwrite them.
docVersion = SnapshotVersion.NONE;
} else {
throw fail("Unexpected document type in transaction: " + doc.getClass().getCanonicalName());
}

if (readVersions.containsKey(doc.getKey())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ public <D extends MaybeDocument> DocumentChanges computeDocChanges(
while (newDocumentSet.size() > query.getLimit()) {
Document oldDoc = newDocumentSet.getLastDocument();
newDocumentSet = newDocumentSet.remove(oldDoc.getKey());
newMutatedKeys = newMutatedKeys.remove(oldDoc.getKey());
changeSet.addChange(DocumentViewChange.create(Type.REMOVED, oldDoc));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.firebase.firestore.model.MaybeDocument;
import com.google.firebase.firestore.model.NoDocument;
import com.google.firebase.firestore.model.SnapshotVersion;
import com.google.firebase.firestore.model.UnknownDocument;
import com.google.firebase.firestore.model.mutation.Mutation;
import com.google.firebase.firestore.model.mutation.MutationBatch;
import com.google.firebase.firestore.model.value.FieldValue;
Expand All @@ -48,25 +49,35 @@ com.google.firebase.firestore.proto.MaybeDocument encodeMaybeDocument(MaybeDocum
com.google.firebase.firestore.proto.MaybeDocument.Builder builder =
com.google.firebase.firestore.proto.MaybeDocument.newBuilder();
if (document instanceof NoDocument) {
builder.setNoDocument(encodeNoDocument((NoDocument) document));

NoDocument noDocument = (NoDocument) document;
builder.setNoDocument(encodeNoDocument(noDocument));
builder.setHasCommittedMutations(noDocument.hasCommittedMutations());
} else if (document instanceof Document) {
builder.setDocument(encodeDocument((Document) document));
Document existingDocument = (Document) document;
builder.setDocument(encodeDocument(existingDocument));
builder.setHasCommittedMutations(existingDocument.hasCommittedMutations());
} else if (document instanceof UnknownDocument) {
builder.setUnknownDocument(encodeUnknownDocument((UnknownDocument) document));
builder.setHasCommittedMutations(true);
} else {
throw fail("Unknown document type %s", document.getClass().getCanonicalName());
}

return builder.build();
}

/** Decodes a MaybeDocument proto to the equivalent model. */
MaybeDocument decodeMaybeDocument(com.google.firebase.firestore.proto.MaybeDocument proto) {
switch (proto.getDocumentTypeCase()) {
case DOCUMENT:
return decodeDocument(proto.getDocument());
return decodeDocument(proto.getDocument(), proto.getHasCommittedMutations());

case NO_DOCUMENT:
return decodeNoDocument(proto.getNoDocument());

case UNKNOWN_DOCUMENT:
return decodeUnknownDocument(proto.getUnknownDocument());

default:
throw fail("Unknown MaybeDocument %s", proto);
}
Expand All @@ -93,11 +104,18 @@ private com.google.firestore.v1beta1.Document encodeDocument(Document document)
}

/** Decodes a Document proto to the equivalent model. */
private Document decodeDocument(com.google.firestore.v1beta1.Document document) {
private Document decodeDocument(
com.google.firestore.v1beta1.Document document, boolean hasCommittedMutations) {
DocumentKey key = rpcSerializer.decodeKey(document.getName());
ObjectValue value = rpcSerializer.decodeFields(document.getFieldsMap());
SnapshotVersion version = rpcSerializer.decodeVersion(document.getUpdateTime());
return new Document(key, version, value, false);
return new Document(
key,
version,
value,
hasCommittedMutations
? Document.DocumentState.COMMITTED_MUTATIONS
: Document.DocumentState.SYNCED);
}

/** Encodes a NoDocument value to the equivalent proto. */
Expand All @@ -116,6 +134,24 @@ private NoDocument decodeNoDocument(com.google.firebase.firestore.proto.NoDocume
return new NoDocument(key, version);
}

/** Encodes a UnknownDocument value to the equivalent proto. */
private com.google.firebase.firestore.proto.UnknownDocument encodeUnknownDocument(
UnknownDocument document) {
com.google.firebase.firestore.proto.UnknownDocument.Builder builder =
com.google.firebase.firestore.proto.UnknownDocument.newBuilder();
builder.setName(rpcSerializer.encodeKey(document.getKey()));
builder.setVersion(rpcSerializer.encodeTimestamp(document.getVersion().getTimestamp()));
return builder.build();
}

/** Decodes a UnknownDocument proto to the equivalent model. */
private UnknownDocument decodeUnknownDocument(
com.google.firebase.firestore.proto.UnknownDocument proto) {
DocumentKey key = rpcSerializer.decodeKey(proto.getName());
SnapshotVersion version = rpcSerializer.decodeVersion(proto.getVersion());
return new UnknownDocument(key, version);
}

/** Encodes a MutationBatch model for local storage in the mutation queue. */
com.google.firebase.firestore.proto.WriteBatch encodeMutationBatch(MutationBatch batch) {
com.google.firebase.firestore.proto.WriteBatch.Builder result =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ public ImmutableSortedMap<DocumentKey, MaybeDocument> applyRemoteEvent(RemoteEve
// resolution failing).
if (existingDoc == null
|| doc.getVersion().equals(SnapshotVersion.NONE)
|| authoritativeUpdates.contains(doc.getKey())
|| (authoritativeUpdates.contains(doc.getKey()) && !existingDoc.hasPendingWrites())
|| doc.getVersion().compareTo(existingDoc.getVersion()) >= 0) {
remoteDocuments.add(doc);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@
*/
public class Document extends MaybeDocument {

/** Describes the `hasPendingWrites` state of a document. */
public enum DocumentState {
/** Local mutations applied via the mutation queue. Document is potentially inconsistent. */
LOCAL_MUTATIONS,
/** Mutations applied based on a write acknowledgment. Document is potentially inconsistent. */
COMMITTED_MUTATIONS,
/** No mutations applied. Document was sent to us by Watch. */
SYNCED
}

private static final Comparator<Document> KEY_COMPARATOR =
new Comparator<Document>() {
@Override
Expand All @@ -40,13 +50,13 @@ public static Comparator<Document> keyComparator() {

private final ObjectValue data;

private final boolean hasLocalMutations;
private final DocumentState documentState;

public Document(
DocumentKey key, SnapshotVersion version, ObjectValue data, boolean hasLocalMutations) {
DocumentKey key, SnapshotVersion version, ObjectValue data, DocumentState documentState) {
super(key, version);
this.data = data;
this.hasLocalMutations = hasLocalMutations;
this.documentState = documentState;
}

public ObjectValue getData() {
Expand All @@ -63,7 +73,16 @@ public ObjectValue getData() {
}

public boolean hasLocalMutations() {
return hasLocalMutations;
return documentState.equals(DocumentState.LOCAL_MUTATIONS);
}

public boolean hasCommittedMutations() {
return documentState.equals(DocumentState.COMMITTED_MUTATIONS);
}

@Override
public boolean hasPendingWrites() {
return this.hasLocalMutations() || this.hasCommittedMutations();
}

@Override
Expand All @@ -79,7 +98,7 @@ public boolean equals(Object o) {

return getVersion().equals(document.getVersion())
&& getKey().equals(document.getKey())
&& hasLocalMutations == document.hasLocalMutations
&& documentState.equals(document.documentState)
&& data.equals(document.data);
}

Expand All @@ -88,7 +107,7 @@ public int hashCode() {
int result = getKey().hashCode();
result = 31 * result + data.hashCode();
result = 31 * result + getVersion().hashCode();
result = 31 * result + (hasLocalMutations ? 1 : 0);
result = 31 * result + documentState.hashCode();
return result;
}

Expand All @@ -101,8 +120,8 @@ public String toString() {
+ data
+ ", version="
+ getVersion()
+ ", hasLocalMutations="
+ hasLocalMutations
+ ", documentState="
+ documentState.name()
+ '}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,9 @@ public DocumentKey getKey() {
public SnapshotVersion getVersion() {
return version;
}

/**
* Whether this document has a local mutation applied that has not yet been acknowledged by Watch.
*/
public abstract boolean hasPendingWrites();
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,17 @@ public boolean equals(Object o) {
return getVersion().equals(that.getVersion()) && getKey().equals(that.getKey());
}

@Override
public boolean hasPendingWrites() {
return hasCommittedMutations();
}

public boolean hasCommittedMutations() {
// We currently don't raise `hasPendingWrites` for deleted documents, even if Watch hasn't
// caught up to the deleted version yet.
return false;
}

@Override
public int hashCode() {
int result = getKey().hashCode();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright 2018 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package com.google.firebase.firestore.model;

/**
* A class representing an existing document whose data is unknown (e.g. a document that was updated
* without a known base document).
*/
public class UnknownDocument extends MaybeDocument {
public UnknownDocument(DocumentKey key, SnapshotVersion version) {
super(key, version);
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}

UnknownDocument that = (UnknownDocument) o;

return getVersion().equals(that.getVersion()) && getKey().equals(that.getKey());
}

@Override
public boolean hasPendingWrites() {
return true;
}

@Override
public int hashCode() {
int result = getKey().hashCode();
result = 31 * result + getVersion().hashCode();
return result;
}

@Override
public String toString() {
return "UnknownDocument{key=" + getKey() + ", version=" + getVersion() + '}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ public MaybeDocument applyToRemoteDocument(
// Unlike applyToLocalView, if we're applying a mutation to a remote document the server has
// accepted the mutation so the precondition must have held.

return new NoDocument(getKey(), SnapshotVersion.NONE);
// We store the deleted document at the commit version of the delete. Any document version
// that the server sends us before the delete was applied is discarded
return new NoDocument(getKey(), mutationResult.getVersion());
}

@Nullable
Expand Down
Loading