Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

Add pagination and ratelimit support for responses from Instagram #76

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add pagination and ratelimit support for responses from Instagram #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f84b520
Allow for pagination results to be requested from a given Instagram::…
Jun 17, 2013
4c9bdf9
Remove unnecessary regex interpolation.
Jun 17, 2013
3969d3d
Use class method to get Instagram client object instead of direct ins…
Jun 17, 2013
fb48951
Return an empty array to conform with initialization of Response object.
Jun 17, 2013
e82f64e
Fix test.
Jun 17, 2013
9d2fec8
Add ratelimit header information to every normal Instagram response.
Jun 18, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ API Usage Examples
------------------
require "rubygems"
require "instagram"

# All methods require authentication (either by client ID or access token).
# To get your Instagram OAuth credentials, register an app at http://instagr.am/oauth/client/register/
Instagram.configure do |config|
Expand All @@ -84,6 +84,15 @@ API Usage Examples
page_2_max_id = page_1.pagination.next_max_id
page_2 = Instagram.user_recent_media(777, :max_id => page_2_max_id ) unless page_2_max_id.nil?

# Get next page of data from response
page_1 = Instagram.user_recent_media(777)
page_2 = page_1.next

# Inspect rate limit data provided by Instagram API (5000 requests/client id/hour, 5000 requests/access token/hour)
page_1 = Instagram.user_recent_media(777)
limit = page_1.ratelimit.limit
remaining = page1.ratelimit.remaining

# Get the currently authenticated user's media feed
puts Instagram.user_media_feed

Expand All @@ -105,7 +114,7 @@ API Usage Examples
# Search for a location by Fousquare ID (v2)
puts Instagram.location_search("3fd66200f964a520c5f11ee3")




