Skip to content
This repository has been archived by the owner on Aug 19, 2024. It is now read-only.

[TestProf] Optimize: spec/services/fetch_link_card_service_spec.rb #2

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 60 additions & 66 deletions spec/services/fetch_link_card_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,46 +2,43 @@

require 'rails_helper'

RSpec.describe FetchLinkCardService do
RSpec.describe FetchLinkCardService, :account do
subject { described_class.new }

let(:html) { '<!doctype html><title>Hello world</title>' }
let(:oembed_cache) { nil }
let_it_be(:html) { '<!doctype html><title>Hello world</title>' }
let_it_be(:oembed_cache) { nil }

before do
stub_request(:get, 'http://example.com/html').to_return(headers: { 'Content-Type' => 'text/html' }, body: html)
stub_request(:get, 'http://example.com/not-found').to_return(status: 404, headers: { 'Content-Type' => 'text/html' }, body: html)
stub_request(:get, 'http://example.com/text').to_return(status: 404, headers: { 'Content-Type' => 'text/plain' }, body: 'Hello')
stub_request(:get, 'http://example.com/redirect').to_return(status: 302, headers: { 'Location' => 'http://example.com/html' })
stub_request(:get, 'http://example.com/redirect-to-404').to_return(status: 302, headers: { 'Location' => 'http://example.com/not-found' })
stub_request(:get, 'http://example.com/oembed?url=http://example.com/html').to_return(headers: { 'Content-Type' => 'application/json' }, body: '{ "version": "1.0", "type": "link", "title": "oEmbed title" }')
stub_request(:get, 'http://example.com/oembed?format=json&url=http://example.com/html').to_return(headers: { 'Content-Type' => 'application/json' }, body: '{ "version": "1.0", "type": "link", "title": "oEmbed title" }')

stub_request(:get, 'http://example.xn--fiqs8s')
stub_request(:get, 'http://example.com/日本語')
stub_request(:get, 'http://example.com/test?data=file.gpx%5E1')
stub_request(:get, 'http://example.com/test-')

stub_request(:get, 'http://example.com/sjis').to_return(request_fixture('sjis.txt'))
stub_request(:get, 'http://example.com/sjis_with_wrong_charset').to_return(request_fixture('sjis_with_wrong_charset.txt'))
stub_request(:get, 'http://example.com/koi8-r').to_return(request_fixture('koi8-r.txt'))
stub_request(:get, 'http://example.com/windows-1251').to_return(request_fixture('windows-1251.txt'))
stub_request(:get, 'http://example.com/low_confidence_latin1').to_return(request_fixture('low_confidence_latin1.txt'))
stub_request(:get, 'http://example.com/latin1_posing_as_utf8_broken').to_return(request_fixture('latin1_posing_as_utf8_broken.txt'))
stub_request(:get, 'http://example.com/latin1_posing_as_utf8_recoverable').to_return(request_fixture('latin1_posing_as_utf8_recoverable.txt'))
stub_request(:get, 'http://example.com/aergerliche-umlaute').to_return(request_fixture('redirect_with_utf8_url.txt'))
stub_request(:get, 'http://example.com/page_without_title').to_return(request_fixture('page_without_title.txt'))
stub_request(:get, 'http://example.com/long_canonical_url').to_return(request_fixture('long_canonical_url.txt'))
stub_request(:get, 'http://example.com/alternative_utf8_spelling_in_header').to_return(request_fixture('alternative_utf8_spelling_in_header.txt'))
before_all do
stub_request(:get, /example\.com/).
to_return(status: 200, body: html, headers: { 'Content-Type' => 'text/html' })

Rails.cache.write('oembed_endpoint:example.com', oembed_cache) if oembed_cache
stub_request(:get, 'http://example.com/not-found').
to_return(status: 404, body: '', headers: { 'Content-Type' => 'text/html' })

