Skip to content

Commit

Permalink
Merge pull request #27 from nicolaracco/master
Browse files Browse the repository at this point in the history
cache using full url and stripping api_key
  • Loading branch information
intinig committed Jul 9, 2014
2 parents 52ed9a5 + a379825 commit fe2d0e4
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 10 deletions.
5 changes: 4 additions & 1 deletion lib/lol/request.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require "uri"
require "active_support/core_ext/object/to_query"

module Lol
class NotFound < StandardError; end
Expand Down Expand Up @@ -39,7 +40,9 @@ def api_url path, params = {}
# Returns just a path from a full api url
# @return [String]
def clean_url(url)
URI.parse(url).path
uri = URI.parse(url)
uri.query = CGI.parse(uri.query).reject { |k| k == 'api_key' }.to_query
uri.to_s
end

# Calls the API via HTTParty and handles errors
Expand Down
18 changes: 9 additions & 9 deletions spec/lol/request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,19 @@

it "calls HTTParty get" do
expect(subject.class).to receive(:get).and_return(error401)
expect { subject.perform_request "foo"}.to raise_error(InvalidAPIResponse)
expect { subject.perform_request "foo?api_key=asd"}.to raise_error(InvalidAPIResponse)
end

it "handles 401" do
expect(subject.class).to receive(:get).and_return(error401)
expect { subject.perform_request "foo" }.to raise_error(InvalidAPIResponse)
expect { subject.perform_request "foo?api_key=asd" }.to raise_error(InvalidAPIResponse)
end

it "handles 404" do
expect(error401).to receive(:respond_to?).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"}.to raise_error(NotFound)
expect { subject.perform_request "foo?api_key=asd"}.to raise_error(NotFound)
end

context "caching" do
Expand All @@ -74,27 +74,27 @@ 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").and_return({foo: "bar"}).at_least(:once)
first_result = request.perform_request "/foo"
expect(request.class).to receive(:get).with("/foo?api_key=asd").and_return({foo: "bar"}).at_least(:once)
first_result = request.perform_request "/foo?api_key=asd"
end

it "is cached" do
expect(request.class).not_to receive(:get)
request.perform_request "/foo"
request.perform_request "/foo?api_key=asd"
end

it "serializes cached responses" do
expect(JSON).to receive(:parse)
request.perform_request "/foo"
request.perform_request "/foo?api_key=asd"
end

it "sets ttl" do
expect(fake_redis.get("/foo:ttl")).to eq(60)
expect(fake_redis.get("/foo?:ttl")).to eq(60)
end

it "uses clean urls" do
expect(request).to receive(:clean_url).at_least(:once)
request.perform_request "/foo"
request.perform_request "/foo?api_key=asd"
end
end
end
Expand Down

0 comments on commit fe2d0e4

Please sign in to comment.