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

Query get() method for RTDB #3812

Merged
merged 38 commits into from
Nov 17, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
86363d6
Query get() method for RTDB
jmwski Sep 18, 2020
0abd55c
Fix yarn test
jmwski Sep 18, 2020
437b8a7
Call onDataUpdate_ to simulater server push
jmwski Sep 18, 2020
8b289f8
Run yarn prettier
jmwski Sep 18, 2020
153dbcf
cleanup
jmwski Sep 18, 2020
61665bf
Add basic tests for get
jmwski Sep 23, 2020
9b2cf00
remove callback arguments to get()
jmwski Sep 23, 2020
ab34805
get rid of todo
jmwski Sep 23, 2020
f20c903
Use promises instead + caching test
jmwski Oct 6, 2020
d9530ee
Uncomment tests
jmwski Oct 6, 2020
48c6d39
test rename
jmwski Oct 6, 2020
2bae15b
fix checks
jmwski Oct 6, 2020
578f9ca
x
jmwski Oct 9, 2020
b6cef9f
actually called repo.get
jmwski Oct 9, 2020
5802286
more fixes
jmwski Oct 9, 2020
5eae993
remove extra diff
jmwski Oct 9, 2020
2a7f81f
fixup query tests
jmwski Oct 12, 2020
00a0959
more test fixes
jmwski Oct 12, 2020
7ef7e69
use deep.equal in tests
jmwski Oct 12, 2020
272587f
Undo package.json changes
jmwski Oct 12, 2020
377a263
remove unused imports
jmwski Oct 12, 2020
d400785
remove accidental push
jmwski Oct 12, 2020
b19f026
more unused imports cleanup
jmwski Oct 12, 2020
c0160b6
Merge branch 'master' into jw/rtdb-query-get
jmwski Oct 12, 2020
c050c0d
lint stuff
jmwski Oct 12, 2020
bdc10ca
point CI to emulator v4.6.0
jmwski Oct 13, 2020
950de5a
address review feedback
jmwski Oct 21, 2020
fdc379d
undo package.json change
jmwski Oct 21, 2020
b81d898
review feedback
jmwski Oct 22, 2020
483cedc
update changelog
jmwski Oct 22, 2020
a7b0e67
Create many-snails-kneel.md
jmwski Oct 22, 2020
2a947f7
import fixes
jmwski Oct 22, 2020
acdd4ca
review feedback, queue outstanding gets
jmwski Nov 4, 2020
c0f627c
lint
jmwski Nov 4, 2020
f6dcb38
review
jmwski Nov 12, 2020
26703f5
remove extra comment
jmwski Nov 12, 2020
008e6fb
Update .changeset/many-snails-kneel.md
jmwski Nov 17, 2020
2643f8a
Update packages/database/src/core/Repo.ts
jmwski Nov 17, 2020
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
15 changes: 15 additions & 0 deletions packages/database/src/api/Query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ export interface SnapshotCallback {
(a: DataSnapshot, b?: string | null): unknown;
}

export interface FailureCallback {
(a: Error): void;
}

/**
* A Query represents a filter to be applied to a firebase location. This object purely represents the
* query expression (and exposes our public API to build the query). The actual query logic is in ViewBase.js.
Expand Down Expand Up @@ -295,6 +299,17 @@ export class Query {
this.repo.removeEventCallbackForQuery(this, container);
}

get(): Promise<DataSnapshot> {
const deferred = new Deferred<DataSnapshot>();
const callback = (snapshot: DataSnapshot) => {
deferred.resolve(snapshot);
};
this.repo.get(this, callback, err => {
deferred.reject(err);
});
jmwski marked this conversation as resolved.
Show resolved Hide resolved
return deferred.promise;
}

/**
* Attaches a listener, waits for the first event, and then removes the listener
* @param {!string} eventType
Expand Down
58 changes: 58 additions & 0 deletions packages/database/src/core/PersistentConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ interface OutstandingPut {
onComplete: (a: string, b?: string) => void;
}

interface OutstandingGet {
action: string;
jmwski marked this conversation as resolved.
Show resolved Hide resolved
request: object;
queued?: boolean;
onComplete: (a: string, b?: string) => void;
jmwski marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Firebase connection. Abstracts wire protocol and handles reconnecting.
*
Expand All @@ -92,6 +99,8 @@ export class PersistentConnection extends ServerActions {
/* path */ string,
Map</* queryId */ string, ListenSpec>
> = new Map();
private outstandingGets_: OutstandingGet[] = [];
private outstandingGetCount_ = 0;
private outstandingPuts_: OutstandingPut[] = [];
private outstandingPutCount_ = 0;
private onDisconnectRequestQueue_: OnDisconnectRequest[] = [];
Expand Down Expand Up @@ -184,6 +193,29 @@ export class PersistentConnection extends ServerActions {
}
}

get(query: Query, onComplete: (a: string, b: unknown) => void) {
jmwski marked this conversation as resolved.
Show resolved Hide resolved
const action = 'g';

const req: { [k: string]: unknown } = {
p: query.path.toString(),
q: query.queryObject()
};

this.outstandingGets_.push({
action,
request: req,
onComplete
});

this.outstandingGetCount_++;

if (this.connected_) {
this.sendGet_(this.outstandingGets_.length - 1);
} else {
this.log_('Buffering get: ' + query.path.toString());
}
}

