[openstreetmap/openstreetmap-website] JSON output nodes, ways, relations, map (#2485)

[mmd]

mmd-osm commented on this pull request.



> @@ -3,6 +3,52 @@ class ApiController < ApplicationController
 
   private
 
+  ##
+  # Set default request format to xml unless a client requests a specific format,
+  # which can be done via (a) URL suffix and/or (b) HTTP Accept header, where
+  # the URL suffix always takes precedence over the Accept header.
+  def set_default_request_format

>The edge-case is that if the header exists, and is parseable, but doesn't contain either xml, json, or '/', then request.format isn't set. I'm not sure if that has any consequences though.

That's not really an edge case, leaving request.format as is is intended behaviour and covered by the last Accept header unit test  "text/json is in invalid format, ActionController::UnknownFormat error is expected". If a user requests a single format we don't support at all, we can't fall back to XML here, as it would be violating the respective RFC.

In any case, it would be better to return 406 Not Acceptable instead of HTTP 500. Somehow the Rails framework doesn't handle this properly. In any case, that's outside of anything I control.

>  assume request.format = "xml" on the first line of the function. 

No, don't do that. As I said, it would introduce incorrect behaviour.

> , if you've had enough of refactoring this function already, I understand.

Right. No more refactoring from my side. I needed some code I can still read in a week's time. You're of course free to make it look more Rails like. It would take me more than an hour and the result would still be poor.



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/2485#discussion_r369617630
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20200122/e8a3d9c0/attachment.htm>