Astronomy module to track direct manipulations on DB/collections. about meteor-astronomy

[lukejagodzinski]

Right now I'm not going to mess with the mechanism of Database Watch because I'm not familiar with it yet.

For now I'm going to focus on collection level events and validation. To solve this problem I have two solution.

VALIDATION:

1. Write a wrapper around all collection methods like insert, update etc.
2. Write deny rules that won't allow operation if a document is not valid.

EVENTS:

1. Wrapper around all collection methods.

So I will probably implement it using wrapper, however I don't like this approach. Maybe someone would have better solution?

[steph643]

I don't use allow/deny, but I think they work only with db operations triggered by the client. If that is the case, they cannot be used for this feature.

[steph643]

About the other proposed solutions, what I can say is that aldeed:meteor-collection2 does the job well (although with poor usability).

[lukejagodzinski]

Yes you are right about allow/deny rules. I have to make a good decision if it goes about API. If I override collection methods, then I should have some way of accessing original methods.

In your opinion, what is the worst part of aldeed:meteor-collection2 that causes poor usability. What would you improve? I want to get as many opinions as possible.

[steph643]

There are 3 things that annoy me in aldeed:meteor-collection2:

1. It tries to ensure database integrity, which makes it difficult to understand. See this discussion. What I need is only the validation of the arguments being passed to insert/update, nothing more.
2. It manipulates data on-the-fly in a complex way (it is called "cleaning", see here), leading to painful bugs.
3. Error messages are cryptic. Example here.

[lukejagodzinski]

1. In fact the first point for me is a bug in SimpleSchema and could be easily fixed.
2. Astronomy does two of the things that SimpleSchema does. If it does not do it's a bug :P
   • Removing properties that are not in the schema
   • Conversion of values to match what schema expects
3. Fortunately, in Astronomy error generation mechanism is very flexible.

So, I assume that the you are happy with the API of SimpleSchema which executes validation:

Books.insert({
  field: 'value'
});

I don't like idea of overriding collection's method like insert or update. I would propose something different:

Books.astro.insert({
  field: 'value'
});

or some similar approach not messing with currently existing methods of collection.

[dalerka]

I'm not sure it is a better option, but the following API syntax might have few benefits:

Astro.insert('Books', {
  field: 'value'
});

1. it makes it clear that Astronomy package is used here to handle certain operations (easier for newcomers to the codebase);
2. it is similar to using Underscore like _.map(...);

Cons: someone might go looking for an "Astro" collection :)

What do you think guys?

[lukejagodzinski]

Yep, seems to be good idea. However, maybe it would be better to pass just a collection not collection name.

Astro.insert(Books, {
  field: 'value'
}, callback /* optional */);

Astro.update(Books, selector, modifier, callback /* optional */);

[dalerka]

Agree, I just came back to edit "Books" to Books :)

[lukejagodzinski]

What do you think about the syntax like:

Book.insert();

Not Books. We can insert, update etc. from the level of the Class not Collection.

[frabrunelle]

👍 for Book.insert(); I like it 😄

[lukejagodzinski]

Hi, I'm almost done with the direct database access methods. There is only one problem: validation. And I want to know your opinion on this subject. Let met explain the problem.

Right now, I've implemented all collection methods:

Post = Astro.Class({
  name: 'Post',
  collection: Posts,
  fields: {
    title: {
      default: 'Default title'
    }
  }
});

Post.insert();
Post.upsert();
Post.update();
Post.remove();
Post.find();
Post.findOne();

It works great and such statement:

Post.insert({});

will insert the following document:

{
  title: 'Default title'
}

All event will be called on the document, so everything works as expected. But what about validation? In fact I think that validating object on direct database access is bad thing. Let's take SimpleSchema as an example. You have to create validation context, validate query and later retrieve errors. If I would like to implement this in Astronomy I would have to do it in the following way:

Post.update(selector, modifier, {
  forEach: function(post) {
    if (post.validate()) {
      return true;
    }
    // Do something with the errors.
    post.getValidationErrors();
    return false;
  }
});

It's ugly but works. However, the problem is even more absurd on insert or remove. Why would I ever need to use forEach method if I'm inserting only one document? Why don't just:

var post = new Post();
if (post.validate()) {
  post.save();
}

And validating document on remove operation? I don't see the point. Validation is only for checking document's integrity, no to check if I can remove given document. For that case, we need security package.

So, my question is:
Do you need at all validation on direct collection access?
If yes, then:
Do you need validation on direction collection insert and remove?

[steph643]

I agree validation of a single object before insertion should be left to the programmer.

[dalerka]

I think we have deviated from the original purpose of this Issue. But I'll appreciate any comments which can help contributors to progress in their work.

[lukejagodzinski]

Ok, I decided to make validation possible only on document update. Direct database access have been implemented and @dalerka your original issue is solved by this feature.

Post.insert({field: 'value'});

will trigger all events and model logic as expected. The same is true about other collection operations.

For now I'm closing this issue. It will appear on 1.0 release.