diff --git a/Gemfile b/Gemfile index 2496ad0..f949d2c 100644 --- a/Gemfile +++ b/Gemfile @@ -5,4 +5,5 @@ git_source(:github) { |repo_name| "https://github.com/#{repo_name}" } # Specify your gem's dependencies in active_storage-cloudinary_service.gemspec gemspec +gem 'activestorage', '~> 5.2.0', require: false gem 'cloudinary', '~> 1.8.2', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 7ca2737..8a67c76 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,29 +1,82 @@ PATH remote: . specs: - activestorage-cloudinary-service (0.1.0) + activestorage-cloudinary-service (0.2.0) GEM remote: https://rubygems.org/ specs: + actionpack (5.2.0) + actionview (= 5.2.0) + activesupport (= 5.2.0) + rack (~> 2.0) + rack-test (>= 0.6.3) + rails-dom-testing (~> 2.0) + rails-html-sanitizer (~> 1.0, >= 1.0.2) + actionview (5.2.0) + activesupport (= 5.2.0) + builder (~> 3.1) + erubi (~> 1.4) + rails-dom-testing (~> 2.0) + rails-html-sanitizer (~> 1.0, >= 1.0.3) + activemodel (5.2.0) + activesupport (= 5.2.0) + activerecord (5.2.0) + activemodel (= 5.2.0) + activesupport (= 5.2.0) + arel (>= 9.0) + activestorage (5.2.0) + actionpack (= 5.2.0) + activerecord (= 5.2.0) + marcel (~> 0.3.1) + activesupport (5.2.0) + concurrent-ruby (~> 1.0, >= 1.0.2) + i18n (>= 0.7, < 2) + minitest (~> 5.1) + tzinfo (~> 1.1) + arel (9.0.0) aws_cf_signer (0.1.3) + builder (3.2.3) cloudinary (1.8.2) aws_cf_signer rest-client coderay (1.1.2) + concurrent-ruby (1.0.5) + crass (1.0.4) diff-lcs (1.3) domain_name (0.5.20170404) unf (>= 0.0.5, < 1.0.0) + erubi (1.7.1) http-cookie (1.0.3) domain_name (~> 0.5) + i18n (1.0.1) + concurrent-ruby (~> 1.0) + loofah (2.2.2) + crass (~> 1.0.2) + nokogiri (>= 1.5.9) + marcel (0.3.2) + mimemagic (~> 0.3.2) method_source (0.9.0) mime-types (3.1) mime-types-data (~> 3.2015) mime-types-data (3.2016.0521) + mimemagic (0.3.2) + mini_portile2 (2.3.0) + minitest (5.11.3) netrc (0.11.0) + nokogiri (1.8.4) + mini_portile2 (~> 2.3.0) pry (0.11.3) coderay (~> 1.1.0) method_source (~> 0.9.0) + rack (2.0.5) + rack-test (1.1.0) + rack (>= 1.0, < 3) + rails-dom-testing (2.0.3) + activesupport (>= 4.2.0) + nokogiri (>= 1.6) + rails-html-sanitizer (1.0.4) + loofah (~> 2.2, >= 2.2.2) rake (10.5.0) rest-client (2.0.2) http-cookie (>= 1.0.2, < 2.0) @@ -42,6 +95,9 @@ GEM diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.7.0) rspec-support (3.7.0) + thread_safe (0.3.6) + tzinfo (1.2.5) + thread_safe (~> 0.1) unf (0.1.4) unf_ext unf_ext (0.0.7.4) @@ -50,6 +106,7 @@ PLATFORMS ruby DEPENDENCIES + activestorage (~> 5.2.0) activestorage-cloudinary-service! bundler (~> 1.16) cloudinary (~> 1.8.2) diff --git a/activestorage-cloudinary-service.gemspec b/activestorage-cloudinary-service.gemspec index 2faaa79..943bf26 100644 --- a/activestorage-cloudinary-service.gemspec +++ b/activestorage-cloudinary-service.gemspec @@ -21,7 +21,7 @@ Gem::Specification.new do |spec| spec.require_paths = ['lib'] spec.add_development_dependency 'bundler', '~> 1.16' + spec.add_development_dependency 'pry', '~> 0.11.3' spec.add_development_dependency 'rake', '~> 10.0' spec.add_development_dependency 'rspec', '~> 3.7.0' - spec.add_development_dependency 'pry', '~> 0.11.3' end diff --git a/lib/active_storage/service/cloudinary_service.rb b/lib/active_storage/service/cloudinary_service.rb index c7b4c11..f0e1f5e 100644 --- a/lib/active_storage/service/cloudinary_service.rb +++ b/lib/active_storage/service/cloudinary_service.rb @@ -1,8 +1,12 @@ require 'cloudinary' -require 'open-uri' +require_relative 'download_utils' module ActiveStorage + # Wraps the Cloudinary as an Active Storage service. + # See ActiveStorage::Service for the generic API documentation that applies to all services. class Service::CloudinaryService < Service + include DownloadUtils + # FIXME: implement setup for private resource type # FIXME: allow configuration via cloudinary url def initialize(cloud_name:, api_key:, api_secret:, **options) @@ -22,23 +26,27 @@ def upload(key, io, checksum: nil) end # Return the content of the file at the +key+. - def download(key) - tmp_file = open(url_for_public_id(key)) + def download(key, &block) if block_given? instrument :streaming_download, key: key do - File.open(tmp_file, 'rb') do |file| - while (data = file.read(64.kilobytes)) - yield data - end - end + source = cloudinary_url_for_key(key) + stream_download(source, &block) end else instrument :download, key: key do - File.binread tmp_file + Cloudinary::Downloader.download(key) end end end + # Return the partial content in the byte +range+ of the file at the +key+. + def download_chunk(key, range) + instrument :download_chunk, key: key, range: range do + source = cloudinary_url_for_key(key) + download_range(source, range) + end + end + # Delete the file at the +key+. def delete(key) instrument :delete, key: key do @@ -49,9 +57,7 @@ def delete(key) # Delete files at keys starting with the +prefix+. def delete_prefixed(prefix) instrument :delete_prefixed, prefix: prefix do - find_resources_with_public_id_prefix(prefix).each do |resource| - delete_resource_with_public_id(resource['public_id']) - end + Cloudinary::Api.delete_resources_by_prefix(prefix) end end @@ -87,9 +93,15 @@ def url_for_direct_upload(key, expires_in:, content_type:, content_length:, chec expires_in: expires_in, content_type: content_type, content_length: content_length, - checksum: checksum + checksum: checksum, + resource_type: 'auto' } - signed_upload_url_for_public_id(key, options) + + # FIXME: Cloudinary Ruby SDK does't expose an api for signed upload url + # The expected url is similar to the private_download_url + # with download replaced with upload + signed_download_url_for_public_id(key, options) + .sub(/download/, 'upload') end end @@ -108,37 +120,16 @@ def find_resource_with_public_id(public_id) Cloudinary::Api.resources_by_ids(public_id).fetch('resources') end - def find_resources_with_public_id_prefix(prefix) - Cloudinary::Api.resources( - type: :upload, - prefix: prefix - ).fetch('resources') - end - def delete_resource_with_public_id(public_id) Cloudinary::Uploader.destroy(public_id) end - def url_for_public_id(public_id) - Cloudinary::Api.resource(public_id)['secure_url'] - end - - # FIXME: Cloudinary Ruby SDK does't expose an api for signed upload url - # The expected url is similar to the private_download_url - # with download replaced with upload - def signed_upload_url_for_public_id(public_id, options) - # allow the server to auto detect the resource_type - options[:resource_type] ||= 'auto' - signed_download_url_for_public_id(public_id, options) - .sub(/download/, 'upload') - end - def signed_download_url_for_public_id(public_id, options) - extension = resource_format(options) + extension = resource_format(options[:filename]) options[:resource_type] ||= resource_type(extension) Cloudinary::Utils.private_download_url( - finalize_public_id(public_id, extension, options), + finalize_public_id(public_id, extension, options[:resource_type]), extension, signed_url_options(options) ) @@ -146,8 +137,8 @@ def signed_download_url_for_public_id(public_id, options) # TODO: for assets of type raw, # cloudinary request the extension to be part of the public_id - def finalize_public_id(public_id, extension, options) - return public_id unless options[:resource_type] == 'raw' + def finalize_public_id(public_id, extension, resource_type) + return public_id unless resource_type == 'raw' public_id + '.' + extension end @@ -160,13 +151,17 @@ def signed_url_options(options) } end - def resource_format(options) - extension = options[:filename]&.extension_with_delimiter || '' + def resource_format(filename) + extension = filename&.extension_with_delimiter || '' extension.sub('.', '') end def resource_type(extension) Cloudinary::Utils.resource_type_for_format(extension) end + + def cloudinary_url_for_key(key) + Cloudinary::Utils.cloudinary_url(key) + end end end diff --git a/lib/active_storage/service/download_utils.rb b/lib/active_storage/service/download_utils.rb new file mode 100644 index 0000000..499bcb3 --- /dev/null +++ b/lib/active_storage/service/download_utils.rb @@ -0,0 +1,50 @@ +require 'net/http' +require 'openssl' + +module DownloadUtils + def stream_download(source, chunk_size = 5_242_880) + url = URI.parse(source) + http, req = setup_connection(url) + + content_length = http.request_head(url).content_length + upper_limit = content_length + (content_length % chunk_size) + offset = 0 + + http.start do |agent| + while offset < upper_limit + lim = (offset + chunk_size) + # QUESTION: is it relevant to set the last chunk + # to the exact remaining bytes + # lim = content_length if lim > content_length + req.range = (offset..lim) + + chunk = agent.request(req).body + yield chunk.force_encoding(Encoding::BINARY) + + offset += chunk_size + 1 + end + end + end + + def download_range(source, range) + url = URI.parse(source) + http, req = setup_connection(url) + req.range = range + + chunk = http.start { |agent| agent.request(req).body } + chunk.force_encoding(Encoding::BINARY) + end + + private + + def setup_connection(url) + http = Net::HTTP.new(url.host, url.port) + req = Net::HTTP::Get.new(url.request_uri) + + if url.port == 443 + http.use_ssl = true + http.verify_mode = OpenSSL::SSL::VERIFY_NONE + end + [http, req] + end +end diff --git a/lib/active_storage/service/version.rb b/lib/active_storage/service/version.rb index 9259ef5..4dcbef0 100644 --- a/lib/active_storage/service/version.rb +++ b/lib/active_storage/service/version.rb @@ -1,5 +1,5 @@ module ActiveStorage module CloudinaryService - VERSION = '0.1.0'.freeze + VERSION = '0.2.0'.freeze end end diff --git a/spec/cloudinary_service_spec.rb b/spec/cloudinary_service_spec.rb index 97a9881..869b172 100644 --- a/spec/cloudinary_service_spec.rb +++ b/spec/cloudinary_service_spec.rb @@ -1,7 +1,10 @@ +require 'download_utils' + RSpec.describe ActiveStorage::Service::CloudinaryService do let(:subject) { ActiveStorage::Service::CloudinaryService.new(config) } let(:key) { 'some-resource-key' } let(:file) { double } + let(:download_util) { double } let(:checksum) { 'zyxddfs' } let(:config) do @@ -17,6 +20,8 @@ allow(file).to receive(:extension_with_delimiter).and_return('.png') end + include_examples 'download utils' + describe '#new' do it 'setups cloudinary sdk with the given config' do ActiveStorage::Service::CloudinaryService.new(config) @@ -52,13 +57,52 @@ end describe '#download' do - # it 'instruments the operation' do - # options = { key: key } - # expect_any_instance_of(ActiveStorage::Service) - # .to receive(:instrument).with(:download, options) - # - # subject.download(key) - # end + context 'when block is given' do + it 'calls the stream_download method' do + block = -> { 'some block' } + + expect(subject).to receive(:stream_download) + .with('https://some-resource-key') { |&blk| expect(blk).to be(block) } + expect(Cloudinary::Downloader).not_to receive(:download) + + subject.download(key, &block) + end + end + + context 'when no block is given' do + it 'calls the cloudinary downloader download method' do + expect(Cloudinary::Downloader).to receive(:download).with(key) + expect(subject).not_to receive(:stream_download) + + subject.download(key) + end + end + + it 'instruments the operation' do + options = { key: key } + expect_any_instance_of(ActiveStorage::Service) + .to receive(:instrument).with(:download, options) + + subject.download(key) + end + end + + describe '#download_chunk' do + let(:range) { 1..10 } + + it 'calls the download range method' do + expect(subject).to receive(:download_range) + .with('https://some-resource-key', range) + subject.download_chunk(key, range) + end + + it 'instruments the operation' do + options = { key: key, range: range } + expect_any_instance_of(ActiveStorage::Service) + .to receive(:instrument).with(:download_chunk, options) + + subject.download_chunk(key, range) + end end describe '#delete' do @@ -80,24 +124,14 @@ describe '#delete_prefixed' do let(:prefix) { 'some-key-prefix' } - it 'calls the resources method on the cloudinary sdk with the given prefix' do + it 'calls the delete_resources_by_prefix method on the cloudinary sdk' do expect(Cloudinary::Api) - .to receive(:resources) - .with(type: :upload, prefix: prefix) + .to receive(:delete_resources_by_prefix) + .with(prefix) .and_return('resources' => []) subject.delete_prefixed(prefix) end - it 'calls the delete method on the cloudinary sdk with the given args' do - allow(Cloudinary::Api) - .to receive(:resources) - .with(type: :upload, prefix: prefix) - .and_return('resources' => ['public_id' => prefix]) - - expect(Cloudinary::Uploader).to receive(:destroy).with(prefix) - subject.delete_prefixed(prefix) - end - it 'instruments the operation' do options = { prefix: key } expect_any_instance_of(ActiveStorage::Service) @@ -182,22 +216,6 @@ end end - xcontext 'resource type' do - it 'defaults to image if no resource type in the options' do - end - - it 'uses the resource type in the options' do - end - end - - xcontext 'type' do - it 'defaults to upload if no type in the options' do - end - - it 'uses the type in the option' do - end - end - it 'instruments the operation' do expect_any_instance_of(ActiveStorage::Service) .to receive(:instrument).with(:url, key: key) @@ -228,9 +246,7 @@ expect(Cloudinary::Utils) .to receive(:private_download_url) .with(key, '', hash_including(signed_options)) - .and_return( - "https://cloudinary.api/signed/url/for/#{key}/" - ) + .and_return("https://cloudinary.api/signed/url/for/#{key}/") subject.url_for_direct_upload(key, options) end diff --git a/spec/download_utils.rb b/spec/download_utils.rb new file mode 100644 index 0000000..f7d2914 --- /dev/null +++ b/spec/download_utils.rb @@ -0,0 +1,62 @@ +RSpec.shared_examples 'download utils' do + let(:dummy_http) { double } + let(:dummy_req) { double } + let(:source) { 'https://res.image.cloudinary.loc' } + let(:body) { 'some string thingy' } + + before do + stub_const('Net::HTTP::Get', dummy_req) + allow(Net::HTTP).to receive(:new) { dummy_http } + + allow(dummy_http).to receive(:start).and_yield(dummy_http) + allow(dummy_http).to receive(:use_ssl=) + allow(dummy_http).to receive(:verify_mode=) + allow(dummy_http).to receive(:force_encoding) + allow(dummy_http).to receive(:body).and_return(body) + allow(dummy_http).to receive(:request).with(dummy_req) { dummy_http } + + allow(dummy_req).to receive(:new) { dummy_req } + end + + + describe 'download_range' do + let(:range) { 4..16 } + + it 'sets the request range to the given range' do + expect(dummy_req).to receive(:range=).with(range) + + subject.download_range(source, range) + end + end + + + describe 'stream_download' do + let(:chunks) { [] } + let(:block) { ->(chunk) { chunks << chunk } } + before do + allow(dummy_http).to receive(:request_head).and_return(dummy_http) + allow(dummy_http).to receive(:content_length).and_return(7) + end + + context 'chunk size is greater than content_length' do + it 'yields all the content' do + expect(dummy_req).to receive(:range=).with(0..100) + + subject.stream_download(source, 100, &block) + + expect(chunks).to eq [body] + end + end + + context 'chunk_size is less than content_length' do + it 'yields the content in chunks of the given chunk size' do + expect(dummy_req).to receive(:range=).once.ordered.with(0..2) + expect(dummy_req).to receive(:range=).once.ordered.with(3..5) + expect(dummy_req).to receive(:range=).once.ordered.with(6..8) + + subject.stream_download(source, 2, &block) + expect(chunks.size).to eq 3 + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0190a49..f971307 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -47,20 +47,20 @@ # triggering implicit auto-inclusion in groups with matching metadata. config.shared_context_metadata_behavior = :apply_to_host_groups -# The settings below are suggested to provide a good initial experience -# with RSpec, but feel free to customize to your heart's content. -=begin + # The settings below are suggested to provide a good initial experience + # with RSpec, but feel free to customize to your heart's content. + # This allows you to limit a spec run to individual examples or groups # you care about by tagging them with `:focus` metadata. When nothing # is tagged with `:focus`, all examples get run. RSpec also provides # aliases for `it`, `describe`, and `context` that include `:focus` # metadata: `fit`, `fdescribe` and `fcontext`, respectively. - config.filter_run_when_matching :focus + # config.filter_run_when_matching :focus # Allows RSpec to persist some state between runs in order to support # the `--only-failures` and `--next-failure` CLI options. We recommend # you configure your source control system to ignore this file. - config.example_status_persistence_file_path = "spec/examples.txt" + # config.example_status_persistence_file_path = "spec/examples.txt" # Limits the available syntax to the non-monkey patched syntax that is # recommended. For more details, see: @@ -80,13 +80,13 @@ # Use the documentation formatter for detailed output, # unless a formatter has already been configured # (e.g. via a command-line flag). - config.default_formatter = "doc" + config.default_formatter = 'doc' end # Print the 10 slowest examples and example groups at the # end of the spec run, to help surface which specs are running # particularly slow. - config.profile_examples = 10 + # config.profile_examples = 10 # Run specs in random order to surface order dependencies. If you find an # order dependency and want to debug it, you can fix the order by providing @@ -99,5 +99,4 @@ # test failures related to randomization by passing the same `--seed` value # as the one that triggered the failure. Kernel.srand config.seed -=end end diff --git a/spec/test_dummies.rb b/spec/test_dummies.rb index badca7e..f234135 100644 --- a/spec/test_dummies.rb +++ b/spec/test_dummies.rb @@ -27,6 +27,12 @@ class Api def self.resources(_options); end def self.resources_by_ids(_public_id); end + + def self.delete_resources_by_prefix(_prefix); end + end + + class Downloader + def self.download(_key); end end class Utils @@ -35,6 +41,10 @@ def self.private_download_url(_public_id, _format, _options); end def self.resource_type_for_format(ext) %w[png pdf].include?(ext) ? 'image' : 'raw' end + + def self.cloudinary_url(key) + "https://#{key}" + end end end