Skip to content

Commit

Permalink
RTDB get shouldn't send requests for listened-to data (#4299)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmwski committed Jan 20, 2021
1 parent 898bf58 commit 04a0fea
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 34 deletions.
5 changes: 5 additions & 0 deletions .changeset/hip-turtles-travel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@firebase/database': patch
---

get()s issued for queries that are being listened to no longer send backend requests.
53 changes: 19 additions & 34 deletions packages/database/src/core/Repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,29 +307,29 @@ export class Repo {
* The purpose of `getValue` is to return the latest known value
* satisfying `query`.
*
* If the client is connected, this method will send a request
* to the server. If the client is not connected, then either:
* This method will first check for in-memory cached values
* belonging to active listeners. If they are found, such values
* are considered to be the most up-to-date.
*
* 1. The client was once connected, but not anymore.
* 2. The client has never connected, this is the first operation
* this repo is handling.
*
* In case (1), it's possible that the client still has an active
* listener, with cached data. Since this is the latest known
* value satisfying the query, that's what getValue will return.
* If there is no cached data, `getValue` surfaces an "offline"
* error.
*
* In case (2), `getValue` will trigger a time-limited connection
* attempt. If the client is unable to connect to the server, it
* will surface an "offline" error because there cannot be any
* cached data. On the other hand, if the client is able to connect,
* `getValue` will return the server's value for the query, if one
* exists.
* If the client is not connected, this method will try to
* establish a connection and request the value for `query`. If
* the client is not able to retrieve the query result, it reports
* an error.
*
* @param query - The query to surface a value for.
*/
getValue(query: Query): Promise<DataSnapshot> {
// Only active queries are cached. There is no persisted cache.
const cached = this.serverSyncTree_.calcCompleteEventCache(query.path);
if (!cached.isEmpty()) {
return Promise.resolve(
new DataSnapshot(
cached,
query.getRef(),
query.getQueryParams().getIndex()
)
);
}
return this.server_.get(query).then(
payload => {
const node = nodeFromJSON(payload as string);
Expand All @@ -347,22 +347,7 @@ export class Repo {
);
},
err => {
this.log_(
'get for query ' +
stringify(query) +
' falling back to cache after error: ' +
err
);
const cached = this.serverSyncTree_.calcCompleteEventCache(query.path);
if (!cached.isEmpty()) {
return Promise.resolve(
new DataSnapshot(
cached,
query.getRef(),
query.getQueryParams().getIndex()
)
);
}
this.log_('get for query ' + stringify(query) + ' failed: ' + err);
return Promise.reject(new Error(err as string));
}
);
Expand Down
6 changes: 6 additions & 0 deletions packages/database/test/query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3128,6 +3128,12 @@ describe('Query Tests', () => {
}
});

it('get returns the latest value', async () => {
const node = getRandomNode() as Reference;
await node.set({ foo: 'bar' });
expect(node.get()).to.eventually.equal({ foo: 'bar' });
});

it('get reads from cache if database is not connected', async () => {
const node = getRandomNode() as Reference;
const node2 = getFreshRepo(node.path);
Expand Down

0 comments on commit 04a0fea

Please sign in to comment.