Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #123: simpler pagination with .pages() without first .get #131

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 13 additions & 2 deletions tapioca/tapioca.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,19 @@ def _reached_max_limits(self, page_count, item_count, max_pages,
reached_item_limit = max_items is not None and max_items <= item_count
return reached_page_limit or reached_item_limit

def pages(self, max_pages=None, max_items=None, **kwargs):
executor = self
def pages(self, max_pages=None, max_items=None, params=None, **kwargs):
if self._response is not None:
if params is not None:
raise Exception("Since you're paging after the first .get call, "
Copy link
Contributor

Choose a reason for hiding this comment

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

Just improving the message:
You cannot pass new params after the first request is done. Use the ".get" method to pass parameters

"you can't pass params here."
"Pass params on the .get call instead.")
executor = self
else:
# this mean .pages() was called before the first .get,
Copy link
Contributor

Choose a reason for hiding this comment

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

this means ...

# like `api.statuses_user_timeline().pages()`
response = self.get(params=params)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we simply pass all the kwargs?
Passing only the params will limit the options that can be passed in the request, a header for example.

executor = response()

iterator_list = executor._get_iterator_list()
page_count = 0
item_count = 0
Expand Down
3 changes: 2 additions & 1 deletion tests/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ def get_iterator_next_request_kwargs(self, iterator_request_kwargs,
url = paging.get('next')

if url:
return {'url': url}
iterator_request_kwargs['url'] = url
return iterator_request_kwargs


TesterClient = generate_wrapper_from_adapter(TesterClientAdapter)
Expand Down
94 changes: 94 additions & 0 deletions tests/test_tapioca.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,14 @@ def test_simple_pages_iterator(self):

self.assertEqual(iterations_count, 2)

responses.mock.calls.reset()
iterations_count = 0
for item in self.wrapper.test().pages():
self.assertIn(item.key().data, 'value')
iterations_count += 1

self.assertEqual(iterations_count, 2)

@responses.activate
def test_simple_pages_with_max_items_iterator(self):
next_url = 'http://api.teste.com/next_batch'
Expand All @@ -356,6 +364,14 @@ def test_simple_pages_with_max_items_iterator(self):

self.assertEqual(iterations_count, 3)

responses.mock.calls.reset()
iterations_count = 0
for item in self.wrapper.test().pages(max_items=3, max_pages=2):
self.assertIn(item.key().data, 'value')
iterations_count += 1

self.assertEqual(iterations_count, 3)

@responses.activate
def test_simple_pages_with_max_pages_iterator(self):
next_url = 'http://api.teste.com/next_batch'
Expand Down Expand Up @@ -388,6 +404,14 @@ def test_simple_pages_with_max_pages_iterator(self):

self.assertEqual(iterations_count, 7)

responses.mock.calls.reset()
iterations_count = 0
for item in self.wrapper.test().pages(max_pages=3):
self.assertIn(item.key().data, 'value')
iterations_count += 1

self.assertEqual(iterations_count, 7)

@responses.activate
def test_simple_pages_max_page_zero_iterator(self):
next_url = 'http://api.teste.com/next_batch'
Expand All @@ -411,6 +435,14 @@ def test_simple_pages_max_page_zero_iterator(self):

self.assertEqual(iterations_count, 0)

responses.mock.calls.reset()
iterations_count = 0
for item in self.wrapper.test().pages(max_pages=0):
self.assertIn(item.key().data, 'value')
iterations_count += 1

self.assertEqual(iterations_count, 0)

@responses.activate
def test_simple_pages_max_item_zero_iterator(self):
next_url = 'http://api.teste.com/next_batch'
Expand All @@ -434,6 +466,68 @@ def test_simple_pages_max_item_zero_iterator(self):

self.assertEqual(iterations_count, 0)

responses.mock.calls.reset()
iterations_count = 0
for item in self.wrapper.test().pages(max_items=0):
self.assertIn(item.key().data, 'value')
iterations_count += 1

self.assertEqual(iterations_count, 0)

@responses.activate
def test_cant_pass_param_after_first_get(self):
next_url = 'http://api.teste.com/next_batch'

responses.add(responses.GET, self.wrapper.test().data,
body='{"data": [{"key": "value"}], "paging": {"next": "%s"}}' % next_url,
status=200,
content_type='application/json')

responses.add(responses.GET, next_url,
body='{"data": [{"key": "value"}], "paging": {"next": ""}}',
status=200,
content_type='application/json')

with self.assertRaises(Exception) as exception_cm:
response = self.wrapper.test().get()
for item in response().pages(params={'param-key': 'param-value'}):
break

self.assertEqual(str(exception_cm.exception),
"Since you're paging after the first .get call, "
"you can't pass params here."
"Pass params on the .get call instead.")

def test_params_are_kept_between_pages_gets(self):
def add_responses(rsps):
next_url = 'http://api.teste.com/next_batch'

rsps.add(responses.GET, self.wrapper.test().data + '?keep-me=please',
body='{"data": [{"key": "value"}], "paging": {"next": "%s"}}' % next_url,
status=200,
content_type='application/json',
match_querystring=True)

rsps.add(responses.GET, next_url + '?keep-me=please',
body='{"data": [{"key": "value"}], "paging": {"next": ""}}',
status=200,
content_type='application/json',
match_querystring=True)

with responses.RequestsMock(assert_all_requests_are_fired=True) as rsps:
add_responses(rsps)

response = self.wrapper.test().get(params={'keep-me': 'please'})

for item in response().pages():
self.assertIn(item.key().data, 'value')

with responses.RequestsMock(assert_all_requests_are_fired=True) as rsps:
add_responses(rsps)

for item in self.wrapper.test().pages(params={'keep-me': 'please'}):
self.assertIn(item.key().data, 'value')


class TestTokenRefreshing(unittest.TestCase):

Expand Down