From cc11e0294302bb1df9053ab606a9619817bc0bf9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Loix?= <sabee77@gmail.com>
Date: Tue, 10 Sep 2019 09:48:21 +0200
Subject: [PATCH] fix(model): throw NOT_FOUND error when trying to update a
 Model that does not exist (#181)

fix #164
---
 lib/model.js              | 36 ++++++++++++------------------------
 test/integration/model.js | 34 +++++++++++++++++++++-------------
 test/model-test.js        |  5 +++--
 3 files changed, 36 insertions(+), 39 deletions(-)

diff --git a/lib/model.js b/lib/model.js
index f9dd8a9..60ebf16 100755
--- a/lib/model.js
+++ b/lib/model.js
@@ -210,7 +210,6 @@ class Model extends Entity {
         const _this = this;
 
         let entityUpdated;
-        let error = {};
 
         const key = this.key(id, ancestors, namespace);
         const replace = options && options.replace === true;
@@ -223,7 +222,8 @@ class Model extends Entity {
          */
         if (replace) {
             return saveEntity({ key, data })
-                .then(onEntityUpdated, onUpdateError);
+                .then(onEntityUpdated)
+                .catch(onUpdateError);
         }
 
         if (typeof transaction === 'undefined' || transaction === null) {
@@ -232,22 +232,21 @@ class Model extends Entity {
             return transaction
                 .run()
                 .then(getAndUpdate)
-                .catch(onTransactionError);
+                .catch(onUpdateError);
         }
 
         if (transaction.constructor.name !== 'Transaction') {
             return Promise.reject(new Error('Transaction needs to be a gcloud Transaction'));
         }
 
-        return getAndUpdate()
-            .catch(onTransactionError);
+        return getAndUpdate();
 
         // ---------------------------------------------------------
 
         function getAndUpdate() {
             return getEntity()
                 .then(saveEntity)
-                .then(onEntityUpdated, onUpdateError);
+                .then(onEntityUpdated);
         }
 
         function getEntity() {
@@ -257,11 +256,10 @@ class Model extends Entity {
                     const entity = getData[0];
 
                     if (typeof entity === 'undefined') {
-                        error = new GstoreError(
+                        throw (new GstoreError(
                             errorCodes.ERR_ENTITY_NOT_FOUND,
                             `Entity { ${id.toString()} } to update not found`
-                        );
-                        throw (error);
+                        ));
                     }
 
                     extend(false, entity, data);
@@ -272,10 +270,6 @@ class Model extends Entity {
                     };
 
                     return result;
-                })
-                .catch(err => {
-                    error = err;
-                    return err;
                 });
         }
 
@@ -319,14 +313,16 @@ class Model extends Entity {
         }
 
         function onUpdateError(err) {
-            error = err;
+            const error = Array.isArray(err) ? err[0] : err;
             if (internalTransaction) {
                 // If we created the Transaction instance internally for the update, we rollback it
                 // otherwise we leave the rollback() call to the transaction creator
-                return transaction.rollback().then(() => onTransactionError([err]));
+                return transaction.rollback().then(() => {
+                    throw error;
+                });
             }
 
-            return onTransactionError([err]);
+            throw error;
         }
 
         function onTransactionSuccess() {
@@ -349,14 +345,6 @@ class Model extends Entity {
 
             return entityUpdated;
         }
-
-        function onTransactionError(transactionError = {}) {
-            const apiResponse = transactionError && Array.isArray(transactionError)
-                ? transactionError[0]
-                : transactionError;
-            extend(apiResponse, transactionError);
-            throw apiResponse;
-        }
     }
 
     static delete(id, ancestors, namespace, transaction, key, options = {}) {
diff --git a/test/integration/model.js b/test/integration/model.js
index 8e1e0d8..6a9e4e9 100644
--- a/test/integration/model.js
+++ b/test/integration/model.js
@@ -258,20 +258,20 @@ describe('Model (Integration Tests)', () => {
 
     describe('update()', () => {
         describe('transaction()', () => {
-            it('should update entity inside a transaction', () => {
-                const userSchema = new Schema({
-                    name: { joi: Joi.string().required() },
-                    lastname: { joi: Joi.string() },
-                    password: { joi: Joi.string() },
-                    coins: { joi: Joi.number().integer().min(0) },
-                    email: { joi: Joi.string().email() },
-                    createdAt: { joi: Joi.date() },
-                    access_token: { joi: [Joi.string(), Joi.number()] },
-                    birthyear: { joi: Joi.number().integer().min(1900).max(2013) },
-                }, { joi: true });
-
-                const User = gstore.model('ModelTestsTransaction-User', userSchema);
+            const userSchema = new Schema({
+                name: { joi: Joi.string().required() },
+                lastname: { joi: Joi.string() },
+                password: { joi: Joi.string() },
+                coins: { joi: Joi.number().integer().min(0) },
+                email: { joi: Joi.string().email() },
+                createdAt: { joi: Joi.date() },
+                access_token: { joi: [Joi.string(), Joi.number()] },
+                birthyear: { joi: Joi.number().integer().min(1900).max(2013) },
+            }, { joi: true });
+
+            const User = gstore.model('ModelTestsTransaction-User', userSchema);
 
+            it('should update entity inside a transaction', () => {
                 function transferCoins(fromUser, toUser, amount) {
                     return new Promise((resolve, reject) => {
                         const transaction = gstore.transaction();
@@ -319,6 +319,14 @@ describe('Model (Integration Tests)', () => {
                         return transferCoins(fromUser, toUser, 1000);
                     });
             });
+
+            it('should throw a 404 Not found when trying to update a non existing entity', done => {
+                User.update(randomName(), { name: 'test' })
+                    .catch(err => {
+                        expect(err.code).equal('ERR_ENTITY_NOT_FOUND');
+                        done();
+                    });
+            });
         });
     });
 
diff --git a/test/model-test.js b/test/model-test.js
index 2a59697..3adad25 100755
--- a/test/model-test.js
+++ b/test/model-test.js
@@ -709,13 +709,14 @@ describe('Model', () => {
             });
         });
 
-        it('should return error if any while saving', () => {
+        it('should return error if any while saving', done => {
             transaction.run.restore();
             const error = { code: 500, message: 'Houston wee need you.' };
             sinon.stub(transaction, 'run').rejects([error]);
 
-            return GstoreModel.update(123).catch(err => {
+            GstoreModel.update(123).catch(err => {
                 expect(err).equal(error);
+                done();
             });
         });