summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorClaire <claire.github-309c@sitedethib.com>2021-05-06 14:22:54 +0200
committerGitHub <noreply@github.com>2021-05-06 14:22:54 +0200
commit566fc909134586d1746ad60ee455832dec6bc61a (patch)
tree26c8f77002555a8e7277d6ab9b2f4241b3fdbc38
parent0a3fa034fc66246dbf9dfb4627a983e0903042d4 (diff)
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
-rw-r--r--.circleci/config.yml27
-rw-r--r--Gemfile2
-rw-r--r--Gemfile.lock6
-rw-r--r--app/controllers/activitypub/outboxes_controller.rb2
-rw-r--r--app/controllers/api/v1/accounts_controller.rb4
-rw-r--r--app/controllers/api/v1/follow_requests_controller.rb2
-rw-r--r--app/models/session_activation.rb2
-rw-r--r--app/models/user.rb19
-rw-r--r--app/views/admin/dashboard/index.html.haml2
-rw-r--r--app/workers/import/relationship_worker.rb6
-rw-r--r--config/application.rb1
-rw-r--r--config/initializers/session_store.rb5
-rw-r--r--lib/paperclip/validation_extensions.rb58
-rw-r--r--lib/terrapin/multi_pipe_extensions.rb87
-rw-r--r--spec/controllers/admin/dashboard_controller_spec.rb12
-rw-r--r--spec/mailers/notification_mailer_spec.rb4
-rw-r--r--spec/mailers/user_mailer_spec.rb4
-rw-r--r--spec/models/session_activation_spec.rb6
-rw-r--r--spec/presenters/account_relationships_presenter_spec.rb2
19 files changed, 177 insertions, 74 deletions
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 862fa126b7f..2f3860d7c1c 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 6ca0a81deb7..5a55d6b0463 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 b1ae4fd226a..980750b63bc 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 5fd735ad6af..11128503669 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 996f1b79bf5..95869f5541f 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 b34c76f29e9..f4b2a74d0ae 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 b0ce9d11282..3a59bad9395 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 0440627c598..4973c68b61b 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 57a753e6b6d..e8a2b46fd77 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 4a7100435f7..6791b15c302 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 37a996224f7..08a4e4c97a2 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 e5d1be4c6c7..3d9bf96fd7c 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 00000000000..0df0434f664
--- /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 51d7de37c59..209f4ad6ced 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<IO>] 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 73b50e72188..7824854f9c7 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 3ae10621877..9b645bad892 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 6b430b5056d..9c866788f20 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 2aa6950370a..450dc1399f7 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 f8b048d38af..edfbbb35493 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 } }