search

purekid / mongodm

MongoDB ORM that includes support for references,embed and multilevel inheritance.
MIT License
200 stars 47 forks source link

Remove MongoId requirement #70

Closed nodefortytwo closed 10 years ago

nodefortytwo commented 10 years ago

Hi! as far as i can tell, the model _id has to be a MongoId. that isn't a requirement for mongodb and there are a few cases where specifying a different type of ID makes sense.

I would love to be able to use this ODM but that limitation prevents it ;(

jrschumacher commented 10 years ago

Could you describe some of the use cases?

We have plans to work on a v2.0 and this sounds like a feature which we need to plan for. I'm not sure how easy it will be to patch without.

jrschumacher commented 10 years ago

I read some of the use cases with regards to storing auto increment numbers or uuids. I'll look into how easy this would be.

nodefortytwo commented 10 years ago

yep those are valid use cases, you can also use complex types as ids, for example, in one application we have ids are that {"_id" : {account: "account_id", id:"id"}}. there are obviously other ways of doing the same thing but its a nice feature of mongodb

jrschumacher commented 10 years ago

Good to know. So with that the MongoId::id(array("account" => 123)) would return one or more than one? In you opinion?

jrschumacher commented 10 years ago

Ahh I see what you are doing. Nesting multiple document types within one collection. But still give the scenario above how would you handle a partial query?

nodefortytwo commented 10 years ago

yeah thats interesting, i guess ::id should always return a single result, so a partial query wouldn't return any matches?

jrschumacher commented 10 years ago

That was my thought too. Alright give me a few days.

jrschumacher commented 10 years ago

@nodefortytwo so I'm considering a simple solution to patch this. Defining a new static variable like $customIdType which by default is false. If set to true then we bypass any assumptions about MongoId. Would this work for you? It would work like the static $useType = false;

A further solution would be to not assume ids are MongoId type unless specifically stated if the use of automatic converters and helpers were desired, but this will be a 2.0 feature.

nodefortytwo commented 10 years ago

that would work for me, in fact, in my own horrible ODM thats exactly how I handle it.

Thanks for taking the time to look at this. I love the rest of the ODM.

jrschumacher commented 10 years ago

Sure thing. We use it in our production systems and want to see it succeed at being a light and robust solution. Feel free to get involved!

jrschumacher commented 10 years ago

@nodefortytwo could you test this branch with your app to see if it works for you?

nodefortytwo commented 10 years ago

Apologies for the slow reply, This looks perfect, I carried out a few basic tests and everything worked perfectly. thanks guys

jrschumacher commented 10 years ago

Glad to hear it!