Olavo Alexandrino
Olavo Alexandrino

Reputation: 75

How to concatenate a string that is created over two callback calls via Mongoose in Node.js?

I have two models that are related to each other. If a mongo document for the first model is successful deleted I have to delete its parent document.

An exception can be raised at the time of trying to execute the second deletion or not. With errors or without them I would like to concatenate the first and the second messages, but the scope of "message" variable inside the "findOneAndDelete" is different of its parent.

How to accomplish this requirement?

The code snippet below works fine with the exception of only the main message is delivered.

    var id = req.params.id;
    var valid = mongoose.Types.ObjectId.isValid(id);

    if (valid) {

        MyModel
        .findByIdAndDelete({"_id": id})
        .exec(function (err, result) {
            var message = "Item has been removed successful.";
            if (!result) {
                message = "Error at deleting an about item.";
            } else {

                // finding and deleting related about image item
                var query = { "id": id };

                SecondModel
                    .findOneAndDelete(query).exec(function (err, result) {

                    // deletion is OK, but the concatenation does not work
                    // how to return this "message" scope to its parent?

                    message += " Image item has been removed successful.";
                    if (!result) {
                        message += " Error trying to remove about image item.";
                    }

                });

            }

            // the concatenation should be returned here

            responseUtilities.sendJsonResponse(res, err, {"message": message});
        });
    } else {
        responseUtilities.sendJsonResponse(res, false, { "message": "Item id is not valid." });
    }



Upvotes: 1

Views: 275

Answers (3)

danh
danh

Reputation: 62676

Glad you found an answer. Here's a handful of structure and style ideas that will make this code easier to read (by you in the future or by a colleague) and make it easier to debug and extend. Start by building small, promise-returning functions for the two async operations.

function deleteAboutWithId(id) {
  return new Promise((resolve, reject) => {
    AboutModel.findByIdAndDelete({"_id": id}).exec(function (err, result) {
      err ? reject(err) : resolve(result)
    })
  })
}

function deleteImageWithAboutId(id) {
  return new Promise((resolve, reject) => {
    AboutImageModel.findOneAndDelete({ aboutId: id }).exec(function (err, result) {
      err ? reject(err) : resolve(result)
    })
  })
}

A nice benefit of these is that they can be unit tested, reused, generalized, etc. The other benefit is that using them makes your business logic is clear and concise. The code reads the way you'd describe the function to a colleague...

  • Delete the item
  • Delete the image
  • If both succeed, answer a compound success message
  • If either fails, answer a (possibly) compound error message

The public function:

const msgIdError = 'About item id is not valid.'
const msgItemSuccess = 'About item has been removed successful.'
const msgImageSuccess = ' Its image item has also been removed successful.'
const msgItemError = 'Error at deleting an about item.'
const msgImageError = ' Error trying to remove about image item.'

module.exports.delete = function (req, res) {
  const idAbout = req.params.idAbout
  if (!mongoose.Types.ObjectId.isValid(idAbout)) {
    responseUtilities.sendJsonResponse(res, false, { "message": msgIdError })
  }
  let message = ''
  deleteAboutWithId(idAbout).then(() => {
    message += msgItemSuccess
    return deleteImageWithAboutId(idAbout)
  }).then(() => {
    message += msgImageSuccess
    responseUtilities.sendJsonResponse(res, err, { "message": message })
  }).catch(err => {
    // if message is empty, the first block threw the error
    message += message.length ? msgImageError : msgItemError
    responseUtilities.sendJsonResponse(res, err, { "message": message });
  })
}

A couple quick notes: (1) it's nice to get the strings out of the way. Those might someday be kept in config, translated to other languages, etc. (2) It cleans up the code a little to check for invalid inputs at the top and bail out at the top if the inputs are invalid.

Upvotes: 1

Oluwafemi Sule
Oluwafemi Sule

Reputation: 38982

Query.exec receives a callback that is processed later.

In your outer Query.exec call when there is no results returned, you send a response. Otherwise, you perform another Query.exec that processes another callback which returns no response.

A quick fix would be to send a response in the inner Query.exec.

This is a sequence of operations is performed in a chain order and can be better coordinated using promises. For example,

const message = (prefix='') => msg => `${prefix} ${msg}`
const asyncChain = (outer, inner) => { 
  return outer
  .then(message())
  .then(outerResult => inner
      .then(message(outerResult))
      .catch(message(outerResult))
  )
  .catch(message())
  .then(console.log);
}

asyncChain(Promise.resolve('outer1✅'), Promise.resolve('inner1✅'))
// outer1✅ inner1✅

asyncChain(Promise.resolve('outer2✅'), Promise.reject('inner2❌'))
// outer2✅ inner2❌

asyncChain(Promise.reject('outer3❌'), Promise.resolve('inner3✅'))
// outer3❌

Taking knowledge that Query.exec returns a Promise object, one can coordinate the query operations as follows:

MyModel.findByIdAndDelete({ _id: id })
  .exec()
  .then(function() {
    return "Item has been removed successful.";
  })
  .then(function(deleteItemSuccessResult) {
    return SecondModel.findOneAndDelete(query)
      .exec()
      .then(function() {
        return (
          deleteItemSuccessResult + " Image item has been removed successful."
        );
      })
      .catch(function() {
        return (
          deleteItemSuccessResult + " Error trying to remove about image item."
        );
      });
  })
  .catch(function() {
    return "Error trying to remove item";
  })
  .then(function(message) {
    responseUtilities.sendJsonResponse(res, err, { message: message });
  });

Upvotes: 0

Olavo Alexandrino
Olavo Alexandrino

Reputation: 75

I have found myself a solution to this requirement but I am not sure that the approach that I followed is the best way to do so. So, I have implemented a Promise and I call an outside function, then I am sure that it has finished. Thus, I can now concatenate the message. Maybe there may be a more elegant version of this solution. If anyone could review I wonder.

module.exports.delete = function (req, res) {

    var idAbout = req.params.idAbout;
    var valid = mongoose.Types.ObjectId.isValid(idAbout);

    if (valid) {

        var message;

        AboutModel
        .findByIdAndDelete({"_id": idAbout})
        .exec(function (err, result) {

            message = "About item has been removed successful.";

            if (!result) {
                message = "Error at deleting an about item.";
            } else {

                new Promise((resolve, reject) => {
                    deleteImage(idAbout);
                    resolve();
                }).then(() => {
                    message += " Its image item has also been removed successful.";
                    responseUtilities.sendJsonResponse(res, err, { "message": message });
                }).catch(
                    error => {
                        message += " Error trying to remove about image item. ";
                        message += error.message;
                        responseUtilities.sendJsonResponse(res, err, { "message": message });
                    }
                );
            }

        });
    } else {
        responseUtilities.sendJsonResponse(res, false, { "message": "About item id is not valid." });
    }
};

// outside function

function deleteImage(idAbout) {
    var query = { "aboutId": idAbout };
    AboutImageModel.findOneAndDelete(query).exec(function (err, result) {});
}

Upvotes: 0

Related Questions