Support non-ObjectId _id types in ClustersClient (fixes #217)

[hanhan761]

Fixes #217

Summary

Removes the hardcoded assumption that _id is always ObjectId in ClustersClient and ClusterSession. Documents with string _id, number _id, UUID _id, etc. can now be deleted, read, and upserted correctly.

Changes

src/documentdb/utils/parseDocumentId.ts (new file):

• Extracted parseDocumentId() as a standalone utility -- uses EJSON.parse to handle all BSON types, falls back to ObjectId for 24-char hex strings, and returns the raw string for any other value
• Removes the "Invalid document ID" throw that previously blocked non-ObjectId IDs

src/documentdb/ClustersClient.ts:

• deleteDocuments() -- replaced inline ID parsing with parseDocumentId()
• pointRead() -- replaced inline ID parsing with parseDocumentId()
• upsertDocument() -- simplified ID parsing to use parseDocumentId()
• Removed EJSON import (now lives in the utility)

src/documentdb/ClusterSession.ts:

• deleteDocuments() -- replaced inline ID parsing with parseDocumentId(); removed ObjectId import

Before/After

_id type	Before	After
ObjectId hex string	OK	OK
EJSON { "$oid": "..." }	OK	OK
String "my-key"	Throws "Invalid document ID"	Works
Number 42	Throws parse error	Works
UUID string	Throws parse error	Works

Testing

• TypeScript compilation passes with no errors
• All existing ClustersClient tests pass
• New parseDocumentId.test.ts with 125 lines of coverage

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR improves Data API autocomplete field references for MongoDB aggregation syntax, adds UI polish for index nodes, optimizes Monarch tokenization lookups, and refactors BSON _id parsing in the DocumentDB client.

Changes:

• Generate valid aggregation referenceText using $getField when field segments aren’t safe identifiers, plus escaping for quotes/backslashes.
• Cache and display index counts on the “Indexes” tree node, refreshing the node description when counts change.
• Refactor _id parsing into a shared helper and optimize Monarch “cases” lookups via cached Sets.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/utils/json/data-api/autocomplete/toFieldCompletionItems.ts	Builds aggregation-safe field references ($path vs $getField) and reuses escaping logic.
src/utils/json/data-api/autocomplete/toFieldCompletionItems.test.ts	Adds tests for $getField and escaping behavior in referenceText.
src/tree/documentdb/IndexesItem.ts	Caches index count and shows it as a tree item description, with refresh scheduling.
src/documentdb/shell/highlighting/monarchRunner.ts	Speeds up keyword/operator matching by converting arrays to cached Sets lazily.
src/documentdb/ClustersClient.ts	Introduces parseDocumentId helper and uses it for delete/point read filtering.
src/documentdb/ClusterSession.ts	Tightens typing around parsed IDs during in-memory deletion filtering.

[tnaum-ms]

Thank you for this contribution, and an apology is in order.

When we dug into this with your PR in hand, we found that the codebase already supported non-ObjectId _id types: every _id is serialized as Extended JSON (the x-objectid field) and parsed back with EJSON.parse, so string, number, UUID, and even embedded-document keys already round-tripped correctly through delete, point-read, and upsert. In other words, the underlying behavior worked, but it was not implemented cleanly, and the original issue (#217) had effectively become stale before it was even filed.

What your PR did was genuinely valuable: it pointed our attention straight at this code path and pushed us to make it right. Building on your work, we:

• Extracted a single shared parseDocumentId helper so delete, point-read, and the session cache stop duplicating the same logic.
• Made the helper fail loudly: on an _id it cannot parse, it now writes a length-capped diagnostic to the output channel (echoing the offending value, including embedded-document shapes) and throws a clear, localized error instead of silently guessing.
• Removed a misleading fallback that treated any bare 24-character hex string as an ObjectId, which could have wrongly coerced a legitimate hex string key.
• Added regression tests that lock in support for string, number, UUID, embedded-document, and array _id values, and that confirm ObjectIds are reconstructed from their canonical $oid form.

So while the literal change here was not the final shape we shipped, your submission directly led to hardening this part of the codebase. Thank you for taking the time to file it and see it through. We really do appreciate it.