feat: remove valuesAs just use indexing

Author: nbbeeken

src/cmap/wire_protocol/on_demand/document.ts

@@ -58,7 +58,7 @@ export class OnDemandDocument {
   private readonly indexFound: Record<number, boolean> = Object.create(null);
 
   /** All bson elements in this document */
-  private readonly elements: BSONElement[];
+  private readonly elements: ReadonlyArray<BSONElement>;
 
   constructor(
     /** BSON bytes, this document begins at offset */
@@ -97,14 +97,30 @@ export class OnDemandDocument {
    * @param name - a basic latin string name of a BSON element
    * @returns
    */
-  private getElement(name: string): CachedBSONElement | null {
+  private getElement(name: string | number): CachedBSONElement | null {
     const cachedElement = this.cache[name];
     if (cachedElement === false) return null;
 
     if (cachedElement != null) {
       return cachedElement;
     }
 
+    if (typeof name === 'number') {
+      if (this.isArray) {
+        if (name < this.elements.length) {
+          const element = this.elements[name];
+          const cachedElement = { element, value: undefined };
+          this.cache[name] = cachedElement;
+          this.indexFound[name] = true;
+          return cachedElement;
+        } else {
+          return null;
+        }
+      } else {
+        return null;
+      }
+    }
+
     for (let index = 0; index < this.elements.length; index++) {
       const element = this.elements[index];
 
@@ -229,16 +245,20 @@ export class OnDemandDocument {
    * @param required - whether or not the element is expected to exist, if true this function will throw if it is not present
    */
   public get<const T extends keyof JSTypeOf>(
-    name: string,
+    name: string | number,
     as: T,
     required?: false | undefined
   ): JSTypeOf[T] | null;
 
   /** `required` will make `get` throw if name does not exist or is null/undefined */
-  public get<const T extends keyof JSTypeOf>(name: string, as: T, required: true): JSTypeOf[T];
+  public get<const T extends keyof JSTypeOf>(
+    name: string | number,
+    as: T,
+    required: true
+  ): JSTypeOf[T];
 
   public get<const T extends keyof JSTypeOf>(
-    name: string,
+    name: string | number,
     as: T,
     required?: boolean
   ): JSTypeOf[T] | null {
@@ -315,22 +335,4 @@ export class OnDemandDocument {
     const size = getInt32LE(this.bson, this.offset);
     return this.bson.subarray(this.offset, this.offset + size);
   }
-
-  /**
-   * Iterates through the elements of a document reviving them using the `as` BSONType.
-   *
-   * @param as - The type to revive all elements as
-   */
-  public *valuesAs<const T extends keyof JSTypeOf>(as: T): Generator<JSTypeOf[T], void, void> {
-    if (!this.isArray) {
-      throw new BSONError('Unexpected conversion of non-array value to array');
-    }
-    let counter = 0;
-    for (const element of this.elements) {
-      const value = this.toJSValue<T>(element, as);
-      this.cache[counter] = { element, value };
-      yield value;
-      counter += 1;
-    }
-  }
 }

src/cmap/wire_protocol/responses.ts

@@ -147,11 +147,10 @@ export class CursorResponse extends MongoDBResponse {
   public batchSize = 0;
 
   private batch: OnDemandDocument | null = null;
-  private values: Generator<OnDemandDocument, void, void> | null = null;
   private iterated = 0;
 
-  constructor(b: Uint8Array, o?: number, a?: boolean) {
-    super(b, o, a);
+  constructor(bytes: Uint8Array, offset?: number, isArray?: boolean) {
+    super(bytes, offset, isArray);
 
     if (this.isError) return;
 
@@ -175,20 +174,26 @@ export class CursorResponse extends MongoDBResponse {
   }
 
   shift(options?: BSONSerializeOptions): any {
+    if (this.iterated >= this.batchSize) {
+      return null;
+    }
+
+    const result = this.batch?.get(this.iterated, BSONType.object, true) ?? null;
     this.iterated += 1;
-    this.values ??= this.batch?.valuesAs(BSONType.object) ?? null;
-    const result = this.values?.next();
-    if (!result || result.done) return null;
+
+    if (result == null) {
+      throw new MongoUnexpectedServerResponseError('Cursor batch contains null values');
+    }
+
     if (options?.raw) {
-      return result.value.toBytes();
+      return result.toBytes();
     } else {
-      return result.value.toObject(options);
+      return result.toObject(options);
     }
   }
 
   clear() {
     this.iterated = this.batchSize;
-    this.values?.return();
   }
 
   pushMany() {

test/unit/cmap/wire_protocol/on_demand/document.test.ts

@@ -73,6 +73,7 @@ describe('class OnDemandDocument', () => {
 
   context('get()', () => {
     let document: OnDemandDocument;
+    let array: OnDemandDocument;
     const input = {
       int: 1,
       double: 1.2,
@@ -86,12 +87,27 @@ describe('class OnDemandDocument', () => {
       date: new Date(0),
       object: { a: 1 },
       array: [1, 2],
-      unsupportedType: /abc/
+      unsupportedType: /abc/,
+      [233]: 3
     };
 
     beforeEach(async function () {
       const bytes = BSON.serialize(input);
       document = new OnDemandDocument(bytes);
+      array = new OnDemandDocument(
+        BSON.serialize(Object.fromEntries(Object.values(input).entries())),
+        0,
+        true
+      );
+    });
+
+    it('supports access by number for arrays', () => {
+      expect(array.get(1, BSONType.int)).to.equal(1);
+    });
+
+    it('does not support access by number for objects', () => {
+      expect(document.get(233, BSONType.int)).to.be.null;
+      expect(document.get('233', BSONType.int)).to.equal(3);
     });
 
     it('returns null if the element does not exist', () => {
@@ -277,39 +293,4 @@ describe('class OnDemandDocument', () => {
       expect(document.getNumber('boolTrue')).to.equal(1);
     });
   });
-
-  context('*valuesAs()', () => {
-    let array: OnDemandDocument;
-    beforeEach(async function () {
-      const bytes = BSON.serialize(
-        Object.fromEntries(Array.from({ length: 10 }, () => 1).entries())
-      );
-      array = new OnDemandDocument(bytes, 0, true);
-    });
-
-    it('throws if document is not an array', () => {
-      const bytes = BSON.serialize(
-        Object.fromEntries(Array.from({ length: 10 }, () => 1).entries())
-      );
-      array = new OnDemandDocument(bytes, 0, false);
-      expect(() => array.valuesAs(BSONType.int).next()).to.throw();
-    });
-
-    it('returns a generator that yields values matching the as BSONType parameter', () => {
-      let didRun = false;
-      for (const item of array.valuesAs(BSONType.int)) {
-        didRun = true;
-        expect(item).to.equal(1);
-      }
-      expect(didRun).to.be.true;
-    });
-
-    it('caches the results of array', () => {
-      const generator = array.valuesAs(BSONType.int);
-      generator.next();
-      generator.next();
-      expect(array).to.have.nested.property('cache.0.value', 1);
-      expect(array).to.have.nested.property('cache.1.value', 1);
-    });
-  });
 });