diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteSchema.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteSchema.java index 97c09efc415..838aba94159 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteSchema.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteSchema.java @@ -14,6 +14,7 @@ package com.google.firebase.firestore.local; +import static com.google.firebase.firestore.util.Assert.fail; import static com.google.firebase.firestore.util.Assert.hardAssert; import android.content.ContentValues; @@ -26,7 +27,9 @@ import androidx.annotation.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.firebase.firestore.model.ResourcePath; +import com.google.firebase.firestore.proto.Target; import com.google.firebase.firestore.util.Consumer; +import com.google.protobuf.InvalidProtocolBufferException; import java.util.ArrayList; import java.util.List; @@ -136,7 +139,16 @@ void runMigrations(int fromVersion, int toVersion) { } if (fromVersion < 9 && toVersion >= 9) { - addReadTime(); + if (!hasReadTime()) { + addReadTime(); + } else { + // Index-free queries rely on the fact that documents updated after a query's last limbo + // free snapshot version are persisted with their read-time. If a customer upgrades to + // schema version 9, downgrades and then upgrades again, some queries may have a last limbo + // free snapshot version despite the fact that not all updated document have an associated + // read time. + dropLastLimboFreeSnapshotVersion(); + } } /* @@ -363,14 +375,39 @@ private void addSequenceNumber() { } } + private boolean hasReadTime() { + boolean hasReadTimeSeconds = tableContainsColumn("remote_documents", "read_time_seconds"); + boolean hasReadTimeNanos = tableContainsColumn("remote_documents", "read_time_nanos"); + + hardAssert( + hasReadTimeSeconds == hasReadTimeNanos, + "Table contained just one of read_time_seconds or read_time_nanos"); + + return hasReadTimeSeconds && hasReadTimeNanos; + } + private void addReadTime() { - if (!tableContainsColumn("remote_documents", "read_time_seconds")) { - hardAssert( - !tableContainsColumn("remote_documents", "read_time_nanos"), - "Table contained read_time_nanos, but is missing read_time_seconds"); - db.execSQL("ALTER TABLE remote_documents ADD COLUMN read_time_seconds INTEGER"); - db.execSQL("ALTER TABLE remote_documents ADD COLUMN read_time_nanos INTEGER"); - } + db.execSQL("ALTER TABLE remote_documents ADD COLUMN read_time_seconds INTEGER"); + db.execSQL("ALTER TABLE remote_documents ADD COLUMN read_time_nanos INTEGER"); + } + + private void dropLastLimboFreeSnapshotVersion() { + new SQLitePersistence.Query(db, "SELECT target_id, target_proto FROM targets") + .forEach( + cursor -> { + int targetId = cursor.getInt(0); + byte[] targetProtoBytes = cursor.getBlob(1); + + try { + Target targetProto = Target.parseFrom(targetProtoBytes); + targetProto = targetProto.toBuilder().clearLastLimboFreeSnapshotVersion().build(); + db.execSQL( + "UPDATE targets SET target_proto = ? WHERE target_id = ?", + new Object[] {targetProto.toByteArray(), targetId}); + } catch (InvalidProtocolBufferException e) { + throw fail("Failed to decode Query data for target %s", targetId); + } + }); } /** diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java index 8b16cbe9440..495bb6f71f8 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java @@ -21,6 +21,7 @@ import static com.google.firebase.firestore.testutil.TestUtil.path; import static com.google.firebase.firestore.testutil.TestUtil.query; import static com.google.firebase.firestore.testutil.TestUtil.version; +import static com.google.firebase.firestore.util.Assert.fail; import static java.util.Arrays.asList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -36,10 +37,13 @@ import com.google.firebase.firestore.model.DocumentKey; import com.google.firebase.firestore.model.ResourcePath; import com.google.firebase.firestore.proto.MaybeDocument; +import com.google.firebase.firestore.proto.Target; import com.google.firebase.firestore.proto.WriteBatch; import com.google.firebase.firestore.remote.RemoteSerializer; import com.google.firestore.v1.Document; import com.google.firestore.v1.Write; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.Timestamp; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -436,6 +440,68 @@ public void existingDocumentsRemainReadableAfterIndexFreeMigration() { assertResultsContain(results, "coll/new"); } + @Test + public void dropsLastLimboFreeSnapshotIfPreviouslyDowngraded() { + schema.runMigrations(0, 9); + + db.execSQL( + "INSERT INTO targets (target_id, canonical_id, target_proto) VALUES (?,?, ?)", + new Object[] {1, "foo", createDummyQueryTargetWithLimboFreeVersion(1).toByteArray()}); + db.execSQL( + "INSERT INTO targets (target_id, canonical_id, target_proto) VALUES (?, ?, ?)", + new Object[] {2, "bar", createDummyQueryTargetWithLimboFreeVersion(2).toByteArray()}); + db.execSQL( + "INSERT INTO targets (target_id, canonical_id, target_proto) VALUES (?,?, ?)", + new Object[] {3, "baz", createDummyQueryTargetWithLimboFreeVersion(3).toByteArray()}); + + schema.runMigrations(0, 8); + schema.runMigrations(8, 9); + + int rowCount = + new SQLitePersistence.Query(db, "SELECT target_id, target_proto FROM targets") + .forEach( + cursor -> { + int targetId = cursor.getInt(0); + byte[] targetProtoBytes = cursor.getBlob(1); + + try { + Target targetProto = Target.parseFrom(targetProtoBytes); + assertEquals(targetId, targetProto.getTargetId()); + assertFalse(targetProto.hasLastLimboFreeSnapshotVersion()); + } catch (InvalidProtocolBufferException e) { + fail("Failed to decode Query data"); + } + }); + + assertEquals(3, rowCount); + } + + @Test + public void keepsLastLimboFreeSnapshotIfNotDowngraded() { + schema.runMigrations(0, 9); + + db.execSQL( + "INSERT INTO targets (target_id, canonical_id, target_proto) VALUES (?,?, ?)", + new Object[] {1, "foo", createDummyQueryTargetWithLimboFreeVersion(1).toByteArray()}); + + // Make sure that we don't drop the lastLimboFreeSnapshotVersion if we are already on schema + // version 9. + schema.runMigrations(9, 9); + + new SQLitePersistence.Query(db, "SELECT target_proto FROM targets") + .forEach( + cursor -> { + byte[] targetProtoBytes = cursor.getBlob(0); + + try { + Target targetProto = Target.parseFrom(targetProtoBytes); + assertTrue(targetProto.hasLastLimboFreeSnapshotVersion()); + } catch (InvalidProtocolBufferException e) { + fail("Failed to decode Query data"); + } + }); + } + private SQLiteRemoteDocumentCache createRemoteDocumentCache() { DatabaseId databaseId = DatabaseId.forProject("foo"); LocalSerializer serializer = new LocalSerializer(new RemoteSerializer(databaseId)); @@ -460,6 +526,13 @@ private byte[] createDummyDocument(String name) { .toByteArray(); } + private Target createDummyQueryTargetWithLimboFreeVersion(int targetId) { + return Target.newBuilder() + .setTargetId(targetId) + .setLastLimboFreeSnapshotVersion(Timestamp.newBuilder().setSeconds(42)) + .build(); + } + private void assertResultsContain( ImmutableSortedMap actualResults, String... docs) {