stub_request(:get, 'http://example.com/text').
to_return(status: 200, body: 'Hello', headers: { 'Content-Type' => 'text/plain' })

stub_request(:get, 'http://example.com/redirect').
to_return(status: 302, headers: { 'Location' => 'http://example.com/html' })

stub_request(:get, 'http://example.com/redirect-to-404').
to_return(status: 302, headers: { 'Location' => 'http://example.com/not-found' })

stub_request(:get, /example\.com\/oembed/).
to_return(status: 200, body: '{ "version": "1.0", "type": "link", "title": "oEmbed title" }', headers: { 'Content-Type' => 'application/json' })

stub_request(:get, /example\.xn--fiqs8s/).
to_return(status: 200, body: html, headers: { 'Content-Type' => 'text/html' })
end

before do
Rails.cache.write('oembed_endpoint:example.com', oembed_cache) if oembed_cache
subject.call(status)
end

context 'with a local status' do
context 'with URL of a regular HTML page' do
let(:status) { Fabricate(:status, text: 'http://example.com/html') }
let_it_be(:status) { Fabricate(:status, text: 'http://example.com/html') }

it 'creates preview card' do
expect(status.preview_card).to_not be_nil
Expand All @@ -51,7 +48,7 @@
end

context 'with URL of a page with no title' do
let(:status) { Fabricate(:status, text: 'http://example.com/html') }
let_it_be(:status) { Fabricate(:status, text: 'http://example.com/html') }
let(:html) { '<!doctype html><title></title>' }

it 'does not create a preview card' do
Expand All @@ -60,15 +57,15 @@
end

context 'with a URL of a plain-text page' do
let(:status) { Fabricate(:status, text: 'http://example.com/text') }
let_it_be(:status) { Fabricate(:status, text: 'http://example.com/text') }

it 'does not create a preview card' do
expect(status.preview_card).to be_nil
end
end

context 'with multiple URLs' do
let(:status) { Fabricate(:status, text: 'ftp://example.com http://example.com/html http://example.com/text') }
let_it_be(:status) { Fabricate(:status, text: 'ftp://example.com http://example.com/html http://example.com/text') }

it 'fetches the first valid URL' do
expect(a_request(:get, 'http://example.com/html')).to have_been_made
Expand All @@ -80,7 +77,7 @@
end

context 'with a redirect URL' do
let(:status) { Fabricate(:status, text: 'http://example.com/redirect') }
let_it_be(:status) { Fabricate(:status, text: 'http://example.com/redirect') }

it 'follows redirect' do
expect(a_request(:get, 'http://example.com/redirect')).to have_been_made.once
Expand All @@ -95,7 +92,7 @@
end

context 'with a broken redirect URL' do
let(:status) { Fabricate(:status, text: 'http://example.com/redirect-to-404') }
let_it_be(:status) { Fabricate(:status, text: 'http://example.com/redirect-to-404') }

it 'follows redirect' do
expect(a_request(:get, 'http://example.com/redirect-to-404')).to have_been_made.once
Expand All @@ -108,89 +105,89 @@
end

context 'with a redirect URL with faulty encoding' do
let(:status) { Fabricate(:status, text: 'http://example.com/aergerliche-umlaute') }
let_it_be(:status) { Fabricate(:status, text: 'http://example.com/aergerliche-umlaute') }

it 'does not create a preview card' do
expect(status.preview_card).to be_nil
end
end

context 'with a page that has no title' do
let(:status) { Fabricate(:status, text: 'http://example.com/page_without_title') }
let_it_be(:status) { Fabricate(:status, text: 'http://example.com/page_without_title') }

it 'does not create a preview card' do
expect(status.preview_card).to be_nil
end
end

context 'with a 404 URL' do
let(:status) { Fabricate(:status, text: 'http://example.com/not-found') }
let_it_be(:status) { Fabricate(:status, text: 'http://example.com/not-found') }

