Skip to content

Index-Free (1/6): Adding LocalStoreTestHelper for NO_CHANGE event #614

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.

Sign up for GitHub

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 4 commits into from
Jul 22, 2019
Merged

Index-Free (1/6): Adding LocalStoreTestHelper for NO_CHANGE event #614

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cce8fd0
Adding LocalStoreTestHelper for NO_CHANGE event
schmidt-sebastian Jul 14, 2019
2bf12c2
Update ktx's test-util
schmidt-sebastian Jul 15, 2019
da8f59a
Make TestTargetMetadataProvider top-level class
schmidt-sebastian Jul 18, 2019
9b85728
Java Format
schmidt-sebastian Jul 18, 2019
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
30 changes: 0 additions & 30 deletions firebase-firestore/ktx/src/test/java/com/google/firebase/firestore/TestUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,36 +138,6 @@ public static QuerySnapshot querySnapshot(
return new QuerySnapshot(query(path), viewSnapshot, FIRESTORE);
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for doing this. At some point we should probably cut these down to the minimum, given the assumption there's no reasonable way to share with the main Firestore testutil.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW, only half of the methods are currently used, but it doesn't seem trivial to just put them in the test class (so I refrained from doing this for now).

* An implementation of TargetMetadataProvider that provides controlled access to the
* `TargetMetadataProvider` callbacks. Any target accessed via these callbacks must be registered
* beforehand via `setSyncedKeys()`.
*/
public static class TestTargetMetadataProvider
implements WatchChangeAggregator.TargetMetadataProvider {
final Map<Integer, ImmutableSortedSet<DocumentKey>> syncedKeys = new HashMap<>();
final Map<Integer, QueryData> queryData = new HashMap<>();

@Override
public ImmutableSortedSet<DocumentKey> getRemoteKeysForTarget(int targetId) {
return syncedKeys.get(targetId) != null
? syncedKeys.get(targetId)
: DocumentKey.emptyKeySet();
}

@Nullable
@Override
public QueryData getQueryDataForTarget(int targetId) {
return queryData.get(targetId);
}

/** Sets or replaces the local state for the provided query data. */
public void setSyncedKeys(QueryData queryData, ImmutableSortedSet<DocumentKey> keys) {
this.queryData.put(queryData.getTargetId(), queryData);
this.syncedKeys.put(queryData.getTargetId(), keys);
}
}

public static <T> T waitFor(Task<T> task) {
if (!task.isComplete()) {
Robolectric.flushBackgroundThreadScheduler();
Expand Down
34 changes: 0 additions & 34 deletions firebase-firestore/src/roboUtil/java/com/google/firebase/firestore/TestUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,17 @@
import static com.google.firebase.firestore.testutil.TestUtil.key;
import static org.mockito.Mockito.mock;

import androidx.annotation.Nullable;
import com.google.android.gms.tasks.Task;
import com.google.firebase.database.collection.ImmutableSortedSet;
import com.google.firebase.firestore.core.DocumentViewChange;
import com.google.firebase.firestore.core.DocumentViewChange.Type;
import com.google.firebase.firestore.core.ViewSnapshot;
import com.google.firebase.firestore.local.QueryData;
import com.google.firebase.firestore.model.Document;
import com.google.firebase.firestore.model.DocumentKey;
import com.google.firebase.firestore.model.DocumentSet;
import com.google.firebase.firestore.model.ResourcePath;
import com.google.firebase.firestore.model.value.ObjectValue;
import com.google.firebase.firestore.remote.WatchChangeAggregator;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.junit.Assert;
Expand Down Expand Up @@ -138,36 +134,6 @@ public static QuerySnapshot querySnapshot(
return new QuerySnapshot(query(path), viewSnapshot, FIRESTORE);
}

/**
* An implementation of TargetMetadataProvider that provides controlled access to the
* `TargetMetadataProvider` callbacks. Any target accessed via these callbacks must be registered
* beforehand via `setSyncedKeys()`.
*/
public static class TestTargetMetadataProvider
implements WatchChangeAggregator.TargetMetadataProvider {
final Map<Integer, ImmutableSortedSet<DocumentKey>> syncedKeys = new HashMap<>();
final Map<Integer, QueryData> queryData = new HashMap<>();

@Override
public ImmutableSortedSet<DocumentKey> getRemoteKeysForTarget(int targetId) {
return syncedKeys.get(targetId) != null
? syncedKeys.get(targetId)
: DocumentKey.emptyKeySet();
}

@Nullable
@Override
public QueryData getQueryDataForTarget(int targetId) {
return queryData.get(targetId);
}

/** Sets or replaces the local state for the provided query data. */
public void setSyncedKeys(QueryData queryData, ImmutableSortedSet<DocumentKey> keys) {
this.queryData.put(queryData.getTargetId(), queryData);
this.syncedKeys.put(queryData.getTargetId(), keys);
}
}

public static <T> T waitFor(Task<T> task) {
if (!task.isComplete()) {
Robolectric.flushBackgroundThreadScheduler();
Expand Down
44 changes: 6 additions & 38 deletions firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static com.google.firebase.firestore.testutil.TestUtil.doc;
import static com.google.firebase.firestore.testutil.TestUtil.key;
import static com.google.firebase.firestore.testutil.TestUtil.map;
import static com.google.firebase.firestore.testutil.TestUtil.noChangeEvent;
import static com.google.firebase.firestore.testutil.TestUtil.patchMutation;
import static com.google.firebase.firestore.testutil.TestUtil.query;
import static com.google.firebase.firestore.testutil.TestUtil.resumeToken;
Expand All @@ -46,7 +47,6 @@
import com.google.firebase.database.collection.ImmutableSortedMap;
import com.google.firebase.database.collection.ImmutableSortedSet;
import com.google.firebase.firestore.FieldValue;
import com.google.firebase.firestore.TestUtil.TestTargetMetadataProvider;
import com.google.firebase.firestore.auth.User;
import com.google.firebase.firestore.core.Query;
import com.google.firebase.firestore.model.Document;
Expand All @@ -61,13 +61,9 @@
import com.google.firebase.firestore.model.mutation.MutationResult;
import com.google.firebase.firestore.model.mutation.SetMutation;
import com.google.firebase.firestore.remote.RemoteEvent;
import com.google.firebase.firestore.remote.WatchChange.WatchTargetChange;
import com.google.firebase.firestore.remote.WatchChange.WatchTargetChangeType;
import com.google.firebase.firestore.remote.WatchChangeAggregator;
import com.google.firebase.firestore.remote.WatchStream;
import com.google.firebase.firestore.remote.WriteStream;
import com.google.firebase.firestore.testutil.TestUtil;
import com.google.protobuf.ByteString;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -912,26 +908,15 @@ public void testPersistsResumeTokens() {

Query query = query("foo/bar");
int targetId = allocateQuery(query);
ByteString resumeToken = resumeToken(1000);

QueryData queryData = TestUtil.queryData(targetId, QueryPurpose.LISTEN, "foo/bar");
TestTargetMetadataProvider testTargetMetadataProvider = new TestTargetMetadataProvider();
testTargetMetadataProvider.setSyncedKeys(queryData, DocumentKey.emptyKeySet());

WatchChangeAggregator aggregator = new WatchChangeAggregator(testTargetMetadataProvider);

WatchTargetChange watchChange =
new WatchTargetChange(WatchTargetChangeType.Current, asList(targetId), resumeToken);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a current change, not a no-change. Why are these are substitutable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since works because we persist the resume token for both NO_CHANGE and CURRENT:

firebase-android-sdk/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/WatchChangeAggregator.java

Line 109 in d9ed980

targetState.updateResumeToken(targetChange.getResumeToken());

firebase-android-sdk/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/WatchChangeAggregator.java

Line 138 in d9ed980

targetState.updateResumeToken(targetChange.getResumeToken());

aggregator.handleTargetChange(watchChange);
RemoteEvent remoteEvent = aggregator.createRemoteEvent(version(1000));
applyRemoteEvent(remoteEvent);
applyRemoteEvent(noChangeEvent(targetId, 1000));

// Stop listening so that the query should become inactive (but persistent)
localStore.releaseQuery(query);

// Should come back with the same resume token
QueryData queryData2 = localStore.allocateQuery(query);
assertEquals(resumeToken, queryData2.getResumeToken());
assertEquals(resumeToken(1000), queryData2.getResumeToken());
}

@Test
Expand All @@ -943,35 +928,18 @@ public void testDoesNotReplaceResumeTokenWithEmptyByteString() {

Query query = query("foo/bar");
int targetId = allocateQuery(query);
ByteString resumeToken = resumeToken(1000);

QueryData queryData = TestUtil.queryData(targetId, QueryPurpose.LISTEN, "foo/bar");
TestTargetMetadataProvider testTargetMetadataProvider = new TestTargetMetadataProvider();
testTargetMetadataProvider.setSyncedKeys(queryData, DocumentKey.emptyKeySet());

WatchChangeAggregator aggregator1 = new WatchChangeAggregator(testTargetMetadataProvider);

WatchTargetChange watchChange1 =
new WatchTargetChange(WatchTargetChangeType.Current, asList(targetId), resumeToken);
aggregator1.handleTargetChange(watchChange1);
RemoteEvent remoteEvent1 = aggregator1.createRemoteEvent(version(1000));
applyRemoteEvent(remoteEvent1);
applyRemoteEvent(noChangeEvent(targetId, 1000));

// New message with empty resume token should not replace the old resume token
WatchChangeAggregator aggregator2 = new WatchChangeAggregator(testTargetMetadataProvider);
WatchTargetChange watchChange2 =
new WatchTargetChange(
WatchTargetChangeType.Current, asList(targetId), WatchStream.EMPTY_RESUME_TOKEN);
aggregator2.handleTargetChange(watchChange2);
RemoteEvent remoteEvent2 = aggregator2.createRemoteEvent(version(2000));
applyRemoteEvent(remoteEvent2);
applyRemoteEvent(TestUtil.noChangeEvent(targetId, 2000, WatchStream.EMPTY_RESUME_TOKEN));

// Stop listening so that the query should become inactive (but persistent)
localStore.releaseQuery(query);

// Should come back with the same resume token
QueryData queryData2 = localStore.allocateQuery(query);
assertEquals(resumeToken, queryData2.getResumeToken());
assertEquals(resumeToken(1000), queryData2.getResumeToken());
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion firebase-firestore/src/test/java/com/google/firebase/firestore/remote/RemoteEventTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@
import static org.junit.Assert.fail;

import com.google.firebase.database.collection.ImmutableSortedSet;
import com.google.firebase.firestore.TestUtil.TestTargetMetadataProvider;
import com.google.firebase.firestore.local.QueryData;
import com.google.firebase.firestore.model.Document;
import com.google.firebase.firestore.model.DocumentKey;
import com.google.firebase.firestore.model.NoDocument;
import com.google.firebase.firestore.remote.WatchChange.DocumentChange;
import com.google.firebase.firestore.remote.WatchChange.WatchTargetChange;
import com.google.firebase.firestore.remote.WatchChange.WatchTargetChangeType;
import com.google.firebase.firestore.testutil.TestUtil.TestTargetMetadataProvider;
import com.google.protobuf.ByteString;
import java.util.ArrayList;
import java.util.Collections;
Expand Down
49 changes: 49 additions & 0 deletions firebase-firestore/src/testUtil/java/com/google/firebase/firestore/testutil/TestUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import com.google.firebase.firestore.model.value.ObjectValue;
import com.google.firebase.firestore.remote.RemoteEvent;
import com.google.firebase.firestore.remote.TargetChange;
import com.google.firebase.firestore.remote.WatchChange;
import com.google.firebase.firestore.remote.WatchChange.DocumentChange;
import com.google.firebase.firestore.remote.WatchChangeAggregator;
import com.google.protobuf.ByteString;
Expand Down Expand Up @@ -374,6 +375,24 @@ public static Map<Integer, QueryData> activeLimboQueries(String docKey, Integer.
return activeLimboQueries(docKey, asList(targets));
}

public static RemoteEvent noChangeEvent(int targetId, int version) {
return noChangeEvent(targetId, version, resumeToken(version));
}

public static RemoteEvent noChangeEvent(int targetId, int version, ByteString resumeToken) {
QueryData queryData = TestUtil.queryData(targetId, QueryPurpose.LISTEN, "foo/bar");
TestTargetMetadataProvider testTargetMetadataProvider = new TestTargetMetadataProvider();
testTargetMetadataProvider.setSyncedKeys(queryData, DocumentKey.emptyKeySet());

WatchChangeAggregator aggregator = new WatchChangeAggregator(testTargetMetadataProvider);

WatchChange.WatchTargetChange watchChange =
new WatchChange.WatchTargetChange(
WatchChange.WatchTargetChangeType.NoChange, asList(targetId), resumeToken);
aggregator.handleTargetChange(watchChange);
return aggregator.createRemoteEvent(version(version));
}

public static RemoteEvent addedRemoteEvent(
MaybeDocument doc, List<Integer> updatedInTargets, List<Integer> removedFromTargets) {
DocumentChange change =
Expand Down Expand Up @@ -526,6 +545,36 @@ public static ByteString streamToken(String contents) {
return ByteString.copyFrom(contents, Charsets.UTF_8);
}

/**
* An implementation of TargetMetadataProvider that provides controlled access to the
* `TargetMetadataProvider` callbacks. Any target accessed via these callbacks must be registered
* beforehand via `setSyncedKeys()`.
*/
public static class TestTargetMetadataProvider
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider making this just a top-level class (i.e. in its own source file).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

implements WatchChangeAggregator.TargetMetadataProvider {
final Map<Integer, ImmutableSortedSet<DocumentKey>> syncedKeys = new HashMap<>();
final Map<Integer, QueryData> queryData = new HashMap<>();

@Override
public ImmutableSortedSet<DocumentKey> getRemoteKeysForTarget(int targetId) {
return syncedKeys.get(targetId) != null
? syncedKeys.get(targetId)
: DocumentKey.emptyKeySet();
}

@androidx.annotation.Nullable
@Override
public QueryData getQueryDataForTarget(int targetId) {
return queryData.get(targetId);
}

/** Sets or replaces the local state for the provided query data. */
public void setSyncedKeys(QueryData queryData, ImmutableSortedSet<DocumentKey> keys) {
this.queryData.put(queryData.getTargetId(), queryData);
this.syncedKeys.put(queryData.getTargetId(), keys);
}
}

private static Map<String, Object> fromJsonString(String json) {
try {
ObjectMapper mapper = new ObjectMapper();
Expand Down