Skip to content

feat: migrate S3 storage to aws-sdk v2 and bump to 2.7.0.3 [no-ado]#5

Draft
undefinedacai wants to merge 9 commits intov2.7.0.fix-02from
000-2.7.0.3-aws-sdk-v2-migration
Draft

feat: migrate S3 storage to aws-sdk v2 and bump to 2.7.0.3 [no-ado]#5
undefinedacai wants to merge 9 commits intov2.7.0.fix-02from
000-2.7.0.3-aws-sdk-v2-migration

Conversation

@undefinedacai
Copy link
Copy Markdown
Collaborator

  • Replace AWS::S3.new with Aws::S3::Resource.new
  • Replace s3_interface.buckets[name] with s3_interface.bucket(name)
  • Replace s3_bucket.objects[key] with s3_bucket.object(key)
  • Replace s3_object.write(file, opts) with s3_object.put(body: file, opts)
  • Replace s3_object.read with s3_object.get.body.read
  • Replace s3_interface.buckets.create with s3_interface.create_bucket
  • Replace expiring_url to use Aws::S3::Presigner#presigned_url
  • Replace all AWS::Errors/AWS::S3::Errors rescues with Aws:: equivalents
  • Convert ACL symbols (:public_read) to hyphenated strings ("public-read")
  • Add translate_s3_options to map v1 keys (s3_force_path_style, use_ssl) to their v2 equivalents or drop them appropriately
  • Remove dead establish_connection! and use_secure_protocol? methods
  • Build endpoint from URI scheme instead of :use_ssl option
  • Require :region (falls back to AWS_REGION env or us-east-1)

- Replace AWS::S3.new with Aws::S3::Resource.new
- Replace s3_interface.buckets[name] with s3_interface.bucket(name)
- Replace s3_bucket.objects[key] with s3_bucket.object(key)
- Replace s3_object.write(file, opts) with s3_object.put(body: file, opts)
- Replace s3_object.read with s3_object.get.body.read
- Replace s3_interface.buckets.create with s3_interface.create_bucket
- Replace expiring_url to use Aws::S3::Presigner#presigned_url
- Replace all AWS::Errors/AWS::S3::Errors rescues with Aws:: equivalents
- Convert ACL symbols (:public_read) to hyphenated strings ("public-read")
- Add translate_s3_options to map v1 keys (s3_force_path_style, use_ssl)
  to their v2 equivalents or drop them appropriately
- Remove dead establish_connection! and use_secure_protocol? methods
- Build endpoint from URI scheme instead of :use_ssl option
- Require :region (falls back to AWS_REGION env or us-east-1)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates Paperclip’s S3 storage backend from aws-sdk v1-style APIs to aws-sdk v2 APIs and bumps the gem version.

Changes:

  • Updated S3 client/resource usage, bucket/object access, uploads/downloads, and error classes to aws-sdk v2 equivalents.
  • Replaced expiring URL generation with Aws::S3::Presigner#presigned_url and adjusted ACL handling to v2’s expected format.
  • Bumped Paperclip version to 2.7.0.3.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
lib/paperclip/version.rb Version bump to reflect the S3 SDK migration release.
lib/paperclip/storage/s3.rb Core migration of S3 storage implementation to aws-sdk v2 patterns, including presigned URLs and option translation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/paperclip/storage/s3.rb
Comment thread lib/paperclip/storage/s3.rb
Comment thread lib/paperclip/storage/s3.rb
undefinedacai and others added 2 commits April 9, 2026 12:40
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix exists? method: add missing rescue body (false) and closing end
- Update expiring_url tests to mock Aws::S3::Presigner instead of url_for
- Update flush_writes tests to assert put() with :body and string ACLs
- Migrate test setup from AWS v1 (require 'aws', AWS.config) to v2
- Update remaining v1 class references in tests (bucket creation, delete, exists)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/paperclip/storage/s3.rb
Comment thread lib/paperclip/storage/s3.rb
Comment thread paperclip.gemspec
Comment thread test/storage/s3_test.rb
Comment thread test/storage/s3_test.rb
- Fix s3_interface endpoint: only set custom :endpoint for non-AWS hosts
  (match *.amazonaws.com) instead of just s3.amazonaws.com, preventing
  signature/redirect issues with AWS regional hostnames
- Fix s3_protocol default proc: normalize permission to string before
  comparing, so both :public_read and "public-read" resolve correctly
- Remove Gemfile.lock from tracking (already in .gitignore) since it
  still pinned aws-sdk v1
- Update test s3_endpoint/config assertions to use s3_host_name and
  s3_credentials instead of v1 s3_bucket.config accessors
- Update all remaining .write expectations to .put with :body and
  string ACLs across 8 additional test locations

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/paperclip/storage/s3.rb
Comment thread lib/paperclip/storage/s3.rb
Comment thread lib/paperclip/storage/s3.rb Outdated
- Infer :region from s3_host_name when it matches an AWS regional
  pattern (e.g. s3-ap-northeast-1.amazonaws.com) and no explicit
  region is provided, preventing wrong-region requests
- Translate legacy :s3_endpoint option to :endpoint in
  translate_s3_options to avoid ArgumentError from aws-sdk v2
- Preserve s3_protocol behavior in expiring_url by applying protocol
  override to presigned URLs via apply_s3_protocol_to_url helper

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/paperclip/storage/s3.rb Outdated
Add aws_region_token? guard so infer_region_from_host only returns
a region for tokens matching the AWS pattern (e.g. us-east-1,
ap-northeast-1), preventing invalid regions from hostnames like
s3-world-end.amazonaws.com.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/paperclip/storage/s3.rb
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/storage/s3_test.rb Outdated
Comment thread test/storage/s3_test.rb Outdated
Comment thread test/storage/s3_test.rb Outdated
Comment thread lib/paperclip/storage/s3.rb
… fix presigner test returns [no-ado]

- Upcase storage_class and server_side_encryption in flush_writes for v2 compat
  (e.g. :reduced_redundancy -> REDUCED_REDUNDANCY, :aes256 -> AES256)
- Add .returns() to presigner expectations so apply_s3_protocol_to_url
  doesn't raise NoMethodError on nil
- Update test assertions to expect uppercase enum strings

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/paperclip/storage/s3.rb Outdated
Comment thread paperclip.gemspec
String#match? was added in Ruby 2.4; use =~ instead since
aws-sdk v2 supports Ruby >= 1.9.3.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants