Adding default properties and methods to documents/objects returned by a collection

[Andrew Ferk]

This is pretty closely related to the discussions about adding a Model, Document, Collection wrapper, etc. Essentially, my suggestion is to allow the optional options paramater in new Meteor.Collection(name, [options]) to support a defaults objects which would include default properties and methods. In addition, collection.find and collection.findAll would be updated to extend the found objects with the default object. Finally, collection.create([attrs]) (or collection.new) would be a new method that returns an object of the option attrs object extended with the default object.

Here are some examples:

var Dogs = new Meteor.Collection("dogs", {defaults: {

  barkSound: "ruff",

  bark: function() {

    console.log(this.barkSound);

  }

});

var normalDog = Dogs.create();

normalDog.bark(); // ruff

var smallDog = Dogs.create({

  barkSound: "yip"

});

smallDog.bark(); // yip

Dogs.insert(smallDog, function(error, id) {

  Dogs.findOne(id).bark(); // yip

});

I pretty much implemented the features needed by overriding Meteor.Collection: http://stackoverflow.com/questions/15007231/whats-the-best-way-to-attach-behavior-to-a-meteor-collection/15013002#15013002. But if people like this idea, I think it would be best to write the functionality into Meteor.Collection, instead of overriding it.

Let me know what you guys think about this. This is my attempt at solving the model problem with minimum changes to the code and limited extension to the API.

Thanks,

Andrew

[Tom Coleman]

Hi Andrew.

Although it would be great to get official model support into Meteor Collections, I don't think it's something that's likely to land any time soon (I've had a PR--out of date now--open for around 9 months: https://github.com/meteor/meteor/pull/267). 

My current plan (which will be implemented very soon) is to create a ModelledCollection smart package, which does something very similar to what you are doing in that SO answer. That way we don't need to wait for core. My thoughts on what you've done there:

1. I think it's better to pass in a ctor and call `new ctor(document)` rather than extending. But I'm open to debate.

2. I think it's better to wrap Collection (so users would do `new ModelledCollection(name, {ctor: Dog})`) rather than "monkey-patching".

Tom

--
You received this message because you are subscribed to the Google Groups "meteor-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to meteor-core...@googlegroups.com.
To post to this group, send email to meteo...@googlegroups.com.
Visit this group at http://groups.google.com/group/meteor-core?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Andrew Ferk]

Hi Tom,

Can you give an example of passing in a constructor? Also, the only reason I monkey-patched Collection was to limit the extension of the API. Essentially, this solution only adds to the API in two places: passing in the defaults object, and the collection.create method. I hate the monkey-patch, which is why I would rather have this functionality built-in.

Essentially, I don't think this solves the model support, or at least what people from the MVC, MVVM, MVP, etc. world consider a model. What this solves is the ability to define behavior to objects in a collection.

If someone really wanted a model, they could use Backbone, and instantiate a Backbone.Model by passing in the object returned from a Meteor.Collection's find or findOne.

While I'm not against the idea of having an actual model, where you can do model.save(), model.destroy(), etc., this adds to the API simply by duplicating functionality that is already provided.

I've attempted to wrap Backbone.js around Meteor, and failed. I think the architectural approaches are too different to make it worthwhile. I don't think the MVC or MVVC approach fits appropriately with Meteor. Possibly, the MVP will work (which is what I have been attempting), but either way, maybe Meteor is best left at being functional. But the lack of encapsulation reminds me of C. It's a dilema I'm having :)

Thanks for you feedback!

[Tom Coleman]

On Friday, 22 February 2013 at 9:53 AM, Andrew Ferk wrote:

Hi Tom,

Can you give an example of passing in a constructor? Also, the only reason I monkey-patched Collection was to limit the extension of the API. Essentially, this solution only adds to the API in two places: passing in the defaults object, and the collection.create method. I hate the monkey-patch, which is why I would rather have this functionality built-in.

