Redirect as plain text with message.

[tylerdooling]

Summary

This change forces redirect to use a content type of text/plain to both bypass any serialization attempts by the formatters and more closely meet the spec defined for redirection.

Backstory

Original issue #1157. Related issues #1162, #1159

The original issue here was concerned with the XML formatter attempting to serialize an empty body when an redirect was invoked. The situation can happen when an API is limited to XML only, or when XML is supported and requested.

The spec

The HTTP spec for redirection states:

Unless the request method was HEAD, the entity of the response SHOULD contain a short hypertext note with a hyperlink to the new URI(s).

I interpret this as indicating that a message body is preferred when responding with a redirect for statuses 301, 302, 303.

Proposal

In #1162 I brought up a couple of ideas to address this problem:

• set the body of a redirect to an object that supports all default serializations - #to_json, #to_xml, #to_s - and allow the client to negotiate the content type
• default redirect to use text/plain and allow the user to override when invoked

As I though about it over the weekend a third option seemed to make sense - really a derivation of the second above - where we just simple force the content type to text/plain and add a nicely formed message that fulfills the theme of the spec.

This is relatively safe in that it doesn't limit the framework from supporting optional extensions for redirects in the future. The biggest gain I think is that it removes complication for 99% of the users out there as it will 'just work' as I believe they'll expect it to even if they choose to limit or customize content types.

Hopefully this will work or at least spark a solution. Please let me know if you all have better ideas.

[dblock]

I would be down with merging this.

I wonder whether the body should not be set if there's already one?
How do I restore old behavior if I want it? I think that should be clearly explained in UPGRADING.

Anyone has any other opinions?

[tylerdooling]

Good question. I took the path of setting it regardless because it was being cleared unconditionally before. Are you thinking it would be good allow an option for backward compatibility like omit_body: true or a body override potentially? That would provide a path for replicating legacy behavior if need be.

[dblock]

I think we want a body override, but I don't want to pollute the method with a message, so I would do something like:

body message unless body

[tylerdooling]

Should we be concerned about legacy implementations that set a body and then made the decision to redirect? Previously there would have been no body in the redirect response and with the above logic there now would be. Additionally, that body might not have anything to do with the redirect, though I may well be misjudging the likelihood of that.

[dblock]

@tylerdooling True. I want to leave this open for a bit and see what others say.

[franzliedke]

👍

[dblock]

Why don't we just allow specifying message inside options and call it a day? @tylerdooling ?

[tylerdooling]

So just to clarify, are you saying that we no longer force the text/plain content type and allow users to supply an optional message to deal with serialization issues?

[dblock]

I think we should force text/plain, all I am saying is allow supplying the assignment to message =.

[tylerdooling]

Thanks for clarifying @dblock - makes sense. Should be adjusted accordingly now.

[dblock]

One last nitpick, sorry. These are full sentences, right? So they should have a period at the end I think.

[dblock]

I feel bad writing this comment, so thanks for hanging in here @tylerdooling!

[tylerdooling]

No worries - good point.

Should we add a validator to the optional message override to ensure it adheres grammatically or just open up a separate issue for that? 😄

[dblock]

I don't think we need a validator, if someone wants to send something as
text in another language we shouldn't impose English rules :)

On Tue, Nov 10, 2015 at 10:16 AM, Tyler Dooling notifications@github.com
wrote:

In lib/grape/dsl/inside_route.rb
#1194 (comment):

       status 301

•      body_message ||= "This resource has been moved permanently to #{url}"
  

No worries - good point.

Should we add a validator to the optional message override to ensure it
adheres grammatically or just open up a separate issue for that? [image:
😄]

—
Reply to this email directly or view it on GitHub
https://github.com/ruby-grape/grape/pull/1194/files#r44417636.

dB. | Moscow - Geneva - Seattle - New York
code.dblock.org - @dblockdotorg http://twitter.com/#!/dblockdotorg -
artsy.net - github/dblock https://github.com/dblock

[dblock]

Build borked ;)

[tylerdooling]

De-borked.

[dblock]

Merged.