From 3e3793cf748a09bd0a3302602d91fd6344a4be0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Wed, 31 Jul 2013 23:23:07 +0200 Subject: [PATCH 1/3] update fog gem to latest --- Gemfile.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index fb384003d..a0761302c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -168,7 +168,7 @@ GEM handlebars-source (= 1.0.12) erubis (2.7.0) eventmachine (1.0.3) - excon (0.20.1) + excon (0.25.3) execjs (1.4.0) multi_json (~> 1.0) fabrication (2.6.5) @@ -181,15 +181,15 @@ GEM fast_xs (0.8.0) fastimage (1.3.0) ffi (1.8.1) - fog (1.10.1) + fog (1.14.0) builder - excon (~> 0.20) + excon (~> 0.25.0) formatador (~> 0.2.0) mime-types multi_json (~> 1.0) net-scp (~> 1.1) net-ssh (>= 2.1.3) - nokogiri (~> 1.5.0) + nokogiri (~> 1.5) ruby-hmac formatador (0.2.4) fspath (2.0.4) @@ -247,9 +247,9 @@ GEM multi_json (1.7.7) multipart-post (1.2.0) mustache (0.99.4) - net-scp (1.1.0) + net-scp (1.1.2) net-ssh (>= 2.6.5) - net-ssh (2.6.7) + net-ssh (2.6.8) nokogiri (1.5.9) oauth (0.4.7) oauth2 (0.8.1) From 36b6b8d78e359f2e93c7bf3088c7c03f28a07cd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Wed, 31 Jul 2013 23:24:16 +0200 Subject: [PATCH 2/3] removed old rake task until it's updated --- lib/tasks/images.rake | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/lib/tasks/images.rake b/lib/tasks/images.rake index c8d15eebc..74c856421 100644 --- a/lib/tasks/images.rake +++ b/lib/tasks/images.rake @@ -10,27 +10,6 @@ task "images:compress" => :environment do end end -desc "updates reverse index of image uploads" -task "images:reindex" => :environment do - RailsMultisite::ConnectionManagement.each_connection do |db| - puts "Reindexing #{db}" - Post.select([:id, :cooked]).find_each do |p| - doc = Nokogiri::HTML::fragment(p.cooked) - doc.search("img").each do |img| - src = img['src'] - if src.present? && Upload.has_been_uploaded?(src) && m = Upload.uploaded_regex.match(src) - begin - PostUpload.create({ post_id: p.id, upload_id: m[:upload_id] }) - rescue ActiveRecord::RecordNotUnique - end - end - end - putc "." - end - end - puts "\ndone." -end - desc "clean orphan uploaded files" task "images:clean_orphans" => :environment do RailsMultisite::ConnectionManagement.each_connection do |db| From ed9417fa3bf8609728735ed2c968e96fdc0881ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Wed, 31 Jul 2013 23:26:34 +0200 Subject: [PATCH 3/3] enable thumbnailing on S3 - added url to optimized image model - refactored s3_store & local_store --- app/models/optimized_image.rb | 54 +++----- app/models/post.rb | 2 - app/models/post_analyzer.rb | 10 +- app/models/topic_link.rb | 4 +- app/models/upload.rb | 46 ++----- ...30728172550_add_url_to_optimized_images.rb | 22 ++++ lib/cooked_post_processor.rb | 28 ++--- lib/discourse.rb | 12 +- lib/file_store/local_store.rb | 93 ++++++++++++++ lib/file_store/s3_store.rb | 119 ++++++++++++++++++ lib/image_optimizer.rb | 96 -------------- lib/local_store.rb | 47 ------- lib/s3_store.rb | 70 ----------- spec/components/cooked_post_processor_spec.rb | 21 ++-- spec/components/discourse_spec.rb | 13 ++ .../components/file_store/local_store_spec.rb | 76 +++++++++++ spec/components/file_store/s3_store_spec.rb | 84 +++++++++++++ spec/components/local_store_spec.rb | 30 ----- spec/components/s3_store_spec.rb | 36 ------ .../fabricators/optimized_image_fabricator.rb | 3 +- spec/fabricators/upload_fabricator.rb | 1 + spec/models/optimized_image_spec.rb | 58 ++++++--- spec/models/upload_spec.rb | 75 +---------- 23 files changed, 522 insertions(+), 478 deletions(-) create mode 100644 db/migrate/20130728172550_add_url_to_optimized_images.rb create mode 100644 lib/file_store/local_store.rb create mode 100644 lib/file_store/s3_store.rb delete mode 100644 lib/image_optimizer.rb delete mode 100644 lib/local_store.rb delete mode 100644 lib/s3_store.rb create mode 100644 spec/components/file_store/local_store_spec.rb create mode 100644 spec/components/file_store/s3_store_spec.rb delete mode 100644 spec/components/local_store_spec.rb delete mode 100644 spec/components/s3_store_spec.rb diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb index 09d3c176c..aee807d1e 100644 --- a/app/models/optimized_image.rb +++ b/app/models/optimized_image.rb @@ -8,62 +8,46 @@ class OptimizedImage < ActiveRecord::Base @image_sorcery_loaded ||= require "image_sorcery" - original_path = "#{Rails.root}/public#{upload.url}" + external_copy = Discourse.store.download(upload) if Discourse.store.external? + original_path = if Discourse.store.external? + external_copy.path + else + Discourse.store.path_for(upload) + end + # create a temp file with the same extension as the original - temp_file = Tempfile.new(["discourse", File.extname(original_path)]) + temp_file = Tempfile.new(["discourse-thumbnail", File.extname(original_path)]) temp_path = temp_file.path if ImageSorcery.new(original_path).convert(temp_path, resize: "#{width}x#{height}") - thumbnail = OptimizedImage.new({ + thumbnail = OptimizedImage.create!( upload_id: upload.id, sha1: Digest::SHA1.file(temp_path).hexdigest, extension: File.extname(temp_path), width: width, - height: height - }) - # make sure the directory exists - FileUtils.mkdir_p Pathname.new(thumbnail.path).dirname - # move the temp file to the right location - File.open(thumbnail.path, "wb") do |f| - f.write temp_file.read - end + height: height, + url: "", + ) + # store the optimized image and update its url + thumbnail.url = Discourse.store.store_optimized_image(temp_file, thumbnail) + thumbnail.save end # close && remove temp file - temp_file.close - temp_file.unlink + temp_file.close! + # make sure we remove the cached copy from external stores + external_copy.close! if Discourse.store.external? thumbnail end def destroy OptimizedImage.transaction do - remove_file + Discourse.store.remove_file(url) super end end - def remove_file - File.delete path - rescue Errno::ENOENT - end - - def url - "#{LocalStore.base_url}/#{optimized_path}/#{filename}" - end - - def path - "#{LocalStore.base_path}/#{optimized_path}/#{filename}" - end - - def optimized_path - "_optimized/#{sha1[0..2]}/#{sha1[3..5]}" - end - - def filename - "#{sha1[6..16]}_#{width}x#{height}#{extension}" - end - end # == Schema Information diff --git a/app/models/post.rb b/app/models/post.rb index a62bea04d..9d141f479 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1,7 +1,5 @@ require_dependency 'jobs' require_dependency 'pretty_text' -require_dependency 'local_store' -require_dependency 's3_store' require_dependency 'rate_limiter' require_dependency 'post_revisor' require_dependency 'enum' diff --git a/app/models/post_analyzer.rb b/app/models/post_analyzer.rb index 58115839b..533c0eda7 100644 --- a/app/models/post_analyzer.rb +++ b/app/models/post_analyzer.rb @@ -35,13 +35,9 @@ class PostAnalyzer # How many attachments are present in the post def attachment_count return 0 unless @raw.present? - - if SiteSetting.enable_s3_uploads? - cooked_document.css("a.attachment[href^=\"#{S3Store.base_url}\"]") - else - cooked_document.css("a.attachment[href^=\"#{LocalStore.directory}\"]") + - cooked_document.css("a.attachment[href^=\"#{LocalStore.base_url}\"]") - end.count + attachments = cooked_document.css("a.attachment[href^=\"#{Discourse.store.absolute_base_url}\"]") + attachments += cooked_document.css("a.attachment[href^=\"#{Discourse.store.relative_base_url}\"]") if Discourse.store.internal? + attachments.count end def raw_mentions diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb index 1e86d11ad..3279af882 100644 --- a/app/models/topic_link.rb +++ b/app/models/topic_link.rb @@ -103,8 +103,8 @@ class TopicLink < ActiveRecord::Base topic_id = nil post_number = nil - if Upload.has_been_uploaded?(url) - internal = !Upload.is_on_s3?(url) + if Discourse.store.has_been_uploaded?(url) + internal = Discourse.store.internal? elsif parsed.host == Discourse.current_hostname || !parsed.host internal = true diff --git a/app/models/upload.rb b/app/models/upload.rb index 180681131..2b1c35f96 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -2,8 +2,6 @@ require 'digest/sha1' require 'image_sizer' require 'tempfile' require 'pathname' -require_dependency 's3_store' -require_dependency 'local_store' class Upload < ActiveRecord::Base belongs_to :user @@ -20,17 +18,12 @@ class Upload < ActiveRecord::Base optimized_images.where(width: width, height: height).first end - def thumbnail_url - thumbnail.url if has_thumbnail? - end - def has_thumbnail? thumbnail.present? end def create_thumbnail! return unless SiteSetting.create_thumbnails? - return if SiteSetting.enable_s3_uploads? return if has_thumbnail? thumbnail = OptimizedImage.create_for(self, width, height) optimized_images << thumbnail if thumbnail @@ -38,7 +31,7 @@ class Upload < ActiveRecord::Base def destroy Upload.transaction do - Upload.remove_file url + Discourse.store.remove_file(url) super end end @@ -58,7 +51,7 @@ class Upload < ActiveRecord::Base file.rewind end # create a db record (so we can use the id) - upload = Upload.create!({ + upload = Upload.create!( user_id: user_id, original_filename: file.original_filename, filesize: filesize, @@ -66,9 +59,9 @@ class Upload < ActiveRecord::Base url: "", width: width, height: height, - }) + ) # store the file and update its url - upload.url = Upload.store_file(file, sha1, upload.id) + upload.url = Discourse.store.store_upload(file, upload) # save the url upload.save end @@ -76,36 +69,11 @@ class Upload < ActiveRecord::Base upload end - def self.store_file(file, sha1, upload_id) - return S3Store.store_file(file, sha1, upload_id) if SiteSetting.enable_s3_uploads? - return LocalStore.store_file(file, sha1, upload_id) - end - - def self.remove_file(url) - return S3Store.remove_file(url) if SiteSetting.enable_s3_uploads? - return LocalStore.remove_file(url) - end - - def self.has_been_uploaded?(url) - is_relative?(url) || is_local?(url) || is_on_s3?(url) - end - - def self.is_relative?(url) - url.start_with?(LocalStore.directory) - end - - def self.is_local?(url) - !SiteSetting.enable_s3_uploads? && url.start_with?(LocalStore.base_url) - end - - def self.is_on_s3?(url) - SiteSetting.enable_s3_uploads? && url.start_with?(S3Store.base_url) - end - def self.get_from_url(url) # we store relative urls, so we need to remove any host/cdn - url = url.gsub(/^#{LocalStore.asset_host}/i, "") if LocalStore.asset_host.present? - Upload.where(url: url).first if has_been_uploaded?(url) + asset_host = Rails.configuration.action_controller.asset_host + url = url.gsub(/^#{asset_host}/i, "") if asset_host.present? + Upload.where(url: url).first if Discourse.store.has_been_uploaded?(url) end end diff --git a/db/migrate/20130728172550_add_url_to_optimized_images.rb b/db/migrate/20130728172550_add_url_to_optimized_images.rb new file mode 100644 index 000000000..f22892014 --- /dev/null +++ b/db/migrate/20130728172550_add_url_to_optimized_images.rb @@ -0,0 +1,22 @@ +class AddUrlToOptimizedImages < ActiveRecord::Migration + def up + # add a nullable url column + add_column :optimized_images, :url, :string + # compute the url for existing images + execute "UPDATE optimized_images + SET url = substring(u.url from '^\/uploads\/[^/]+\/') + || '_optimized/' + || substring(oi.sha1 for 3) || '/' + || substring(oi.sha1 from 4 for 3) || '/' + || substring(oi.sha1 from 7 for 11) || oi.extension + FROM optimized_images oi + JOIN uploads u ON u.id = oi.upload_id + WHERE optimized_images.id = oi.id;" + # change the column to be non nullable + change_column :optimized_images, :url, :string, null: false + end + + def down + remove_column :optimized_images, :url + end +end diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb index 094fef76e..b72e4eaf0 100644 --- a/lib/cooked_post_processor.rb +++ b/lib/cooked_post_processor.rb @@ -77,7 +77,7 @@ class CookedPostProcessor end def relative_to_absolute(src) - if src =~ /\A\/[^\/]/ + if src =~ /^\/[^\/]/ Discourse.base_url_no_prefix + src else src @@ -98,7 +98,7 @@ class CookedPostProcessor def associate_to_post(upload) return if PostUpload.where(post_id: @post.id, upload_id: upload.id).count > 0 - PostUpload.create({ post_id: @post.id, upload_id: upload.id }) + PostUpload.create(post_id: @post.id, upload_id: upload.id) rescue ActiveRecord::RecordNotUnique # do not care if it's already associated end @@ -155,7 +155,7 @@ class CookedPostProcessor a.add_child(img) # replace the image by its thumbnail - img['src'] = upload.thumbnail_url if upload && upload.has_thumbnail? + img['src'] = relative_to_absolute(upload.thumbnail.url) if upload && upload.has_thumbnail? # then, some overlay informations meta = Nokogiri::XML::Node.new("div", @doc) @@ -206,12 +206,13 @@ class CookedPostProcessor end def get_size(url) - # make sure s3 urls have a scheme (otherwise, FastImage will fail) - url = "http:" + url if Upload.is_on_s3?(url) - return unless is_valid_image_uri?(url) + uri = url + # make sure urls have a scheme (otherwise, FastImage will fail) + uri = (SiteSetting.use_ssl? ? "https:" : "http:") + url if url.start_with?("//") + return unless is_valid_image_uri?(uri) # we can *always* crawl our own images - return unless SiteSetting.crawl_images? || Upload.has_been_uploaded?(url) - @size_cache[url] ||= FastImage.size(url) + return unless SiteSetting.crawl_images? || Discourse.store.has_been_uploaded?(url) + @size_cache[url] ||= FastImage.size(uri) rescue Zlib::BufError # FastImage.size raises BufError for some gifs end @@ -222,14 +223,9 @@ class CookedPostProcessor end def attachments - if SiteSetting.enable_s3_uploads? - @doc.css("a.attachment[href^=\"#{S3Store.base_url}\"]") - else - # local uploads are identified using a relative uri - @doc.css("a.attachment[href^=\"#{LocalStore.directory}\"]") + - # when cdn is enabled, we have the whole url - @doc.css("a.attachment[href^=\"#{LocalStore.base_url}\"]") - end + attachments = @doc.css("a.attachment[href^=\"#{Discourse.store.absolute_base_url}\"]") + attachments += @doc.css("a.attachment[href^=\"#{Discourse.store.relative_base_url}\"]") if Discourse.store.internal? + attachments end def dirty? diff --git a/lib/discourse.rb b/lib/discourse.rb index f8fe64be8..b35801ae4 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -81,7 +81,7 @@ module Discourse def self.git_version return $git_version if $git_version - f = Rails.root.to_s + "/config/version" + f = Rails.root.to_s + "/lib/version" require f if File.exists?("#{f}.rb") begin @@ -98,6 +98,16 @@ module Discourse user end + def self.store + if SiteSetting.enable_s3_uploads? + @s3_store_loaded ||= require 'file_store/s3_store' + S3Store.new + else + @local_store_loaded ||= require 'file_store/local_store' + LocalStore.new + end + end + private def self.maintenance_mode_key diff --git a/lib/file_store/local_store.rb b/lib/file_store/local_store.rb new file mode 100644 index 000000000..01bdf3516 --- /dev/null +++ b/lib/file_store/local_store.rb @@ -0,0 +1,93 @@ +class LocalStore + + def store_upload(file, upload) + unique_sha1 = Digest::SHA1.hexdigest("#{Time.now.to_s}#{file.original_filename}")[0,16] + extension = File.extname(file.original_filename) + clean_name = "#{unique_sha1}#{extension}" + path = "#{relative_base_url}/#{upload.id}/#{clean_name}" + # copy the file to the right location + copy_file(file, "#{public_dir}#{path}") + # url + Discourse.base_uri + path + end + + def store_optimized_image(file, optimized_image) + # 1234567890ABCDEF_100x200.jpg + filename = [ + optimized_image.sha1[6..16], + "_#{optimized_image.width}x#{optimized_image.height}", + optimized_image.extension, + ].join + # /public/uploads/site/_optimized/123/456/ + path = File.join( + relative_base_url, + "_optimized", + optimized_image.sha1[0..2], + optimized_image.sha1[3..5], + filename + ) + # copy the file to the right location + copy_file(file, "#{public_dir}#{path}") + # url + Discourse.base_uri + path + end + + def remove_file(url) + File.delete("#{public_dir}#{url}") if has_been_uploaded?(url) + rescue Errno::ENOENT + # don't care if the file isn't there + end + + def has_been_uploaded?(url) + is_relative?(url) || is_local?(url) + end + + def absolute_base_url + url = asset_host.present? ? asset_host : Discourse.base_url_no_prefix + "#{url}#{relative_base_url}" + end + + def relative_base_url + "/uploads/#{RailsMultisite::ConnectionManagement.current_db}" + end + + def external? + !internal? + end + + def internal? + true + end + + def path_for(upload) + "#{public_dir}#{upload.url}" + end + + private + + def copy_file(file, path) + FileUtils.mkdir_p Pathname.new(path).dirname + # move the file to the right location + # not using cause mv, cause permissions are no good on move + File.open(path, "wb") do |f| + f.write(file.read) + end + end + + def is_relative?(url) + url.start_with?(relative_base_url) + end + + def is_local?(url) + url.start_with?(absolute_base_url) + end + + def public_dir + "#{Rails.root}/public" + end + + def asset_host + Rails.configuration.action_controller.asset_host + end + +end diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb new file mode 100644 index 000000000..e864591d8 --- /dev/null +++ b/lib/file_store/s3_store.rb @@ -0,0 +1,119 @@ +require 'digest/sha1' +require 'open-uri' + +class S3Store + + def store_upload(file, upload) + extension = File.extname(file.original_filename) + remote_filename = "#{upload.id}#{upload.sha1}#{extension}" + + # if this fails, it will throw an exception + upload(file.tempfile, remote_filename, file.content_type) + + # returns the url of the uploaded file + "#{absolute_base_url}/#{remote_filename}" + end + + def store_optimized_image(file, optimized_image) + extension = File.extname(file.path) + remote_filename = [ + optimized_image.id, + optimized_image.sha1, + "_#{optimized_image.width}x#{optimized_image.height}", + extension + ].join + + # if this fails, it will throw an exception + upload(file, remote_filename) + + # returns the url of the uploaded file + "#{absolute_base_url}/#{remote_filename}" + end + + def remove_file(url) + check_missing_site_settings + return unless has_been_uploaded?(url) + name = File.basename(url) + remove(name) + end + + def has_been_uploaded?(url) + url.start_with?(absolute_base_url) + end + + def absolute_base_url + "//#{s3_bucket}.s3.amazonaws.com" + end + + def external? + true + end + + def internal? + !external? + end + + def download(upload) + temp_file = Tempfile.new(["discourse-s3", File.extname(upload.original_filename)]) + url = (SiteSetting.use_ssl? ? "https:" : "http:") + upload.url + + File.open(temp_file.path, "wb") do |f| + f.write open(url, "rb", read_timeout: 20).read + end + + temp_file + end + + private + + def s3_bucket + SiteSetting.s3_upload_bucket.downcase + end + + def check_missing_site_settings + raise Discourse::SiteSettingMissing.new("s3_upload_bucket") if SiteSetting.s3_upload_bucket.blank? + raise Discourse::SiteSettingMissing.new("s3_access_key_id") if SiteSetting.s3_access_key_id.blank? + raise Discourse::SiteSettingMissing.new("s3_secret_access_key") if SiteSetting.s3_secret_access_key.blank? + end + + def get_or_create_directory(name) + check_missing_site_settings + + @fog_loaded ||= require 'fog' + + fog = Fog::Storage.new generate_options + + directory = fog.directories.get(name) + directory = fog.directories.create(key: name) unless directory + directory + end + + def generate_options + options = { + provider: 'AWS', + aws_access_key_id: SiteSetting.s3_access_key_id, + aws_secret_access_key: SiteSetting.s3_secret_access_key, + } + options[:region] = SiteSetting.s3_region unless SiteSetting.s3_region.empty? + options + end + + def upload(file, name, content_type=nil) + args = { + key: name, + public: true, + body: file, + } + args[:content_type] = content_type if content_type + directory.files.create(args) + end + + def remove(name) + directory.files.destroy(key: name) + end + + def directory + get_or_create_directory(s3_bucket) + end + +end diff --git a/lib/image_optimizer.rb b/lib/image_optimizer.rb deleted file mode 100644 index 30df65d81..000000000 --- a/lib/image_optimizer.rb +++ /dev/null @@ -1,96 +0,0 @@ -# -# This class is used to download and optimize images. -# - -require 'image_sorcery' -require 'digest/sha1' -require 'open-uri' - -class ImageOptimizer - attr_accessor :url - - # url is a url of an image ex: - # 'http://site.com/image.png' - # '/uploads/site/image.png' - def initialize(url) - @url = url - # make sure directories exists - FileUtils.mkdir_p downloads_dir - FileUtils.mkdir_p optimized_dir - end - - # return the path of an optimized image, - # if already cached return cached, else download and cache - # at the original size. - # if size is specified return a resized image - # if height or width are nil maintain aspect ratio - # - # Optimised image is the "most efficient" storage for an image - # at the basic level it runs through image_optim https://github.com/toy/image_optim - # it also has a failsafe that converts jpg to png or the opposite. if jpg size is 1.5* - # as efficient as png it flips formats. - def optimized_image_url (width = nil, height = nil) - begin - unless has_been_uploaded? - return @url unless SiteSetting.crawl_images? - # download the file if it hasn't been cached yet - download! unless File.exists?(cached_path) - end - - # resize the image using Image Magick - result = ImageSorcery.new(cached_path).convert(optimized_path, resize: "#{width}x#{height}") - return optimized_url if result - @url - rescue - @url - end - end - -private - - def public_dir - @public_dir ||= "#{Rails.root}/public" - end - - def downloads_dir - @downloads_dir ||= "#{public_dir}/downloads/#{RailsMultisite::ConnectionManagement.current_db}" - end - - def optimized_dir - @optimized_dir ||= "#{public_dir}/images/#{RailsMultisite::ConnectionManagement.current_db}" - end - - def has_been_uploaded? - @url.start_with?(Discourse.base_url_no_prefix) - end - - def cached_path - @cached_path ||= if has_been_uploaded? - "#{public_dir}#{@url[Discourse.base_url_no_prefix.length..-1]}" - else - "#{downloads_dir}/#{file_name(@url)}" - end - end - - def optimized_path - @optimized_path ||= "#{optimized_dir}/#{file_name(cached_path)}" - end - - def file_name (uri) - image_info = FastImage.new(uri) - name = Digest::SHA1.hexdigest(uri)[0,16] - name << ".#{image_info.type}" - name - end - - def download! - File.open(cached_path, "wb") do |f| - f.write open(@url, "rb", read_timeout: 20).read - end - end - - def optimized_url - @optimized_url ||= Discourse.base_url_no_prefix + "/images/#{RailsMultisite::ConnectionManagement.current_db}/#{file_name(cached_path)}" - end - -end diff --git a/lib/local_store.rb b/lib/local_store.rb deleted file mode 100644 index ac2154281..000000000 --- a/lib/local_store.rb +++ /dev/null @@ -1,47 +0,0 @@ -module LocalStore - - def self.store_file(file, sha1, upload_id) - unique_sha1 = Digest::SHA1.hexdigest("#{Time.now.to_s}#{file.original_filename}")[0,16] - extension = File.extname(file.original_filename) - clean_name = "#{unique_sha1}#{extension}" - url_root = "#{directory}/#{upload_id}" - path = "#{Rails.root}/public#{url_root}" - - FileUtils.mkdir_p path - - # not using cause mv, cause permissions are no good on move - File.open("#{path}/#{clean_name}", "wb") do |f| - f.write File.read(file.tempfile) - end - - # url - Discourse::base_uri + "#{url_root}/#{clean_name}" - end - - def self.remove_file(url) - File.delete("#{Rails.root}/public#{url}") - rescue Errno::ENOENT - end - - def self.uploaded_regex - /\/uploads\/#{RailsMultisite::ConnectionManagement.current_db}\/(?\d+)\/[0-9a-f]{16}\.(png|jpg|jpeg|gif|tif|tiff|bmp)/ - end - - def self.base_url - url = asset_host.present? ? asset_host : Discourse.base_url_no_prefix - "#{url}#{directory}" - end - - def self.base_path - "#{Rails.root}/public#{directory}" - end - - def self.directory - "/uploads/#{RailsMultisite::ConnectionManagement.current_db}" - end - - def self.asset_host - Rails.configuration.action_controller.asset_host - end - -end diff --git a/lib/s3_store.rb b/lib/s3_store.rb deleted file mode 100644 index 3079a9df2..000000000 --- a/lib/s3_store.rb +++ /dev/null @@ -1,70 +0,0 @@ -module S3Store - - def self.store_file(file, sha1, upload_id) - S3Store.check_missing_site_settings - - directory = S3Store.get_or_create_directory(SiteSetting.s3_upload_bucket) - extension = File.extname(file.original_filename) - remote_filename = "#{upload_id}#{sha1}#{extension}" - - # if this fails, it will throw an exception - file = S3Store.upload(file, remote_filename, directory) - "#{S3Store.base_url}/#{remote_filename}" - end - - def self.base_url - "//#{SiteSetting.s3_upload_bucket.downcase}.s3.amazonaws.com" - end - - def self.remove_file(url) - S3Store.check_missing_site_settings - - directory = S3Store.get_or_create_directory(SiteSetting.s3_upload_bucket) - - file = S3Store.destroy(url, directory) - end - - def self.check_missing_site_settings - raise Discourse::SiteSettingMissing.new("s3_upload_bucket") if SiteSetting.s3_upload_bucket.blank? - raise Discourse::SiteSettingMissing.new("s3_access_key_id") if SiteSetting.s3_access_key_id.blank? - raise Discourse::SiteSettingMissing.new("s3_secret_access_key") if SiteSetting.s3_secret_access_key.blank? - end - - def self.get_or_create_directory(name) - @fog_loaded = require 'fog' unless @fog_loaded - - options = S3Store.generate_options - - fog = Fog::Storage.new(options) - - directory = fog.directories.get(name) - directory = fog.directories.create(key: name) unless directory - - directory - end - - def self.generate_options - options = { - provider: 'AWS', - aws_access_key_id: SiteSetting.s3_access_key_id, - aws_secret_access_key: SiteSetting.s3_secret_access_key - } - options[:region] = SiteSetting.s3_region unless SiteSetting.s3_region.empty? - - options - end - - def self.upload(file, name, directory) - directory.files.create( - key: name, - public: true, - body: file.tempfile, - content_type: file.content_type - ) - end - - def self.destroy(name, directory) - directory.files.destroy(key: name) - end - -end diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb index 55fae32e2..bd9d1f577 100644 --- a/spec/components/cooked_post_processor_spec.rb +++ b/spec/components/cooked_post_processor_spec.rb @@ -31,9 +31,7 @@ describe CookedPostProcessor do Upload.expects(:get_from_url).returns(upload) cpp.post_process_attachments # ensures absolute urls on attachment - cpp.html.should =~ /#{LocalStore.base_url}/ - # ensure name is present - cpp.html.should =~ /archive.zip/ + cpp.html.should =~ /#{Discourse.store.absolute_base_url}/ # keeps the reverse index up to date post.uploads.reload post.uploads.count.should == 1 @@ -74,7 +72,7 @@ describe CookedPostProcessor do Upload.expects(:get_from_url).returns(upload) cpp.post_process_images # ensures absolute urls on uploaded images - cpp.html.should =~ /#{LocalStore.base_url}/ + cpp.html.should =~ /#{LocalStore.new.absolute_base_url}/ # dirty cpp.should be_dirty # keeps the reverse index up to date @@ -227,7 +225,6 @@ describe CookedPostProcessor do let(:cpp) { CookedPostProcessor.new(post) } it "ensures s3 urls have a default scheme" do - Upload.stubs(:is_on_s3?).returns(true) FastImage.stubs(:size) cpp.expects(:is_valid_image_uri?).with("http://bucket.s3.aws.amazon.com/image.jpg") cpp.get_size("//bucket.s3.aws.amazon.com/image.jpg") @@ -239,13 +236,15 @@ describe CookedPostProcessor do it "doesn't call FastImage" do FastImage.expects(:size).never - cpp.get_size("http://foo.bar/image.png").should == nil + cpp.get_size("http://foo.bar/image1.png").should == nil end - it "is always allowed to crawled our own images" do - Upload.expects(:has_been_uploaded?).returns(true) + it "is always allowed to crawl our own images" do + store = {} + Discourse.expects(:store).returns(store) + store.expects(:has_been_uploaded?).returns(true) FastImage.expects(:size).returns([100, 200]) - cpp.get_size("http://foo.bar/image.png").should == [100, 200] + cpp.get_size("http://foo.bar/image2.png").should == [100, 200] end end @@ -253,8 +252,8 @@ describe CookedPostProcessor do it "caches the results" do SiteSetting.stubs(:crawl_images?).returns(true) FastImage.expects(:size).returns([200, 400]) - cpp.get_size("http://foo.bar/image.png") - cpp.get_size("http://foo.bar/image.png").should == [200, 400] + cpp.get_size("http://foo.bar/image3.png") + cpp.get_size("http://foo.bar/image3.png").should == [200, 400] end end diff --git a/spec/components/discourse_spec.rb b/spec/components/discourse_spec.rb index 6426bdb20..a330261f9 100644 --- a/spec/components/discourse_spec.rb +++ b/spec/components/discourse_spec.rb @@ -65,5 +65,18 @@ describe Discourse do end + context "#store" do + + it "returns LocalStore by default" do + Discourse.store.should be_a(LocalStore) + end + + it "returns S3Store when S3 is enabled" do + SiteSetting.expects(:enable_s3_uploads?).returns(true) + Discourse.store.should be_a(S3Store) + end + + end + end diff --git a/spec/components/file_store/local_store_spec.rb b/spec/components/file_store/local_store_spec.rb new file mode 100644 index 000000000..aca9e4b48 --- /dev/null +++ b/spec/components/file_store/local_store_spec.rb @@ -0,0 +1,76 @@ +require 'spec_helper' +require 'file_store/local_store' + +describe LocalStore do + + let(:store) { LocalStore.new } + + let(:upload) { build(:upload) } + let(:uploaded_file) do + ActionDispatch::Http::UploadedFile.new({ + filename: 'logo.png', + tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") + }) + end + + let(:optimized_image) { build(:optimized_image) } + + it "is internal" do + store.internal?.should == true + store.external?.should == false + end + + describe "store_upload" do + + it "returns a relative url" do + Time.stubs(:now).returns(Time.utc(2013, 2, 17, 12, 0, 0, 0)) + upload.stubs(:id).returns(42) + store.expects(:copy_file) + store.store_upload(uploaded_file, upload).should == "/uploads/default/42/253dc8edf9d4ada1.png" + end + + end + + describe "store_optimized_image" do + + it "returns a relative url" do + store.expects(:copy_file) + store.store_optimized_image({}, optimized_image).should == "/uploads/default/_optimized/86f/7e4/37faa5a7fce_100x200.png" + end + + end + + describe "remove_file" do + + it "does not delete any file" do + File.expects(:delete).never + store.remove_file("/path/to/file") + end + + it "deletes the file locally" do + File.expects(:delete) + store.remove_file("/uploads/default/42/253dc8edf9d4ada1.png") + end + + end + + describe "has_been_uploaded?" do + + it "identifies local or relatives urls" do + Discourse.expects(:base_url_no_prefix).returns("http://discuss.site.com") + store.has_been_uploaded?("http://discuss.site.com/uploads/default/42/0123456789ABCDEF.jpg").should == true + store.has_been_uploaded?("/uploads/default/42/0123456789ABCDEF.jpg").should == true + end + + it "identifies local urls when using a CDN" do + Rails.configuration.action_controller.stubs(:asset_host).returns("http://my.cdn.com") + store.has_been_uploaded?("http://my.cdn.com/uploads/default/42/0123456789ABCDEF.jpg").should == true + end + + it "does not match dummy urls" do + store.has_been_uploaded?("http://domain.com/uploads/default/42/0123456789ABCDEF.jpg").should == false + end + + end + +end diff --git a/spec/components/file_store/s3_store_spec.rb b/spec/components/file_store/s3_store_spec.rb new file mode 100644 index 000000000..8b38a4d91 --- /dev/null +++ b/spec/components/file_store/s3_store_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' +require 'fog' +require 'file_store/s3_store' + +describe S3Store do + + let(:store) { S3Store.new } + + let(:upload) { build(:upload) } + let(:uploaded_file) do + ActionDispatch::Http::UploadedFile.new({ + filename: 'logo.png', + tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") + }) + end + + let(:optimized_image) { build(:optimized_image) } + let(:optimized_image_file) do + ActionDispatch::Http::UploadedFile.new({ + filename: 'logo.png', + tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") + }) + end + + before(:each) do + SiteSetting.stubs(:s3_upload_bucket).returns("S3_Upload_Bucket") + SiteSetting.stubs(:s3_access_key_id).returns("s3_access_key_id") + SiteSetting.stubs(:s3_secret_access_key).returns("s3_secret_access_key") + Fog.mock! + end + + after(:each) { Fog.unmock! } + + it "is internal" do + store.external?.should == true + store.internal?.should == false + end + + describe "store_upload" do + + it "returns a relative url" do + upload.stubs(:id).returns(42) + store.store_upload(uploaded_file, upload).should == "//s3_upload_bucket.s3.amazonaws.com/42e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98.png" + end + + end + + describe "store_optimized_image" do + + it "returns a relative url" do + optimized_image.stubs(:id).returns(42) + store.store_optimized_image(optimized_image_file, optimized_image).should == "//s3_upload_bucket.s3.amazonaws.com/4286f7e437faa5a7fce15d1ddcb9eaeaea377667b8_100x200.png" + end + + end + + describe "remove_file" do + + it "does not delete any file" do + store.expects(:remove).never + store.remove_file("//other_bucket.s3.amazonaws.com/42.png") + end + + it "deletes the file on s3" do + store.expects(:remove) + store.remove_file("//s3_upload_bucket.s3.amazonaws.com/42.png") + end + + end + + describe "has_been_uploaded?" do + + it "identifies S3 uploads" do + SiteSetting.stubs(:enable_s3_uploads).returns(true) + store.has_been_uploaded?("//s3_upload_bucket.s3.amazonaws.com/1337.png").should == true + end + + it "does not match other s3 urls" do + store.has_been_uploaded?("//s3.amazonaws.com/Bucket/1337.png").should == false + end + + end + +end diff --git a/spec/components/local_store_spec.rb b/spec/components/local_store_spec.rb deleted file mode 100644 index 3e87081e1..000000000 --- a/spec/components/local_store_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -require 'spec_helper' -require 'local_store' - -describe LocalStore do - - describe "store_file" do - - let(:file) do - ActionDispatch::Http::UploadedFile.new({ - filename: 'logo.png', - content_type: 'image/png', - tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") - }) - end - - let(:image_info) { FastImage.new(file) } - - it 'returns the url of the uploaded file if successful' do - # prevent the tests from creating directories & files... - FileUtils.stubs(:mkdir_p) - File.stubs(:open) - # The Time needs to be frozen as it is used to generate a clean & unique name - Time.stubs(:now).returns(Time.utc(2013, 2, 17, 12, 0, 0, 0)) - # - LocalStore.store_file(file, "", 1).should == '/uploads/default/1/253dc8edf9d4ada1.png' - end - - end - -end diff --git a/spec/components/s3_store_spec.rb b/spec/components/s3_store_spec.rb deleted file mode 100644 index 1d8062407..000000000 --- a/spec/components/s3_store_spec.rb +++ /dev/null @@ -1,36 +0,0 @@ -require 'spec_helper' -require 'fog' -require 's3_store' - -describe S3Store do - - describe "store_file" do - - let(:file) do - ActionDispatch::Http::UploadedFile.new({ - filename: 'logo.png', - content_type: 'image/png', - tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") - }) - end - - let(:image_info) { FastImage.new(file) } - - before(:each) do - SiteSetting.stubs(:s3_upload_bucket).returns("S3_Upload_Bucket") - SiteSetting.stubs(:s3_access_key_id).returns("s3_access_key_id") - SiteSetting.stubs(:s3_secret_access_key).returns("s3_secret_access_key") - Fog.mock! - end - - it 'returns the url of the S3 upload if successful' do - S3Store.store_file(file, "SHA", 1).should == '//s3_upload_bucket.s3.amazonaws.com/1SHA.png' - end - - after(:each) do - Fog.unmock! - end - - end - -end diff --git a/spec/fabricators/optimized_image_fabricator.rb b/spec/fabricators/optimized_image_fabricator.rb index 3ca2fe71f..30fafd2f4 100644 --- a/spec/fabricators/optimized_image_fabricator.rb +++ b/spec/fabricators/optimized_image_fabricator.rb @@ -1,7 +1,8 @@ Fabricator(:optimized_image) do upload - sha1 "abcdef" + sha1 "86f7e437faa5a7fce15d1ddcb9eaeaea377667b8" extension ".png" width 100 height 200 + url "138569_100x200.png" end diff --git a/spec/fabricators/upload_fabricator.rb b/spec/fabricators/upload_fabricator.rb index 53a7911ea..60bf415a1 100644 --- a/spec/fabricators/upload_fabricator.rb +++ b/spec/fabricators/upload_fabricator.rb @@ -1,5 +1,6 @@ Fabricator(:upload) do user + sha1 "e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98" original_filename "uploaded.jpg" filesize 1234 width 100 diff --git a/spec/models/optimized_image_spec.rb b/spec/models/optimized_image_spec.rb index 2d2cd0c1a..b2d2db798 100644 --- a/spec/models/optimized_image_spec.rb +++ b/spec/models/optimized_image_spec.rb @@ -4,25 +4,55 @@ describe OptimizedImage do it { should belong_to :upload } - let(:upload) { build(:upload) } - let(:oi) { OptimizedImage.create_for(upload, 100, 100) } + let(:upload) { Fabricate(:upload) } + let(:oi) { OptimizedImage.create_for(upload, 100, 200) } describe ".create_for" do - before(:each) do - ImageSorcery.any_instance.expects(:convert).returns(true) - # make sure we don't hit the filesystem - FileUtils.stubs(:mkdir_p) - File.stubs(:open) + before { ImageSorcery.any_instance.expects(:convert).returns(true) } + + describe "internal store" do + + it "works" do + Tempfile.any_instance.expects(:close!) + oi.sha1.should == "da39a3ee5e6b4b0d3255bfef95601890afd80709" + oi.extension.should == ".jpg" + oi.width.should == 100 + oi.height.should == 200 + oi.url.should == "/uploads/default/_optimized/da3/9a3/ee5e6b4b0d3_100x200.jpg" + end + end - it "works" do - Tempfile.any_instance.expects(:close) - Tempfile.any_instance.expects(:unlink) - oi.sha1.should == "da39a3ee5e6b4b0d3255bfef95601890afd80709" - oi.extension.should == ".jpg" - oi.width.should == 100 - oi.height.should == 100 + describe "external store" do + + require 'file_store/s3_store' + require 'fog' + + let(:store) { S3Store.new } + + before do + Discourse.stubs(:store).returns(store) + SiteSetting.stubs(:s3_upload_bucket).returns("S3_Upload_Bucket") + SiteSetting.stubs(:s3_access_key_id).returns("s3_access_key_id") + SiteSetting.stubs(:s3_secret_access_key).returns("s3_secret_access_key") + Fog.mock! + end + + it "works" do + # fake downloaded file + downloaded_file = {} + downloaded_file.expects(:path).returns("/path/to/fake.png") + downloaded_file.expects(:close!) + store.expects(:download).returns(downloaded_file) + # assertions + oi.sha1.should == "da39a3ee5e6b4b0d3255bfef95601890afd80709" + oi.extension.should == ".png" + oi.width.should == 100 + oi.height.should == 200 + oi.url.should =~ /^\/\/s3_upload_bucket.s3.amazonaws.com\/[0-9a-f]+_100x200.png/ + end + end end diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 12de3eb20..f5a3dd148 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -42,20 +42,12 @@ describe Upload do it "does not create a thumbnail when disabled" do SiteSetting.stubs(:create_thumbnails?).returns(false) - SiteSetting.expects(:enable_s3_uploads?).never - upload.create_thumbnail! - end - - it "does not create a thumbnail when using S3" do - SiteSetting.expects(:create_thumbnails?).returns(true) - SiteSetting.expects(:enable_s3_uploads?).returns(true) - upload.expects(:has_thumbnail?).never + OptimizedImage.expects(:create_for).never upload.create_thumbnail! end it "does not create another thumbnail" do SiteSetting.expects(:create_thumbnails?).returns(true) - SiteSetting.expects(:enable_s3_uploads?).returns(false) upload.expects(:has_thumbnail?).returns(true) OptimizedImage.expects(:create_for).never upload.create_thumbnail! @@ -65,7 +57,6 @@ describe Upload do upload = Fabricate(:upload) thumbnail = Fabricate(:optimized_image, upload: upload) SiteSetting.expects(:create_thumbnails?).returns(true) - SiteSetting.expects(:enable_s3_uploads?).returns(false) upload.expects(:has_thumbnail?).returns(false) OptimizedImage.expects(:create_for).returns(thumbnail) upload.create_thumbnail! @@ -104,7 +95,9 @@ describe Upload do end it "saves proper information" do - Upload.expects(:store_file).returns(url) + store = {} + Discourse.expects(:store).returns(store) + store.expects(:store_upload).returns(url) upload = Upload.create_for(user_id, image, image_filesize) upload.user_id.should == user_id upload.original_filename.should == image.original_filename @@ -117,66 +110,6 @@ describe Upload do end - context ".store_file" do - - it "store files on s3 when enabled" do - SiteSetting.expects(:enable_s3_uploads?).returns(true) - LocalStore.expects(:store_file).never - S3Store.expects(:store_file) - Upload.store_file(image, image_sha1, 1) - end - - it "store files locally by default" do - S3Store.expects(:store_file).never - LocalStore.expects(:store_file) - Upload.store_file(image, image_sha1, 1) - end - - end - - context ".remove_file" do - - it "remove files on s3 when enabled" do - SiteSetting.expects(:enable_s3_uploads?).returns(true) - LocalStore.expects(:remove_file).never - S3Store.expects(:remove_file) - Upload.remove_file(upload.url) - end - - it "remove files locally by default" do - S3Store.expects(:remove_file).never - LocalStore.expects(:remove_file) - Upload.remove_file(upload.url) - end - - end - - context ".has_been_uploaded?" do - - it "identifies internal or relatives urls" do - Discourse.expects(:base_url_no_prefix).returns("http://discuss.site.com") - Upload.has_been_uploaded?("http://discuss.site.com/uploads/default/42/0123456789ABCDEF.jpg").should == true - Upload.has_been_uploaded?("/uploads/default/42/0123456789ABCDEF.jpg").should == true - end - - it "identifies internal urls when using a CDN" do - Rails.configuration.action_controller.expects(:asset_host).returns("http://my.cdn.com").twice - Upload.has_been_uploaded?("http://my.cdn.com/uploads/default/42/0123456789ABCDEF.jpg").should == true - end - - it "identifies S3 uploads" do - SiteSetting.stubs(:enable_s3_uploads).returns(true) - SiteSetting.stubs(:s3_upload_bucket).returns("Bucket") - Upload.has_been_uploaded?("//bucket.s3.amazonaws.com/1337.png").should == true - end - - it "identifies external urls" do - Upload.has_been_uploaded?("http://domain.com/uploads/default/42/0123456789ABCDEF.jpg").should == false - Upload.has_been_uploaded?("//s3.amazonaws.com/Bucket/1337.png").should == false - end - - end - context ".get_from_url" do it "works when the file has been uploaded" do