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
