On Mon, Jun 25, 2012 at 12:33 PM, Henrik Ingo <henrik.i...@avoinelama.fi> wrote: > if((not _id || strcmp(_id,"")==0) && _req->type==EVHTTP_REQ_DELETE > && !allow_drop_table) > { > generateDropTableError(); > return true; > } > > This is wrong. _id can be given also in query object.
You fixed it. Now remove the TODO :-) > Did this pass testing? If yes, please add a test which catches this error. Did you add a test? I still don't see a test like this: --exec curl -X DELETE 'http://localhost:$JSON_SERVER_PORT/json?schema=json&table=people&query={"query":{"_id": 1 }}' Hmm... For that matter, I still don't see a GET test that does this either: --exec curl 'http://localhost:$JSON_SERVER_PORT/json?schema=json&table=people&query={"query":{"_id": 1 }}' Please add both. (I don't know about curl, you may need to urlencode this part: {"query":{"_id": 1 }} ) > Related to these parts of code, could you please also test the > following bad input and of course also add a test case for it: > > POST /json/schema=test&table=test&_id=1 > > ...with no post data / no query document. Please do this too. > This is ok, but it looks like you don't need to make _exception a > member variable of SQLToJsonGenerator. Since it is only used in one > method, you can instantiate it there: Now you have: sql::Exception _exception= _sql_executor->getException(); Style: Please remove the underscore now that it is a local variable. >> yeah you are right but json_out is changing from outside of this class too , >> That why I passed it into this function. Or another way is use a setter >> function. > > Yes, but different objects can keep a pointer/reference to json_out > and change it when needed. As discussed, here we yield to weird behavior of the json parser :-( Please add the tests, remove the underscore, then do merge proposal. Let's push this baby in! henrik -- henrik.i...@avoinelama.fi +358-40-8211286 skype: henrik.ingo irc: hingo www.openlife.cc My LinkedIn profile: http://www.linkedin.com/profile/view?id=9522559 _______________________________________________ Mailing list: https://launchpad.net/~drizzle-discuss Post to : drizzle-discuss@lists.launchpad.net Unsubscribe : https://launchpad.net/~drizzle-discuss More help : https://help.launchpad.net/ListHelp