diff --git a/packages/mongodb-schema/src/schema-analyzer.ts b/packages/mongodb-schema/src/schema-analyzer.ts index e8b373d20..3c3c60486 100644 --- a/packages/mongodb-schema/src/schema-analyzer.ts +++ b/packages/mongodb-schema/src/schema-analyzer.ts @@ -256,7 +256,10 @@ function fieldComparator( return 1; } // Otherwise sort case-insensitively. - return aName.toLowerCase() < bName.toLowerCase() ? -1 : 1; + const aLower = aName.toLowerCase(); + const bLower = bName.toLowerCase(); + + return aLower < bLower ? -1 : aLower > bLower ? 1 : 0; } /** @@ -603,7 +606,7 @@ export class SchemaAnalyzer { allowAbortDuringAnalysis(): Promise { // Allow aborting the analysis. if (this.fieldAndTypeAnalysisCounter++ % ALLOW_ABORT_INTERVAL_COUNT === 0) { - void allowAbort(); + return allowAbort(this.options.signal); } return Promise.resolve(); } diff --git a/packages/mongodb-schema/src/schema-converters/internal-to-mongodb.ts b/packages/mongodb-schema/src/schema-converters/internal-to-mongodb.ts index 9ff1dd837..3e2f7be04 100644 --- a/packages/mongodb-schema/src/schema-converters/internal-to-mongodb.ts +++ b/packages/mongodb-schema/src/schema-converters/internal-to-mongodb.ts @@ -59,7 +59,7 @@ async function parseType( }; if (type.bsonType === 'Array') { - const items = await parseTypes((type as ArraySchemaType).types); + const items = await parseTypes((type as ArraySchemaType).types, signal); // Don't include empty bson type arrays (it's invalid). if (!items.bsonType || items.bsonType?.length > 0) { schema.items = items; diff --git a/packages/mongodb-schema/src/to-typescript.spec.ts b/packages/mongodb-schema/src/to-typescript.spec.ts index 6b85e83f6..8b091d618 100644 --- a/packages/mongodb-schema/src/to-typescript.spec.ts +++ b/packages/mongodb-schema/src/to-typescript.spec.ts @@ -22,6 +22,7 @@ import { } from 'bson'; import { inspect } from 'util'; +import * as ts from 'typescript'; function convertAndCompare(schema: MongoDBJSONSchema, expected: string) { const code = toTypescriptTypeDefinition(schema); @@ -42,6 +43,21 @@ function convertAndCompare(schema: MongoDBJSONSchema, expected: string) { throw err; } + + // Comparing strings cannot tell us whether the generated code is actually + // valid TypeScript, so parse it as well. This is a syntax-only check, which + // means unresolved names like `bson.Double` are not reported. + const { diagnostics } = ts.transpileModule(`type Generated = ${code};`, { + reportDiagnostics: true, + compilerOptions: {}, + }); + assert.deepEqual( + (diagnostics ?? []).map((diagnostic) => + ts.flattenDiagnosticMessageText(diagnostic.messageText, ' '), + ), + [], + 'generated type definition is not valid TypeScript', + ); } describe('toTypescriptTypeDefinition', function () { @@ -104,7 +120,7 @@ describe('toTypescriptTypeDefinition', function () { schema, `{ _id?: bson.ObjectId; - array?: bson.Double | number)[]; + array?: (bson.Double | number)[]; binaries?: { binaryOld?: bson.Binary; compressedTimeSeries?: bson.Binary; @@ -275,6 +291,42 @@ describe('toTypescriptTypeDefinition', function () { }; }; b?: any[]; +}`, + ); + }); + + it('quotes field names that are not valid identifiers', async function () { + const docs = [ + { + // Field names the analyzer already supports elsewhere, see + // test/nested-document-path.test.ts and test/basic.test.ts. + 'bar.with.dot': 'foo', + 'name[]': 'Annabeth Frankie', + 'has space': 1, + '2leading': 'foo', + 'quote"inside': 'foo', + valid_one: 'foo', + nested: { + 'a.b': [1, 2, 3], + }, + }, + ]; + + const analyzedDocuments = await analyzeDocuments(docs); + const schema = await analyzedDocuments.getMongoDBJsonSchema(); + + convertAndCompare( + schema, + `{ + "2leading"?: string; + "bar.with.dot"?: string; + "has space"?: bson.Double | number; + "name[]"?: string; + nested?: { + "a.b"?: (bson.Double | number)[]; + }; + "quote\\"inside"?: string; + valid_one?: string; }`, ); }); diff --git a/packages/mongodb-schema/src/to-typescript.ts b/packages/mongodb-schema/src/to-typescript.ts index 7b2c181e6..f28c35328 100644 --- a/packages/mongodb-schema/src/to-typescript.ts +++ b/packages/mongodb-schema/src/to-typescript.ts @@ -105,7 +105,11 @@ function arrayType(types: string[]) { if (types.length === 1) { return `${types[0]}[]`; } - return `${types.join(' | ')})[]`; + return `(${types.join(' | ')})[]`; +} + +function propertyKey(name: string): string { + return /^[A-Za-z_$][A-Za-z0-9_$]*$/.test(name) ? name : JSON.stringify(name); } function toTypescriptType( @@ -117,20 +121,22 @@ function toTypescriptType( switch (getBSONType(schema)) { case 'array': assertIsDefined(schema.items, 'schema.items must be defined'); - return `${indentSpaces(indent)}${propertyName}?: ${arrayType([ - ...uniqueTypes(schema.items), - ])}`; + return `${indentSpaces(indent)}${propertyKey( + propertyName, + )}?: ${arrayType([...uniqueTypes(schema.items)])}`; case 'object': assertIsDefined( schema.properties, 'schema.properties must be defined', ); - return `${indentSpaces(indent)}${propertyName}?: ${toTypescriptType( + return `${indentSpaces(indent)}${propertyKey( + propertyName, + )}?: ${toTypescriptType( schema.properties as Record, indent + 1, )}`; default: - return `${indentSpaces(indent)}${propertyName}?: ${[ + return `${indentSpaces(indent)}${propertyKey(propertyName)}?: ${[ ...uniqueTypes(schema), ].join(' | ')}`; } diff --git a/packages/mongodb-schema/test/bloated.test.ts b/packages/mongodb-schema/test/bloated.test.ts index eeec6d347..918088160 100644 --- a/packages/mongodb-schema/test/bloated.test.ts +++ b/packages/mongodb-schema/test/bloated.test.ts @@ -2,6 +2,7 @@ import assert from 'assert'; import { Binary, Code } from 'bson'; import type { PrimitiveSchemaType } from '../src/schema-analyzer'; +import { SchemaAnalyzer } from '../src/schema-analyzer'; import getSchema from '../src'; function generateRandomString(length: number) { @@ -139,4 +140,45 @@ describe('bloated documents', function () { } }); }); + + describe('abort signal', function () { + // `analyzeDoc` is exercised directly here: `getCompletedSchemaAnalyzer` + // checks the signal in between documents, which would hide whether the + // signal is honoured *during* the analysis of a single document. + const document = { + field1: { + field2: 'abc', + }, + field3: 'bca', + }; + + it('aborts during the analysis of a document with the signal reason', async function () { + const controller = new AbortController(); + const reason = new Error('Analysis no longer needed'); + controller.abort(reason); + + const analyzer = new SchemaAnalyzer({ signal: controller.signal }); + await assert.rejects(analyzer.analyzeDoc(document), reason); + }); + + it("aborts during the analysis of a document with the signal's default reason", async function () { + const controller = new AbortController(); + controller.abort(); + + const analyzer = new SchemaAnalyzer({ signal: controller.signal }); + await assert.rejects(analyzer.analyzeDoc(document), { + name: 'AbortError', + }); + }); + + it('does not abort while the signal is not aborted', async function () { + const controller = new AbortController(); + + const analyzer = new SchemaAnalyzer({ signal: controller.signal }); + await analyzer.analyzeDoc(document); + + const fieldNames = analyzer.getResult().fields.map((v) => v.name); + assert.deepStrictEqual(fieldNames, ['field1', 'field3']); + }); + }); }); diff --git a/packages/mongodb-schema/test/field-order.test.ts b/packages/mongodb-schema/test/field-order.test.ts index 538438d9f..ed79a6ab4 100644 --- a/packages/mongodb-schema/test/field-order.test.ts +++ b/packages/mongodb-schema/test/field-order.test.ts @@ -34,4 +34,19 @@ describe('order of fields', function () { ['a', 'b', 'Ca', 'cb', 'cC'], ); }); + it('should keep keys that only differ in case in the order they appear in', async function () { + // The comparator treats these as equal, so the sort - being stable - has to + // leave them in the order the documents introduced them in. + const upperFirst = await getSchema([{ cb: 1, Ca: 1, ca: 1, a: 1 }]); + assert.deepEqual( + upperFirst.fields.map((v) => v.name), + ['a', 'Ca', 'ca', 'cb'], + ); + + const lowerFirst = await getSchema([{ cb: 1, ca: 1, Ca: 1, a: 1 }]); + assert.deepEqual( + lowerFirst.fields.map((v) => v.name), + ['a', 'ca', 'Ca', 'cb'], + ); + }); }); diff --git a/packages/mongodb-schema/test/integration/generate-and-validate.test.ts b/packages/mongodb-schema/test/integration/generate-and-validate.test.ts index 2d71a6cac..f2b8ea990 100644 --- a/packages/mongodb-schema/test/integration/generate-and-validate.test.ts +++ b/packages/mongodb-schema/test/integration/generate-and-validate.test.ts @@ -54,6 +54,8 @@ describe('Documents -> Generate schema -> Validate Documents against the schema' }); describe('With a MongoDB Cluster', function () { + this.timeout(30_000); + let client: MongoClient; let db: Db; const cluster = mochaTestServer();