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
GmongoClient should extend GMongo #29
Comments
Hello davy, I choose to not inherit from Mongo, this was my design decision. Inheritance comes with some problems and I rather using composition with delegation. In the ideal scenario Mongo/MongoClient should be an interface with GMongo implementing it but there isn't such interface. That said, I know what you mean and what you want, I will take a look at your solution. Thanks! |
Thanks for looking- what you have is super and makes sense. The issue we are facing is that the grails team has code that says: GMongo mongo - and below - if auth is involved set mongo = new GMongoClient. |
I agree, it's easy to think that GMongoClient extends GMongo and maybe it really should. But if what you are saying is true, you mean the mongodb plugin is failing with compilation errors? |
Not compilation - it's in groovy code - happens at startup and causes a crash. Depends how you configure the mongo plugin for auth - here is the code where you can see the Gmongo declaration and assignment of a GMongoClient to it - the line it halts on. This is from the grails datastore mongo plugin 3.0.2 branch. 3.0.0 never used GMongoClient and auth against 2.6+ wouldn't work - 3.0.1 didn't work at all and had dependency issues. In their upcoming grails 3 release they change this all by building up an actual Mongo or MongoClient and then using the GMonog(mongo) constructor which should work. There still might be an advantage to extending and ensuring that using GMongo injections exposes everything MongoClient has. |
I see, thanks for all the feedback. |
In the java mongo driver
MongoClient
actually extendsMongo
. So i think there can be some slight simplification here and addition of this chain of inheritence so it would be more consistent to use. We are running into a case where that was assumed and its failing as a result. We are working on a PR with a fix. just wanted to give you a heads up. Thanks on releasing 1.4 btw.The text was updated successfully, but these errors were encountered: