Skip to content
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.

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

fix(NODE-3585): MongoClientOptions#compressors has incorrect type #2976

Merged
merged 2 commits into from Sep 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/connection_string.ts
Expand Up @@ -32,7 +32,7 @@ import type { TagSet } from './sdam/server_description';
import { Logger, LoggerLevel } from './logger';
import { PromiseProvider } from './promise_provider';
import { Encrypter } from './encrypter';
import { Compressor } from './cmap/wire_protocol/compression';
import { Compressor, CompressorName } from './cmap/wire_protocol/compression';

const VALID_TXT_RECORDS = ['authSource', 'replicaSet', 'loadBalanced'];

Expand Down Expand Up @@ -612,8 +612,14 @@ export const OPTIONS = {
target: 'compressors',
transform({ values }) {
const compressionList = new Set();
for (const compVal of values as string[]) {
for (const c of compVal.split(',')) {
for (const compVal of values as (CompressorName[] | string)[]) {
const compValArray = typeof compVal === 'string' ? compVal.split(',') : compVal;
if (!Array.isArray(compValArray)) {
throw new MongoInvalidArgumentError(
'compressors must be an array or a comma-delimited list of strings'
);
}
for (const c of compValArray) {
if (Object.keys(Compressor).includes(String(c))) {
compressionList.add(String(c));
} else {
Expand Down
6 changes: 3 additions & 3 deletions src/mongo_client.ts
Expand Up @@ -128,8 +128,8 @@ export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeC
connectTimeoutMS?: number;
/** The time in milliseconds to attempt a send or receive on a socket before the attempt times out. */
socketTimeoutMS?: number;
/** Comma-delimited string of compressors to enable network compression for communication between this client and a mongod/mongos instance. */
compressors?: CompressorName[];
/** An array or comma-delimited string of compressors to enable network compression for communication between this client and a mongod/mongos instance. */
compressors?: CompressorName[] | string;
/** An integer that specifies the compression level if using zlib for network compression. */
zlibCompressionLevel?: 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 | undefined;
/** The maximum number of connections in the connection pool. */
Expand Down Expand Up @@ -620,7 +620,6 @@ export interface MongoOptions
Pick<
MongoClientOptions,
| 'autoEncryption'
| 'compressors'
| 'connectTimeoutMS'
| 'directConnection'
| 'driverInfo'
Expand Down Expand Up @@ -659,6 +658,7 @@ export interface MongoOptions
readConcern: ReadConcern;
loadBalanced: boolean;
serverApi: ServerApi;
compressors: CompressorName[];
writeConcern: WriteConcern;
dbName: string;
metadata: ClientMetadata;
Expand Down
2 changes: 1 addition & 1 deletion test/types/community/changes_from_36.test-d.ts
Expand Up @@ -69,7 +69,7 @@ expectAssignable<((host: string, cert: PeerCertificate) => Error | undefined) |
// compression options have simpler specification:
// old way: {compression: { compressors: ['zlib', 'snappy'] }}
expectType<PropExists<MongoClientOptions, 'compression'>>(false);
expectType<('none' | 'snappy' | 'zlib')[] | undefined>(options.compressors);
expectType<('none' | 'snappy' | 'zlib')[] | string | undefined>(options.compressors);

// Removed cursor API
const cursor = new MongoClient('').db().aggregate();
Expand Down
54 changes: 53 additions & 1 deletion test/unit/mongo_client_options.test.js
Expand Up @@ -78,7 +78,7 @@ describe('MongoOptions', function () {
autoEncryption: { bypassAutoEncryption: true },
checkKeys: true,
checkServerIdentity: false,
compressors: 'snappy', // TODO
compressors: 'snappy,zlib',
connectTimeoutMS: 123,
directConnection: true,
dbName: 'test',
Expand Down Expand Up @@ -385,6 +385,58 @@ describe('MongoOptions', function () {
expect(optionsUndefined.checkServerIdentity).to.equal(undefined);
});

describe('compressors', function () {
it('can be set when passed in as an array in the options object', function () {
const clientViaOpt = new MongoClient('mongodb://localhost', {
compressors: ['zlib', 'snappy']
});
expect(clientViaOpt.options)
.to.have.property('compressors')
.deep.equal(['zlib', 'snappy', 'none']);
});

it('can be set when passed in as a comma-delimited string in the options object or URI', function () {
const clientViaOpt = new MongoClient('mongodb://localhost', {
compressors: 'zlib,snappy'
});
const clientViaUri = new MongoClient('mongodb://localhost?compressors=zlib,snappy');
expect(clientViaOpt.options)
.to.have.property('compressors')
.deep.equal(['zlib', 'snappy', 'none']);
expect(clientViaUri.options)
.to.have.property('compressors')
.deep.equal(['zlib', 'snappy', 'none']);
});

it('should validate that a string or an array of strings is provided as input', function () {
expect(
() =>
new MongoClient('mongodb://localhost', {
compressors: { zlib: true }
})
).to.throw(/^compressors must be an array or a comma-delimited list of strings/);
});

it('should throw an error if an unrecognized compressor is specified', function () {
const expectedErrRegex = /not a valid compression mechanism/;
expect(
() =>
new MongoClient('mongodb://localhost', {
compressors: ['invalid']
})
).to.throw(expectedErrRegex);
expect(
() =>
new MongoClient('mongodb://localhost', {
compressors: 'invalid'
})
).to.throw(expectedErrRegex);
expect(() => new MongoClient('mongodb://localhost?compressors=invalid')).to.throw(
expectedErrRegex
);
});
});

describe('serverApi', function () {
it('is supported as a client option when it is a valid ServerApiVersion string', function () {
const validVersions = Object.values(ServerApiVersion);
Expand Down