Skip to content

Commit adf106f

Browse files
APIClient::item_iterator: reset query_parameters after returning all items
The item_iterator method uses a query parameter called `since` to get the next results from the API resource when the previous response includes a hasMore=True property. The problem here is the last API call leaves the query parameter defined to a value related to the previous resource retrieved, and if you use the same api client to iterate over different resources (i.e. campaigns), the results will always be affected by the previous call.
1 parent 5a90bae commit adf106f

File tree

2 files changed

+27
-1
lines changed

2 files changed

+27
-1
lines changed

tests.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import logging
2+
from unittest.mock import call
23
import requests
34
import usabilla as ub
45

@@ -124,18 +125,41 @@ def test_item_iterator(self):
124125
items = ['one', 'two', 'three', 'four']
125126
has_more = {'hasMore': True, 'items': items[:2], 'lastTimestamp': 1400000000001}
126127
no_more = {'hasMore': False, 'items': items[2:], 'lastTimestamp': 1400000000002}
128+
127129
self.client.set_query_parameters = Mock()
128130
self.client.send_signed_request = Mock(side_effect=[has_more, no_more])
131+
129132
index = 0
130133
for item in self.client.item_iterator('/some/url'):
131134
self.assertEqual(item, items[index])
132135
index += 1
133-
self.client.set_query_parameters.assert_called_with({'since': 1400000000002})
136+
137+
self.client.set_query_parameters.assert_any_call({'since': 1400000000001})
138+
self.client.set_query_parameters.assert_any_call({'since': 1400000000002})
139+
self.client.set_query_parameters.assert_any_call({})
134140
self.assertEqual(self.client.send_signed_request.call_count, 2)
141+
135142
self.client.send_signed_request.side_effect = requests.exceptions.HTTPError('mocked error')
136143
with self.assertRaises(requests.exceptions.HTTPError):
137144
list(self.client.item_iterator('/some/url'))
138145

146+
def test_item_iterator_resets_query_parameters_after_returning_all_items(self):
147+
first_response = {'hasMore': True, 'items': [1], 'lastTimestamp': 1400000000001}
148+
second_response = {'hasMore': False, 'items': [2], 'lastTimestamp': 1400000000002}
149+
third_response = {'hasMore': False, 'items': [3], 'lastTimestamp': 1400000000003}
150+
151+
self.client.send_signed_request = Mock(side_effect=[first_response, second_response, third_response])
152+
153+
expected_query_parameters = ['', 'since=1400000000001', '']
154+
155+
index = 0
156+
for response in [self.client.item_iterator('/some/url'), self.client.item_iterator('/some/url')]:
157+
for _ in response:
158+
self.assertEqual(expected_query_parameters[index], self.client.get_query_parameters())
159+
index+=1
160+
161+
self.assertEqual(self.client.send_signed_request.call_count, 3)
162+
139163
def test_get_resource(self):
140164
self.client.item_iterator = Mock()
141165
self.client.send_signed_request = Mock()

usabilla.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,8 @@ def item_iterator(self, url):
311311
yield item
312312
self.set_query_parameters({'since': results['lastTimestamp']})
313313

314+
self.set_query_parameters({})
315+
314316
def get_resource(self, scope, product, resource, resource_id=None, iterate=False):
315317
"""Retrieves resources of the specified type
316318

0 commit comments

Comments
 (0)