5

This is my MongoDB schema:

var partnerSchema = new mongoose.Schema({
    name: String,
    products: [
        {
            type: mongoose.Schema.Types.ObjectId,
            ref: 'Product'
        }]
});

var productSchema = new mongoose.Schema({
    name: String,
    campaign: [
        {
            type: mongoose.Schema.Types.ObjectId,
            ref: 'Campaign'
        }
    ]
});

var campaignSchema = new mongoose.Schema({
    name: String,
});


module.exports = {
    Partner: mongoose.model('Partner', partnerSchema),
    Product: mongoose.model('Product', productSchema),
    Campaign: mongoose.model('Campaign', campaignSchema)
}

And I wondering how can I remove object from any document taking into account references (maybe should I use somehow populate from mongoose)? For example if I will remove Product then I assume that I will remove also ref ID in Partner and all Campaigns which belong to this Product.

At the moment I removing in this way:

var campSchema = require('../model/camp-schema');

    router.post('/removeProduct', function (req, res) {
            campSchema.Product.findOneAndRemove({ _id: req.body.productId }, function (err, response) {
                if (err) throw err;
                res.json(response);
            });
        });

However in mongo still left references.

DiPix
  • 5,755
  • 15
  • 61
  • 108

1 Answers1

10

You would have to nest your calls to remove the product id from the other model. For instance, in your call to remove the product from the Product collection, you could also make another call to remove the ref from the Partner model within the results callback. Removing the product by default will remove its refs to the Campaign Model.

The following code shows the intuition above:

var campSchema = require('../model/camp-schema');

router.post('/removeProduct', function (req, res) {
    campSchema.Product.findOneAndRemove({ _id: req.body.productId }, function (err, response) {
        if (err) throw err;
        campSchema.Partner.update(
            { "products": req.body.productId },
            { "$pull": { "products": req.body.productId } },
            function (err, res){
                if (err) throw err;
                res.json(res);
            }
        );
    });
});

To remove the associated campaigns then you may need an extra remove operation that takes in the associated campaign id fro a given product id. Consider the following dirty hack which may potentially award you a one-way ticket to callback hell if not careful with the callback nesting:

router.post('/removeProduct', function (req, res) {
    campSchema.Product.findOneAndRemove(
        { _id: req.body.productId }, 
        { new: true },
        function (err, product) {
            if (err) throw err;
            campSchema.Partner.update(
                { "products": req.body.productId },
                { "$pull": { "products": req.body.productId } },
                function (err, res){
                    if (err) throw err;
                    var campaignList = product.campaign
                    campSchema.Campaign.remove({ "_id": { "$in": campaignList } })
                                .exec(function (err, res){
                                    if (err) throw err;
                                    res.json(product);
                                })
                }
            );
        }
    );
});

Although it works, the above potential pitfall can be avoided by using async/await or the async library. But firstly, to give you a better understanding of the using multiple callbacks with the async module, let's illustrate this with an example from Seven Things You Should Stop Doing with Node.js of multiple operations with callbacks to find a parent entity, then find child entities that belong to the parent:

methodA(function(a){
    methodB(function(b){
        methodC(function(c){
            methodD(function(d){
                // Final callback code        
            })
        })
    })
})

With async/await, your calls will be restructured structured as

router.post('/removeProduct', async (req, res) => {
    try {
        const product = await campSchema.Product.findOneAndRemove(
            { _id: req.body.productId }, 
            { new: true }
        )

        await campSchema.Partner.update(
            { "products": req.body.productId },
            { "$pull": { "products": req.body.productId } }
        )

        await campSchema.Campaign.remove({ "_id": { "$in": product.campaign } })

        res.json(product)
    } catch(err) {
        throw err
    }
})

With the async module, you can either use the series method to address the use of callbacks for nesting code of multiple methods which may result in Callback Hell:

Series:

async.series([
    function(callback){
        // code a
        callback(null, 'a')
    },
    function(callback){
        // code b
        callback(null, 'b')
    },
    function(callback){
        // code c
        callback(null, 'c')
    },
    function(callback){
        // code d
        callback(null, 'd')
    }],
    // optional callback
    function(err, results){
        // results is ['a', 'b', 'c', 'd']
        // final callback code
    }
)

Or the waterfall:

async.waterfall([
    function(callback){
        // code a
        callback(null, 'a', 'b')
    },
    function(arg1, arg2, callback){
        // arg1 is equals 'a' and arg2 is 'b'
        // Code c
        callback(null, 'c')
    },
    function(arg1, callback){      
        // arg1 is 'c'
        // code d
        callback(null, 'd');
    }], function (err, result) {
        // result is 'd'    
    }
)

Now going back to your code, using the async waterfall method you could then restructure your code to

router.post('/removeProduct', function (req, res) {
    async.waterfall([
        function (callback) {
            // code a: Remove Product
            campSchema.Product.findOneAndRemove(
                { _id: req.body.productId }, 
                function (err, product) {
                    if (err) callback(err);
                    callback(null, product);
                }
            );
        },

        function (doc, callback) {
            // code b: Remove associated campaigns
            var campaignList = doc.campaign;
            campSchema.Campaign
                .remove({ "_id": { "$in": campaignList } })
                .exec(function (err, res) {
                if (err) callback(err);
                callback(null, doc);
            }
            );
        },

        function (doc, callback) {
            // code c: Remove related partner
            campSchema.Partner.update(
                { "products": doc._id },
                { "$pull": { "products": doc._id } },
                function (err, res) {
                    if (err) callback(err);
                    callback(null, doc);
                }
            );
        }
    ], function (err, result) {
        if (err) throw err;
        res.json(result);  // OUTPUT OK
    });
});
chridam
  • 100,957
  • 23
  • 236
  • 235
  • I understood. But in document `campaign` is still object assigned to removed product. Should I remove him by using next callback? – DiPix Jun 24 '16 at 11:00
  • Oh, you want to also remove the campaign associated with the Product as well? I thought you only meant to remove the product only and its refs from the other models, i.e. its relationships and not ncessarily remove the models that are related to the deleted product? – chridam Jun 24 '16 at 11:09
  • Well, campaign without assigned product is unnecessary. So I shouldn't keep it in document. – DiPix Jun 24 '16 at 11:19
  • From the design it looks unnecessary, why don't you just embed the schema within the `Product` model without necessarily creating a separate model of it's own? This is because from your requirements, `Campaign` is dependant on the `Product` model directly and deleting the product would also mean deleting its campaign, so embedding it makes more sense than keeping it as a separate rerefrenced model. – chridam Jun 24 '16 at 11:30
  • Because it's easy to add and edit objects if are in separate documents. That's why I split them. As you see these documents are nested. – DiPix Jun 24 '16 at 11:34
  • I think, I will just add function in callback to remove all object from campaign document with `id = id from product.campaign` – DiPix Jun 24 '16 at 11:58
  • So first part of your last edit I should avoiding - because it works but also is a example of "callback hell"? And I should focus to making similarly code by using async.waterfall? Am I right? :) – DiPix Jun 24 '16 at 15:17
  • @DiPix That is correct. Avoid nesting callbacks as this is dangerous practice which can be really hard to debug as your codebase grows. A personal rule of thumb is anything which calls for more than 2 levels of nesting is code smell which needs serious refactoring. – chridam Jun 24 '16 at 15:20
  • now I have time to test your code, and I have an error: `500 (Internal Server Error)` after `async.waterfall([` it means that program never reach first breakpoint at first function - `code a` – DiPix Jun 27 '16 at 06:30
  • 1
    Did you require the async library? i.e. `var async = require('async');`? – chridam Jun 27 '16 at 06:34
  • Visual studio highlight errors in code so I had to remove `if (err) throw err;` from CODE_C, also I changed `if (err) callback(err);` to `if (err) throw err;` at the end. But now I have an error when i'm trying to remove product: `MongoError: Cannot specify both new=true and remove=true; 'remove' always returns the deleted document` - It's a error from CODE_A – DiPix Jun 27 '16 at 06:56
  • So I just removed `{ new: true },` and it looks like now is fine. Check my edit suggestion. – DiPix Jun 27 '16 at 07:06
  • Nice one. Indeed that was the cause of the error; could not test the code unfortunately :P – chridam Jun 27 '16 at 07:33
  • Did you write this code without compilator? Niceee! – DiPix Jun 27 '16 at 08:04
  • I've got one more question. Probably the last one, since I almost done part with MongoDB in my project. If you have some time then check this: http://stackoverflow.com/questions/38072792/how-to-get-data-from-mongodb-to-simple-array-using-node-js-and-mongoose – DiPix Jun 28 '16 at 10:23