Skip to content
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

get() reimplementation #3887

Merged
merged 32 commits into from
Jul 29, 2022
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
3e085d4
Updated getValue so that it doesn't update the SyncTree
maneesht Jul 8, 2022
34e251d
Fixed formatting issues
maneesht Jul 12, 2022
dcdff66
WIP
maneesht Jul 14, 2022
7b6f9f4
Merge remote-tracking branch 'origin/master' into mtewani/fix-get-cache
maneesht Jul 14, 2022
bec3e87
Merge remote-tracking branch 'origin/master' into mtewani/fix-get-cache
maneesht Jul 15, 2022
0276e1c
Merge remote-tracking branch 'origin/master' into mtewani/fix-get-cache
maneesht Jul 18, 2022
606b9c1
Used addEventRegistration and removeEventRegistration to add to the t…
maneesht Jul 19, 2022
299c2df
Fixed test and ran formatter
maneesht Jul 19, 2022
02af45d
Added comments
maneesht Jul 19, 2022
1babdc2
Fixed formatting
maneesht Jul 19, 2022
989f124
Added test annotation
maneesht Jul 19, 2022
8002947
WIP
maneesht Jul 25, 2022
b95be2c
get test suggest
jmwski Jul 20, 2022
6f5861a
WIP
maneesht Jul 26, 2022
3f3813a
Removed extra event propagation
maneesht Jul 27, 2022
13e2dc5
Included more tests
maneesht Jul 27, 2022
481acd7
Added missing link
maneesht Jul 27, 2022
69b4bef
Fixed formatting
maneesht Jul 27, 2022
cc6e3a1
Fixed formatting
maneesht Jul 27, 2022
cc70e55
Reverted retryrule to 3
maneesht Jul 27, 2022
1d58f9c
Resolved all TODOs
maneesht Jul 27, 2022
5cee9c7
Removed unnecessary nesting
maneesht Jul 27, 2022
d9a97f2
Removed event listener. WIP
maneesht Jul 27, 2022
23bde0b
Fixed tests
maneesht Jul 28, 2022
e60273c
Removed comment
maneesht Jul 28, 2022
94c91fc
Removed comment
maneesht Jul 28, 2022
844c8cc
Addressed comments
maneesht Jul 29, 2022
3bdf63a
Removed usage of 34L
maneesht Jul 29, 2022
a590590
Removed question
maneesht Jul 29, 2022
67bf0ce
Addressed comments
maneesht Jul 29, 2022
15bbeda
Fixed formatting
maneesht Jul 29, 2022
2d7b15f
Addressed comments
maneesht Jul 29, 2022
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 @@ -15,6 +15,7 @@
package com.google.firebase.database;

import static android.os.AsyncTask.THREAD_POOL_EXECUTOR;
import static com.google.android.gms.tasks.Tasks.await;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
Expand All @@ -28,7 +29,6 @@
import androidx.test.platform.app.InstrumentationRegistry;
import com.google.android.gms.tasks.OnCompleteListener;
import com.google.android.gms.tasks.Task;
import com.google.android.gms.tasks.Tasks;
import com.google.firebase.FirebaseApp;
import com.google.firebase.FirebaseOptions;
import com.google.firebase.database.core.DatabaseConfig;
Expand Down Expand Up @@ -592,10 +592,10 @@ public void testStartAfterWithOrderByKey()
DatabaseReference ref = IntegrationTestHelpers.getRandomNode();
DatabaseReference childOne = ref.push();
DatabaseReference childTwo = ref.push();
Tasks.await(childOne.setValue(1L));
Tasks.await(childTwo.setValue(2L));
await(childOne.setValue(1L));
await(childTwo.setValue(2L));

DataSnapshot snapshot = Tasks.await(ref.orderByKey().startAfter(childOne.getKey()).get());
DataSnapshot snapshot = await(ref.orderByKey().startAfter(childOne.getKey()).get());

Map<String, Long> values = (Map<String, Long>) snapshot.getValue();

Expand All @@ -610,10 +610,10 @@ public void testEndBeforeWithOrderByKey()
DatabaseReference ref = IntegrationTestHelpers.getRandomNode();
DatabaseReference childOne = ref.push();
DatabaseReference childTwo = ref.push();
Tasks.await(childOne.setValue(1L));
Tasks.await(childTwo.setValue(2L));
await(childOne.setValue(1L));
await(childTwo.setValue(2L));

