From 20c37ed0f988298de75a3c41472222076806d243 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Fri, 5 May 2017 11:26:04 -0400 Subject: [PATCH] Add specs (and refactor) of FetchRemoteResourceService and SearchService (#2812) * Coverage for fetch remote resource service * Refactor fetch remote resource service * Coverage for search service * Refactor search service --- app/services/fetch_remote_resource_service.rb | 37 +++++-- app/services/search_service.rb | 41 ++++--- .../fetch_remote_resource_service_spec.rb | 53 +++++++++ spec/services/search_service_spec.rb | 101 ++++++++++++++++++ 4 files changed, 213 insertions(+), 19 deletions(-) create mode 100644 spec/services/fetch_remote_resource_service_spec.rb create mode 100644 spec/services/search_service_spec.rb diff --git a/app/services/fetch_remote_resource_service.rb b/app/services/fetch_remote_resource_service.rb index 2185ceb20..a0c270673 100644 --- a/app/services/fetch_remote_resource_service.rb +++ b/app/services/fetch_remote_resource_service.rb @@ -1,18 +1,41 @@ # frozen_string_literal: true class FetchRemoteResourceService < BaseService + attr_reader :url + def call(url) - atom_url, body = FetchAtomService.new.call(url) + @url = url + process_url unless atom_url.nil? + end - return nil if atom_url.nil? + private - xml = Nokogiri::XML(body) - xml.encoding = 'utf-8' - - if xml.root.name == 'feed' + def process_url + case xml_root + when 'feed' FetchRemoteAccountService.new.call(atom_url, body) - elsif xml.root.name == 'entry' + when 'entry' FetchRemoteStatusService.new.call(atom_url, body) end end + + def fetched_atom_feed + @_fetched_atom_feed ||= FetchAtomService.new.call(url) + end + + def atom_url + fetched_atom_feed.first + end + + def body + fetched_atom_feed.last + end + + def xml_root + xml_data.root.name + end + + def xml_data + @_xml_data ||= Nokogiri::XML(body, nil, 'utf-8') + end end diff --git a/app/services/search_service.rb b/app/services/search_service.rb index e9745010b..1ed3f0032 100644 --- a/app/services/search_service.rb +++ b/app/services/search_service.rb @@ -1,21 +1,38 @@ # frozen_string_literal: true class SearchService < BaseService + attr_accessor :query + def call(query, limit, resolve = false, account = nil) - results = { accounts: [], hashtags: [], statuses: [] } + @query = query - return results if query.blank? - - if query =~ /\Ahttps?:\/\// - resource = FetchRemoteResourceService.new.call(query) - - results[:accounts] << resource if resource.is_a?(Account) - results[:statuses] << resource if resource.is_a?(Status) - else - results[:accounts] = AccountSearchService.new.call(query, limit, resolve, account) - results[:hashtags] = Tag.search_for(query.gsub(/\A#/, ''), limit) unless query.start_with?('@') + default_results.tap do |results| + if url_query? + results.merge!(remote_resource_results) unless remote_resource.nil? + elsif query.present? + results[:accounts] = AccountSearchService.new.call(query, limit, resolve, account) + results[:hashtags] = Tag.search_for(query.gsub(/\A#/, ''), limit) unless query.start_with?('@') + end end + end - results + def default_results + { accounts: [], hashtags: [], statuses: [] } + end + + def url_query? + query =~ /\Ahttps?:\/\// + end + + def remote_resource_results + { remote_resource_symbol => [remote_resource] } + end + + def remote_resource + @_remote_resource ||= FetchRemoteResourceService.new.call(query) + end + + def remote_resource_symbol + remote_resource.class.name.downcase.pluralize.to_sym end end diff --git a/spec/services/fetch_remote_resource_service_spec.rb b/spec/services/fetch_remote_resource_service_spec.rb new file mode 100644 index 000000000..79834842a --- /dev/null +++ b/spec/services/fetch_remote_resource_service_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe FetchRemoteResourceService do + subject { described_class.new } + + describe '#call' do + it 'returns nil when there is no atom url' do + url = 'http://example.com/missing-atom' + service = double + allow(FetchAtomService).to receive(:new).and_return service + allow(service).to receive(:call).with(url).and_return([nil, 'body']) + + result = subject.call(url) + expect(result).to be_nil + end + + it 'fetches remote accounts for feed types' do + url = 'http://example.com/atom-feed' + service = double + allow(FetchAtomService).to receive(:new).and_return service + feed_url = 'http://feed-url' + feed_content = 'contents' + allow(service).to receive(:call).with(url).and_return([feed_url, feed_content]) + + account_service = double + allow(FetchRemoteAccountService).to receive(:new).and_return(account_service) + allow(account_service).to receive(:call) + + _result = subject.call(url) + + expect(account_service).to have_received(:call).with(feed_url, feed_content) + end + + it 'fetches remote statuses for entry types' do + url = 'http://example.com/atom-entry' + service = double + allow(FetchAtomService).to receive(:new).and_return service + feed_url = 'http://feed-url' + feed_content = 'contents' + allow(service).to receive(:call).with(url).and_return([feed_url, feed_content]) + + account_service = double + allow(FetchRemoteStatusService).to receive(:new).and_return(account_service) + allow(account_service).to receive(:call) + + _result = subject.call(url) + + expect(account_service).to have_received(:call).with(feed_url, feed_content) + end + end +end diff --git a/spec/services/search_service_spec.rb b/spec/services/search_service_spec.rb new file mode 100644 index 000000000..00475c699 --- /dev/null +++ b/spec/services/search_service_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe SearchService do + subject { described_class.new } + + describe '#call' do + describe 'with a blank query' do + it 'returns empty results without searching' do + allow(AccountSearchService).to receive(:new) + allow(Tag).to receive(:search_for) + results = subject.call('', 10) + + expect(results).to eq(empty_results) + expect(AccountSearchService).not_to have_received(:new) + expect(Tag).not_to have_received(:search_for) + end + end + + describe 'with an url query' do + before do + @query = 'http://test.host/query' + end + + context 'that does not find anything' do + it 'returns the empty results' do + service = double(call: nil) + allow(FetchRemoteResourceService).to receive(:new).and_return(service) + results = subject.call(@query, 10) + + expect(service).to have_received(:call).with(@query) + expect(results).to eq empty_results + end + end + + context 'that finds an account' do + it 'includes the account in the results' do + account = Account.new + service = double(call: account) + allow(FetchRemoteResourceService).to receive(:new).and_return(service) + + results = subject.call(@query, 10) + expect(service).to have_received(:call).with(@query) + expect(results).to eq empty_results.merge(accounts: [account]) + end + end + + context 'that finds a status' do + it 'includes the status in the results' do + status = Status.new + service = double(call: status) + allow(FetchRemoteResourceService).to receive(:new).and_return(service) + + results = subject.call(@query, 10) + expect(service).to have_received(:call).with(@query) + expect(results).to eq empty_results.merge(statuses: [status]) + end + end + end + + describe 'with a non-url query' do + context 'that matches an account' do + it 'includes the account in the results' do + query = 'username' + account = Account.new + service = double(call: [account]) + allow(AccountSearchService).to receive(:new).and_return(service) + + results = subject.call(query, 10) + expect(service).to have_received(:call).with(query, 10, false, nil) + expect(results).to eq empty_results.merge(accounts: [account]) + end + end + + context 'that matches a tag' do + it 'includes the tag in the results' do + query = '#tag' + tag = Tag.new + allow(Tag).to receive(:search_for).with('tag', 10).and_return([tag]) + + results = subject.call(query, 10) + expect(Tag).to have_received(:search_for).with('tag', 10) + expect(results).to eq empty_results.merge(hashtags: [tag]) + end + it 'does not include tag when starts with @ character' do + query = '@username' + allow(Tag).to receive(:search_for) + + results = subject.call(query, 10) + expect(Tag).not_to have_received(:search_for) + expect(results).to eq empty_results + end + end + end + end + + def empty_results + { accounts: [], hashtags: [], statuses: [] } + end +end