Skip to content

Commit

Permalink
Add Ruby 3.0 support (mastodon#16046)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
ClearlyClaire authored May 6, 2021
1 parent 0a3fa03 commit 566fc90
Show file tree
Hide file tree
Showing 19 changed files with 177 additions and 74 deletions.
27 changes: 27 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -227,6 +246,10 @@ workflows:
requires:
- install
- install-ruby2.7
- install-ruby3.0:
requires:
- install
- install-ruby2.7
- build:
requires:
- install-ruby2.7
Expand All @@ -241,6 +264,10 @@ workflows:
requires:
- install-ruby2.6
- build
- test-ruby3.0:
requires:
- install-ruby3.0
- build
- test-webui:
requires:
- install
Expand Down
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
@@ -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'

Expand Down
6 changes: 3 additions & 3 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/activitypub/outboxes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def show
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,
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/v1/accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def follow
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
Expand Down Expand Up @@ -70,7 +70,7 @@ def set_account
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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/follow_requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def account
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
Expand Down
2 changes: 1 addition & 1 deletion app/models/session_activation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def active?(id)
end

def activate(**options)
activation = create!(options)
activation = create!(**options)
purge_old
activation
end
Expand Down
19 changes: 12 additions & 7 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -370,15 +370,20 @@ def generate_sign_in_token

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

Expand All @@ -389,8 +394,8 @@ def recent_ip?(ip)
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
Expand All @@ -403,8 +408,8 @@ def pending_devise_notifications
@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
Expand Down
2 changes: 1 addition & 1 deletion app/views/admin/dashboard/index.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions app/workers/import/relationship_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) }
Expand All @@ -16,7 +16,7 @@ def perform(account_id, target_account_uri, relationship, options = {})
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
Expand All @@ -27,7 +27,7 @@ def perform(account_id, target_account_uri, relationship, options = {})
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
Expand Down
1 change: 1 addition & 0 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
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'
Expand Down
5 changes: 2 additions & 3 deletions config/initializers/session_store.rb
Original file line number Diff line number Diff line change
@@ -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
58 changes: 58 additions & 0 deletions lib/paperclip/validation_extensions.rb
Original file line number Diff line number Diff line change
@@ -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)
Loading

0 comments on commit 566fc90

Please sign in to comment.