From 85b00d19b840bae98322c2372e8e0e3ebd66f2e0 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 26 Mar 2016 13:42:10 +0100 Subject: [PATCH] Moving Salmon notifications to background processing, fixing mini-profiler behaviour with Turbolinks enabled, optimizing Rabl for production --- Gemfile | 3 +++ Gemfile.lock | 11 +++++++++++ app/assets/javascripts/profiler.coffee | 7 +++++-- app/controllers/auth/registrations_controller.rb | 2 +- app/models/status.rb | 2 +- app/services/favourite_service.rb | 8 +------- app/services/follow_service.rb | 6 +----- app/services/process_mentions_service.rb | 6 +----- app/services/reblog_service.rb | 8 ++------ app/services/unfollow_service.rb | 8 +------- app/workers/notification_worker.rb | 7 +++++++ config/environments/development.rb | 6 ++++++ config/environments/production.rb | 2 +- config/initializers/rabl_init.rb | 3 +++ spec/rails_helper.rb | 5 +++++ 15 files changed, 49 insertions(+), 35 deletions(-) create mode 100644 app/workers/notification_worker.rb diff --git a/Gemfile b/Gemfile index c0d36ba5b..4fb7b2a73 100644 --- a/Gemfile +++ b/Gemfile @@ -51,6 +51,7 @@ end group :test do gem 'simplecov', require: false gem 'webmock' + gem 'rspec-sidekiq' end group :development do @@ -59,6 +60,8 @@ group :development do gem 'better_errors' gem 'binding_of_caller' gem 'letter_opener' + gem 'bullet' + gem 'memory_profiler' end group :production do diff --git a/Gemfile.lock b/Gemfile.lock index b7000b0b6..37ec40b48 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -47,6 +47,9 @@ GEM binding_of_caller (0.7.2) debug_inspector (>= 0.0.1) builder (3.2.2) + bullet (5.0.0) + activesupport (>= 3.0.0) + uniform_notifier (~> 1.9.0) climate_control (0.0.3) activesupport (>= 3.0) cocaine (0.5.8) @@ -139,6 +142,7 @@ GEM nokogiri (>= 1.5.9) mail (2.6.3) mime-types (>= 1.16, < 3) + memory_profiler (0.9.6) method_source (0.8.2) mime-types (2.99.1) mimemagic (0.3.0) @@ -252,6 +256,9 @@ GEM rspec-expectations (~> 3.4.0) rspec-mocks (~> 3.4.0) rspec-support (~> 3.4.0) + rspec-sidekiq (2.2.0) + rspec (~> 3.0, >= 3.0.0) + sidekiq (>= 2.4.0) rspec-support (3.4.1) rubocop (0.38.0) parser (>= 2.3.0.6, < 3.0) @@ -316,6 +323,7 @@ GEM unf_ext unf_ext (0.0.7.2) unicode-display_width (1.0.2) + uniform_notifier (1.9.0) warden (1.2.6) rack (>= 1.0) web-console (2.3.0) @@ -336,6 +344,7 @@ DEPENDENCIES addressable better_errors binding_of_caller + bullet coffee-rails (~> 4.1.0) devise doorkeeper @@ -352,6 +361,7 @@ DEPENDENCIES jbuilder (~> 2.0) jquery-rails letter_opener + memory_profiler nokogiri oj onebox @@ -370,6 +380,7 @@ DEPENDENCIES rails_autolink redis (~> 3.2) rspec-rails + rspec-sidekiq rubocop sass-rails (~> 5.0) sdoc (~> 0.4.0) diff --git a/app/assets/javascripts/profiler.coffee b/app/assets/javascripts/profiler.coffee index bcdcc1e59..40dfd0af6 100644 --- a/app/assets/javascripts/profiler.coffee +++ b/app/assets/javascripts/profiler.coffee @@ -1,2 +1,5 @@ -$(document).on 'turbolinks:load', -> - window.MiniProfiler.pageTransition() unless typeof window.MiniProfiler == 'undefined' +$ -> + $(document).on 'turbolinks:load', -> + unless typeof window.MiniProfiler == 'undefined' + window.MiniProfiler.init() + window.MiniProfiler.pageTransition() diff --git a/app/controllers/auth/registrations_controller.rb b/app/controllers/auth/registrations_controller.rb index 5e1f5214b..ad0fd4437 100644 --- a/app/controllers/auth/registrations_controller.rb +++ b/app/controllers/auth/registrations_controller.rb @@ -16,7 +16,7 @@ class Auth::RegistrationsController < Devise::RegistrationsController end end - def after_sign_up_path_for(resource) + def after_sign_up_path_for(_resource) root_path end end diff --git a/app/models/status.rb b/app/models/status.rb index 439cd3053..7ea92df2c 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -17,7 +17,7 @@ class Status < ActiveRecord::Base validates :text, presence: true, if: Proc.new { |s| s.local? && !s.reblog? } scope :with_counters, -> { select('statuses.*, (select count(r.id) from statuses as r where r.reblog_of_id = statuses.id) as reblogs_count, (select count(f.id) from favourites as f where f.status_id = statuses.id) as favourites_count') } - scope :with_includes, -> { includes(:account, :mentions, :stream_entry, reblog: [:account, :mentions], thread: [:account, :mentions]) } + scope :with_includes, -> { includes(:account, :mentions, :stream_entry, reblog: [:account, :mentions], thread: :account) } def local? self.uri.nil? diff --git a/app/services/favourite_service.rb b/app/services/favourite_service.rb index 4a8ecfacb..3d0bc718c 100644 --- a/app/services/favourite_service.rb +++ b/app/services/favourite_service.rb @@ -10,15 +10,9 @@ class FavouriteService < BaseService if status.local? NotificationMailer.favourite(status, account).deliver_later else - send_interaction_service.(favourite.stream_entry, status.account) + NotificationWorker.perform_async(favourite.stream_entry.id, status.account_id) end favourite end - - private - - def send_interaction_service - @send_interaction_service ||= SendInteractionService.new - end end diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb index 0661c63f7..06bbfa8f0 100644 --- a/app/services/follow_service.rb +++ b/app/services/follow_service.rb @@ -8,7 +8,7 @@ class FollowService < BaseService return nil if target_account.nil? follow = source_account.follow!(target_account) - send_interaction_service.(follow.stream_entry, target_account) + NotificationWorker.perform_async(follow.stream_entry.id, target_account.id) source_account.ping!(account_url(source_account, format: 'atom'), [Rails.configuration.x.hub_url]) follow end @@ -18,8 +18,4 @@ class FollowService < BaseService def follow_remote_account_service @follow_remote_account_service ||= FollowRemoteAccountService.new end - - def send_interaction_service - @send_interaction_service ||= SendInteractionService.new - end end diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index ba9486c1f..aec150a94 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -24,7 +24,7 @@ class ProcessMentionsService < BaseService if mentioned_account.local? NotificationMailer.mention(mentioned_account, status).deliver_later else - send_interaction_service.(status.stream_entry, mentioned_account) + NotificationWorker.perform_async(status.stream_entry.id, mentioned_account.id) end end end @@ -34,8 +34,4 @@ class ProcessMentionsService < BaseService def follow_remote_account_service @follow_remote_account_service ||= FollowRemoteAccountService.new end - - def send_interaction_service - @send_interaction_service ||= SendInteractionService.new - end end diff --git a/app/services/reblog_service.rb b/app/services/reblog_service.rb index 606970e23..f149477c7 100644 --- a/app/services/reblog_service.rb +++ b/app/services/reblog_service.rb @@ -5,13 +5,13 @@ class ReblogService < BaseService # @return [Status] def call(account, reblogged_status) reblog = account.statuses.create!(reblog: reblogged_status, text: '') - fan_out_on_write_service.(reblog) + DistributionWorker.perform_async(reblog.id) account.ping!(account_url(account, format: 'atom'), [Rails.configuration.x.hub_url]) if reblogged_status.local? NotificationMailer.reblog(reblogged_status, account).deliver_later else - send_interaction_service.(reblog.stream_entry, reblogged_status.account) + NotificationWorker.perform_async(reblog.stream_entry.id, reblogged_status.account_id) end reblog @@ -22,8 +22,4 @@ class ReblogService < BaseService def send_interaction_service @send_interaction_service ||= SendInteractionService.new end - - def fan_out_on_write_service - @fan_out_on_write_service ||= FanOutOnWriteService.new - end end diff --git a/app/services/unfollow_service.rb b/app/services/unfollow_service.rb index 039838d32..92d86a2c5 100644 --- a/app/services/unfollow_service.rb +++ b/app/services/unfollow_service.rb @@ -4,12 +4,6 @@ class UnfollowService < BaseService # @param [Account] target_account Which to unfollow def call(source_account, target_account) follow = source_account.unfollow!(target_account) - send_interaction_service.(follow.stream_entry, target_account) unless target_account.local? - end - - private - - def send_interaction_service - @send_interaction_service ||= SendInteractionService.new + NotificationWorker.perform_async(follow.stream_entry.id, target_account.id) unless target_account.local? end end diff --git a/app/workers/notification_worker.rb b/app/workers/notification_worker.rb new file mode 100644 index 000000000..129512a5a --- /dev/null +++ b/app/workers/notification_worker.rb @@ -0,0 +1,7 @@ +class NotificationWorker + include Sidekiq::Worker + + def perform(stream_entry_id, target_account_id) + SendInteractionService.new.(StreamEntry.find(stream_entry_id), Account.find(target_account_id)) + end +end diff --git a/config/environments/development.rb b/config/environments/development.rb index 76e52312d..564630813 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -40,4 +40,10 @@ Rails.application.configure do # config.action_view.raise_on_missing_translations = true config.action_mailer.delivery_method = :letter_opener + + config.after_initialize do + Bullet.enable = true + Bullet.bullet_logger = true + Bullet.rails_logger = true + end end diff --git a/config/environments/production.rb b/config/environments/production.rb index 1f08fad5a..968960eb6 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -55,7 +55,7 @@ Rails.application.configure do # config.logger = ActiveSupport::TaggedLogging.new(SyslogLogger.new) # Use a different cache store in production. - # config.cache_store = :mem_cache_store + config.cache_store = :memory_store, size: 128.megabytes # Enable serving of images, stylesheets, and JavaScripts from an asset server. # config.action_controller.asset_host = 'http://assets.example.com' diff --git a/config/initializers/rabl_init.rb b/config/initializers/rabl_init.rb index f11eb190d..d14f8c54e 100644 --- a/config/initializers/rabl_init.rb +++ b/config/initializers/rabl_init.rb @@ -1,3 +1,6 @@ Rabl.configure do |config| + config.cache_all_output = true + config.cache_sources = !!Rails.env.production? config.include_json_root = false + config.view_paths = [Rails.root.join('app/views')] end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index cf21bcd8d..71e99e16e 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -9,6 +9,7 @@ require 'webmock/rspec' ActiveRecord::Migration.maintain_test_schema! WebMock.disable_net_connect! +Sidekiq::Testing.inline! RSpec.configure do |config| config.fixture_path = "#{::Rails.root}/spec/fixtures" @@ -20,6 +21,10 @@ RSpec.configure do |config| config.include Devise::TestHelpers, type: :view end +RSpec::Sidekiq.configure do |config| + config.warn_when_jobs_not_processed_by_sidekiq = false +end + def request_fixture(name) File.read(File.join(Rails.root, 'spec', 'fixtures', 'requests', name)) end