@pablobm commented on this pull request.
The PR description is now out of date, with the mention of Windows users, etc.
Mind updating it? It'll make it easier to future readers.
General question: do we have any idea of how common is it to receive requests
for the xml/gpx versions of the traces? The `request.format == Mime[:xml]`, etc.
The website doesn't link to those, so I guess any website visitors will get the
`trace.file.attached?` version. However the [API docs do describe this
option](https://wiki.openstreetmap.org/wiki/API_v0.6#Download_Data:_GET_/api/0.6/gpx/#id/data):
> The response will always be a GPX format file if you use a .gpx URL suffix, a
> XML file in an undocumented format if you use a .xml URL suffix, otherwise
> the response will be the exact file that was uploaded.
Is this something that is very common and could generate this extra effort on
our side, decompressing every time?
> + if accepts_gzip
+ response.headers["Content-Encoding"] = "gzip"
+ response.headers["Vary"] = [response.headers["Vary"],
"Accept-Encoding"].compact.join(", ")
+ send_data(trace.file.download, :filename => filename, :type =>
"application/gpx+xml", :disposition => "attachment")
+ else
+ send_data(trace.xml_file.read, :filename => filename, :type =>
"application/gpx+xml", :disposition => "attachment")
```suggestion
data =
if accepts_gzip
response.headers["Content-Encoding"] = "gzip"
response.headers["Vary"] = [response.headers["Vary"],
"Accept-Encoding"].compact.join(", ")
trace.file.download
else
trace.xml_file.read
send_data(data, :filename => filename, :type => "application/gpx+xml",
:disposition => "attachment")
```
> @@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module GpxDownloadMethods
+ extend ActiveSupport::Concern
+
+ private
+
+ # The server gzipped the plain GPX on upload. Now when the user requests the
file and
+ # the client accepts gzip, send it as gzip and let the client unzip it, if
not, unzip in the server and send
+ def send_gpx_from_gzip(trace, filename)
+ accepts_gzip = request.accept_encoding.to_s.split(",").any? { |encoding|
encoding.split(";").first.strip == "gzip" }
+
+ if accepts_gzip
+ response.headers["Content-Encoding"] = "gzip"
+ response.headers["Vary"] = [response.headers["Vary"],
"Accept-Encoding"].compact.join(", ")
I wouldn't have thought of this 😅
On app/controllers/traces/data_controller.rb:
Not for this PR, but the `show` actions in both controllers are so similar that
I have to wonder if the differences are intentional. They do feel a bit strange.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/7124#pullrequestreview-4478484875
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/7124/review/[email protected]>_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
