Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 5 additions & 2 deletions packages/service-provider-core/src/admin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type {
AutoEncryptionOptions,
Collection,
} from './all-transport-types';
import type { ConnectionExtraInfo } from './index';
import type { ConnectionExtraInfo, ServiceProvider } from './index';
import type { ReplPlatform } from './platform';
import type {
AWSEncryptionKeyOptions,
Expand Down Expand Up @@ -90,7 +90,10 @@ export default interface Admin {
* @param uri
* @param options
*/
getNewConnection(uri: string, options: MongoClientOptions): Promise<any>; // returns the ServiceProvider instance
getNewConnection(
uri: string,
options: MongoClientOptions
): Promise<ServiceProvider>;

/**
* Return the URI for the current connection, if this ServiceProvider is connected.
Expand Down
7 changes: 7 additions & 0 deletions packages/service-provider-core/src/cursors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,10 @@ export interface ServiceProviderChangeStream<TSchema = Document>
next(): Promise<TSchema>;
readonly resumeToken: ResumeToken;
}

export type ServiceProviderAnyCursor<TSchema = Document> =
| ServiceProviderAggregationCursor<TSchema>
| ServiceProviderFindCursor<TSchema>
| ServiceProviderRunCommandCursor<TSchema>
| ServiceProviderFindCursor<TSchema>
| ServiceProviderChangeStream<TSchema>;
1 change: 1 addition & 0 deletions packages/service-provider-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export {
ServiceProviderFindCursor,
ServiceProviderRunCommandCursor,
ServiceProviderChangeStream,
ServiceProviderAnyCursor,
} from './cursors';

export {
Expand Down
257 changes: 257 additions & 0 deletions packages/shell-api/src/deep-inspect-service-provider-wrapper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,257 @@
import type {
ServiceProvider,
ServiceProviderAbstractCursor,
} from '@mongosh/service-provider-core';
import { ServiceProviderCore } from '@mongosh/service-provider-core';
import type { InspectOptions } from 'util';
import { inspect } from 'util';
import type { Document } from '@mongosh/service-provider-core';

export class DeepInspectServiceProviderWrapper
extends ServiceProviderCore
implements ServiceProvider
{
_sp: ServiceProvider;

constructor(sp: ServiceProvider) {
super(sp.bsonLibrary);
this._sp = sp;

for (const prop of Object.keys(this)) {
if (typeof (this as any)[prop] === 'function' && !(prop in sp)) {
(this as any)[prop] = undefined;
}
}
}

aggregate = cursorMethod('aggregate');
aggregateDb = cursorMethod('aggregateDb');
count = forwardedMethod('count');
estimatedDocumentCount = forwardedMethod('estimatedDocumentCount');
countDocuments = forwardedMethod('countDocuments');
distinct = bsonMethod('distinct');
find = cursorMethod('find');
findOneAndDelete = bsonMethod('findOneAndDelete');
findOneAndReplace = bsonMethod('findOneAndReplace');
findOneAndUpdate = bsonMethod('findOneAndUpdate');
getTopologyDescription = forwardedMethod('getTopologyDescription');
getIndexes = bsonMethod('getIndexes');
listCollections = bsonMethod('listCollections');
readPreferenceFromOptions = forwardedMethod('readPreferenceFromOptions');
// TODO: this should be a cursor method, but the types are incompatible
watch = forwardedMethod('watch');
getSearchIndexes = bsonMethod('getSearchIndexes');
runCommand = bsonMethod('runCommand');
runCommandWithCheck = bsonMethod('runCommandWithCheck');
runCursorCommand = cursorMethod('runCursorCommand');
dropDatabase = bsonMethod('dropDatabase');
dropCollection = forwardedMethod('dropCollection');
bulkWrite = bsonMethod('bulkWrite');
clientBulkWrite = bsonMethod('clientBulkWrite');
deleteMany = bsonMethod('deleteMany');
updateMany = bsonMethod('updateMany');
updateOne = bsonMethod('updateOne');
deleteOne = bsonMethod('deleteOne');
createIndexes = bsonMethod('createIndexes');
insertMany = bsonMethod('insertMany');
insertOne = bsonMethod('insertOne');
replaceOne = bsonMethod('replaceOne');
initializeBulkOp = forwardedMethod('initializeBulkOp'); // you cannot extend the return value here
createSearchIndexes = forwardedMethod('createSearchIndexes');
close = forwardedMethod('close');
suspend = forwardedMethod('suspend');
renameCollection = forwardedMethod('renameCollection');
dropSearchIndex = forwardedMethod('dropSearchIndex');
updateSearchIndex = forwardedMethod('updateSearchIndex');
listDatabases = bsonMethod('listDatabases');
authenticate = forwardedMethod('authenticate');
createCollection = forwardedMethod('createCollection');
getReadPreference = forwardedMethod('getReadPreference');
getReadConcern = forwardedMethod('getReadConcern');
getWriteConcern = forwardedMethod('getWriteConcern');

get platform() {
return this._sp.platform;
}
get initialDb() {
return this._sp.initialDb;
}

getURI = forwardedMethod('getURI');
getConnectionInfo = forwardedMethod('getConnectionInfo');
resetConnectionOptions = forwardedMethod('resetConnectionOptions');
startSession = forwardedMethod('startSession');
getRawClient = forwardedMethod('getRawClient');
createClientEncryption = forwardedMethod('createClientEncryption');
getFleOptions = forwardedMethod('getFleOptions');
createEncryptedCollection = forwardedMethod('createEncryptedCollection');

async getNewConnection(
...args: Parameters<ServiceProvider['getNewConnection']>
): Promise<ServiceProvider> {
const sp = await this._sp.getNewConnection(...args);
return new DeepInspectServiceProviderWrapper(sp as ServiceProvider);
}
}

type PickMethodsByReturnType<T, R> = {
[k in keyof T as NonNullable<T[k]> extends (...args: any[]) => R
? k
: never]: T[k];
};

function cursorMethod<
K extends keyof PickMethodsByReturnType<
ServiceProvider,
ServiceProviderAbstractCursor
>
>(
key: K
): (
...args: Parameters<Required<ServiceProvider>[K]>
) => ReturnType<Required<ServiceProvider>[K]> {
return function (
this: DeepInspectServiceProviderWrapper,
...args: Parameters<ServiceProvider[K]>
): ReturnType<ServiceProvider[K]> {
// The problem here is that ReturnType<ServiceProvider[K]> results in
// ServiceProviderAnyCursor which includes ServiceProviderChangeStream which
// doesn't have readBufferedDocuments or toArray. We can try cast things to
// ServiceProviderAbstractCursor, but then that's not assignable to
// ServiceProviderAnyCursor. And that's why there's so much casting below.
const cursor = (this._sp[key] as any)(...args) as any;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just had the thought that one obvious way around this is to just have cursorMethod() for regular cursors and changeCursorMethod() for change stream cursors. And just replace ReturnType<ServiceProvider[K]> with something a lot more specific. Then we can probably have much better type safety.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, although I think either way is fine here


cursor.next = cursorNext(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd consider returning a wrapped cursor instead of modifying the existing one, if that's possible – like the wrapping for the ServiceProvider itself, this would make sure that we actually wrap all methods that we should and aren't forgetting about any

cursor.next.bind(cursor) as () => Promise<Document | null>
);
cursor.tryNext = cursorTryNext(
cursor.tryNext.bind(cursor) as () => Promise<Document | null>
);

if (cursor.readBufferedDocuments) {
cursor.readBufferedDocuments = cursorReadBufferedDocuments(
cursor.readBufferedDocuments.bind(cursor) as (
number?: number
) => Document[]
);
}
if (cursor.toArray) {
cursor.toArray = cursorToArray(
cursor.toArray.bind(cursor) as () => Promise<Document[]>
);
}

return cursor;
};
}

const customInspectSymbol = Symbol.for('nodejs.util.inspect.custom');

function cursorNext(
original: () => Promise<Document | null>
Copy link
Contributor Author

@lerouxb lerouxb Nov 20, 2025

Choose a reason for hiding this comment

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

I can probably use some typescript to automatically pick this type off abstract cursor. Something like that. Same for the other cursor methods below.

): () => Promise<Document | null> {
return async function (): Promise<Document | null> {
const result = await original();
if (result) {
replaceWithCustomInspect(result);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just suddenly had the realisation that this is a terrible function name. addCustomInspect() rather?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it's that bad, but yes, addCustomInspect() sounds good 🙂

}
return result;
};
}

const cursorTryNext = cursorNext;

function cursorReadBufferedDocuments(
original: (number?: number) => Document[]
): (number?: number) => Document[] {
return function (number?: number): Document[] {
const results = original(number);

replaceWithCustomInspect(results);

return results;
};
}

function cursorToArray(
original: () => Promise<Document[]>
): () => Promise<Document[]> {
return async function (): Promise<Document[]> {
const results = await original();

replaceWithCustomInspect(results);

return results;
};
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

^ All of these methods are essentially the same, no?


function bsonMethod<
K extends keyof PickMethodsByReturnType<ServiceProvider, Promise<any>>
>(
key: K
): (
...args: Parameters<Required<ServiceProvider>[K]>
) => ReturnType<Required<ServiceProvider>[K]> {
return async function (
this: DeepInspectServiceProviderWrapper,
...args: Parameters<Required<ServiceProvider>[K]>
): // eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore The returntype already contains a promise
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A way around all this is to just make one function similar to bsonMethod() for each unique return type, kinda like what I did for the cursor methods.

ReturnType<Required<ServiceProvider>[K]> {
const result = await (this._sp[key] as any)(...args);
replaceWithCustomInspect(result);
return result;
};
}

function forwardedMethod<
K extends keyof PickMethodsByReturnType<ServiceProvider, any>
>(
key: K
): (
...args: Parameters<Required<ServiceProvider>[K]>
) => ReturnType<Required<ServiceProvider>[K]> {
return function (
this: DeepInspectServiceProviderWrapper,
...args: Parameters<Required<ServiceProvider>[K]>
): ReturnType<Required<ServiceProvider>[K]> {
// not wrapping the result at all because forwardedMethod() is for simple
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made the decision here to leave everything that's not definitely a bson document or array alone. Make sense?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes! 🙂

// values only
return (this._sp[key] as any)(...args);
};
}

function customDocumentInspect(
this: Document,
depth: number,
inspectOptions: InspectOptions
) {
const newInspectOptions = {
...inspectOptions,
depth: Infinity,
maxArrayLength: Infinity,
maxStringLength: Infinity,
};

// reuse the standard inpect logic for an object without causing infinite
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feels very hacky, but I haven't thought of a better way to do it yet. I'm trying to avoid having to write my own function that will format an array or object exactly the way util.inspect() already does.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We know it's BSON, so it should be fine to use a shallow copy of the document/array, right?

// recursion
const inspectBackup = (this as any)[customInspectSymbol];
delete (this as any)[customInspectSymbol];
const result = inspect(this, newInspectOptions);
(this as any)[customInspectSymbol] = inspectBackup;
return result;
}

function replaceWithCustomInspect(obj: any) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something obvious to note here: I'm mutating the obj in place rather than creating new objects. I did it that way for simplicity and because I'm worried about the impact on performance making a gazillion new objects, but admittedly I haven't profiled that yet. Let me know if you think there's a better way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, this should be fine 👍

if (Array.isArray(obj)) {
(obj as any)[customInspectSymbol] = customDocumentInspect;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
(obj as any)[customInspectSymbol] = customDocumentInspect;
(obj as any)[customInspectSymbol] ??= customDocumentInspect;

maybe unnecessarily defensive, but if something already has a custom inspect function for some reason, we can probably leave that in place

for (const item of obj) {
replaceWithCustomInspect(item);
}
} else if (obj && typeof obj === 'object' && obj !== null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here, again, I made the decision to only touch arrays and documents/sub-documents and leave simple values alone. BUT this currently also installs our inspect function on BSON types like long, decimal, etc. Which we probably don't want?

What's the best way to do that? Look for obj._bsontype?

Copy link
Contributor Author

@lerouxb lerouxb Nov 20, 2025

Choose a reason for hiding this comment

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

Ok for some reason this code doesn't mess up BSON object formatting 🤔 I suppose the prototype still has the BSONValue one? https://github.com/mongodb/js-bson/blob/main/src/bson_value.ts#L38

Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the best way to do that? Look for obj._bsontype?

We're pretty close to adopting BSON 7.x, where we have obj[bsonType] as a more reliable symbol property (but yeah, that's the best way to check for existing BSON objects)

obj[customInspectSymbol] = customDocumentInspect;
for (const value of Object.values(obj)) {
replaceWithCustomInspect(value);
}
}
}
9 changes: 6 additions & 3 deletions packages/shell-api/src/shell-instance-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import type { AutocompletionContext } from '@mongodb-js/mongodb-ts-autocomplete'
import type { JSONSchema } from 'mongodb-schema';
import { analyzeDocuments } from 'mongodb-schema';
import type { BaseCursor } from './abstract-cursor';
import { DeepInspectServiceProviderWrapper } from './deep-inspect-service-provider-wrapper';

/**
* The subset of CLI options that is relevant for the shell API's behavior itself.
Expand Down Expand Up @@ -203,7 +204,9 @@ export class ShellInstanceState {
cliOptions: ShellCliOptions = {},
bsonLibrary: BSONLibrary = initialServiceProvider.bsonLibrary
) {
this.initialServiceProvider = initialServiceProvider;
this.initialServiceProvider = new DeepInspectServiceProviderWrapper(
initialServiceProvider
);
this.bsonLibrary = bsonLibrary;
this.messageBus = messageBus;
this.shellApi = new ShellApi(this);
Expand All @@ -220,11 +223,11 @@ export class ShellInstanceState {
undefined,
undefined,
undefined,
initialServiceProvider
this.initialServiceProvider
Copy link
Contributor Author

Choose a reason for hiding this comment

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

btw it is VERY easy to accidentally pass initialServiceProvider (ie. the unwrapped value) to something in place of this.initialServiceProvider. Ask me how I know..

);
this.mongos.push(mongo);
this.currentDb = mongo.getDB(
initialServiceProvider.initialDb || DEFAULT_DB
this.initialServiceProvider.initialDb || DEFAULT_DB
);
} else {
this.currentDb = new NoDatabase() as DatabaseWithSchema;
Expand Down
Loading