Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modifiers should be chainable #349

Open
scohen opened this issue Nov 15, 2011 · 6 comments
Open

Modifiers should be chainable #349

scohen opened this issue Nov 15, 2011 · 6 comments

Comments

@scohen
Copy link

scohen commented Nov 15, 2011

It'd be awesome if you could chain the atomic modifiers to update several fields in the database all at once.
Something like:

User.atomic.where(:first_name => "Eric").increment(:viewed_count => 1).add_to_set(:tags => "winner").execute

I have an initial implementation with tests on my chainable-atomics branch here:

https://github.com/scohen/mongomapper

@bkeepers
Copy link
Contributor

I'm not sure I love the API, especially with the #execute kicker on the end.

For some reason I thought you were working on making dirty tracking and the normal #save build atomic updates, so I just to step back an think about this a little more.

@scohen
Copy link
Author

scohen commented Nov 15, 2011

I figured most people would use the dsl like:

User.atomic.where(:first_name => "Eric") do
  increment :viewed_count => 1
  add_to_set :tags => "winner"
end

Honestly, the biggest deficiency I've encountered in MM is this, in all of my code, the only reason I drop into the low level driver is when I need to do so.

@bkeepers
Copy link
Contributor

Ok, thought through this a little more…

My biggest problem with this style of atomic updates are that they completely ignore any constraints and logic that you might have in your model. It's something we've been struggling with on a couple apps that do a lot of atomic updates. So how do we solve that problem?

The API for chaining atomic updates is interesting, but really it only provides a different syntax for something that already exists (using collection.update and building the modifiers by hand). What I personally am more interesting in seeing added to MongoMapper is atomic updates around changes to an instance. Take this for example:

u = User.first
u.name = 'Brandon'
u.tags << "winner"
u.increment(:view_count, 10)
u.save

Which would then generate an update like:

collection.set(:_id => u.id}, {:name => 'Brandon', :tags => {:$addToSet => 'winner'}, :view_count => {:$inc => 10}})

This to me would be extremely useful because it allows people to use their models like they always have. I realize this is not exactly the problem you're setting out to solve. Thoughts?

@scohen
Copy link
Author

scohen commented Nov 16, 2011

You're right, it's not the problem that I have. What I don't like about what you suggested is that it requires you to have the model in order to update it.

Imagine a model with tags. I'd really like to be able to update tags in the database atomically (and possibly a tag count and other fields) without first having to fetch the model.

We've used atomic updates to implement likes, tags and feed items and it's worked extremely well --especially when you're adding an eDoc to a document. Requiring you to first fetch the model, then modify it and then save the results gets away from what I was trying to accomplish (and it has really lousy performance in some instances).

My thoughts were along these lines: MM has atomic updates, but they're horribly limited with respect to not being able to do several things at once. I was trying to scratch the itch that I've had in the past.

If I'm not mistaken, what you're suggesting will actually change how save works to use atomic updates rather than the default 'overwrite the old document' method. That will be a breaking change, no?

@bkeepers
Copy link
Contributor

Makes sense. I'm all for scratching one's own itch!

In order for me to give better feedback on the API, help me understand your itch a little more. You're basically wanting to add an API in MM like:

User.atomic.where(:first_name => "Eric").increment(:viewed_count => 1).add_to_set(:tags => "winner").execute

which generates:

User.collection.update({:first_name => 'Eric'}, {:tags => {:$addToSet => 'winner'}, :view_count => {:$inc => 10}})

Does it do anything else?

@scohen
Copy link
Author

scohen commented Nov 16, 2011

What you have above is what it does.
The block syntax does make it much easier to understand what's going on than does the update syntax with its three hashes.

Also, I struggled a bit with a proper syntax. I'm not a huge fan of User.atomic.where, I'd also be fine with User.atomically_update(...), but that seemed long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants