Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot pass null to @Body #1488

Closed
danvinokour opened this issue Jan 18, 2016 · 9 comments
Closed

Cannot pass null to @Body #1488

danvinokour opened this issue Jan 18, 2016 · 9 comments
Milestone

Comments

@danvinokour
Copy link

I have a call which gets "@Body RequestBody body"
I dont have always a need to pass body to it nd i cannot use null.

@JayNewstrom
Copy link
Contributor

@danvinokour
Copy link
Author

Yeah, but i have api where i dont always have to pass @Body so this behaviour makes me pass empty RequestBody.
Why cant it be like @field? (When null it is just omitted)

@JakeWharton
Copy link
Member

I'm not sure about supporting this. Passing null is almost always an inadvertent application bug, so it's nice to catch that case. Beyond that, we couldn't even send a Content-Type header if we did support null because the Converter would never be called to produce one.

@danvinokour
Copy link
Author

Well its a real case, before moving to retrofit we used to form this request with the relevant content-type header if it had body.
Now you cannot just rewrite the call so this dynamic behaviour is kind of expected.
Like you can handle null Fields and Queries i think you should handle null bodies.

@JakeWharton JakeWharton modified the milestone: 2.0 Feb 4, 2016
@JakeWharton
Copy link
Member

@swankjesse have an opinion here? We support creating an empty body automatically when no @Body is specified for an HTTP method that requires it (e.g., @POST). Should we support passing null to a @Body parameter and do the same thing?

@Tolriq
Copy link

Tolriq commented Feb 22, 2016

Reading this it seems converters would never be called for null even if accepted.

How should we handle JSON RPC 2.0 where params (so in that case body object) can be omitted ? (Based on https://github.com/segmentio/retrofit-jsonrpc).

To have the converter call we need to pass an object but to completely omit the params part we'd need to either add a param or use a specific object and test for it.

Is there a better way ?

@JakeWharton
Copy link
Member

@swankjesse ping. got an opinion here?

@JakeWharton
Copy link
Member

@Tolriq We recommend you either use a sentinel value (Whatever.NONE) or a wrapper type (like Optional or equivalent-style "box") to indicate no body. This way the correct converter can still be selected and it can choose to still create the JSON-RPC envelope.

@JakeWharton
Copy link
Member

So we aren't going to make any fundamental changes. I'm going to try and encapsulate all our thinking here.

null is allowed for @Query, @Field, and @Part because these are fractions of the thing they are assembling and their omission has no effect on the larger construction of the URL or body in a way that makes things ambiguous. It's worth noting here that multipart requires at least one non-null part because it would make things ambiguous (and violates the MIME spec) if we had an empty multipart body.

null is currently allowed for @FieldMap and @PartMap values which causes them to be omitted from the request. We're going to change this to be forbidden. Just omit the values from the map.

null is not allowed for @Body because it creates an ambiguity around the Content-Type header. When you omit a body param we send an empty body with no content type. You can override this behavior with a @Headers annotation on the method or a @Header("Content-Type") param. For @Body methods, we have selected a Converter which provides the Content-Type header, but since it won't be called for null (because all converters can't support null) there won't be a header. This makes the behavior subtly different. I also reiterate that allowing null for the @Body argument has the potential to hide programming errors.

If you want to create a request with an optional body, we are going to recommend you create an overload without a @Body param (and potentially with an explicit content type header) that you conditionally call from your application code. This make the desire to have an empty body explicit.

Remember, while Retrofit offers a lot of flexibility in customizing the service interfaces so that they are easy to use, it is not a high-level interface that can encode every business logic rule of your application layer. Sometimes you need to leverage multiple methods on the service interface conditionally from your application code. If you're using Java 8 default methods work great here, otherwise a helper method or completely abstracting Retrofit interfaces behind a domain-specific API for your application works well here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants