From 06123147d51b64ab5ae09dcc7d69f20de414cb69 Mon Sep 17 00:00:00 2001 From: KMY Date: Thu, 4 Jan 2024 15:32:58 +0900 Subject: [PATCH] =?UTF-8?q?Fix:=20=E3=82=A2=E3=83=B3=E3=83=86=E3=83=8A?= =?UTF-8?q?=E3=81=AB=E7=99=BB=E9=8C=B2=E3=81=95=E3=82=8C=E3=81=9F=E6=8A=95?= =?UTF-8?q?=E7=A8=BF=E3=81=8C=E3=82=A2=E3=83=B3=E3=83=86=E3=83=8A=E5=89=8A?= =?UTF-8?q?=E9=99=A4=E6=99=82Redis=E3=81=8B=E3=82=89=E5=89=8A=E9=99=A4?= =?UTF-8?q?=E3=81=95=E3=82=8C=E3=81=AA=E3=81=84=E5=95=8F=E9=A1=8C=20(#417)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix: アンテナに登録された投稿がRedisから削除されない問題 * Fix test * Tootctlに変更 * 処理を共通化 --- app/lib/vacuum/feeds_vacuum.rb | 11 ++ app/models/antenna.rb | 8 + lib/mastodon/cli/feeds.rb | 25 +++ spec/lib/vacuum/feeds_vacuum_spec.rb | 14 ++ spec/requests/api/v1/antennas_spec.rb | 234 ++++++++++++++++++++++++++ 5 files changed, 292 insertions(+) create mode 100644 spec/requests/api/v1/antennas_spec.rb diff --git a/app/lib/vacuum/feeds_vacuum.rb b/app/lib/vacuum/feeds_vacuum.rb index fb0b8a8472..a9e96accdd 100644 --- a/app/lib/vacuum/feeds_vacuum.rb +++ b/app/lib/vacuum/feeds_vacuum.rb @@ -4,6 +4,7 @@ class Vacuum::FeedsVacuum def perform vacuum_inactive_home_feeds! vacuum_inactive_list_feeds! + vacuum_inactive_antenna_feeds! end private @@ -20,6 +21,12 @@ class Vacuum::FeedsVacuum end end + def vacuum_inactive_antenna_feeds! + inactive_users_antennas.select(:id).in_batches do |antennas| + feed_manager.clean_feeds!(:antenna, antennas.ids) + end + end + def inactive_users User.confirmed.inactive end @@ -28,6 +35,10 @@ class Vacuum::FeedsVacuum List.where(account_id: inactive_users.select(:account_id)) end + def inactive_users_antennas + Antenna.where(account_id: inactive_users.select(:account_id)) + end + def feed_manager FeedManager.instance end diff --git a/app/models/antenna.rb b/app/models/antenna.rb index a35fa6dd17..c64e66bde2 100644 --- a/app/models/antenna.rb +++ b/app/models/antenna.rb @@ -55,11 +55,15 @@ class Antenna < ApplicationRecord scope :available_stls, -> { where(available: true, stl: true) } scope :available_ltls, -> { where(available: true, stl: false, ltl: true) } + validates :title, presence: true + validate :list_owner validate :validate_limit validate :validate_stl_limit validate :validate_ltl_limit + before_destroy :clean_feed_manager + def list_owner raise Mastodon::ValidationError, I18n.t('antennas.errors.invalid_list_owner') if !list_id.zero? && list.present? && list.account != account end @@ -121,4 +125,8 @@ class Antenna < ApplicationRecord ltls.any? { |tl| !tl.insert_feeds } end end + + def clean_feed_manager + FeedManager.instance.clean_feeds!(:antenna, [id]) + end end diff --git a/lib/mastodon/cli/feeds.rb b/lib/mastodon/cli/feeds.rb index 3467dd427c..4f2de4ebb4 100644 --- a/lib/mastodon/cli/feeds.rb +++ b/lib/mastodon/cli/feeds.rb @@ -48,10 +48,35 @@ module Mastodon::CLI say('OK', :green) end + desc 'remove_legacy', 'Remove old list and antenna feeds from Redis' + def remove_legacy + current_id = 1 + List.reorder(:id).select(:id).find_in_batches do |lists| + current_id = remove_legacy_feeds(:list, lists, current_id) + end + + current_id = 1 + Antenna.reorder(:id).select(:id).find_in_batches do |antennas| + current_id = remove_legacy_feeds(:antenna, antennas, current_id) + end + + say('OK', :green) + end + private def active_user_accounts Account.joins(:user).merge(User.active) end + + def remove_legacy_feeds(type, items, current_id) + exist_ids = items.pluck(:id) + last_id = exist_ids.max + + ids = Range.new(current_id, last_id).to_a - exist_ids + FeedManager.instance.clean_feeds!(type, ids) + + last_id + 1 + end end end diff --git a/spec/lib/vacuum/feeds_vacuum_spec.rb b/spec/lib/vacuum/feeds_vacuum_spec.rb index ede1e3c360..fa5381c8ae 100644 --- a/spec/lib/vacuum/feeds_vacuum_spec.rb +++ b/spec/lib/vacuum/feeds_vacuum_spec.rb @@ -8,12 +8,16 @@ RSpec.describe Vacuum::FeedsVacuum do describe '#perform' do let!(:active_user) { Fabricate(:user, current_sign_in_at: 2.days.ago) } let!(:inactive_user) { Fabricate(:user, current_sign_in_at: 22.days.ago) } + let!(:list) { Fabricate(:list, account: inactive_user.account) } + let!(:antenna) { Fabricate(:antenna, account: inactive_user.account) } before do redis.zadd(feed_key_for(inactive_user), 1, 1) redis.zadd(feed_key_for(active_user), 1, 1) redis.zadd(feed_key_for(inactive_user, 'reblogs'), 2, 2) redis.sadd(feed_key_for(inactive_user, 'reblogs:2'), 3) + redis.zadd(list_key_for(list), 1, 1) + redis.zadd(antenna_key_for(antenna), 1, 1) subject.perform end @@ -23,10 +27,20 @@ RSpec.describe Vacuum::FeedsVacuum do expect(redis.zcard(feed_key_for(active_user))).to eq 1 expect(redis.exists?(feed_key_for(inactive_user, 'reblogs'))).to be false expect(redis.exists?(feed_key_for(inactive_user, 'reblogs:2'))).to be false + expect(redis.zcard(list_key_for(list))).to eq 0 + expect(redis.zcard(antenna_key_for(antenna))).to eq 0 end end def feed_key_for(user, subtype = nil) FeedManager.instance.key(:home, user.account_id, subtype) end + + def list_key_for(list) + FeedManager.instance.key(:list, list.id) + end + + def antenna_key_for(antenna) + FeedManager.instance.key(:antenna, antenna.id) + end end diff --git a/spec/requests/api/v1/antennas_spec.rb b/spec/requests/api/v1/antennas_spec.rb new file mode 100644 index 0000000000..1597bbd81f --- /dev/null +++ b/spec/requests/api/v1/antennas_spec.rb @@ -0,0 +1,234 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Antennas' do + let(:user) { Fabricate(:user) } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: scopes) } + let(:scopes) { 'read:lists write:lists' } + let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } + + describe 'GET /api/v1/antennas' do + subject do + get '/api/v1/antennas', headers: headers + end + + let!(:antennas) do + [ + Fabricate(:antenna, account: user.account, title: 'first antenna'), + Fabricate(:antenna, account: user.account, title: 'second antenna', with_media_only: true), + Fabricate(:antenna, account: user.account, title: 'third antenna', stl: true), + Fabricate(:antenna, account: user.account, title: 'fourth antenna', ignore_reblog: true), + ] + end + + let(:expected_response) do + antennas.map do |antenna| + { + id: antenna.id.to_s, + title: antenna.title, + with_media_only: antenna.with_media_only, + ignore_reblog: antenna.ignore_reblog, + stl: antenna.stl, + ltl: antenna.ltl, + insert_feeds: antenna.insert_feeds, + list: nil, + accounts_count: 0, + domains_count: 0, + tags_count: 0, + keywords_count: 0, + } + end + end + + before do + Fabricate(:antenna) + end + + it_behaves_like 'forbidden for wrong scope', 'write write:lists' + + it 'returns the expected antennas', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(body_as_json).to match_array(expected_response) + end + end + + describe 'GET /api/v1/antennas/:id' do + subject do + get "/api/v1/antennas/#{antenna.id}", headers: headers + end + + let(:antenna) { Fabricate(:antenna, account: user.account) } + + it_behaves_like 'forbidden for wrong scope', 'write write:lists' + + it 'returns the requested antenna correctly', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(body_as_json).to eq({ + id: antenna.id.to_s, + title: antenna.title, + with_media_only: antenna.with_media_only, + ignore_reblog: antenna.ignore_reblog, + stl: antenna.stl, + ltl: antenna.ltl, + insert_feeds: antenna.insert_feeds, + list: nil, + accounts_count: 0, + domains_count: 0, + tags_count: 0, + keywords_count: 0, + }) + end + + context 'when the antenna belongs to a different user' do + let(:antenna) { Fabricate(:antenna) } + + it 'returns http not found' do + subject + + expect(response).to have_http_status(404) + end + end + + context 'when the antenna does not exist' do + it 'returns http not found' do + get '/api/v1/antennas/-1', headers: headers + + expect(response).to have_http_status(404) + end + end + end + + describe 'POST /api/v1/antennas' do + subject do + post '/api/v1/antennas', headers: headers, params: params + end + + let(:params) { { title: 'my antenna', ltl: 'true' } } + + it_behaves_like 'forbidden for wrong scope', 'read read:lists' + + it 'returns the new antenna', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(body_as_json).to match(a_hash_including(title: 'my antenna', ltl: true)) + expect(Antenna.where(account: user.account).count).to eq(1) + end + + context 'when a title is not given' do + let(:params) { { title: '' } } + + it 'returns http unprocessable entity' do + subject + + expect(response).to have_http_status(422) + end + end + end + + describe 'PUT /api/v1/antennas/:id' do + subject do + put "/api/v1/antennas/#{antenna.id}", headers: headers, params: params + end + + let(:antenna) { Fabricate(:antenna, account: user.account, title: 'my antenna') } + let(:params) { { title: 'antenna', ignore_reblog: 'true', insert_feeds: 'true' } } + + it_behaves_like 'forbidden for wrong scope', 'read read:lists' + + it 'returns the updated antenna and updates values', :aggregate_failures do + expect { subject } + .to change_antenna_title + .and change_antenna_ignore_reblog + .and change_antenna_insert_feeds + + expect(response).to have_http_status(200) + antenna.reload + + expect(body_as_json).to eq({ + id: antenna.id.to_s, + title: antenna.title, + with_media_only: antenna.with_media_only, + ignore_reblog: antenna.ignore_reblog, + stl: antenna.stl, + ltl: antenna.ltl, + insert_feeds: antenna.insert_feeds, + list: nil, + accounts_count: 0, + domains_count: 0, + tags_count: 0, + keywords_count: 0, + }) + end + + def change_antenna_title + change { antenna.reload.title }.from('my antenna').to('antenna') + end + + def change_antenna_ignore_reblog + change { antenna.reload.ignore_reblog }.from(false).to(true) + end + + def change_antenna_insert_feeds + change { antenna.reload.insert_feeds }.from(false).to(true) + end + + context 'when the antenna does not exist' do + it 'returns http not found' do + put '/api/v1/antennas/-1', headers: headers, params: params + + expect(response).to have_http_status(404) + end + end + + context 'when the antenna belongs to another user' do + let(:antenna) { Fabricate(:antenna) } + + it 'returns http not found' do + subject + + expect(response).to have_http_status(404) + end + end + end + + describe 'DELETE /api/v1/antennas/:id' do + subject do + delete "/api/v1/antennas/#{antenna.id}", headers: headers + end + + let(:antenna) { Fabricate(:antenna, account: user.account) } + + it_behaves_like 'forbidden for wrong scope', 'read read:lists' + + it 'deletes the antenna', :aggregate_failures do + subject + + expect(response).to have_http_status(200) + expect(Antenna.where(id: antenna.id)).to_not exist + end + + context 'when the antenna does not exist' do + it 'returns http not found' do + delete '/api/v1/antennas/-1', headers: headers + + expect(response).to have_http_status(404) + end + end + + context 'when the antenna belongs to another user' do + let(:antenna) { Fabricate(:antenna) } + + it 'returns http not found' do + subject + + expect(response).to have_http_status(404) + end + end + end +end