Review

Author: schmidt-sebastian

packages/firestore/src/core/target.ts

@@ -21,8 +21,7 @@ import {
   FieldIndex,
   fieldIndexGetArraySegment,
   fieldIndexGetDirectionalSegments,
-  IndexKind,
-  IndexSegment
+  IndexKind
 } from '../model/field_index';
 import { FieldPath, ResourcePath } from '../model/path';
 import {
@@ -308,8 +307,12 @@ export function targetGetLowerBound(
   for (const segment of fieldIndexGetDirectionalSegments(fieldIndex)) {
     const segmentBound =
       segment.kind === IndexKind.ASCENDING
-        ? targetGetAscendingBound(target, segment, target.startAt)
-        : targetGetDescendingBound(target, segment, target.startAt);
+        ? targetGetLowerBoundForField(target, segment.fieldPath, target.startAt)
+        : targetGetUpperBoundForField(
+            target,
+            segment.fieldPath,
+            target.startAt
+          );
 
     if (!segmentBound.value) {
       // No lower bound exists
@@ -338,8 +341,8 @@ export function targetGetUpperBound(
   for (const segment of fieldIndexGetDirectionalSegments(fieldIndex)) {
     const segmentBound =
       segment.kind === IndexKind.ASCENDING
-        ? targetGetDescendingBound(target, segment, target.endAt)
-        : targetGetAscendingBound(target, segment, target.endAt);
+        ? targetGetUpperBoundForField(target, segment.fieldPath, target.endAt)
+        : targetGetLowerBoundForField(target, segment.fieldPath, target.endAt);
 
     if (!segmentBound.value) {
       // No upper bound exists
@@ -352,19 +355,20 @@ export function targetGetUpperBound(
   return new Bound(values, inclusive);
 }
 
-function targetGetAscendingBound(
+/**
+ * Returns the value to use as the lower bound for ascending index segment at
+ * the provided `fieldPath` (or the upper bound for an descending segment).
+ */
+function targetGetLowerBoundForField(
   target: Target,
-  segment: IndexSegment,
+  fieldPath: FieldPath,
   bound: Bound | null
 ): { value: ProtoValue | undefined; inclusive: boolean } {
   let value: ProtoValue | undefined = undefined;
   let inclusive = true;
 
   // Process all filters to find a value for the current field segment
-  for (const fieldFilter of targetGetFieldFiltersForPath(
-    target,
-    segment.fieldPath
-  )) {
+  for (const fieldFilter of targetGetFieldFiltersForPath(target, fieldPath)) {
     let filterValue: ProtoValue | undefined = undefined;
     let filterInclusive = true;
 
@@ -401,7 +405,7 @@ function targetGetAscendingBound(
   if (bound !== null) {
     for (let i = 0; i < target.orderBy.length; ++i) {
       const orderBy = target.orderBy[i];
-      if (orderBy.field.isEqual(segment.fieldPath)) {
+      if (orderBy.field.isEqual(fieldPath)) {
         const cursorValue = bound.position[i];
         if (valuesMax(value, cursorValue) === cursorValue) {
           value = cursorValue;
@@ -415,19 +419,20 @@ function targetGetAscendingBound(
   return { value, inclusive };
 }
 
-function targetGetDescendingBound(
+/**
+ * Returns the value to use as the upper bound for ascending index segment at
+ * the provided `fieldPath` (or the lower bound for an descending segment).
+ */
+function targetGetUpperBoundForField(
   target: Target,
-  segment: IndexSegment,
+  fieldPath: FieldPath,
   bound: Bound | null
 ): { value: ProtoValue | undefined; inclusive: boolean } {
   let value: ProtoValue | undefined = undefined;
   let inclusive = true;
 
   // Process all filters to find a value for the current field segment
-  for (const fieldFilter of targetGetFieldFiltersForPath(
-    target,
-    segment.fieldPath
-  )) {
+  for (const fieldFilter of targetGetFieldFiltersForPath(target, fieldPath)) {
     let filterValue: ProtoValue | undefined = undefined;
     let filterInclusive = true;
 
@@ -465,7 +470,7 @@ function targetGetDescendingBound(
   if (bound !== null) {
     for (let i = 0; i < target.orderBy.length; ++i) {
       const orderBy = target.orderBy[i];
-      if (orderBy.field.isEqual(segment.fieldPath)) {
+      if (orderBy.field.isEqual(fieldPath)) {
         const cursorValue = bound.position[i];
         if (valuesMin(value, cursorValue) === cursorValue) {
           value = cursorValue;

packages/firestore/src/local/indexeddb_index_manager.ts

@@ -295,8 +295,8 @@ export class IndexedDbIndexManager implements IndexManager {
                 .loadFirst(indexRange, target.limit)
                 .next(entries => {
                   entries.forEach(entry => {
-                    const documentKey = new DocumentKey(
-                      decodeResourcePath(entry.documentKey)
+                    const documentKey = DocumentKey.fromSegments(
+                      entry.documentKey
                     );
                     if (!existingKeys.has(documentKey)) {
                       existingKeys = existingKeys.add(documentKey);
@@ -737,7 +737,7 @@ export class IndexedDbIndexManager implements IndexManager {
       arrayValue: indexEntry.arrayValue,
       directionalValue: indexEntry.directionalValue,
       orderedDocumentKey: this.encodeDirectionalKey(fieldIndex, document.key),
-      documentKey: encodeResourcePath(document.key.path)
+      documentKey: document.key.path.toArray()
     });
   }
 
@@ -754,7 +754,7 @@ export class IndexedDbIndexManager implements IndexManager {
       indexEntry.arrayValue,
       indexEntry.directionalValue,
       this.encodeDirectionalKey(fieldIndex, document.key),
-      encodeResourcePath(document.key.path)
+      document.key.path.toArray()
     ]);
   }
 
@@ -935,16 +935,16 @@ export class IndexedDbIndexManager implements IndexManager {
             this.uid,
             bounds[i].arrayValue,
             bounds[i].directionalValue,
-            new Uint8Array(),
-            ''
+            EMPTY_VALUE,
+            []
           ] as DbIndexEntryKey,
           [
             bounds[i + 1].indexId,
             this.uid,
             bounds[i + 1].arrayValue,
             bounds[i + 1].directionalValue,
-            new Uint8Array(),
-            ''
+            EMPTY_VALUE,
+            []
           ] as DbIndexEntryKey
         )
       );

packages/firestore/src/local/indexeddb_schema.ts

@@ -519,8 +519,8 @@ export interface DbIndexEntry {
    * encoder to match the key order of the index.
    */
   orderedDocumentKey: Uint8Array;
-  /** The document key this entry points to. */
-  documentKey: EncodedResourcePath;
+  /** The segments of the document key this entry points to. */
+  documentKey: string[];
 }
 
 /**

packages/firestore/src/local/indexeddb_sentinels.ts

@@ -293,7 +293,7 @@ export type DbIndexEntryKey = [
   Uint8Array,
   Uint8Array,
   Uint8Array,
-  string
+  string[]
 ];
 
 /** Name of the IndexedDb object store. */
@@ -313,7 +313,7 @@ export const DbIndexEntryDocumentKeyIndex = 'documentKeyIndex';
 export const DbIndexEntryDocumentKeyIndexPath = [
   'indexId',
   'uid',
-  'documentKey'
+  'orderedDocumentKey'
 ];
 
 export type DbDocumentOverlayKey = [

packages/firestore/src/model/field_index.ts

@@ -80,7 +80,7 @@ export function fieldIndexGetDirectionalSegments(
 }
 
 /**
- * Returns the order of the document key component.
+ * Returns the order of the document key component for the given index.
  *
  * PORTING NOTE: This is only used in the Web IndexedDb implementation.
  */