Well my idea would do the same (i.e. ModelledCollection would have the same API as Collection and could be used whereever you'd use a collection). The problem with monkey patching as you have is that mongo-livedata (the package that syncs client + server collections) will no longer work as it relies on the objects that come out of collections being POJOs. So without patching that too… (now we are back at my PR).

Here's what I mean by using a constructor:

var Dog = function(attrs) {

   _.extend(this, attrs);

}

Dog.prototype.bark = function() { console.log('bark'); }

var Dogs = new ModelledCollection('dogs', {ctor: Dog});

Essentially, I don't think this solves the model support, or at least what people from the MVC, MVVM, MVP, etc. world consider a model. What this solves is the ability to define behavior to objects in a collection.

If someone really wanted a model, they could use Backbone, and instantiate a Backbone.Model by passing in the object returned from a Meteor.Collection's find or findOne.

While I'm not against the idea of having an actual model, where you can do model.save(), model.destroy(), etc., this adds to the API simply by duplicating functionality that is already provided.

Oh. By "Model Support" I mean putting the pieces in place to support someone else writing a model. Yes, you can call new Dog(data) every time something comes out of a collection, but let me assure you, it's a such a pain that you usually just don't bother.

What exactly a Meteor Model should do is much more controversial, and I think various packages will appear with various functionalities. But it's all a bit academic while it's such a pain to use them.

I've attempted to wrap Backbone.js around Meteor, and failed. I think the architectural approaches are too different to make it worthwhile. I don't think the MVC or MVVC approach fits appropriately with Meteor. Possibly, the MVP will work (which is what I have been attempting), but either way, maybe Meteor is best left at being functional. But the lack of encapsulation reminds me of C. It's a dilema I'm having :)

I'm not so sure. I think meteor is MVC, at least in the same way that angular is. The template helpers + event handlers together constitute the controllers. Although to be honest I get really confused by all the acronyms and exactly what they are supposed to mean. Some encapsulation / namespacing / scoping is sorely needed though. I think there's a card on the trello that relates to this.

 T

[matt debergalis]

i've long been fond of adding a `ctor` option to the Meteor.Collection
constructor (on both sides of the wire).

[Naomi Seyfer]

I just committed something to devel that may do what you want.  It's not at the Collection level, though, it's at the EJSON level.

Now (on devel), as long as EJSON knows about the type you'd like to serialize, it will serialize & deserialize to/from Mongo correctly (with your methods and such).  I even used the Dog example in the tests.

The merge is at bcc675e40a802907ae65d3510b9c0a4309f21320

--Naomi

[Andrew Ferk]

Hi Naomi,

This is quite an interesting approach. Essentially, if one would want to make a "Model", one could have it act as a user-defined EJSON type, which then could be inserted into collections. I have been playing around with EJSON since it's release with 0.5.7 to already create my "Model" implementation. I actually like this approach, as it leads a way for one to make a scheme in a scheme-less database.

Does everything in dev eventually make it's way to master?

Thanks,

Andrew

[Andrew Ferk]

I meant schema and schema-less :) Also, if anyone is interested in what I'm working on: https://github.com/andrewferk/meteor-mvp.

[Naomi Seyfer]

On Feb 25, 2013, at 12:41 PM, Andrew Ferk <andre...@gmail.com> wrote:
>
> Does everything in dev eventually make it's way to master?

Unless it's reverted, yeah. I still want to talk about this solution with Geoff a bit before I'm sure about releasing it.

--N

[Tom Coleman]

Oh cool! I'd make two quick comments:

This means that attributes of documents can have prototypes right? Although that's a lot better than nothing, it doesn't hit the most obvious use case, which is doing things like:

  var dog = Dogs.findOne(id);

  dog.name = "Max";

  dog.save();

Or have I read the code wrong?

Also, have you considered that doing things like this (allowing deserialization to arbitrary classes) is the attack vector of multiple recent Rails (and Django?) security vulnerabilities. Not saying it's an inherently bad idea, just that it does increase the security surface area.

Interesting stuff!

Tom

[Naomi Seyfer]

On Feb 25, 2013, at 5:53 PM, Tom Coleman <t...@thesnail.org> wrote:

This means that attributes of documents can have prototypes right? Although that's a lot better than nothing, it doesn't hit the most obvious use case, which is doing things like:

  var dog = Dogs.findOne(id);

  dog.name = "Max";

  dog.save();

