diff options
Diffstat (limited to 'spec')
| -rw-r--r-- | spec/fabricators/account_note_fabricator.rb | 5 | ||||
| -rw-r--r-- | spec/models/concerns/remotable_spec.rb | 274 | ||||
| -rw-r--r-- | spec/workers/move_worker_spec.rb | 27 |
3 files changed, 158 insertions, 148 deletions
diff --git a/spec/fabricators/account_note_fabricator.rb b/spec/fabricators/account_note_fabricator.rb new file mode 100644 index 000000000..1b061745a --- /dev/null +++ b/spec/fabricators/account_note_fabricator.rb @@ -0,0 +1,5 @@ +Fabricator(:account_note) do + account + target_account { Fabricate(:account) } + comment "User note text" +end diff --git a/spec/models/concerns/remotable_spec.rb b/spec/models/concerns/remotable_spec.rb index 99a60cbf6..2e6c8a9c6 100644 --- a/spec/models/concerns/remotable_spec.rb +++ b/spec/models/concerns/remotable_spec.rb @@ -29,200 +29,178 @@ RSpec.describe Remotable do end end - context 'Remotable module is included' do - before do - class Foo - include Remotable - remotable_attachment :hoge, 1.kilobyte - end - end + before do + class Foo + include Remotable - let(:attribute_name) { "#{hoge}_remote_url".to_sym } - let(:code) { 200 } - let(:file) { 'filename="foo.txt"' } - let(:foo) { Foo.new } - let(:headers) { { 'content-disposition' => file } } - let(:hoge) { :hoge } - let(:url) { 'https://google.com' } - - let(:request) do - stub_request(:get, url) - .to_return(status: code, headers: headers) + remotable_attachment :hoge, 1.kilobyte end + end - it 'defines a method #hoge_remote_url=' do - expect(foo).to respond_to(:hoge_remote_url=) + let(:attribute_name) { "#{hoge}_remote_url".to_sym } + let(:code) { 200 } + let(:file) { 'filename="foo.txt"' } + let(:foo) { Foo.new } + let(:headers) { { 'content-disposition' => file } } + let(:hoge) { :hoge } + let(:url) { 'https://google.com' } + + it 'defines a method #hoge_remote_url=' do + expect(foo).to respond_to(:hoge_remote_url=) + end + + it 'defines a method #reset_hoge!' do + expect(foo).to respond_to(:reset_hoge!) + end + + it 'defines a method #download_hoge!' do + expect(foo).to respond_to(:download_hoge!) + end + + describe '#hoge_remote_url=' do + before do + stub_request(:get, url).to_return(status: code, headers: headers) end - it 'defines a method #reset_hoge!' do - expect(foo).to respond_to(:reset_hoge!) + it 'always returns its argument' do + [nil, '', [], {}].each do |arg| + expect(foo.hoge_remote_url = arg).to be arg + end end - describe '#hoge_remote_url' do + context 'with an invalid URL' do before do - request + allow(Addressable::URI).to receive_message_chain(:parse, :normalize).with(url).with(no_args).and_raise(Addressable::URI::InvalidURIError) end - it 'always returns arg' do - [nil, '', [], {}].each do |arg| - expect(foo.hoge_remote_url = arg).to be arg - end + it 'makes no request' do + foo.hoge_remote_url = url + expect(a_request(:get, url)).to_not have_been_made end + end - context 'Addressable::URI::InvalidURIError raised' do - it 'makes no request' do - allow(Addressable::URI).to receive_message_chain(:parse, :normalize) - .with(url).with(no_args).and_raise(Addressable::URI::InvalidURIError) + context 'with scheme that is neither http nor https' do + let(:url) { 'ftp://google.com' } - foo.hoge_remote_url = url - expect(request).not_to have_been_requested - end + it 'makes no request' do + foo.hoge_remote_url = url + expect(a_request(:get, url)).to_not have_been_made end + end - context 'scheme is neither http nor https' do - let(:url) { 'ftp://google.com' } + context 'with relative URL' do + let(:url) { 'https:///path' } - it 'makes no request' do - foo.hoge_remote_url = url - expect(request).not_to have_been_requested - end + it 'makes no request' do + foo.hoge_remote_url = url + expect(a_request(:get, url)).to_not have_been_made end + end - context 'parsed_url.host is empty' do - it 'makes no request' do - parsed_url = double(scheme: 'https', host: double(blank?: true)) - allow(Addressable::URI).to receive_message_chain(:parse, :normalize) - .with(url).with(no_args).and_return(parsed_url) + context 'when URL has not changed' do + it 'makes no request if file is already saved' do + allow(foo).to receive(:[]).with(attribute_name).and_return(url) + allow(foo).to receive(:hoge_file_name).and_return('foo.jpg') - foo.hoge_remote_url = url - expect(request).not_to have_been_requested - end + foo.hoge_remote_url = url + expect(a_request(:get, url)).to_not have_been_made end - context 'parsed_url.host is nil' do - it 'makes no request' do - parsed_url = Addressable::URI.parse('https:https://example.com/path/file.png') - allow(Addressable::URI).to receive_message_chain(:parse, :normalize) - .with(url).with(no_args).and_return(parsed_url) + it 'makes request if file is not already saved' do + allow(foo).to receive(:[]).with(attribute_name).and_return(url) + allow(foo).to receive(:hoge_file_name).and_return(nil) - foo.hoge_remote_url = url - expect(request).not_to have_been_requested - end + foo.hoge_remote_url = url + expect(a_request(:get, url)).to have_been_made end + end - context 'foo[attribute_name] == url' do - it 'makes no request if file is saved' do - allow(foo).to receive(:[]).with(attribute_name).and_return(url) - allow(foo).to receive(:hoge_file_name).and_return('foo.jpg') - - foo.hoge_remote_url = url - expect(request).not_to have_been_requested - end - - it 'makes request if file is not saved' do - allow(foo).to receive(:[]).with(attribute_name).and_return(url) - allow(foo).to receive(:hoge_file_name).and_return(nil) + context 'when instance has no attribute for URL' do + before do + allow(foo).to receive(:has_attribute?).with(attribute_name).and_return(false) + end - foo.hoge_remote_url = url - expect(request).to have_been_requested - end + it 'does not try to write attribute' do + expect(foo).to_not receive('[]=').with(attribute_name, url) + foo.hoge_remote_url = url end + end - context "scheme is https, parsed_url.host isn't empty, and foo[attribute_name] != url" do - it 'makes a request' do - foo.hoge_remote_url = url - expect(request).to have_been_requested - end + context 'when instance has an attribute for URL' do + before do + allow(foo).to receive(:has_attribute?).with(attribute_name).and_return(true) + end - context 'response.code != 200' do - let(:code) { 500 } + it 'does not try to write attribute' do + expect(foo).to receive('[]=').with(attribute_name, url) + foo.hoge_remote_url = url + end + end - it 'calls not send' do - expect(foo).not_to receive(:send).with("#{hoge}=", any_args) - expect(foo).not_to receive(:send).with("#{hoge}_file_name=", any_args) - foo.hoge_remote_url = url - end - end + context 'with a valid URL' do + it 'makes a request' do + foo.hoge_remote_url = url + expect(a_request(:get, url)).to have_been_made + end - context 'response.code == 200' do - let(:code) { 200 } + context 'when the response is not successful' do + let(:code) { 500 } - context 'response contains headers["content-disposition"]' do - let(:file) { 'filename="foo.txt"' } - let(:headers) { { 'content-disposition' => file } } + it 'does not assign file' do + expect(foo).not_to receive(:public_send).with("#{hoge}=", any_args) + expect(foo).not_to receive(:public_send).with("#{hoge}_file_name=", any_args) - it 'calls send' do - string_io = StringIO.new('') - extname = '.txt' - basename = '0123456789abcdef' + foo.hoge_remote_url = url + end + end - allow(SecureRandom).to receive(:hex).and_return(basename) - allow(StringIO).to receive(:new).with(anything).and_return(string_io) + context 'when the response is successful' do + let(:code) { 200 } - expect(foo).to receive(:send).with("#{hoge}=", string_io) - expect(foo).to receive(:send).with("#{hoge}_file_name=", basename + extname) - foo.hoge_remote_url = url - end - end + context 'and contains Content-Disposition header' do + let(:file) { 'filename="foo.txt"' } + let(:headers) { { 'content-disposition' => file } } - context 'if has_attribute?' do - it 'calls foo[attribute_name] = url' do - allow(foo).to receive(:has_attribute?).with(attribute_name).and_return(true) - expect(foo).to receive('[]=').with(attribute_name, url) - foo.hoge_remote_url = url - end - end + it 'assigns file' do + string_io = StringIO.new('') + extname = '.txt' + basename = '0123456789abcdef' - context 'unless has_attribute?' do - it 'calls not foo[attribute_name] = url' do - allow(foo).to receive(:has_attribute?) - .with(attribute_name).and_return(false) - expect(foo).not_to receive('[]=').with(attribute_name, url) - foo.hoge_remote_url = url - end - end - end + allow(SecureRandom).to receive(:hex).and_return(basename) + allow(StringIO).to receive(:new).with(anything).and_return(string_io) - context 'an error raised during the request' do - let(:request) { stub_request(:get, url).to_raise(error_class) } + expect(foo).to receive(:public_send).with("download_#{hoge}!", url) - error_classes = [ - HTTP::TimeoutError, - HTTP::ConnectionError, - OpenSSL::SSL::SSLError, - Paperclip::Errors::NotIdentifiedByImageMagickError, - Addressable::URI::InvalidURIError, - ] + foo.hoge_remote_url = url - error_classes.each do |error_class| - let(:error_class) { error_class } + expect(foo).to receive(:public_send).with("#{hoge}=", string_io) + expect(foo).to receive(:public_send).with("#{hoge}_file_name=", basename + extname) - it 'calls Rails.logger.debug' do - expect(Rails.logger).to receive(:debug).with(/^Error fetching remote #{hoge}: /) - foo.hoge_remote_url = url - end + foo.download_hoge!(url) end end end - end - describe '#reset_hoge!' do - context 'if url.blank?' do - it 'returns nil, without clearing foo[attribute_name] and calling #hoge_remote_url=' do - url = nil - expect(foo).not_to receive(:send).with(:hoge_remote_url=, url) - foo[attribute_name] = url - expect(foo.reset_hoge!).to be_nil - expect(foo[attribute_name]).to be_nil + context 'when an error is raised during the request' do + before do + stub_request(:get, url).to_raise(error_class) end - end - context 'unless url.blank?' do - it 'clears foo[attribute_name] and calls #hoge_remote_url=' do - foo[attribute_name] = url - expect(foo).to receive(:send).with(:hoge_remote_url=, url) - foo.reset_hoge! - expect(foo[attribute_name]).to be '' + error_classes = [ + HTTP::TimeoutError, + HTTP::ConnectionError, + OpenSSL::SSL::SSLError, + Paperclip::Errors::NotIdentifiedByImageMagickError, + Addressable::URI::InvalidURIError, + ] + + error_classes.each do |error_class| + let(:error_class) { error_class } + + it 'calls Rails.logger.debug' do + expect(Rails.logger).to receive(:debug).with(/^Error fetching remote #{hoge}: /) + foo.hoge_remote_url = url + end end end end diff --git a/spec/workers/move_worker_spec.rb b/spec/workers/move_worker_spec.rb index b8f4d9900..77b921eaf 100644 --- a/spec/workers/move_worker_spec.rb +++ b/spec/workers/move_worker_spec.rb @@ -6,6 +6,8 @@ describe MoveWorker do let(:local_follower) { Fabricate(:user, email: 'bob@example.com', account: Fabricate(:account, username: 'bob')).account } let(:source_account) { Fabricate(:account, protocol: :activitypub, domain: 'example.com') } let(:target_account) { Fabricate(:account, protocol: :activitypub, domain: 'example.com') } + let(:local_user) { Fabricate(:user) } + let!(:account_note) { Fabricate(:account_note, account: local_user.account, target_account: source_account) } subject { described_class.new } @@ -13,6 +15,25 @@ describe MoveWorker do local_follower.follow!(source_account) end + shared_examples 'user note handling' do + it 'copies user note' do + allow(UnfollowFollowWorker).to receive(:push_bulk) + subject.perform(source_account.id, target_account.id) + expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(source_account.acct) + expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(account_note.comment) + end + + it 'merges user notes when needed' do + new_account_note = AccountNote.create!(account: account_note.account, target_account: target_account, comment: 'new note prior to move') + + allow(UnfollowFollowWorker).to receive(:push_bulk) + subject.perform(source_account.id, target_account.id) + expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(source_account.acct) + expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(account_note.comment) + expect(AccountNote.find_by(account: account_note.account, target_account: target_account).comment).to include(new_account_note.comment) + end + end + context 'both accounts are distant' do describe 'perform' do it 'calls UnfollowFollowWorker' do @@ -20,6 +41,8 @@ describe MoveWorker do subject.perform(source_account.id, target_account.id) expect(UnfollowFollowWorker).to have_received(:push_bulk).with([local_follower.id]) end + + include_examples 'user note handling' end end @@ -32,6 +55,8 @@ describe MoveWorker do subject.perform(source_account.id, target_account.id) expect(UnfollowFollowWorker).to have_received(:push_bulk).with([local_follower.id]) end + + include_examples 'user note handling' end end @@ -45,6 +70,8 @@ describe MoveWorker do expect(local_follower.following?(target_account)).to be true end + include_examples 'user note handling' + it 'does not fail when a local user is already following both accounts' do double_follower = Fabricate(:user, email: 'eve@example.com', account: Fabricate(:account, username: 'eve')).account double_follower.follow!(source_account) |