Meteor-Community-Packages / meteor-collection2

A Meteor package that extends Mongo.Collection to provide support for specifying a schema and then validating against that schema when inserting and updating.
https://packosphere.com/aldeed/collection2
MIT License
1.02k stars 108 forks source link

Empty modifier passes validation, then wipes document #377

Open coagmano opened 6 years ago

coagmano commented 6 years ago

When performing an update with an empty modifier {}, it passes validation and then Meteor treats it as an literal document and replaces the document with the empty one.


I've got a very simple setup:

export const Pages = new Meteor.Collection('pages');
export const pageSchema = new SimpleSchema({
    projectId: SimpleSchema.RegEx.Id,
    name: String,
});
Pages.attachSchema(pageSchema);
Factory.define('page', Pages, {
    projectId: Factory.get('project'),
    name: () => faker.random.word(),
});

To illustrate, I'll create a test document in meteor shell:

> Factory.create('page')
Document {
  _id: 'imerjq2XrTKw8t6Lh',
  projectId: 'bh52bEnkd8nPP7TLj',
  name: 'alarm' }

Tests with modifiers that violate the schema fail:

Pages.update('imerjq2XrTKw8t6Lh', { $unset: { name: 'test' } }) -> throws ValidationError

Tests with literal documents fail validation:

Pages.update('imerjq2XrTKw8t6Lh', {name: 'test'}) -> throws ValidationError

But passing in the empty modifier:

Pages.update('imerjq2XrTKw8t6Lh', {}) -> returns 1

Wipes the document:

> Pages.findOne('imerjq2XrTKw8t6Lh')
Document { _id: 'imerjq2XrTKw8t6Lh' }

Is this a bug?

What's the recommended way of preventing this? Should I extend and overload update to check for empty objects?

aldeed commented 6 years ago

This certainly feels like a bug, but in reality it is the way that Mongo updates work without any validation attached. Still, if you do attach validation, it makes sense that you might want to be warned about this. Not sure exactly what this would look like off the top of my head, but I guess some way to inform it that you explicitly want to allow a whole-document update?