Skip to content

Update invokeRun*QueryRpc functions to support paths with special characters #7402

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.

Sign up for GitHub

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 15 commits into from
Dec 20, 2023
Merged

Update invokeRun*QueryRpc functions to support paths with special characters #7402

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
1d76d33
Update invokeRun*QueryRpc functions to support paths with special cha…
MarkDuckworth Jun 28, 2023
2a8559e
Create shaggy-garlics-leave.md
MarkDuckworth Jun 28, 2023
77fac22
Fixing typos.
MarkDuckworth Jun 28, 2023
b112a15
Merge branch 'master' of github.com:firebase/firebase-js-sdk into mar…
MarkDuckworth Jun 28, 2023
7128eec
Merge branch 'markduckworth/run-rpc-path-special-chars' of github.com…
MarkDuckworth Jun 28, 2023
48c09d5
Merge remote-tracking branch 'origin/master' into markduckworth/run-r…
dconeybe Jun 29, 2023
a1707ea
Denver's fix
dconeybe Jun 29, 2023
aa7346a
Merge branch 'master' of github.com:firebase/firebase-js-sdk into mar…
MarkDuckworth Dec 13, 2023
cc839f4
PR feedback.
MarkDuckworth Dec 15, 2023
fbe27c3
Moving path encoding code from the Datastore implementation to the Co…
MarkDuckworth Dec 18, 2023
d0f2e87
Unit test bug.
MarkDuckworth Dec 18, 2023
ca6a28c
More fixes.
MarkDuckworth Dec 18, 2023
608ff83
Code cleanup: string[] replaced with ResourcePath.
MarkDuckworth Dec 20, 2023
a6c3460
Fix use of ResourcePath.
MarkDuckworth Dec 20, 2023
1ce5e4c
More build errors and cleanup.
MarkDuckworth Dec 20, 2023
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
4 changes: 2 additions & 2 deletions packages/firestore/src/platform/node/grpc_connection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export class GrpcConnection implements Connection {

invokeRPC<Req, Resp>(
rpcName: string,
path: string,
path: string[],
request: Req,
authToken: Token | null,
appCheckToken: Token | null
Expand Down Expand Up @@ -166,7 +166,7 @@ export class GrpcConnection implements Connection {

invokeStreamingRPC<Req, Resp>(
rpcName: string,
path: string,
path: string[],
request: Req,
authToken: Token | null,
appCheckToken: Token | null,
Expand Down
10 changes: 6 additions & 4 deletions packages/firestore/src/remote/connection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,15 @@ export interface Connection {
* representing the JSON to send.
*
* @param rpcName - the name of the RPC to invoke
* @param path - the path to invoke this RPC on
* @param path - the path to invoke this RPC on. An array of path segments
* that will be encoded and joined with path separators when required.
* @param request - the Raw JSON object encoding of the request message
* @param token - the Token to use for the RPC.
* @returns a Promise containing the JSON object encoding of the response
*/
invokeRPC<Req, Resp>(
rpcName: string,
path: string,
path: string[],
request: Req,
authToken: Token | null,
appCheckToken: Token | null
Expand All @@ -57,15 +58,16 @@ export interface Connection {
* completion and then returned as an array.
*
* @param rpcName - the name of the RPC to invoke
* @param path - the path to invoke this RPC on
* @param path - the path to invoke this RPC on. An array of path segments
* that will be encoded and joined with path separators when required.
* @param request - the Raw JSON object encoding of the request message
* @param token - the Token to use for the RPC.
* @returns a Promise containing an array with the JSON object encodings of the
* responses
*/
invokeStreamingRPC<Req, Resp>(
rpcName: string,
path: string,
path: string[],
request: Req,
authToken: Token | null,
appCheckToken: Token | null,
Expand Down
10 changes: 2 additions & 8 deletions packages/firestore/src/remote/datastore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,7 @@ class DatastoreImpl extends Datastore {
this.appCheckCredentials.getToken()
])
.then(([authToken, appCheckToken]) => {
const path = toResourcePath(databaseId, resourcePath)
.toArray()
.map(encodeURIComponent)
.join('/');
const path = toResourcePath(databaseId, resourcePath).toArray();
return this.connection.invokeRPC<Req, Resp>(
rpcName,
path,
Expand Down Expand Up @@ -145,10 +142,7 @@ class DatastoreImpl extends Datastore {
this.appCheckCredentials.getToken()
])
.then(([authToken, appCheckToken]) => {
const path = toResourcePath(databaseId, resourcePath)
.toArray()
.map(encodeURIComponent)
.join('/');
const path = toResourcePath(databaseId, resourcePath).toArray();
return this.connection.invokeStreamingRPC<Req, Resp>(
rpcName,
path,
Expand Down
6 changes: 3 additions & 3 deletions packages/firestore/src/remote/rest_connection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,13 @@ export abstract class RestConnection implements Connection {

invokeRPC<Req, Resp>(
rpcName: string,
path: string,
path: string[],
req: Req,
authToken: Token | null,
appCheckToken: Token | null
): Promise<Resp> {
const streamId = generateUniqueDebugId();
const url = this.makeUrl(rpcName, path);
const url = this.makeUrl(rpcName, path.map(encodeURIComponent).join('/'));
logDebug(LOG_TAG, `Sending RPC '${rpcName}' ${streamId}:`, url, req);

const headers: StringMap = {
Expand Down Expand Up @@ -119,7 +119,7 @@ export abstract class RestConnection implements Connection {

invokeStreamingRPC<Req, Resp>(
rpcName: string,
path: string,
path: string[],
request: Req,
authToken: Token | null,
appCheckToken: Token | null,
Expand Down
4 changes: 2 additions & 2 deletions packages/firestore/test/unit/remote/datastore.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('Datastore', () => {

invokeRPC<Req, Resp>(
rpcName: string,
path: string,
path: string[],
request: Req,
token: Token | null
): Promise<Resp> {
Expand All @@ -58,7 +58,7 @@ describe('Datastore', () => {

invokeStreamingRPC<Req, Resp>(
rpcName: string,
path: string,
path: string[],
request: Req,
token: Token | null
): Promise<Resp[]> {
Expand Down
10 changes: 5 additions & 5 deletions packages/firestore/test/unit/remote/rest_connection.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('RestConnection', () => {
it('url uses from path', async () => {
await connection.invokeRPC(
'Commit',
'projects/testproject/databases/(default)/documents',
'projects/testproject/databases/(default)/documents'.split('/'),
{},
null,
null
Expand All @@ -86,7 +86,7 @@ describe('RestConnection', () => {
it('merges headers', async () => {
await connection.invokeRPC(
'RunQuery',
'projects/testproject/databases/(default)/documents/foo',
'projects/testproject/databases/(default)/documents/foo'.split('/'),
{},
new OAuthToken('owner', User.UNAUTHENTICATED),
new AppCheckToken('some-app-check-token')
Expand All @@ -105,7 +105,7 @@ describe('RestConnection', () => {
it('empty app check token is not added to headers', async () => {
await connection.invokeRPC(
'RunQuery',
'projects/testproject/databases/(default)/documents/foo',
'projects/testproject/databases/(default)/documents/foo'.split('/'),
{},
null,
new AppCheckToken('')
Expand All @@ -124,7 +124,7 @@ describe('RestConnection', () => {
connection.nextResponse = Promise.resolve({ response: true });
const response = await connection.invokeRPC(
'RunQuery',
'projects/testproject/databases/(default)/documents/coll',
'projects/testproject/databases/(default)/documents/coll'.split('/'),
{},
null,
null
Expand All @@ -138,7 +138,7 @@ describe('RestConnection', () => {
return expect(
connection.invokeRPC(
'RunQuery',
'projects/testproject/databases/(default)/documents/coll',
'projects/testproject/databases/(default)/documents/coll'.split('/'),
{},
null,
null
Expand Down