From 5a143c0c6e99767ecb20ea7d526eca423e18ff21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Fri, 29 May 2015 18:39:47 +0200 Subject: [PATCH] storage engines refactor --- lib/file_store/base_store.rb | 30 +++-- lib/file_store/local_store.rb | 56 +++----- lib/file_store/s3_store.rb | 125 ++++++++---------- .../components/file_store/local_store_spec.rb | 2 - spec/components/file_store/s3_store_spec.rb | 2 - spec/models/optimized_image_spec.rb | 10 +- 6 files changed, 93 insertions(+), 132 deletions(-) diff --git a/lib/file_store/base_store.rb b/lib/file_store/base_store.rb index c5b2d80cb..29e9fe97f 100644 --- a/lib/file_store/base_store.rb +++ b/lib/file_store/base_store.rb @@ -3,33 +3,50 @@ module FileStore class BaseStore def store_upload(file, upload, content_type = nil) + path = get_path_for_upload(upload) + store_file(file, path) end def store_optimized_image(file, optimized_image) + path = get_path_for_optimized_image(optimized_image) + store_file(file, path) + end + + def store_file(file, path, opts = {}) end def remove_upload(upload) + remove_file(upload.url) end def remove_optimized_image(optimized_image) + remove_file(optimized_image.url) + end + + def remove_file(url) end def has_been_uploaded?(url) end + def download_url(upload) + end + + def cdn_url(url) + url + end + def absolute_base_url end def relative_base_url end - def download_url(upload) - end - def external? end def internal? + !external? end def path_for(upload) @@ -38,16 +55,9 @@ module FileStore def download(upload) end - def avatar_template(avatar) - end - def purge_tombstone(grace_period) end - def cdn_url(url) - url - end - def get_path_for(type, id, sha, extension) depth = [0, Math.log(id / 1_000.0, 16).ceil].max tree = File.join(*sha[0, depth].split(""), "") diff --git a/lib/file_store/local_store.rb b/lib/file_store/local_store.rb index 1d009c178..d173bec43 100644 --- a/lib/file_store/local_store.rb +++ b/lib/file_store/local_store.rb @@ -4,22 +4,19 @@ module FileStore class LocalStore < BaseStore - def store_upload(file, upload, content_type = nil) - path = get_path_for_upload(upload) - store_file(file, path) + def store_file(file, path) + copy_file(file, "#{public_dir}#{path}") + "#{Discourse.base_uri}#{path}" end - def store_optimized_image(file, optimized_image) - path = get_path_for_optimized_image(optimized_image) - store_file(file, path) - end - - def remove_upload(upload) - remove_file(upload.url) - end - - def remove_optimized_image(optimized_image) - remove_file(optimized_image.url) + def remove_file(url) + return unless is_relative?(url) + path = public_dir + url + tombstone = public_dir + url.gsub("/uploads/", "/tombstone/") + FileUtils.mkdir_p(Pathname.new(tombstone).dirname) + FileUtils.move(path, tombstone) + rescue Errno::ENOENT + # don't care if the file isn't there end def has_been_uploaded?(url) @@ -34,19 +31,15 @@ module FileStore "/uploads/#{RailsMultisite::ConnectionManagement.current_db}" end + def external? + false + end + def download_url(upload) return unless upload "#{relative_base_url}/#{upload.sha1}" end - def external? - !internal? - end - - def internal? - true - end - def path_for(upload) "#{public_dir}#{upload.url}" end @@ -55,19 +48,10 @@ module FileStore `find #{tombstone_dir} -mtime +#{grace_period} -type f -delete` end - private - def get_path_for(type, upload_id, sha, extension) "#{relative_base_url}/#{super(type, upload_id, sha, extension)}" end - def store_file(file, path) - # copy the file to the right location - copy_file(file, "#{public_dir}#{path}") - # url - "#{Discourse.base_uri}#{path}" - end - def copy_file(file, path) FileUtils.mkdir_p(Pathname.new(path).dirname) # move the file to the right location @@ -75,16 +59,6 @@ module FileStore File.open(path, "wb") { |f| f.write(file.read) } end - def remove_file(url) - return unless is_relative?(url) - path = public_dir + url - tombstone = public_dir + url.gsub("/uploads/", "/tombstone/") - FileUtils.mkdir_p(Pathname.new(tombstone).dirname) - FileUtils.move(path, tombstone) - rescue Errno::ENOENT - # don't care if the file isn't there - end - def is_relative?(url) url.present? && url.start_with?(relative_base_url) end diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index 2570d4782..44ed2cdda 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -13,22 +13,37 @@ module FileStore @s3_helper = s3_helper || S3Helper.new(s3_bucket, TOMBSTONE_PREFIX) end - def store_upload(file, upload, content_type=nil) + def store_upload(file, upload, content_type = nil) path = get_path_for_upload(upload) store_file(file, path, filename: upload.original_filename, content_type: content_type, cache_locally: true) end - def store_optimized_image(file, optimized_image) - path = get_path_for_optimized_image(optimized_image) - store_file(file, path) + # options + # - filename + # - content_type + # - cache_locally + def store_file(file, path, opts={}) + filename = opts[:filename].presence + content_type = opts[:content_type].presence + # cache file locally when needed + cache_file(file, File.basename(path)) if opts[:cache_locally] + # stored uploaded are public by default + options = { acl: "public-read" } + # add a "content disposition" header for "attachments" + options[:content_disposition] = "attachment; filename=\"#{filename}\"" if filename && !FileHelper.is_image?(filename) + # add a "content type" header when provided + options[:content_type] = content_type if content_type + # if this fails, it will throw an exception + @s3_helper.upload(file, path, options) + # return the upload url + "#{absolute_base_url}/#{path}" end - def remove_upload(upload) - remove_file(upload.url) - end - - def remove_optimized_image(optimized_image) - remove_file(optimized_image.url) + def remove_file(url) + return unless has_been_uploaded?(url) + filename = File.basename(url) + # copy the removed file to tombstone + @s3_helper.remove(filename, true) end def has_been_uploaded?(url) @@ -51,10 +66,6 @@ module FileStore true end - def internal? - !external? - end - def download(upload) return unless has_been_uploaded?(upload.url) @@ -85,69 +96,47 @@ module FileStore end def cdn_url(url) - if SiteSetting.s3_cdn_url.present? - url.sub(absolute_base_url, SiteSetting.s3_cdn_url) - else - url - end + return url if SiteSetting.s3_cdn_url.blank? + url.sub(absolute_base_url, SiteSetting.s3_cdn_url) end - private + def cache_avatar(avatar, user_id) + source = avatar.url.sub(absolute_base_url + "/", "") + destination = avatar_template(avatar, user_id).sub(absolute_base_url + "/", "") + @s3_helper.copy(source, destination) + end - # options - # - filename - # - content_type - # - cache_locally - def store_file(file, path, opts={}) - filename = opts[:filename].presence - content_type = opts[:content_type].presence - # cache file locally when needed - cache_file(file, File.basename(path)) if opts[:cache_locally] - # stored uploaded are public by default - options = { acl: "public-read" } - # add a "content disposition" header for "attachments" - options[:content_disposition] = "attachment; filename=\"#{filename}\"" if filename && !FileHelper.is_image?(filename) - # add a "content type" header when provided - options[:content_type] = content_type if content_type - # if this fails, it will throw an exception - @s3_helper.upload(file, path, options) - # return the upload url - "#{absolute_base_url}/#{path}" - end + def avatar_template(avatar, user_id) + UserAvatar.external_avatar_url(user_id, avatar.upload_id, avatar.width) + end - def remove_file(url) - return unless has_been_uploaded?(url) - filename = File.basename(url) - # copy the removed file to tombstone - @s3_helper.remove(filename, true) - end + CACHE_DIR ||= "#{Rails.root}/tmp/s3_cache/" + CACHE_MAXIMUM_SIZE ||= 500 - CACHE_DIR ||= "#{Rails.root}/tmp/s3_cache/" - CACHE_MAXIMUM_SIZE ||= 500 + def get_cache_path_for(filename) + "#{CACHE_DIR}#{filename}" + end - def get_cache_path_for(filename) - "#{CACHE_DIR}#{filename}" - end + def get_from_cache(filename) + path = get_cache_path_for(filename) + File.open(path) if File.exists?(path) + end - def get_from_cache(filename) - path = get_cache_path_for(filename) - File.open(path) if File.exists?(path) - end + def cache_file(file, filename) + path = get_cache_path_for(filename) + dir = File.dirname(path) + FileUtils.mkdir_p(dir) unless Dir[dir].present? + FileUtils.cp(file.path, path) + # keep up to 500 files + `ls -tr #{CACHE_DIR} | head -n +#{CACHE_MAXIMUM_SIZE} | xargs rm -f` + end - def cache_file(file, filename) - path = get_cache_path_for(filename) - dir = File.dirname(path) - FileUtils.mkdir_p(dir) unless Dir[dir].present? - FileUtils.cp(file.path, path) - # keep up to 500 files - `ls -tr #{CACHE_DIR} | head -n +#{CACHE_MAXIMUM_SIZE} | xargs rm -f` - end + def s3_bucket + return @s3_bucket if @s3_bucket + raise Discourse::SiteSettingMissing.new("s3_upload_bucket") if SiteSetting.s3_upload_bucket.blank? + @s3_bucket = SiteSetting.s3_upload_bucket.downcase + end - def s3_bucket - return @s3_bucket if @s3_bucket - raise Discourse::SiteSettingMissing.new("s3_upload_bucket") if SiteSetting.s3_upload_bucket.blank? - @s3_bucket = SiteSetting.s3_upload_bucket.downcase - end end end diff --git a/spec/components/file_store/local_store_spec.rb b/spec/components/file_store/local_store_spec.rb index 44f93056b..d3d523d74 100644 --- a/spec/components/file_store/local_store_spec.rb +++ b/spec/components/file_store/local_store_spec.rb @@ -10,8 +10,6 @@ describe FileStore::LocalStore do let(:optimized_image) { Fabricate(:optimized_image) } - let(:avatar) { Fabricate(:upload) } - describe ".store_upload" do it "returns a relative url" do diff --git a/spec/components/file_store/s3_store_spec.rb b/spec/components/file_store/s3_store_spec.rb index f4161f288..20361d219 100644 --- a/spec/components/file_store/s3_store_spec.rb +++ b/spec/components/file_store/s3_store_spec.rb @@ -13,8 +13,6 @@ describe FileStore::S3Store do let(:optimized_image) { Fabricate(:optimized_image) } let(:optimized_image_file) { file_from_fixtures("logo.png") } - let(:avatar) { Fabricate(:upload) } - before(:each) do SiteSetting.stubs(:s3_upload_bucket).returns("S3_Upload_Bucket") SiteSetting.stubs(:s3_access_key_id).returns("s3_access_key_id") diff --git a/spec/models/optimized_image_spec.rb b/spec/models/optimized_image_spec.rb index b3a8da52d..b4c81a19d 100644 --- a/spec/models/optimized_image_spec.rb +++ b/spec/models/optimized_image_spec.rb @@ -110,12 +110,8 @@ end class FakeInternalStore - def internal? - true - end - def external? - !internal? + false end def path_for(upload) @@ -134,10 +130,6 @@ class FakeExternalStore true end - def internal? - !external? - end - def store_optimized_image(file, optimized_image) "/externally/stored/optimized/image#{optimized_image.extension}" end