Skip to content

Commit 844c8cc

Browse files
maneeshtmaneesht
committed
Addressed comments
1 parent 94c91fc commit 844c8cc

File tree

3 files changed

+112
-109
lines changed

3 files changed

+112
-109
lines changed

‎firebase-database/src/androidTest/java/com/google/firebase/database/QueryTest.java

Lines changed: 103 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -4594,7 +4594,6 @@ public void testGetResolvesToPersistentCacheWhenOfflineAndNoListeners()
45944594
db.setPersistenceEnabled(true);
45954595
DatabaseReference node = db.getReference().push();
45964596
long val = 34;
4597-
Thread.currentThread().getId();
45984597

45994598
try {
46004599
await(node.setValue(val));
@@ -4617,7 +4616,6 @@ public void testGetResolvesToCacheWhenOnlineAndParentListener()
46174616
// To ensure that we don't read the cached results, we need a separate app.
46184617
FirebaseApp readApp =
46194618
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
4620-
// String topLevelKey = UUID.randomUUID().toString();
46214619
FirebaseDatabase writeDb = FirebaseDatabase.getInstance(writeApp);
46224620
FirebaseDatabase readDb = FirebaseDatabase.getInstance(readApp);
46234621
long val = 34;
@@ -4635,7 +4633,6 @@ public void testGetResolvesToCacheWhenOnlineAndParentListener()
46354633
assertEquals(1, events.size());
46364634
DataSnapshot childNode = events.get(0).getSnapshot().child(writeKey);
46374635
assertEquals(34L, childNode.getValue());
4638-
46394636
return true;
46404637
});
46414638
readFuture.timedGet();
@@ -4649,42 +4646,32 @@ public void testGetResolvesToCacheWhenOnlineAndParentListener()
46494646

