merged in feature branch

Author: DellaBitta

.changeset/shaggy-garlics-leave.md

@@ -0,0 +1,5 @@
+---
+"@firebase/firestore": patch
+---
+
+Support special characters in query paths sent to `getCountFromServer(...)`, `getCount(...)` (lite API), and `getDocs(...)` (lite API). 

.github/CODEOWNERS

@@ -34,10 +34,10 @@ packages/messaging-interop-types @zwu52 @firebase/jssdk-global-approvers
 integration/messaging @zwu52 @firebase/jssdk-global-approvers
 
 # Auth Code
-packages/auth @lisajian @prameshj @renkelvin @sam-gc @firebase/jssdk-global-approvers
-packages/auth-compat @lisajian @prameshj @renkelvin @sam-gc @firebase/jssdk-global-approvers
-packages/auth-types @lisajian @prameshj @renkelvin @sam-gc @firebase/jssdk-global-approvers
-packages/auth-interop-types @lisajian @prameshj @renkelvin @sam-gc @firebase/jssdk-global-approvers
+packages/auth @lisajian @Xiaoshouzi-gh @renkelvin @sam-gc @firebase/jssdk-global-approvers
+packages/auth-compat @lisajian @Xiaoshouzi-gh @renkelvin @sam-gc @firebase/jssdk-global-approvers
+packages/auth-types @lisajian @Xiaoshouzi-gh @renkelvin @sam-gc @firebase/jssdk-global-approvers
+packages/auth-interop-types @lisajian @Xiaoshouzi-gh @renkelvin @sam-gc @firebase/jssdk-global-approvers
 
 # Testing Code
 packages/rules-unit-testing  @avolkovi @sam-gc @yuchenshi @firebase/jssdk-global-approvers

packages/firestore/src/local/local_serializer.ts

@@ -285,7 +285,7 @@ export function toDbTarget(
     queryProto = toQueryTarget(
       localSerializer.remoteSerializer,
       targetData.target
-    );
+    ).queryTarget;
   }
 
   // We can't store the resumeToken as a ByteString in IndexedDb, so we

packages/firestore/src/model/path.ts

@@ -215,6 +215,15 @@ export class ResourcePath extends BasePath<ResourcePath> {
     return this.canonicalString();
   }
 
+  /**
+   * Returns a string representation of this path
+   * where each path segment has been encoded with
+   * `encodeURIComponent`.
+   */
+  toUriEncodedString(): string {
+    return this.toArray().map(encodeURIComponent).join('/');
+  }
+
   /**
    * Creates a resource path from the given slash-delimited string. If multiple
    * arguments are provided, all components are combined. Leading and trailing

packages/firestore/src/platform/node/grpc_connection.ts

@@ -22,6 +22,7 @@ import * as grpc from '@grpc/grpc-js';
 import { Token } from '../../api/credentials';
 import { DatabaseInfo } from '../../core/database_info';
 import { SDK_VERSION } from '../../core/version';
+import { ResourcePath } from '../../model/path';
 import { Connection, Stream } from '../../remote/connection';
 import { mapCodeFromRpcCode } from '../../remote/rpc_error';
 import { StreamBridge } from '../../remote/stream_bridge';
@@ -114,7 +115,7 @@ export class GrpcConnection implements Connection {
 
   invokeRPC<Req, Resp>(
     rpcName: string,
-    path: string,
+    path: ResourcePath,
     request: Req,
     authToken: Token | null,
     appCheckToken: Token | null
@@ -166,7 +167,7 @@ export class GrpcConnection implements Connection {
 
   invokeStreamingRPC<Req, Resp>(
     rpcName: string,
-    path: string,
+    path: ResourcePath,
     request: Req,
     authToken: Token | null,
     appCheckToken: Token | null,

packages/firestore/src/remote/connection.ts

@@ -16,6 +16,7 @@
  */
 
 import { Token } from '../api/credentials';