Or have I read the code wrong?

You are correct.  This hits the use case of "I put a dog in my document, and I registered the Dog type with EJSON, why does it not come out right on the other end like it does with Session and minimongo", not "everything in this collection is a Dog, I want you to automatically know that from now on".  Both use cases are totally valid; the reason I see some chance that this change will get reverted is that I'd rather not head towards a world where the two use cases are easily confusable for each other.  

Advantages of the EJSON stuff in that commit:

* Fields carry their own type information; you don't have to be doing anything that smells of pre-specifying a schema

* Custom-specified EJSON objects work the way you expect them to with Mongo

Advantages of specifying a filter on a per-collection basis:

* The whole document can be a customized thing

* Can work better with other consumers of the database that know nothing of EJSON

I suspect people want to do both kinds of things, but I want to make sure they're not confused between their options.

Also, have you considered that doing things like this (allowing deserialization to arbitrary classes) is the attack vector of multiple recent Rails (and Django?) security vulnerabilities. Not saying it's an inherently bad idea, just that it does increase the security surface area.

The security surface area here seems to me to be proportional to the code you provide EJSON to deserialize the types you register in your application.  When you register a type you provide EJSON with the type name and the factory -- I don't *think* there are any clever ways to call arbitrary code from that, any more than a dispatch to any registry of functions inspired by input from the client would.  I was actually designing the code for this in my head while reading about the recent Rails exploits, and am pretty sure that this code doesn't do something like "pull a constructor out of the global namespace to run".

I've studied security, but only to the point that the Dunning-Kruger effect lets me know that I've got a lot left to learn -- I'm *very* open to friendly input on this.

--Naomi

[Tom Coleman]

Sounds like you've thought about all this a lot more than I have!

One intermediary solution which _may_ be worth thinking about:

 - If I could attach a single type (call it Model) to _all_ Collections, then I could get some of the advantages of models without venturing into schema territory.

- Coupled with the EJSON data types, perhaps that's enough rope?

Tom

--

[erundook]

So is there a way to wrap the whole document into a custom EJSON type?

вторник, 26 февраля 2013 г., 8:39:45 UTC+4 пользователь Naomi Seyfer написал:

[Andrew Ferk]

Naomi,

What you say makes sense, but I didn't understand it immediately. With the example I gave in my first post, I am forcing/assuming, or pre-specifying, that every document is a Dog or has the attributes defined by the defaults attribute. In your solution one isn't pre-specifying a scheme. This is very good.

I would assume if this was accepted, down the road, many developers wouldn't directly interact with this feature, but instead an abstraction on top of it. I think it's a great tool for package developers.

Regards,

Andrew

[Phil Cockfield]

For what it's worth, I wanted to throw up into a Gist for you guys to show you the wrapper approach we've been taking to get model methods around a document.

There's a bunch of clever thinking in this thread that we hadn't approached yet, in particular to do with having the collection itself cough up model instances. We've (so far) just taken the approach of passing the document into a model wrapper.

https://gist.github.com/philcockfield/5033395

What we're doing is working with two kinds of classes (in CoffeeScript...just prototypes in JS of course) a:

• Schema
  
• Model
  

The schema gives us some control in defining fields within the document.  The model then uses the schema to automatically create property-functions for reading and writing to the document (like jQuery, passing no value to the function is a read operation, passing a value is a write).  For example:

  ###

  Defines a user of the system.

  ###

  class UserSchema extends APP.core.Schema

    constructor: (fields...) -> super fields,

      services:   undefined  # Services field that gets created by the Meteor framework

      name:                  # Name object.

        field: 'profile.name'

        modelRef: -> ns.Name

      email:

        field: 'profile.email'

      roleRefs:  # Collection of ID references to roles the user is within

        field: 'profile.roleRefs'

  ###

  Represents a user of the system.

  ###

  class User extends APP.core.DocumentModel

    constructor: (doc) ->

      super doc, UserSchema, meteor.users

In the example above, we're able to map the property-functions deep into the document, in this case allowing us to wrap a model around the Meteor "user.profile" document structure.

We have two types of models:

• Model
  The base model behavior that handles property-functions
  
  
• DocumentModel (inherits from Model)
  Knows about a collection, and is able to save changes back into mongo.

We broke these up into distinct things because you may want sub-models within a root document-model to represent portions of the document if it is complex or deep in structure.

Having property functions automatically generated gives us the opportunity to put consistent lifecycle hooks around reading and writing.  We can catch values before or after they are written, where we can validate the value, mutate the value etc, etc.

Also, by having these read/write property functions automatically created in the base class, it leaves the deriving Model clean to just contain the custom methods and business logic we want to associate with that document.

There's a few other things we're doing in there, like automatic lookups for parent:child associations between documents, as well as handling 1..many associations. Otherwise that's pretty much it.

Wanted to share in case it sparks any ideas.  And I'm interested to see how I could automate the creation of these models directly out of the collection, as I think (but haven't quite followed yet) this discussion is illuminating.

https://gist.github.com/philcockfield/5033395

Cheers!

[Andrew Ferk]

Hi Phil,

I like how your approach does a great job separating concerns. I think it's a great engineering principle, but I have a feeling some may find it too verbose or abstract :)

My only immediate suggestion would be to consider making an interface for an Association and a couple concrete classes for BelongsTo, HasOne, HasMany, etc. Then your Model or DocumentModel can delegate association calls to the Association interface.

I love all the work being done with this. Everyone seems to have a different approach, but many ideas are better than trying to spend time getting the "right" idea.

Regards,

Andrew

[Phil Cockfield]

My only immediate suggestion would be to consider making an interface for an Association and a couple concrete classes for BelongsTo, HasOne, HasMany, etc. Then your Model or DocumentModel can delegate association calls to the Association interface.

That's a really nice idea Andrew!

Say, when you're using the term "interface" within the context of JavaScript, what technique are you referring to?

--
You received this message because you are subscribed to a topic in the Google Groups "meteor-core" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/meteor-core/hsJB9Y5JeLQ/unsubscribe?hl=en.
To unsubscribe from this group and all its topics, send an email to meteor-core...@googlegroups.com.

To post to this group, send email to meteo...@googlegroups.com.
Visit this group at http://groups.google.com/group/meteor-core?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

--

Phil Cockfield

[Andrew Ferk]

I'm referring to software interfaces from OO principles. I suppose in coffeescript you would make Association a class, and BelongsTo, HasOne, HasMany, etc. would extend Association, as interfaces don't exist in Javascript. But to stick with the interface principle, BelongsTo, HasOne, and HasMany should all have the same "public" methods which would be used by Model or DocumentModel. By doing this Model/DocumentModel wouldn't care if the association is BelongsTo, HasOne, or HasMany. I'm not entirely sure if it's possible to decouple Associations from Models the way I'm suggesting, but it might be worth the attempt.

[Phil Cockfield]

Got it!  Cool.

Was wondering if there was some new clever way to get strongly typed interfaces into JavaScript!

I like this thinking Andrew

--

Phil Cockfield

[Naomi Seyfer]

I suspect people want to do both kinds of things, but I want to make sure they're not confused between their options.

I've now had a chat with Geoff, and am planning on both keeping the ability to put arbitrary EJSON in your Collections, and also adding some support for the use case of having every document in a collection represent some specific kind of object.  Here's my current planned design:

* `find()` and `findOne()` currently take an options argument.  Another option I'll be adding is `factory`, which is a function that your documents get passed to before they come out of the API.  `factory` should return the object you'd like to see, which may be either an entirely new object or a mutation of its argument.

* The `Collection` constructor will take an options argument, and an option you'll be able to specify is `defaultFactory`, which will act as a default factory for all cursors on that `Collection`.  If you want the raw documents back, you can specify `{factory: null}` on a cursor and that particular cursor will not pass its documents through any factory. (This is what you may want when only iterating over a subset of the fields in your document, among other use cases).

* If you call `observeChanges()` on your cursor, the factory on that cursor is never involved -- it just doesn't make sense to be passing an unpredictable subset of fields through the factory.  On the other hand, the factory *is* used for `observe()`