DataSnapshot snapshot = Tasks.await(ref.orderByKey().endBefore(childTwo.getKey()).get());
DataSnapshot snapshot = await(ref.orderByKey().endBefore(childTwo.getKey()).get());
Map<String, Long> values = (Map<String, Long>) snapshot.getValue();

assertNotNull(values);
Expand All @@ -629,8 +629,8 @@ public void testEndBeforeWithOrderByKeyOverlappingListener()
DatabaseReference ref = IntegrationTestHelpers.getRandomNode();
DatabaseReference childOne = ref.push();
DatabaseReference childTwo = ref.push();
Tasks.await(childOne.setValue(1L));
Tasks.await(childTwo.setValue(2L));
await(childOne.setValue(1L));
await(childTwo.setValue(2L));

Semaphore semaphore = new Semaphore(0);
ValueEventListener listener =
Expand All @@ -648,7 +648,7 @@ public void onCancelled(@NonNull DatabaseError error) {}

IntegrationTestHelpers.waitFor(semaphore);

DataSnapshot snapshot = Tasks.await(ref.orderByKey().endBefore(childTwo.getKey()).get());
DataSnapshot snapshot = await(ref.orderByKey().endBefore(childTwo.getKey()).get());
Map<String, Long> values = (Map<String, Long>) snapshot.getValue();

assertNotNull(values);
Expand All @@ -663,8 +663,8 @@ public void testStartAfterWithOrderByKeyOverlappingListener()
DatabaseReference ref = IntegrationTestHelpers.getRandomNode();
DatabaseReference childOne = ref.push();
DatabaseReference childTwo = ref.push();
Tasks.await(childOne.setValue(1L));
Tasks.await(childTwo.setValue(2L));
await(childOne.setValue(1L));
await(childTwo.setValue(2L));

Semaphore semaphore = new Semaphore(0);
ValueEventListener listener =
Expand All @@ -681,7 +681,7 @@ public void onCancelled(@NonNull DatabaseError error) {}

IntegrationTestHelpers.waitFor(semaphore);

DataSnapshot snapshot = Tasks.await(ref.orderByKey().startAfter(childOne.getKey()).get());
DataSnapshot snapshot = await(ref.orderByKey().startAfter(childOne.getKey()).get());
Map<String, Long> values = (Map<String, Long>) snapshot.getValue();

assertNotNull(values);
Expand Down Expand Up @@ -4554,7 +4554,7 @@ public void testGetReturnsNullForEmptyNodeWhenOnline()
FirebaseApp app =
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
FirebaseDatabase db = FirebaseDatabase.getInstance(app);
assertNull(Tasks.await(db.getReference(UUID.randomUUID().toString()).get()).getValue());
assertNull(await(db.getReference(UUID.randomUUID().toString()).get()).getValue());
}

@Test
Expand All @@ -4578,22 +4578,188 @@ public void testGetWaitsForConnection() throws DatabaseException, InterruptedExc
}
});
try {
Tasks.await(node.get());
await(node.get());
receivedValue.set(true);
} catch (ExecutionException e) {
fail("get threw an exception: " + e);
}
}

@Test
public void testGetResolvesToCacheWhenOfflineAndNoListeners()
throws DatabaseException, InterruptedException {
FirebaseApp app =
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
FirebaseDatabase db = FirebaseDatabase.getInstance(app);
DatabaseReference node =
db.getReference().child(Objects.requireNonNull(db.getReference().push().getKey()));
jmwski marked this conversation as resolved.
Show resolved Hide resolved
long val = 34;
node.setValue(val);
db.goOffline();
try {
DataSnapshot snapshot = await(node.get());
assertEquals(snapshot.getValue(), val); // TODO(mtewani): Check why this is always long?
jmwski marked this conversation as resolved.
Show resolved Hide resolved
} catch (ExecutionException e) {
fail("get threw an exception: " + e);
}
}

@Test
public void testGetResolvesToCacheWhenOnlineAndNoListeners()
throws DatabaseException, InterruptedException {
FirebaseApp app =
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
FirebaseDatabase db = FirebaseDatabase.getInstance(app);
DatabaseReference node =
db.getReference().child(Objects.requireNonNull(db.getReference().push().getKey()));
jmwski marked this conversation as resolved.
Show resolved Hide resolved
long val = 34;
try {
node.setValue(val);
DataSnapshot snapshot = await(node.get());
assertEquals(snapshot.getValue(), val); // TODO(mtewani): Check why this is always long?
} catch (ExecutionException e) {
fail("get threw an exception: " + e);
}
}

@Test
public void testGetResolvesToCacheWhenOnlineAndParentListener()
throws DatabaseException, InterruptedException {
FirebaseApp app =
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
FirebaseDatabase db = FirebaseDatabase.getInstance(app);
DatabaseReference topLevelNode = db.getReference();
Semaphore semaphore = new Semaphore(0);
long val = 34;
DatabaseReference node =
db.getReference().child(Objects.requireNonNull(db.getReference().push().getKey()));
ValueEventListener listener =
new ValueEventListener() {
@Override
public void onDataChange(@NonNull DataSnapshot snapshot) {
assertTrue(snapshot.hasChildren());
DataSnapshot childNode = null;
for (DataSnapshot child : snapshot.getChildren()) {
if (child.getKey().equals(node.getKey())) {
childNode = child;
}
}
jmwski marked this conversation as resolved.
Show resolved Hide resolved
assertEquals(childNode.getValue(), 34L);
semaphore.release();
}

@Override
public void onCancelled(@NonNull DatabaseError error) {
// no-op
}
};
node.setValue(val);
topLevelNode.addValueEventListener(listener);

try {
IntegrationTestHelpers.waitFor(semaphore);
jmwski marked this conversation as resolved.
Show resolved Hide resolved
DataSnapshot snapshot = await(node.get());
assertEquals(
snapshot.getValue(),
val); // TODO(mtewani): We might be getting this data straight from serverCache, so we may
jmwski marked this conversation as resolved.
Show resolved Hide resolved
// need to rewrite it
} catch (ExecutionException e) {
fail("get threw an exception: " + e);
}
}

public void testGetResolvesToCacheWhenOnlineAndSameLevelListener()
throws DatabaseException, InterruptedException {
FirebaseApp app =
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
FirebaseDatabase db = FirebaseDatabase.getInstance(app);
Semaphore semaphore = new Semaphore(0);
long val = 34;
DatabaseReference node =
db.getReference().child(Objects.requireNonNull(db.getReference().push().getKey()));
node.setValue(val);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this setValue be moved into the try {} catch {} and await'd, as in testGetResolvesToPersistentCacheWhenOfflineAndNoListeners?

ValueEventListener listener =
new ValueEventListener() {
@Override
public void onDataChange(@NonNull DataSnapshot snapshot) {
assertEquals(snapshot.getValue(), 34L);
semaphore.release();
}

@Override
public void onCancelled(@NonNull DatabaseError error) {
// no-op
}
};
node.addValueEventListener(listener);

try {
IntegrationTestHelpers.waitFor(semaphore);
DataSnapshot snapshot = await(node.get());
assertEquals(
snapshot.getValue(),
val); // TODO(mtewani): We might be getting this data straight from serverCache, so we may
// need to rewrite it
} catch (ExecutionException e) {
fail("get threw an exception: " + e);
}
}

public void testGetResolvesToCacheWhenOnlineAndChildLevelListener()
throws DatabaseException, InterruptedException {
FirebaseApp app =
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
FirebaseDatabase db = FirebaseDatabase.getInstance(app);
Semaphore semaphore = new Semaphore(0);
long val = 34;
DatabaseReference node =
db.getReference().child(Objects.requireNonNull(db.getReference().push().getKey()));
DatabaseReference childNode =
db.getReference().child(Objects.requireNonNull(node.push().getKey()));
jmwski marked this conversation as resolved.
Show resolved Hide resolved
node.setValue(val);
ValueEventListener listener = new ValueEventListener() { // TODO: move to child reference
@Override
public void onDataChange(@NonNull DataSnapshot snapshot) {
assertEquals(snapshot.getValue(), 34L);
semaphore.release();
}

@Override
public void onCancelled(@NonNull DatabaseError error) {
// no-op
}
};
childNode.addValueEventListener(listener);

try {
IntegrationTestHelpers.waitFor(semaphore);
DataSnapshot snapshot = await(node.get());
jmwski marked this conversation as resolved.
Show resolved Hide resolved
assertEquals(
snapshot.getValue(),
val); // TODO(mtewani): We might be getting this data straight from serverCache, so we may
// need to rewrite it
} catch (ExecutionException e) {
fail("get threw an exception: " + e);
}
}
/*
* Test:
* 1. Resolve to cache when offline
* 2. Resolve to non-cache when online
* 3. What happens when you have a listener on the same path as the get
* 4. What happens when you have a listener on the child path as the get
* 5. What happens when you have a listener on the parent path as the get
jmwski marked this conversation as resolved.
Show resolved Hide resolved
*/

