From a8deb6648bc348e64469cc3451040b46ea057b77 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 13 Dec 2017 12:15:28 +0100 Subject: [PATCH] Fix redundant HTTP request in FetchLinkCardService (#6002) --- app/lib/provider_discovery.rb | 19 ++++++-- app/services/fetch_link_card_service.rb | 58 ++++++++++++------------- 2 files changed, 45 insertions(+), 32 deletions(-) diff --git a/app/lib/provider_discovery.rb b/app/lib/provider_discovery.rb index bcc4ed500eb..04ba381010e 100644 --- a/app/lib/provider_discovery.rb +++ b/app/lib/provider_discovery.rb @@ -2,13 +2,26 @@ class ProviderDiscovery < OEmbed::ProviderDiscovery class << self + def get(url, **options) + provider = discover_provider(url, options) + + options.delete(:html) + + provider.get(url, options) + end + def discover_provider(url, **options) - res = Request.new(:get, url).perform format = options[:format] - raise OEmbed::NotFound, url if res.code != 200 || res.mime_type != 'text/html' + if options[:html] + html = Nokogiri::HTML(options[:html]) + else + res = Request.new(:get, url).perform - html = Nokogiri::HTML(res.to_s) + raise OEmbed::NotFound, url if res.code != 200 || res.mime_type != 'text/html' + + html = Nokogiri::HTML(res.to_s) + end if format.nil? || format == :json provider_endpoint ||= html.at_xpath('//link[@type="application/json+oembed"]')&.attribute('href')&.value diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb index 09534d0ff03..d0472a1d7fb 100644 --- a/app/services/fetch_link_card_service.rb +++ b/app/services/fetch_link_card_service.rb @@ -40,6 +40,12 @@ class FetchLinkCardService < BaseService return if res.code != 405 && (res.code != 200 || res.mime_type != 'text/html') + @response = Request.new(:get, @url).perform + + return if @response.code != 200 || @response.mime_type != 'text/html' + + @html = @response.to_s + attempt_oembed || attempt_opengraph end @@ -70,32 +76,32 @@ class FetchLinkCardService < BaseService end def attempt_oembed - response = OEmbed::Providers.get(@url) + embed = OEmbed::Providers.get(@url, html: @html) - return false unless response.respond_to?(:type) + return false unless embed.respond_to?(:type) - @card.type = response.type - @card.title = response.respond_to?(:title) ? response.title : '' - @card.author_name = response.respond_to?(:author_name) ? response.author_name : '' - @card.author_url = response.respond_to?(:author_url) ? response.author_url : '' - @card.provider_name = response.respond_to?(:provider_name) ? response.provider_name : '' - @card.provider_url = response.respond_to?(:provider_url) ? response.provider_url : '' + @card.type = embed.type + @card.title = embed.respond_to?(:title) ? embed.title : '' + @card.author_name = embed.respond_to?(:author_name) ? embed.author_name : '' + @card.author_url = embed.respond_to?(:author_url) ? embed.author_url : '' + @card.provider_name = embed.respond_to?(:provider_name) ? embed.provider_name : '' + @card.provider_url = embed.respond_to?(:provider_url) ? embed.provider_url : '' @card.width = 0 @card.height = 0 case @card.type when 'link' - @card.image = URI.parse(response.thumbnail_url) if response.respond_to?(:thumbnail_url) + @card.image = URI.parse(embed.thumbnail_url) if embed.respond_to?(:thumbnail_url) when 'photo' - return false unless response.respond_to?(:url) - @card.embed_url = response.url - @card.image = URI.parse(response.url) - @card.width = response.width.presence || 0 - @card.height = response.height.presence || 0 + return false unless embed.respond_to?(:url) + @card.embed_url = embed.url + @card.image = URI.parse(embed.url) + @card.width = embed.width.presence || 0 + @card.height = embed.height.presence || 0 when 'video' - @card.width = response.width.presence || 0 - @card.height = response.height.presence || 0 - @card.html = Formatter.instance.sanitize(response.html, Sanitize::Config::MASTODON_OEMBED) + @card.width = embed.width.presence || 0 + @card.height = embed.height.presence || 0 + @card.html = Formatter.instance.sanitize(embed.html, Sanitize::Config::MASTODON_OEMBED) when 'rich' # Most providers rely on