refactoring and correctly set headers

lib/lol/request.rb

@@ -38,19 +38,6 @@ def api_url path, params = {}
       "#{url}?#{api_query_string params}"
     end
 
-    def post_api_url path, params = {}
-      {
-        url: api_url(path, params),
-        options: {
-          headers: {
-            "X-Riot-Token" => api_key,
-            "Content-Type" => "application/json",
-            "Accept"       => "application/json"
-          }
-        }
-      }
-    end
-
     def api_base_url
       "https://#{region}.api.pvp.net"
     end
@@ -73,25 +60,39 @@ def clean_url(url)
     # @param body [Hash] Body for POST request
     # @param options [Hash] Options passed to HTTParty
     # @return [String] raw response of the call
-    def perform_request url, verb = :get, body = {}, options = {}
+    def perform_request url, verb = :get, body = nil, options = {}
+      options_id = options.inspect
       can_cache = [:post, :put].include?(verb) ? false : cached?
-      if can_cache && result = store.get("#{clean_url(url)}#{options.inspect}")
+      if can_cache && result = store.get("#{clean_url(url)}#{options_id}")
         return JSON.parse(result)
       end
+      response = perform_uncached_request url, verb, body, options
+      store.setex "#{clean_url(url)}#{options_id}", ttl, response.to_json if can_cache
+      response
+    end
 
-      params = [:post, :put].include?(verb) ? [url, options.merge({body: body.to_json})] : url
-      response = self.class.send(verb, *params)
+    def perform_uncached_request url, verb = :get, body = nil, options = {}
+      options[:headers] ||= {}
+      options[:headers].merge!({
+        "Content-Type" => "application/json",
+        "Accept"       => "application/json"
+      })
+      if [:post, :put].include?(verb)
+        options[:body] = body.to_json if body
+        options[:headers]['X-Riot-Token'] = api_key
+      end
+      response = self.class.send(verb, url, options)
       if response.respond_to?(:code) && !(200...300).include?(response.code)
         raise NotFound.new("404 Not Found") if response.not_found?
         raise TooManyRequests.new('429 Rate limit exceeded') if response.code == 429
         raise InvalidAPIResponse.new(url, response)
       end
 
-      response = response.parsed_response if response.respond_to?(:parsed_response)
-
-      store.setex "#{clean_url(url)}#{options.inspect}", ttl, response.to_json if can_cache
-
-      response
+      if response.respond_to?(:parsed_response)
+        response.parsed_response
+      else
+        response
+      end
     end
 
     # @return [Redis] returns the cache store

lib/lol/tournament_provider_request.rb

@@ -14,12 +14,11 @@ def api_base_url
     # @param path [String] API path to call
     # @return [String] full fledged url
     def api_url path, params = {}
-      url = "#{api_base_url}/tournament/public/#{self.class.api_version}/#{path}"
+      "#{api_base_url}/tournament/public/#{self.class.api_version}/#{path}"
     end
 
     def tournament_request path, body
-      pau = post_api_url path
-      perform_request(pau[:url], :post, body, pau[:options])
+      perform_request(api_url(path), :post, body)
     end
 
     # Returns a tournament code
@@ -87,8 +86,7 @@ def update_code tournament_code, options = {}
         mapType: map_type
       }.reject{ |k,v| v.nil? }
 
-      pau = post_api_url "code/#{tournament_code}"
-      perform_request(pau[:url], :put, body, pau[:options])
+      perform_request(api_url("code/#{tournament_code}"), :put, body)
       get_code tournament_code
     end
 

spec/lol/request_spec.rb

@@ -37,25 +37,60 @@
 
     it "calls HTTParty get" do
       expect(subject.class).to receive(:get).and_return(error401)
-      expect { subject.perform_request "foo?api_key=asd"}.to raise_error(InvalidAPIResponse)
+      expect { subject.perform_request subject.api_url("foo")}.to raise_error(InvalidAPIResponse)
+    end
+
+    it "sets content type in get requests" do
+      expect(subject.class).to receive(:get) do |url, options|
+        expect(options[:headers]["Content-Type"]).to eq "application/json"
+      end
+      subject.perform_request subject.api_url("foo")
+    end
+
+    it "sets the accept header in get requests" do
+      expect(subject.class).to receive(:get) do |url, options|
+        expect(options[:headers]["Accept"]).to eq "application/json"
+      end
+      subject.perform_request subject.api_url("foo")
+    end
+
+    it "sets content type in post requests" do
+      expect(subject.class).to receive(:post) do |url, options|
+        expect(options[:headers]["Content-Type"]).to eq "application/json"
+      end
+      subject.perform_request subject.api_url("foo"), :post
+    end
+
+    it "sets the accept header in post requests" do
+      expect(subject.class).to receive(:post) do |url, options|
+        expect(options[:headers]["Accept"]).to eq "application/json"
+      end
+      subject.perform_request subject.api_url("foo"), :post
+    end
+
+    it "sets the api key header in post requests" do
+      expect(subject.class).to receive(:post) do |url, options|
+        expect(options[:headers]["X-Riot-Token"]).to eq(subject.api_key)
+      end
+      subject.perform_request subject.api_url("foo"), :post
     end
 
     it "handles 401" do
       expect(subject.class).to receive(:get).and_return(error401)
