refactor twitter service model

Changelog.md

@@ -9,6 +9,7 @@
 * Refactor 404.html, fix [#4078](https://github.com/diaspora/diaspora/issues/4078)
 * Remove the (now useless) last post link from the user profile. [#4540](https://github.com/diaspora/diaspora/pull/4540)
 * Refactor ConversationsController, move query building to User model. [#4547](https://github.com/diaspora/diaspora/pull/4547)
+* Refactor the Twitter service model [#4387](https://github.com/diaspora/diaspora/pull/4387)
 
 ## Bug fixes
 * Highlight down arrow at the user menu on hover [#4441](https://github.com/diaspora/diaspora/pull/4441)

app/models/services/twitter.rb

@@ -1,103 +1,111 @@
 class Services::Twitter < Service
   include ActionView::Helpers::TextHelper
+  include Rails.application.routes.url_helpers
   include MarkdownifyHelper
 
   MAX_CHARACTERS = 140
   SHORTENED_URL_LENGTH = 21
-  
   LINK_PATTERN = %r{https?://\S+}
 
   def provider
     "twitter"
   end
 
-  def post(post, url='')
+  def post post, url=''
-    Rails.logger.debug("event=post_to_service type=twitter sender_id=#{self.user_id}")
+    Rails.logger.debug "event=post_to_service type=twitter sender_id=#{self.user_id}"
-    (0...20).each do |retry_count|
+    tweet = attempt_post post
-      begin
+    post.tweet_id = tweet.id
-        message = build_twitter_post(post, url, retry_count)
-        @tweet = client.update(message)
-        break
-      rescue Twitter::Error::Forbidden => e
-        if e.message != 'Status is over 140 characters' || retry_count == 20
-          raise e
-        end
-      end
-    end
-    post.tweet_id = @tweet.id
     post.save
   end
 
-  def adjust_length_for_urls(post_text)
-    real_length = post_text.length
-    URI.extract( post_text, ['http','https'] ) do |a_url|
-      # add or subtract from real length - urls for tweets are always 
-      # shortened to SHORTENED_URL_LENGTH
-      if a_url.length >= SHORTENED_URL_LENGTH
-        real_length -= a_url.length - SHORTENED_URL_LENGTH
-      else
-        real_length += SHORTENED_URL_LENGTH - a_url.length 
-      end
-    end
-    return real_length
-  end
-
-  def add_post_link(post, post_text, maxchars)
-    post_url = Rails.application.routes.url_helpers.short_post_url(
-      post, 
-      :protocol => AppConfig.pod_uri.scheme, 
-      :host => AppConfig.pod_uri.authority
-    )
-    truncated_text = truncate post_text, length: maxchars - SHORTENED_URL_LENGTH + 1
-    truncated_text = restore_truncated_url truncated_text, post_text, maxchars
-
-    "#{truncated_text} #{post_url}"
-  end
-
-  def build_twitter_post(post, url, retry_count=0)
-    maxchars = MAX_CHARACTERS - retry_count*5
-    post_text = strip_markdown(post.text(:plain_text => true))
-    #if photos, always include url, otherwise not for short posts
-    if adjust_length_for_urls(post_text) > maxchars || post.photos.any?
-      post_text = add_post_link(post, post_text, maxchars)
-    end
-    return post_text
-  end
-
   def profile_photo_url
-    client.user(nickname).profile_image_url_https("original")
+    client.user(nickname).profile_image_url_https "original"
   end
 
-  def delete_post(post)
+  def delete_post post
     if post.present? && post.tweet_id.present?
-      Rails.logger.debug("event=delete_from_service type=twitter sender_id=#{self.user_id}")
+      Rails.logger.debug "event=delete_from_service type=twitter sender_id=#{self.user_id}"
-      delete_from_twitter(post.tweet_id)
+      delete_from_twitter post.tweet_id
     end
   end
 
-  def delete_from_twitter(service_post_id)
-    client.status_destroy(service_post_id)
-  end
-
   private
+
   def client
     @client ||= Twitter::Client.new(
       oauth_token: self.access_token,
       oauth_token_secret: self.access_secret
     )
   end
-  
-  def restore_truncated_url truncated_text, post_text, maxchars
-      return truncated_text if truncated_text !~ /#{LINK_PATTERN}\Z/
 
-      url = post_text.match(LINK_PATTERN, truncated_text.rindex('http'))[0]
+  def attempt_post post, retry_count=0
-      truncated_text = truncate(
+    message = build_twitter_post post, retry_count
-        post_text,
+    tweet = client.update message
-        length: maxchars - SHORTENED_URL_LENGTH + 2,
+  rescue Twitter::Error::Forbidden => e
-        separator: ' ',
+    if e.message != 'Status is over 140 characters' || retry_count == 20
-        omission: ''
+      raise e
-      )
+    else
+      attempt_post post, retry_count+1
+    end
+  end
+
+  def build_twitter_post post, retry_count=0
+    max_characters = MAX_CHARACTERS - retry_count * 5
+
+    post_text = strip_markdown post.text(plain_text: true)
+    truncate_and_add_post_link post, post_text, max_characters
+  end
+
+  def truncate_and_add_post_link post, post_text, max_characters
+    return post_text unless needs_link? post, post_text, max_characters
+
+    post_url = short_post_url(
+      post,
+      protocol: AppConfig.pod_uri.scheme,
+      host: AppConfig.pod_uri.authority
+    )
+
+    truncated_text = truncate post_text, length: max_characters - SHORTENED_URL_LENGTH + 1
+    truncated_text = restore_truncated_url truncated_text, post_text, max_characters
+
+    "#{truncated_text} #{post_url}"
+  end
+
+  def needs_link? post, post_text, max_characters
+    adjust_length_for_urls(post_text) > max_characters || post.photos.any?
+  end
+
+  def adjust_length_for_urls post_text
+    real_length = post_text.length
+
+    URI.extract(post_text, ['http','https']) do |url|
+      # add or subtract from real length - urls for tweets are always
+      # shortened to SHORTENED_URL_LENGTH
+      if url.length >= SHORTENED_URL_LENGTH
+        real_length -= url.length - SHORTENED_URL_LENGTH
+      else
+        real_length += SHORTENED_URL_LENGTH - url.length
+      end
+    end
+
+    real_length
+  end
+
+  def restore_truncated_url truncated_text, post_text, max_characters
+    return truncated_text if truncated_text !~ /#{LINK_PATTERN}\Z/
+
+    url = post_text.match(LINK_PATTERN, truncated_text.rindex('http'))[0]
+    truncated_text = truncate(
+      post_text,
+      length: max_characters - SHORTENED_URL_LENGTH + 2,
+      separator: ' ',
+      omission: ''
+    )
 
     "#{truncated_text} #{url} ..."
   end
+
+  def delete_from_twitter service_post_id
+    client.status_destroy service_post_id
+  end
 end

spec/models/services/twitter_spec.rb

@@ -23,7 +23,7 @@ describe Services::Twitter do
     it 'sets the tweet_id on the post' do
       @service.post(@post)
       @post.tweet_id.should match "1234"
-    end    
+    end
 
     it 'swallows exception raised by twitter always being down' do
       pending
@@ -33,18 +33,18 @@ describe Services::Twitter do
 
     it 'should call build_twitter_post' do
       url = "foo"
-      @service.should_receive(:build_twitter_post).with(@post, url, 0)
+      @service.should_receive(:build_twitter_post).with(@post, 0)
       @service.post(@post, url)
     end
-    
+
     it 'removes text formatting markdown from post text' do
       message = "Text with some **bolded** and _italic_ parts."
       post = stub(:text => message, :photos => [])
-      @service.build_twitter_post(post, '').should match "Text with some bolded and italic parts."
+      @service.send(:build_twitter_post, post).should match "Text with some bolded and italic parts."
     end
-    
+
   end
-  
+
   describe "message size limits" do
     before :each do
       @long_message_start = SecureRandom.hex(25)
@@ -54,38 +54,38 @@ describe Services::Twitter do
     it "should not truncate a short message" do
       short_message = SecureRandom.hex(20)
       short_post = stub(:text => short_message, :photos => [])
-      @service.build_twitter_post(short_post, '').should match short_message
+      @service.send(:build_twitter_post, short_post).should match short_message
     end
 
     it "should truncate a long message" do
       long_message = SecureRandom.hex(220)
       long_post = stub(:text => long_message, :id => 1, :photos => [])
-      @service.build_twitter_post(long_post, '').length.should be < long_message.length
+      @service.send(:build_twitter_post, long_post).length.should be < long_message.length
     end
 
     it "should not truncate a long message with an http url" do
       long_message = " http://joindiaspora.com/a-very-long-url-name-that-will-be-shortened.html " + @long_message_end
       long_post = stub(:text => long_message, :id => 1, :photos => [])
       @post.text = long_message
-      answer = @service.build_twitter_post(@post, '')
+      answer = @service.send(:build_twitter_post, @post)
 
       answer.should_not match /\.\.\./
     end
-    
+
     it "should not cut links when truncating a post" do
       long_message = SecureRandom.hex(40) + " http://joindiaspora.com/a-very-long-url-name-that-will-be-shortened.html " + SecureRandom.hex(55)
       long_post = stub(:text => long_message, :id => 1, :photos => [])
-      answer = @service.build_twitter_post(long_post, '')
+      answer = @service.send(:build_twitter_post, long_post)
 
       answer.should match /\.\.\./
       answer.should match /shortened\.html/
     end
-    
+
     it "should append the otherwise-cut link when truncating a post" do
       long_message = "http://joindiaspora.com/a-very-long-decoy-url.html " + SecureRandom.hex(20) + " http://joindiaspora.com/a-very-long-url-name-that-will-be-shortened.html " + SecureRandom.hex(55) + " http://joindiaspora.com/a-very-long-decoy-url-part-2.html"
       long_post = stub(:text => long_message, :id => 1, :photos => [])
-      answer = @service.build_twitter_post(long_post, '')
+      answer = @service.send(:build_twitter_post, long_post)
-      
+
       answer.should match /\.\.\./
       answer.should match /shortened\.html/
     end
@@ -93,28 +93,28 @@ describe Services::Twitter do
     it "should not truncate a long message with an https url" do
       long_message = " https://joindiaspora.com/a-very-long-url-name-that-will-be-shortened.html " + @long_message_end
       @post.text = long_message
-      answer = @service.build_twitter_post(@post, '')
+      answer = @service.send(:build_twitter_post, @post)
       answer.should_not match /\.\.\./
     end
 
     it "should truncate a long message with an ftp url" do
       long_message = @long_message_start + " ftp://joindiaspora.com/a-very-long-url-name-that-will-be-shortened.html " + @long_message_end
       long_post = stub(:text => long_message, :id => 1, :photos => [])
-      answer = @service.build_twitter_post(long_post, '')
+      answer = @service.send(:build_twitter_post, long_post)
 
       answer.should match /\.\.\./
     end
-    
+
     it "should not truncate a message of maximum length" do
         exact_size_message = SecureRandom.hex(70)
         exact_size_post = stub(:text => exact_size_message, :id => 1, :photos => [])
-        answer = @service.build_twitter_post(exact_size_post, '')
+        answer = @service.send(:build_twitter_post, exact_size_post)
-        
+
         answer.should match exact_size_message
     end
-    
+
   end
-  
+
   describe "with photo" do
     before do
       @photos = [alice.build_post(:photo, :pending => true, :user_file=> File.open(photo_fixture_name)),
@@ -128,14 +128,14 @@ describe Services::Twitter do
       @status_message.save!
       alice.add_to_streams(@status_message, alice.aspects)
     end
-    
+
     it "should include post url in short message with photos" do
-        answer = @service.build_twitter_post(@status_message, '')
+        answer = @service.send(:build_twitter_post, @status_message)
         answer.should include 'http'
     end
-    
+
   end
-  
+
   describe "#profile_photo_url" do
     it 'returns the original profile photo url' do
       user_stub = stub