Having a denyUpdate() and denyInsert() validators

[ghost]

Since they would be used for many reason, like for example not permitting to change a userId after the first insert that would have been done server side, i think denyUpdate() and denyInsert() could be part of the core astronomy validators. They could look like this :

Astro.createValidator({
    name: 'denyUpdate',
    validate: function(fieldValue, fieldName, options) {
        if(this._id && _.has(this.getModified(), fieldName)) {
            return false;
        }
        return true;
    }
});

What do you think ?

[lukejagodzinski]

I think it's not the role of a validator. It's more like the application logic. However if you want such validator you can always write your own :-). As I said you should rather implement it outside of Astronomy internals.

[ghost]

Hum, i'm not sure i see why it should be outside of a validator, since it's just to validate that a value hasn't changed...Especially if you use like an update directly from the client instead of using a Meteor.methods . But i do see your point as we can implement such things into the Allow/Deny that meteor provides. I just find it easier to directly put it inside the schema.

[lukejagodzinski]

A validator should check a value agains certain rules you specified. Checking if a value had been modified is not checking value itself but it's document's or in this situation field's state. So that's why it should be outside the validator.

Meteor.methods({
  '/update/item': function(doc) {
    if(doc._id && _.has(doc.getModified(), fieldName)) {
      throw new Meteor.Error('Can not change it');
    }
  }
});

In fact, you should do something like this (it works only in the newest version).

Meteor.methods({
  '/update/item': function(doc) {
    if (doc.validate()) {
      doc.save([
        'first',
        'second',
        'third'
      ]);
    }
  }
});

However, I'm thing about some replacement for my and your solution. How is it made in other databases and tools? Do you have some example? How do "mark" field as unchangeable?

Maybe it's a role for a behavior that would use e.preventDefault() in the beforeSet event.

events: {
  beforeSet: function(e) {
    if (this._id && e.field === 'fieldName') {
      e.preventDefault();
    }
  }
}

I have not implemented preventDefault for the set events but I will :)

[ghost]

Well, i do get the idea when using a methods. But if i want to use the save() function client side, then it's not really possible, has it would be possible to override the validation. And in this case, i would have to either implement the idea in my Collection.allow() which i don't really like or check it against a Validators which i'm currently doing with the one on my first post. Now, because of LazyCompensation, it maybe better to actually implement everything with methods directly...

As for others tool and DB, i don't think any DB actually implement this denyUpdate() idea, has it's not really it's role (but i may be wrong, to be checked...). Homewer, in other tools, and most notably to one you know about which is Collection2(@aldeed), there's simply two properties called denyUpdate and denyInsert with a boolean set to false by default, like this:

createdAt: {
    type: Date,
    denyUpdate: true
  }

[lukejagodzinski]

Maybe it should be an extra option in the field definition. I would rather call it updateable. And I don't see any reason for denyInsert property. For what I've read in the simple schema it's like optional. And we have the required property.

Item = Astro.Class({
  name: 'Item',
  collection: Items,
  fields: {
    createdAt: {
      type: 'date',
      updateable: false
    }
  }
});

I will add this to the features list however it won't appear in the next version because even though I have a lot of work and it would post pone the release even more :)

[ghost]

Well, it's definitely a trivial feature anyway, has we already have multiple ways to implement it ourselves. It's sort of a convenience feature, so not having it like on the next version wouldn't be a drama :D .

updateable is perfectly fine for me, it's even more in accordance with the way your seing the feature so...

And i've actually never used the denyInsert() idea, apart from ensuring something like making sure that the updatedAt doesn't get set on insert, which is kind of an non-issue and with your timestamp behaviors, it's just, well, useless... Now there may be some reason behind this idea, i'll try to find out if it could actually be useful in some situation.

[lukejagodzinski]

Ok so we will come back to discuss it when I start implementing it.

And of course if you find any use case for denyInsert then let me know :)

[aldeed]

Someone wanted these in collection2 and I added it. Personally I think it's better to handle all security in a single place, and not in a schema. But there is some gray area, for example preventing createdAt from ever being updated is more of a schema/data integrity thing than it is security. It might make more sense with a name that doesn't imply security, like immutable: true.

[ghost]

You're probably right in my case scenario of forbidding to update an userId field. But i feel it's also convenient to use it this way, and i do feel that in those rather simple but important cases, handling it directly at the source (schema) gets you a clearer view of your data.

[lukejagodzinski]

Yes, the immutable name describes its usage very well. I think that need for the denyInsert was caused by somebody's wrong application logic. I've never seen such thing before but it doesn't mean that I'm right.

[lukejagodzinski]

The immutable property implemented. Will appear in the 1.0 release.