Retrofit: New @Path not working with encoded=True

[lunoho]

Hello!

Rewritting a project for retrofit2, and it looks like the new encoded value is not being evaluated properly.

@GET("{path}/venues/")
Observable <ArrayList<Venue>> getVenuesForPath(@Path(value = "path", encoded = true) String path);

When I call the above with client.getVenuesForPath("this/is/a/test"), the resulting URL improperly encodes the slashes to "this%2Fis%2Fa%2Ftest".

Thanks again for a great utility, really appreciate all the hard work.

[tomkoptel]

After looking in sources it looks like the following condition makes decision to encode path without respecting implicit encoded flag.

@JakeWharton is that acceptable to have this type of condition? I think this should help to fix an issue.

if ( 
 codePoint < 0x20 || codePoint >= 0x7f 
 || (PATH_SEGMENT_ENCODE_SET.indexOf(codePoint) != -1 && !alreadyEncoded)
 || (codePoint == '%' && !alreadyEncoded) ) {
// ...
}

So far this condition will not encode "/" symbol, as soon as we have explicitly enforced check on alreadyEncoded flag.

[JakeWharton]

The code is copied from OkHttp and will be removed when #1129 is done. This is very similar to the rejected PR #1159.

[dneves]

Hi. Was using beta1 and encoded=true was working fine... Now upgraded to beta2 to get the new stuff (specifically the fix on the @DELETE method without body) but now encoded=true is not working

[JakeWharton]

For the record, this _is_ actually is working properly as / is not a valid character inside a path segment which is how the replacement is being interpreted.

If you are replacing multiple segments in the path, you can use @Url on the first parameter of the method and omit the relative URL from the HTTP method annotation. This will let you pass the full relative URL (or even a whole absolute URL!) to the method.

It's unclear whether or not we'll actually support path replacements interpreted as multiple segments at this time.

[elektron9]

@JakeWharton could you provide a small code snippet of the solution in your last comment?

[JakeWharton]

https://github.com/square/retrofit/blob/1887e011a119190759e3eb8aa6af359362765b51/retrofit/src/test/java/retrofit/RequestBuilderTest.java#L900-L913

[elektron9]

@JakeWharton excellent, thank you!

[JakeWharton]

Path replacements that span multiple path segments aren't going to be supported, you should use @Url to build the full relative URL programmatically if the number of path segments varies dynamically. See the link above for an example of its use. @Path replacement, whether encoded is set to true or false, will always encode the / character because it replaces inside single path segments. Without this behavior, user input that is included in path segments would erroneously break the URL.