Automattic / mongoose

MongoDB object modeling designed to work in an asynchronous environment.
https://mongoosejs.com
MIT License
27.01k stars 3.85k forks source link

Overwrite document on update #1081

Closed alexlatchford closed 11 years ago

alexlatchford commented 12 years ago

Hi,

I know by design $set is injected to prevent overwriting but this causing some problems for me and I'm requesting either a work around or better an option flag to (which defaults to $set) to turn this off please :)

http://mongoosejs.com/docs/api.html#model_Model-findByIdAndUpdate https://github.com/LearnBoost/mongoose/blob/master/lib/query.js#L1546

My particular use case is that I have a JSON structure coming in and in it, it has a nested list of image media this works great when adding/removing 1, 2, 3, 100 items to this list.. But not when going from 1 to 0 items (i.e I don't supply that key anymore).. It won't remove the last one because it doesn't allow me to overwrite and I cannot seem to work out how to do this from the API docs.

I've worked around this for now by calling findById, manually setting the length of the array to zero and then calling save().. This works but is really suboptimal for me and is a short term fix because there is a real possibility of concurrency issues on this particular project so by not having this operation atomic, you know what'll happen....

Any help would be much appreciated!

Thanks, Alex

PS. Other than this, loving using this project! PPS. I'm on v3.0.3

aheckmann commented 12 years ago

thanks for the report. would you mind posting an example?

alexlatchford commented 12 years ago

Yeah sure, the code I'm working on currently is delivering advert playlists to cinemas.. We get multiple playlist feeds coming in from different upstream sources via multiple cron jobs and then these trigger a job per playlist to actually do the outbound delivery down to individual sites. Fairly easy, no concurrency problems here yet! But we also have another cron job that rounds up playlists that need to be removed (i.e. we don't get them from the feeds any more but they're still sitting in the cinemas themselves) and this is essentially continually running so might occur in between the read and the write of the scenario listed above, (slim but I'd rather be sure) and this job essentially just updates the state of each playlist in this central location. Strange as it may seem we need 0 items in a playlist because often they'll send down blank playlists as placeholders for individual cinemas to place local advertising in.

Another scenario I've found this to be a problem is when trying to do a PUT request with raw form data, if the nested list has no items then no key is sent from the form!

Schema example, let me know if you need more, it's slimmed down (syntax may not be 100%).. The playlist attribute is not actually required in this example to my understanding (not sure if there is a way to enforce a minOccurs/maxOccurs, would be useful to know if there is a way), however if you supply the full schema (without the playlist attr) the playlist will remain there because it always does $set and not an overwrite..

var S = new mongoose.Schema({
    start_date      : { type: Date, required: true },
    end_date        : { type: Date, required: true },
    site            : { type: mongoose.Schema.ObjectId, ref: 'Site', required: true },
    screen      : { type: mongoose.Schema.ObjectId, ref: 'Site.Screen' },
    film_release    : { type: mongoose.Schema.ObjectId, ref: 'FilmRelease', required: true },
    playlist        : [{
        name    : {type: String, required: true},
        rating  : {type: String, required: true},
        cpl_uuid: {type: String, required: true}
    }],
    block_number    : { type: Number, required: true },
    source          : {
        id      : { type: Number, required: true },
        name    : { type: String, required: true, enum: [...] }
    },
    pack_uuid       : { type: String, sparse:true, unique:true },
    state           : { type: String, required: true, enum: ['dirty', 'sending', 'sent', 'needs_deleting', 'deleted'] }
});
var M = mongoose.model('AdPack', S);
aheckmann commented 12 years ago

ok, please give an example of the update you are executing.

alexlatchford commented 12 years ago

Apologies, existing data (stored in mongo, this is what I'm trying to update):

{
    start_date      : '2012-08-31T12:00:00Z',
    end_date        : '2012-08-31T15:00:00Z',
    site            : 'some ObjectId',
    screen          : 'some ObjectId',
    film_release    : 'some ObjectId',
    playlist        : [{
        name    : 'MICROSOFT INTERNET EXPLORER "A BEAUTIFUL WEB FY13 (V2)" 60"',
        rating  : 'U',
        cpl_uuid: 'ef18a134-df8c-4b02-9812-76c76d9a0db8'
    },
    {
        name    : 'HARIBO STAR MIX "JUST TOO GOOD" 30"',
        rating  : 'U',
        cpl_uuid: '72623f7f-8d39-4ab0-af66-d9acac3c8181'
    }]
}

What needs to be overwritten, this example the times have changed and no longer has any items in the playlist:

{
    start_date      : '2012-08-31T12:30:00Z',
    end_date        : '2012-08-31T15:30:00Z',
    site            : 'some ObjectId',
    screen          : 'some ObjectId',
    film_release    : 'some ObjectId',
}

The example below works fine, it'll update the playlist to add the 3rd item, but the above one won't because of $set usage.

{
    start_date      : '2012-08-31T12:30:00Z',
    end_date        : '2012-08-31T15:30:00Z',
    site            : 'some ObjectId',
    screen          : 'some ObjectId',
    film_release    : 'some ObjectId',
    playlist        : [{
        name    : 'MICROSOFT INTERNET EXPLORER "A BEAUTIFUL WEB FY13 (V2)" 60"',
        rating  : 'U',
        cpl_uuid: 'ef18a134-df8c-4b02-9812-76c76d9a0db8'
    },
    {
        name    : 'HARIBO STAR MIX "JUST TOO GOOD" 30"',
        rating  : 'U',
        cpl_uuid: '72623f7f-8d39-4ab0-af66-d9acac3c8181'
    },
    {
        name    : 'DCM CLOSING IDENT 2011 "CINEMA 2011 REVISED AUDIO" 10"',
        rating  : 'U',
        cpl_uuid: '9ef06738-bc35-417c-ab01-42626f47ffeb'
    }]
}

The actual code I'm using to do this is where current_pack is any of the JSON structures above:

models.Pack.findOne({
    'block_number': current_pack.block_number,
    'source.id': current_pack.source.id,
    'source.name': source_name
}, 
function(err, doc) {
    if (err) return cb(err);

    var docKeys = doc['_doc'];
    for (var key in docKeys) {
        if (!docKeys.hasOwnProperty(key) || key === '_id') continue;
        doc[key] = current_pack[key]; //Set any keys to undefined which have not been passed in the body (e.g. empty arrays)
    }
    doc.save(function (err) {
        if (err) return cb(err);
        res.send({success:true, _id:doc.id});
    });
});

Ideally it should be something like I envisage, this would keep it atomic..

models.Pack.findOneAndUpdate({
    'block_number': current_pack.block_number,
    'source.id': current_pack.source.id,
    'source.name': source_name
},
current_pack,
{upsert: true, overwrite: true}, <--------
function(err, data) {
    if(err) {
        return callback('_create_pack: The pack could not be created.. Err: ' + err + ', Pack: ' + util.inspect(current_pack, true, null), null);
    } else {
        callback(null, data._id);
    }
});
alexlatchford commented 12 years ago

While I'm here is there a way to specify min/maxOccurs in a nested list?

Cheers Aaron :)

temiyemi commented 12 years ago

Not sure I fully understood you though but I think I'd do something like this using _:

    function empty(params) {
      return _.any(_.values(params), function (v) {
         return v == ''
      })
    }

    var params = current_pack; // or req.body.anything if it comes via post/put

    var playlist = _.filter(params.playlist, function (item) {
       return (typeof item == 'object' && !empty(item))
    });

    _.extend(params, { playlist:playlist });

    Pack.update({ 
        block_number : params.block_number, 
        'source.id':params.source.id, 
        'source.name':params.source.name,
        $atomic:true // this is what makes it atomic
      }, 
      { $set : params }, 
      {}, 
      function (err, data) {
        if (!err) callback(null, data.id);
        else callback('Pack not saved or created.');
      }
    );

That's what I use if I need to $set an empty [] via update.

Talking about min/max, assuming you wish to set that on the rating field in your playlist schema:

     ...
     playlist:[{
       ...
       rating: { type: Number, min:0, max:5, required:true }, // min/max apply only to Number not String
       ...
     }],
     ...
aheckmann commented 12 years ago

ok, i see what you mean with the overwrite. we sort of had support a loooong time ago. i like the 'overwrite' option. we'll get this fixed.

ebensing commented 11 years ago

This was added when mquery was pulled in PR #1562