Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cache for OEmbed endpoints to avoid extra HTTP requests #12403

Merged
merged 16 commits into from
Nov 17, 2019
23 changes: 16 additions & 7 deletions app/services/fetch_link_card_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ def call(status)
def process_url
@card ||= PreviewCard.new(url: @url)

attempt_oembed || attempt_opengraph
end

def html
return @html if defined?(@html)

Request.new(:get, @url).perform do |res|
if res.code == 200 && res.mime_type == 'text/html'
@html = res.body_with_limit
Expand All @@ -48,10 +54,6 @@ def process_url
@html_charset = nil
end
end

return if @html.nil?

attempt_oembed || attempt_opengraph
end

def attach_card
Expand Down Expand Up @@ -88,12 +90,17 @@ def skip_link?(a)
end

def attempt_oembed
service = FetchOEmbedService.new
embed = service.call(@url, html: @html)
url = Addressable::URI.parse(service.endpoint_url)
service = FetchOEmbedService.new
url_domain = Addressable::URI.parse(@url).normalized_host
cached_endpoint = Rails.cache.read("oembed_endpoint:#{url_domain}")

embed = service.call(@url, cached_endpoint: cached_endpoint) unless cached_endpoint.nil?
embed ||= service.call(@url, html: html) unless html.nil?

return false if embed.nil?

url = Addressable::URI.parse(service.endpoint_url)

@card.type = embed[:type]
@card.title = embed[:title] || ''
@card.author_name = embed[:author_name] || ''
Expand Down Expand Up @@ -127,6 +134,8 @@ def attempt_oembed
end

def attempt_opengraph
return if html.nil?

detector = CharlockHolmes::EncodingDetector.new
detector.strip_tags = true

Expand Down
31 changes: 30 additions & 1 deletion app/services/fetch_oembed_service.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,20 @@
# frozen_string_literal: true

class FetchOEmbedService
ENDPOINT_CACHE_EXPIRES_IN = 24.hours.freeze

attr_reader :url, :options, :format, :endpoint_url

def call(url, options = {})
@url = url
@options = options

discover_endpoint!
if @options[:cached_endpoint]
parse_cached_endpoint!
else
discover_endpoint!
end

fetch!
end

Expand All @@ -32,10 +39,32 @@ def discover_endpoint!
return if @endpoint_url.blank?

@endpoint_url = (Addressable::URI.parse(@url) + @endpoint_url).to_s

cache_endpoint!
rescue Addressable::URI::InvalidURIError
@endpoint_url = nil
end

def parse_cached_endpoint!
cached = @options[:cached_endpoint]

return if cached[:endpoint].nil? || cached[:format].nil?

@endpoint_url = Addressable::Template.new(cached[:endpoint]).expand(url: @url).to_s
@format = cached[:format]
end

def cache_endpoint!
url_domain = Addressable::URI.parse(@url).normalized_host

endpoint_hash = {
endpoint: @endpoint_url.gsub(URI.encode_www_form_component(@url), '{url}'),
format: @format,
}

Rails.cache.write("oembed_endpoint:#{url_domain}", endpoint_hash, expires_in: ENDPOINT_CACHE_EXPIRES_IN)
end

def fetch!
return if @endpoint_url.blank?

Expand Down
18 changes: 18 additions & 0 deletions spec/services/fetch_oembed_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,24 @@

end

context 'when endpoint is cached' do
before do
stub_request(:get, 'http://www.youtube.com/oembed?format=json&url=https://www.youtube.com/watch?v=dqwpQarrDwk').to_return(
status: 200,
headers: { 'Content-Type': 'text/html' },
body: request_fixture('oembed_json_empty.html')
)
end

it 'returns new provider without fetching original URL first' do
subject.call('https://www.youtube.com/watch?v=dqwpQarrDwk', cached_endpoint: { endpoint: 'http://www.youtube.com/oembed?format=json&url={url}', format: :json })
expect(a_request(:get, 'https://www.youtube.com/watch?v=dqwpQarrDwk')).to_not have_been_made
expect(subject.endpoint_url).to eq 'http://www.youtube.com/oembed?format=json&url=https%3A%2F%2Fwww.youtube.com%2Fwatch%3Fv%3DdqwpQarrDwk'
expect(subject.format).to eq :json
expect(a_request(:get, 'http://www.youtube.com/oembed?format=json&url=https%3A%2F%2Fwww.youtube.com%2Fwatch%3Fv%3DdqwpQarrDwk')).to have_been_made
end
end

context 'when status code is not 200' do
before do
stub_request(:get, 'https://host.test/oembed.html').to_return(
Expand Down