I'm going to be starting on implementation of this now, but am certainly not closed to good ideas coming from y'all that might adjust what I do, especially if the discussion happens today.  My goal is to have this feature shippable by the end of the day today, but we shall see.

--Naomi

[Phil Cockfield]

This sounds really great Naomi.

--

You received this message because you are subscribed to the Google Groups "meteor-core" group.

To unsubscribe from this group and stop receiving emails from it, send an email to meteor-core...@googlegroups.com.
To post to this group, send email to meteo...@googlegroups.com.

Visit this group at http://groups.google.com/group/meteor-core?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

--

Phil Cockfield

[Tom Coleman]

Hi Naomi,

I've run into a couple of things in semi-implementing this (which I've done a couple of times[1], looking forward to the real thing!).

1. Be careful making assumptions about the structure of documents that come out of the factory (or be very clear about what they need to look like).

- I pointed out how EJSON.clone didn't quite work properly when documents had prototypes

- Likewise, Spark's apply changes function is fine for simple documents, but maybe you should let objects do it in their own way if they want to: 

if (_.isFunction(doc.applyChanges)) {

          doc.applyChanges(changeFields).

        } else {

          // ..copy attributes over

        }

In short, Spark does need to deal with modelled documents (so it can set them as data contexts for {{#each}} amongst other things).

2. There are some points where meteor internally calls observe + friends. Specifically I am thinking of mongo-livedata. I've run into lots of problems because livedata expects to just transfer the properties of what comes out of observe() over DDP, and throw them directly in minimongo at the other end. 

You've probably thought of this, but I think you'll need to do one of:

a) ensure livedata calls observe(…, …, {factory: null})

b) get livedata to call doc.toRawObject() if it exists on the document [2] OR

c) very clearly specify what the structure of modelled documents needs to be.

There the main bugbears I've run into. If I were you I'd also take a look at my PR for a couple of tests that might still prove useful to you.

Really excited this is happening! Please feel free to totally ignore the above as the ranting of someone who doesn't understand the livedata system properly :)

Tom

