diff --git a/README.md b/README.md index 73602db219..b768520756 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # Active Storage -Active Storage makes it simple to upload and reference files in cloud sites, like Amazon S3 or Google Cloud Storage, -and attach those files to Active Records. It also provides a disk site for testing or local deployments, but the +Active Storage makes it simple to upload and reference files in cloud services, like Amazon S3 or Google Cloud Storage, +and attach those files to Active Records. It also provides a disk service for testing or local deployments, but the focus is on cloud storage. ## Example @@ -55,7 +55,7 @@ end ## Configuration -Add `require "active_storage"` to config/application.rb and create a `config/initializers/active_storage_sites.rb` with the following: +Add `require "active_storage"` to config/application.rb and create a `config/initializers/active_storage_services.rb` with the following: ```ruby @@ -65,10 +65,10 @@ Add `require "active_storage"` to config/application.rb and create a `config/ini - Strip Download of its resposibilities and delete class - Proper logging -- Convert MirrorSite to use threading +- Convert MirrorService to use threading - Read metadata via Marcel? - Copy over migration to app via rake task -- Add Migrator to copy/move between sites +- Add Migrator to copy/move between services - Explore direct uploads to cloud - Extract VerifiedKeyWithExpiration into Rails as a feature of MessageVerifier diff --git a/lib/active_storage.rb b/lib/active_storage.rb index e87eb8a506..f72fe0d017 100644 --- a/lib/active_storage.rb +++ b/lib/active_storage.rb @@ -5,5 +5,5 @@ module ActiveStorage extend ActiveSupport::Autoload autoload :Blob - autoload :Site + autoload :Service end diff --git a/lib/active_storage/blob.rb b/lib/active_storage/blob.rb index edf57b5c78..4ce344e2a1 100644 --- a/lib/active_storage/blob.rb +++ b/lib/active_storage/blob.rb @@ -1,4 +1,4 @@ -require "active_storage/site" +require "active_storage/service" require "active_storage/filename" require "active_storage/purge_job" @@ -9,7 +9,7 @@ class ActiveStorage::Blob < ActiveRecord::Base has_secure_token :key store :metadata, coder: JSON - class_attribute :site + class_attribute :service class << self def build_after_upload(io:, filename:, content_type: nil, metadata: nil) @@ -37,24 +37,24 @@ class ActiveStorage::Blob < ActiveRecord::Base end def url(expires_in: 5.minutes, disposition: :inline) - site.url key, expires_in: expires_in, disposition: disposition, filename: filename + service.url key, expires_in: expires_in, disposition: disposition, filename: filename end def upload(io) - site.upload(key, io) + service.upload(key, io) - self.checksum = site.checksum(key) - self.byte_size = site.byte_size(key) + self.checksum = service.checksum(key) + self.byte_size = service.byte_size(key) end def download - site.download key + service.download key end def delete - site.delete key + service.delete key end def purge diff --git a/lib/active_storage/config/sites.yml b/lib/active_storage/config/sites.yml index 43bc77fbf9..317ef2b9b7 100644 --- a/lib/active_storage/config/sites.yml +++ b/lib/active_storage/config/sites.yml @@ -1,25 +1,25 @@ # Configuration should be something like this: # # config/environments/development.rb -# config.active_storage.site = :local +# config.active_storage.service = :local # # config/environments/production.rb -# config.active_storage.site = :amazon +# config.active_storage.service = :amazon local: - site: Disk + service: Disk root: <%%= File.join(Dir.tmpdir, "active_storage") %> amazon: - site: S3 + service: S3 access_key_id: <%%= Rails.application.secrets.aws[:access_key_id] %> secret_access_key: <%%= Rails.application.secrets.aws[:secret_access_key] %> region: us-east-1 bucket: <%= Rails.application.class.name.remove(/::Application$/).underscore %> google: - site: GCS + service: GCS mirror: - site: Mirror + service: Mirror primary: amazon secondaries: google diff --git a/lib/active_storage/site.rb b/lib/active_storage/service.rb similarity index 58% rename from lib/active_storage/site.rb rename to lib/active_storage/service.rb index b3b0221c63..038b6ccb53 100644 --- a/lib/active_storage/site.rb +++ b/lib/active_storage/service.rb @@ -1,11 +1,11 @@ -# Abstract class serving as an interface for concrete sites. -class ActiveStorage::Site - def self.configure(site, **options) +# Abstract class serving as an interface for concrete services. +class ActiveStorage::Service + def self.configure(service, **options) begin - require "active_storage/site/#{site.to_s.downcase}_site" - ActiveStorage::Site.const_get(:"#{site}Site").new(**options) + require "active_storage/service/#{service.to_s.downcase}_service" + ActiveStorage::Service.const_get(:"#{service}Service").new(**options) rescue LoadError => e - puts "Couldn't configure site: #{site} (#{e.message})" + puts "Couldn't configure service: #{service} (#{e.message})" end end diff --git a/lib/active_storage/site/disk_site.rb b/lib/active_storage/service/disk_service.rb similarity index 95% rename from lib/active_storage/site/disk_site.rb rename to lib/active_storage/service/disk_service.rb index 2ff0b22fae..6977b5b82e 100644 --- a/lib/active_storage/site/disk_site.rb +++ b/lib/active_storage/service/disk_service.rb @@ -1,7 +1,7 @@ require "fileutils" require "pathname" -class ActiveStorage::Site::DiskSite < ActiveStorage::Site +class ActiveStorage::Service::DiskService < ActiveStorage::Service attr_reader :root def initialize(root:) diff --git a/lib/active_storage/site/gcs_site.rb b/lib/active_storage/service/gcs_service.rb similarity index 93% rename from lib/active_storage/site/gcs_site.rb rename to lib/active_storage/service/gcs_service.rb index bf681ca6a2..18ec1de133 100644 --- a/lib/active_storage/site/gcs_site.rb +++ b/lib/active_storage/service/gcs_service.rb @@ -1,7 +1,7 @@ require "google/cloud/storage" require "active_support/core_ext/object/to_query" -class ActiveStorage::Site::GCSSite < ActiveStorage::Site +class ActiveStorage::Service::GCSService < ActiveStorage::Service attr_reader :client, :bucket def initialize(project:, keyfile:, bucket:) diff --git a/lib/active_storage/service/mirror_service.rb b/lib/active_storage/service/mirror_service.rb new file mode 100644 index 0000000000..2a3518e59e --- /dev/null +++ b/lib/active_storage/service/mirror_service.rb @@ -0,0 +1,51 @@ +class ActiveStorage::Service::MirrorService < ActiveStorage::Service + attr_reader :services + + def initialize(services:) + @services = services + end + + def upload(key, io) + services.collect do |service| + service.upload key, io + io.rewind + end + end + + def download(key) + services.detect { |service| service.exist?(key) }.download(key) + end + + def delete(key) + perform_across_services :delete, key + end + + def exist?(key) + perform_across_services(:exist?, key).any? + end + + + def url(key, **options) + primary_service.url(key, **options) + end + + def byte_size(key) + primary_service.byte_size(key) + end + + def checksum(key) + primary_service.checksum(key) + end + + private + def primary_service + services.first + end + + def perform_across_services(method, *args) + # FIXME: Convert to be threaded + services.collect do |service| + service.public_send method, *args + end + end +end diff --git a/lib/active_storage/site/s3_site.rb b/lib/active_storage/service/s3_service.rb similarity index 95% rename from lib/active_storage/site/s3_site.rb rename to lib/active_storage/service/s3_service.rb index 65dad37cfe..811321a172 100644 --- a/lib/active_storage/site/s3_site.rb +++ b/lib/active_storage/service/s3_service.rb @@ -1,6 +1,6 @@ require "aws-sdk" -class ActiveStorage::Site::S3Site < ActiveStorage::Site +class ActiveStorage::Service::S3Service < ActiveStorage::Service attr_reader :client, :bucket def initialize(access_key_id:, secret_access_key:, region:, bucket:) diff --git a/lib/active_storage/site/mirror_site.rb b/lib/active_storage/site/mirror_site.rb deleted file mode 100644 index ba3ef0ef0e..0000000000 --- a/lib/active_storage/site/mirror_site.rb +++ /dev/null @@ -1,51 +0,0 @@ -class ActiveStorage::Site::MirrorSite < ActiveStorage::Site - attr_reader :sites - - def initialize(sites:) - @sites = sites - end - - def upload(key, io) - sites.collect do |site| - site.upload key, io - io.rewind - end - end - - def download(key) - sites.detect { |site| site.exist?(key) }.download(key) - end - - def delete(key) - perform_across_sites :delete, key - end - - def exist?(key) - perform_across_sites(:exist?, key).any? - end - - - def url(key, **options) - primary_site.url(key, **options) - end - - def byte_size(key) - primary_site.byte_size(key) - end - - def checksum(key) - primary_site.checksum(key) - end - - private - def primary_site - sites.first - end - - def perform_across_sites(method, *args) - # FIXME: Convert to be threaded - sites.collect do |site| - site.public_send method, *args - end - end -end diff --git a/test/attachments_test.rb b/test/attachments_test.rb index 6e25002bb1..33bbff716d 100644 --- a/test/attachments_test.rb +++ b/test/attachments_test.rb @@ -36,7 +36,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase @user.avatar.purge assert_not @user.avatar.attached? - assert_not ActiveStorage::Blob.site.exist?(avatar_key) + assert_not ActiveStorage::Blob.service.exist?(avatar_key) end test "purge attached blob later when the record is destroyed" do @@ -47,7 +47,7 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase @user.destroy assert_nil ActiveStorage::Blob.find_by(key: avatar_key) - assert_not ActiveStorage::Blob.site.exist?(avatar_key) + assert_not ActiveStorage::Blob.service.exist?(avatar_key) end end @@ -74,8 +74,8 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase @user.highlights.purge assert_not @user.highlights.attached? - assert_not ActiveStorage::Blob.site.exist?(highlight_keys.first) - assert_not ActiveStorage::Blob.site.exist?(highlight_keys.second) + assert_not ActiveStorage::Blob.service.exist?(highlight_keys.first) + assert_not ActiveStorage::Blob.service.exist?(highlight_keys.second) end test "purge attached blobs later when the record is destroyed" do @@ -86,10 +86,10 @@ class ActiveStorage::AttachmentsTest < ActiveSupport::TestCase @user.destroy assert_nil ActiveStorage::Blob.find_by(key: highlight_keys.first) - assert_not ActiveStorage::Blob.site.exist?(highlight_keys.first) + assert_not ActiveStorage::Blob.service.exist?(highlight_keys.first) assert_nil ActiveStorage::Blob.find_by(key: highlight_keys.second) - assert_not ActiveStorage::Blob.site.exist?(highlight_keys.second) + assert_not ActiveStorage::Blob.service.exist?(highlight_keys.second) end end end diff --git a/test/site/.gitignore b/test/service/.gitignore similarity index 100% rename from test/site/.gitignore rename to test/service/.gitignore diff --git a/test/site/configurations-example.yml b/test/service/configurations-example.yml similarity index 100% rename from test/site/configurations-example.yml rename to test/service/configurations-example.yml diff --git a/test/service/disk_service_test.rb b/test/service/disk_service_test.rb new file mode 100644 index 0000000000..5dd7cff303 --- /dev/null +++ b/test/service/disk_service_test.rb @@ -0,0 +1,8 @@ +require "tmpdir" +require "service/shared_service_tests" + +class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase + SERVICE = ActiveStorage::Service.configure(:Disk, root: File.join(Dir.tmpdir, "active_storage")) + + include ActiveStorage::Service::SharedServiceTests +end diff --git a/test/service/gcs_service_test.rb b/test/service/gcs_service_test.rb new file mode 100644 index 0000000000..42f9cd3061 --- /dev/null +++ b/test/service/gcs_service_test.rb @@ -0,0 +1,20 @@ +require "service/shared_service_tests" + +if SERVICE_CONFIGURATIONS[:gcs] + class ActiveStorage::Service::GCSServiceTest < ActiveSupport::TestCase + SERVICE = ActiveStorage::Service.configure(:GCS, SERVICE_CONFIGURATIONS[:gcs]) + + include ActiveStorage::Service::SharedServiceTests + + test "signed URL generation" do + travel_to Time.now do + url = SERVICE.bucket.signed_url(FIXTURE_KEY, expires: 120) + + "&response-content-disposition=inline%3B+filename%3D%22test.txt%22" + + assert_equal url, @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: "test.txt") + end + end + end +else + puts "Skipping GCS Service tests because no GCS configuration was supplied" +end diff --git a/test/service/mirror_service_test.rb b/test/service/mirror_service_test.rb new file mode 100644 index 0000000000..3b22c4f049 --- /dev/null +++ b/test/service/mirror_service_test.rb @@ -0,0 +1,30 @@ +require "tmpdir" +require "service/shared_service_tests" + +class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase + PRIMARY_DISK_SERVICE = ActiveStorage::Service.configure(:Disk, root: File.join(Dir.tmpdir, "active_storage")) + SECONDARY_DISK_SERVICE = ActiveStorage::Service.configure(:Disk, root: File.join(Dir.tmpdir, "active_storage_mirror")) + + SERVICE = ActiveStorage::Service.configure :Mirror, services: [ PRIMARY_DISK_SERVICE, SECONDARY_DISK_SERVICE ] + + include ActiveStorage::Service::SharedServiceTests + + test "uploading was done to all services" do + begin + key = SecureRandom.base58(24) + data = "Something else entirely!" + io = StringIO.new(data) + @service.upload(key, io) + + assert_equal data, PRIMARY_DISK_SERVICE.download(key) + assert_equal data, SECONDARY_DISK_SERVICE.download(key) + ensure + @service.delete key + end + end + + test "existing in all services" do + assert PRIMARY_DISK_SERVICE.exist?(FIXTURE_KEY) + assert SECONDARY_DISK_SERVICE.exist?(FIXTURE_KEY) + end +end diff --git a/test/service/s3_service_test.rb b/test/service/s3_service_test.rb new file mode 100644 index 0000000000..604dfd6c60 --- /dev/null +++ b/test/service/s3_service_test.rb @@ -0,0 +1,11 @@ +require "service/shared_service_tests" + +if SERVICE_CONFIGURATIONS[:s3] + class ActiveStorage::Service::S3ServiceTest < ActiveSupport::TestCase + SERVICE = ActiveStorage::Service.configure(:S3, SERVICE_CONFIGURATIONS[:s3]) + + include ActiveStorage::Service::SharedServiceTests + end +else + puts "Skipping S3 Service tests because no S3 configuration was supplied" +end diff --git a/test/service/shared_service_tests.rb b/test/service/shared_service_tests.rb new file mode 100644 index 0000000000..16672ab49b --- /dev/null +++ b/test/service/shared_service_tests.rb @@ -0,0 +1,63 @@ +require "test_helper" +require "active_support/core_ext/securerandom" +require "yaml" + +SERVICE_CONFIGURATIONS = begin + YAML.load_file(File.expand_path("../configurations.yml", __FILE__)).deep_symbolize_keys +rescue Errno::ENOENT + puts "Missing service configuration file in test/services/configurations.yml" +end + +module ActiveStorage::Service::SharedServiceTests + extend ActiveSupport::Concern + + FIXTURE_KEY = SecureRandom.base58(24) + FIXTURE_FILE = StringIO.new("Hello world!") + + included do + setup do + @service = self.class.const_get(:SERVICE) + @service.upload FIXTURE_KEY, FIXTURE_FILE + FIXTURE_FILE.rewind + end + + teardown do + @service.delete FIXTURE_KEY + FIXTURE_FILE.rewind + end + + test "uploading" do + begin + key = SecureRandom.base58(24) + data = "Something else entirely!" + @service.upload(key, StringIO.new(data)) + + assert_equal data, @service.download(key) + ensure + @service.delete key + end + end + + test "downloading" do + assert_equal FIXTURE_FILE.read, @service.download(FIXTURE_KEY) + end + + test "existing" do + assert @service.exist?(FIXTURE_KEY) + assert_not @service.exist?(FIXTURE_KEY + "nonsense") + end + + test "deleting" do + @service.delete FIXTURE_KEY + assert_not @service.exist?(FIXTURE_KEY) + end + + test "sizing" do + assert_equal FIXTURE_FILE.size, @service.byte_size(FIXTURE_KEY) + end + + test "checksumming" do + assert_equal Digest::MD5.hexdigest(FIXTURE_FILE.read), @service.checksum(FIXTURE_KEY) + end + end +end diff --git a/test/site/disk_site_test.rb b/test/site/disk_site_test.rb deleted file mode 100644 index a04414ea68..0000000000 --- a/test/site/disk_site_test.rb +++ /dev/null @@ -1,8 +0,0 @@ -require "tmpdir" -require "site/shared_site_tests" - -class ActiveStorage::Site::DiskSiteTest < ActiveSupport::TestCase - SITE = ActiveStorage::Site.configure(:Disk, root: File.join(Dir.tmpdir, "active_storage")) - - include ActiveStorage::Site::SharedSiteTests -end diff --git a/test/site/gcs_site_test.rb b/test/site/gcs_site_test.rb deleted file mode 100644 index 98b1a3d767..0000000000 --- a/test/site/gcs_site_test.rb +++ /dev/null @@ -1,20 +0,0 @@ -require "site/shared_site_tests" - -if SITE_CONFIGURATIONS[:gcs] - class ActiveStorage::Site::GCSSiteTest < ActiveSupport::TestCase - SITE = ActiveStorage::Site.configure(:GCS, SITE_CONFIGURATIONS[:gcs]) - - include ActiveStorage::Site::SharedSiteTests - - test "signed URL generation" do - travel_to Time.now do - url = SITE.bucket.signed_url(FIXTURE_KEY, expires: 120) + - "&response-content-disposition=inline%3B+filename%3D%22test.txt%22" - - assert_equal url, @site.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: "test.txt") - end - end - end -else - puts "Skipping GCS Site tests because no GCS configuration was supplied" -end diff --git a/test/site/mirror_site_test.rb b/test/site/mirror_site_test.rb deleted file mode 100644 index 7ced377cde..0000000000 --- a/test/site/mirror_site_test.rb +++ /dev/null @@ -1,30 +0,0 @@ -require "tmpdir" -require "site/shared_site_tests" - -class ActiveStorage::Site::MirrorSiteTest < ActiveSupport::TestCase - PRIMARY_DISK_SITE = ActiveStorage::Site.configure(:Disk, root: File.join(Dir.tmpdir, "active_storage")) - SECONDARY_DISK_SITE = ActiveStorage::Site.configure(:Disk, root: File.join(Dir.tmpdir, "active_storage_mirror")) - - SITE = ActiveStorage::Site.configure :Mirror, sites: [ PRIMARY_DISK_SITE, SECONDARY_DISK_SITE ] - - include ActiveStorage::Site::SharedSiteTests - - test "uploading was done to all sites" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - io = StringIO.new(data) - @site.upload(key, io) - - assert_equal data, PRIMARY_DISK_SITE.download(key) - assert_equal data, SECONDARY_DISK_SITE.download(key) - ensure - @site.delete key - end - end - - test "existing in all sites" do - assert PRIMARY_DISK_SITE.exist?(FIXTURE_KEY) - assert SECONDARY_DISK_SITE.exist?(FIXTURE_KEY) - end -end diff --git a/test/site/s3_site_test.rb b/test/site/s3_site_test.rb deleted file mode 100644 index a9cb6ca618..0000000000 --- a/test/site/s3_site_test.rb +++ /dev/null @@ -1,11 +0,0 @@ -require "site/shared_site_tests" - -if SITE_CONFIGURATIONS[:s3] - class ActiveStorage::Site::S3SiteTest < ActiveSupport::TestCase - SITE = ActiveStorage::Site.configure(:S3, SITE_CONFIGURATIONS[:s3]) - - include ActiveStorage::Site::SharedSiteTests - end -else - puts "Skipping S3 Site tests because no S3 configuration was supplied" -end diff --git a/test/site/shared_site_tests.rb b/test/site/shared_site_tests.rb deleted file mode 100644 index 687c35e941..0000000000 --- a/test/site/shared_site_tests.rb +++ /dev/null @@ -1,63 +0,0 @@ -require "test_helper" -require "active_support/core_ext/securerandom" -require "yaml" - -SITE_CONFIGURATIONS = begin - YAML.load_file(File.expand_path("../configurations.yml", __FILE__)).deep_symbolize_keys -rescue Errno::ENOENT - puts "Missing site configuration file in test/sites/configurations.yml" -end - -module ActiveStorage::Site::SharedSiteTests - extend ActiveSupport::Concern - - FIXTURE_KEY = SecureRandom.base58(24) - FIXTURE_FILE = StringIO.new("Hello world!") - - included do - setup do - @site = self.class.const_get(:SITE) - @site.upload FIXTURE_KEY, FIXTURE_FILE - FIXTURE_FILE.rewind - end - - teardown do - @site.delete FIXTURE_KEY - FIXTURE_FILE.rewind - end - - test "uploading" do - begin - key = SecureRandom.base58(24) - data = "Something else entirely!" - @site.upload(key, StringIO.new(data)) - - assert_equal data, @site.download(key) - ensure - @site.delete key - end - end - - test "downloading" do - assert_equal FIXTURE_FILE.read, @site.download(FIXTURE_KEY) - end - - test "existing" do - assert @site.exist?(FIXTURE_KEY) - assert_not @site.exist?(FIXTURE_KEY + "nonsense") - end - - test "deleting" do - @site.delete FIXTURE_KEY - assert_not @site.exist?(FIXTURE_KEY) - end - - test "sizing" do - assert_equal FIXTURE_FILE.size, @site.byte_size(FIXTURE_KEY) - end - - test "checksumming" do - assert_equal Digest::MD5.hexdigest(FIXTURE_FILE.read), @site.checksum(FIXTURE_KEY) - end - end -end diff --git a/test/test_helper.rb b/test/test_helper.rb index f354a995e4..dcabe33c18 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -6,8 +6,8 @@ require "byebug" require "active_storage" -require "active_storage/site" -ActiveStorage::Blob.site = ActiveStorage::Site.configure(:Disk, root: File.join(Dir.tmpdir, "active_storage")) +require "active_storage/service" +ActiveStorage::Blob.service = ActiveStorage::Service.configure(:Disk, root: File.join(Dir.tmpdir, "active_storage")) require "active_storage/verified_key_with_expiration" ActiveStorage::VerifiedKeyWithExpiration.verifier = ActiveSupport::MessageVerifier.new("Testing")