Btukfyl

I have built a simple MERN app where users can rate phone numbers. Users just fill in the phone number, choose rating (1 - 5 star rating), their city & short text. The app has search function with filter & sorting options. It all works good enough ATM but I think it might break when multiple concurrent users use the website because I update the phone number model (mobileSchema) after a rating (messageSchema) has been submitted - using Mongoose middlewares (post hooks).

For example, I need to calculate number of ratings (messagesCount) for phone number. I use Message.countDocuments({ mobile: mobile._id }) for that. However, I also need to update other properties of phone number (mobileSchema - lastMessageDate, globalRating, averageRating) so that operation takes some time. I believe the number of ratings might not be right when 2 users submit rating at the same time - it will increment the number of ratings (messagesCount) by 1 instead of 2.

Is there a better approach? Can a post hook be fired after the previous post hook already finished?

Sample code:

const mobileSchema = new Schema({

    number: { type: String, required: true },

    plan: { type: String, required: true },

    date: { type: Date, default: Date.now, required: true, index: 1 },

    messagesCount: { type: Number, default: 0, index: 1 },

    lastMessageDate: { type: Date, index: 1 },

    // normal mean

    globalRating: { type: Number, default: 0, index: 1 },

    // weighted mean

    averageRating: { type: Number, default: 0, index: 1 }

});



const messageSchema = new Schema({

    comment: { type: String, required: true },

    city: { type: Schema.Types.ObjectId, ref: 'City', required: true, index: 1 },

    rating: { type: Number, required: true, index: 1 },

    date: { type: Date, default: Date.now, required: true, index: 1 },

    mobile: { type: Schema.Types.ObjectId, ref: 'Mobile', required: true },

    user: { type: Schema.Types.ObjectId, ref: 'User', required: true }

});



messageSchema.post('save', function (message, next) {

    const messageModel = this.constructor;

    return updateMobile(messageModel, message, next, 1);

});



const updateMobile = (messageModel, message, next, addMessage) => {

    const { _id } = message.mobile;

    const cityId = message.city._id;

    const lastMessageDate = message.date;

    let mobile;

    hooks.get(Mobile, { _id })

        .then(mobileRes => {

            mobile = mobileRes;

            return Message.countDocuments({ mobile: mobile._id })

        })

        .then(messagesCount => {

            if (messagesCount <= 0) {

                const deleteMobile = Mobile.findOneAndDelete({ _id: mobile._id })

                const deleteSeen = SeenMobile.findOneAndDelete({ mobile: mobile._id, user: message.user._id })

                const cityMobile = updateCityMobile(messageModel, mobile, cityId)

                Promise.all([deleteMobile, deleteSeen, cityMobile])

                    .then(() => {

                        return next();

                    })

                    .catch((err) => {

                        console.log(err);

                        return next();

                    })

            }

            else {

                if (addMessage === -1) lastMessageDate = mobile.lastMessageDate;

                const ratings = hooks.updateGlobalRating(mobile, messageModel)

                    .then(() => hooks.updateAverageRating(mobile, messageModel))

                    .then(() => {

                        return new Promise((resolve, reject) => {

                            mobile.set({

                                messagesCount,

                                lastMessageDate

                            });

                            mobile.save((err, mobile) => {

                                if (err) return reject(err);

                                resolve();

                            });

                        })

                    })

                const cityMobile = updateCityMobile(messageModel, mobile, cityId)

                Promise.all([ratings, cityMobile])

                    .then(([ratings, cityMobile]) => {

                        return next();

                    })

                    .catch(err => console.log(err))

            }

        })

        .catch(err => {

            console.log(err);

        })

}

I think you are always going to run into async issues with your approach. I don't believe you can "synchronize" the hooks; seems to go against everything that is true about MongoDB. However, at a high level, you might have more success grabbing the totals/summaries at run-time, rather than trying to keep them always in sync. For instance, if you need the total number of messages for a given mobile device, why not:

Messages.find({mobile: mobile._id})

and then count the results? That will save you storing the summaries and keeping them updated. However, I also think your current approach could work, but you probably need to scrap the "countDocuments". Something a bit more async friendly, like:

Mobile.aggregation([

    { $match: { _id: mobile._id } },

    { $add: [ "$mobile.messagesCount", 1 ] }

]);

Ultimately I think your design would be strengthened if you stored Messages as an array inside of Mobile, so you can just push the message on it. But to answer the question directly, the aggregation should keep everything tidy.

I found this answer: Locking a document in MongoDB

I will calculate all the values I need (messagesCount, globalRating etc.) in post hook and then I will check if the mobile document has the same __v (version) value during final findOneAndUpdate operation (because this operation locks the document and can increment __v). If it has different __v then I will call the post hook again to ensure it will calculate the right values.

First we need to fix some database structure here

Mobile schema

const mobileSchema = new Schema({

  number: { type: String, required: true },

  plan: { type: String, required: true },

  date: { type: Date, default: Date.now, required: true, index: 1 },

  //messagesCount: { type: Number, default: 0, index: 1 },

  //lastMessageDate: { type: Date, index: 1 },

  // normal mean

  //globalRating: { type: Number, default: 0, index: 1 },

  // weighted mean

  //averageRating: { type: Number, default: 0, index: 1 }

});

Message schema

const messageSchema = new Schema({

  comment: { type: String, required: true },

  city: { type: Schema.Types.ObjectId, ref: 'City', required: true, index: 1 },

  //rating: { type: Number, required: true, index: 1 },

  date: { type: Date, default: Date.now, required: true, index: 1 },

  mobile: { type: Schema.Types.ObjectId, ref: 'Mobile', required: true },

  user: { type: Schema.Types.ObjectId, ref: 'User', required: true }

});

Rating system (take all rating or make them a set)
(numerator & denominator after 100 ratings it is difficult to read every single one) can also check for the mobile

const ratingSchema = new Schema({

  mobile: { type: String, required: true },

  commmentId:{type:String, required: true, index: 1}

  rate: { type: Number required: true,  },

  //rating: { type: Number, required: true, index: 1 },

  timestamp: { type: Date, default: Date.now, required: true, index: 1 }

  denominator:{ type: Number},

  numerator:{type:Number}

});

Thanks