Finish wiring up .updateOne()

balderdashy · Jul 4, 2018 · 2f861cf · 2f861cf
1 parent 24c32ca
commit 2f861cf
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 32 deletions.
diff --git a/lib/waterline/MetaModel.js b/lib/waterline/MetaModel.js
@@ -149,8 +149,11 @@ _.extend(
     create: require('./methods/create'),
     createEach: require('./methods/create-each'),
     update: require('./methods/update'),
+    updateOne: require('./methods/update-one'),
     destroy: require('./methods/destroy'),
+    destroyOne: require('./methods/destroy-one'),
     archive: require('./methods/archive'),
+    archiveOne: require('./methods/archive-one'),
     addToCollection: require('./methods/add-to-collection'),
     removeFromCollection: require('./methods/remove-from-collection'),
     replaceCollection: require('./methods/replace-collection'),

diff --git a/lib/waterline/methods/archive-one.js b/lib/waterline/methods/archive-one.js
@@ -1 +1,2 @@
+module.exports = function(){ throw new Error('TODO'); };
 // TODO
diff --git a/lib/waterline/methods/destroy-one.js b/lib/waterline/methods/destroy-one.js
@@ -1 +1,2 @@
+module.exports = function(){ throw new Error('TODO'); };
 // TODO
diff --git a/lib/waterline/methods/find-one.js b/lib/waterline/methods/find-one.js
@@ -272,14 +272,14 @@ module.exports = function findOne( /* criteria?, populates?, explicitCbMaybe?, m
           if (populatedRecords.length > 1) {
             return done(new Error(
               'More than one matching record found for `.findOne()`:\n'+
-              '```\n'+
+              '···\n'+
               _.pluck(populatedRecords, WLModel.primaryKey)+'\n'+
-              '```\n'+
+              '···\n'+
               '\n'+
               'Criteria used:\n'+
-              '```\n'+
+              '···\n'+
               util.inspect(query.criteria,{depth:5})+''+
-              '```'
+              '···'
             ));
           }//-•
 

diff --git a/lib/waterline/methods/update-one.js b/lib/waterline/methods/update-one.js
@@ -2,6 +2,7 @@
  * Module dependencies
  */
 
+var util = require('util');
 var _ = require('@sailshq/lodash');
 var flaverr = require('flaverr');
 var parley = require('parley');
@@ -148,30 +149,48 @@ module.exports = function updateOne(criteria, valuesToSet, explicitCbMaybe, meta
         }
       }
 
-      // Build a modified shallow clone of the originally-provided `meta` from
-      // userland, but that also has `fetch: true` and the private/experimental
-      // flag, `skipEncryption: true`.  For context on the bit about encryption,
-      // see: https://github.com/balderdashy/sails/issues/4302#issuecomment-363883885
-      // > PLEASE DO NOT RELY ON `skipEncryption` IN YOUR OWN CODE- IT COULD CHANGE
-      // > AT ANY TIME AND BREAK YOUR APP OR PLUGIN!
-      var modifiedMetaForUpdate = _.extend({}, query.meta || {}, {
-        fetch: true,
-        skipEncryption: true
-      });
-
       // Do a .count() to ensure that there are ≤1 matching records.
       // FUTURE: Make this transactional, if supported by the underlying adapter.
-      // TODO
-
-      // Note that we always get `affectedRecords` here because "fetch" is enabled.
-      WLModel.update(query.criteria, query.valuesToSet, function _afterPotentiallyFinding(err, affectedRecords) {
+      var modifiedCriteriaForCount = _.omit(query.criteria, ['select', 'omit', 'limit', 'skip', 'sort']);
+      WLModel.count(modifiedCriteriaForCount, function _afterCounting(err, total) {
         if (err) {
           return done(err);
         }
 
-        return done(undefined, affectedRecords[0]);
-
-      }, modifiedMetaForUpdate);//_∏_ //</.update()>
+        // If more than one matching record was found, then consider this an error.
+        if (total > 1) {
+          return done(new Error(
+            'Preventing `.'+query.method+'()`: found too many ('+total+') matching records.\n'+
+            '\n'+
+            'Criteria used:\n'+
+            '···\n'+
+            util.inspect(modifiedCriteriaForCount,{depth:5})+''+
+            '···'
+          ));
+        }//-•
+
+        // Build a modified shallow clone of the originally-provided `meta` from
+        // userland, but that also has `fetch: true` and the private/experimental
+        // flag, `skipEncryption: true`.  For context on the bit about encryption,
+        // see: https://github.com/balderdashy/sails/issues/4302#issuecomment-363883885
+        // > PLEASE DO NOT RELY ON `skipEncryption` IN YOUR OWN CODE- IT COULD CHANGE
+        // > AT ANY TIME AND BREAK YOUR APP OR PLUGIN!
+        var modifiedMetaForUpdate = _.extend({}, query.meta || {}, {
+          fetch: true,
+          skipEncryption: true
+        });
+
+        var modifiedCriteriaForUpdate = _.omit(query.criteria, ['select', 'omit', 'limit', 'skip', 'sort']);
+        WLModel.update(modifiedCriteriaForUpdate, query.valuesToSet, function _afterUpdating(err, affectedRecords) {
+          if (err) {
+            return done(err);
+          }
+
+          // Note that we always get `affectedRecords` here because "fetch" is enabled.
+          return done(undefined, affectedRecords[0]);
+
+        }, modifiedMetaForUpdate);//_∏_  </.update()>
+      }, query.meta);//_∏_  </.count()>
     },
 
 