@Test
public void testGetSendsServerGetForNodeWithNoListenerWhenOnline()
throws DatabaseException, InterruptedException, ExecutionException {
FirebaseApp app =
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
FirebaseDatabase db = FirebaseDatabase.getInstance(app);
DatabaseReference node = db.getReference();
Tasks.await(node.setValue(42));
assertEquals(42L, Tasks.await(node.get()).getValue());
await(node.setValue(42));
assertEquals(42L, await(node.get()).getValue());
}

@Test
Expand All @@ -4618,7 +4784,7 @@ public void testGetProbesInMemoryCacheForActiveListenerWhenOffline()
try {
new WriteFuture(writer, 42L).timedGet();

assertEquals(Tasks.await(reader.get()).getValue(), 42L);
assertEquals(await(reader.get()).getValue(), 42L);

Semaphore semaphore = new Semaphore(0);
listener =
Expand All @@ -4635,7 +4801,7 @@ public void onCancelled(@NonNull DatabaseError error) {}
reader.addValueEventListener(listener);
IntegrationTestHelpers.waitFor(semaphore);
readerDb.goOffline();
assertEquals(Tasks.await(reader.get()).getValue(), 42L);
assertEquals(await(reader.get()).getValue(), 42L);
} finally {
if (listener != null) {
reader.removeEventListener(listener);
Expand All @@ -4651,7 +4817,7 @@ public void testGetWithPendingWrites() throws ExecutionException, InterruptedExc
try {
Map<String, Object> expected = new MapBuilder().put("foo", "bar").build();
node.setValue(expected);
DataSnapshot snapshot = Tasks.await(node.get());
DataSnapshot snapshot = await(node.get());
assertEquals(snapshot.getValue(), expected);
} finally {
node.getDatabase().goOnline();
Expand All @@ -4664,7 +4830,7 @@ public void testGetChildOfPendingWrites() throws ExecutionException, Interrupted
node.getDatabase().goOffline();
try {
node.setValue(new MapBuilder().put("foo", "bar").build());
DataSnapshot snapshot = Tasks.await(node.child("foo").get());
DataSnapshot snapshot = await(node.child("foo").get());
assertEquals(snapshot.getValue(), "bar");
} finally {
node.getDatabase().goOnline();
Expand Down Expand Up @@ -4710,7 +4876,7 @@ public void onCancelled(@NonNull DatabaseError error) {}
readerDb.goOffline();
try {
reader.removeEventListener(listener);
assertEquals(Tasks.await(reader.get()).getValue(), 42L);
assertEquals(await(reader.get()).getValue(), 42L);
} finally {
readerDb.goOnline();
}
Expand All @@ -4735,7 +4901,7 @@ public void testGetUpdatesPersistenceCacheWhenEnabled()
writer = writer.child(key);

assertNull(new WriteFuture(writer, 42L).timedGet());
assertEquals(42L, Tasks.await(reader.get()).getValue());
assertEquals(42L, await(reader.get()).getValue());

readerDb.goOffline();
try {
Expand Down Expand Up @@ -4783,7 +4949,7 @@ public void onCancelled(@NonNull DatabaseError error) {}
reader.removeEventListener(listener);

assertNull(new WriteFuture(writer, 43L).timedGet());
assertEquals(43L, Tasks.await(reader.get()).getValue());
assertEquals(43L, await(reader.get()).getValue());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
/**
* This class is an Android/SQL-backed implementation of PersistenceStorageEngine.
*
* <p>The implementation uses 3 tables for persistence: - writes: A list of all currently
* <p>The implementation uses 4 tables for persistence: - writes: A list of all currently
* outstanding (visible) writes. A row represents a single write containing a unique-across-restarts
* write id, the path string, the type which can be 'o' for an overwrite or 'm' for a merge, and the
* serialized node wrapped. Part number is NULL for normal writes. Large writes (>256K) are split
Expand Down Expand Up @@ -1058,6 +1058,7 @@ private Cursor loadNestedQuery(Path path, String[] columns) {
arguments[path.size() + 1] = pathPrefixStart;
arguments[path.size() + 2] = pathPrefixEnd;
String orderBy = PATH_COLUMN_NAME;
// TODO: We need to modify this to make sure that we limit based on trackedquerykeys

return database.query(SERVER_CACHE_TABLE, columns, whereClause, arguments, null, null, orderBy);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ public Path(String pathString) {
count++;
}
}
// We are essentially doing the same work twice; once here, and once again in the constructor on
// line 60
pieces = new ChildKey[count];
int j = 0;
for (String segment : segments) {
Expand Down
Loading