-
Notifications
You must be signed in to change notification settings - Fork 259
[feat]: upgrade to mongodb driver 6 #499
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
Changes from all commits
bb3d120
b70cb73
fa906e5
5d1ef99
6e918a5
ec3dad7
c579e58
15fe1c4
6318039
0ec5228
2a39aad
31e6a9a
1906335
b301464
16506bd
337fac4
97f9810
0636f4c
cf3b4fa
222d25f
c913834
1ea5d21
8bd1d34
12c669d
d327eea
591f701
9cb19f5
5c0a0e3
3615b2c
a8c6f6a
8034502
82168af
ec5b6a6
d764465
a20b8fb
2cde4d1
69f725a
e5924a6
efe3235
6d41daa
c636498
ae9fbb9
d220a41
8d82b32
ea9d867
bef4089
cd4bee3
b951b05
0a29e4d
d6819fa
10ee8aa
2a04922
bb786c1
2147264
6cf7cc2
14c2d72
19e0d03
55e7001
61629ce
a8bf4fe
e3872d7
0574896
a5f8e12
765c9d9
68f8b94
056ccde
533f6d5
998b2ea
93d399f
467b8ea
cc52765
e739209
8370a46
cc6d982
5bdce05
0463010
17f1656
a80f925
94097b9
1f058f7
e0fb78d
cb5699a
0de57fe
fba5a2a
966450a
66b1490
96531c3
cde8fcd
c46934a
4fd3e26
59b4682
915e89c
e38df9a
23aeb4a
240e8b8
ffff3f1
640519c
404a92d
472c68e
1057938
b4a7a66
b456449
3fe2b84
be326d9
8feb197
895fed0
c54d1f7
28b7cfd
7203ae8
3886dd7
6f94e5f
fd98a29
6b6e8a8
721095d
5a3bf56
0e857a3
bb27cdd
bca5215
594afd6
272b2fe
a347693
fb3e1d1
800656d
b9b7041
838a0d6
c814854
624e784
570881a
3b6dc39
1829c07
12ad6e0
9291e9a
13cfa65
d1e3e15
817a9e3
b109abc
529d82f
772a717
11aacde
2cd402e
fc73eaa
da8e6fb
5ecf56a
8cd7034
36a4e30
507afd3
b2795b5
f362416
162a5b4
5d5ea5c
76da6f8
f6bc8f4
229c363
fb34f80
715f0d4
0e5c0b0
3de69b4
bf4d976
80296ac
c2e4ac0
171fd4e
f8e4972
a9791c3
c313798
8322fa6
6e0de20
0e78883
4b12cf6
852a17c
77b24fc
0afecb9
1ef1b8c
19e0869
0de5119
6b989a3
3ccd0e7
60fb9c8
5a30188
1ad41ac
acfd322
49eeeb5
0f856d4
4558ab6
856a9e1
4b69023
dff7767
f190ffb
817c742
c151a31
4cc13ca
a7c8f6f
49948b5
ecf1f42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,24 @@ | ||
| adapter: | ||
| image: node:12 | ||
| volumes: | ||
| - $PWD:/home/node/sails-mongo | ||
| links: | ||
| - mongo | ||
| environment: | ||
| - WATERLINE_ADAPTER_TESTS_DATABASE=sails-mongo | ||
| - WATERLINE_ADAPTER_TESTS_URL=mongo/testdb | ||
| - WATERLINE_ADAPTER_TESTS_HOST=mongo | ||
| - NODE_ENV=test | ||
| user: node | ||
| working_dir: /home/node/sails-mongo | ||
| command: | ||
| - bash -c "npm test" | ||
| version: '3' | ||
| services: | ||
| adapter: | ||
| image: node:20 | ||
| volumes: | ||
| - $PWD:/home/node/sails-mongo | ||
| links: | ||
| - mongo | ||
| environment: | ||
| - WATERLINE_ADAPTER_TESTS_DATABASE=sails-mongo | ||
| - WATERLINE_ADAPTER_TESTS_URL=mongo/testdb | ||
| - WATERLINE_ADAPTER_TESTS_HOST=mongo | ||
| - NODE_ENV=test | ||
| user: node | ||
| working_dir: /home/node/sails-mongo | ||
| command: | ||
| - bash -c "npm test" | ||
|
|
||
| mongo: | ||
| image: mongo:4.2 | ||
| restart: always | ||
| command: "--logpath=/dev/null" | ||
| ports: | ||
| - "27017:27017" | ||
| mongo: | ||
| image: mongo:7 | ||
| restart: always | ||
| command: "--logpath=/dev/null" | ||
| ports: | ||
| - "27017:27017" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -125,16 +125,8 @@ module.exports = { | |
| var mongoUrl = _clientConfig.url; | ||
| _clientConfig = _.omit(_clientConfig, ['url', 'user', 'password', 'host', 'port', 'database']); | ||
|
|
||
| // Use unified topology. MongoDB node maintainers recommends this to be enabled | ||
| // https://github.com/mongodb/node-mongodb-native/releases/tag/v3.2.1 | ||
| // Use new url parser to remove warnings | ||
| _clientConfig = Object.assign({ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @mikermcneil this line has to go because the configs we are setting here has been deprecated and produces warnings. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this could be related to the |
||
| useNewUrlParser: true, | ||
| useUnifiedTopology: true | ||
| }, _clientConfig); | ||
|
|
||
| // http://mongodb.github.io/node-mongodb-native/3.5/api/MongoClient.html#.connect | ||
| NodeMongoDBNativeLib.MongoClient.connect(mongoUrl, _clientConfig, function connectCb(err, client) { | ||
| // https://mongodb.github.io/node-mongodb-native/6.3/classes/MongoClient.html#connect | ||
| (function(iifeDone){ NodeMongoDBNativeLib.MongoClient.connect(mongoUrl, _clientConfig).then(function(client){ iifeDone(undefined, client);}).catch(function(err) { iifeDone(err);});})(function(err, client){ | ||
| if (err) { | ||
| return exits.error(err); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,7 +76,7 @@ module.exports = { | |
| // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - | ||
| var db = inputs.connection; | ||
| var mongoCollection = db.collection(tableName); | ||
| mongoCollection.insertOne(s3q.newRecord, function (err, nativeResult) { | ||
| (function(iifeDone){ mongoCollection.insertOne(s3q.newRecord).then(function(nativeResult) { iifeDone(undefined, nativeResult);}).catch(function(err){ iifeDone(err);});})(function(err, nativeResult) { | ||
| if (err) { | ||
| err = processNativeError(err); | ||
| if (err.footprint && err.footprint.identity === 'notUnique') { | ||
|
|
@@ -102,23 +102,28 @@ module.exports = { | |
|
|
||
| // Otherwise, IWMIH we'll be sending back a record: | ||
| // ============================================ | ||
|
|
||
| // Sanity check: Verify that there is only one record. | ||
| if (nativeResult.ops.length !== 1) { | ||
| return exits.error(new Error('Consistency violation: Unexpected # of records returned from Mongo (in `.ops`). Native result:\n```\n'+util.inspect(nativeResult, {depth: 5})+'\n```')); | ||
| // Sanity check: Verify that an ID was sent back. | ||
| if (_.isUndefined(nativeResult.insertedId)) { | ||
| return exits.error(new Error('Consistency violation: Unable to retrieve insertedId from the result. This might indicate a consistency violation. Native result details:\n```\n' + util.inspect(nativeResult, {depth: 5}) + '\n```')); | ||
| } | ||
|
|
||
|
|
||
| // ╔═╗╦═╗╔═╗╔═╗╔═╗╔═╗╔═╗ ┌┐┌┌─┐┌┬┐┬┬ ┬┌─┐ ┬─┐┌─┐┌─┐┌─┐┬─┐┌┬┐ | ||
| // ╠═╝╠╦╝║ ║║ ║╣ ╚═╗╚═╗ │││├─┤ │ │└┐┌┘├┤ ├┬┘├┤ │ │ │├┬┘ ││ | ||
| // ╩ ╩╚═╚═╝╚═╝╚═╝╚═╝╚═╝ ┘└┘┴ ┴ ┴ ┴ └┘ └─┘ ┴└─└─┘└─┘└─┘┴└──┴┘ | ||
| // Process record (mutate in-place) to wash away adapter-specific eccentricities. | ||
| var phRecord = nativeResult.ops[0]; | ||
| try { | ||
| processNativeRecord(phRecord, WLModel, s3q.meta); | ||
| } catch (e) { return exits.error(e); } | ||
|
|
||
| // Then send it back. | ||
| return exits.success(phRecord); | ||
| // Use the new record ID to find the record that was created. | ||
| (function(iifeDone) { mongoCollection.findOne({ _id: nativeResult.insertedId }).then(function(phRecord) { iifeDone(undefined, phRecord);}).catch(function(err) { iifeDone(err);});})(function(err, phRecord) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the useNewUrlParser @mikermcneil that's no longer a supported config option in the newer version of the MongoDB driver. @eashaw can you confirm if that's the case for you? |
||
| if (err) { | ||
| return exits.error(err); | ||
| } | ||
| try { | ||
| // Process record (mutate in-place) to wash away adapter-specific eccentricities. | ||
| processNativeRecord(phRecord, WLModel, s3q.meta); | ||
| } catch (e) { return exits.error(e); } | ||
|
|
||
| // Then send it back. | ||
| return exits.success(phRecord); | ||
| }); | ||
|
|
||
| }); // </ mongoCollection.insertOne() > | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should paste in (and slightly tweak) the assertion from here: https://github.com/balderdashy/sails-mongo/pull/499/files#diff-962fecaf7496f0dc0f4c49842e1e52f7035018902cd4530b86833883185c0166L106
that way we'll know if something changes in Mongo in the future and keep things just as easy to follow for migrating to future mongo driver releases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right @mikermcneil. I just added the assertions and tweaked it to match what we are checking.