[openstreetmap/openstreetmap-website] Compress plain GPX uploads with gzip (PR #7124)

[Pablo Brasero]

@pablobm commented on this pull request.



> @@ -226,6 +224,7 @@ csp_enforce: false
 #csp_report_url: ""
 # Storage services to use in production mode
 avatar_storage: "local"
+# Use a GpxS3 service to store plain GPX gzipped on S3.

```suggestion
# Use the GpxS3 service to automatically gzip plain GPX files before storing them in S3.
```

On test/lib/active_storage/service/gpx_s3_service_test.rb:

This test file is only covering the `upload_with_single_part` case. We should be covering the multipart case too.

It should be possible to DRY this so that code doesn't need to be repeated. I played a bit with it and got this:

```diff
diff --git a/test/lib/active_storage/service/gpx_s3_service_test.rb b/test/lib/active_storage/service/gpx_s3_service_test.rb
index a010b126e..17b1e0d6b 100644
--- a/test/lib/active_storage/service/gpx_s3_service_test.rb
+++ b/test/lib/active_storage/service/gpx_s3_service_test.rb
@@ -3,6 +3,25 @@
 require "test_helper"

 class GpxS3ServiceTest < ActiveSupport::TestCase
+  class MultipartTest < self
+    def setup
+      @service = ActiveStorage::Service::GpxS3Service.new(
+        :bucket => "test-bucket",
+        :region => "eu-west-1",
+        :stub_responses => true,
+        :upload => {
+          :multipart_threshold => 1.byte
+        }
+      )
+    end
+
+    private
+
+    def find_latest_upload_request
+      @service.client.client.api_requests.reverse_each.find { |request| request[:operation_name] == :create_multipart_upload }
+    end
+  end
+
   def setup
     @service = ActiveStorage::Service::GpxS3Service.new(
       :bucket => "test-bucket",
@@ -16,7 +35,7 @@ class GpxS3ServiceTest < ActiveSupport::TestCase
                     :content_type => "application/gpx+xml",
                     :custom_metadata => { "server_gzipped" => true })

-    request = find_latest_put_object_request
+    request = find_latest_upload_request
     assert_equal "gzip", request[:params][:content_encoding]
     # The flags drive the header, they aren't stored as S3 metadata.
     assert_empty(request[:params][:metadata] || {})
@@ -25,7 +44,7 @@ class GpxS3ServiceTest < ActiveSupport::TestCase
   def test_keeps_default_upload_without_server_gzipped_flag
     @service.upload("key", StringIO.new("data"), :content_type => "application/gpx+xml")

-    assert_nil find_latest_put_object_request[:params][:content_encoding]
+    assert_nil find_latest_upload_request[:params][:content_encoding]
   end

   def test_keeps_metadata_flag
@@ -51,7 +70,7 @@ class GpxS3ServiceTest < ActiveSupport::TestCase

   private

-  def find_latest_put_object_request
+  def find_latest_upload_request
     @service.client.client.api_requests.reverse_each.find { |request| request[:operation_name] == :put_object }
   end
 end
```

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/7124#pullrequestreview-4746377976
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/7124/review/4746377976 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20260721/cdaab85a/attachment.htm>