On Fri, 2012-07-06 at 18:25 +0100, sebb wrote: > On 4 July 2012 10:18, Oleg Kalnichevski <[email protected]> wrote: > > On Tue, 2012-07-03 at 16:39 +0100, sebb wrote: > >> On 3 July 2012 16:04, Oleg Kalnichevski <[email protected]> wrote: > >> > On Tue, 2012-06-26 at 17:55 +0100, sebb wrote: > >> >> On 26 June 2012 14:33, sebb <[email protected]> wrote: > >> >> > On 26 June 2012 13:43, Oleg Kalnichevski <[email protected]> wrote: > >> >> >> On Tue, 2012-06-26 at 12:13 +0100, sebb wrote: > >> >> >>> The setQuery(String) method currently expects an escaped, ASCII-only > >> >> >>> string - basically encoded form data - whereas all the other set > >> >> >>> methods expect unescaped input. > >> >> >>> This is a bit confusing. > >> >> >>> > >> >> >> > >> >> >> Yes, I am aware of it. I was going to deprecate this method in 4.3 > >> >> >> and > >> >> >> replace it with something like #parseQuery. > >> >> >> > >> >> >>> AFAIK, a query string does not have to consist of name value pairs > >> >> >>> (i.e. form data), it can be any arbitrary string. > >> >> >>> There's currently no way for the end-user to provide such a query. > >> >> >>> They would have to use the URI class. > >> >> >> > >> >> >> In this case it is basically a lone parameter without a value. > >> >> > > >> >> > Yes and no; depends what is allowed for a parameter name. > >> >> > >> >> The URI class allows spaces in the query segment. > >> >> AFAICT this is currently impossible to replicate using URIBuilder, > >> >> because it converts space to '+'. > >> >> > >> >> > Also, '+' only means space in the context of form data; otherwise it > >> >> > is just another safe character in the query. > >> >> > > >> >> > Presumably applications that use arbitrary query strings decode the > >> >> > query differently from ones that expect form data. > >> >> > > >> >> >>> Maybe that is OK, in which case we just need to clarify the > >> >> >>> URIBuilder > >> >> >>> Javadoc to state that it is only intended for use with form data > >> >> >>> queries. > >> >> >>> > >> >> >>> Perhaps there should be a setQuery(NVP) method which handles form > >> >> >>> data directly? > >> >> >>> > >> >> >> > >> >> >> Makes sense. The question is whether we should add this method in > >> >> >> 4.2.1 > >> >> >> or wait until 4.3. > >> >> > > >> >> > It could wait. > >> >> > > >> >> > As could providing support for non-form data query strings; the URI > >> >> > class can be used meanwhile. > >> >> > >> > > >> > Sebastian > >> > > >> > I deprecated #setQuery method and added new method to set arbitrary > >> > custom query component. Please review. > >> > > >> > http://svn.apache.org/viewvc?rev=1356775&view=rev > >> > >> Mostly OK. > >> > >> I'm not quite sure why query and queryParams are both used to build > >> the same URI query > >> [Equally, why setCustomQuery does not set queryParams=null.] > >> > >> Is there a use case that requires both a custom query and query params? > >> If so, then there should be a test case for it. > >> > > > > URI uri = new URIBuilder() > > .setCustomQuery("this") > > .addParameter("that", null).build(); > > System.out.println(uri); > > > > What do you see as an expected result of this operation? > > '?this&that' or '?that'? > > I'm not sure the sequence makes sense, so perhaps the code should > throw a runtime error. > > A custom query string is not form data, so space should be encoded as > '%20' rather than '+'. >
I do not know if it makes any sense or not but it is certainly legal. Throwing a runtime exception does not sound quite right to me but I can make custom query and form parameters mutually exclusive if you think that makes more sense. Cheers Oleg --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
