Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overwrite document on update #1081

Closed
alexlatchford opened this issue Aug 28, 2012 · 8 comments
Closed

Overwrite document on update #1081

alexlatchford opened this issue Aug 28, 2012 · 8 comments
Labels
new feature This change adds new functionality, like a new method or class
Milestone

Comments

@alexlatchford
Copy link

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
Copy link
Collaborator

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

@alexlatchford
Copy link
Author

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
Copy link
Collaborator

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

@alexlatchford
Copy link
Author

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
Copy link
Author

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

Cheers Aaron :)

@temiyemi
Copy link

temiyemi commented Sep 7, 2012

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
Copy link
Collaborator

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
Copy link
Contributor

ebensing commented Aug 1, 2013

This was added when mquery was pulled in PR #1562

@ebensing ebensing closed this as completed Aug 1, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This change adds new functionality, like a new method or class
Projects
None yet
Development

No branches or pull requests

4 participants