Skip to content

STREAMS-1982: Add singleton compat for MinKey #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

STREAMS-1982: Add singleton compat for MinKey #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c1a582c
STREAMS-1982: Add singleton compat for MinKey
tcannon91 Nov 12, 2025
1c01dab
add zed editor to gitignore
tcannon91 Nov 18, 2025
b9e5a85
remove zed from gitignore
tcannon91 Nov 18, 2025
df43365
Add test serializing as bson
tcannon91 Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions snippets/mongocompat/mongotypes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,41 @@ if (typeof (gc) == "undefined") {
};
}

// MinKey
if (typeof (MinKey) != "undefined") {
const OriginalMinKey = MinKey;
MinKey = function () {
if (MinKey.prototype.__instance__ === undefined) {
MinKey.prototype.__instance__ = new OriginalMinKey();
}

return MinKey.prototype.__instance__;
};

MinKey.prototype = OriginalMinKey.prototype;

for (const key of Object.getOwnPropertyNames(OriginalMinKey)) {
// Skip prototype, length, name(function internals)
if (key !== 'prototype' && key !== 'length' && key !== 'name') {
MinKey[key] = OriginalMinKey[key];
}
}

MinKey.prototype.toJSON = function () {
return this.tojson();
};

MinKey.prototype.tojson = function () {
return "{ \"$minKey\" : 1 }";
};

MinKey.prototype.toString = function () {
return "[object Function]";
};
} else {
print("warning: no MinKey class");
}

// Free Functions
tojsononeline = function(x) {
return tojson(x, " ", true);
Expand Down
20 changes: 19 additions & 1 deletion snippets/mongocompat/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
load(__dirname + '/index.js');

const bson = require('bson');

assert.strictEqual(ObjectId('0123456789abcdef01234567').tojson(), 'ObjectId("0123456789abcdef01234567")');

assert.strictEqual(BinData(4, 'abcdefgh').toString(), 'BinData(4,"abcdefgh")');
Expand Down Expand Up @@ -87,7 +89,7 @@ const tsFromStr = Timestamp.fromString('ff', 16);
assert.strictEqual(tsFromStr.i, 255);
assert.strictEqual(tsFromStr.t, 0);
assert.strictEqual(Timestamp.MAX_VALUE._bsontype, 'Long');
assert.strictEqual(Timestamp.MAX_VALUE, Long.MAX_UNSIGNED_VALUE);
assert.strictEqual(Timestamp.MAX_VALUE, Long.MAX_UNSIGNED_VALUE);

const id = ObjectId('68ffa28b77bba38c9ddcf376');
const dbRef = DBRef('testColl', id, 'testDb');
Expand Down Expand Up @@ -161,3 +163,19 @@ assert(sortedArrayJson.indexOf('"a"') < sortedArrayJson.indexOf('"z"'), 'Array.t
assert(sortedArrayJson.indexOf('"b"') < sortedArrayJson.indexOf('"y"'), 'Array.tojson with sortedKeys=true should sort object keys in array elements');
assert(unsortedArrayJson.indexOf('"z"') < unsortedArrayJson.indexOf('"a"'), 'Array.tojson with sortedKeys=false should not sort keys');
assert(defaultArrayJson.indexOf('"z"') < defaultArrayJson.indexOf('"a"'), 'Array.tojson without sortedKeys should not sort keys');

// Test MinKey
const minKey = new MinKey();
assert(minKey instanceof MinKey, "minKey should be an instance of MinKey");
assert.strictEqual(minKey.tojson(), '{ "$minKey" : 1 }', "minKey should serialize correctly");
assert.strictEqual(minKey.toString(), "[object Function]", "minKey toString should work");
assert.strictEqual(minKey.toJSON(), '{ "$minKey" : 1 }', "minKey toJSON should work");
Copy link
Collaborator

@addaleax addaleax Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal, but passing a static string as a message for assert.strictEqual() and friends is an antipattern since it will hide information about the actual comparison results – if you want to add this type of information, add a comment


// Test that multiple references return the same instance
const anotherMinKeyRef = new MinKey();
assert.strictEqual(minKey, anotherMinKeyRef, "minKey should be a singleton - v1");
assert.strictEqual(MinKey(), MinKey(), "minKey should be a singleton - v2");
Copy link
Collaborator

@addaleax addaleax Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to make sure that MinKey still serializes properly to BSON/EJSON without calling it

Copy link
Collaborator Author

@tcannon91 tcannon91 Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify what you mean by "without calling it"? If you have a reference test I can look at, that would be helpful

Copy link
Collaborator Author

@tcannon91 tcannon91 Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have something like this, but I am not certain if it accomplishes what you are looking for 😓

const serializedBsonMinKey = bson.serialize({ key: minKey });
const deserializedBsonMinKey = bson.deserialize(serializedBsonMinKey);
assert.deepEqual(minKey, deserializedBsonMinKey.key, "should be equal after bson serialization");

const serializedEJSONMinKey = EJSON.stringify({ key: minKey });
const deserializedEJSONMinKey = EJSON.parse(serializedEJSONMinKey);
assert.deepEqual(minKey, deserializedEJSONMinKey.key, "should be equal after ejson serialization");

Copy link
Collaborator

@addaleax addaleax Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify what you mean by "without calling it"?

One oddity of MongoDB JS environments like the legacy shell and $function is that

db.coll.insertOne({ a: MinKey, b: MaxKey })

and

db.coll.insertOne({ a: MinKey(), b: MaxKey() })

are equivalent – and it's not immediately true whether that's still the case after this patch or not; and even if it's the case already, I'd say we should still add a test to make sure it keeps being true regardless of future modifications to this code

I have something like this, but I am not certain if it accomplishes what you are looking for

Right, doing that with those values should be fine 🙂

(Aside from the fact that you almost certainly want assert.deepStrictEqual instead of assert.deepEqual)

Copy link
Collaborator Author

@tcannon91 tcannon91 Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know what you think of the changes. I adjusted the test from what I originally included in the thread.

  • I did a different comparison that I think better captures the nuance of the example you shared
  • I removed the ejson stringify/parse because I couldn't get it working (😖) but also I don't think it was important related to your shared example. The bson parsing seemed to be what mattered.

Copy link
Collaborator

@addaleax addaleax Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EJSON would be kind of nice to test since it's built into mongosh (unlike direct bson serialization/deserialization) but that's fine, yeah


const serializedBsonMinKey = bson.serialize({ key1: MinKey, key2: MinKey() });
const deserializedBsonMinKey = bson.deserialize(serializedBsonMinKey);
assert.deepStrictEqual(deserializedBsonMinKey.key1, deserializedBsonMinKey.key2, "should be equal after bson serialization");