Welcome to OGeek Q&A Community for programmer and developer-Open, Learning and Share
Welcome To Ask or Share your Answers For Others

Categories

0 votes
449 views
in Technique[技术] by (71.8m points)

node.js - Upsert and $inc Sub-document in Array

The following schema is intended to record total views and views for a very specific day only.

const usersSchema = new Schema({
    totalProductsViews: {type: Number, default: 0},

    productsViewsStatistics: [{
        day: {type: String, default: new Date().toISOString().slice(0, 10), unique: true},
        count: {type: Number, default: 0}
    }],
});

So today views will be stored in another subdocument different from yesterday. To implement this I tried to use upsert so as subdocument will be created each day when product is viewed and counts will be incremented and recorded based on a particular day. I tried to use the following function but seems not to work the way I intended.

usersSchema.statics.increaseProductsViews = async function (id) {
    //Based on day only.
    const todayDate = new Date().toISOString().slice(0, 10);

    const result = await this.findByIdAndUpdate(id, {
            $inc: {
                totalProductsViews: 1,
                'productsViewsStatistics.$[sub].count': 1
            },
        },
        {
            upsert: true,
            arrayFilters: [{'sub.day': todayDate}],
            new: true
        });
    console.log(result);
    return result;
};

What do I miss to get the functionality I want? Any help will be appreciated.

See Question&Answers more detail:os

与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
Welcome To Ask or Share your Answers For Others

1 Reply

0 votes
by (71.8m points)

What you are trying to do here actually requires you to understand some concepts you may not have grasped yet. The two primary ones being:

  • You cannot use any positional update as part of an upsert since it requires data to be present

  • Adding items into arrays mixed with "upsert" is generally a problem that you cannot do in a single statement.

It's a little unclear if "upsert" is your actual intention anyway or if you just presumed that was what you had to add in order to get your statement to work. It does complicate things if that is your intent, even if it's unlikely give the finByIdAndUpdate() usage which would imply you were actually expecting the "document" to be always present.

At any rate, it's clear you actually expect to "Update the array element when found, OR insert a new array element where not found". This is actually a two write process, and three when you consider the "upsert" case as well.

For this, you actually need to invoke the statements via bulkWrite():

usersSchema.statics.increaseProductsViews = async function (_id) {
  //Based on day only.
  const todayDate = new Date().toISOString().slice(0, 10);

  await this.bulkWrite([
    // Try to match an existing element and update it ( do NOT upsert )
    {
      "updateOne": {
        "filter": { _id, "productViewStatistics.day": todayDate },
        "update": {
          "$inc": {
            "totalProductsViews": 1,
            "productViewStatistics.$.count": 1
          }
        }
      }
    },

    // Try to $push where the element is not there but document is - ( do NOT upsert )
    {
      "updateOne": {
        "filter": { _id, "productViewStatistics.day": { "$ne": todayDate } },
        "update": {
          "$inc": { "totalProductViews": 1 },
          "$push": { "productViewStatistics": { "day": todayDate, "count": 1 } }
        }
      }
    },

    // Finally attempt upsert where the "document" was not there at all,
    // only if you actually mean it - so optional
    {
      "updateOne": {
        "filter": { _id },
        "update": {
          "$setOnInsert": {
            "totalProductViews": 1,
            "productViewStatistics": [{ "day": todayDate, "count": 1 }]
          }
        }
    }
  ])

  // return the modified document if you really must
  return this.findById(_id); // Not atomic, but the lesser of all evils
}

So there's a real good reason here why the positional filtered [<identifier>] operator does not apply here. The main good reason is the intended purpose is to update multiple matching array elements, and you only ever want to update one. This actually has a specific operator in the positional $ operator which does exactly that. It's condition however must be included within the query predicate ( "filter" property in UpdateOne statements ) just as demonstrated in the first two statements of the bulkWrite() above.

So the main problems with using positional filtered [<identifier>] are that just as the first two statements show, you cannot actually alternate between the $inc or $push as would depend on if the document actually contained an array entry for the day. All that will happen is at best no update will be applied when the current day is not matched by the expression in arrayFilters.

The at worst case is an actual "upsert" will throw an error due to MongoDB not being able to decipher the "path name" from the statement, and of course you simply cannot $inc something that does not exist as a "new" array element. That needs a $push.

That leaves you with the mechanic that you also cannot do both the $inc and $push within a single statement. MongoDB will error that you are attempting to "modify the same path" as an illegal operation. Much the same applies to $setOnInsert since whilst that operator only applies to "upsert" operations, it does not preclude the other operations from happening.

Thus the logical steps fall back to what the comments in the code also describe:

  1. Attempt to match where the document contains an existing array element, then update that element. Using $inc in this case

  2. Attempt to match where the document exists but the array element is not present and then $push a new element for the given day with the default count, updating other elements appropriately

  3. IF you actually did intend to upsert documents ( not array elements, because that's the above steps ) then finally actually attempt an upsert creating new properties including a new array.

Finally there is the issue of the bulkWrite(). Whilst this is a single request to the server with a single response, it still is effectively three ( or two if that's all you need ) operations. There is no way around that and it is better than issuing chained separate requests using findByIdAndUpdate() or even updateOne().

Of course the main operational difference from the perspective of code you attempted to implement is that method does not return the modified document. There is no way to get a "document response" from any "Bulk" operation at all.

As such the actual "bulk" process will only ever modify a document with one of the three statements submitted based on the presented logic and most importantly the order of those statements, which is important. But if you actually wanted to "return the document" after modification then the only way to do that is with a separate request to fetch the document.

The only caveat here is that there is the small possibility that other modifications could have occurred to the document other than the "array upsert" since the read and update are separated. There really is no way around that, without possibly "chaining" three separate requests to the server and then deciding which "response document" actually applied the update you wanted to achieve.

So with that context it's generally considered the lesser of evils to do the read separately. It's not ideal, but it's the best option available from a bad bunch.


As a final note, I would strongly suggest actually storing the the day property as a BSON Date instead of as a string. It actually takes less bytes to store and is far more useful in that form. As such the following constructor is probably the clearest and least hacky:

 const todayDate = new Date(new Date().setUTCHours(0,0,0,0))

与恶龙缠斗过久,自身亦成为恶龙;凝视深渊过久,深渊将回以凝视…
OGeek|极客中国-欢迎来到极客的世界,一个免费开放的程序员编程交流平台!开放,进步,分享!让技术改变生活,让极客改变未来! Welcome to OGeek Q&A Community for programmer and developer-Open, Learning and Share
Click Here to Ask a Question

...