Support for keywords with a namespace has been added. #61

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

maxweber commented Mar 9, 2012

Best regards

Max

Collaborator

purcell commented Mar 9, 2012

Are string keys like "namespace/foobar" converted back into namespaced keywords? Could this have unexpected consequences, e.g. where a namespace has not yet been loaded? I'm concerned that this might break existing apps, and might not be a desirable default behaviour.

Could you please supply a couple of tests to demonstrate? The current tests are runnable with "lein test".

-Steve

Collaborator

seancorfield commented Mar 11, 2012

I'd also like to know what the use case is for this? What problem is it solving?

Contributor

maxweber commented Mar 11, 2012

Without my change it is also the default behaviour that a "namespace/foobar" string becomes a :namespace/foobar keyword. So this change will not break any existing code on the mongo->clojure side. On the clojure->mongo side I would say that the change implements the desired default behaviour. I would guess that keywords with a namespace are used very seldom and no one used them in combination with congomongo before.
My use case for this is that I am using a schema which is similar to the one of Datomic (http://datomic.com/company/resources/schema), where keywords with a namespace are used everywhere.
I can add a test on Monday, when I'm back at my working place.

Max Weber If a keyword has a namespace and is used as a key of a map entry, the…
… namespace can now contain dots. Dots are not allowed in MongoDB document field names, so that every dot is replaced with a given replacement string (here: _DOT_). When the document is fetched from MongoDB, then the replacements are undone, so that this change works transparently for the caller.
9e55fd3

@purcell You are right. This change could break existing apps. I've chosen the wrong place for the change, due to the fact that the above change would also convert keywords which are the value of a map entry.

This would be the right place for the change. However to support Clojure keywords with a namespace you would also have to support JSON map keys which contains dots like :bank.account/number But dots are not allowed inside BSON map keys of MongoDB, so that we have to replace the dots with something else like "DOT". The approach would work transparently for the caller, but not for other things which interact with MongoDB (like the MongoDB console). Furthermore I would guess that the change impacts the performance negatively.
In the end I would suggest to simply close this pull request without applying the change, due to the issues mentioned above.

Collaborator

seancorfield commented Jul 19, 2012

Closing per Max's last comment about not being a transparent change for other MongoDB clients.

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