[1]  - Firstly in a couple of PRs (https://github.com/meteor/meteor/pull/267) that are stale now. 

      - Just in the last couple of days here (https://github.com/tmeasday/meteor-modelled-collection), which works, but I don't think is really bullet-proof.

[2] see my out-of-date PR, this is the approach I took, I think I used doc._meteorRawData()

--

[Theodore Blackman]

+1 Spark does need to use the modeled documents. I actually feel pretty strongly about this point.

Naomi, maybe templating/deftemplate.js could check If the cursor passed to #each has a factory, and if so apply the factory method to the items before sticking them in the data contexts? That way it could still keep the observeChanges call, and I don't think it would require changes to any other piece of code you're working on.

[Naomi Seyfer]

On Mar 7, 2013, at 11:31 AM, Theodore Blackman <ted.bl...@gmail.com> wrote:

> +1 Spark does need to use the modeled documents. I actually feel pretty strongly about this point.
>
> Naomi, maybe templating/deftemplate.js could check If the cursor passed to #each has a factory, and if so apply the factory method to the items before sticking them in the data contexts? That way it could still keep the observeChanges call, and I don't think it would require changes to any other piece of code you're working on.

I agree with this plan -- it's another thing to track when doing this project, though. As I got distracted by bugs in EJSON yesterday, I'll be working on this today, but I'm realizing that there's enough subtlety to this plan that it's going to take more than one day to do as a project.

Another thing that may sound trivial but I actually care a little bit about is that I don't like the names "factory" and "defaultFactory" very much for these options that people will be passing, but I am not thinking of anything better. Accepting suggestions.

--N

[Phil Cockfield]

Hi Naomi,

I thought "factory" was pretty clear and helpful, as the name pointed to the pattern being used.

I think of the things produced by these factory functions as "models" - so perhaps you could clarify what the factory is for:

• modelFactory
• defaultModelFactory

That's less concise, and includes the original name ("factory") you were wanting to back away from - so maybe the wrong direction.  

But somehow using the term "models" here feels right to me.

--

You received this message because you are subscribed to the Google Groups "meteor-core" group.

To unsubscribe from this group and stop receiving emails from it, send an email to meteor-core...@googlegroups.com.

To post to this group, send email to meteo...@googlegroups.com.
Visit this group at http://groups.google.com/group/meteor-core?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

--

Phil Cockfield

[Andrew Ferk]

It sounds like there is a lot of complexity here. It seems like you can't think of EJSON, Collection, Spark, or mongo-livedata separately. While this is all working together great for now, how long will it last if it really is all complected? I don't have a solution, and I'm not sure if my point is valid. But from what I've heard, this does not seem to be simple and could become a bottleneck in the future. I only bring this up because I just re-watched Rich Hickey's "Simple Made Easy" and read the seventh Meteor principle:

Simplicity Equals Productivity. The best way to make something seem simple is to have it actually be simple. Accomplish this through clean, classically beautiful APIs.

[Naomi Seyfer]

Okay, I've had first contact with getting a lot of this working.  You can play with what I'm doing on the `models` branch.  I've not touched allow or deny rules yet, but I think everything else is working. (for the record, the plan for allow and deny is to add "factory" as an option to them too, and you get to specify null, and by default you still get the default factory).

On Mar 6, 2013, at 2:54 PM, Tom Coleman <t...@thesnail.org> wrote:

Hi Naomi,

I've run into a couple of things in semi-implementing this (which I've done a couple of times[1], looking forward to the real thing!).

1. Be careful making assumptions about the structure of documents that come out of the factory (or be very clear about what they need to look like).

- I pointed out how EJSON.clone didn't quite work properly when documents had prototypes

EJSON.clone and EJSON.equals had a decent amount of code that was just moved over from LocalCollection._deepcopy and LocalCollection._equals that was just plain wrong.  Mea culpa.  I *think* I've fixed that now.

- Likewise, Spark's apply changes function is fine for simple documents, but maybe you should let objects do it in their own way if they want to: 

if (_.isFunction(doc.applyChanges)) {

          doc.applyChanges(changeFields).

        } else {

          // ..copy attributes over

        }

Spark manages the structure of objects raw (that is, not touching whatever the default factory on the collection or cursor is) (because it uses observeChanges) and then passes them through the factory before handing them off to the render functions.  Cursors helpfully have a getFactory() method on them for people who want to follow a similar pattern themselves.

In short, Spark does need to deal with modelled documents (so it can set them as data contexts for {{#each}} amongst other things).

See above ^^

2. There are some points where meteor internally calls observe + friends. Specifically I am thinking of mongo-livedata. I've run into lots of problems because livedata expects to just transfer the properties of what comes out of observe() over DDP, and throw them directly in minimongo at the other end. 

Almost all these places (actually, I think truly all of them) are observeChanges these days and not observe; observeChanges always gets the raw fields.  This actually came together a lot more cleanly than I thought it would.

c) very clearly specify what the structure of modelled documents needs to be.

I think the only thing I am worried about is whether or not the modeled document still needs an _id field;  I have yet to write tests that try not having one.  I *think* it will still work, but thinking and actuality are different things.

There the main bugbears I've run into. If I were you I'd also take a look at my PR for a couple of tests that might still prove useful to you.

I'd love to port over some tests.  Do you have a specific file you could attach to a personal email or at least point me to?

[Tom Coleman]

Hi Naomi,

It's looking good! The observeChanges() and getFactory() pattern does seem like a good one. I'm very excited about this.

Sorry about the late response, I was away for the (long) weekend.

My only comment is that one place I've noticed not having an _id is a problem was in `{{each}}`. I think someone mentioned running into it outside of this context anyway, so perhaps it is just a bug in the each implementation.

Anyway, if you still want them, my tests are in these two places:

1. This PR: https://github.com/meteor/meteor/pull/267/files 

- A little out of date now, so perhaps not useful at all

2. This package: https://github.com/tmeasday/meteor-modelled-collection/blob/master/modelled-collection-tests.js

- I wrote this last week so the tests are certainly up to date, at least against master.

T

--