From 5c42f47617d311219d06e082e4daa41e671903c8 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 1 Oct 2019 01:19:11 +0200 Subject: Fix records not being indexed sometimes (#12024) It's possible that after commit callbacks were not firing when exceptions occurred in the process. Also, the default Sidekiq strategy does not push indexing jobs immediately, which is not necessary and could be part of the issue too. --- spec/rails_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 3a5e7491e..6fbceca53 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -12,7 +12,7 @@ require 'capybara/rspec' Dir[Rails.root.join('spec/support/**/*.rb')].each { |f| require f } ActiveRecord::Migration.maintain_test_schema! -WebMock.disable_net_connect! +WebMock.disable_net_connect!(allow: Chewy.settings[:host]) Redis.current = Redis::Namespace.new("mastodon_test#{ENV['TEST_ENV_NUMBER']}", redis: Redis.current) Sidekiq::Testing.inline! Sidekiq::Logging.logger = nil -- cgit From 9ba40a6bfdb65f6e48eb7de07b2d55314c54fa83 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 1 Oct 2019 04:54:10 +0200 Subject: Remove HEAD request from fetching link previews (#12028) It is not really necessary and we need to reduce requests --- app/services/fetch_link_card_service.rb | 6 ------ spec/services/fetch_link_card_service_spec.rb | 13 +++---------- 2 files changed, 3 insertions(+), 16 deletions(-) (limited to 'spec') diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb index ac5503d46..f0b1169db 100644 --- a/app/services/fetch_link_card_service.rb +++ b/app/services/fetch_link_card_service.rb @@ -39,12 +39,6 @@ class FetchLinkCardService < BaseService def process_url @card ||= PreviewCard.new(url: @url) - failed = Request.new(:head, @url).perform do |res| - res.code != 405 && res.code != 501 && (res.code != 200 || res.mime_type != 'text/html') - end - - return if failed - Request.new(:get, @url).perform do |res| if res.code == 200 && res.mime_type == 'text/html' @html = res.body_with_limit diff --git a/spec/services/fetch_link_card_service_spec.rb b/spec/services/fetch_link_card_service_spec.rb index 50c60aafd..9761c5f06 100644 --- a/spec/services/fetch_link_card_service_spec.rb +++ b/spec/services/fetch_link_card_service_spec.rb @@ -4,20 +4,13 @@ RSpec.describe FetchLinkCardService, type: :service do subject { FetchLinkCardService.new } before do - stub_request(:head, 'http://example.xn--fiqs8s/').to_return(status: 200, headers: { 'Content-Type' => 'text/html' }) stub_request(:get, 'http://example.xn--fiqs8s/').to_return(request_fixture('idn.txt')) - stub_request(:head, 'http://example.com/sjis').to_return(status: 200, headers: { 'Content-Type' => 'text/html' }) stub_request(:get, 'http://example.com/sjis').to_return(request_fixture('sjis.txt')) - stub_request(:head, 'http://example.com/sjis_with_wrong_charset').to_return(status: 200, headers: { 'Content-Type' => 'text/html' }) stub_request(:get, 'http://example.com/sjis_with_wrong_charset').to_return(request_fixture('sjis_with_wrong_charset.txt')) - stub_request(:head, 'http://example.com/koi8-r').to_return(status: 200, headers: { 'Content-Type' => 'text/html' }) stub_request(:get, 'http://example.com/koi8-r').to_return(request_fixture('koi8-r.txt')) - stub_request(:head, 'http://example.com/日本語').to_return(status: 200, headers: { 'Content-Type' => 'text/html' }) stub_request(:get, 'http://example.com/日本語').to_return(request_fixture('sjis.txt')) - stub_request(:head, 'https://github.com/qbi/WannaCry').to_return(status: 404) - stub_request(:head, 'http://example.com/test-').to_return(status: 200, headers: { 'Content-Type' => 'text/html' }) + stub_request(:get, 'https://github.com/qbi/WannaCry').to_return(status: 404) stub_request(:get, 'http://example.com/test-').to_return(request_fixture('idn.txt')) - stub_request(:head, 'http://example.com/windows-1251').to_return(status: 200, headers: { 'Content-Type' => 'text/html' }) stub_request(:get, 'http://example.com/windows-1251').to_return(request_fixture('windows-1251.txt')) subject.call(status) @@ -90,11 +83,11 @@ RSpec.describe FetchLinkCardService, type: :service do let(:status) { Fabricate(:status, account: Fabricate(:account, domain: 'example.com'), text: 'Habt ihr ein paar gute Links zu #Wannacry herumfliegen? Ich will mal unter
https://github.com/qbi/WannaCry was sammeln. !security ') } it 'parses out URLs' do - expect(a_request(:head, 'https://github.com/qbi/WannaCry')).to have_been_made.at_least_once + expect(a_request(:get, 'https://github.com/qbi/WannaCry')).to have_been_made.at_least_once end it 'ignores URLs to hashtags' do - expect(a_request(:head, 'https://quitter.se/tag/wannacry')).to_not have_been_made + expect(a_request(:get, 'https://quitter.se/tag/wannacry')).to_not have_been_made end end end -- cgit From 3a4d994c40962a9abe45565a34e3d7a3ca1ccd49 Mon Sep 17 00:00:00 2001 From: ThibG Date: Tue, 1 Oct 2019 15:10:00 +0200 Subject: Fix BootstrapTimelineService crashing when bootstrapped accounts are invalid (#12037) * Add test to handle suspended and missing users in BootstrapTimelineService * Fix BootstrapTimelineService crashing when bootstrapped accounts are invalid --- app/services/bootstrap_timeline_service.rb | 8 +++++++- spec/services/bootstrap_timeline_service_spec.rb | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-) (limited to 'spec') diff --git a/app/services/bootstrap_timeline_service.rb b/app/services/bootstrap_timeline_service.rb index db2c83e5d..c489601c1 100644 --- a/app/services/bootstrap_timeline_service.rb +++ b/app/services/bootstrap_timeline_service.rb @@ -17,7 +17,11 @@ class BootstrapTimelineService < BaseService def autofollow_bootstrap_timeline_accounts! bootstrap_timeline_accounts.each do |target_account| - FollowService.new.call(@source_account, target_account) + begin + FollowService.new.call(@source_account, target_account) + rescue ActiveRecord::RecordNotFound, Mastodon::NotPermittedError + nil + end end end @@ -40,7 +44,9 @@ class BootstrapTimelineService < BaseService def local_unlocked_accounts(usernames) Account.local + .without_suspended .where(username: usernames) .where(locked: false) + .where(moved_to_account_id: nil) end end diff --git a/spec/services/bootstrap_timeline_service_spec.rb b/spec/services/bootstrap_timeline_service_spec.rb index a765de791..a28d2407c 100644 --- a/spec/services/bootstrap_timeline_service_spec.rb +++ b/spec/services/bootstrap_timeline_service_spec.rb @@ -22,9 +22,10 @@ RSpec.describe BootstrapTimelineService, type: :service do context 'when setting is set' do let!(:alice) { Fabricate(:account, username: 'alice') } let!(:bob) { Fabricate(:account, username: 'bob') } + let!(:eve) { Fabricate(:account, username: 'eve', suspended: true) } before do - Setting.bootstrap_timeline_accounts = 'alice, bob' + Setting.bootstrap_timeline_accounts = 'alice, @bob, eve, unknown' subject.call(source_account) end @@ -32,6 +33,10 @@ RSpec.describe BootstrapTimelineService, type: :service do expect(source_account.following?(alice)).to be true expect(source_account.following?(bob)).to be true end + + it 'does not follow suspended account' do + expect(source_account.following?(eve)).to be false + end end end end -- cgit From 62f60e86c281ae1cd0356a7630dbb76069e2ffde Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 2 Oct 2019 04:59:37 +0200 Subject: Fix account counters being overwritten by parallel writes (#12045) --- app/models/account_stat.rb | 22 +++++++++ ...1001213028_add_lock_version_to_account_stats.rb | 15 ++++++ db/schema.rb | 3 +- spec/models/account_stat_spec.rb | 53 ++++++++++++++++++++++ 4 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20191001213028_add_lock_version_to_account_stats.rb (limited to 'spec') diff --git a/app/models/account_stat.rb b/app/models/account_stat.rb index 1351f7d8a..c84e4217c 100644 --- a/app/models/account_stat.rb +++ b/app/models/account_stat.rb @@ -11,6 +11,7 @@ # created_at :datetime not null # updated_at :datetime not null # last_status_at :datetime +# lock_version :integer default(0), not null # class AccountStat < ApplicationRecord @@ -20,10 +21,26 @@ class AccountStat < ApplicationRecord def increment_count!(key) update(attributes_for_increment(key)) + rescue ActiveRecord::StaleObjectError + begin + reload_with_id + rescue ActiveRecord::RecordNotFound + # Nothing to do + else + retry + end end def decrement_count!(key) update(key => [public_send(key) - 1, 0].max) + rescue ActiveRecord::StaleObjectError + begin + reload_with_id + rescue ActiveRecord::RecordNotFound + # Nothing to do + else + retry + end end private @@ -33,4 +50,9 @@ class AccountStat < ApplicationRecord attrs[:last_status_at] = Time.now.utc if key == :statuses_count attrs end + + def reload_with_id + self.id = find_by!(account: account).id if new_record? + reload + end end diff --git a/db/migrate/20191001213028_add_lock_version_to_account_stats.rb b/db/migrate/20191001213028_add_lock_version_to_account_stats.rb new file mode 100644 index 000000000..47f37cca2 --- /dev/null +++ b/db/migrate/20191001213028_add_lock_version_to_account_stats.rb @@ -0,0 +1,15 @@ +require Rails.root.join('lib', 'mastodon', 'migration_helpers') + +class AddLockVersionToAccountStats < ActiveRecord::Migration[5.2] + include Mastodon::MigrationHelpers + + disable_ddl_transaction! + + def up + safety_assured { add_column_with_default :account_stats, :lock_version, :integer, allow_null: false, default: 0 } + end + + def down + remove_column :account_stats, :lock_version + end +end diff --git a/db/schema.rb b/db/schema.rb index 557b777e0..2d516965c 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: 2019_09_27_232842) do +ActiveRecord::Schema.define(version: 2019_10_01_213028) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -97,6 +97,7 @@ ActiveRecord::Schema.define(version: 2019_09_27_232842) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.datetime "last_status_at" + t.integer "lock_version", default: 0, null: false t.index ["account_id"], name: "index_account_stats_on_account_id", unique: true end diff --git a/spec/models/account_stat_spec.rb b/spec/models/account_stat_spec.rb index a94185109..8adc0d1d6 100644 --- a/spec/models/account_stat_spec.rb +++ b/spec/models/account_stat_spec.rb @@ -1,4 +1,57 @@ require 'rails_helper' RSpec.describe AccountStat, type: :model do + describe '#increment_count!' do + it 'increments the count' do + account_stat = AccountStat.create(account: Fabricate(:account)) + expect(account_stat.followers_count).to eq 0 + account_stat.increment_count!(:followers_count) + expect(account_stat.followers_count).to eq 1 + end + + it 'increments the count in multi-threaded an environment' do + account_stat = AccountStat.create(account: Fabricate(:account), statuses_count: 0) + increment_by = 15 + wait_for_start = true + + threads = Array.new(increment_by) do + Thread.new do + true while wait_for_start + AccountStat.find(account_stat.id).increment_count!(:statuses_count) + end + end + + wait_for_start = false + threads.each(&:join) + + expect(account_stat.reload.statuses_count).to eq increment_by + end + end + + describe '#decrement_count!' do + it 'decrements the count' do + account_stat = AccountStat.create(account: Fabricate(:account), followers_count: 15) + expect(account_stat.followers_count).to eq 15 + account_stat.decrement_count!(:followers_count) + expect(account_stat.followers_count).to eq 14 + end + + it 'decrements the count in multi-threaded an environment' do + account_stat = AccountStat.create(account: Fabricate(:account), statuses_count: 15) + decrement_by = 10 + wait_for_start = true + + threads = Array.new(decrement_by) do + Thread.new do + true while wait_for_start + AccountStat.find(account_stat.id).decrement_count!(:statuses_count) + end + end + + wait_for_start = false + threads.each(&:join) + + expect(account_stat.reload.statuses_count).to eq 5 + end + end end -- cgit