jclouds-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ignasi Barrera <notificati...@github.com>
Subject Re: [jclouds] JCLOUDS-1008: Add @Encoded annotation. (#861)
Date Tue, 06 Oct 2015 08:44:21 GMT
I was a bit confused by the use of the `javax.ws.rs.Encoded` annotation. As I understand its
meaning, it just says "do not decode this parameter; just pass the value as-is".

This made me think that the implementation would delegate to the user the responsibility of
encoding the parameters, which would seem more consistent? If the method is annotated `@Encoded`,
then jclouds will assume everything is properly encoded and won't attempt to encode anything.
With this approach, and now that I properly understand the impl, it makes sense the original
commit where the `@Encoded` annotation was at parameter level: parameters annotated `@Encoded`
should *not* be encoded by jclouds, and the others will (and the URL as a whole won't be encoded
if any parameter is annotated).

The current implementation, and apologies for not having understood properly this in the first
review round, looks a bit difficult to understand: if the method is annotated `@Encoded`,
then the URl as a whole won't be encoded but every path parameter will be encoded.

I think the first one (annotation per param) is consistent with the meaning of the `javax.ws.rs.Encoded`
annotation, and the current one make sense too, but then I'd create a different annotation
`@OnlyEncodeParameters` or similar, to properly illustrate the behavior.

Honestly I think the first approach is cleaner. Apologies for the misunderstanding!

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/861#issuecomment-145785758
Mime
View raw message