This needs to url encode the string, not just quote it. it will be an API change, but it's the right thing to do. We also need to document that we're expecting the string to be in the platform's default character encoding, just to match what is commonly done in the JDK[1].

For example, someone could call setKey() with the following valid JSON:["val1","val2"]
And I think we just need to verify that we're converting it to URL encoded before sending the HTTP GET to the sserver:
%5B%22val1%22%2C%22val2%22%5D

Or, more likely, they're using a JSON library to generate the string that's getting passed in to setKey(). In fact, our test should do that.

I think this is the right behavior for setKey(String key) at the moment. We may later want to add a setRawKey() or a setKey(String key, CouchbaseClient.encoding encoding), where that second param is some kind of encoding scheme we come up with if there's not a good standard one to use. That does bring up that we should probably have setKey() that takes a String and a Charset argument, since they may not supply UTF-8.

Matt Ingenthron
added a comment - 03/Oct/11 12:54 PM - edited This needs to url encode the string, not just quote it. it will be an API change, but it's the right thing to do. We also need to document that we're expecting the string to be in the platform's default character encoding, just to match what is commonly done in the JDK [1] .
For example, someone could call setKey() with the following valid JSON:
["val1","val2"]
And I think we just need to verify that we're converting it to URL encoded before sending the HTTP GET to the sserver:
%5B%22val1%22%2C%22val2%22%5D
The Java method call itself would look like: setKey(" [\"val1\", \"val2\"] ");
Or, more likely, they're using a JSON library to generate the string that's getting passed in to setKey(). In fact, our test should do that.
I think this is the right behavior for setKey(String key) at the moment. We may later want to add a setRawKey() or a setKey(String key, CouchbaseClient.encoding encoding), where that second param is some kind of encoding scheme we come up with if there's not a good standard one to use. That does bring up that we should probably have setKey() that takes a String and a Charset argument, since they may not supply UTF-8.
[1] : http://download.oracle.com/javase/6/docs/api/java/net/URLEncoder.html

Lowering priority because this isn't the cause of a bug. It might be nice to add this in the future, but at the moment it is not a mandatory fix. Also, URLEncoder is deprecated so if this is fixed we should probably use something else for the encoding.

Mike Wiederhold
added a comment - 04/Oct/11 6:19 PM Lowering priority because this isn't the cause of a bug. It might be nice to add this in the future, but at the moment it is not a mandatory fix. Also, URLEncoder is deprecated so if this is fixed we should probably use something else for the encoding.