From 0aca6b33289249f602e80def413a188c3e9c2701 Mon Sep 17 00:00:00 2001 From: Kyle Drake Date: Tue, 19 Aug 2014 18:06:35 -0700 Subject: [PATCH] fixes for thumbnail worker --- .gitignore | 2 ++ environment.rb | 11 ++++++----- models/site.rb | 12 +++++++----- tests/site_file_tests.rb | 20 ++++++++++++++++++++ workers/screenshot_worker.rb | 13 +++++++------ workers/thumbnail_worker.rb | 11 ++++++----- 6 files changed, 48 insertions(+), 21 deletions(-) diff --git a/.gitignore b/.gitignore index 4d898869..8dfbbe8e 100644 --- a/.gitignore +++ b/.gitignore @@ -22,5 +22,7 @@ public/assets/css/.sass-cache/ public/site_thumbnails public/sites public/site_screenshots +public/site_screenshots_test +public/site_thumbnails_test *.swp files/map.txt diff --git a/environment.rb b/environment.rb index e7f5c8ef..4fc702f8 100644 --- a/environment.rb +++ b/environment.rb @@ -27,8 +27,6 @@ end DB = Sequel.connect $config['database'], sslmode: 'disable', max_connections: $config['database_pool'] DB.extension :pagination -Dir.glob('workers/*.rb').each {|w| require File.join(DIR_ROOT, "/#{w}") } - if defined?(Pry) Pry.commands.alias_command 'c', 'continue' Pry.commands.alias_command 's', 'step' @@ -59,9 +57,9 @@ if $config['pubsub_url'].nil? && ENV['RACK_ENV'] == 'production' raise 'pubsub_url is missing from config' end -require File.join(DIR_ROOT, 'workers', 'thumbnail_worker.rb') -require File.join(DIR_ROOT, 'workers', 'screenshot_worker.rb') -require File.join(DIR_ROOT, 'workers', 'email_worker.rb') +#require File.join(DIR_ROOT, 'workers', 'thumbnail_worker.rb') +#require File.join(DIR_ROOT, 'workers', 'screenshot_worker.rb') +#require File.join(DIR_ROOT, 'workers', 'email_worker.rb') Sequel.datetime_class = Time Sequel.extension :core_extensions @@ -76,6 +74,9 @@ Sequel::Migrator.apply DB, './migrations' Stripe.api_key = $config['stripe_api_key'] Dir.glob('models/*.rb').each {|m| require File.join(DIR_ROOT, "#{m}") } +Dir.glob('workers/*.rb').each {|w| require File.join(DIR_ROOT, "/#{w}") } + + #DB.loggers << Logger.new(STDOUT) if ENV['RACK_ENV'] == 'development' if ENV['RACK_ENV'] == 'development' diff --git a/models/site.rb b/models/site.rb index 6492586c..ce79dacb 100644 --- a/models/site.rb +++ b/models/site.rb @@ -51,8 +51,8 @@ class Site < Sequel::Model SITE_FILES_ROOT = File.join PUBLIC_ROOT, (ENV['RACK_ENV'] == 'test' ? 'sites_test' : 'sites') SCREENSHOTS_ROOT = File.join(PUBLIC_ROOT, (ENV['RACK_ENV'] == 'test' ? 'site_screenshots_test' : 'site_screenshots')) THUMBNAILS_ROOT = File.join(PUBLIC_ROOT, (ENV['RACK_ENV'] == 'test' ? 'site_thumbnails_test' : 'site_thumbnails')) - SCREENSHOTS_URL_ROOT = '/site_screenshots' - THUMBNAILS_URL_ROOT = '/site_thumbnails' + SCREENSHOTS_URL_ROOT = ENV['RACK_ENV'] == 'test' ? '/site_screenshots_test' : '/site_screenshots' + THUMBNAILS_URL_ROOT = ENV['RACK_ENV'] == 'test' ? '/site_thumbnails_test' : '/site_thumbnails' IMAGE_REGEX = /jpg|jpeg|png|bmp|gif/ LOSSLESS_IMAGE_REGEX = /png|bmp|gif/ LOSSY_IMAGE_REGEX = /jpg|jpeg/ @@ -334,7 +334,9 @@ class Site < Sequel::Model end def store_file(path, uploaded) - path = files_path(path) + relative_path = scrubbed_path path + path = files_path path + if File.exist?(path) && Digest::SHA2.file(path).digest == Digest::SHA2.file(uploaded.path).digest return false @@ -366,7 +368,7 @@ class Site < Sequel::Model if ext.match HTML_REGEX ScreenshotWorker.perform_async values[:username], path elsif ext.match IMAGE_REGEX - ThumbnailWorker.perform_async values[:username], path + ThumbnailWorker.perform_async values[:username], relative_path end SiteChange.record self, path @@ -613,7 +615,7 @@ class Site < Sequel::Model clean << part if part != '..' end - clean + clean.join '/' end def files_path(path='') diff --git a/tests/site_file_tests.rb b/tests/site_file_tests.rb index ac920ff7..24eeb50e 100644 --- a/tests/site_file_tests.rb +++ b/tests/site_file_tests.rb @@ -12,6 +12,7 @@ describe 'site_files' do it 'succeeds with valid file' do site = Fabricate :site PurgeCacheWorker.jobs.clear + ThumbnailWorker.jobs.clear post '/site_files/upload', { 'files[]' => Rack::Test::UploadedFile.new('./tests/files/test.jpg', 'image/jpeg'), 'csrf_token' => 'abcd' @@ -22,10 +23,18 @@ describe 'site_files' do queue_args = PurgeCacheWorker.jobs.first['args'].first queue_args['site'].must_equal site.username queue_args['path'].must_equal '/test.jpg' + + ThumbnailWorker.jobs.length.must_equal 1 + ThumbnailWorker.drain + + Site::THUMBNAIL_RESOLUTIONS.each do |resolution| + File.exists?(site.thumbnail_path('test.jpg', resolution)).must_equal true + end end it 'works with directory path' do site = Fabricate :site + ThumbnailWorker.jobs.clear PurgeCacheWorker.jobs.clear post '/site_files/upload', { 'dir' => 'derpie/derptest', @@ -35,8 +44,19 @@ describe 'site_files' do last_response.body.must_match /successfully uploaded/i File.exists?(site.files_path('derpie/derptest/test.jpg')).must_equal true + PurgeCacheWorker.jobs.length.must_equal 1 queue_args = PurgeCacheWorker.jobs.first['args'].first queue_args['path'].must_equal '/derpie/derptest/test.jpg' + + ThumbnailWorker.jobs.length.must_equal 1 + ThumbnailWorker.drain + + Site::THUMBNAIL_RESOLUTIONS.each do |resolution| + File.exists?(site.thumbnail_path('derpie/derptest/test.jpg', resolution)).must_equal true + site.thumbnail_url('derpie/derptest/test.jpg', resolution).must_equal( + File.join "#{Site::THUMBNAILS_URL_ROOT}", site.username, "/derpie/derptest/test.jpg.#{resolution}.jpg" + ) + end end end end \ No newline at end of file diff --git a/workers/screenshot_worker.rb b/workers/screenshot_worker.rb index 5610eb5e..84869bf5 100644 --- a/workers/screenshot_worker.rb +++ b/workers/screenshot_worker.rb @@ -27,24 +27,25 @@ module Phantomjs end class ScreenshotWorker - SCREENSHOTS_PATH = File.join DIR_ROOT, 'public', 'site_screenshots' + SCREENSHOTS_PATH = Site::SCREENSHOTS_ROOT include Sidekiq::Worker sidekiq_options queue: :screenshots, retry: 3, backtrace: true - def perform(username, filename) + def perform(username, path) + path = "/#{path}" unless path[0] == '/' screenshot = Tempfile.new 'neocities_screenshot' screenshot.close screenshot_output_path = screenshot.path+'.png' begin - f = Screencap::Fetcher.new("http://#{username}.neocities.org/#{filename}") + f = Screencap::Fetcher.new("http://#{username}.neocities.org#{path}") f.fetch( output: screenshot_output_path, width: 1280, height: 720 ) rescue Timeout::Error - puts "#{username}/#{filename} is timing out, discontinuing" + puts "#{username}/#{path} is timing out, discontinuing" site = Site[username: username] site.update is_crashing: true @@ -54,7 +55,7 @@ class ScreenshotWorker EmailWorker.perform_async({ from: 'web@neocities.org', to: site.email, - subject: "[NeoCities] The web page \"#{filename}\" on your site (#{username}.neocities.org) is slow", + subject: "[NeoCities] The web page \"#{path}\" on your site (#{username}.neocities.org) is slow", body: "Hi there! This is an automated email to inform you that we're having issues loading your site to take a "+ "screenshot. It is possible that this is an error specific to our screenshot program, but it is much more "+ "likely that your site is too slow to be used with browsers. We don't want Neocities sites crashing browsers, "+ @@ -82,7 +83,7 @@ class ScreenshotWorker FileUtils.mkdir_p user_screenshots_path Site::SCREENSHOT_RESOLUTIONS.each do |res| - img.scale(*res.split('x').collect {|r| r.to_i}).write(File.join(user_screenshots_path, "#{filename}.#{res}.jpg")) { + img.scale(*res.split('x').collect {|r| r.to_i}).write(File.join(user_screenshots_path, "#{path}.#{res}.jpg")) { self.quality = 90 } end diff --git a/workers/thumbnail_worker.rb b/workers/thumbnail_worker.rb index d09881c4..9f0a558f 100644 --- a/workers/thumbnail_worker.rb +++ b/workers/thumbnail_worker.rb @@ -1,25 +1,26 @@ require 'RMagick' class ThumbnailWorker - THUMBNAILS_PATH = File.join DIR_ROOT, 'public', 'site_thumbnails' + THUMBNAILS_PATH = Site::THUMBNAILS_ROOT include Sidekiq::Worker sidekiq_options queue: :thumbnails, retry: 3, backtrace: true - def perform(username, filename) + def perform(username, path) img_list = Magick::ImageList.new - img_list.from_blob File.read(File.join(Site::SITE_FILES_ROOT, username, filename)) + img_list.from_blob File.read(File.join(Site::SITE_FILES_ROOT, username, path)) img = img_list.first user_thumbnails_path = File.join THUMBNAILS_PATH, username FileUtils.mkdir_p user_thumbnails_path + FileUtils.mkdir_p File.join(user_thumbnails_path, File.dirname(path)) Site::THUMBNAIL_RESOLUTIONS.each do |res| resimg = img.resize_to_fit(*res.split('x').collect {|r| r.to_i}) - format = File.extname(filename).gsub('.', '') + format = File.extname(path).gsub('.', '') save_ext = format.match(Site::LOSSY_IMAGE_REGEX) ? 'jpg' : 'png' - resimg.write(File.join(user_thumbnails_path, "#{filename}.#{res}.#{save_ext}")) { + resimg.write(File.join(user_thumbnails_path, "#{path}.#{res}.#{save_ext}")) { self.quality = 90 } end