/**
* @inheritDoc
*/
Expand Down Expand Up @@ -221,6 +253,26 @@ export class PersistentConnection extends ServerActions {
}
}

private sendGet_(index: number) {
const action = this.outstandingGets_[index].action;
const request = this.outstandingGets_[index].request;
const onComplete = this.outstandingGets_[index].onComplete;
this.outstandingGets_[index].queued = this.connected_;

this.sendRequest(action, request, (message: { [k: string]: unknown }) => {
delete this.outstandingGets_[index];
this.outstandingGetCount_--;

if (this.outstandingGetCount_ === 0) {
this.outstandingGets_ = [];
}

if (onComplete) {
onComplete(message['s'] as string, message['d'] as string);
}
});
}

private sendListen_(listenSpec: ListenSpec) {
const query = listenSpec.query;
const pathString = query.path.toString();
Expand Down Expand Up @@ -935,6 +987,12 @@ export class PersistentConnection extends ServerActions {
}
}

for (let i = 0; i < this.outstandingGets_.length; i++) {
if (this.outstandingGets_[i]) {
this.sendGet_(i);
}
}

for (let i = 0; i < this.outstandingPuts_.length; i++) {
if (this.outstandingPuts_[i]) {
this.sendPut_(i);
Expand Down
28 changes: 27 additions & 1 deletion packages/database/src/core/Repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@ import { ReadonlyRestClient } from './ReadonlyRestClient';
import { FirebaseApp } from '@firebase/app-types';
import { RepoInfo } from './RepoInfo';
import { Database } from '../api/Database';
import { DataSnapshot } from '../api/DataSnapshot';
import { ServerActions } from './ServerActions';
import { Query } from '../api/Query';
import { FailureCallback, SnapshotCallback, Query } from '../api/Query';
import { EventRegistration } from './view/EventRegistration';
import { StatsCollection } from './stats/StatsCollection';
import { Event } from './view/Event';
Expand Down Expand Up @@ -296,6 +297,31 @@ export class Repo {
return this.nextWriteId_++;
}

get(query: Query, onComplete: SnapshotCallback, onFailure: FailureCallback) {
this.server_.get(query, (status, payload) => {
if (status === 'ok') {
// TODO(wyszynski): Is this the correct way to update client SDK caches
// with the data that we've received from get()?
this.onDataUpdate_(
query.path.toString(),
payload,
/*isMerge=*/ false,
/*tag=*/ null
);
const newNode = nodeFromJSON(payload as string);
onComplete(
new DataSnapshot(
newNode,
query.getRef(),
query.getQueryParams().getIndex()
)
);
} else {
onFailure(Error(payload as string));
}
});
}

setWithPriority(
path: Path,
newVal: unknown,
Expand Down
7 changes: 7 additions & 0 deletions packages/database/src/core/ServerActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ export abstract class ServerActions {
*/
abstract unlisten(query: Query, tag: number | null): void;

/**
* Get the server value satisfying this query.
* @param query
jmwski marked this conversation as resolved.
Show resolved Hide resolved
* @param onComplete
*/
get(query: Query, onComplete: (a: string, b: unknown) => void): void {}

/**
* @param {string} pathString
* @param {*} data
Expand Down
43 changes: 43 additions & 0 deletions packages/database/test/query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2191,6 +2191,49 @@ describe('Query Tests', () => {
});
});

it('get() at empty root returns null', done => {
jmwski marked this conversation as resolved.
Show resolved Hide resolved
jmwski marked this conversation as resolved.
Show resolved Hide resolved
const node = getRandomNode() as Reference;
node.get().then(snapshot => {
const val = snapshot.val();
expect(val).to.be.null;
done();
});
jmwski marked this conversation as resolved.
Show resolved Hide resolved
});

it('get() at non-empty root returns correct value', done => {
jmwski marked this conversation as resolved.
Show resolved Hide resolved
const nodes = getRandomNode(2) as Reference;
const reader = nodes[0];
const writer = nodes[1];
writer.set({ foo: 'a', bar: 'b' }, (err, dummy) => {
reader.get().then(snapshot => {
const val = snapshot.val();
expect(val['foo']).to.equal('a');
expect(val['bar']).to.equal('b');
done();
});
});
jmwski marked this conversation as resolved.
Show resolved Hide resolved
});

it('get() for removed node returns correct value', done => {
const nodes = getRandomNode(2) as Reference;
const reader = nodes[0];
const writer = nodes[1];
writer.set({ foo: 'a', bar: 'b' }, (err, dummy) => {
reader.get().then(snapshot => {
const val = snapshot.val();
expect(val['foo']).to.equal('a');
expect(val['bar']).to.equal('b');
writer.remove().then(err => {
reader.get().then(snapshot => {
const val = snapshot.val();
expect(val).to.be.null;
done();
});
});
});
});
});

it('set() at query root raises correct value event', done => {
const nodePair = getRandomNode(2);
const writer = nodePair[0];
Expand Down