Contributing
Expand Down
3 changes: 2 additions & 1 deletion lib/instagram/request.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ def request(method, path, options, raw=false, unformatted=false, no_response_wra
end
return response if raw
return response.body if no_response_wrapper
return Response.create( response.body )
return Response.create( response.body, {:limit => response.headers['x-ratelimit-limit'].to_i,
:remaining => response.headers['x-ratelimit-remaining'].to_i} )
end

def formatted_path(path)
Expand Down
15 changes: 14 additions & 1 deletion lib/instagram/response.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,29 @@
module Instagram
module Response
def self.create( response_hash )
def self.create( response_hash, ratelimit_hash )
data = response_hash.data.dup rescue response_hash
data.extend( self )
data.instance_exec do
@pagination = response_hash.pagination
@meta = response_hash.meta
@ratelimit = ::Hashie::Mash.new(ratelimit_hash)
end
data
end

def next
if pagination.next_url
client = Instagram.client(Instagram.options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you feel this is less than ideal, in my opinion it is a deal-breaker.

Consider the use-case:

  1. Create a new client with a user's access_token
  2. get a page of protected media that they have access to (either their own protected profile's recent media or that of a protected user they follow)
  3. use the response's next method
  4. the request is made with Instagram.options[:client_id] or perhaps a default token Instagram.options[:access_token] that does not have access to the protected content
  5. surprise: 400 response

The client object will need to be passed to the response so it can be used again for the next page. I considered suggesting the parsing of the next_url param, but that solution is nearly as clunky since it doesn't accomodate for the client's connection parameters or other options unless they are baked into the ::Instagram.options singleton (which is a bad pattern in many applications).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused. Instagram.options[:access_token] is set upon the initial client creation, so wouldn't that be what is used for the call to next?

Are you raising a case where there may be multiple Instagram clients acting in an application, each with their own access_token, and interleaving calls between them would cause problems?

Agreed that passing the client object is a better approach. I didn't want to "break" the current architecture, which is based on one-request/one-response. I couldn't come up with a seemingly good way to share the client object internally.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use Instagram::Client.new(options) as our primary interface for creating a new client, and yes we use many clients with many access tokens in our application, avoiding the Instagram.options defaults entirely. This enables us to use each client separately, concurrently in a threadsafe manner.

In this case, you're already changing the API a bit by adding the non-optional ratelimit_hash param, so it's a good time to really take a look at it, to make sure that the changes we're introducing now make potential future changes simple to do without breaking interface.

Since the response object is only ever created by a client request, its API isn't as critical as the public object-creators. In this case, I'd consider this as a possible new interface:

# @param client [Instagram::Client]
# @param response_hash
# @param extra_params [Hash<Symbol,#to_hash>]
# @options extra_params [#to_hash] :ratelimit (Hashie::new)
def self.create(client, response_hash, extra_params = {})
  # ...
end

Also, why Instagram::Response is a module and not a class, especially since they bake in class-ish stuff on top of it with this ::create method is beyond me.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems much better. Agreed on Response being a class and not a module.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(just a note that I'm not a maintainer, so you don't have to follow my suggestions; I just use this and many other API clients and therefore I Have Opinions 😸 )

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are good suggestions and I appreciate them.

pagination.next_url.sub!('http://', 'https://') #Make the URL secure if it isn't already
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really a problem? Aren't all of the API methods limited to https?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The next URLs that were returned to me in result sets were 'http'.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😞

response = client.get(pagination.next_url.sub(Instagram.endpoint, ''), {}, false, true)
response
else
[]
end
end

attr_reader :pagination
attr_reader :meta
attr_reader :ratelimit
end
end
1 change: 1 addition & 0 deletions spec/fixtures/user_media_feed_next.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"pagination": {}, "meta": {"code": 200}, "data": []}
82 changes: 51 additions & 31 deletions spec/instagram/client/users_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@
stub_get("users/self/feed.#{format}").
with(:query => {:access_token => @client.access_token}).
to_return(:body => fixture("user_media_feed.#{format}"), :headers => {:content_type => "application/#{format}; charset=utf-8"})

stub_get("users/self/feed?access_token=f59def8.001cde77128843169627c0308237bafa&max_id=22063131").
to_return(:body => fixture("user_media_feed_next.#{format}"), :headers => {:content_type => "application/#{format}; charset=utf-8"})
end

it "should get the correct resource" do
Expand All @@ -171,6 +174,7 @@

context Instagram::Response do
let(:user_media_feed_response){ @client.user_media_feed }
let(:next_url){ 'http://api.instagram.com/v1/users/self/feed?access_token=f59def8.001cde77128843169627c0308237bafa&max_id=22063131' }
subject{ user_media_feed_response }

it{ should be_an_instance_of(Array) }
Expand All @@ -182,7 +186,7 @@
subject{ user_media_feed_response.pagination }

it{ should be_an_instance_of(Hashie::Mash) }
its(:next_url){ should == 'http://api.instagram.com/v1/users/self/feed?access_token=f59def8.001cde77128843169627c0308237bafa&max_id=22063131' }
its(:next_url){ should == next_url }
its(:next_max_id){ should == '22063131' }
end

Expand All @@ -192,11 +196,27 @@
it{ should be_an_instance_of(Hashie::Mash) }
its(:code){ should == 200 }
end

context 'next' do
subject{ user_media_feed_response }

it{ should respond_to(:next) }

it "should handle a next call correctly" do
next_feed = user_media_feed_response.next
a_get(next_url.sub('http://api.instagram.com/v1/', '')).should have_been_made
next_feed.should be_an_instance_of(Array)
next_feed.should be_a_kind_of(Instagram::Response)
next_feed.pagination.should be_empty
next_feed.next.should be_empty
end
end

end
end

describe ".user_liked_media" do

before do
stub_get("users/self/media/liked.#{format}").
with(:query => {:access_token => @client.access_token}).
Expand All @@ -210,7 +230,7 @@
should have_been_made
end
end

describe ".user_recent_media" do

context "with user ID passed" do
Expand Down Expand Up @@ -273,148 +293,148 @@
users.first.username.should == "shayne"
end
end

describe ".user_relationship" do

before do
stub_get("users/4/relationship.#{format}").
with(:query => {:access_token => @client.access_token}).
to_return(:body => fixture("relationship.#{format}"), :headers => {:content_type => "application/#{format}; charset=utf-8"})
end

it "should get the correct resource" do
@client.user_relationship(4)
a_get("users/4/relationship.#{format}").
with(:query => {:access_token => @client.access_token}).
should have_been_made
end

it "should return a relationship status response" do
status = @client.user_relationship(4)
status.incoming_status.should == "requested_by"
end
end

describe ".follow_user" do

before do
stub_post("users/4/relationship.#{format}").
with(:body => {:action => "follow", :access_token => @client.access_token}).
to_return(:body => fixture("follow_user.#{format}"), :headers => {:content_type => "application/#{format}; charset=utf-8"})
end

it "should get the correct resource" do
@client.follow_user(4)
a_post("users/4/relationship.#{format}").
with(:body => {:action => "follow", :access_token => @client.access_token}).
should have_been_made
end

it "should return a relationship status response" do
status = @client.follow_user(4)
status.outgoing_status.should == "requested"
end
end

describe ".unfollow_user" do

before do
stub_post("users/4/relationship.#{format}").
with(:body => {:action => "unfollow", :access_token => @client.access_token}).
to_return(:body => fixture("unfollow_user.#{format}"), :headers => {:content_type => "application/#{format}; charset=utf-8"})
end

it "should get the correct resource" do
@client.unfollow_user(4)
a_post("users/4/relationship.#{format}").
with(:body => {:action => "unfollow", :access_token => @client.access_token}).
should have_been_made
end

it "should return a relationship status response" do
status = @client.unfollow_user(4)
status.outgoing_status.should == "none"
end
end

describe ".block_user" do

before do
stub_post("users/4/relationship.#{format}").
with(:body => {:action => "block", :access_token => @client.access_token}).
to_return(:body => fixture("block_user.#{format}"), :headers => {:content_type => "application/#{format}; charset=utf-8"})
end

it "should get the correct resource" do
@client.block_user(4)
a_post("users/4/relationship.#{format}").
with(:body => {:action => "block", :access_token => @client.access_token}).
should have_been_made
end

it "should return a relationship status response" do
status = @client.block_user(4)
status.outgoing_status.should == "none"
end
end

describe ".unblock_user" do

before do
stub_post("users/4/relationship.#{format}").
with(:body => {:action => "unblock", :access_token => @client.access_token}).
to_return(:body => fixture("unblock_user.#{format}"), :headers => {:content_type => "application/#{format}; charset=utf-8"})
end

it "should get the correct resource" do
@client.unblock_user(4)
a_post("users/4/relationship.#{format}").
with(:body => {:action => "unblock", :access_token => @client.access_token}).
should have_been_made
end

it "should return a relationship status response" do
status = @client.unblock_user(4)
status.outgoing_status.should == "none"
end
end

describe ".approve_user" do

before do
stub_post("users/4/relationship.#{format}").
with(:body => {:action => "approve", :access_token => @client.access_token}).
to_return(:body => fixture("approve_user.#{format}"), :headers => {:content_type => "application/#{format}; charset=utf-8"})
end

it "should get the correct resource" do
@client.approve_user(4)
a_post("users/4/relationship.#{format}").
with(:body => {:action => "approve", :access_token => @client.access_token}).
should have_been_made
end

it "should return a relationship status response" do
status = @client.approve_user(4)
status.outgoing_status.should == "follows"
end
end

describe ".deny_user" do

before do
stub_post("users/4/relationship.#{format}").
with(:body => {:action => "deny", :access_token => @client.access_token}).
to_return(:body => fixture("deny_user.#{format}"), :headers => {:content_type => "application/#{format}; charset=utf-8"})
end

it "should get the correct resource" do
@client.deny_user(4)
a_post("users/4/relationship.#{format}").
with(:body => {:action => "deny", :access_token => @client.access_token}).
should have_been_made
end

it "should return a relationship status response" do
status = @client.deny_user(4)
status.outgoing_status.should == "none"
Expand Down
28 changes: 28 additions & 0 deletions spec/instagram/response_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
require File.expand_path('../../spec_helper', __FILE__)

describe Instagram::Response do
Instagram::Configuration::VALID_FORMATS.each do |format|
context ".new(:format => '#{format}')" do
before do
@client = Instagram::Client.new(:format => format, :client_id => 'CID', :client_secret => 'CS', :access_token => 'AT')
end

context 'to a standard request' do
before do
stub_get("users/4.#{format}").
with(:query => {:access_token => @client.access_token}).
to_return(:body => fixture("mikeyk.#{format}"), :headers => {:content_type => "application/#{format}; charset=utf-8",
'x-ratelimit-limit' => '5000',
'x-ratelimit-remaining' => '4999'})
end

it 'should provide rate limit information on every object returned' do
user = @client.user(4)
user.ratelimit.should_not be_nil
user.ratelimit.limit.should == 5000
user.ratelimit.remaining.should == 4999
end
end
end
end
end