diff --git a/lib/waterline/utils/query/forge-stage-two-query.js b/lib/waterline/utils/query/forge-stage-two-query.js
@@ -179,8 +179,10 @@ module.exports = function forgeStageTwoQuery(query, orm) {
       case 'findOrCreate':         return [ 'criteria', 'newRecord' ];
 
       case 'update':               return [ 'criteria', 'valuesToSet' ];
+      case 'updateOne':            return [ 'criteria', 'valuesToSet' ];
       case 'destroy':              return [ 'criteria' ];
-      case 'archive':              return [ 'criteria' ];
+      case 'destroyOne':           return [ 'criteria' ];
+      case 'archiveOne':           return [ 'criteria' ];
       case 'addToCollection':      return [ 'targetRecordIds', 'collectionAttrName', 'associatedIds' ];
       case 'removeFromCollection': return [ 'targetRecordIds', 'collectionAttrName', 'associatedIds' ];
       case 'replaceCollection':    return [ 'targetRecordIds', 'collectionAttrName', 'associatedIds' ];
@@ -397,12 +399,13 @@ module.exports = function forgeStageTwoQuery(query, orm) {
         );
       }
 
-      // If this is a findOrCreate query, make sure that the `fetch` meta key hasn't
-      // been explicitly set (because that wouldn't make any sense).
-      if (query.method === 'findOrCreate') {
+      // If this is a findOrCreate/updateOne/destroyOne/archiveOne query,
+      // make sure that the `fetch` meta key hasn't been explicitly set
+      // (because that wouldn't make any sense).
+      if (_.contains(['findOrCreate', 'updateOne', 'destroyOne', 'archiveOne'], query.method)) {
         throw buildUsageError(
           'E_INVALID_META',
-          'The `fetch` meta key should not be provided when calling .findOrCreate().  '+
+          'The `fetch` meta key should not be provided when calling .'+query.method+'().  '+
           'This method always behaves as if `fetch` was set to `true`, and, if successful, '+
           'guarantees a result.',
           query.using
@@ -642,9 +645,10 @@ module.exports = function forgeStageTwoQuery(query, orm) {
     //  ┌─    ┬┌─┐  ┌┬┐┬ ┬┬┌─┐  ┬┌─┐  ┌─┐  ╔═╗╦╔╗╔╔╦╗  ╔═╗╔╗╔╔═╗  ┌─┐ ┬ ┬┌─┐┬─┐┬ ┬    ─┐
     //  │───  │├┤    │ ├─┤│└─┐  │└─┐  ├─┤  ╠╣ ║║║║ ║║  ║ ║║║║║╣   │─┼┐│ │├┤ ├┬┘└┬┘  ───│
     //  └─    ┴└     ┴ ┴ ┴┴└─┘  ┴└─┘  ┴ ┴  ╚  ╩╝╚╝═╩╝  ╚═╝╝╚╝╚═╝  └─┘└└─┘└─┘┴└─ ┴     ─┘
-    // If this is a `findOne` query, then if `where` clause is not defined, or if it is `{}`,
-    // then fail with a usage error for clarity.
-    if (query.method === 'findOne' && _.isEqual(query.criteria.where, {})) {
+    // If this is a `findOne`/`updateOne`/`destroyOne`/`archiveOne` query,
+    // and the `where` clause is not defined, or if it is `{}`, then fail
+    // with a usage error (for clarity's sake).
+    if (_.contains(['findOne','updateOne','destroyOne','archiveOne'], query.method) && _.isEqual(query.criteria.where, {})) {
 
       throw buildUsageError('E_INVALID_CRITERIA', 'Cannot `findOne()` without specifying a more specific `where` clause.  (If you want to work around this, use `.find().limit(1)`.)', query.using);
 

diff --git a/lib/waterline/utils/query/get-query-modifier-methods.js b/lib/waterline/utils/query/get-query-modifier-methods.js
@@ -717,10 +717,10 @@ module.exports = function getQueryModifierMethods(category){
     case 'findOrCreate':         _.extend(queryMethods, FILTER_Q_METHODS, SET_Q_METHODS, DECRYPT_Q_METHODS); break;
 
     case 'update':               _.extend(queryMethods, FILTER_Q_METHODS, SET_Q_METHODS, FETCH_Q_METHODS, DECRYPT_Q_METHODS); break;
-    case 'destroy':              _.extend(queryMethods, FILTER_Q_METHODS, FETCH_Q_METHODS, DECRYPT_Q_METHODS); break;
-    case 'archive':              _.extend(queryMethods, FILTER_Q_METHODS, FETCH_Q_METHODS, DECRYPT_Q_METHODS); break;
     case 'updateOne':            _.extend(queryMethods, FILTER_Q_METHODS, SET_Q_METHODS, DECRYPT_Q_METHODS); break;
+    case 'destroy':              _.extend(queryMethods, FILTER_Q_METHODS, FETCH_Q_METHODS, DECRYPT_Q_METHODS); break;
     case 'destroyOne':           _.extend(queryMethods, FILTER_Q_METHODS, DECRYPT_Q_METHODS); break;
+    case 'archive':              _.extend(queryMethods, FILTER_Q_METHODS, FETCH_Q_METHODS, DECRYPT_Q_METHODS); break;
     case 'archiveOne':           _.extend(queryMethods, FILTER_Q_METHODS, DECRYPT_Q_METHODS); break;
 
     case 'addToCollection':      _.extend(queryMethods, COLLECTION_Q_METHODS); break;
Original file line number	Diff line number	Diff line change
		@@ -1 +1,2 @@
		module.exports = function(){ throw new Error('TODO'); };
		// TODO