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.

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

Jump to bottom

fix(NODE-4031): options parsing for array options #3193

Merged
merged 3 commits into from
Apr 18, 2022
Merged

fix(NODE-4031): options parsing for array options #3193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b6eeff
fix: move optoins parsing into tests
baileympearson Apr 6, 2022
4b9400b
fix: address bug
baileympearson Apr 7, 2022
c2cd22d
chore: add jira ticket and todo
baileympearson Apr 7, 2022
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
39 changes: 25 additions & 14 deletions src/connection_string.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
ServerApi,
ServerApiVersion
} from './mongo_client';
import type { OneOrMore } from './mongo_types';
import { PromiseProvider } from './promise_provider';
import { ReadConcern, ReadConcernLevel } from './read_concern';
import { ReadPreference, ReadPreferenceMode } from './read_preference';
Expand Down Expand Up @@ -220,12 +219,7 @@ function getUint(name: string, value: unknown): number {
return parsedValue;
}

/** Wrap a single value in an array if the value is not an array */
function toArray<T>(value: OneOrMore<T>): T[] {
return Array.isArray(value) ? value : [value];
}

function* entriesFromString(value: string) {
function* entriesFromString(value: string): Generator<[string, string]> {
const keyValuePairs = value.split(',');
for (const keyValue of keyValuePairs) {
const [key, value] = keyValue.split(':');
Expand Down Expand Up @@ -333,11 +327,19 @@ export function parseOptions(
]);

for (const key of allKeys) {
const values = [objectOptions, urlOptions, DEFAULT_OPTIONS].flatMap(optionsObject => {
const options = optionsObject.get(key) ?? [];
return toArray(options);
});

const values = [];
const objectOptionValue = objectOptions.get(key);
if (objectOptionValue != null) {
values.push(objectOptionValue);
}
const urlValue = urlOptions.get(key);
if (urlValue != null) {
values.push(...urlValue);
}
const defaultOptionsValue = DEFAULT_OPTIONS.get(key);
if (defaultOptionsValue != null) {
values.push(defaultOptionsValue);
}
allOptions.set(key, values);
}

Expand Down Expand Up @@ -982,9 +984,18 @@ export const OPTIONS = {
},
readPreferenceTags: {
target: 'readPreference',
transform({ values, options }) {
transform({
values,
options
}: {
values: Array<string | Record<string, string>[]>;
options: MongoClientOptions;
}) {
const tags: Array<string | Record<string, string>> = Array.isArray(values[0])
? values[0]
: (values as Array<string>);
const readPreferenceTags = [];
for (const tag of values) {
for (const tag of tags) {
const readPreferenceTag: TagSet = Object.create(null);
if (typeof tag === 'string') {
for (const [k, v] of entriesFromString(tag)) {
Expand Down
158 changes: 140 additions & 18 deletions test/unit/connection_string.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,138 @@ describe('Connection String', function () {
expect(options.hosts[0].port).to.equal(27017);
});

context('readPreferenceTags', function () {
it('should parse multiple readPreferenceTags when passed in the uri', () => {
const options = parseOptions(
'mongodb://hostname?readPreferenceTags=bar:foo&readPreferenceTags=baz:bar'
);
expect(options.readPreference.tags).to.deep.equal([{ bar: 'foo' }, { baz: 'bar' }]);
describe('ca option', () => {
context('when set in the options object', () => {
it('should parse a string', () => {
const options = parseOptions('mongodb://localhost', {
ca: 'hello'
});
expect(options).to.have.property('ca').to.equal('hello');
});

it('should parse a NodeJS buffer', () => {
const options = parseOptions('mongodb://localhost', {
ca: Buffer.from([1, 2, 3, 4])
});

expect(options)
.to.have.property('ca')
.to.deep.equal(Buffer.from([1, 2, 3, 4]));
});

it('should parse arrays with a single element', () => {
const options = parseOptions('mongodb://localhost', {
ca: ['hello']
});
expect(options).to.have.property('ca').to.deep.equal(['hello']);
});

it('should parse an empty array', () => {
const options = parseOptions('mongodb://localhost', {
ca: []
});
expect(options).to.have.property('ca').to.deep.equal([]);
});

it('should parse arrays with multiple elements', () => {
const options = parseOptions('mongodb://localhost', {
ca: ['hello', 'world']
});
expect(options).to.have.property('ca').to.deep.equal(['hello', 'world']);
});
});

// TODO(NODE-4172): align uri behavior with object options behavior
context('when set in the uri', () => {
it('should parse a string value', () => {
const options = parseOptions('mongodb://localhost?ca=hello', {});
expect(options).to.have.property('ca').to.equal('hello');
});

it('should throw an error with a buffer value', () => {
const buffer = Buffer.from([1, 2, 3, 4]);
expect(() => {
parseOptions(`mongodb://localhost?ca=${buffer.toString()}`, {});
}).to.throw(MongoAPIError);
});

it('should not parse multiple string values (array of options)', () => {
const options = parseOptions('mongodb://localhost?ca=hello,world', {});
expect(options).to.have.property('ca').to.equal('hello,world');
});
});

it('should prioritize options set in the object over those set in the URI', () => {
const options = parseOptions('mongodb://localhost?ca=hello', {
ca: ['world']
});
expect(options).to.have.property('ca').to.deep.equal(['world']);
});
});

it('should parse multiple readPreferenceTags when passed in options object', () => {
const options = parseOptions('mongodb://hostname?', {
describe('readPreferenceTags option', function () {
context('when the option is passed in the uri', () => {
it('should throw an error if no value is passed for readPreferenceTags', () => {
expect(() => parseOptions('mongodb://hostname?readPreferenceTags=')).to.throw(
MongoAPIError
);
});
it('should parse a single read preference tag', () => {
const options = parseOptions('mongodb://hostname?readPreferenceTags=bar:foo');
expect(options.readPreference.tags).to.deep.equal([{ bar: 'foo' }]);
});
it('should parse multiple readPreferenceTags', () => {
const options = parseOptions(
'mongodb://hostname?readPreferenceTags=bar:foo&readPreferenceTags=baz:bar'
);
expect(options.readPreference.tags).to.deep.equal([{ bar: 'foo' }, { baz: 'bar' }]);
});
it('should parse multiple readPreferenceTags for the same key', () => {
const options = parseOptions(
'mongodb://hostname?readPreferenceTags=bar:foo&readPreferenceTags=bar:banana&readPreferenceTags=baz:bar'
);
expect(options.readPreference.tags).to.deep.equal([
{ bar: 'foo' },
{ bar: 'banana' },
{ baz: 'bar' }
]);
});
});

context('when the option is passed in the options object', () => {
it('should not parse an empty readPreferenceTags object', () => {
const options = parseOptions('mongodb://hostname?', {
readPreferenceTags: []
});
expect(options.readPreference.tags).to.deep.equal([]);
});
it('should parse a single readPreferenceTags object', () => {
const options = parseOptions('mongodb://hostname?', {
readPreferenceTags: [{ bar: 'foo' }]
});
expect(options.readPreference.tags).to.deep.equal([{ bar: 'foo' }]);
});
it('should parse multiple readPreferenceTags', () => {
const options = parseOptions('mongodb://hostname?', {
readPreferenceTags: [{ bar: 'foo' }, { baz: 'bar' }]
});
expect(options.readPreference.tags).to.deep.equal([{ bar: 'foo' }, { baz: 'bar' }]);
});

it('should parse multiple readPreferenceTags for the same key', () => {
const options = parseOptions('mongodb://hostname?', {
readPreferenceTags: [{ bar: 'foo' }, { bar: 'banana' }, { baz: 'bar' }]
});
expect(options.readPreference.tags).to.deep.equal([
{ bar: 'foo' },
{ bar: 'banana' },
{ baz: 'bar' }
]);
});
});

it('should prioritize options from the options object over the uri options', () => {
const options = parseOptions('mongodb://hostname?readPreferenceTags=a:b', {
readPreferenceTags: [{ bar: 'foo' }, { baz: 'bar' }]
});
expect(options.readPreference.tags).to.deep.equal([{ bar: 'foo' }, { baz: 'bar' }]);
Expand Down Expand Up @@ -174,25 +296,25 @@ describe('Connection String', function () {
context('when the options are provided in the URI', function () {
context('when the options are equal', function () {
context('when both options are true', function () {
const options = parseOptions('mongodb://localhost/?tls=true&ssl=true');

it('sets the tls option', function () {
const options = parseOptions('mongodb://localhost/?tls=true&ssl=true');
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
expect(options.tls).to.be.true;
});

it('does not set the ssl option', function () {
const options = parseOptions('mongodb://localhost/?tls=true&ssl=true');
expect(options).to.not.have.property('ssl');
});
});

context('when both options are false', function () {
const options = parseOptions('mongodb://localhost/?tls=false&ssl=false');

it('sets the tls option', function () {
const options = parseOptions('mongodb://localhost/?tls=false&ssl=false');
expect(options.tls).to.be.false;
});

it('does not set the ssl option', function () {
const options = parseOptions('mongodb://localhost/?tls=false&ssl=false');
expect(options).to.not.have.property('ssl');
});
});
Expand All @@ -210,38 +332,38 @@ describe('Connection String', function () {
context('when the options are provided in the options', function () {
context('when the options are equal', function () {
context('when both options are true', function () {
const options = parseOptions('mongodb://localhost/', { tls: true, ssl: true });

it('sets the tls option', function () {
const options = parseOptions('mongodb://localhost/', { tls: true, ssl: true });
expect(options.tls).to.be.true;
});

it('does not set the ssl option', function () {
const options = parseOptions('mongodb://localhost/', { tls: true, ssl: true });
expect(options).to.not.have.property('ssl');
});
});

context('when both options are false', function () {
context('when the URI is an SRV URI', function () {
const options = parseOptions('mongodb+srv://localhost/', { tls: false, ssl: false });

it('overrides the tls option', function () {
const options = parseOptions('mongodb+srv://localhost/', { tls: false, ssl: false });
expect(options.tls).to.be.false;
});

it('does not set the ssl option', function () {
const options = parseOptions('mongodb+srv://localhost/', { tls: false, ssl: false });
expect(options).to.not.have.property('ssl');
});
});

context('when the URI is not SRV', function () {
const options = parseOptions('mongodb://localhost/', { tls: false, ssl: false });

it('sets the tls option', function () {
const options = parseOptions('mongodb://localhost/', { tls: false, ssl: false });
expect(options.tls).to.be.false;
});

it('does not set the ssl option', function () {
const options = parseOptions('mongodb://localhost/', { tls: false, ssl: false });
expect(options).to.not.have.property('ssl');
});
});
Expand Down