peerlibrary / meteor-peerdb

Reactive database layer with references, generators, triggers, migrations, etc.
http://atmospherejs.com/peerlibrary/peerdb
BSD 3-Clause "New" or "Revised" License
130 stars 15 forks source link

Allow and Deny #15

Closed Sivli-Embir closed 10 years ago

Sivli-Embir commented 10 years ago

I would like proper Allow and Deny support. Currently I am using this work around:

Comment._collections.Comments.allow
  insert: (userId, doc) -> userId == doc.user._id #can not create a comment for some other users.
  update: (userId, doc, fieldNames, modifier) -> 
    return true if userId == doc.user._id 
    ...
  remove: (userId, doc) -> false #never delete anything -- apparently
mitar commented 10 years ago

I think he reason why this is not supported it because I very dislike a limited interface they provide you. For example, you cannot get a connection reference to check for an IP and allow/deny based on that. So this is why I added a concept of managers and you have this documents and then wrapped methods for accessing, so that we can roll out our own allow/deny rules sometime, without breaking anything. But I have not yet implemented that because in my projects allow/deny rules are simply to simple.

BTW, probably better way is to do Comment.Meta.collection.allow. This is official way to get access to underlying collection, for example, to define indexes:

Person.Meta.collection._ensureIndex
  slug: 1
,
  unique: 1
Sivli-Embir commented 10 years ago

Well I will give you the bit about IP but how much does that really matter? I love the allow/deny personally and at some point I want to use this package with a fixed version of my reactive-schema. The beauty of that gives lets me just call validate in an allow.

mitar commented 10 years ago

So, yes. Use Comment.Meta.collection.allow. This is official.

mitar commented 10 years ago

I would love to have schema validation in PeerDB, BTW.

Sivli-Embir commented 10 years ago

Ok cool thanks! As long as the api does not change then I am happy. And I did see that. What do you think about something like my reactive-schema? I like the idea of letting it auto validate. But I did find an issue with nested object properties. Not sure on how to work around that quite yet.

Update: autocorrect fixes

mitar commented 10 years ago

What do you think about something like my reactive-schema?

How do you intercept object changes?

For PeerDB, I already have a place where to put validation, I just didn't yet get to it. See here: https://github.com/peerlibrary/meteor-peerdb/blob/master/lib.coffee#L80

Sivli-Embir commented 10 years ago

I built Reactive Objects to manage the changes. It overloads the object properties ECMA-262 [[get]] / [[set]] style. So a [[set]] invalidates Tracker and a [[get]] call Tracker depend(). I will check out what you have already.

mitar commented 10 years ago

But that works reliably only on some browsers. But it does work on the server side.

Sivli-Embir commented 10 years ago

OH! Thanks for reminding me. Yes there has been some talk about that and we may change that. Regardless the general concept remains the same. When a property changes validate it. I am also really a fan of validating in a function rather then by 'type' like simple schema does. You can always check for 'type' in the function.

mitar commented 10 years ago

Yes there has been some talk about that and we may change that.

From what I read is that this is not something which will go into the core if they want to keep pre IE 9 compatibility. So I would not bet on this being mainstream way of doing it. Sadly. I also thought about that in my cases (I even proposed that in publish, userId should be defined through getter, so that only if you access it, publish function would rerun when user logs in, if you do not, then it should not).

So while I like the approach, sadly I don't think it is yet time for it. So my approach is more to require users to do MongoDB queries and validate at that stage (which is also a good question how to do: does Meteor provide somewhere a hook to get a future object as it will be in the database, after running the query, but before it really runs? probably with all minimongo support one could do that, simulate a query in memory, run validation, if it passes, send to the server; another approach would be to send to the server and then have a PeerDB trigger check the value and if it fails, it rolls back the document to previous version). Instead of mapping object changes directly to queries.

I am also really a fan of validating in a function rather then by 'type' like simple schema does. You can always check for 'type' in the function.

I don't understand what you are saying here. Can you elaborate/provide examples? (I have almost zero knowledge how simple schema does things.)

mitar commented 10 years ago

(Currently PeerDB validation happens only objectification step. So when you read from the database.)

Sivli-Embir commented 10 years ago

@mitar not sure if you saw David Greenspan's update but did you see https://meteor.hackpad.com/Lickable-Forms-and-Components-6CVspZsVwJY? I am still processing this but it sounds like this may go core.

And I hear what are saying about MongoDB queries. I think figuring that out will be important regardless of what schema pattern you use. That is an issue I have been pondering for quite a while. Also what are we, the community, going to do when we start adding more database engine themselves (MySQL)? I will keep thinking about this.

So in SimpleSchema you create a property like such:

BookSchema = new SimpleSchema
  title: 
    type: String #I don't like this
    label: "Title"
    max: 200

I feel it should be more like the following. Note that this is just for example and not how I would like to actually work:

stringWithLimit = (limit) ->
  return false if check(@value, String) #or any other validation you want
  return false if this.value > limit
  true

BookSchema = new TheSchema
  title: 
    validator: stringWithLimit
    params: 200
    title: 'Title

The benefit to my way is you can have lots of different type checks and you can do things like

email = () ->
  valid = true
  valid =  false unless isEmail(@value)
  message = whatEmailAreYou(@value) if valid
  return {valid: valid, message: message}

User = new TheSchema
  email: 
    validator: email
    title: 'Email'

# whatEmailAreYou
# returns domain of email, e.g. gmail returns 'Gmail'
mitar commented 10 years ago

I am still processing this but it sounds like this may go core.

I will wait for that. :-) Until then I will work with what we have.

And I hear what are saying about MongoDB queries. I think figuring that out will be important regardless of what schema pattern you use. That is an issue I have been pondering for quite a while. Also what are we, the community, going to do when we start adding more database engine themselves (MySQL)? I will keep thinking about this.

I like MongoDB so for me questions about MySQL are not so important. Many of PeerDB features are built around MongoDB so not sure if there is really any use in SQL. In fact I see PeerDB as having the best of both words: on-write joins, so reads are fast and can be easily parallelized.

So my proposal for schema is like this:

fields: =>
  email: new EMailField title: 'Email'
  title: new MaxLengthField maxLength: 200, title: 'Title'
mitar commented 10 years ago

See here for more examples.

Sivli-Embir commented 10 years ago

Many of PeerDB features are built around MongoDB so not sure if there is really any use in SQL.

Valid point, so long as enough devs are using PeerDB I also don't mind. My biggest concern with PeerDB is that it will lose support. It requires that you commit your app to it, so using comes with a risk. That said you are doing a great job so I don't feel that is likely.

Given those examples I think you should really look at simple schema. I may not like it but if thats the route you are going then they have done most of the work for you. Its also reasonable that we could set up some kind of reactive validation using it in the future.

Anyway thanks for all your feedback. It makes me more comfortable in my choice to commit to this package.