Merge 9f133083d0 into c2d8666bbf

Gemfile

@@ -158,3 +158,4 @@ gem 'concurrent-ruby', require: false
 gem 'connection_pool', require: false
 gem 'xorcist', '~> 1.1'
 gem 'cocoon', '~> 1.2'
+gem 'mail', '~> 2.8'

Gemfile.lock

@@ -816,6 +816,7 @@ DEPENDENCIES
   letter_opener_web (~> 2.0)
   link_header (~> 0.0)
   lograge (~> 0.12)
+  mail (~> 2.8)
   makara (~> 0.5)
   mario-redis-lock (~> 1.2)
   memory_profiler

app/controllers/admin/domain_allows_controller.rb

@@ -25,6 +25,8 @@ class Admin::DomainAllowsController < Admin::BaseController
   def destroy
     authorize @domain_allow, :destroy?
     UnallowDomainService.new.call(@domain_allow)
+    log_action :destroy, @domain_allow
+
     redirect_to admin_instances_path, notice: I18n.t('admin.domain_allows.destroyed_msg')
   end
 

app/controllers/api/v1/admin/domain_blocks_controller.rb

@@ -19,10 +19,11 @@ class Api::V1::Admin::DomainBlocksController < Api::BaseController
   def create
     authorize :domain_block, :create?
 
+    @domain_block = DomainBlock.new(resource_params)
     existing_domain_block = resource_params[:domain].present? ? DomainBlock.rule_for(resource_params[:domain]) : nil
-    return render json: existing_domain_block, serializer: REST::Admin::ExistingDomainBlockErrorSerializer, status: 422 if existing_domain_block.present?
+    return render json: existing_domain_block, serializer: REST::Admin::ExistingDomainBlockErrorSerializer, status: 422 if conflicts_with_existing_block?(@domain_block, existing_domain_block)
 
-    @domain_block = DomainBlock.create!(resource_params)
+    @domain_block.save!
     DomainBlockWorker.perform_async(@domain_block.id)
     log_action :create, @domain_block
     render json: @domain_block, serializer: REST::Admin::DomainBlockSerializer
@@ -55,6 +56,10 @@ class Api::V1::Admin::DomainBlocksController < Api::BaseController
 
   private
 
+  def conflicts_with_existing_block?(domain_block, existing_domain_block)
+    existing_domain_block.present? && (existing_domain_block.domain == TagManager.instance.normalize_domain(domain_block.domain) || !domain_block.stricter_than?(existing_domain_block))
+  end
+
   def set_domain_blocks
     @domain_blocks = filtered_domain_blocks.order(id: :desc).to_a_paginated_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id))
   end

app/controllers/api/v1/featured_tags/suggestions_controller.rb

@@ -12,6 +12,10 @@ class Api::V1::FeaturedTags::SuggestionsController < Api::BaseController
   private
 
   def set_recently_used_tags
-    @recently_used_tags = Tag.recently_used(current_account).where.not(id: current_account.featured_tags).limit(10)
+    @recently_used_tags = Tag.recently_used(current_account).where.not(id: featured_tag_ids).limit(10)
+  end
+
+  def featured_tag_ids
+    current_account.featured_tags.pluck(:tag_id)
   end
 end

app/controllers/concerns/cache_concern.rb

@@ -28,29 +28,19 @@ module CacheConcern
     response.headers['Vary'] = public_fetch_mode? ? 'Accept' : 'Accept, Signature'
   end
 
+  # TODO: Rename this method, as it does not perform any caching anymore.
   def cache_collection(raw, klass)
-    return raw unless klass.respond_to?(:with_includes)
+    return raw unless klass.respond_to?(:preload_cacheable_associations)
 
-    raw = raw.cache_ids.to_a if raw.is_a?(ActiveRecord::Relation)
-    return [] if raw.empty?
+    records = raw.to_a
 
-    cached_keys_with_value = Rails.cache.read_multi(*raw).transform_keys(&:id)
-    uncached_ids           = raw.map(&:id) - cached_keys_with_value.keys
+    klass.preload_cacheable_associations(records)
 
-    klass.reload_stale_associations!(cached_keys_with_value.values) if klass.respond_to?(:reload_stale_associations!)
-
-    unless uncached_ids.empty?
-      uncached = klass.where(id: uncached_ids).with_includes.index_by(&:id)
-
-      uncached.each_value do |item|
-        Rails.cache.write(item, item)
-      end
-    end
-
-    raw.filter_map { |item| cached_keys_with_value[item.id] || uncached[item.id] }
+    records
   end
 
