bump Typhoeus and refactor HydraWrapper

Changelog.md

@@ -4,6 +4,7 @@
 
 * Refactored config/ directory [#4145](https://github.com/diaspora/diaspora/pull/4145).
 * Drop misleading fallback donation form. [Proposal](https://www.loomio.org/discussions/1045?proposal=2722)
+* Update Typhoeus to 0.6.3 and refactor HydraWrapper. [#4162](https://github.com/diaspora/diaspora/pull/4162)
 
 ## Bug fixes
 

Gemfile

@@ -82,7 +82,7 @@ gem 'acts-as-taggable-on', '2.4.0'
 gem 'addressable',        '2.3.4', :require => 'addressable/uri'
 gem 'faraday',            '0.8.7'
 gem 'faraday_middleware', '0.9.0'
-gem 'typhoeus',           '0.3.3'
+gem 'typhoeus',           '0.6.3'
 
 # Views
 
@@ -184,7 +184,7 @@ group :test do
 
   gem 'factory_girl_rails', '4.2.1'
   gem 'timecop',            '0.6.1'
-  gem 'webmock',            '1.8.11', :require => false
+  gem 'webmock',            '1.11.0', :require => false
 end
 
 

Gemfile.lock

@@ -96,6 +96,9 @@ GEM
       warden (~> 1.2.1)
     diff-lcs (1.2.3)
     erubis (2.7.0)
+    ethon (0.5.12)
+      ffi (>= 1.3.0)
+      mime-types (~> 1.18)
     excon (0.20.1)
     execjs (1.4.0)
       multi_json (~> 1.0)
@@ -203,7 +206,7 @@ GEM
       redcarpet (>= 2.0)
     messagebus_ruby_api (1.0.3)
     method_source (0.8.1)
-    mime-types (1.22)
+    mime-types (1.23)
     mini_magick (3.5.0)
       subexec (~> 0.2.1)
     mobile-fu (1.1.1)
@@ -382,8 +385,8 @@ GEM
       faraday (~> 0.8, < 0.10)
       multi_json (~> 1.0)
       simple_oauth (~> 0.2)
-    typhoeus (0.3.3)
+    typhoeus (0.6.3)
-      mime-types
+      ethon (~> 0.5.11)
     tzinfo (0.3.37)
     uglifier (2.0.1)
       execjs (>= 0.3.0)
@@ -394,9 +397,9 @@ GEM
       raindrops (~> 0.7)
     warden (1.2.1)
       rack (>= 1.0)
-    webmock (1.8.11)
+    webmock (1.11.0)
       addressable (>= 2.2.7)
-      crack (>= 0.1.7)
+      crack (>= 0.3.2)
     websocket (1.0.7)
     will_paginate (3.0.4)
     xpath (0.1.4)
@@ -477,8 +480,8 @@ DEPENDENCIES
   spork (= 1.0.0rc3)
   timecop (= 0.6.1)
   twitter (= 4.6.2)
-  typhoeus (= 0.3.3)
+  typhoeus (= 0.6.3)
   uglifier (= 2.0.1)
   unicorn (= 4.6.2)
-  webmock (= 1.8.11)
+  webmock (= 1.11.0)
   will_paginate (= 3.0.4)

config/defaults.yml

@@ -61,6 +61,7 @@ defaults:
       enable: false
       suggest_email:
     typhoeus_verbose: false
+    typhoeus_concurrency: 20
     username_blacklist:
       - 'admin'
       - 'administrator'

config/diaspora.yml.example

@@ -233,6 +233,12 @@ configuration: ## Section
       ## in the spotlight to.
       #suggest_email: 'admin@example.org'
     
+
+    ## Maximum number of parallel HTTP requests made to other pods
+    ## Be careful, raising this setting will heavily increase the
+    ## memory usage of your Sidekiq workers
+    #typhoeus_concurrency: 20
+
     ## CURL debug
     ## Turn on extra verbose output when sending stuff. No you
     ## don't need to touch this unless explicitly told to.

lib/hydra_wrapper.rb

@@ -4,86 +4,97 @@
 
 class HydraWrapper
 
-  OPTS = {:max_redirects => 3, :timeout => 25000, :method => :post,
+  OPTS = {
-          :verbose => AppConfig.settings.typhoeus_verbose?,
+    maxredirs: 3,
-          :ssl_cacert => AppConfig.environment.certificate_authorities.get,
+    timeout: 25,
-          :headers => {'Expect'            => '',
+    method: :post,
-                       'Transfer-Encoding' => ''}
+    verbose: AppConfig.settings.typhoeus_verbose?,
+    cainfo: AppConfig.environment.certificate_authorities.get,
+    headers: {
+      'Expect'            => '',
+      'Transfer-Encoding' => '',
+      'User-Agent'        => "Diaspora #{AppConfig.version_string}"
+    }
   }
 
   attr_reader :failed_people, :user, :encoded_object_xml
-  attr_accessor :dispatcher_class, :people, :hydra
+  attr_accessor :dispatcher_class, :people
+  delegate :run, to: :hydra
 
-  def initialize(user, people, encoded_object_xml, dispatcher_class)
+  def initialize user, people, encoded_object_xml, dispatcher_class
     @user = user
     @failed_people = []
-    @hydra = Typhoeus::Hydra.new
     @people = people
     @dispatcher_class = dispatcher_class
     @encoded_object_xml = encoded_object_xml
   end
 
-  # Delegates run to the @hydra
-  def run
-    @hydra.run
-  end
-
-  # @return [Salmon]
-  def xml_factory
-    @xml_factory ||= @dispatcher_class.salmon(@user, Base64.decode64(@encoded_object_xml))
-  end
-
-  # Group people on their receiving_urls
-  # @return [Hash] People grouped by receive_url ([String] => [Array<Person>])
-  def grouped_people
-    @people.group_by do |person|
-      @dispatcher_class.receive_url_for(person)
-    end
-  end 
-
   # Inserts jobs for all @people
   def enqueue_batch
     grouped_people.each do |receive_url, people_for_receive_url|
       if xml = xml_factory.xml_for(people_for_receive_url.first)
-        self.insert_job(receive_url, xml, people_for_receive_url)
+        insert_job(receive_url, xml, people_for_receive_url)
       end
     end
   end
 
-  # Prepares and inserts job into the @hydra queue
+  private
+
+  def hydra
+    @hydra ||= Typhoeus::Hydra.new(max_concurrency: AppConfig.settings.typhoeus_concurrency.to_i)
+  end
+
+  # @return [Salmon]
+  def xml_factory
+    @xml_factory ||= @dispatcher_class.salmon @user, Base64.decode64(@encoded_object_xml)
+  end
+
+  # Group people on their receiving_urls
+  # @return [Hash] People grouped by receive_url ([String] => [Array<Person>])
+  def grouped_people
+    @people.group_by { |person|
+      @dispatcher_class.receive_url_for person
+    }
+  end 
+
+  # Prepares and inserts job into the hydra queue
   # @param url [String]
   # @param xml [String]
   # @params people [Array<Person>]
-  def insert_job(url, xml, people)
+  def insert_job url, xml, people
-    request = Typhoeus::Request.new(url, OPTS.merge(:params => {:xml => CGI::escape(xml)}))
+    request = Typhoeus::Request.new url, OPTS.merge(body: {xml: CGI.escape(xml)})
-    prepare_request!(request, people)
+    prepare_request request, people
-    @hydra.queue(request)
+    hydra.queue request
   end
 
   # @param request [Typhoeus::Request]
   # @param person [Person]
-  def prepare_request!(request, people_for_receive_url)
+  def prepare_request request, people_for_receive_url
     request.on_complete do |response|
       # Save the reference to the pod to the database if not already present
-      Pod.find_or_create_by_url(response.effective_url)
+      Pod.find_or_create_by_url response.effective_url
 
-      if redirecting_to_https?(response) 
+      if redirecting_to_https? response
-        Person.url_batch_update(people_for_receive_url, response.headers_hash['Location'])
+        Person.url_batch_update people_for_receive_url, response.headers_hash['Location']
       end
 
       unless response.success?
-        Rails.logger.info("event=http_multi_fail sender_id=#{@user.id}  url=#{response.effective_url} response_code='#{response.code}'")
+        message = {
-        @failed_people += people_for_receive_url.map{|i| i.id}
+          event: "http_multi_fail",
+          sender_id: @user.id,
+          url: response.effective_url,
+          response_code: response.code
+        }
+        message[:response_message] = response.return_message if response.code == 0
+        Rails.logger.info message.to_a.map { |k,v| "#{k}=#{v}" }.join(' ')
+        @failed_people += people_for_receive_url.map(&:id)
       end
     end
   end
 
   # @return [Boolean]
-  def redirecting_to_https?(response)
+  def redirecting_to_https? response
-    if response.code >= 300 && response.code < 400
+    response.code >= 300 && response.code < 400 &&
     response.headers_hash['Location'] == response.request.url.sub('http://', 'https://')
-    else
-      false
-    end
   end
 end

spec/lib/hydra_wrapper_spec.rb

@@ -2,12 +2,12 @@
 #   licensed under the Affero General Public License version 3 or later.  See
 #   the COPYRIGHT file.
 
-require 'hydra_wrapper'
+require 'spec_helper'
 
 describe HydraWrapper do
   before do
     @people = ["person", "person2", "person3"]
-    @wrapper = HydraWrapper.new(stub, @people, "<encoded_xml>", stub)
+    @wrapper = HydraWrapper.new stub, @people, "<encoded_xml>", stub
   end
 
   describe 'initialize' do
@@ -16,7 +16,7 @@ describe HydraWrapper do
       encoded_object_xml = "encoded xml"
       dispatcher_class = "Postzord::Dispatcher::Private"
 
-      wrapper = HydraWrapper.new(user, @people, encoded_object_xml, dispatcher_class)
+      wrapper = HydraWrapper.new user, @people, encoded_object_xml, dispatcher_class
       wrapper.user.should == user
       wrapper.people.should == @people
       wrapper.encoded_object_xml.should == encoded_object_xml
@@ -25,40 +25,41 @@ describe HydraWrapper do
 
   describe '#run' do
     it 'delegates #run to the @hydra' do
-      @wrapper.hydra = stub.as_null_object
+      hydra = stub.as_null_object
-      @wrapper.hydra.should_receive(:run)
+      @wrapper.instance_variable_set :@hydra, hydra
+      hydra.should_receive :run
       @wrapper.run
     end
   end
 
   describe '#xml_factory' do
     it 'calls the salmon method on the dispatcher class (and memoizes)' do
-      Base64.stub(:decode64).and_return(@wrapper.encoded_object_xml + 'decoded')
+      Base64.stub(:decode64).and_return "#{@wrapper.encoded_object_xml} encoded"
-      decoded = Base64.decode64(@wrapper.encoded_object_xml)
+      decoded = Base64.decode64 @wrapper.encoded_object_xml
-      @wrapper.dispatcher_class.should_receive(:salmon).with(@wrapper.user, decoded).once.and_return(true)
+      @wrapper.dispatcher_class.should_receive(:salmon).with(@wrapper.user, decoded).once.and_return true
-      @wrapper.xml_factory
+      @wrapper.send :xml_factory
-      @wrapper.xml_factory
+      @wrapper.send :xml_factory
     end
   end
 
   describe '#grouped_people' do
     it 'groups people given their receive_urls' do
-      @wrapper.dispatcher_class.should_receive(:receive_url_for).and_return("foo.com","bar.com","bar.com")
+      @wrapper.dispatcher_class.should_receive(:receive_url_for).and_return "foo.com", "bar.com", "bar.com"
 
-      @wrapper.grouped_people.should == {"foo.com" => [@people[0]], "bar.com" => @people[1,2]}
+      @wrapper.send(:grouped_people).should == {"foo.com" => [@people[0]], "bar.com" => @people[1,2]}
     end
   end
 
   describe '#enqueue_batch' do
     it 'calls #grouped_people' do
-      @wrapper.should_receive(:grouped_people).and_return([])
+      @wrapper.should_receive(:grouped_people).and_return []
       @wrapper.enqueue_batch
     end
 
     it 'inserts a job for every group of people' do
       Base64.stub(:decode64)
-      @wrapper.dispatcher_class = stub(:salmon => stub(:xml_for => "<XML>"))
+      @wrapper.dispatcher_class = stub salmon: stub(xml_for: "<XML>")
-      @wrapper.stub(:grouped_people).and_return({'https://foo.com' => @wrapper.people})
+      @wrapper.stub(:grouped_people).and_return('https://foo.com' => @wrapper.people)
       @wrapper.people.should_receive(:first).once
       @wrapper.should_receive(:insert_job).with('https://foo.com', "<XML>", @wrapper.people).once
       @wrapper.enqueue_batch
@@ -66,9 +67,9 @@ describe HydraWrapper do
 
     it 'does not insert a job for a person whos xml returns false' do
       Base64.stub(:decode64)
-      @wrapper.stub(:grouped_people).and_return({'https://foo.com' => [stub]})
+      @wrapper.stub(:grouped_people).and_return('https://foo.com' => [stub])
-      @wrapper.dispatcher_class = stub(:salmon => stub(:xml_for => false))
+      @wrapper.dispatcher_class = stub salmon: stub(xml_for: false)
-      @wrapper.should_not_receive(:insert_job)
+      @wrapper.should_not_receive :insert_job
       @wrapper.enqueue_batch
     end
 
@@ -76,22 +77,34 @@ describe HydraWrapper do
 
   describe '#redirecting_to_https?!' do
     it 'does not execute unless response has a 3xx code' do
-      resp = stub(:code => 200)
+      resp = stub code: 200
-      @wrapper.redirecting_to_https?(resp).should be_false
+      @wrapper.send(:redirecting_to_https?, resp).should be_false
     end
 
     it "returns true if just the protocol is different" do
       host = "the-same.com/"
-      resp = stub(:request => stub(:url => "http://#{host}"), :code => 302, :headers_hash => {'Location' => "https://#{host}"})
+      resp = stub(
+        request: stub(url: "http://#{host}"), 
+        code: 302, 
+        headers_hash: {
+          'Location' => "https://#{host}"
+        }
+      )
 
-      @wrapper.redirecting_to_https?(resp).should be_true
+      @wrapper.send(:redirecting_to_https?, resp).should be_true
     end
 
     it "returns false if not just the protocol is different" do
       host = "the-same.com/"
-      resp = stub(:request => stub(:url => "http://#{host}"), :code => 302, :headers_hash => {'Location' => "https://not-the-same/"})
+      resp = stub(
+        request: stub(url: "http://#{host}"), 
+        code: 302,
+        headers_hash: {
+          'Location' => "https://not-the-same/"
+        }
+      )
 
-      @wrapper.redirecting_to_https?(resp).should be_false
+      @wrapper.send(:redirecting_to_https?, resp).should be_false
     end
   end
 end

spec/workers/http_multi_spec.rb

@@ -2,7 +2,8 @@ require 'spec_helper'
 
 describe Workers::HttpMulti do
   before :all do
-    WebMock.disable_net_connect!(:allow_localhost => true)
+    WebMock.disable_net_connect! allow_localhost: true
+    WebMock::HttpLibAdapters::TyphoeusAdapter.disable!
     enable_typhoeus
   end
   after :all do
@@ -12,60 +13,65 @@ describe Workers::HttpMulti do
 
   before do
     @people = [FactoryGirl.create(:person), FactoryGirl.create(:person)]
-    @post_xml = Base64.encode64("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAH")
+    @post_xml = Base64.encode64 "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAH"
 
     @hydra = Typhoeus::Hydra.new
-    @response = Typhoeus::Response.new(:code => 200, :headers => "", :body => "", :time => 0.2, :effective_url => 'http://foobar.com')
+    Typhoeus::Hydra.stub!(:new).and_return(@hydra)
-    @failed_response = Typhoeus::Response.new(:code => 504, :headers => "", :body => "", :time => 0.2, :effective_url => 'http://foobar.com')
+    @salmon = Salmon::EncryptedSlap.create_by_user_and_activity bob, Base64.decode64(@post_xml)
+    Salmon::EncryptedSlap.stub(:create_by_user_and_activity).and_return @salmon
+    @body = "encrypted things"
+    @salmon.stub(:xml_for).and_return @body
+
+    @response = Typhoeus::Response.new(
+      code: 200,
+      body: "",
+      time: 0.2,
+      effective_url: 'http://foobar.com'
+    )
+    @failed_response = Typhoeus::Response.new(
+      code: 504,
+      body: "",
+      time: 0.2,
+      effective_url: 'http://foobar.com'
+    )
   end
 
   it 'POSTs to more than one person' do
     @people.each do |person|
-      @hydra.stub(:post, person.receive_url).and_return(@response)
+      Typhoeus.stub(person.receive_url).and_return @response
     end
 
     @hydra.should_receive(:queue).twice
     @hydra.should_receive(:run).once
-    Typhoeus::Hydra.stub!(:new).and_return(@hydra)
 
-    people_ids = @people.map{ |p| p.id }
+    Workers::HttpMulti.new.perform bob.id, @post_xml, @people.map(&:id), "Postzord::Dispatcher::Private"
-    Workers::HttpMulti.new.perform(bob.id, @post_xml, people_ids, "Postzord::Dispatcher::Private")
   end
 
   it 'retries' do
-    person = @people[0]
+    person = @people.first
 
-    @hydra.stub(:post, person.receive_url).and_return(@failed_response)
+    Typhoeus.stub(person.receive_url).and_return @failed_response
-
-    Typhoeus::Hydra.stub!(:new).and_return(@hydra)
     
     Workers::HttpMulti.should_receive(:perform_in).with(1.hour, bob.id, @post_xml, [person.id], anything, 1).once
-    Workers::HttpMulti.new.perform(bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private")
+    Workers::HttpMulti.new.perform bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private"
   end
 
   it 'max retries' do
-    person = @people[0]
+    person = @people.first
 
-    @hydra.stub(:post, person.receive_url).and_return(@failed_response)
+    Typhoeus.stub(person.receive_url).and_return @failed_response
 
-    Typhoeus::Hydra.stub!(:new).and_return(@hydra)
+    Workers::HttpMulti.should_not_receive :perform_in
-
+    Workers::HttpMulti.new.perform bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private", 3
-    Workers::HttpMulti.should_not_receive(:perform_in)
-    Workers::HttpMulti.new.perform(bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private", 3)
   end
 
   it 'generates encrypted xml for people' do
-    person = @people[0]
+    person = @people.first
 
-    @hydra.stub(:post, person.receive_url).and_return(@response)
+    Typhoeus.stub(person.receive_url).and_return @response
+    @salmon.should_receive(:xml_for).and_return @body
 
-    Typhoeus::Hydra.stub!(:new).and_return(@hydra)
+    Workers::HttpMulti.new.perform bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private"
-
-    salmon = Salmon::EncryptedSlap.create_by_user_and_activity(bob, Base64.decode64(@post_xml))
-    Salmon::EncryptedSlap.stub(:create_by_user_and_activity).and_return(salmon)
-    salmon.should_receive(:xml_for).and_return("encrypted things")
-
-    Workers::HttpMulti.new.perform(bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private")
   end
 
   it 'updates http users who have moved to https' do
@@ -73,27 +79,31 @@ describe Workers::HttpMulti do
     person.url = 'http://remote.net/'
     person.save
 
-    stub_request(:post, person.receive_url).to_return(:status=>200, :body=>"", :headers=>{})
+    response = Typhoeus::Response.new(
-    response = Typhoeus::Response.new(:code => 301,:effective_url => 'https://foobar.com', :headers_hash => {"Location" => person.receive_url.sub('http://', 'https://')}, :body => "", :time => 0.2)
+      code: 301,
-    @hydra.stub(:post, person.receive_url).and_return(response)
+      effective_url: 'https://foobar.com',
+      response_headers: "Location: #{person.receive_url.sub('http://', 'https://')}",
+      body: "",
+      time: 0.2
+    )
+    Typhoeus.stub(person.receive_url).and_return response
 
-    Typhoeus::Hydra.stub!(:new).and_return(@hydra)
+    Workers::HttpMulti.new.perform bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private"
-
-    Workers::HttpMulti.new.perform(bob.id, @post_xml, [person.id], "Postzord::Dispatcher::Private")
     person.reload
     person.url.should == "https://remote.net/"
   end
 
   it 'only sends to users with valid RSA keys' do
-    person = @people[0]
+    person = @people.first
     person.serialized_public_key = "-----BEGIN RSA PUBLIC KEY-----\nPsych!\n-----END RSA PUBLIC KEY-----"
     person.save
 
-    @hydra.stub(:post, @people[0].receive_url).and_return(@response)
+    Salmon::EncryptedSlap.rspec_reset
-    @hydra.stub(:post, @people[1].receive_url).and_return(@response)
+
-    Typhoeus::Hydra.stub!(:new).and_return(@hydra)
+    Typhoeus.stub(person.receive_url).and_return @response
+    Typhoeus.stub(@people[1].receive_url).and_return @response
 
     @hydra.should_receive(:queue).once
-    Workers::HttpMulti.new.perform(bob.id, @post_xml, [@people[0].id, @people[1].id], "Postzord::Dispatcher::Private")
+    Workers::HttpMulti.new.perform bob.id, @post_xml, @people.map(&:id), "Postzord::Dispatcher::Private"
   end
 end