Merge pull request #3255 from evazion/fix-3254

Fix #3254, #3072: Some Twitter URLs upload wrong image.

app/logical/sources/strategies/twitter.rb

@@ -1,5 +1,7 @@
 module Sources::Strategies
   class Twitter < Base
+    attr_reader :image_urls
+
     def self.url_match?(url)
       url =~ %r!https?://(?:mobile\.)?twitter\.com/\w+/status/\d+! || url =~ %r{https?://pbs\.twimg\.com/media/}
     end
@@ -18,26 +20,23 @@ module Sources::Strategies
 
     def api_response
       status_id = status_id_from_url(url)
-      @api_response ||= TwitterService.new.client.status(status_id)
+      @api_response ||= TwitterService.new.client.status(status_id, tweet_mode: "extended")
     end
 
     def get
       attrs = api_response.attrs
       @artist_name = attrs[:user][:name]
       @profile_url = "https://twitter.com/" + attrs[:user][:screen_name]
-      @image_url = image_urls.first
+      @image_urls = TwitterService.new.image_urls(api_response)
+      @image_url = @image_urls.first
       @artist_commentary_title = ""
-      @artist_commentary_desc = attrs[:text]
+      @artist_commentary_desc = attrs[:full_text]
       @tags = attrs[:entities][:hashtags].map do |text:, indices:|
         [text, "https://twitter.com/hashtag/#{text}"]
       end
     rescue ::Twitter::Error::Forbidden
     end
 
-    def image_urls
-      TwitterService.new.image_urls(url)
-    end
-
     def normalize_for_artist_finder!
       url.downcase
     end
@@ -47,11 +46,11 @@ module Sources::Strategies
     end
 
     def dtext_artist_commentary_desc
-      url_replacements = Array(api_response.attrs[:entities][:urls]).map do |url:, expanded_url:, **attrs|
+      url_replacements = api_response.urls.map do |obj|
-        [url, expanded_url]
+        [obj.url.to_s, obj.expanded_url.to_s]
       end
-      url_replacements += Array(api_response.attrs[:entities][:media]).map do |url:, expanded_url:, **attrs|
+      url_replacements += api_response.media.map do |obj|
-        [url, ""]
+        [obj.url.to_s, ""]
       end
       url_replacements = url_replacements.to_h
 

app/logical/twitter_api.rb

@@ -1,12 +0,0 @@
-class TwitterApi
-  def client
-    api_token = Cache.get("twitter-api-token")
-    hoge = Twitter::Rest::Client.new do |config|
-      config.consumer_key = Danbooru.config.twiter_api_key
-      config.consumer_secret = Danbooru.config.twitter_api_secret
-      config.bearer_token = api_token if api_token
-    end
-    Cache.put("twitter-api-token", hoge.bearer_token)
-    hoge
-  end
-end

app/logical/twitter_service.rb

@@ -20,7 +20,7 @@ class TwitterService
   def extract_urls_for_status(tweet)
     tweet.media.map do |obj|
       if obj.is_a?(Twitter::Media::Photo)
-        obj.media_url.to_s + ":orig"
+        obj.media_url_https.to_s + ":orig"
       elsif obj.is_a?(Twitter::Media::Video)
         video = obj.video_info.variants.select do |x|
           x.content_type == "video/mp4"
@@ -29,7 +29,7 @@ class TwitterService
           video.url.to_s
         end
       end
-    end.compact
+    end.compact.uniq
   end
 
   def extract_og_image_from_page(url)
@@ -50,20 +50,11 @@ class TwitterService
     [extract_og_image_from_page(url)].compact
   end
 
-  def image_urls(tweet_url)
+  def image_urls(tweet)
-    tweet_url =~ %r{/status/(\d+)}
-    twitter_id = $1
-    tweet = client.status(twitter_id)
-    urls = []
-
     if tweet.media.any?
-      urls = extract_urls_for_status(tweet)
+      extract_urls_for_status(tweet)
     elsif tweet.urls.any?
-      urls = extract_urls_for_card(tweet)
+      extract_urls_for_card(tweet)
     end
-
-    urls.uniq
-  rescue => e
-    []
   end
 end

test/unit/sources/twitter_test.rb

@@ -57,6 +57,28 @@ module Sources
       end
     end
 
+    context "An extended tweet" do
+      should "extract the correct image url" do
+        @site = Sources::Site.new("https://twitter.com/onsen_musume_jp/status/865534101918330881")
+        @site.get
+
+        assert_equal(["https://pbs.twimg.com/media/DAL-ntWV0AEbhes.jpg:orig"], @site.image_urls)
+      end
+
+      should "extract all the image urls" do
+        @site = Sources::Site.new("https://twitter.com/aoimanabu/status/892370963630743552")
+        @site.get
+
+        urls = %w[
+          https://pbs.twimg.com/media/DGJWp59UIAA_-en.jpg:orig
+          https://pbs.twimg.com/media/DGJWqGLUwAAn2RL.jpg:orig
+          https://pbs.twimg.com/media/DGJWqT_UMAAvmSK.jpg:orig
+        ]
+
+        assert_equal(urls, @site.image_urls)
+      end
+    end
+
     context "The source site for a restricted twitter" do
       setup do
         @site = Sources::Site.new("https://mobile.twitter.com/Strangestone/status/556440271961858051")
@@ -64,7 +86,7 @@ module Sources
       end
 
       should "get the image url" do
-        assert_equal("http://pbs.twimg.com/media/B7jfc1JCcAEyeJh.png:orig", @site.image_url)
+        assert_equal("https://pbs.twimg.com/media/B7jfc1JCcAEyeJh.png:orig", @site.image_url)
       end
     end
 
@@ -83,7 +105,7 @@ module Sources
       end
 
       should "get the image url" do
-        assert_equal("http://pbs.twimg.com/media/B4HSEP5CUAA4xyu.png:orig", @site.image_url)
+        assert_equal("https://pbs.twimg.com/media/B4HSEP5CUAA4xyu.png:orig", @site.image_url)
       end
 
       should "get the tags" do