diff --git a/Changelog.md b/Changelog.md index cd59ef273..b851c2d80 100644 --- a/Changelog.md +++ b/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 diff --git a/Gemfile b/Gemfile index ba814e704..22646f5ce 100644 --- a/Gemfile +++ b/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 diff --git a/Gemfile.lock b/Gemfile.lock index ba77b24a8..7500fc138 100644 --- a/Gemfile.lock +++ b/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) - mime-types + typhoeus (0.6.3) + 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) diff --git a/config/defaults.yml b/config/defaults.yml index 4c7ec4817..d5fa2bf7e 100644 --- a/config/defaults.yml +++ b/config/defaults.yml @@ -61,6 +61,7 @@ defaults: enable: false suggest_email: typhoeus_verbose: false + typhoeus_concurrency: 20 username_blacklist: - 'admin' - 'administrator' diff --git a/config/diaspora.yml.example b/config/diaspora.yml.example index 3449c28b0..5dc0b94f0 100644 --- a/config/diaspora.yml.example +++ b/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. diff --git a/lib/hydra_wrapper.rb b/lib/hydra_wrapper.rb index a2d02307a..562a7ef26 100644 --- a/lib/hydra_wrapper.rb +++ b/lib/hydra_wrapper.rb @@ -4,86 +4,97 @@ class HydraWrapper - OPTS = {:max_redirects => 3, :timeout => 25000, :method => :post, - :verbose => AppConfig.settings.typhoeus_verbose?, - :ssl_cacert => AppConfig.environment.certificate_authorities.get, - :headers => {'Expect' => '', - 'Transfer-Encoding' => ''} - } + OPTS = { + maxredirs: 3, + timeout: 25, + method: :post, + 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 + # 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) + insert_job(receive_url, xml, people_for_receive_url) + end + end + end + + 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)) + @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]) def grouped_people - @people.group_by do |person| - @dispatcher_class.receive_url_for(person) - end + @people.group_by { |person| + @dispatcher_class.receive_url_for person + } 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) - end - end - end - - # Prepares and inserts job into the @hydra queue + # Prepares and inserts job into the hydra queue # @param url [String] # @param xml [String] # @params people [Array] - def insert_job(url, xml, people) - request = Typhoeus::Request.new(url, OPTS.merge(:params => {:xml => CGI::escape(xml)})) - prepare_request!(request, people) - @hydra.queue(request) + def insert_job url, xml, people + request = Typhoeus::Request.new url, OPTS.merge(body: {xml: CGI.escape(xml)}) + prepare_request request, people + 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) - Person.url_batch_update(people_for_receive_url, response.headers_hash['Location']) + if redirecting_to_https? response + 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}'") - @failed_people += people_for_receive_url.map{|i| i.id} + message = { + 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) - if response.code >= 300 && response.code < 400 - response.headers_hash['Location'] == response.request.url.sub('http://', 'https://') - else - false - end + def redirecting_to_https? response + response.code >= 300 && response.code < 400 && + response.headers_hash['Location'] == response.request.url.sub('http://', 'https://') end end diff --git a/spec/lib/hydra_wrapper_spec.rb b/spec/lib/hydra_wrapper_spec.rb index 97549d812..0e0b2344f 100644 --- a/spec/lib/hydra_wrapper_spec.rb +++ b/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, "", stub) + @wrapper = HydraWrapper.new stub, @people, "", 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 - @wrapper.hydra.should_receive(:run) + hydra = stub.as_null_object + @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') - decoded = Base64.decode64(@wrapper.encoded_object_xml) - @wrapper.dispatcher_class.should_receive(:salmon).with(@wrapper.user, decoded).once.and_return(true) - @wrapper.xml_factory - @wrapper.xml_factory + Base64.stub(:decode64).and_return "#{@wrapper.encoded_object_xml} encoded" + decoded = Base64.decode64 @wrapper.encoded_object_xml + @wrapper.dispatcher_class.should_receive(:salmon).with(@wrapper.user, decoded).once.and_return true + @wrapper.send :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 => "")) - @wrapper.stub(:grouped_people).and_return({'https://foo.com' => @wrapper.people}) + @wrapper.dispatcher_class = stub salmon: stub(xml_for: "") + @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', "", @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.dispatcher_class = stub(:salmon => stub(:xml_for => false)) - @wrapper.should_not_receive(:insert_job) + @wrapper.stub(:grouped_people).and_return('https://foo.com' => [stub]) + @wrapper.dispatcher_class = stub salmon: stub(xml_for: false) + @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) - @wrapper.redirecting_to_https?(resp).should be_false + resp = stub code: 200 + @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 diff --git a/spec/workers/http_multi_spec.rb b/spec/workers/http_multi_spec.rb index b111d8571..c0b42ab40 100644 --- a/spec/workers/http_multi_spec.rb +++ b/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') - @failed_response = Typhoeus::Response.new(:code => 504, :headers => "", :body => "", :time => 0.2, :effective_url => 'http://foobar.com') + Typhoeus::Hydra.stub!(:new).and_return(@hydra) + @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_ids, "Postzord::Dispatcher::Private") + Workers::HttpMulti.new.perform bob.id, @post_xml, @people.map(&:id), "Postzord::Dispatcher::Private" end it 'retries' do - person = @people[0] + person = @people.first - @hydra.stub(:post, person.receive_url).and_return(@failed_response) - - Typhoeus::Hydra.stub!(:new).and_return(@hydra) + Typhoeus.stub(person.receive_url).and_return @failed_response 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) - - 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") + 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(:code => 301,:effective_url => 'https://foobar.com', :headers_hash => {"Location" => person.receive_url.sub('http://', 'https://')}, :body => "", :time => 0.2) - @hydra.stub(:post, person.receive_url).and_return(response) + response = Typhoeus::Response.new( + code: 301, + 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) - @hydra.stub(:post, @people[1].receive_url).and_return(@response) - Typhoeus::Hydra.stub!(:new).and_return(@hydra) + Salmon::EncryptedSlap.rspec_reset + + 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