Skip to content

RTDB get shouldn't send requests for listened-to data #4299

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

Merged
merged 3 commits into from
Jan 20, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
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
---

RTDB query gets shouldn't send server requests for listened-to data
52 changes: 18 additions & 34 deletions packages/database/src/core/Repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,29 +307,28 @@ 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> {
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 +346,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