From 7b4c3775530438417b3e0efdc128ad88d42835bb Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Sun, 23 Sep 2018 10:35:48 -0700 Subject: [PATCH] Remove no longer needed MutationBatch methods --- .../firebase/firestore/local/LocalStore.java | 3 - .../firestore/local/MemoryMutationQueue.java | 22 ---- .../firestore/local/MutationQueue.java | 17 --- .../firestore/local/SQLiteMutationQueue.java | 16 --- .../local/MutationQueueTestCase.java | 101 +----------------- 5 files changed, 5 insertions(+), 154 deletions(-) diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java index adffa6c0ffa..ffd8166fcfd 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java @@ -229,9 +229,6 @@ public ImmutableSortedMap rejectBatch(int batchId) { MutationBatch toReject = mutationQueue.lookupMutationBatch(batchId); hardAssert(toReject != null, "Attempt to reject nonexistent batch!"); - int lastAcked = mutationQueue.getHighestAcknowledgedBatchId(); - hardAssert(batchId > lastAcked, "Acknowledged batches can't be rejected."); - mutationQueue.removeMutationBatch(toReject); mutationQueue.performConsistencyCheck(); return localDocuments.getDocuments(toReject.getKeys()); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryMutationQueue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryMutationQueue.java index 9dd697f1d82..5ff240de16e 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryMutationQueue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryMutationQueue.java @@ -111,11 +111,6 @@ public int getNextBatchId() { return nextBatchId; } - @Override - public int getHighestAcknowledgedBatchId() { - return highestAcknowledgedBatchId; - } - @Override public void acknowledgeBatch(MutationBatch batch, ByteString streamToken) { int batchId = batch.getBatchId(); @@ -216,23 +211,6 @@ public List getAllMutationBatches() { return getAllLiveMutationBatchesBeforeIndex(queue.size()); } - @Override - public List getAllMutationBatchesThroughBatchId(int batchId) { - int count = queue.size(); - - int endIndex = indexOfBatchId(batchId); - if (endIndex < 0) { - endIndex = 0; - } else if (endIndex >= count) { - endIndex = count; - } else { - // The endIndex is in the queue so increment to pull everything in the queue including it. - endIndex += 1; - } - - return getAllLiveMutationBatchesBeforeIndex(endIndex); - } - @Override public List getAllMutationBatchesAffectingDocumentKey(DocumentKey documentKey) { DocumentReference start = new DocumentReference(documentKey, 0); diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MutationQueue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MutationQueue.java index a47fab64576..bb8061e1d02 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MutationQueue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/MutationQueue.java @@ -47,12 +47,6 @@ interface MutationQueue { */ int getNextBatchId(); - /** - * Returns the highest batchId that has been acknowledged. If no batches have been acknowledged or - * if there are no batches in the queue this can return {@link MutationBatch#UNKNOWN}. - */ - int getHighestAcknowledgedBatchId(); - /** Acknowledges the given batch. */ void acknowledgeBatch(MutationBatch batch, ByteString streamToken); @@ -85,17 +79,6 @@ interface MutationQueue { // cheaply, we should replace this. List getAllMutationBatches(); - /** - * Finds all mutations with a batchId less than or equal to the given batchId. - * - *

Generally the caller should be asking for the next unacknowledged batchId and the number of - * acknowledged batches should be very small when things are functioning well. - * - * @param batchId The batch to search through. - * @return an List containing all batches with matching batchIds. - */ - List getAllMutationBatchesThroughBatchId(int batchId); - /** * Finds all mutation batches that could @em possibly affect the given document key. Not all * mutations in a batch will necessarily affect the document key, so when looping through the diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java index 75f3994fc64..c3cfa7dfd42 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteMutationQueue.java @@ -164,11 +164,6 @@ public int getNextBatchId() { return nextBatchId; } - @Override - public int getHighestAcknowledgedBatchId() { - return lastAcknowledgedBatchId; - } - @Override public void acknowledgeBatch(MutationBatch batch, ByteString streamToken) { int batchId = batch.getBatchId(); @@ -267,17 +262,6 @@ public List getAllMutationBatches() { return result; } - @Override - public List getAllMutationBatchesThroughBatchId(int batchId) { - List result = new ArrayList<>(); - db.query( - "SELECT mutations FROM mutations " - + "WHERE uid = ? AND batch_id <= ? ORDER BY batch_id ASC") - .binding(uid, batchId) - .forEach(row -> result.add(decodeMutationBatch(row.getBlob(0)))); - return result; - } - @Override public List getAllMutationBatchesAffectingDocumentKey(DocumentKey documentKey) { String path = EncodedPath.encode(documentKey.getPath()); diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MutationQueueTestCase.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MutationQueueTestCase.java index ed837b072d4..aecda8b8c29 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MutationQueueTestCase.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/MutationQueueTestCase.java @@ -23,8 +23,6 @@ import static com.google.firebase.firestore.testutil.TestUtil.streamToken; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; -import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.lessThan; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -103,38 +101,6 @@ public void testCountBatches() { assertTrue(mutationQueue.isEmpty()); } - @Test - public void testAcknowledgeBatchId() { - // Initial state of an empty queue - assertEquals(MutationBatch.UNKNOWN, mutationQueue.getHighestAcknowledgedBatchId()); - - // Adding mutation batches should not change the highest acked batchId. - MutationBatch batch1 = addMutationBatch(); - MutationBatch batch2 = addMutationBatch(); - MutationBatch batch3 = addMutationBatch(); - assertThat(batch1.getBatchId(), greaterThan(MutationBatch.UNKNOWN)); - assertThat(batch2.getBatchId(), greaterThan(batch1.getBatchId())); - assertThat(batch3.getBatchId(), greaterThan(batch2.getBatchId())); - - assertEquals(MutationBatch.UNKNOWN, mutationQueue.getHighestAcknowledgedBatchId()); - - acknowledgeBatch(batch1); - assertEquals(batch1.getBatchId(), mutationQueue.getHighestAcknowledgedBatchId()); - - acknowledgeBatch(batch2); - assertEquals(batch2.getBatchId(), mutationQueue.getHighestAcknowledgedBatchId()); - - removeMutationBatches(batch1); - assertEquals(batch2.getBatchId(), mutationQueue.getHighestAcknowledgedBatchId()); - - removeMutationBatches(batch2); - assertEquals(batch2.getBatchId(), mutationQueue.getHighestAcknowledgedBatchId()); - - // Batch 3 never acknowledged. - removeMutationBatches(batch3); - assertEquals(batch2.getBatchId(), mutationQueue.getHighestAcknowledgedBatchId()); - } - @Test public void testAcknowledgeThenRemove() { MutationBatch batch1 = addMutationBatch(); @@ -147,44 +113,6 @@ public void testAcknowledgeThenRemove() { }); assertEquals(0, batchCount()); - assertEquals(batch1.getBatchId(), mutationQueue.getHighestAcknowledgedBatchId()); - } - - @Test - public void testHighestAcknowledgedBatchIdNeverExceedsNextBatchId() { - MutationBatch batch1 = addMutationBatch(); - MutationBatch batch2 = addMutationBatch(); - acknowledgeBatch(batch1); - acknowledgeBatch(batch2); - assertEquals(batch2.getBatchId(), mutationQueue.getHighestAcknowledgedBatchId()); - - removeMutationBatches(batch1, batch2); - assertEquals(batch2.getBatchId(), mutationQueue.getHighestAcknowledgedBatchId()); - - // Restart the queue so that nextBatchId will be reset. - mutationQueue = persistence.getMutationQueue(User.UNAUTHENTICATED); - mutationQueue.start(); - - persistence.runTransaction("Start mutationQueue", () -> mutationQueue.start()); - - // Verify that on restart with an empty queue, nextBatchId falls to a lower value. - assertThat(mutationQueue.getNextBatchId(), lessThan(batch2.getBatchId())); - - // As a result highestAcknowledgedBatchId must also reset lower. - assertEquals(MutationBatch.UNKNOWN, mutationQueue.getHighestAcknowledgedBatchId()); - - // The mutation queue will reset the next batchId after all mutations are removed so adding - // another mutation will cause a collision. - MutationBatch newBatch = addMutationBatch(); - assertEquals(batch1.getBatchId(), newBatch.getBatchId()); - - // Restart the queue with one unacknowledged batch in it. - persistence.runTransaction("Start mutationQueue", () -> mutationQueue.start()); - - assertEquals(newBatch.getBatchId() + 1, mutationQueue.getNextBatchId()); - - // highestAcknowledgedBatchId must still be MutationBatch.UNKNOWN. - assertEquals(MutationBatch.UNKNOWN, mutationQueue.getHighestAcknowledgedBatchId()); } @Test @@ -265,24 +193,6 @@ public void testNextMutationBatchAfterBatchIdSkipsAcknowledgedBatches() { mutationQueue.getNextMutationBatchAfterBatchId(batches.get(1).getBatchId())); } - @Test - public void testAllMutationBatchesThroughBatchID() { - List batches = createBatches(10); - makeHoles(asList(2, 6, 7), batches); - - List found; - List expected; - - found = mutationQueue.getAllMutationBatchesThroughBatchId(batches.get(0).getBatchId() - 1); - assertEquals(emptyList(), found); - - for (int i = 0; i < batches.size(); i++) { - found = mutationQueue.getAllMutationBatchesThroughBatchId(batches.get(i).getBatchId()); - expected = batches.subList(0, i + 1); - assertEquals(expected, found); - } - } - @Test public void testAllMutationBatchesAffectingDocumentKey() { List mutations = @@ -442,7 +352,7 @@ public void testRemoveMutationBatches() { List found; - found = mutationQueue.getAllMutationBatchesThroughBatchId(last.getBatchId()); + found = mutationQueue.getAllMutationBatches(); assertEquals(batches, found); assertEquals(9, found.size()); @@ -452,14 +362,14 @@ public void testRemoveMutationBatches() { batches.remove(batches.get(0)); assertEquals(6, batchCount()); - found = mutationQueue.getAllMutationBatchesThroughBatchId(last.getBatchId()); + found = mutationQueue.getAllMutationBatches(); assertEquals(batches, found); assertEquals(6, found.size()); removeMutationBatches(batches.remove(batches.size() - 1)); assertEquals(5, batchCount()); - found = mutationQueue.getAllMutationBatchesThroughBatchId(last.getBatchId()); + found = mutationQueue.getAllMutationBatches(); assertEquals(batches, found); assertEquals(5, found.size()); @@ -469,12 +379,12 @@ public void testRemoveMutationBatches() { removeMutationBatches(batches.remove(1)); assertEquals(3, batchCount()); - found = mutationQueue.getAllMutationBatchesThroughBatchId(last.getBatchId()); + found = mutationQueue.getAllMutationBatches(); assertEquals(batches, found); assertEquals(3, found.size()); removeMutationBatches(batches.toArray(new MutationBatch[0])); - found = mutationQueue.getAllMutationBatchesThroughBatchId(last.getBatchId()); + found = mutationQueue.getAllMutationBatches(); assertEquals(emptyList(), found); assertEquals(0, found.size()); assertTrue(mutationQueue.isEmpty()); @@ -496,7 +406,6 @@ public void testStreamToken() { persistence.runTransaction( "acknowledgeBatchId", () -> mutationQueue.acknowledgeBatch(batch1, streamToken2)); - assertEquals(batch1.getBatchId(), mutationQueue.getHighestAcknowledgedBatchId()); assertEquals(streamToken2, mutationQueue.getLastStreamToken()); }