+import { ResourcePath } from '../model/path';
 import { FirestoreError } from '../util/error';
 
 /**
@@ -38,14 +39,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: ResourcePath,
     request: Req,
     authToken: Token | null,
     appCheckToken: Token | null
@@ -57,15 +59,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: ResourcePath,
     request: Req,
     authToken: Token | null,
     appCheckToken: Token | null,

packages/firestore/src/remote/datastore.ts

@@ -18,10 +18,12 @@
 import { CredentialsProvider } from '../api/credentials';
 import { User } from '../auth/user';
 import { Aggregate } from '../core/aggregate';
+import { DatabaseId } from '../core/database_info';
 import { queryToAggregateTarget, Query, queryToTarget } from '../core/query';
 import { Document } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { Mutation } from '../model/mutation';
+import { ResourcePath } from '../model/path';
 import {
   ApiClientObjectMap,
   BatchGetDocumentsRequest as ProtoBatchGetDocumentsRequest,
@@ -47,11 +49,11 @@ import {
 import {
   fromDocument,
   fromBatchGetDocumentsResponse,
-  getEncodedDatabaseId,
   JsonProtoSerializer,
   toMutation,
   toName,
   toQueryTarget,
+  toResourcePath,
   toRunAggregationQueryRequest
 } from './serializer';
 
@@ -94,7 +96,8 @@ class DatastoreImpl extends Datastore {
   /** Invokes the provided RPC with auth and AppCheck tokens. */
   invokeRPC<Req, Resp>(
     rpcName: string,
-    path: string,
+    databaseId: DatabaseId,
+    resourcePath: ResourcePath,
     request: Req
   ): Promise<Resp> {
     this.verifyInitialized();
@@ -105,7 +108,7 @@ class DatastoreImpl extends Datastore {
       .then(([authToken, appCheckToken]) => {
         return this.connection.invokeRPC<Req, Resp>(
           rpcName,
-          path,
+          toResourcePath(databaseId, resourcePath),
           request,
           authToken,
           appCheckToken
@@ -127,7 +130,8 @@ class DatastoreImpl extends Datastore {
   /** Invokes the provided RPC with streamed results with auth and AppCheck tokens. */
   invokeStreamingRPC<Req, Resp>(
     rpcName: string,
-    path: string,
+    databaseId: DatabaseId,
+    resourcePath: ResourcePath,
     request: Req,
     expectedResponseCount?: number
   ): Promise<Resp[]> {
@@ -139,7 +143,7 @@ class DatastoreImpl extends Datastore {
       .then(([authToken, appCheckToken]) => {
         return this.connection.invokeStreamingRPC<Req, Resp>(
           rpcName,
-          path,
+          toResourcePath(databaseId, resourcePath),
           request,
           authToken,
           appCheckToken,
@@ -186,26 +190,35 @@ export async function invokeCommitRpc(
   mutations: Mutation[]
 ): Promise<void> {
   const datastoreImpl = debugCast(datastore, DatastoreImpl);
-  const path = getEncodedDatabaseId(datastoreImpl.serializer) + '/documents';
   const request = {
     writes: mutations.map(m => toMutation(datastoreImpl.serializer, m))
   };
-  await datastoreImpl.invokeRPC('Commit', path, request);
+  await datastoreImpl.invokeRPC(
+    'Commit',
+    datastoreImpl.serializer.databaseId,
+    ResourcePath.emptyPath(),
+    request
+  );
 }
 
 export async function invokeBatchGetDocumentsRpc(
   datastore: Datastore,
   keys: DocumentKey[]
 ): Promise<Document[]> {
   const datastoreImpl = debugCast(datastore, DatastoreImpl);
-  const path = getEncodedDatabaseId(datastoreImpl.serializer) + '/documents';
   const request = {
     documents: keys.map(k => toName(datastoreImpl.serializer, k))
   };
   const response = await datastoreImpl.invokeStreamingRPC<
     ProtoBatchGetDocumentsRequest,
     ProtoBatchGetDocumentsResponse
-  >('BatchGetDocuments', path, request, keys.length);
+  >(
+    'BatchGetDocuments',
+    datastoreImpl.serializer.databaseId,
+    ResourcePath.emptyPath(),
+    request,
+    keys.length
+  );
 
   const docs = new Map<string, Document>();
   response.forEach(proto => {
@@ -226,11 +239,16 @@ export async function invokeRunQueryRpc(
   query: Query
 ): Promise<Document[]> {
   const datastoreImpl = debugCast(datastore, DatastoreImpl);
-  const request = toQueryTarget(datastoreImpl.serializer, queryToTarget(query));
+  const { queryTarget, parent } = toQueryTarget(
+    datastoreImpl.serializer,
+    queryToTarget(query)
+  );
   const response = await datastoreImpl.invokeStreamingRPC<
     ProtoRunQueryRequest,
     ProtoRunQueryResponse
-  >('RunQuery', request.parent!, { structuredQuery: request.structuredQuery });
+  >('RunQuery', datastoreImpl.serializer.databaseId, parent, {
+    structuredQuery: queryTarget.structuredQuery
+  });
   return (
     response
       // Omit RunQueryResponses that only contain readTimes.
@@ -247,20 +265,25 @@ export async function invokeRunAggregationQueryRpc(
   aggregates: Aggregate[]
 ): Promise<ApiClientObjectMap<Value>> {
   const datastoreImpl = debugCast(datastore, DatastoreImpl);
-  const { request, aliasMap } = toRunAggregationQueryRequest(
+  const { request, aliasMap, parent } = toRunAggregationQueryRequest(
     datastoreImpl.serializer,
     queryToAggregateTarget(query),
     aggregates
   );
 
-  const parent = request.parent;
   if (!datastoreImpl.connection.shouldResourcePathBeIncludedInRequest) {
     delete request.parent;
   }
   const response = await datastoreImpl.invokeStreamingRPC<
     ProtoRunAggregationQueryRequest,
     ProtoRunAggregationQueryResponse
-  >('RunAggregationQuery', parent!, request, /*expectedResponseCount=*/ 1);
+  >(
+    'RunAggregationQuery',
+    datastoreImpl.serializer.databaseId,
+    parent,
+    request,
+    /*expectedResponseCount=*/ 1
+  );
 
   // Omit RunAggregationQueryResponse that only contain readTimes.
   const filteredResult = response.filter(proto => !!proto.result);

packages/firestore/src/remote/rest_connection.ts

@@ -22,6 +22,7 @@ import {
   DatabaseInfo,
   DEFAULT_DATABASE_NAME
 } from '../core/database_info';
+import { ResourcePath } from '../model/path';
 import { debugAssert } from '../util/assert';
 import { generateUniqueDebugId } from '../util/debug_uid';
 import { FirestoreError } from '../util/error';
@@ -82,13 +83,13 @@ export abstract class RestConnection implements Connection {
 
   invokeRPC<Req, Resp>(
     rpcName: string,
-    path: string,
+    path: ResourcePath,
     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.toUriEncodedString());
     logDebug(LOG_TAG, `Sending RPC '${rpcName}' ${streamId}:`, url, req);
 
     const headers: StringMap = {
@@ -119,7 +120,7 @@ export abstract class RestConnection implements Connection {
 
   invokeStreamingRPC<Req, Resp>(
     rpcName: string,
-    path: string,
+    path: ResourcePath,
     request: Req,
     authToken: Token | null,
     appCheckToken: Token | null,