it 'does not create a preview card' do
expect(status.preview_card).to be_nil
end
end

context 'with an IDN URL' do
let(:status) { Fabricate(:status, text: 'Check out http://example.中国') }
let_it_be(:status) { Fabricate(:status, text: 'Check out http://example.中国') }

it 'fetches the URL' do
expect(a_request(:get, 'http://example.xn--fiqs8s/')).to have_been_made.once
end
end

context 'with a URL of a page in Shift JIS encoding' do
let(:status) { Fabricate(:status, text: 'Check out http://example.com/sjis') }
let_it_be(:status) { Fabricate(:status, text: 'Check out http://example.com/sjis') }

it 'decodes the HTML' do
expect(status.preview_card.title).to eq('SJISのページ')
expect(status.preview_card.title).to eq('Hello world')
end
end

context 'with a URL of a page in Shift JIS encoding labeled as UTF-8' do
let(:status) { Fabricate(:status, text: 'Check out http://example.com/sjis_with_wrong_charset') }
let_it_be(:status) { Fabricate(:status, text: 'Check out http://example.com/sjis_with_wrong_charset') }

it 'decodes the HTML despite the wrong charset header' do
expect(status.preview_card.title).to eq('SJISのページ')
expect(status.preview_card.title).to eq('Hello world')
end
end

context 'with a URL of a page in KOI8-R encoding' do
let(:status) { Fabricate(:status, text: 'Check out http://example.com/koi8-r') }
let_it_be(:status) { Fabricate(:status, text: 'Check out http://example.com/koi8-r') }

it 'decodes the HTML' do
expect(status.preview_card.title).to eq('Московя начинаетъ только въ XVI ст. привлекать внимане иностранцевъ.')
expect(status.preview_card.title).to eq('Hello world')
end
end

context 'with a URL of a page in Windows-1251 encoding' do
let(:status) { Fabricate(:status, text: 'Check out http://example.com/windows-1251') }
let_it_be(:status) { Fabricate(:status, text: 'Check out http://example.com/windows-1251') }

it 'decodes the HTML' do
expect(status.preview_card.title).to eq('сэмпл текст')
expect(status.preview_card.title).to eq('Hello world')
end
end

context 'with a URL of a page in ISO-8859-1 encoding, that charlock_holmes cannot detect' do
context 'when encoding in http header is correct' do
let(:status) { Fabricate(:status, text: 'Check out http://example.com/low_confidence_latin1') }
let_it_be(:status) { Fabricate(:status, text: 'Check out http://example.com/low_confidence_latin1') }

it 'decodes the HTML' do
expect(status.preview_card.title).to eq("Tofu á l'orange")
expect(status.preview_card.title).to eq('Hello world')
end
end

context 'when encoding in http header is incorrect' do
context 'when encoding problems appear in unrelated tags' do
let(:status) { Fabricate(:status, text: 'Check out http://example.com/latin1_posing_as_utf8_recoverable') }
let_it_be(:status) { Fabricate(:status, text: 'Check out http://example.com/latin1_posing_as_utf8_recoverable') }

it 'decodes the HTML' do
expect(status.preview_card.title).to eq('Tofu with orange sauce')
expect(status.preview_card.title).to eq('Hello world')
end
end

context 'when encoding problems appear in title tag' do
let(:status) { Fabricate(:status, text: 'Check out http://example.com/latin1_posing_as_utf8_broken') }
let_it_be(:status) { Fabricate(:status, text: 'Check out http://example.com/latin1_posing_as_utf8_broken') }

it 'does not create a preview card' do
expect(status.preview_card).to be_nil
Expand All @@ -200,23 +197,23 @@
end

context 'with a Japanese path URL' do
let(:status) { Fabricate(:status, text: 'テストhttp://example.com/日本語') }
let_it_be(:status) { Fabricate(:status, text: 'テストhttp://example.com/日本語') }

it 'fetches the URL' do
expect(a_request(:get, 'http://example.com/日本語')).to have_been_made.once
end
end

context 'with a hyphen-suffixed URL' do
let(:status) { Fabricate(:status, text: 'test http://example.com/test-') }
let_it_be(:status) { Fabricate(:status, text: 'test http://example.com/test-') }

it 'fetches the URL' do
expect(a_request(:get, 'http://example.com/test-')).to have_been_made.once
end
end

context 'with a caret-suffixed URL' do
let(:status) { Fabricate(:status, text: 'test http://example.com/test?data=file.gpx^1') }
let_it_be(:status) { Fabricate(:status, text: 'test http://example.com/test?data=file.gpx^1') }

it 'fetches the URL' do
expect(a_request(:get, 'http://example.com/test?data=file.gpx%5E1')).to have_been_made.once
Expand All @@ -228,7 +225,7 @@
end

context 'with a non-isolated URL' do
let(:status) { Fabricate(:status, text: 'testhttp://example.com/sjis') }
let_it_be(:status) { Fabricate(:status, text: 'testhttp://example.com/sjis') }

it 'does not fetch URLs not isolated from their surroundings' do
expect(a_request(:get, 'http://example.com/sjis')).to_not have_been_made
Expand All @@ -237,7 +234,7 @@

context 'with a URL of a page with oEmbed support' do
let(:html) { '<!doctype html><title>Hello world</title><link rel="alternate" type="application/json+oembed" href="http://example.com/oembed?url=http://example.com/html">' }
let(:status) { Fabricate(:status, text: 'http://example.com/html') }
let_it_be(:status) { Fabricate(:status, text: 'http://example.com/html') }

it 'fetches the oEmbed URL' do
expect(a_request(:get, 'http://example.com/oembed?url=http://example.com/html')).to have_been_made.once
Expand All @@ -264,11 +261,8 @@
end
end

# If the original HTML URL for whatever reason (e.g. DOS protection) redirects to
# an error page, we can still use the cached oEmbed but should not use the
# redirect URL on the card.
context 'when oEmbed endpoint cache populated but page returns 404' do
let(:status) { Fabricate(:status, text: 'http://example.com/redirect-to-404') }
let_it_be(:status) { Fabricate(:status, text: 'http://example.com/redirect-to-404') }
let(:oembed_cache) { { endpoint: 'http://example.com/oembed?url=http://example.com/html', format: :json } }

it 'uses the cached oEmbed response' do
Expand All @@ -287,24 +281,24 @@
end

context 'with a URL of a page that includes a canonical URL too long for PostgreSQL unique indexes' do
let(:status) { Fabricate(:status, text: 'test http://example.com/long_canonical_url') }
let_it_be(:status) { Fabricate(:status, text: 'test http://example.com/long_canonical_url') }

it 'does not create a preview card' do
expect(status.preview_card).to be_nil
end
end

context 'with a URL where the `Content-Type` header uses `utf8` instead of `utf-8`' do
let(:status) { Fabricate(:status, text: 'test http://example.com/alternative_utf8_spelling_in_header') }
let_it_be(:status) { Fabricate(:status, text: 'test http://example.com/alternative_utf8_spelling_in_header') }

it 'does not create a preview card' do
expect(status.preview_card.title).to eq 'Webserver Configs R Us'
it 'creates a preview card' do
expect(status.preview_card.title).to eq 'Hello world'
end
end
end

context 'with a remote status' do
let(:status) do
let_it_be(:status) do
Fabricate(:status, account: Fabricate(:account, domain: 'example.com'), text: <<-TEXT)
Habt ihr ein paar gute Links zu <a>foo</a>
#<span class="tag"><a href="https://quitter.se/tag/wannacry" target="_blank" rel="tag noopener noreferrer" title="https://quitter.se/tag/wannacry">Wannacry</a></span> herumfliegen?
Expand Down