Skip to content

Commit

Permalink
fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
emadum committed Feb 10, 2021
1 parent 176a56b commit 70f8935
Show file tree
Hide file tree
Showing 12 changed files with 81 additions and 42 deletions.
4 changes: 0 additions & 4 deletions src/cmap/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,10 +367,6 @@ export class Connection extends EventEmitter {

const inTransaction = session && (session.inTransaction() || isTransactionCommand(finalCmd));

if (!inTransaction && !finalCmd.getMore && this.serverApi) {
applyServerApiVersion(finalCmd, this.serverApi);
}

const commandResponseHandler = inTransaction
? (err?: AnyError, ...args: Document[]) => {
// We need to add a TransientTransactionError errorLabel, as stated in the transaction spec.
Expand Down
12 changes: 11 additions & 1 deletion src/connection_string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import {
MongoClient,
MongoClientOptions,
MongoOptions,
PkFactory
PkFactory,
ServerApi
} from './mongo_client';
import { MongoCredentials } from './cmap/auth/mongo_credentials';
import type { TagSet } from './sdam/server_description';
Expand Down Expand Up @@ -572,6 +573,15 @@ export const OPTIONS = {
autoEncryption: {
type: 'record'
},
serverApi: {
target: 'serverApi',
transform({ values: [version] }): ServerApi {
if (typeof version === 'string') {
return { version };
}
return version as ServerApi;
}
},
checkKeys: {
type: 'boolean'
},
Expand Down
19 changes: 8 additions & 11 deletions src/mongo_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ export interface MongoClientOptions extends BSONSerializeOptions, SupportedNodeC
logger?: Logger;
/** Enable command monitoring for this client */
monitorCommands?: boolean;
/** Server API version */
serverApi?: ServerApiVersion | ServerApi;
/** Optionally enable client side auto encryption */
autoEncryption?: AutoEncryptionOptions;
/** Allows a wrapping driver to amend the client metadata generated by the driver to include information about the wrapping driver */
Expand All @@ -243,13 +245,13 @@ export interface MongoClientPrivate {
readConcern?: ReadConcern;
writeConcern?: WriteConcern;
readPreference: ReadPreference;
serverApi: ServerApi;
bsonOptions: BSONSerializeOptions;
namespace: MongoDBNamespace;
logger: Logger;
}

const kOptions = Symbol('options');
const kServerApi = Symbol('serverApi');

/**
* The **MongoClient** class is a class that allows for making Connections to MongoDB.
Expand Down Expand Up @@ -302,24 +304,17 @@ export class MongoClient extends EventEmitter {
*/
[kOptions]: MongoOptions;

/**
* The MongoDB Server API version
* @internal
* */
[kServerApi]: ServerApi;

// debugging
originalUri;
originalOptions;

constructor(url: string, options?: MongoClientOptions, serverApi?: ServerApi) {
constructor(url: string, options?: MongoClientOptions) {
super();

this.originalUri = url;
this.originalOptions = options;

this[kOptions] = parseOptions(url, this, options);
this[kServerApi] = Object.freeze({ version: ServerApiVersion.v1, ...serverApi });

// The internal state
this.s = {
Expand All @@ -329,6 +324,7 @@ export class MongoClient extends EventEmitter {
readConcern: this[kOptions].readConcern,
writeConcern: this[kOptions].writeConcern,
readPreference: this[kOptions].readPreference,
serverApi: this[kOptions].serverApi,
bsonOptions: resolveBSONOptions(this[kOptions]),
namespace: ns('admin'),
logger: this[kOptions].logger
Expand All @@ -339,8 +335,8 @@ export class MongoClient extends EventEmitter {
return Object.freeze({ ...this[kOptions] });
}

get serverApi(): Readonly<ServerApi> {
return this[kServerApi];
get serverApi(): Readonly<ServerApi | undefined> {
return this[kOptions].serverApi && Object.freeze({ ...this[kOptions].serverApi });
}

get autoEncrypter(): AutoEncrypter | undefined {
Expand Down Expand Up @@ -652,6 +648,7 @@ export interface MongoOptions
credentials?: MongoCredentials;
readPreference: ReadPreference;
readConcern: ReadConcern;
serverApi: ServerApi;
writeConcern: WriteConcern;
dbName: string;
metadata: ClientMetadata;
Expand Down
15 changes: 14 additions & 1 deletion src/operations/command.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import { Aspect, AbstractOperation, OperationOptions } from './operation';
import { ReadConcern } from '../read_concern';
import { WriteConcern, WriteConcernOptions } from '../write_concern';
import { maxWireVersion, MongoDBNamespace, Callback, decorateWithExplain } from '../utils';
import {
applyServerApiVersion,
maxWireVersion,
MongoDBNamespace,
Callback,
decorateWithExplain
} from '../utils';
import type { ReadPreference } from '../read_preference';
import { ClientSession, commandSupportsReadConcern } from '../sessions';
import { MongoError } from '../error';
Expand Down Expand Up @@ -173,6 +179,13 @@ export abstract class CommandOperation<T> extends AbstractOperation<T> {
}
}

const notMidTransaction = !inTransaction || !this.session.transaction.hasStarted;
// FIXME: get these hacked conditions in the right place
const hackedConditions = !cmd.getMore && !cmd.commitTransaction;
if (notMidTransaction && hackedConditions && server.serverApi) {
applyServerApiVersion(cmd, server.serverApi);
}

server.command(this.ns, cmd, { fullResult: !!this.fullResponse, ...options }, callback);
}
}
6 changes: 5 additions & 1 deletion src/operations/find.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import {
MongoDBNamespace,
Callback,
normalizeHintField,
decorateWithExplain
decorateWithExplain,
applyServerApiVersion
} from '../utils';
import { MongoError } from '../error';
import type { Document } from '../bson';
Expand Down Expand Up @@ -153,6 +154,9 @@ export class FindOperation extends CommandOperation<Document> {
if (this.explain) {
findCommand = decorateWithExplain(findCommand, this.explain);
}
if (server.serverApi) {
applyServerApiVersion(findCommand, server.serverApi);
}

server.command(
this.ns,
Expand Down
4 changes: 3 additions & 1 deletion src/sdam/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import type { ServerHeartbeatSucceededEvent } from './events';
import type { ClientSession } from '../sessions';
import type { Document, Long } from '../bson';
import type { AutoEncrypter } from '../deps';
import type { ServerApi } from '../mongo_client';

// Used for filtering out fields for logging
const DEBUG_FIELDS = [
Expand Down Expand Up @@ -106,6 +107,7 @@ export interface ServerPrivate {
export class Server extends EventEmitter {
/** @internal */
s: ServerPrivate;
serverApi?: ServerApi;
clusterTime?: ClusterTime;
ismaster?: Document;
[kMonitor]: Monitor;
Expand All @@ -131,7 +133,7 @@ export class Server extends EventEmitter {
constructor(topology: Topology, description: ServerDescription, options: ServerOptions) {
super();

options.serverApi = topology.serverApi;
this.serverApi = options.serverApi = topology.serverApi;

const poolOptions = { hostAddress: description.hostAddress, ...options };

Expand Down
5 changes: 5 additions & 0 deletions src/transactions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@ export class Transaction {
return !!this.server;
}

/** @returns Whether the transaction has started */
get hasStarted(): boolean {
return this.state === TxnState.TRANSACTION_IN_PROGRESS;
}

/**
* @returns Whether this session is presently in a transaction
*/
Expand Down
6 changes: 5 additions & 1 deletion test/functional/unified-spec-runner/entities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ export class UnifiedMongoClient extends MongoClient {
} as const;

constructor(url: string, description: ClientEntity) {
super(url, { monitorCommands: true, ...description.uriOptions }, description.serverApi);
super(url, {
monitorCommands: true,
...description.uriOptions,
serverApi: description.serverApi
});
this.events = [];
this.failPoints = [];
this.ignoredEvents = [
Expand Down
4 changes: 2 additions & 2 deletions test/functional/unified-spec-runner/unified-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ export async function topologySatisfies(
Sharded: 'sharded'
}[config.topologyType];

if (r.topologies.includes('sharded-replicaset')) {
if (r.topologies.includes('sharded-replicaset') && topologyType === 'sharded') {
const shards = await utilClient.db('config').collection('shards').find({}).toArray();
ok &&= shards.every(shard => shard.host.split(',').length > 1);
ok &&= shards.length > 0 && shards.every(shard => shard.host.split(',').length > 1);
} else {
if (!topologyType) throw new Error(`Topology undiscovered: ${config.topologyType}`);
ok &&= r.topologies.includes(topologyType);
Expand Down
23 changes: 13 additions & 10 deletions test/functional/versioned-api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,19 @@ describe('Versioned API', function () {
expect(versionedApiTest).to.exist;
context(String(versionedApiTest.description), function () {
for (const test of versionedApiTest.tests) {
it(String(test.description), async function () {
try {
await runUnifiedTest(this, versionedApiTest, test);
} catch (error) {
if (error.message.includes('not implemented.')) {
console.log(`${test.description}: was skipped due to missing functionality`);
console.log(error.stack);
this.skip();
} else {
throw error;
it(String(test.description), {
metadata: { sessions: { skipLeakTests: true } },
test: async function () {
try {
await runUnifiedTest(this, versionedApiTest, test);
} catch (error) {
if (error.message.includes('not implemented.')) {
console.log(`${test.description}: was skipped due to missing functionality`);
console.log(error.stack);
this.skip();
} else {
throw error;
}
}
}
});
Expand Down
17 changes: 9 additions & 8 deletions test/tools/runner/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ function convertToConnStringMap(obj) {
}

class TestConfiguration {
constructor(uri, context, serverApiVersion) {
constructor(uri, context) {
const { url, hosts } = parseURI(uri);
const hostAddresses = hosts.map(HostAddress.fromString);
this.topologyType = context.topologyType;
this.version = context.version;
this.serverApiVersion = serverApiVersion;
this.clientSideEncryption = context.clientSideEncryption;
this.serverApi = context.serverApi;
this.parameters = undefined;
this.options = {
hosts,
Expand Down Expand Up @@ -97,17 +97,17 @@ class TestConfiguration {
}

newClient(dbOptions, serverOptions) {
const defaultOptions = { minHeartbeatFrequencyMS: 100 };
if (this.serverApi) {
Object.assign(defaultOptions, { serverApi: this.serverApi });
}
// support MongoClient constructor form (url, options) for `newClient`
if (typeof dbOptions === 'string') {
return new MongoClient(
dbOptions,
Object.assign({ minHeartbeatFrequencyMS: 100 }, serverOptions),
{ version: this.serverApiVersion }
);
return new MongoClient(dbOptions, Object.assign(defaultOptions, serverOptions));
}

dbOptions = dbOptions || {};
serverOptions = Object.assign({}, { minHeartbeatFrequencyMS: 100 }, serverOptions);
serverOptions = Object.assign({}, defaultOptions, serverOptions);

// Fall back
let dbHost = (serverOptions && serverOptions.host) || this.options.host;
Expand Down Expand Up @@ -166,6 +166,7 @@ class TestConfiguration {
if (Reflect.has(serverOptions, 'host') || Reflect.has(serverOptions, 'port')) {
throw new Error(`Cannot use options to specify host/port, must be in ${connectionString}`);
}

return new MongoClient(connectionString, serverOptions);
}

Expand Down
8 changes: 6 additions & 2 deletions test/tools/runner/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ before(function (_done) {
// )} topology`
// );

const client = new MongoClient(MONGODB_URI);
const options = MONGODB_API_VERSION ? { serverApi: MONGODB_API_VERSION } : {};
const client = new MongoClient(MONGODB_URI, options);
const done = err => client.close(err2 => _done(err || err2));

client.connect(err => {
Expand All @@ -71,14 +72,17 @@ before(function (_done) {
}

initializeFilters(client, (err, context) => {
if (MONGODB_API_VERSION) {
Object.assign(context, { serverApi: MONGODB_API_VERSION });
}
if (err) {
done(err);
return;
}

// replace this when mocha supports dynamic skipping with `afterEach`
filterOutTests(this._runnable.parent);
this.configuration = new TestConfiguration(MONGODB_URI, context, MONGODB_API_VERSION);
this.configuration = new TestConfiguration(MONGODB_URI, context);
done();
});
});
Expand Down

0 comments on commit 70f8935

Please sign in to comment.