search

poiati / gmongo

A Groovy wrapper to the mongodb Java driver
http://blog.paulopoiati.com/2010/06/20/gmongo-0-5-released/
Apache License 2.0
182 stars 44 forks source link

NPE when casting Map to BasicDBObject in find #10

Closed bluesliverx closed 12 years ago

bluesliverx commented 12 years ago

When performing a find using Grails integration with mongo DB plugin, we are converting a map to a basic DB object using code similar to:

DomainClass.collection.find([name:"..."] as BasicDBObject)

This appears to be going through the Patcher.groovy file in gmongo, which is having a null exception when one or some of the property values are null in the map to be converted. This seems to trace to line 114 of Patcher, where the class of each "argument" is retrieved. Output from an integration test:

N/A
java.lang.NullPointerException
    at com.gmongo.internal.Patcher._types(Patcher.groovy:114)
    at com.gmongo.internal.Patcher._patchInternal_closure1(Patcher.groovy:33)
    at com.ace.mws.reports.Something.execute(Something.groovy:33)
    at com.ace.mws.reports.SomethingIntegrationSpec.Feature here(SomethingIntegrationSpec.groovy:22)

I'm not sure of the args property in the patcher, but can we use a null-safe groovy operator here to prevent problems? We are on grails 1.3.7 which means we use gmongo 0.9.1 as far as I can tell.

I'll submit a pull request, and let me know what you think. Thanks, Brian

bluesliverx commented 12 years ago

Seems a good workaround is to use new BasicDBObject([map:"here"]) instead of using "as BasicDBObject" cast. This seemed to fix in the problem we were encountering.

poiati commented 12 years ago

Hi bluesliverx,

Why you need the cast? This should work in GMongo:

DomainClass.collection.find([name:"..."])

Thanks for the feedback.

bluesliverx commented 12 years ago

It makes it almost impossible to unit test with spock/grails. If I try to override the metaClass, it actually leaks into other tests (a problem I have not been able to nail down). This works correctly for DBCursor, but not for DBCollection.

The fact that you have not extended the DBCollection makes it much harder to test with spock mocks (smocks?) since it complains it can't find a method find with a single argument of a map, since it's added by gmongo dynamically.

poiati commented 12 years ago

It's hard to say something, what do you mean by 'not extended DBCollection' what not extended it? A collection in GMongo is the same of the com.mongodb.Mongo with some metaprogramming tricks.

I can't see your tests and I'm not figured out exactly what you are trying to mock/stub. On top of that I do not have much knowledge in spock.

If you can write a test using GroovyTestCase to reproduce this error I will be very grateful.

Thanks.

bluesliverx commented 12 years ago

I don't believe I can write a GroovyTestCase for this as it occurs during spock mocking. The mocks in spock actually try to implement the real class, such as DBCollection, so your metaprogramming tricks on this class are not recognized in spock mocking and it doesn't work to test.

Regardless, this is a separate issue, but it comes down to the fact that we are running into this error unless we do use BasicDBObject directly in collection calls (ie find). This is a little inconsistent, but the fix I submitted is the source of the errors. Are there any side effects to having null values here?

poiati commented 12 years ago

No, all the tests pass. I will merge your fix into master.

Thanks.

bluesliverx commented 12 years ago

Great, thanks! Just curious - are you going to do a point release of the plugin for this fix? It's breaking quite a bit of our things at times. Thanks again!

poiati commented 12 years ago

I did just now. Should be in the maven central in the next few hours, version 0.9.4.

bluesliverx commented 12 years ago

One more question - is this compatible with grails 1.3.7/groovy 1.7? Thanks!

poiati commented 12 years ago

No, are you running grails 1.3.x?

bluesliverx commented 12 years ago

Yup, we are.

stefankendall commented 12 years ago

@bluesliverx

I've started maintaining a parallel branch of gmongo that compiles with 1.7.10.

Check it out here: https://github.com/stefankendall/gmongo/tree/groovy1.7

I'm running on 1.3.7 with no issue so far.

bluesliverx commented 12 years ago

@stefankendall Thanks for the heads up! Is this branch actually built and deployed anywhere in a maven repo, etc? I built it on my own, but it would be much preferable to get it released as a 0.9.2.1 or something as @poiati mentions in another issue.

stefankendall commented 12 years ago

@bluesliverx Nope. I setup an automated build internally that publishes to a nexus repository, but there's no maven publishing going on. I've also got the go-ahead to upgrade to grails2 across a couple projects, so hopefully I won't need the branch for long :).

That said, there wasn't much required to get 1.7 working beyond setting the groovy version to "1.7". I modified the test classes a bit to support running tests on a non-standard mongo port, but there are no code changes. Manually merging for every update is a pain if you're trying to stay on the bleeding edge of latest, but there really weren't any issues.

And since there are tests, you can verify your merge, which is nice. :P

poiati commented 12 years ago

Ok, I will maintain a separe branch for Groovy 1.7.x support.