fix(bson): ensure wrapped bson value is bson type null when null or undefined

marcj · marcj · commit 9af338e0a5ca · 2025-11-08T01:20:59.000+01:00
fixes #678
diff --git a/packages/bson/src/bson-serializer.ts b/packages/bson/src/bson-serializer.ts
@@ -166,16 +166,19 @@ function getSignedBinaryBigIntSize(value: bigint): number {
 
 export function getValueSize(value: any): number {
     if (value instanceof ValueWithBSONSerializer) {
+        if (value.value === undefined || value.value === null) {
+            return 0;
+        }
+
         if (isUUIDType(value.type)) {
             return 4 + 1 + 16;
         } else if (isMongoIdType(value.type)) {
             return 12;
         } else if (isBinaryBigIntType(value.type)) {
             const binaryBigInt = binaryBigIntAnnotation.getFirst(value.type)!;
             return binaryBigInt === BinaryBigIntType.unsigned ? getBinaryBigIntSize(value.value) : getSignedBinaryBigIntSize(value.value);
-        } else {
-            return getValueSize(value.value);
         }
+        return getValueSize(value.value);
     } else if ('boolean' === typeof value) {
         return 1;
     } else if ('string' === typeof value) {
@@ -553,6 +556,24 @@ export class Writer {
     }
 
     writeObjectId(value: string) {
+        if (!value) {
+            // set all to zero
+            this.buffer[this.offset + 0] = 0;
+            this.buffer[this.offset + 1] = 0;
+            this.buffer[this.offset + 2] = 0;
+            this.buffer[this.offset + 3] = 0;
+            this.buffer[this.offset + 4] = 0;
+            this.buffer[this.offset + 5] = 0;
+            this.buffer[this.offset + 6] = 0;
+            this.buffer[this.offset + 7] = 0;
+            this.buffer[this.offset + 8] = 0;
+            this.buffer[this.offset + 9] = 0;
+            this.buffer[this.offset + 10] = 0;
+            this.buffer[this.offset + 11] = 0;
+            this.offset += 12;
+            return;
+        }
+
         this.buffer[this.offset + 0] = hexToByte(value, 0);
         this.buffer[this.offset + 1] = hexToByte(value, 1);
         this.buffer[this.offset + 2] = hexToByte(value, 2);
@@ -570,25 +591,28 @@ export class Writer {
 
     write(value: any): void {
         if (value instanceof ValueWithBSONSerializer) {
-            if (value.value !== undefined && value.value !== null) {
-                if (isUUIDType(value.type)) {
-                    this.writeType(BSONType.BINARY);
-                    this.writeUUID(value.value);
-                    return;
-                } else if (isMongoIdType(value.type)) {
-                    this.writeType(BSONType.OID);
-                    this.writeObjectId(value.value);
-                    return;
-                } else if (isBinaryBigIntType(value.type)) {
-                    this.writeType(BSONType.BINARY);
-                    const binary = binaryBigIntAnnotation.getFirst(value.type)!;
-                    if (binary === BinaryBigIntType.signed) {
-                        this.writeSignedBigIntBinary(value.value);
-                    } else {
-                        this.writeBigIntBinary(value.value);
-                    }
-                    return;
+            if (value.value === undefined || value.value === null) {
+                this.writeType(BSONType.NULL);
+                return;
+            }
+
+            if (isUUIDType(value.type)) {
+                this.writeType(BSONType.BINARY);
+                this.writeUUID(value.value);
+                return;
+            } else if (isMongoIdType(value.type)) {
+                this.writeType(BSONType.OID);
+                this.writeObjectId(value.value);
+                return;
+            } else if (isBinaryBigIntType(value.type)) {
+                this.writeType(BSONType.BINARY);
+                const binary = binaryBigIntAnnotation.getFirst(value.type)!;
+                if (binary === BinaryBigIntType.signed) {
+                    this.writeSignedBigIntBinary(value.value);
+                } else {
+                    this.writeBigIntBinary(value.value);
                 }
+                return;
             }
             this.write(value.value);
         } else if ('boolean' === typeof value) {
diff --git a/packages/bson/tests/bson-serialize.spec.ts b/packages/bson/tests/bson-serialize.spec.ts
@@ -1,30 +1,6 @@
 import { expect, test } from '@jest/globals';
-import {
-    AutoBuffer,
-    getBSONSerializer,
-    getBSONSizer,
-    getValueSize,
-    hexToByte,
-    serializeBSONWithoutOptimiser,
-    uuidStringToByte,
-    wrapObjectId,
-    wrapUUID,
-    wrapValue,
-} from '../src/bson-serializer.js';
-import {
-    BinaryBigInt,
-    createReference,
-    Excluded,
-    hasCircularReference,
-    MongoId,
-    nodeBufferToArrayBuffer,
-    PrimaryKey,
-    Reference,
-    SignedBinaryBigInt,
-    typeOf,
-    uuid,
-    UUID,
-} from '@deepkit/type';
+import { AutoBuffer, getBSONSerializer, getBSONSizer, getValueSize, hexToByte, serializeBSONWithoutOptimiser, uuidStringToByte, wrapObjectId, wrapUUID, wrapValue } from '../src/bson-serializer.js';
+import { BinaryBigInt, createReference, Excluded, hasCircularReference, MongoId, nodeBufferToArrayBuffer, PrimaryKey, Reference, SignedBinaryBigInt, typeOf, uuid, UUID } from '@deepkit/type';
 import bson from 'bson';
 import { randomBytes } from 'crypto';
 import { BSON_BINARY_SUBTYPE_DEFAULT, BSONType } from '../src/utils.js';
@@ -1670,3 +1646,11 @@ test('string fallback from number', () => {
         expect(back2.v).toEqual(['1', '2']);
     }
 });
+
+test('optional MongoId in object', () => {
+    type T = { v: any };
+    const serialize = getBSONSerializer<T>();
+    const bson = serialize({ v: wrapObjectId(undefined as any) });
+    const back = getBSONDeserializer<T>()(bson);
+    expect(back.v).toBeUndefined();
+});
diff --git a/packages/mongo/src/mongo-serializer.ts b/packages/mongo/src/mongo-serializer.ts
@@ -9,19 +9,7 @@
  */
 
 import { BSONBinarySerializer, ValueWithBSONSerializer } from '@deepkit/bson';
-import {
-    ContainerAccessor,
-    EmptySerializer,
-    executeTemplates,
-    isBinaryBigIntType,
-    isMongoIdType,
-    isReferenceType,
-    isUUIDType,
-    ReflectionClass,
-    ReflectionKind,
-    TemplateState,
-    Type
-} from '@deepkit/type';
+import { ContainerAccessor, EmptySerializer, executeTemplates, isBinaryBigIntType, isMongoIdType, isReferenceType, isUUIDType, ReflectionClass, ReflectionKind, TemplateState, Type } from '@deepkit/type';
 
 /**
  * Serializer class from BSONBinarySerializer with a few adjustments to make sure
@@ -90,65 +78,3 @@ class MongoAnySerializer extends EmptySerializer {
 }
 
 export const mongoSerializer = new MongoAnySerializer();
-
-//
-// export const mongoSerializer = new Serializer('mongo');
-//
-// mongoSerializer.fromClass.register('string', compilerToString);
-// mongoSerializer.fromClass.register('number', compilerToNumber);
-//
-// mongoSerializer.fromClass.register('array', (property: PropertySchema, state: CompilerState) => {
-//     if (property.backReference || (property.parent && property.parent.backReference)) {
-//         //we don't serialize back references to mongodb
-//         state.template = '';
-//     } else {
-//         convertArray(property, state);
-//     }
-// });
-// mongoSerializer.toClass.register('array', (property: PropertySchema, state: CompilerState) => {
-//     if (property.backReference || (property.parent && property.parent.backReference)) {
-//         //we don't serialize back references to mongodb
-//         state.addSetter('undefined');
-//     } else {
-//         convertArray(property, state);
-//     }
-// });
-//
-// mongoSerializer.fromClass.register('class', (property: PropertySchema, state: CompilerState) => {
-//     //When property is a reference we store the actual primary (as foreign key) of the referenced instance instead of the actual instance.
-//     //This way we implemented basically relations in the database
-//     const classSchema = getClassSchema(property.resolveClassType!);
-//
-//     if (property.isReference) {
-//         const classType = state.setVariable('classType', property.resolveClassType);
-//         const primary = classSchema.getPrimaryField();
-//
-//         state.addCodeForSetter(`
-//             if (${state.accessor} instanceof ${classType}) {
-//                 ${getDataConverterJS(state.setter, `${state.accessor}.${primary.name}`, primary, state.serializerCompilers, state.compilerContext, state.jitStack)}
-//             } else {
-//                 //we treat the input as if the user gave the primary key directly
-//                 ${getDataConverterJS(state.setter, `${state.accessor}`, primary, state.serializerCompilers, state.compilerContext, state.jitStack)}
-//             }
-//             `
-//         );
-//     } else if (property.backReference || (property.parent && property.parent.backReference)) {
-//         //we don't serialize back references to mongodb
-//         state.template = '';
-//     } else {
-//         const classToX = state.setVariable('classToX', state.jitStack.getOrCreate(classSchema, () => getClassToXFunction(classSchema, state.serializerCompilers.serializer, state.jitStack)));
-//         state.addSetter(`${classToX}.fn(${state.accessor}, _options, _stack)`);
-//     }
-// });
-//
-// //this is necessary since we use in FindCommand `filter: t.any`, so uuid and objectId need to be a wrapper type so that @deepkit/bson serializes correctly
-// mongoSerializer.fromClass.register('uuid', (property: PropertySchema, state: CompilerState) => {
-//     state.setContext({ UUID });
-//     state.addSetter(`new UUID(${state.accessor})`);
-// });
-//
-// mongoSerializer.fromClass.register('objectId', (property: PropertySchema, state: CompilerState) => {
-//     state.setContext({ ObjectId });
-//
-//     state.addSetter(`new ObjectId(${state.accessor})`);
-// });
diff --git a/packages/mongo/tests/mongo.spec.ts b/packages/mongo/tests/mongo.spec.ts
@@ -1,26 +1,11 @@
 import { expect, jest, test } from '@jest/globals';
-import {
-    arrayBufferFrom,
-    AutoIncrement,
-    BackReference,
-    cast,
-    entity,
-    MongoId,
-    nodeBufferToArrayBuffer,
-    PrimaryKey,
-    Reference,
-    ReflectionClass,
-    ReflectionKind,
-    serialize,
-    Unique,
-    UUID,
-    uuid,
-} from '@deepkit/type';
+import { arrayBufferFrom, AutoIncrement, BackReference, cast, entity, MongoId, nodeBufferToArrayBuffer, PrimaryKey, Reference, ReflectionClass, ReflectionKind, serialize, Unique, UUID, uuid } from '@deepkit/type';
 import { Database, getInstanceStateFromItem, UniqueConstraintFailure } from '@deepkit/orm';
 import { SimpleModel, SuperSimple } from './entities.js';
 import { createDatabase } from './utils.js';
 import { MongoDatabaseAdapter } from '../src/adapter.js';
 import { MemoryLogger } from '@deepkit/logger';
+import { ObjectId } from '@deepkit/bson';
 
 Error.stackTraceLimit = 100;
 jest.setTimeout(10000);
@@ -838,3 +823,39 @@ test('allowDiskUse', async () => {
     }
     database.disconnect();
 });
+
+test('non-optional MongoId', async () => {
+    class Model {
+        _id: MongoId & PrimaryKey = '';
+
+        constructor(public teamId: MongoId) {
+
+        }
+    }
+
+    const database = await createDatabase('non-optional-mongo-id');
+
+    const model1 = new Model(ObjectId.generate());
+    await database.persist(model1);
+
+    const items1 = await database.query(Model)
+        .filter({ teamId: undefined })
+        .find();
+    expect(items1.length).toBe(0);
+
+    const items2 = await database.query(Model)
+        .filter({ teamId: null as any })
+        .find();
+    expect(items2.length).toBe(0);
+
+    const items3 = await database.query(Model)
+        .filter({ teamId: '' })
+        .find();
+    expect(items3.length).toBe(0);
+
+    const items4 = await database.query(Model)
+        .filter({ teamId: model1.teamId })
+        .find();
+    expect(items4.length).toBe(1);
+    expect(items4[0]).toBeInstanceOf(Model);
+});