+  # TODO: Rename this method, as it does not perform any caching anymore.
   def cache_collection_paginated_by_id(raw, klass, limit, options)
-    cache_collection raw.cache_ids.to_a_paginated_by_id(limit, options), klass
+    cache_collection raw.to_a_paginated_by_id(limit, options), klass
   end
 end

app/controllers/well_known/webfinger_controller.rb

@@ -20,7 +20,7 @@ module WellKnown
     def set_account
       username = username_from_resource
       @account = begin
-        if username == Rails.configuration.x.local_domain
+        if username == Rails.configuration.x.local_domain || username == Rails.configuration.x.web_domain
           Account.representative
         else
           Account.find_local!(username)

app/lib/video_metadata_extractor.rb

@@ -22,7 +22,7 @@ class VideoMetadataExtractor
   private
 
   def ffmpeg_command_output
-    command = Terrapin::CommandLine.new('ffprobe', '-i :path -print_format :format -show_format -show_streams -show_error -loglevel :loglevel')
+    command = Terrapin::CommandLine.new(Rails.configuration.x.ffprobe_binary, '-i :path -print_format :format -show_format -show_streams -show_error -loglevel :loglevel')
     command.run(path: @path, format: 'json', loglevel: 'fatal')
   end
 

app/models/concerns/account_interactions.rb

@@ -187,7 +187,7 @@ module AccountInteractions
   end
 
   def unblock_domain!(other_domain)
-    block = domain_blocks.find_by(domain: other_domain)
+    block = domain_blocks.find_by(domain: normalized_domain(other_domain))
     block&.destroy
   end
 
@@ -299,4 +299,8 @@ module AccountInteractions
   def remove_potential_friendship(other_account)
     PotentialFriendshipTracker.remove(id, other_account.id)
   end
+
+  def normalized_domain(domain)
+    TagManager.instance.normalize_domain(domain)
+  end
 end

app/models/concerns/cacheable.rb

@@ -14,6 +14,10 @@ module Cacheable
       includes(@cache_associated)
     end
 
+    def preload_cacheable_associations(records)
+      ActiveRecord::Associations::Preloader.new(records: records, associations: @cache_associated).call
+    end
+
     def cache_ids
       select(:id, :updated_at)
     end

app/models/feed.rb

@@ -28,7 +28,7 @@ class Feed
       unhydrated = redis.zrangebyscore(key, "(#{min_id}", "(#{max_id}", limit: [0, limit], with_scores: true).map(&:first).map(&:to_i)
     end
 
-    Status.where(id: unhydrated).cache_ids
+    Status.where(id: unhydrated)
   end
 
   def key

app/models/public_feed.rb

@@ -29,7 +29,7 @@ class PublicFeed
     scope.merge!(media_only_scope) if media_only?
     scope.merge!(language_scope) if account&.chosen_languages.present?
 
-    scope.cache_ids.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id)
+    scope.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id)
   end
 
   private

app/models/status.rb

@@ -344,38 +344,6 @@ class Status < ApplicationRecord
       StatusPin.select('status_id').where(status_id: status_ids).where(account_id: account_id).each_with_object({}) { |p, h| h[p.status_id] = true }
     end
 
-    def reload_stale_associations!(cached_items)
-      account_ids = []
-
-      cached_items.each do |item|
-        account_ids << item.account_id
-        account_ids << item.reblog.account_id if item.reblog?
-      end
-
-      account_ids.uniq!
-
-      status_ids = cached_items.map { |item| item.reblog? ? item.reblog_of_id : item.id }.uniq
-
-      return if account_ids.empty?
-
-      accounts = Account.where(id: account_ids).includes(:account_stat, :user).index_by(&:id)
-
-      status_stats = StatusStat.where(status_id: status_ids).index_by(&:status_id)
-
-      cached_items.each do |item|
-        item.account = accounts[item.account_id]
-        item.reblog.account = accounts[item.reblog.account_id] if item.reblog?
-
-        if item.reblog?
-          status_stat = status_stats[item.reblog.id]
-          item.reblog.status_stat = status_stat if status_stat.present?
-        else
-          status_stat = status_stats[item.id]
-          item.status_stat = status_stat if status_stat.present?
-        end
-      end
-    end
-
     def from_text(text)
       return [] if text.blank?
 

app/models/tag_feed.rb