46504647
@Test
46514648
public void testGetResolvesToCacheWhenOnlineAndSameLevelListener()
4652-
throws DatabaseException, InterruptedException, ExecutionException {
4649+
throws DatabaseException, InterruptedException, ExecutionException, TestFailure,
4650+
TimeoutException {
46534651
FirebaseApp writeApp =
46544652
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
46554653
// To ensure that we don't read the cached results, we need a separate app.
46564654
FirebaseApp readApp =
46574655
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
46584656
FirebaseDatabase writeDb = FirebaseDatabase.getInstance(writeApp);
46594657
FirebaseDatabase readDb = FirebaseDatabase.getInstance(readApp);
4660-
Semaphore semaphore = new Semaphore(0);
46614658
long val = 34;
46624659
String writeKey = Objects.requireNonNull(writeDb.getReference().push().getKey());
46634660
DatabaseReference writeNode = writeDb.getReference().child(writeKey);
46644661
await(writeNode.setValue(val));
46654662
DatabaseReference readNode = readDb.getReference().child(writeKey);
4666-
final int testArr[] = {0};
4667-
ValueEventListener listener =
4668-
new ValueEventListener() {
4669-
@Override
4670-
public void onDataChange(@NonNull DataSnapshot snapshot) {
4671-
assertEquals(0, testArr[0]++);
4672-
assertEquals(34L, snapshot.getValue());
4673-
semaphore.release();
4674-
}
4675-
4676-
@Override
4677-
public void onCancelled(@NonNull DatabaseError error) {
4678-
// no-op
4679-
}
4680-
};
4681-
readNode.addValueEventListener(listener);
4663+
new ReadFuture(
4664+
readNode,
4665+
events -> {
4666+
assertEquals(1, events.size());
4667+
assertEquals(val, events.get(0).getSnapshot().getValue());
4668+
return true;
4669+
})
4670+
.timedGet();
46824671

46834672
try {
4684-
IntegrationTestHelpers.waitFor(semaphore);
46854673
DataSnapshot snapshot = await(readNode.get());
46864674
assertEquals(val, snapshot.getValue());
4687-
readNode.removeEventListener(listener);
46884675
// need to rewrite it
46894676
} catch (ExecutionException e) {
46904677
fail("get threw an exception: " + e);
@@ -4698,31 +4685,30 @@ public void testGetResolvesToServerCacheWhenListenerIsAvailable()
46984685
FirebaseApp app =
46994686
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
47004687
FirebaseDatabase db = FirebaseDatabase.getInstance(app);
4701-
Semaphore semaphore = new Semaphore(0);
4688+
db.setPersistenceEnabled(false);
47024689
long val = 34;
47034690
DatabaseReference node = db.getReference().push();
4704-
node.setValue(val);
4705-
4706-
ValueEventListener listener =
4707-
new ValueEventListener() {
4708-
@Override
4709-
public void onDataChange(@NonNull DataSnapshot snapshot) {
4710-
assertEquals(34L, snapshot.getValue());
4711-
semaphore.release();
4712-
}
4713-
4714-
@Override
4715-
public void onCancelled(@NonNull DatabaseError error) {
4716-
// no-op
4717-
}
4718-
};
4719-
node.addValueEventListener(listener);
47204691

47214692
try {
4693+
await(node.setValue(val));
4694+
Semaphore semaphore = new Semaphore(0);
4695+
ValueEventListener listener =
4696+
new ValueEventListener() {
4697+
@Override
4698+
public void onDataChange(@NonNull DataSnapshot snapshot) {
4699+
assertEquals(val, snapshot.getValue());
4700+
semaphore.release();
4701+
}
4702+
4703+
@Override
4704+
public void onCancelled(@NonNull DatabaseError error) {}
4705+
};
4706+
node.addValueEventListener(listener);
47224707
IntegrationTestHelpers.waitFor(semaphore);
47234708
db.goOffline();
47244709
DataSnapshot snapshot = await(node.get());
47254710
assertEquals(val, snapshot.getValue());
4711+
node.removeEventListener(listener);
47264712
db.goOnline();
47274713
} catch (ExecutionException e) {
47284714
fail("get threw an exception: " + e);
@@ -4742,15 +4728,14 @@ public void testGetResolvesToCacheWhenOnlineAndChildLevelListener()
47424728
long val = 34;
47434729
String parentNodeKey = Objects.requireNonNull(writeDb.getReference().push().getKey());
47444730
DatabaseReference parentWriteNode = writeDb.getReference().child(parentNodeKey);
4745-
String childReadKey = Objects.requireNonNull(parentWriteNode.push().getKey());
4731+
String childNodeKey = Objects.requireNonNull(parentWriteNode.push().getKey());
47464732
DatabaseReference childReadNode =
4747-
readDb.getReference().child(parentNodeKey).child(childReadKey);
4748-
DatabaseReference childWriteNode = parentWriteNode.child(childReadKey);
4733+
readDb.getReference().child(parentNodeKey).child(childNodeKey);
4734+
DatabaseReference childWriteNode = parentWriteNode.child(childNodeKey);
47494735
ValueEventListener listener =
47504736
new ValueEventListener() {
47514737
@Override
47524738
public void onDataChange(@NonNull DataSnapshot snapshot) {
4753-
assertEquals(val, snapshot.getValue());
47544739
semaphore.release();
47554740
}
47564741

@@ -4762,24 +4747,31 @@ public void onCancelled(@NonNull DatabaseError error) {
47624747

47634748
try {
47644749
await(childWriteNode.setValue(val));
4750+
new ReadFuture(
4751+
childReadNode,
4752+
events -> {
4753+
assertEquals(1, events.size());
4754+
assertEquals(val, events.get(0).getSnapshot().getValue());
4755+
return true;
4756+
})
4757+
.timedGet();
47654758
childReadNode.addValueEventListener(listener);
47664759
DatabaseReference parentReadNode = readDb.getReference().child(parentNodeKey);
47674760
IntegrationTestHelpers.waitFor(semaphore);
47684761
DataSnapshot snapshot = await(parentReadNode.get());
4769-
assertEquals(snapshot.child(childReadKey).getValue(), val);
4770-
parentWriteNode.setValue(new MapBuilder().put(childReadKey, val).build());
4762+
assertEquals(snapshot.child(childNodeKey).getValue(), val);
4763+
await(parentWriteNode.setValue(new MapBuilder().put(childNodeKey, val).build()));
47714764
DataSnapshot parentSnapshot = await(childReadNode.get());
47724765
assertEquals(parentSnapshot.getValue(), val);
47734766
childReadNode.removeEventListener(listener);
4774-
} catch (ExecutionException e) {
4767+
} catch (ExecutionException | TimeoutException | TestFailure e) {
47754768
fail("get threw an exception: " + e);
47764769
}
47774770
}
47784771

47794772
@Test
47804773
public void testLimitToGetWithListenerOnlyFiresOnce()
4781-
throws DatabaseException, InterruptedException, ExecutionException, TestFailure,
4782-
TimeoutException {
4774+
throws DatabaseException, InterruptedException, ExecutionException {
47834775
FirebaseApp readApp =
47844776
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
47854777
FirebaseApp writeApp =
@@ -4789,30 +4781,55 @@ public void testLimitToGetWithListenerOnlyFiresOnce()
47894781
String refKey = UUID.randomUUID().toString();
47904782
DatabaseReference writeRef = writeDb.getReference().child(refKey);
47914783
DatabaseReference readRef = readDb.getReference().child(refKey);
4792-
await(
4793-
writeRef.setValue(new MapBuilder().put("child1", "test1").put("child2", "test2").build()));
4794-
ReadFuture readFuture =
4795-
new ReadFuture(
4796-
readRef,
4797-
events -> {
4798-
assertEquals(1, events.size());
4799-
DataSnapshot snapshot = events.get(0).getSnapshot();
4800-
assertEquals(2, snapshot.getChildrenCount());
4801-
assertEquals("test1", snapshot.child("child1").getValue());
4802-
assertEquals("test2", snapshot.child("child2").getValue());
4803-
return true;
4804-
});
4805-
readFuture.timedGet();
4784+
Map<String, Object> expected =
4785+
new MapBuilder().put("child1", "test1").put("child2", "test2").build();
4786+
try {
4787+
await(writeRef.setValue(expected));
4788+
ReadFuture.untilEquals(readRef, expected).timedGet();
4789+
} catch (Exception e) {
4790+
fail("Test failed with exception" + e);
4791+
}
4792+
48064793
Query query = readRef.limitToFirst(1);
48074794
DataSnapshot snapshot = await(query.get());
48084795
Map<String, Object> map = new MapBuilder().put("child1", "test1").build();
48094796
assertEquals(map, snapshot.getValue());
48104797
}
48114798

4799+
@Test
4800+
public void testDisjointedListenAndGet() {
4801+
FirebaseApp readApp =
4802+
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
4803+
FirebaseApp writeApp =
4804+
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
4805+
FirebaseDatabase readDb = FirebaseDatabase.getInstance(readApp);
4806+
FirebaseDatabase writeDb = FirebaseDatabase.getInstance(writeApp);
4807+
String parentKey = UUID.randomUUID().toString();
4808+
DatabaseReference writeRef = writeDb.getReference().child(parentKey);
4809+
DatabaseReference child1Ref = readDb.getReference().child(parentKey).child("child1");
4810+
Query otherChildrenQuery =
4811+
readDb.getReference().child(parentKey).orderByKey().startAt("child2");
4812+
Map<String, Object> topLevelValues =
4813+
new MapBuilder()
4814+
.put("child1", "test1")
4815+
.put("child2", "test2")
4816+
.put("child3", "test3")
4817+
.build();
4818+
try {
4819+
await(writeRef.setValue(topLevelValues));
4820+
ReadFuture.untilEquals(child1Ref, "test1");
4821+
DataSnapshot snapshot = await(otherChildrenQuery.get());
4822+
assertEquals(
4823+
new MapBuilder().put("child2", "test2").put("child3", "test3").build(),
4824+
snapshot.getValue());
4825+
} catch (Exception e) {
4826+
fail("Test failed with exception" + e);
4827+
}
4828+
}
4829+
48124830
@Test
48134831
public void testGetWithLimitToListenerOnlyFiresOnce()
4814-
throws DatabaseException, InterruptedException, ExecutionException, TestFailure,
4815-
TimeoutException {
4832+
throws DatabaseException, InterruptedException, ExecutionException {
48164833
FirebaseApp readApp =
48174834
appForDatabaseUrl(IntegrationTestValues.getDatabaseUrl(), UUID.randomUUID().toString());
48184835
FirebaseApp writeApp =
@@ -4823,18 +4840,14 @@ public void testGetWithLimitToListenerOnlyFiresOnce()
48234840
DatabaseReference writeRef = writeDb.getReference().child(refKey);
48244841
DatabaseReference defaultRef = readDb.getReference().child(refKey);
48254842
Query readQuery = defaultRef.limitToFirst(1);
4826-
await(
4827-
writeRef.setValue(new MapBuilder().put("child1", "test1").put("child2", "test2").build()));
4828-
ReadFuture readFuture =
4829-
new ReadFuture(
4830-
readQuery,
4831-
events -> {
4832-
assertEquals(1, events.size());
4833-
DataSnapshot snapshot = events.get(0).getSnapshot();
4834-
assertEquals("test1", snapshot.child("child1").getValue());
4835-
return true;
4836-
});
4837-
readFuture.timedGet();
4843+
Map<String, Object> expected =
4844+
new MapBuilder().put("child1", "test1").put("child2", "test2").build();
4845+
try {
4846+
await(writeRef.setValue(expected));
4847+
ReadFuture.untilEquals(readQuery, new MapBuilder().put("child1", "test1").build()).timedGet();
4848+
} catch (Exception e) {
4849+
fail("Test failed with exception" + e);
4850+
}
48384851
DataSnapshot snapshot = await(defaultRef.get());
48394852
Map<String, Object> map =
48404853
new MapBuilder().put("child1", "test1").put("child2", "test2").build();
@@ -4854,26 +4867,19 @@ public void testStartWithGetWithListenerOnlyFiresOnce()
48544867
String refKey = UUID.randomUUID().toString();
48554868
DatabaseReference writeRef = writeDb.getReference().child(refKey);
48564869
DatabaseReference readRef = readDb.getReference().child(refKey);
4857-
await(
4858-
writeRef.setValue(
4859-
new MapBuilder()
4860-
.put("child1", "test1")
4861-
.put("child2", "test2")
4862-
.put("child3", "test3")
4863-
.build()));
4864-
ReadFuture readFuture =
4865-
new ReadFuture(
4866-
readRef,
4867-
events -> {
4868-
assertEquals(1, events.size());
4869-
DataSnapshot snapshot = events.get(0).getSnapshot();
4870-
assertEquals(3, snapshot.getChildrenCount());
4871-
assertEquals("test1", snapshot.child("child1").getValue());
4872-
assertEquals("test2", snapshot.child("child2").getValue());
4873-
assertEquals("test3", snapshot.child("child3").getValue());
4874-
return true;
4875-
});
4876-
readFuture.timedGet();
4870+
Map<String, Object> expected =
4871+
new MapBuilder()
4872+
.put("child1", "test1")
4873+
.put("child2", "test2")
4874+
.put("child3", "test3")
4875+
.build();
4876+
try {
4877+
await(writeRef.setValue(expected));
4878+
ReadFuture.untilEquals(readRef, expected).timedGet();
4879+
} catch (Exception e) {
4880+
fail("Test failed with exception" + e);
4881+
}
4882+
48774883
Query query = readRef.orderByKey().startAt("child2");
48784884
DataSnapshot snapshot = await(query.get());
48794885
Map<String, Object> map =

‎firebase-database/src/main/java/com/google/firebase/database/core/Repo.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -550,10 +550,10 @@ public void run() {
550550
}
551551
} else {
552552
/*
553-
We need to replicate the behavior that occurs when running `once()`. In other words,
554-
we need to create a new eventRegistration, register it with a view and then
555-
overwrite the data at that location, and then remove the view.
556-
*/
553+
* We need to replicate the behavior that occurs when running `once()`. In other words,
554+
* we need to create a new eventRegistration, register it with a view and then
555+
* overwrite the data at that location, and then remove the view.
556+
*/
557557
Node serverNode = NodeUtilities.NodeFromJSON(task.getResult());
558558
QuerySpec spec = query.getSpec();
559559
// EventRegistrations require a listener to be attached, so a dummy
@@ -572,7 +572,8 @@ public void onCancelled(@NonNull DatabaseError error) {
572572
};
573573
ValueEventRegistration eventRegistration =
574574
new ValueEventRegistration(repo, listener, spec);
575-
serverSyncTree.addEventRegistration(eventRegistration, true);
575+
serverSyncTree.addEventRegistration(
576+
eventRegistration, /*skipListenerSetup=*/ true);
576577
if (spec.loadsAllData()) {
577578
serverSyncTree.applyServerOverwrite(spec.getPath(), serverNode);
578579
} else {
@@ -583,7 +584,8 @@ public void onCancelled(@NonNull DatabaseError error) {
583584
InternalHelpers.createDataSnapshot(
584585
query.getRef(),
585586
IndexedNode.from(serverNode, query.getSpec().getIndex())));
586-
serverSyncTree.removeEventRegistration(eventRegistration, true);
587+
serverSyncTree.removeEventRegistration(
588+
eventRegistration, /*skipDedup=*/ true);
587589
}
588590
});
589591
}

‎firebase-database/src/main/java/com/google/firebase/database/core/SyncTree.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -665,19 +665,14 @@ public List<Event> removeEventRegistration(
665665
eventRegistration.getQuerySpec(), eventRegistration, null, skipDedup);
666666
}
667667

668-
public List<Event> removeEventRegistration(
669-
QuerySpec query, @NotNull EventRegistration eventRegistration) {
670-
return this.removeEventRegistration(query, eventRegistration, null, false);
671-
}
672-
673668
/**
674669
* Remove all event callback(s).
675670
*
676671
* <p>If query is the default query, we'll check all queries for the specified eventRegistration.
677672
*/
678673
public List<Event> removeAllEventRegistrations(
679674
@NotNull QuerySpec query, @NotNull DatabaseError error) {
680-
return this.removeEventRegistration(query, null);
675+
return this.removeEventRegistration(query, null, error, false);
681676
}
682677

683678
private List<Event> removeEventRegistration(

0 commit comments

Comments
 (0)