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/workers/activitypub/distribution_worker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/workers') 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