fix: address comments

Author: durran

src/cmap/commands.ts

@@ -30,6 +30,8 @@ const QUERY_FAILURE = 2;
 const SHARD_CONFIG_STALE = 4;
 const AWAIT_CAPABLE = 8;
 
+const encodeUTF8Into = BSON.BSON.onDemand.ByteUtils.encodeUTF8Into;
+
 /** @internal */
 export type WriteProtocolMessageType = OpQueryRequest | OpMsgRequest;
 
@@ -413,11 +415,9 @@ export interface OpMsgOptions {
 
 /** @internal */
 export class DocumentSequence {
-  field: string;
   documents: Document[];
 
-  constructor(field: string, documents: Document[]) {
-    this.field = field;
+  constructor(documents: Document[]) {
     this.documents = documents;
   }
 }
@@ -506,7 +506,7 @@ export class OpMsgRequest {
    */
   makeSections(buffers: Uint8Array[], document: Document): number {
     const sequencesBuffer = this.extractDocumentSequences(document);
-    const payloadTypeBuffer = Buffer.alloc(1);
+    const payloadTypeBuffer = Buffer.allocUnsafe(1);
     payloadTypeBuffer[0] = 0;
 
     const documentBuffer = this.serializeBson(document);
@@ -530,23 +530,20 @@ export class OpMsgRequest {
     for (const [key, value] of Object.entries(document)) {
       if (value instanceof DocumentSequence) {
         // Document sequences starts with type 1 at the first byte.
-        const payloadTypeBuffer = Buffer.alloc(1);
-        payloadTypeBuffer[0] = 1;
-        chunks.push(payloadTypeBuffer);
-        // Second part of the sequence is the length;
-        const lengthBuffer = Buffer.alloc(4);
-        chunks.push(lengthBuffer);
-        // Third part is the field name.
-        const fieldBuffer = Buffer.from(key);
-        chunks.push(fieldBuffer);
+        const buffer = Buffer.allocUnsafe(1 + 4 + key.length);
+        buffer[0] = 1;
+        // Third part is the field name at offset 5.
+        encodeUTF8Into(buffer, key, 5);
+        chunks.push(buffer);
         // Fourth part are the documents' bytes.
         let docsLength = 0;
         for (const doc of value.documents) {
           const docBson = this.serializeBson(doc);
           docsLength += docBson.length;
           chunks.push(docBson);
         }
-        lengthBuffer.writeInt32LE(fieldBuffer.length + docsLength);
+        // Second part of the sequence is the length at offset 1;
+        buffer.writeInt32LE(key.length + docsLength, 1);
         // Why are we removing the field from the command? This is because it needs to be
         // removed in the OP_MSG request first section, and DocumentSequence is not a
         // BSON type and is specific to the MongoDB wire protocol so there's nothing
@@ -560,6 +557,8 @@ export class OpMsgRequest {
     if (chunks.length > 0) {
       return Buffer.concat(chunks);
     }
+    // If we have no document sequences we return an empty buffer for nothing to add
+    // to the payload.
     return Buffer.alloc(0);
   }
 

test/unit/cmap/commands.test.ts

@@ -1,3 +1,4 @@
+import * as BSON from 'bson';
 import { expect } from 'chai';
 
 import { DocumentSequence, OpMsgRequest, OpReply } from '../../mongodb';
@@ -14,11 +15,21 @@ describe('commands', function () {
         context('when there is one document sequence', function () {
           const command = {
             test: 1,
-            field: new DocumentSequence('test', [{ test: 1 }])
+            field: new DocumentSequence([{ test: 1 }])
           };
           const msg = new OpMsgRequest('admin', command, {});
           const buffers = msg.toBin();
 
+          it('keeps the first section as type 0', function () {
+            // The type byte for the first section is at index 1.
+            expect(buffers[1][0]).to.equal(0);
+          });
+
+          it('does not serialize the document sequences in the first section', function () {
+            // Buffer at index 2 is the type 0 section - one document.
+            expect(BSON.deserialize(buffers[2])).to.deep.equal({ test: 1, $db: 'admin' });
+          });
+
           it('removes the document sequence fields from the command', function () {
             expect(command).to.not.haveOwnProperty('field');
           });
@@ -42,12 +53,22 @@ describe('commands', function () {
         context('when there are multiple document sequences', function () {
           const command = {
             test: 1,
-            fieldOne: new DocumentSequence('test', [{ test: 1 }]),
-            fieldTwo: new DocumentSequence('test', [{ test: 1 }])
+            fieldOne: new DocumentSequence([{ test: 1 }]),
+            fieldTwo: new DocumentSequence([{ test: 1 }])
           };
           const msg = new OpMsgRequest('admin', command, {});
           const buffers = msg.toBin();
 
+          it('keeps the first section as type 0', function () {
+            // The type byte for the first section is at index 1.
+            expect(buffers[1][0]).to.equal(0);
+          });
+
+          it('does not serialize the document sequences in the first section', function () {
+            // Buffer at index 2 is the type 0 section - one document.
+            expect(BSON.deserialize(buffers[2])).to.deep.equal({ test: 1, $db: 'admin' });
+          });
+
           it('removes the document sequence fields from the command', function () {
             expect(command).to.not.haveOwnProperty('fieldOne');
             expect(command).to.not.haveOwnProperty('fieldTwo');