@@ -33,7 +33,7 @@ class TagFeed < PublicFeed
     scope.merge!(account_filters_scope) if account?
     scope.merge!(media_only_scope) if media_only?
 
-    scope.cache_ids.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id)
+    scope.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id)
   end
 
   private

app/models/user.rb

@@ -94,6 +94,9 @@ class User < ApplicationRecord
   validates :invite_request, presence: true, on: :create, if: :invite_text_required?
 
   validates :locale, inclusion: I18n.available_locales.map(&:to_s), if: :locale?
+
+  validates :email, presence: true, email_address: true
+
   validates_with BlacklistedEmailValidator, if: -> { ENV['EMAIL_DOMAIN_LISTS_APPLY_AFTER_CONFIRMATION'] == 'true' || !confirmed? }
   validates_with EmailMxValidator, if: :validate_email_dns?
   validates :agreement, acceptance: { allow_nil: false, accept: [true, 'true', '1'] }, on: :create

app/serializers/rest/admin/domain_block_serializer.rb

@@ -1,11 +1,15 @@
 # frozen_string_literal: true
 
 class REST::Admin::DomainBlockSerializer < ActiveModel::Serializer
-  attributes :id, :domain, :created_at, :severity,
+  attributes :id, :domain, :digest, :created_at, :severity,
              :reject_media, :reject_reports,
              :private_comment, :public_comment, :obfuscate
 
   def id
     object.id.to_s
   end
+
+  def digest
+    object.domain_digest
+  end
 end

app/services/post_status_service.rb

@@ -137,7 +137,7 @@ class PostStatusService < BaseService
 
   def idempotency_duplicate
     if scheduled?
-      @account.schedule_statuses.find(@idempotency_duplicate)
+      @account.scheduled_statuses.find(@idempotency_duplicate)
     else
       @account.statuses.find(@idempotency_duplicate)
     end
@@ -189,7 +189,7 @@ class PostStatusService < BaseService
   end
 
   def scheduled_options
-    @options.tap do |options_hash|
+    @options.dup.tap do |options_hash|
       options_hash[:in_reply_to_id]  = options_hash.delete(:thread)&.id
       options_hash[:application_id]  = options_hash.delete(:application)&.id
       options_hash[:scheduled_at]    = nil

app/validators/email_address_validator.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+# NOTE: I initially wrote this as `EmailValidator` but it ended up clashing
+# with an indirect dependency of ours, `validate_email`, which, turns out,
+# has the same approach as we do, but with an extra check disallowing
+# single-label domains. Decided to not switch to `validate_email` because
+# we do want to allow at least `localhost`.
+
+class EmailAddressValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    value = value.strip
+
+    address = Mail::Address.new(value)
+    record.errors.add(attribute, :invalid) if address.address != value
+  rescue Mail::Field::FieldError
+    record.errors.add(attribute, :invalid)
+  end
+end

config/initializers/ffmpeg.rb

@@ -1,3 +1,6 @@
-if ENV['FFMPEG_BINARY'].present?
-    FFMPEG.ffmpeg_binary = ENV['FFMPEG_BINARY']
+# frozen_string_literal: true
+
+Rails.application.configure do
+  config.x.ffmpeg_binary = ENV['FFMPEG_BINARY'] || 'ffmpeg'
+  config.x.ffprobe_binary = ENV['FFPROBE_BINARY'] || 'ffprobe'
 end

lib/mastodon/maintenance_cli.rb

@@ -234,7 +234,7 @@ module Mastodon
         users = User.where(id: row['ids'].split(',')).sort_by(&:updated_at).reverse
         ref_user = users.shift
         @prompt.warn "Multiple users registered with e-mail address #{ref_user.email}."
-        @prompt.warn "e-mail will be disabled for the following accounts: #{user.map(&:account).map(&:acct).join(', ')}"
+        @prompt.warn "e-mail will be disabled for the following accounts: #{users.map(&:account).map(&:acct).join(', ')}"
         @prompt.warn 'Please reach out to them and set another address with `tootctl account modify` or delete them.'
 
         i = 0

lib/paperclip/image_extractor.rb

@@ -35,7 +35,7 @@ module Paperclip
       dst.binmode
 
       begin
-        command = Terrapin::CommandLine.new('ffmpeg', '-i :source -loglevel :loglevel -y :destination', logger: Paperclip.logger)
+        command = Terrapin::CommandLine.new(Rails.configuration.x.ffmpeg_binary, '-i :source -loglevel :loglevel -y :destination', logger: Paperclip.logger)
         command.run(source: @file.path, destination: dst.path, loglevel: 'fatal')
       rescue Terrapin::ExitStatusError
         dst.close(true)

