From 3df62444d6406ae5d05023affca77dd07dff6a08 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= <miroslav@strongloop.com>
Date: Thu, 16 Apr 2015 09:03:56 +0200
Subject: [PATCH] Validate model on updateOrCreate (upsert).

Fix the implementation of updateOrCreate (a.k.a. upsert) to validate
the model before calling the connector.

In order to preserve backwards compatibility, validation errors are
only logged via console.warn by default.

The correct behaviour, where validation errors fail the updateOrCreate
operation, can be enabled via new model setting "validateUpsert".
---
 lib/dao.js               | 20 +++++++++++++++-----
 test/validations.test.js | 21 +++++++++++++++++++++
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/lib/dao.js b/lib/dao.js
index 44979050..26c8de9a 100644
--- a/lib/dao.js
+++ b/lib/dao.js
@@ -403,12 +403,22 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data
         Model.applyProperties(update, inst);
         Model = Model.lookupModel(update);
 
-        // FIXME(bajtos) validate the model!
-        // https://github.com/strongloop/loopback-datasource-juggler/issues/262
+        inst.isValid(function(valid) {
+          if (!valid) {
+            // TODO(bajtos) Remove validateUpsert option in v3.0
+            if (Model.settings.validateUpsert) {
+              return cb(new ValidationError(inst), inst);
+            } else {
+              console.warn('Ignoring validation errors in updateOrCreate():');
+              console.warn('  %s', new ValidationError(inst).message);
+              // continue with updateOrCreate
+            }
+          }
 
-        update = removeUndefined(update);
-        self.getDataSource().connector
-          .updateOrCreate(Model.modelName, update, done);
+          update = removeUndefined(update);
+          self.getDataSource().connector
+            .updateOrCreate(Model.modelName, update, done);
+        }, update);
 
         function done(err, data, info) {
           var obj;
diff --git a/test/validations.test.js b/test/validations.test.js
index 9df68d46..d4f7faed 100644
--- a/test/validations.test.js
+++ b/test/validations.test.js
@@ -184,6 +184,27 @@ describe('validations', function () {
         });
       });
 
+      it('should be skipped on upsert by default', function(done) {
+        delete User.validations;
+        User.validatesPresenceOf('name');
+        // It's important to pass an id value, otherwise DAO falls back
+        // to regular create()
+        User.updateOrCreate({ id: 999 }, done);
+      });
+
+      it('should work on upsert when enabled via settings', function(done) {
+        delete User.validations;
+        User.validatesPresenceOf('name');
+        User.settings.validateUpsert = true;
+        // It's important to pass an id value, otherwise DAO falls back
+        // to regular create()
+        User.upsert({ id: 999 }, function(err, u) {
+          if (!err) return done(new Error('Validation should have failed.'));
+          err.should.be.instanceOf(ValidationError);
+          done();
+        });
+      });
+
       it('should return error code', function (done) {
         delete User.validations;
         User.validatesPresenceOf('name');