From d8ff5eb66916620b64c5fa5f207846e8f2d5f51e Mon Sep 17 00:00:00 2001 From: Ricardo Fontanelli Date: Fri, 21 Oct 2022 13:10:18 +0200 Subject: [PATCH] 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. --- CHANGES.md | 5 +++++ setup.py | 2 +- tests.py | 25 ++++++++++++++++++++++++- usabilla.py | 2 ++ 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index ff296e5..dfd29ad 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,11 @@ Changelog ========= +Version 2.0.2 +------------- + +- Reset query arguments after returning all items from generator + Here you find a full list of changes. Version 2.0.1 diff --git a/setup.py b/setup.py index abc3b90..85c913f 100644 --- a/setup.py +++ b/setup.py @@ -3,7 +3,7 @@ except ImportError: from distutils.core import setup -VERSION = '2.0.1' +VERSION = '2.0.2' setup( name='usabilla-api', diff --git a/tests.py b/tests.py index 239adb3..a163f5c 100644 --- a/tests.py +++ b/tests.py @@ -1,4 +1,5 @@ import logging +from unittest.mock import call import requests import usabilla as ub @@ -124,18 +125,40 @@ def test_item_iterator(self): items = ['one', 'two', 'three', 'four'] has_more = {'hasMore': True, 'items': items[:2], 'lastTimestamp': 1400000000001} no_more = {'hasMore': False, 'items': items[2:], 'lastTimestamp': 1400000000002} + expected_set_query_parameters_calls = [ call({'since': 1400000000001}), call({'since': 1400000000002}), call({}) ] + self.client.set_query_parameters = Mock() self.client.send_signed_request = Mock(side_effect=[has_more, no_more]) + index = 0 for item in self.client.item_iterator('/some/url'): self.assertEqual(item, items[index]) index += 1 - self.client.set_query_parameters.assert_called_with({'since': 1400000000002}) + + self.assertEqual(self.client.set_query_parameters.call_args_list, expected_set_query_parameters_calls) self.assertEqual(self.client.send_signed_request.call_count, 2) + self.client.send_signed_request.side_effect = requests.exceptions.HTTPError('mocked error') with self.assertRaises(requests.exceptions.HTTPError): list(self.client.item_iterator('/some/url')) + def test_item_iterator_resets_query_parameters_after_returning_all_items(self): + first_response = {'hasMore': True, 'items': [1], 'lastTimestamp': 1400000000001} + second_response = {'hasMore': False, 'items': [2], 'lastTimestamp': 1400000000002} + third_response = {'hasMore': False, 'items': [3], 'lastTimestamp': 1400000000003} + + self.client.send_signed_request = Mock(side_effect=[first_response, second_response, third_response]) + + expected_query_parameters = ['', 'since=1400000000001', ''] + + index = 0 + for response in [self.client.item_iterator('/some/url'), self.client.item_iterator('/some/url')]: + for _ in response: + self.assertEqual(expected_query_parameters[index], self.client.get_query_parameters()) + index+=1 + + self.assertEqual(self.client.send_signed_request.call_count, 3) + def test_get_resource(self): self.client.item_iterator = Mock() self.client.send_signed_request = Mock() diff --git a/usabilla.py b/usabilla.py index 3632ef6..46dea62 100644 --- a/usabilla.py +++ b/usabilla.py @@ -311,6 +311,8 @@ def item_iterator(self, url): yield item self.set_query_parameters({'since': results['lastTimestamp']}) + self.set_query_parameters({}) + def get_resource(self, scope, product, resource, resource_id=None, iterate=False): """Retrieves resources of the specified type