@pablobm commented on this pull request.

Serving it directly sounds like a good idea! From a quick online lookup, it 
seems that even MS Edge should support it 👍 

> @@ -36,6 +36,21 @@ def show
 
     private
 
+    # The file is stored gzipped on S3. If the client supports gzip, we send it
+    # as-is and the client unzips it, which saves server CPU and bandwidth.
+    # If the client does not support gzip, the server unzips it before sending.
+    def send_gpx(trace)
+      accepts_gzip = request.accept_encoding.to_s.split(",").any? { |encoding| 
encoding.split(";").first.strip == "gzip" }

We are using this gem https://github.com/iain/http_accept_language. I wonder if 
we should replace it with this one and make this less manual 
https://github.com/socketry/http-accept

> @@ -27,6 +27,8 @@
 #  gpx_files_user_id_fkey  (user_id => users.id)
 #
 
+require "zlib"

This shouldn't be necessary, is it?

If necessary, then better move it to the gemfile with `gem "zlib"`.

> @@ -76,10 +78,26 @@ def tagstring=(s)
   def file=(attachable)
     case attachable
     when ActionDispatch::Http::UploadedFile, Rack::Test::UploadedFile
-      super(:io => attachable,
-            :filename => attachable.original_filename,
-            :content_type => content_type(attachable.path),
-            :identify => false)
+      type = content_type(attachable.path)
+
+      if type == "application/gpx+xml"
+        # Gzip plain GPX before upload to S3
+        gz = Tempfile.new(["trace", ".gpx.gz"])

Put this in a block to ensure the file is closed at the end:

```
Tempfile.new(["trace", ".gpx.gz"]) do |gz|
  ...
end
```

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/7124#pullrequestreview-4429537524
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

Reply via email to