-      expect { subject.perform_request "foo?api_key=asd" }.to raise_error(InvalidAPIResponse)
+      expect { subject.perform_request subject.api_url("foo") }.to raise_error(InvalidAPIResponse)
     end
 
     it "handles 404" do
       expect(error401).to receive(:respond_to?).at_least(:once).and_return(true)
       expect(error401).to receive(:not_found?).and_return(true)
       expect(subject.class).to receive(:get).and_return(error401)
-      expect { subject.perform_request "foo?api_key=asd"}.to raise_error(NotFound)
+      expect { subject.perform_request subject.api_url("foo")}.to raise_error(NotFound)
     end
 
     it 'handles 429' do
       expect(error429).to receive(:respond_to?).and_return(true)
       expect(subject.class).to receive(:get).and_return(error429)
-      expect { subject.perform_request "foo?api_key=asd"}.to raise_error(TooManyRequests)
+      expect { subject.perform_request subject.api_url("foo")}.to raise_error(TooManyRequests)
     end
 
     context "post requests" do
@@ -88,7 +123,7 @@ def setex key, ttl, val
       let(:fake_redis) { FakeRedis.new }
       let(:request) { Request.new "api_key", "euw", {redis: fake_redis, ttl: 60, cached: true }}
       before :each do
-        expect(request.class).to receive(:get).with("/foo?api_key=asd").and_return({foo: "bar"}).at_least(:once)
+        expect(request.class).to receive(:get).with("/foo?api_key=asd", instance_of(Hash)).and_return({foo: "bar"}).at_least(:once)
         first_result = request.perform_request "/foo?api_key=asd"
       end
 
@@ -157,39 +192,6 @@ def setex key, ttl, val
     end
   end
 
-  describe "post_api_url" do
-    let(:pau) { subject.post_api_url "/foo" }
-
-    it "returns an hash" do
-      expect(pau).to be_a(Hash)
-    end
-
-    it "contains an url key" do
-      expect(pau).to have_key(:url)
-    end
-
-    it "contains an options key" do
-      expect(pau).to have_key(:options)
-    end
-
-    it "contains a header key in options" do
-      expect(pau[:options]).to have_key(:headers)
-    end
-
-    it "returns the api key in an header" do
-      expect(pau[:options][:headers]).to have_key("X-Riot-Token")
-      expect(pau[:options][:headers]["X-Riot-Token"]).to eq(subject.api_key)
-    end
-
-    it "includes a content-type header" do
-      expect(pau[:options][:headers]["Content-Type"]).to eq("application/json")
-    end
-
-    it "uses api_url for the url part" do
-      expect(pau[:url]).to eq(subject.api_url "/foo")
-    end
-  end
-
   describe "#api_base_url" do
     it "returns the base domain" do
       expect(subject.api_base_url).to eq "https://euw.api.pvp.net"

spec/lol/tournament_provider_request_spec.rb

@@ -48,7 +48,7 @@
       full_url = request.api_url "code/CODE-FOR-TEST?#{request.api_query_string}"
       fixture_json = load_fixture('get-code', TournamentProviderRequest.api_version)
 
-      expect(TournamentProviderRequest).to receive(:get).with(full_url).and_return(fixture_json)
+      expect(TournamentProviderRequest).to receive(:get).with(full_url, instance_of(Hash)).and_return(fixture_json)
     end
     subject { request.get_code "CODE-FOR-TEST" }
 
@@ -64,8 +64,7 @@
       expect(request).to receive(:perform_request).once.ordered.with(
         instance_of(String),
         :put,
-        { allowedParticipants: "1,2,3,4,5,6,7,8,9,10" },
-        instance_of(Hash)
+        { allowedParticipants: "1,2,3,4,5,6,7,8,9,10" }
       )
       expect(request).to receive(:perform_request).once.ordered.with(instance_of(String)).and_return(fixture)
       request.update_code "CODE-FOR-TEST", { allowed_participants: [1,2,3,4,5,6,7,8,9,10] }

spec/support/helpers.rb

@@ -51,13 +51,13 @@ def stub_request(request_object, fixture_name, url, params={})
     full_url = request_object.api_url(url, params)
     fixture_json = load_fixture(fixture_name, request_class.api_version, :get)
 
-    expect(request_class).to receive(:get).with(full_url).and_return(fixture_json)
+    expect(request_class).to receive(:get).with(full_url, instance_of(Hash)).and_return(fixture_json)
   end
 
   def stub_request_raw(request_object, raw_response, url, params={})
     request_class = request_object.class
     full_url = request_object.api_url(url, params)
 
-    expect(request_class).to receive(:get).with(full_url).and_return(raw_response)
+    expect(request_class).to receive(:get).with(full_url, instance_of(Hash)).and_return(raw_response)
   end
-end
+end