From 28455c901b3d30a701329102424608d42d9aebff Mon Sep 17 00:00:00 2001 From: Anton Shutik Date: Wed, 30 Oct 2024 17:31:42 +0100 Subject: [PATCH 1/3] Renamed query attr, moved hook, tests --- shopify_client/__init__.py | 21 +++------------ shopify_client/graphql.py | 6 ++--- shopify_client/hooks.py | 18 +++++++++++++ tests/conftest.py | 31 ++++++++++++++++++++++- tests/test_client.py | 15 +---------- tests/test_endpoint.py | 10 -------- tests/test_grapgql.py | 21 +++++---------- tests/test_hooks.py | 52 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 115 insertions(+), 59 deletions(-) create mode 100644 shopify_client/hooks.py create mode 100644 tests/test_hooks.py diff --git a/shopify_client/__init__.py b/shopify_client/__init__.py index 5d43d27..f84db18 100644 --- a/shopify_client/__init__.py +++ b/shopify_client/__init__.py @@ -4,6 +4,8 @@ import requests +from shopify_client.hooks import rate_limit + from .endpoint import DraftOrdersEndpoint, Endpoint, OrdersEndpoint from .graphql import GraphQL @@ -107,23 +109,8 @@ def __init__(self, api_url, api_token, api_version=SHOPIFY_API_VERSION): self.webhooks = Endpoint(client=self, endpoint="webhooks") # GraphQL - self.graphql = GraphQL(client=self) - - def rate_limit(response, *args, **kwargs): - max_retry_count = 5 - retry_count = int(response.request.headers.get("X-Retry-Count", 0)) - - if response.status_code == 429 and retry_count < max_retry_count: - retry_after = int(response.headers.get("retry-after", 4)) - logger.warning(f"Shopify service exceeds API call limit; will retry request in {retry_after} seconds") - time.sleep(retry_after) - response.request.headers["X-Retry-Count"] = retry_count + 1 - new_response = response.connection.send(response.request) - new_response.history.append(response) - return rate_limit(new_response, *args, **kwargs) - - return response - + self.query = GraphQL(client=self) + self.hooks["response"].append(rate_limit) def request(self, method, url, *args, **kwargs): diff --git a/shopify_client/graphql.py b/shopify_client/graphql.py index 392bebb..28e6156 100644 --- a/shopify_client/graphql.py +++ b/shopify_client/graphql.py @@ -16,9 +16,9 @@ def __build_url(self, **params): return self.endpoint def __call__(self, *args, **kwargs): - return self.query(*args, **kwargs) + return self.__query(*args, **kwargs) - def query(self, query, variables=None, operation_name=None, paginate=False): + def __query(self, query, variables=None, operation_name=None, paginate=False): if paginate: return self.__paginate(query, variables, operation_name) try: @@ -45,7 +45,7 @@ def __paginate(self, query, variables=None, operation_name=None, page_size=100): while has_next_page: variables["cursor"] = cursor - response = self.query(query, variables, operation_name) + response = self.__query(query, variables, operation_name) page_info = self.__find_page_info(response) has_next_page = page_info.get("hasNextPage", False) cursor = page_info.get("endCursor", None) diff --git a/shopify_client/hooks.py b/shopify_client/hooks.py new file mode 100644 index 0000000..2671690 --- /dev/null +++ b/shopify_client/hooks.py @@ -0,0 +1,18 @@ +import time + + +def rate_limit(response, *args, **kwargs): + max_retry_count = 5 + retry_count = int(response.request.headers.get("X-Retry-Count", 0)) + print(f"Response: {response}") + + if response.status_code == 429 and retry_count < max_retry_count: + retry_after = int(response.headers.get("retry-after", 4)) + # print(f"Retry after: {retry_after}") + time.sleep(retry_after) + response.request.headers["X-Retry-Count"] = retry_count + 1 + new_response = response.connection.send(response.request) + new_response.history.append(response) + return rate_limit(new_response, *args, **kwargs) + + return response \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index 55adad4..4f14997 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1 +1,30 @@ -from .test_endpoint import mock_client \ No newline at end of file +from copy import deepcopy +from unittest.mock import Mock + +from shopify_client import ShopifyClient +from shopify_client.endpoint import Endpoint +import pytest + + +@pytest.fixture +def mock_client(mocker): + client = mocker.Mock() + client.parse_response.side_effect = lambda x: x # Just return the response data as-is + return client + +@pytest.fixture +def endpoint(mock_client): + return Endpoint(client=mock_client, endpoint="test_endpoint") + + +@pytest.fixture +def shopify_client(mocker): + return ShopifyClient(api_url="https://test-shop.myshopify.com", api_token="test-token") + +# Create a new mock that will deepcopy the arguments passed to it + # https://docs.python.org/3.7/library/unittest.mock-examples.html#coping-with-mutable-arguments +class CopyingMock(Mock): + def __call__(self, *args, **kwargs): + args = deepcopy(args) + kwargs = deepcopy(kwargs) + return super().__call__(*args, **kwargs) \ No newline at end of file diff --git a/tests/test_client.py b/tests/test_client.py index 6264b31..bfde93e 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,10 +1,7 @@ import requests import pytest from shopify_client import ShopifyClient - -@pytest.fixture -def shopify_client(mocker): - return ShopifyClient(api_url="https://test-shop.myshopify.com", api_token="test-token") +from tests.conftest import CopyingMock def test_client_initialization(shopify_client): assert shopify_client.headers["X-Shopify-Access-Token"] == "test-token" @@ -19,16 +16,6 @@ def test_request_with_versioning(shopify_client, mocker): "https://test-shop.myshopify.com/admin/api/2024-10/products.json" ) -# def test_rate_limit_handling(shopify_client, mocker): -# rate_limit_mock = mocker.Mock(status_code=429, headers={"retry-after": "2"}) -# successful_mock = mocker.Mock(status_code=200, json=lambda: {"result": "success"}) -# mock_request = mocker.patch("requests.Session.request", side_effect=[rate_limit_mock, successful_mock]) - -# response = shopify_client.request("GET", "products.json") - -# assert response.json() == {"result": "success"} -# assert mock_request.call_count == 2 - def test_parse_response_success(shopify_client, mocker): mock_response = mocker.Mock(status_code=200, json=lambda: {"key": "value"}) parsed = shopify_client.parse_response(mock_response) diff --git a/tests/test_endpoint.py b/tests/test_endpoint.py index dd26258..4d5a2ee 100644 --- a/tests/test_endpoint.py +++ b/tests/test_endpoint.py @@ -2,16 +2,6 @@ import pytest from shopify_client import Endpoint, OrdersEndpoint, DraftOrdersEndpoint -@pytest.fixture -def mock_client(mocker): - client = mocker.Mock() - client.parse_response.side_effect = lambda x: x # Just return the response data as-is - return client - -@pytest.fixture -def endpoint(mock_client): - return Endpoint(client=mock_client, endpoint="test_endpoint") - def test_prepare_params(endpoint): params = { "simple": "value", diff --git a/tests/test_grapgql.py b/tests/test_grapgql.py index c56385b..6647054 100644 --- a/tests/test_grapgql.py +++ b/tests/test_grapgql.py @@ -4,6 +4,7 @@ import requests import pytest from shopify_client.graphql import GraphQL +from tests.conftest import CopyingMock @pytest.fixture def graphql(mock_client): @@ -11,14 +12,14 @@ def graphql(mock_client): def test_graphql_query(graphql, mock_client): mock_client.post.return_value = {"data": {"key": "value"}} - response = graphql.query(query="query { key }") + response = graphql(query="query { key }") mock_client.post.assert_called_once_with("graphql.json", json={"query": "query { key }", "variables": None, "operationName": None}) assert response == {"data": {"key": "value"}} def test_graphql_query_with_variables(graphql, mock_client): mock_client.post.return_value = {"data": {"key": "value"}} variables = {"var1": "value1"} - response = graphql.query(query="query { key }", variables=variables) + response = graphql(query="query { key }", variables=variables) mock_client.post.assert_called_once_with("graphql.json", json={"query": "query { key }", "variables": variables, "operationName": None}) assert response == {"data": {"key": "value"}} @@ -27,19 +28,19 @@ def test_query_paginated(graphql, mock_client): {"data": {"pageInfo": {"hasNextPage": True, "endCursor": "cursor1"}}}, {"data": {"pageInfo": {"hasNextPage": False}}} ] - results = list(graphql.query(query="query { pageInfo { hasNextPage, endCursor } }", paginate=True)) + results = list(graphql(query="query { pageInfo { hasNextPage, endCursor } }", paginate=True)) assert len(results) == 2 assert results[0] == {"data": {"pageInfo": {"hasNextPage": True, "endCursor": "cursor1"}}} assert results[1] == {"data": {"pageInfo": {"hasNextPage": False}}} def test_query_handles_http_error(graphql, mock_client, mocker): mock_client.post.side_effect = requests.exceptions.HTTPError("HTTP Error") - response = graphql.query(query="query { key }") + response = graphql(query="query { key }") assert response == {} def test_query_handles_json_error(graphql, mock_client): mock_client.post.side_effect = json.JSONDecodeError("JSON Decode Error", "", 0) - response = graphql.query(query="query { key }") + response = graphql(query="query { key }") assert response == {} def test_graphql_call(graphql, mock_client): @@ -76,16 +77,8 @@ def test_graphql_query_paginated(graphql, mock_client, mocker): } } - # Create a new mock that will deepcopy the arguments passed to it - # https://docs.python.org/3.7/library/unittest.mock-examples.html#coping-with-mutable-arguments - class CopyingMock(mocker.MagicMock): - def __call__(self, *args, **kwargs): - args = deepcopy(args) - kwargs = deepcopy(kwargs) - return super(CopyingMock, self).__call__(*args, **kwargs) - mock_client.post = CopyingMock(side_effect = [mock_paginated_response_1, mock_paginated_response_2]) - response = list(graphql.query(query="query { items { id } pageInfo { hasNextPage, endCursor } }", paginate=True)) + response = list(graphql(query="query { items { id } pageInfo { hasNextPage, endCursor } }", paginate=True)) assert response == [mock_paginated_response_1, mock_paginated_response_2] assert mock_client.post.call_count == 2 diff --git a/tests/test_hooks.py b/tests/test_hooks.py new file mode 100644 index 0000000..d9ad769 --- /dev/null +++ b/tests/test_hooks.py @@ -0,0 +1,52 @@ +from unittest.mock import MagicMock +import requests + +from shopify_client.hooks import rate_limit +import pytest + +@pytest.fixture +def mock_connection(): + """Fixture to create a mock connection object.""" + return MagicMock() + +@pytest.fixture +def initial_response(mock_connection): + """Fixture to create an initial response object.""" + response = requests.Response() + response.status_code = 429 + response.headers = {"retry-after": "2"} + response.request = requests.Request("GET", "https://api.example.com/resource").prepare() + response.request.headers["X-Retry-Count"] = 0 + response.connection = mock_connection + return response + +def test_rate_limit_retries_on_429(monkeypatch, initial_response, mock_connection): + """Test that the rate_limit function retries on a 429 response.""" + new_response = requests.Response() + new_response.status_code = 200 # Successful response to stop the retry loop + new_response.request = initial_response.request + mock_connection.send.return_value = new_response + + monkeypatch.setattr("time.sleep", lambda x: None) + + final_response = rate_limit(initial_response) + + assert final_response.status_code == 200 + assert mock_connection.send.call_count == 1 + assert final_response.request.headers["X-Retry-Count"] == 1 + +def test_rate_limit_stops_after_max_retries(monkeypatch, initial_response, mock_connection): + failed_response = requests.Response() + failed_response.status_code = 429 + failed_response.request = initial_response.request + failed_response.connection = mock_connection + + mock_connection.send.return_value = failed_response + + monkeypatch.setattr("time.sleep", lambda x: None) + + final_response = rate_limit(initial_response) + + assert final_response.status_code == 429 + assert mock_connection.send.call_count == 5 + assert final_response.request.headers["X-Retry-Count"] == 5 \ No newline at end of file From b4486cd1df0ef085839a558dc805e9c0a086b3d8 Mon Sep 17 00:00:00 2001 From: Anton Shutik Date: Wed, 30 Oct 2024 17:32:26 +0100 Subject: [PATCH 2/3] Updated README --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0b1d4c8..64d0313 100644 --- a/README.md +++ b/README.md @@ -73,10 +73,10 @@ query products($page_size: Int = 100) { } } ''' -response = client.graphql.query(query) +response = client.query(query) # Limit page size -response = client.graphql.query(query, variables={"page_size": 20}) +response = client.query(query, variables={"page_size": 20}) # Use pagination. # Note that "pageIngo" block with at least "hasNextPage" & "startCursor" is required @@ -95,7 +95,7 @@ query products($page_size: Int = 100, $cursor: String) { } } ''' -for page in client.graphql.query_paginated(query) +for page in client.query(query, paginate=True) print(page) ``` From c50c3edb17d44be5392133c68f6bc8112882099c Mon Sep 17 00:00:00 2001 From: Anton Shutik Date: Wed, 30 Oct 2024 17:36:53 +0100 Subject: [PATCH 3/3] fix: typo --- tests/{test_grapgql.py => test_graphql.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{test_grapgql.py => test_graphql.py} (100%) diff --git a/tests/test_grapgql.py b/tests/test_graphql.py similarity index 100% rename from tests/test_grapgql.py rename to tests/test_graphql.py