search

go-bongo / bongo

Go ODM for MongoDB
MIT License
486 stars 40 forks source link

make Delete functions like Find functions #3

Closed sharpner closed 9 years ago

sharpner commented 9 years ago

It would be awesome if Delete would be called DeleteOne like the FindOne method. Additionally I would love the support for complex delete queries, again, like Find.

jraede commented 9 years ago

You can access the *mgo.Collection from the *bongo.Collection through the Collection() method. E.g. bongoConnection.Collection("pages").Collection(). Then you can run any query you want, including batch deletes, etc. No need to reinvent the wheel just to do that stuff directly via Bongo, in my opinion.

That logic also led to the original naming of that method Delete, since any method like DeleteOne or DeleteMany is already available via mgo. bongo.Collection.Delete() takes a bongo.Document, so it's implied that it's deleting one, I would think. However, I'm open to discussion on that one. I'll leave this open for now.

sharpner commented 9 years ago

Yeah I know that I could workaround the problem.

Its just somewhat confusing and incosistent. Without knowing the internals of bongo, there is no way to delete more than one Record with a query. Additionally, if I used find(query) before, I will stumble when using delete :)

jraede commented 9 years ago

I guess it might be beneficial to change it up a bit. I'm thinking, along what you've been saying:

bongo.Collection.Delete(bson.M) would delete ALL documents that match the query bongo.Collection.DeleteOne(bson.M) would delete the FIRST document that matches the query bongo.Collection.DeleteDoc(bongo.Document) would delete just that doc, and run the delete hooks.

The only issue here is that Bongo will be running delete without running the delete hooks, unless we first find them, marshal them into a bongo.Document, and then run DeleteDoc. Thoughts?

sharpner commented 9 years ago

I see your point. Other ODMs handle this, by having only the hooks on document/entitys actions, but no hooks on queries.

So exactly what you suggest. I like the idea. As long as its documented =)