lib/paperclip/transcoder.rb

@@ -51,7 +51,7 @@ module Paperclip
       command_arguments, interpolations = prepare_command(destination)
 
       begin
-        command = Terrapin::CommandLine.new('ffmpeg', command_arguments.join(' '), logger: Paperclip.logger)
+        command = Terrapin::CommandLine.new(Rails.configuration.x.ffmpeg_binary, command_arguments.join(' '), logger: Paperclip.logger)
         command.run(interpolations)
       rescue Terrapin::ExitStatusError => e
         raise Paperclip::Error, "Error while transcoding #{@basename}: #{e}"

lib/tasks/mastodon.rake

@@ -516,6 +516,7 @@ namespace :mastodon do
           owner_role = UserRole.find_by(name: 'Owner')
           user = User.new(email: email, password: password, confirmed_at: Time.now.utc, account_attributes: { username: username }, bypass_invite_request_check: true, role: owner_role)
           user.save(validate: false)
+          user.approve!
 
           Setting.site_contact_username = username
 

spec/controllers/api/v1/admin/domain_blocks_controller_spec.rb

@@ -163,7 +163,44 @@ RSpec.describe Api::V1::Admin::DomainBlocksController, type: :controller do
       expect(DomainBlock.find_by(domain: 'foo.bar.com')).to_not be_nil
     end
 
-    context 'when a stricter domain block already exists' do
+    context 'when a looser domain block already exists on a higher level domain' do
+      let(:params) { { domain: 'foo.bar.com', severity: :suspend } }
+
+      before do
+        Fabricate(:domain_block, domain: 'bar.com', severity: :silence)
+      end
+
+      it 'creates a domain block with the expected domain name and severity', :aggregate_failures do
+        subject
+
+        body = body_as_json
+
+        expect(response).to have_http_status(200)
+        expect(body).to match a_hash_including(
+          {
+            domain: 'foo.bar.com',
+            severity: 'suspend',
+          }
+        )
+
+        expect(DomainBlock.find_by(domain: 'foo.bar.com')).to be_present
+      end
+    end
+
+    context 'when a domain block already exists on the same domain' do
+      before do
+        Fabricate(:domain_block, domain: 'foo.bar.com', severity: :silence)
+      end
+
+      it 'returns existing domain block in error', :aggregate_failures do
+        subject
+
+        expect(response).to have_http_status(422)
+        expect(body_as_json[:existing_domain_block][:domain]).to eq('foo.bar.com')
+      end
+    end
+
+    context 'when a stricter domain block already exists on a higher level domain' do
       let(:existing_block_domain) { 'bar.com' }
 
       it 'returns http unprocessable entity' do

spec/controllers/api/v1/featured_tags/suggestions_controller_spec.rb

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Api::V1::FeaturedTags::SuggestionsController do
+  render_views
+
+  let(:user)    { Fabricate(:user) }
+  let(:token)   { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:accounts') }
+  let(:account) { Fabricate(:account, user: user) }
+
+  before do
+    allow(controller).to receive(:doorkeeper_token) { token }
+  end
+
+  describe 'GET #index' do
+    let!(:unused_featured_tag) { Fabricate(:tag, name: 'unused_featured_tag') }
+    let!(:used_tag) { Fabricate(:tag, name: 'used_tag') }
+    let!(:used_featured_tag) { Fabricate(:tag, name: 'used_featured_tag') }
+
+    before do
+      _unused_tag = Fabricate(:tag, name: 'unused_tag')
+
+      # Make relevant tags used by account
+      status = Fabricate(:status, account: account)
+      status.tags << used_tag
+      status.tags << used_featured_tag
+
+      # Feature the relevant tags
+      Fabricate :featured_tag, account: account, name: unused_featured_tag.name
+      Fabricate :featured_tag, account: account, name: used_featured_tag.name
+    end
+
+    it 'returns http success and recently used but not featured tags', :aggregate_failures do
+      get :index, params: { account_id: account.id, limit: 2 }
+
+      expect(response)
+        .to have_http_status(200)
+      expect(body_as_json)
+        .to contain_exactly(
+          include(name: used_tag.name)
+        )
+    end
+  end
+end

spec/controllers/application_controller_spec.rb

@@ -242,37 +242,4 @@ describe ApplicationController, type: :controller do
 
     include_examples 'respond_with_error', 422
   end
-
-  describe 'cache_collection' do
-    class C < ApplicationController
-      public :cache_collection
-    end
-
-    shared_examples 'receives :with_includes' do |fabricator, klass|
-      it 'uses raw if it is not an ActiveRecord::Relation' do
-        record = Fabricate(fabricator)
-        expect(C.new.cache_collection([record], klass)).to eq [record]
-      end
-    end
-
-    shared_examples 'cacheable' do |fabricator, klass|
-      include_examples 'receives :with_includes', fabricator, klass
-
-      it 'calls cache_ids of raw if it is an ActiveRecord::Relation' do
-        record = Fabricate(fabricator)
-        relation = klass.none
-        allow(relation).to receive(:cache_ids).and_return([record])
-        expect(C.new.cache_collection(relation, klass)).to eq [record]
-      end
-    end
-
-    it 'returns raw unless class responds to :with_includes' do
-      raw = Object.new
-      expect(C.new.cache_collection(raw, Object)).to eq raw
-    end
-
-    context 'Status' do
-      include_examples 'cacheable', :status, Status
-    end
-  end
 end

spec/models/concerns/account_interactions_spec.rb

@@ -248,6 +248,24 @@ describe AccountInteractions do
     end
   end
 
+  describe '#block_idna_domain!' do
+    subject do
+      [
+        account.block_domain!(idna_domain),
+        account.block_domain!(punycode_domain),
+      ]
+    end
+
+    let(:idna_domain) { '대한민국.한국' }
+    let(:punycode_domain) { 'xn--3e0bs9hfvinn1a.xn--3e0b707e' }
+
+    it 'creates single AccountDomainBlock' do
+      expect do
+        expect(subject).to all(be_a AccountDomainBlock)
+      end.to change { account.domain_blocks.count }.by 1
+    end
+  end
+
   describe '#unfollow!' do
     subject { account.unfollow!(target_account) }
 
@@ -343,6 +361,28 @@ describe AccountInteractions do
     end
   end
 
+  describe '#unblock_idna_domain!' do
+    subject { account.unblock_domain!(punycode_domain) }
+
+    let(:idna_domain) { '대한민국.한국' }
+    let(:punycode_domain) { 'xn--3e0bs9hfvinn1a.xn--3e0b707e' }
+
+    context 'when blocking the domain' do
+      it 'returns destroyed AccountDomainBlock' do
+        account_domain_block = Fabricate(:account_domain_block, domain: idna_domain)
+        account.domain_blocks << account_domain_block
+        expect(subject).to be_a AccountDomainBlock
+        expect(subject).to be_destroyed
+      end
+    end
+
+    context 'when unblocking idna domain' do
+      it 'returns nil' do
+        expect(subject).to be_nil
+      end
+    end
+  end
+
   describe '#following?' do
     subject { account.following?(target_account) }
 

spec/models/home_feed_spec.rb

@@ -25,7 +25,6 @@ RSpec.describe HomeFeed, type: :model do
         results = subject.get(3)
 
         expect(results.map(&:id)).to eq [3, 2]
-        expect(results.first.attributes.keys).to eq %w(id updated_at)
       end
     end
 

spec/models/user_spec.rb

@@ -40,6 +40,12 @@ RSpec.describe User, type: :model do
       expect(user.valid?).to be true
     end
 
+    it 'is valid with a localhost e-mail address' do
+      user = Fabricate.build(:user, email: 'admin@localhost')
+      user.valid?
+      expect(user.valid?).to be true
+    end
+
     it 'cleans out empty string from languages' do
       user = Fabricate.build(:user, chosen_languages: [''])
       user.valid?

spec/services/post_status_service_spec.rb

@@ -52,6 +52,13 @@ RSpec.describe PostStatusService, type: :service do
     it 'does not change statuses count' do
       expect { subject.call(account, text: 'Hi future!', scheduled_at: future, thread: previous_status) }.not_to change { [account.statuses_count, previous_status.replies_count] }
     end
+
+    it 'returns existing status when used twice with idempotency key' do
+      account = Fabricate(:account)
+      status1 = subject.call(account, text: 'test', idempotency: 'meepmeep', scheduled_at: future)
+      status2 = subject.call(account, text: 'test', idempotency: 'meepmeep', scheduled_at: future)
+      expect(status2.id).to eq status1.id
+    end
   end
 
   it 'creates response to the original status of boost' do