From f7117646afddb2676e9275d8efe90c3a20c59021 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 12 Apr 2021 12:37:14 +0200 Subject: Add cold-start follow recommendations (#15945) --- .../scheduler/follow_recommendations_scheduler.rb | 61 ++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 app/workers/scheduler/follow_recommendations_scheduler.rb (limited to 'app/workers') diff --git a/app/workers/scheduler/follow_recommendations_scheduler.rb b/app/workers/scheduler/follow_recommendations_scheduler.rb new file mode 100644 index 000000000..0a0286496 --- /dev/null +++ b/app/workers/scheduler/follow_recommendations_scheduler.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +class Scheduler::FollowRecommendationsScheduler + include Sidekiq::Worker + include Redisable + + sidekiq_options retry: 0 + + # The maximum number of accounts that can be requested in one page from the + # API is 80, and the suggestions API does not allow pagination. This number + # leaves some room for accounts being filtered during live access + SET_SIZE = 100 + + def perform + # Maintaining a materialized view speeds-up subsequent queries significantly + AccountSummary.refresh + + fallback_recommendations = FollowRecommendation.safe.filtered.limit(SET_SIZE).index_by(&:account_id) + + I18n.available_locales.each do |locale| + recommendations = begin + if AccountSummary.safe.filtered.localized(locale).exists? # We can skip the work if no accounts with that language exist + FollowRecommendation.safe.filtered.localized(locale).limit(SET_SIZE).index_by(&:account_id) + else + {} + end + end + + # Use language-agnostic results if there are not enough language-specific ones + missing = SET_SIZE - recommendations.keys.size + + if missing.positive? + added = 0 + + # Avoid duplicate results + fallback_recommendations.each_value do |recommendation| + next if recommendations.key?(recommendation.account_id) + + recommendations[recommendation.account_id] = recommendation + added += 1 + + break if added >= missing + end + end + + redis.pipelined do + redis.del(key(locale)) + + recommendations.each_value do |recommendation| + redis.zadd(key(locale), recommendation.rank, recommendation.account_id) + end + end + end + end + + private + + def key(locale) + "follow_recommendations:#{locale}" + end +end -- cgit From 120965eb0b1b0da6906bb242da50a77367defd96 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 12 Apr 2021 14:25:34 +0200 Subject: Change Web Push API deliveries to use request pooling (#16014) --- Gemfile | 2 +- Gemfile.lock | 2 +- app/models/web/push_subscription.rb | 91 +++++++++++------------ app/workers/web/push_notification_worker.rb | 65 +++++++++++++--- spec/workers/web/push_notification_worker_spec.rb | 48 ++++++++++++ 5 files changed, 150 insertions(+), 58 deletions(-) create mode 100644 spec/workers/web/push_notification_worker_spec.rb (limited to 'app/workers') diff --git a/Gemfile b/Gemfile index d4385f014..cc77ee618 100644 --- a/Gemfile +++ b/Gemfile @@ -94,7 +94,7 @@ gem 'tty-prompt', '~> 0.23', require: false gem 'twitter-text', '~> 3.1.0' gem 'tzinfo-data', '~> 1.2021' gem 'webpacker', '~> 5.2' -gem 'webpush' +gem 'webpush', '~> 0.3' gem 'webauthn', '~> 3.0.0.alpha1' gem 'json-ld' diff --git a/Gemfile.lock b/Gemfile.lock index b1fb350d4..1d3c24595 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -808,5 +808,5 @@ DEPENDENCIES webauthn (~> 3.0.0.alpha1) webmock (~> 3.12) webpacker (~> 5.2) - webpush + webpush (~> 0.3) xorcist (~> 1.1) diff --git a/app/models/web/push_subscription.rb b/app/models/web/push_subscription.rb index c407a7789..7609b1bfc 100644 --- a/app/models/web/push_subscription.rb +++ b/app/models/web/push_subscription.rb @@ -24,81 +24,80 @@ class Web::PushSubscription < ApplicationRecord validates :key_p256dh, presence: true validates :key_auth, presence: true - def push(notification) - I18n.with_locale(associated_user&.locale || I18n.default_locale) do - push_payload(payload_for_notification(notification), 48.hours.seconds) - end + delegate :locale, to: :associated_user + + def encrypt(payload) + Webpush::Encryption.encrypt(payload, key_p256dh, key_auth) + end + + def audience + @audience ||= Addressable::URI.parse(endpoint).normalized_site + end + + def crypto_key_header + p256ecdsa = vapid_key.public_key_for_push_header + + "p256ecdsa=#{p256ecdsa}" + end + + def authorization_header + jwt = JWT.encode({ aud: audience, exp: 24.hours.from_now.to_i, sub: "mailto:#{contact_email}" }, vapid_key.curve, 'ES256', typ: 'JWT') + + "WebPush #{jwt}" end def pushable?(notification) - data&.key?('alerts') && ActiveModel::Type::Boolean.new.cast(data['alerts'][notification.type.to_s]) + ActiveModel::Type::Boolean.new.cast(data&.dig('alerts', notification.type.to_s)) end def associated_user return @associated_user if defined?(@associated_user) - @associated_user = if user_id.nil? - session_activation.user - else - user - end + @associated_user = begin + if user_id.nil? + session_activation.user + else + user + end + end end def associated_access_token return @associated_access_token if defined?(@associated_access_token) - @associated_access_token = if access_token_id.nil? - find_or_create_access_token.token - else - access_token.token - end + @associated_access_token = begin + if access_token_id.nil? + find_or_create_access_token.token + else + access_token.token + end + end end class << self def unsubscribe_for(application_id, resource_owner) - access_token_ids = Doorkeeper::AccessToken.where(application_id: application_id, resource_owner_id: resource_owner.id, revoked_at: nil) - .pluck(:id) - + access_token_ids = Doorkeeper::AccessToken.where(application_id: application_id, resource_owner_id: resource_owner.id, revoked_at: nil).pluck(:id) where(access_token_id: access_token_ids).delete_all end end private - def push_payload(message, ttl = 5.minutes.seconds) - Webpush.payload_send( - message: Oj.dump(message), - endpoint: endpoint, - p256dh: key_p256dh, - auth: key_auth, - ttl: ttl, - ssl_timeout: 10, - open_timeout: 10, - read_timeout: 10, - vapid: { - subject: "mailto:#{::Setting.site_contact_email}", - private_key: Rails.configuration.x.vapid_private_key, - public_key: Rails.configuration.x.vapid_public_key, - } - ) - end - - def payload_for_notification(notification) - ActiveModelSerializers::SerializableResource.new( - notification, - serializer: Web::NotificationSerializer, - scope: self, - scope_name: :current_push_subscription - ).as_json - end - def find_or_create_access_token Doorkeeper::AccessToken.find_or_create_for( application: Doorkeeper::Application.find_by(superapp: true), - resource_owner: session_activation.user_id, + resource_owner: user_id || session_activation.user_id, scopes: Doorkeeper::OAuth::Scopes.from_string('read write follow push'), expires_in: Doorkeeper.configuration.access_token_expires_in, use_refresh_token: Doorkeeper.configuration.refresh_token_enabled? ) end + + def vapid_key + @vapid_key ||= Webpush::VapidKey.from_keys(Rails.configuration.x.vapid_public_key, Rails.configuration.x.vapid_private_key) + end + + def contact_email + @contact_email ||= ::Setting.site_contact_email + end end diff --git a/app/workers/web/push_notification_worker.rb b/app/workers/web/push_notification_worker.rb index 46aeaa30b..57f5b5c22 100644 --- a/app/workers/web/push_notification_worker.rb +++ b/app/workers/web/push_notification_worker.rb @@ -3,22 +3,67 @@ class Web::PushNotificationWorker include Sidekiq::Worker - sidekiq_options backtrace: true, retry: 5 + sidekiq_options queue: 'push', retry: 5 + + TTL = 48.hours.to_s + URGENCY = 'normal' def perform(subscription_id, notification_id) - subscription = ::Web::PushSubscription.find(subscription_id) - notification = Notification.find(notification_id) + @subscription = Web::PushSubscription.find(subscription_id) + @notification = Notification.find(notification_id) + + # Polymorphically associated activity could have been deleted + # in the meantime, so we have to double-check before proceeding + return unless @notification.activity.present? && @subscription.pushable?(@notification) + + payload = @subscription.encrypt(push_notification_json) - subscription.push(notification) unless notification.activity.nil? - rescue Webpush::ResponseError => e - code = e.response.code.to_i + request_pool.with(@subscription.audience) do |http_client| + request = Request.new(:post, @subscription.endpoint, body: payload.fetch(:ciphertext), http_client: http_client) - if (400..499).cover?(code) && ![408, 429].include?(code) - subscription.destroy! - else - raise e + request.add_headers( + 'Content-Type' => 'application/octet-stream', + 'Ttl' => TTL, + 'Urgency' => URGENCY, + 'Content-Encoding' => 'aesgcm', + 'Encryption' => "salt=#{Webpush.encode64(payload.fetch(:salt)).delete('=')}", + 'Crypto-Key' => "dh=#{Webpush.encode64(payload.fetch(:server_public_key)).delete('=')};#{@subscription.crypto_key_header}", + 'Authorization' => @subscription.authorization_header + ) + + request.perform do |response| + # If the server responds with an error in the 4xx range + # that isn't about rate-limiting or timeouts, we can + # assume that the subscription is invalid or expired + # and must be removed + + if (400..499).cover?(response.code) && ![408, 429].include?(response.code) + @subscription.destroy! + elsif !(200...300).cover?(response.code) + raise Mastodon::UnexpectedResponseError, response + end + end end rescue ActiveRecord::RecordNotFound true end + + private + + def push_notification_json + json = I18n.with_locale(@subscription.locale || I18n.default_locale) do + ActiveModelSerializers::SerializableResource.new( + @notification, + serializer: Web::NotificationSerializer, + scope: @subscription, + scope_name: :current_push_subscription + ).as_json + end + + Oj.dump(json) + end + + def request_pool + RequestPool.current + end end diff --git a/spec/workers/web/push_notification_worker_spec.rb b/spec/workers/web/push_notification_worker_spec.rb new file mode 100644 index 000000000..5bc24f888 --- /dev/null +++ b/spec/workers/web/push_notification_worker_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Web::PushNotificationWorker do + subject { described_class.new } + + let(:p256dh) { 'BN4GvZtEZiZuqFxSKVZfSfluwKBD7UxHNBmWkfiZfCtgDE8Bwh-_MtLXbBxTBAWH9r7IPKL0lhdcaqtL1dfxU5E=' } + let(:auth) { 'Q2BoAjC09xH3ywDLNJr-dA==' } + let(:endpoint) { 'https://updates.push.services.mozilla.com/push/v1/subscription-id' } + let(:user) { Fabricate(:user) } + let(:notification) { Fabricate(:notification) } + let(:subscription) { Fabricate(:web_push_subscription, user_id: user.id, key_p256dh: p256dh, key_auth: auth, endpoint: endpoint, data: { alerts: { notification.type => true } }) } + let(:vapid_public_key) { 'BB37UCyc8LLX4PNQSe-04vSFvpUWGrENubUaslVFM_l5TxcGVMY0C3RXPeUJAQHKYlcOM2P4vTYmkoo0VZGZTM4=' } + let(:vapid_private_key) { 'OPrw1Sum3gRoL4-DXfSCC266r-qfFSRZrnj8MgIhRHg=' } + let(:vapid_key) { Webpush::VapidKey.from_keys(vapid_public_key, vapid_private_key) } + let(:contact_email) { 'sender@example.com' } + let(:ciphertext) { "+\xB8\xDBT}\x13\xB6\xDD.\xF9\xB0\xA7\xC8\xD2\x80\xFD\x99#\xF7\xAC\x83\xA4\xDB,\x1F\xB5\xB9w\x85>\xF7\xADr" } + let(:salt) { "X\x97\x953\xE4X\xF8_w\xE7T\x95\xC51q\xFE" } + let(:server_public_key) { "\x04\b-RK9w\xDD$\x16lFz\xF9=\xB4~\xC6\x12k\xF3\xF40t\xA9\xC1\fR\xC3\x81\x80\xAC\f\x7F\xE4\xCC\x8E\xC2\x88 n\x8BB\xF1\x9C\x14\a\xFA\x8D\xC9\x80\xA1\xDDyU\\&c\x01\x88#\x118Ua" } + let(:shared_secret) { "\t\xA7&\x85\t\xC5m\b\xA8\xA7\xF8B{1\xADk\xE1y'm\xEDE\xEC\xDD\xEDj\xB3$s\xA9\xDA\xF0" } + let(:payload) { { ciphertext: ciphertext, salt: salt, server_public_key: server_public_key, shared_secret: shared_secret } } + + describe 'perform' do + before do + allow_any_instance_of(subscription.class).to receive(:contact_email).and_return(contact_email) + allow_any_instance_of(subscription.class).to receive(:vapid_key).and_return(vapid_key) + allow(Webpush::Encryption).to receive(:encrypt).and_return(payload) + allow(JWT).to receive(:encode).and_return('jwt.encoded.payload') + + stub_request(:post, endpoint).to_return(status: 201, body: '') + + subject.perform(subscription.id, notification.id) + end + + it 'calls the relevant service with the correct headers' do + expect(a_request(:post, endpoint).with(headers: { + 'Content-Encoding' => 'aesgcm', + 'Content-Type' => 'application/octet-stream', + 'Crypto-Key' => 'dh=BAgtUks5d90kFmxGevk9tH7GEmvz9DB0qcEMUsOBgKwMf-TMjsKIIG6LQvGcFAf6jcmAod15VVwmYwGIIxE4VWE;p256ecdsa=' + vapid_public_key.delete('='), + 'Encryption' => 'salt=WJeVM-RY-F9351SVxTFx_g', + 'Ttl' => '172800', + 'Urgency' => 'normal', + 'Authorization' => 'WebPush jwt.encoded.payload', + }, body: "+\xB8\xDBT}\u0013\xB6\xDD.\xF9\xB0\xA7\xC8Ҁ\xFD\x99#\xF7\xAC\x83\xA4\xDB,\u001F\xB5\xB9w\x85>\xF7\xADr")).to have_been_made + end + end +end -- cgit From f4b7c6b61914070e590507bcb33e4345d3f9b0b9 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 24 Apr 2021 13:35:39 +0200 Subject: Fix nil error when removing status caused by race condition (#16099) --- app/lib/status_reach_finder.rb | 2 +- app/models/status.rb | 4 ++++ app/workers/activitypub/distribution_worker.rb | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) (limited to 'app/workers') diff --git a/app/lib/status_reach_finder.rb b/app/lib/status_reach_finder.rb index 0e755d433..735d66a4f 100644 --- a/app/lib/status_reach_finder.rb +++ b/app/lib/status_reach_finder.rb @@ -62,7 +62,7 @@ class StatusReachFinder end def followers_inboxes - if @status.reply? && @status.thread.account.local? && @status.distributable? + if @status.in_reply_to_local_account? && @status.distributable? @status.account.followers.or(@status.thread.account.followers).inboxes else @status.account.followers.inboxes diff --git a/app/models/status.rb b/app/models/status.rb index 74e81f612..847921ac2 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -161,6 +161,10 @@ class Status < ApplicationRecord attributes['local'] || uri.nil? end + def in_reply_to_local_account? + reply? && thread&.account&.local? + end + def reblog? !reblog_of_id.nil? end diff --git a/app/workers/activitypub/distribution_worker.rb b/app/workers/activitypub/distribution_worker.rb index 9b4814644..09898ca49 100644 --- a/app/workers/activitypub/distribution_worker.rb +++ b/app/workers/activitypub/distribution_worker.rb @@ -35,7 +35,7 @@ class ActivityPub::DistributionWorker # Deliver the status to all followers. # If the status is a reply to another local status, also forward it to that # status' authors' followers. - @inboxes ||= if @status.reply? && @status.thread.account.local? && @status.distributable? + @inboxes ||= if @status.in_reply_to_local_account? && @status.distributable? @account.followers.or(@status.thread.account.followers).inboxes else @account.followers.inboxes -- cgit From f78cbc0c32d5fc648c92b5e1de02105d6a8594c0 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 26 Apr 2021 18:56:45 +0200 Subject: Fix thread resolve worker retrying when status no longer exists (#16109) --- app/workers/thread_resolve_worker.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'app/workers') diff --git a/app/workers/thread_resolve_worker.rb b/app/workers/thread_resolve_worker.rb index 8bba9ca75..1b77dfdd9 100644 --- a/app/workers/thread_resolve_worker.rb +++ b/app/workers/thread_resolve_worker.rb @@ -14,5 +14,7 @@ class ThreadResolveWorker child_status.thread = parent_status child_status.save! + rescue ActiveRecord::RecordNotFound + true end end -- cgit From fab65848d2eb8065ef3e49aaca4e4fb33f94f2b1 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 4 May 2021 04:45:08 +0200 Subject: Fix empty home feed before first follow has finished processing (#16152) Change queue of merge worker from pull to default --- app/models/concerns/account_interactions.rb | 8 ++++++++ app/models/user.rb | 4 +--- app/services/follow_service.rb | 9 +++++++++ app/workers/merge_worker.rb | 4 ++-- 4 files changed, 20 insertions(+), 5 deletions(-) (limited to 'app/workers') diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index 51e8e04a8..958f6c78e 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -184,6 +184,14 @@ module AccountInteractions active_relationships.where(target_account: other_account).exists? end + def following_anyone? + active_relationships.exists? + end + + def not_following_anyone? + !following_anyone? + end + def blocking?(other_account) block_relationships.where(target_account: other_account).exists? end diff --git a/app/models/user.rb b/app/models/user.rb index 5a149f573..0440627c5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -458,9 +458,7 @@ class User < ApplicationRecord end def regenerate_feed! - return unless Redis.current.setnx("account:#{account_id}:regeneration", true) - Redis.current.expire("account:#{account_id}:regeneration", 1.day.seconds) - RegenerationWorker.perform_async(account_id) + RegenerationWorker.perform_async(account_id) if Redis.current.set("account:#{account_id}:regeneration", true, nx: true, ex: 1.day.seconds) end def needs_feed_update? diff --git a/app/services/follow_service.rb b/app/services/follow_service.rb index d3db07a74..329262cca 100644 --- a/app/services/follow_service.rb +++ b/app/services/follow_service.rb @@ -30,6 +30,11 @@ class FollowService < BaseService ActivityTracker.increment('activity:interactions') + # When an account follows someone for the first time, avoid showing + # an empty home feed while the follow request is being processed + # and the feeds are being merged + mark_home_feed_as_partial! if @source_account.not_following_anyone? + if (@target_account.locked? && !@options[:bypass_locked]) || @source_account.silenced? || @target_account.activitypub? request_follow! elsif @target_account.local? @@ -39,6 +44,10 @@ class FollowService < BaseService private + def mark_home_feed_as_partial! + redis.set("account:#{@source_account.id}:regeneration", true, nx: true, ex: 1.day.seconds) + end + def following_not_possible? @target_account.nil? || @target_account.id == @source_account.id || @target_account.suspended? end diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb index 74ef7d4da..6ebb9a400 100644 --- a/app/workers/merge_worker.rb +++ b/app/workers/merge_worker.rb @@ -3,11 +3,11 @@ class MergeWorker include Sidekiq::Worker - sidekiq_options queue: 'pull' - def perform(from_account_id, into_account_id) FeedManager.instance.merge_into_home(Account.find(from_account_id), Account.find(into_account_id)) rescue ActiveRecord::RecordNotFound true + ensure + Redis.current.del("account:#{into_account_id}:regeneration") end end -- cgit From d9ae3db8d5543cf0b7fa44186c191c9bb2472d23 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 5 May 2021 22:04:52 +0200 Subject: Improve performance of follow recommendation scheduler (#16159) Express follow_recommendations in terms of account_summaries rather than accounts, integrate filters that are unconditionally used, and materialize the resulting view. This should result in the bulk of the computation being performed only once instead of **once per recommendation language**. --- app/models/follow_recommendation.rb | 6 ++-- .../scheduler/follow_recommendations_scheduler.rb | 5 ++-- ...6_update_follow_recommendations_to_version_2.rb | 18 ++++++++++++ db/schema.rb | 28 ++++++++++-------- db/views/follow_recommendations_v02.sql | 34 ++++++++++++++++++++++ 5 files changed, 75 insertions(+), 16 deletions(-) create mode 100644 db/migrate/20210505174616_update_follow_recommendations_to_version_2.rb create mode 100644 db/views/follow_recommendations_v02.sql (limited to 'app/workers') diff --git a/app/models/follow_recommendation.rb b/app/models/follow_recommendation.rb index 6670b6560..1ed6dc49b 100644 --- a/app/models/follow_recommendation.rb +++ b/app/models/follow_recommendation.rb @@ -14,9 +14,11 @@ class FollowRecommendation < ApplicationRecord belongs_to :account_summary, foreign_key: :account_id belongs_to :account, foreign_key: :account_id - scope :safe, -> { joins(:account_summary).merge(AccountSummary.safe) } scope :localized, ->(locale) { joins(:account_summary).merge(AccountSummary.localized(locale)) } - scope :filtered, -> { joins(:account_summary).merge(AccountSummary.filtered) } + + def self.refresh + Scenic.database.refresh_materialized_view(table_name, concurrently: true, cascade: false) + end def readonly? true diff --git a/app/workers/scheduler/follow_recommendations_scheduler.rb b/app/workers/scheduler/follow_recommendations_scheduler.rb index 0a0286496..cb1e15961 100644 --- a/app/workers/scheduler/follow_recommendations_scheduler.rb +++ b/app/workers/scheduler/follow_recommendations_scheduler.rb @@ -14,13 +14,14 @@ class Scheduler::FollowRecommendationsScheduler def perform # Maintaining a materialized view speeds-up subsequent queries significantly AccountSummary.refresh + FollowRecommendation.refresh - fallback_recommendations = FollowRecommendation.safe.filtered.limit(SET_SIZE).index_by(&:account_id) + fallback_recommendations = FollowRecommendation.limit(SET_SIZE).index_by(&:account_id) I18n.available_locales.each do |locale| recommendations = begin if AccountSummary.safe.filtered.localized(locale).exists? # We can skip the work if no accounts with that language exist - FollowRecommendation.safe.filtered.localized(locale).limit(SET_SIZE).index_by(&:account_id) + FollowRecommendation.localized(locale).limit(SET_SIZE).index_by(&:account_id) else {} end diff --git a/db/migrate/20210505174616_update_follow_recommendations_to_version_2.rb b/db/migrate/20210505174616_update_follow_recommendations_to_version_2.rb new file mode 100644 index 000000000..9b2a284e4 --- /dev/null +++ b/db/migrate/20210505174616_update_follow_recommendations_to_version_2.rb @@ -0,0 +1,18 @@ +class UpdateFollowRecommendationsToVersion2 < ActiveRecord::Migration[6.1] + # We're switching from a normal to a materialized view so we need + # custom `up` and `down` paths. + + def up + drop_view :follow_recommendations + create_view :follow_recommendations, version: 2, materialized: true + + # To be able to refresh the view concurrently, + # at least one unique index is required + safety_assured { add_index :follow_recommendations, :account_id, unique: true } + end + + def down + drop_view :follow_recommendations, materialized: true + create_view :follow_recommendations, version: 1 + end +end diff --git a/db/schema.rb b/db/schema.rb index 0d951ee95..88e906079 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_04_25_135952) do +ActiveRecord::Schema.define(version: 2021_05_05_174616) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1114,30 +1114,34 @@ ActiveRecord::Schema.define(version: 2021_04_25_135952) do SQL add_index "account_summaries", ["account_id"], name: "index_account_summaries_on_account_id", unique: true - create_view "follow_recommendations", sql_definition: <<-SQL + create_view "follow_recommendations", materialized: true, sql_definition: <<-SQL SELECT t0.account_id, sum(t0.rank) AS rank, array_agg(t0.reason) AS reason - FROM ( SELECT accounts.id AS account_id, + FROM ( SELECT account_summaries.account_id, ((count(follows.id))::numeric / (1.0 + (count(follows.id))::numeric)) AS rank, 'most_followed'::text AS reason - FROM ((follows - JOIN accounts ON ((accounts.id = follows.target_account_id))) + FROM (((follows + JOIN account_summaries ON ((account_summaries.account_id = follows.target_account_id))) JOIN users ON ((users.account_id = follows.account_id))) - WHERE ((users.current_sign_in_at >= (now() - 'P30D'::interval)) AND (accounts.suspended_at IS NULL) AND (accounts.moved_to_account_id IS NULL) AND (accounts.silenced_at IS NULL) AND (accounts.locked = false) AND (accounts.discoverable = true)) - GROUP BY accounts.id + LEFT JOIN follow_recommendation_suppressions ON ((follow_recommendation_suppressions.account_id = follows.target_account_id))) + WHERE ((users.current_sign_in_at >= (now() - 'P30D'::interval)) AND (account_summaries.sensitive = false) AND (follow_recommendation_suppressions.id IS NULL)) + GROUP BY account_summaries.account_id HAVING (count(follows.id) >= 5) UNION ALL - SELECT accounts.id AS account_id, + SELECT account_summaries.account_id, (sum((status_stats.reblogs_count + status_stats.favourites_count)) / (1.0 + sum((status_stats.reblogs_count + status_stats.favourites_count)))) AS rank, 'most_interactions'::text AS reason - FROM ((status_stats + FROM (((status_stats JOIN statuses ON ((statuses.id = status_stats.status_id))) - JOIN accounts ON ((accounts.id = statuses.account_id))) - WHERE ((statuses.id >= (((date_part('epoch'::text, (now() - 'P30D'::interval)) * (1000)::double precision))::bigint << 16)) AND (accounts.suspended_at IS NULL) AND (accounts.moved_to_account_id IS NULL) AND (accounts.silenced_at IS NULL) AND (accounts.locked = false) AND (accounts.discoverable = true)) - GROUP BY accounts.id + JOIN account_summaries ON ((account_summaries.account_id = statuses.account_id))) + LEFT JOIN follow_recommendation_suppressions ON ((follow_recommendation_suppressions.account_id = statuses.account_id))) + WHERE ((statuses.id >= (((date_part('epoch'::text, (now() - 'P30D'::interval)) * (1000)::double precision))::bigint << 16)) AND (account_summaries.sensitive = false) AND (follow_recommendation_suppressions.id IS NULL)) + GROUP BY account_summaries.account_id HAVING (sum((status_stats.reblogs_count + status_stats.favourites_count)) >= (5)::numeric)) t0 GROUP BY t0.account_id ORDER BY (sum(t0.rank)) DESC; SQL + add_index "follow_recommendations", ["account_id"], name: "index_follow_recommendations_on_account_id", unique: true + end diff --git a/db/views/follow_recommendations_v02.sql b/db/views/follow_recommendations_v02.sql new file mode 100644 index 000000000..673c5cc85 --- /dev/null +++ b/db/views/follow_recommendations_v02.sql @@ -0,0 +1,34 @@ +SELECT + account_id, + sum(rank) AS rank, + array_agg(reason) AS reason +FROM ( + SELECT + account_summaries.account_id AS account_id, + count(follows.id) / (1.0 + count(follows.id)) AS rank, + 'most_followed' AS reason + FROM follows + INNER JOIN account_summaries ON account_summaries.account_id = follows.target_account_id + INNER JOIN users ON users.account_id = follows.account_id + LEFT OUTER JOIN follow_recommendation_suppressions ON follow_recommendation_suppressions.account_id = follows.target_account_id + WHERE users.current_sign_in_at >= (now() - interval '30 days') + AND account_summaries.sensitive = 'f' + AND follow_recommendation_suppressions.id IS NULL + GROUP BY account_summaries.account_id + HAVING count(follows.id) >= 5 + UNION ALL + SELECT account_summaries.account_id AS account_id, + sum(reblogs_count + favourites_count) / (1.0 + sum(reblogs_count + favourites_count)) AS rank, + 'most_interactions' AS reason + FROM status_stats + INNER JOIN statuses ON statuses.id = status_stats.status_id + INNER JOIN account_summaries ON account_summaries.account_id = statuses.account_id + LEFT OUTER JOIN follow_recommendation_suppressions ON follow_recommendation_suppressions.account_id = statuses.account_id + WHERE statuses.id >= ((date_part('epoch', now() - interval '30 days') * 1000)::bigint << 16) + AND account_summaries.sensitive = 'f' + AND follow_recommendation_suppressions.id IS NULL + GROUP BY account_summaries.account_id + HAVING sum(reblogs_count + favourites_count) >= 5 +) t0 +GROUP BY account_id +ORDER BY rank DESC -- cgit From 6d9ad30bf861b2422c5951f7593a657675fedc24 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 5 May 2021 23:46:59 +0200 Subject: Fix media redownload worker retrying on unexpected response codes (#16111) --- app/workers/redownload_media_worker.rb | 11 ++++++++++- lib/exceptions.rb | 4 ++++ 2 files changed, 14 insertions(+), 1 deletion(-) (limited to 'app/workers') diff --git a/app/workers/redownload_media_worker.rb b/app/workers/redownload_media_worker.rb index 0638cd0f0..343caa32c 100644 --- a/app/workers/redownload_media_worker.rb +++ b/app/workers/redownload_media_worker.rb @@ -3,6 +3,7 @@ class RedownloadMediaWorker include Sidekiq::Worker include ExponentialBackoff + include JsonLdHelper sidekiq_options queue: 'pull', retry: 3 @@ -15,6 +16,14 @@ class RedownloadMediaWorker media_attachment.download_thumbnail! media_attachment.save rescue ActiveRecord::RecordNotFound - true + # Do nothing + rescue Mastodon::UnexpectedResponseError => e + response = e.response + + if response_error_unsalvageable?(response) + # Give up + else + raise e + end end end diff --git a/lib/exceptions.rb b/lib/exceptions.rb index 7c8e77871..eb472abaa 100644 --- a/lib/exceptions.rb +++ b/lib/exceptions.rb @@ -12,7 +12,11 @@ module Mastodon class RateLimitExceededError < Error; end class UnexpectedResponseError < Error + attr_reader :response + def initialize(response = nil) + @response = response + if response.respond_to? :uri super("#{response.uri} returned code #{response.code}") else -- cgit From 566fc909134586d1746ad60ee455832dec6bc61a Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 6 May 2021 14:22:54 +0200 Subject: Add Ruby 3.0 support (#16046) * Fix issues with POSIX::Spawn, Terrapin and Ruby 3.0 Also improve the Terrapin monkey-patch for the stderr/stdout issue. * Fix keyword argument handling throughout the codebase * Monkey-patch Paperclip to fix keyword arguments handling in validators * Change validation_extensions to please CodeClimate * Bump microformats from 4.2.1 to 4.3.1 * Allow Ruby 3.0 * Add Ruby 3.0 test target to CircleCI * Add test for admin dashboard warnings * Fix admin dashboard warnings on Ruby 3.0 --- .circleci/config.yml | 27 +++++++ Gemfile | 2 +- Gemfile.lock | 6 +- app/controllers/activitypub/outboxes_controller.rb | 2 +- app/controllers/api/v1/accounts_controller.rb | 4 +- .../api/v1/follow_requests_controller.rb | 2 +- app/models/session_activation.rb | 2 +- app/models/user.rb | 19 +++-- app/views/admin/dashboard/index.html.haml | 2 +- app/workers/import/relationship_worker.rb | 6 +- config/application.rb | 1 + config/initializers/session_store.rb | 5 +- lib/paperclip/validation_extensions.rb | 58 +++++++++++++++ lib/terrapin/multi_pipe_extensions.rb | 87 +++++++++++----------- .../controllers/admin/dashboard_controller_spec.rb | 12 ++- spec/mailers/notification_mailer_spec.rb | 4 +- spec/mailers/user_mailer_spec.rb | 4 +- spec/models/session_activation_spec.rb | 6 +- .../account_relationships_presenter_spec.rb | 2 +- 19 files changed, 177 insertions(+), 74 deletions(-) create mode 100644 lib/paperclip/validation_extensions.rb (limited to 'app/workers') diff --git a/.circleci/config.yml b/.circleci/config.yml index 862fa126b..2f3860d7c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -129,6 +129,13 @@ jobs: environment: *ruby_environment <<: *install_ruby_dependencies + install-ruby3.0: + <<: *defaults + docker: + - image: circleci/ruby:3.0-buster-node + environment: *ruby_environment + <<: *install_ruby_dependencies + build: <<: *defaults steps: @@ -187,6 +194,18 @@ jobs: - image: circleci/redis:5-alpine <<: *test_steps + test-ruby3.0: + <<: *defaults + docker: + - image: circleci/ruby:3.0-buster-node + environment: *ruby_environment + - image: circleci/postgres:12.2 + environment: + POSTGRES_USER: root + POSTGRES_HOST_AUTH_METHOD: trust + - image: circleci/redis:5-alpine + <<: *test_steps + test-webui: <<: *defaults docker: @@ -227,6 +246,10 @@ workflows: requires: - install - install-ruby2.7 + - install-ruby3.0: + requires: + - install + - install-ruby2.7 - build: requires: - install-ruby2.7 @@ -241,6 +264,10 @@ workflows: requires: - install-ruby2.6 - build + - test-ruby3.0: + requires: + - install-ruby3.0 + - build - test-webui: requires: - install diff --git a/Gemfile b/Gemfile index 6ca0a81de..5a55d6b04 100644 --- a/Gemfile +++ b/Gemfile @@ -1,7 +1,7 @@ # frozen_string_literal: true source 'https://rubygems.org' -ruby '>= 2.5.0', '< 3.0.0' +ruby '>= 2.5.0', '< 3.1.0' gem 'pkg-config', '~> 1.4' diff --git a/Gemfile.lock b/Gemfile.lock index b1ae4fd22..980750b63 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -292,7 +292,7 @@ GEM ipaddress (0.8.3) iso-639 (0.3.5) jmespath (1.4.0) - json (2.3.1) + json (2.5.1) json-canonicalization (0.2.1) json-ld (3.1.9) htmlentities (~> 4.3) @@ -344,7 +344,7 @@ GEM redis (>= 3.0.5) memory_profiler (1.0.0) method_source (1.0.0) - microformats (4.2.1) + microformats (4.3.1) json (~> 2.2) nokogiri (~> 1.10) mime-types (3.3.1) @@ -354,7 +354,7 @@ GEM nokogiri (~> 1) rake mini_mime (1.0.3) - mini_portile2 (2.5.0) + mini_portile2 (2.5.1) minitest (5.14.4) msgpack (1.4.2) multi_json (1.15.0) diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb index 5fd735ad6..111285036 100644 --- a/app/controllers/activitypub/outboxes_controller.rb +++ b/app/controllers/activitypub/outboxes_controller.rb @@ -20,7 +20,7 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController def outbox_presenter if page_requested? ActivityPub::CollectionPresenter.new( - id: outbox_url(page_params), + id: outbox_url(**page_params), type: :ordered, part_of: outbox_url, prev: prev_page, diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 996f1b79b..95869f554 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -35,7 +35,7 @@ class Api::V1::AccountsController < Api::BaseController follow = FollowService.new.call(current_user.account, @account, reblogs: params.key?(:reblogs) ? truthy_param?(:reblogs) : nil, notify: params.key?(:notify) ? truthy_param?(:notify) : nil, with_rate_limit: true) options = @account.locked? || current_user.account.silenced? ? {} : { following_map: { @account.id => { reblogs: follow.show_reblogs?, notify: follow.notify? } }, requested_map: { @account.id => false } } - render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(options) + render json: @account, serializer: REST::RelationshipSerializer, relationships: relationships(**options) end def block @@ -70,7 +70,7 @@ class Api::V1::AccountsController < Api::BaseController end def relationships(**options) - AccountRelationshipsPresenter.new([@account.id], current_user.account_id, options) + AccountRelationshipsPresenter.new([@account.id], current_user.account_id, **options) end def account_params diff --git a/app/controllers/api/v1/follow_requests_controller.rb b/app/controllers/api/v1/follow_requests_controller.rb index b34c76f29..f4b2a74d0 100644 --- a/app/controllers/api/v1/follow_requests_controller.rb +++ b/app/controllers/api/v1/follow_requests_controller.rb @@ -29,7 +29,7 @@ class Api::V1::FollowRequestsController < Api::BaseController end def relationships(**options) - AccountRelationshipsPresenter.new([params[:id]], current_user.account_id, options) + AccountRelationshipsPresenter.new([params[:id]], current_user.account_id, **options) end def load_accounts diff --git a/app/models/session_activation.rb b/app/models/session_activation.rb index b0ce9d112..3a59bad93 100644 --- a/app/models/session_activation.rb +++ b/app/models/session_activation.rb @@ -44,7 +44,7 @@ class SessionActivation < ApplicationRecord end def activate(**options) - activation = create!(options) + activation = create!(**options) purge_old activation end diff --git a/app/models/user.rb b/app/models/user.rb index 0440627c5..4973c68b6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -370,15 +370,20 @@ class User < ApplicationRecord protected - def send_devise_notification(notification, *args) + def send_devise_notification(notification, *args, **kwargs) # This method can be called in `after_update` and `after_commit` hooks, # but we must make sure the mailer is actually called *after* commit, # otherwise it may work on stale data. To do this, figure out if we are # within a transaction. + + # It seems like devise sends keyword arguments as a hash in the last + # positional argument + kwargs = args.pop if args.last.is_a?(Hash) && kwargs.empty? + if ActiveRecord::Base.connection.current_transaction.try(:records)&.include?(self) - pending_devise_notifications << [notification, args] + pending_devise_notifications << [notification, args, kwargs] else - render_and_send_devise_message(notification, *args) + render_and_send_devise_message(notification, *args, **kwargs) end end @@ -389,8 +394,8 @@ class User < ApplicationRecord end def send_pending_devise_notifications - pending_devise_notifications.each do |notification, args| - render_and_send_devise_message(notification, *args) + pending_devise_notifications.each do |notification, args, kwargs| + render_and_send_devise_message(notification, *args, **kwargs) end # Empty the pending notifications array because the @@ -403,8 +408,8 @@ class User < ApplicationRecord @pending_devise_notifications ||= [] end - def render_and_send_devise_message(notification, *args) - devise_mailer.send(notification, self, *args).deliver_later + def render_and_send_devise_message(notification, *args, **kwargs) + devise_mailer.send(notification, self, *args, **kwargs).deliver_later end def set_approved diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index 57a753e6b..e8a2b46fd 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -5,7 +5,7 @@ .flash-message-stack - @system_checks.each do |message| .flash-message.warning - = t("admin.system_checks.#{message.key}.message_html", message.value ? { value: content_tag(:strong, message.value) } : {}) + = t("admin.system_checks.#{message.key}.message_html", value: message.value ? content_tag(:strong, message.value) : nil) - if message.action = link_to t("admin.system_checks.#{message.key}.action"), message.action diff --git a/app/workers/import/relationship_worker.rb b/app/workers/import/relationship_worker.rb index 4a7100435..6791b15c3 100644 --- a/app/workers/import/relationship_worker.rb +++ b/app/workers/import/relationship_worker.rb @@ -5,7 +5,7 @@ class Import::RelationshipWorker sidekiq_options queue: 'pull', retry: 8, dead: false - def perform(account_id, target_account_uri, relationship, options = {}) + def perform(account_id, target_account_uri, relationship, options) from_account = Account.find(account_id) target_domain = domain(target_account_uri) target_account = stoplight_wrap_request(target_domain) { ResolveAccountService.new.call(target_account_uri, { check_delivery_availability: true }) } @@ -16,7 +16,7 @@ class Import::RelationshipWorker case relationship when 'follow' begin - FollowService.new.call(from_account, target_account, options) + FollowService.new.call(from_account, target_account, **options) rescue ActiveRecord::RecordInvalid raise if FollowLimitValidator.limit_for_account(from_account) < from_account.following_count end @@ -27,7 +27,7 @@ class Import::RelationshipWorker when 'unblock' UnblockService.new.call(from_account, target_account) when 'mute' - MuteService.new.call(from_account, target_account, options) + MuteService.new.call(from_account, target_account, **options) when 'unmute' UnmuteService.new.call(from_account, target_account) end diff --git a/config/application.rb b/config/application.rb index 37a996224..08a4e4c97 100644 --- a/config/application.rb +++ b/config/application.rb @@ -10,6 +10,7 @@ require_relative '../lib/exceptions' require_relative '../lib/enumerable' require_relative '../lib/sanitize_ext/sanitize_config' require_relative '../lib/redis/namespace_extensions' +require_relative '../lib/paperclip/validation_extensions' require_relative '../lib/paperclip/url_generator_extensions' require_relative '../lib/paperclip/attachment_extensions' require_relative '../lib/paperclip/media_type_spoof_detector_extensions' diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index e5d1be4c6..3d9bf96fd 100644 --- a/config/initializers/session_store.rb +++ b/config/initializers/session_store.rb @@ -1,7 +1,6 @@ # Be sure to restart your server when you modify this file. -Rails.application.config.session_store :cookie_store, { +Rails.application.config.session_store :cookie_store, key: '_mastodon_session', secure: (Rails.env.production? || ENV['LOCAL_HTTPS'] == 'true'), - same_site: :lax, -} + same_site: :lax diff --git a/lib/paperclip/validation_extensions.rb b/lib/paperclip/validation_extensions.rb new file mode 100644 index 000000000..0df0434f6 --- /dev/null +++ b/lib/paperclip/validation_extensions.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +# Monkey-patch various Paperclip validators for Ruby 3.0 compatibility + +module Paperclip + module Validators + module AttachmentSizeValidatorExtensions + def validate_each(record, attr_name, _value) + base_attr_name = attr_name + attr_name = "#{attr_name}_file_size".to_sym + value = record.send(:read_attribute_for_validation, attr_name) + + if value.present? + options.slice(*Paperclip::Validators::AttachmentSizeValidator::AVAILABLE_CHECKS).each do |option, option_value| + option_value = option_value.call(record) if option_value.is_a?(Proc) + option_value = extract_option_value(option, option_value) + + next if value.send(Paperclip::Validators::AttachmentSizeValidator::CHECKS[option], option_value) + + error_message_key = options[:in] ? :in_between : option + [attr_name, base_attr_name].each do |error_attr_name| + record.errors.add(error_attr_name, error_message_key, **filtered_options(value).merge( + min: min_value_in_human_size(record), + max: max_value_in_human_size(record), + count: human_size(option_value) + )) + end + end + end + end + end + + module AttachmentContentTypeValidatorExtensions + def mark_invalid(record, attribute, types) + record.errors.add attribute, :invalid, **options.merge({ types: types.join(', ') }) + end + end + + module AttachmentPresenceValidatorExtensions + def validate_each(record, attribute, _value) + if record.send("#{attribute}_file_name").blank? + record.errors.add(attribute, :blank, **options) + end + end + end + + module AttachmentFileNameValidatorExtensions + def mark_invalid(record, attribute, patterns) + record.errors.add attribute, :invalid, options.merge({ names: patterns.join(', ') }) + end + end + end +end + +Paperclip::Validators::AttachmentSizeValidator.prepend(Paperclip::Validators::AttachmentSizeValidatorExtensions) +Paperclip::Validators::AttachmentContentTypeValidator.prepend(Paperclip::Validators::AttachmentContentTypeValidatorExtensions) +Paperclip::Validators::AttachmentPresenceValidator.prepend(Paperclip::Validators::AttachmentPresenceValidatorExtensions) +Paperclip::Validators::AttachmentFileNameValidator.prepend(Paperclip::Validators::AttachmentFileNameValidatorExtensions) diff --git a/lib/terrapin/multi_pipe_extensions.rb b/lib/terrapin/multi_pipe_extensions.rb index 51d7de37c..209f4ad6c 100644 --- a/lib/terrapin/multi_pipe_extensions.rb +++ b/lib/terrapin/multi_pipe_extensions.rb @@ -1,61 +1,64 @@ # frozen_string_literal: false -# Fix adapted from https://github.com/thoughtbot/terrapin/pull/5 + +require 'fcntl' module Terrapin module MultiPipeExtensions - def read - read_streams(@stdout_in, @stderr_in) - end + def initialize + @stdout_in, @stdout_out = IO.pipe + @stderr_in, @stderr_out = IO.pipe - def close_read - begin - @stdout_in.close - rescue IOError - # Do nothing - end - - begin - @stderr_in.close - rescue IOError - # Do nothing - end + clear_nonblocking_flags! end - def read_streams(output, error) - @stdout_output = '' - @stderr_output = '' + def pipe_options + # Add some flags to explicitly close the other end of the pipes + { out: @stdout_out, err: @stderr_out, @stdout_in => :close, @stderr_in => :close } + end - read_fds = [output, error] + def read + # While we are patching Terrapin, fix child process potentially getting stuck on writing + # to stderr. - until read_fds.empty? - to_read, = IO.select(read_fds) + @stdout_output = +'' + @stderr_output = +'' - if to_read.include?(output) - @stdout_output << read_stream(output) - read_fds.delete(output) if output.closed? - end + fds_to_read = [@stdout_in, @stderr_in] + until fds_to_read.empty? + rs, = IO.select(fds_to_read) - if to_read.include?(error) - @stderr_output << read_stream(error) - read_fds.delete(error) if error.closed? - end + read_nonblocking!(@stdout_in, @stdout_output, fds_to_read) if rs.include?(@stdout_in) + read_nonblocking!(@stderr_in, @stderr_output, fds_to_read) if rs.include?(@stderr_in) end end - def read_stream(io) - result = '' - - begin - while (partial_result = io.read_nonblock(8192)) - result << partial_result - end - rescue EOFError, Errno::EPIPE - io.close - rescue Errno::EINTR, Errno::EWOULDBLOCK, Errno::EAGAIN - # Do nothing + private + + # @param [IO] io IO Stream to read until there is nothing to read + # @param [String] result Mutable string to which read values will be appended to + # @param [Array] fds_to_read Mutable array from which `io` should be removed on EOF + def read_nonblocking!(io, result, fds_to_read) + while (partial_result = io.read_nonblock(8192)) + result << partial_result end + rescue IO::WaitReadable + # Do nothing + rescue EOFError + fds_to_read.delete(io) + end + + def clear_nonblocking_flags! + # Ruby 3.0 sets pipes to non-blocking mode, and resets the flags as + # needed when calling fork/exec-related syscalls, but posix-spawn does + # not currently do that, so we need to do it manually for the time being + # so that the child process do not error out when the buffers are full. + stdout_flags = @stdout_out.fcntl(Fcntl::F_GETFL) + @stdout_out.fcntl(Fcntl::F_SETFL, stdout_flags & ~Fcntl::O_NONBLOCK) if stdout_flags & Fcntl::O_NONBLOCK - result + stderr_flags = @stderr_out.fcntl(Fcntl::F_GETFL) + @stderr_out.fcntl(Fcntl::F_SETFL, stderr_flags & ~Fcntl::O_NONBLOCK) if stderr_flags & Fcntl::O_NONBLOCK + rescue NameError, NotImplementedError, Errno::EINVAL + # Probably on windows, where pipes are blocking by default end end end diff --git a/spec/controllers/admin/dashboard_controller_spec.rb b/spec/controllers/admin/dashboard_controller_spec.rb index 73b50e721..7824854f9 100644 --- a/spec/controllers/admin/dashboard_controller_spec.rb +++ b/spec/controllers/admin/dashboard_controller_spec.rb @@ -3,9 +3,19 @@ require 'rails_helper' describe Admin::DashboardController, type: :controller do + render_views + describe 'GET #index' do - it 'returns 200' do + before do + allow(Admin::SystemCheck).to receive(:perform).and_return([ + Admin::SystemCheck::Message.new(:database_schema_check), + Admin::SystemCheck::Message.new(:rules_check, nil, admin_rules_path), + Admin::SystemCheck::Message.new(:sidekiq_process_check, 'foo, bar'), + ]) sign_in Fabricate(:user, admin: true) + end + + it 'returns 200' do get :index expect(response).to have_http_status(200) diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb index 3ae106218..9b645bad8 100644 --- a/spec/mailers/notification_mailer_spec.rb +++ b/spec/mailers/notification_mailer_spec.rb @@ -10,12 +10,12 @@ RSpec.describe NotificationMailer, type: :mailer do it 'renders subject localized for the locale of the receiver' do locale = %i(de en).sample receiver.update!(locale: locale) - expect(mail.subject).to eq I18n.t(*args, kwrest.merge(locale: locale)) + expect(mail.subject).to eq I18n.t(*args, **kwrest.merge(locale: locale)) end it 'renders subject localized for the default locale if the locale of the receiver is unavailable' do receiver.update!(locale: nil) - expect(mail.subject).to eq I18n.t(*args, kwrest.merge(locale: I18n.default_locale)) + expect(mail.subject).to eq I18n.t(*args, **kwrest.merge(locale: I18n.default_locale)) end end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 6b430b505..9c866788f 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -9,12 +9,12 @@ describe UserMailer, type: :mailer do it 'renders subject localized for the locale of the receiver' do locale = I18n.available_locales.sample receiver.update!(locale: locale) - expect(mail.subject).to eq I18n.t(*args, kwrest.merge(locale: locale)) + expect(mail.subject).to eq I18n.t(*args, **kwrest.merge(locale: locale)) end it 'renders subject localized for the default locale if the locale of the receiver is unavailable' do receiver.update!(locale: nil) - expect(mail.subject).to eq I18n.t(*args, kwrest.merge(locale: I18n.default_locale)) + expect(mail.subject).to eq I18n.t(*args, **kwrest.merge(locale: I18n.default_locale)) end end diff --git a/spec/models/session_activation_spec.rb b/spec/models/session_activation_spec.rb index 2aa695037..450dc1399 100644 --- a/spec/models/session_activation_spec.rb +++ b/spec/models/session_activation_spec.rb @@ -74,13 +74,13 @@ RSpec.describe SessionActivation, type: :model do let(:options) { { user: Fabricate(:user), session_id: '1' } } it 'calls create! and purge_old' do - expect(described_class).to receive(:create!).with(options) + expect(described_class).to receive(:create!).with(**options) expect(described_class).to receive(:purge_old) - described_class.activate(options) + described_class.activate(**options) end it 'returns an instance of SessionActivation' do - expect(described_class.activate(options)).to be_kind_of SessionActivation + expect(described_class.activate(**options)).to be_kind_of SessionActivation end end diff --git a/spec/presenters/account_relationships_presenter_spec.rb b/spec/presenters/account_relationships_presenter_spec.rb index f8b048d38..edfbbb354 100644 --- a/spec/presenters/account_relationships_presenter_spec.rb +++ b/spec/presenters/account_relationships_presenter_spec.rb @@ -13,7 +13,7 @@ RSpec.describe AccountRelationshipsPresenter do allow(Account).to receive(:domain_blocking_map).with(account_ids, current_account_id).and_return(default_map) end - let(:presenter) { AccountRelationshipsPresenter.new(account_ids, current_account_id, options) } + let(:presenter) { AccountRelationshipsPresenter.new(account_ids, current_account_id, **options) } let(:current_account_id) { Fabricate(:account).id } let(:account_ids) { [Fabricate(:account).id] } let(:default_map) { { 1 => true } } -- cgit