From 51fd816b94525769bd6ca50f2ddffe3fb7d77576 Mon Sep 17 00:00:00 2001 From: Eero Ruohola Date: Sun, 16 Jan 2022 17:17:53 +0200 Subject: [PATCH] Send query parameters in addition to path in middlewares --- CHANGELOG.md | 4 +++ README.md | 1 + apilytics/core.py | 6 +++++ apilytics/django.py | 3 ++- apilytics/fastapi.py | 1 + tests/django/test_django.py | 3 ++- tests/fastapi/test_fastapi.py | 3 ++- tests/test_core.py | 48 +++++++++++++++++++++++++++++++++++ 8 files changed, 66 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c7b2c3..2ce98d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Send query parameters in addition to the path. + ## [1.0.2] - 2022-01-16 ### Added diff --git a/README.md b/README.md index fe8e38c..57578e1 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,7 @@ def my_apilytics_middleware(request, get_response): with ApilyticsSender( api_key=api_key, path=request.path, + query=request.query_string, method=request.method, ) as sender: response = get_response(request) diff --git a/apilytics/core.py b/apilytics/core.py index fe318f9..dead062 100644 --- a/apilytics/core.py +++ b/apilytics/core.py @@ -27,6 +27,7 @@ class ApilyticsSender: with ApilyticsSender( api_key="", path=request.path, + query=request.query_string, method=request.method, ) as sender: response = get_response(request) @@ -45,6 +46,7 @@ def __init__( api_key: str, path: str, method: str, + query: Optional[str] = None, apilytics_integration: Optional[str] = None, integrated_library: Optional[str] = None, ) -> None: @@ -54,6 +56,8 @@ def __init__( Args: api_key: The API key for your Apilytics origin. path: Path of the user's HTTP request, e.g. "/foo/bar/123". + query: Optional query string of the user's HTTP request e.g. "key=val&other=123". + An empty string and None are treated equally. Can have an optional "?" at the start. method: Method of the user's HTTP request, e.g. "GET". apilytics_integration: Name of the Apilytics integration that's calling this, e.g. "apilytics-python-django". No need to pass this when calling from user code. @@ -63,6 +67,7 @@ def __init__( self._api_key = api_key self._path = path self._method = method + self._query = query self._status_code: Optional[int] = None self._apilytics_version = self._apilytics_version_template.format( @@ -115,6 +120,7 @@ def _send_metrics(self) -> None: ) data = { "path": self._path, + **({"query": self._query} if self._query else {}), "method": self._method, "statusCode": self._status_code, "timeMillis": (self._end_time_ns - self._start_time_ns) // 1_000_000, diff --git a/apilytics/django.py b/apilytics/django.py index ad626dd..b1c0c6f 100644 --- a/apilytics/django.py +++ b/apilytics/django.py @@ -42,7 +42,8 @@ def __call__(self, request: django.http.HttpRequest) -> django.http.HttpResponse with apilytics.core.ApilyticsSender( api_key=self.api_key, path=request.path, - method=request.method or "", + query=request.META.get("QUERY_STRING"), + method=request.method or "", # Typed as Optional, should never be None. apilytics_integration="apilytics-python-django", integrated_library=f"django/{django.__version__}", ) as sender: diff --git a/apilytics/fastapi.py b/apilytics/fastapi.py index 5d62007..821d2c2 100644 --- a/apilytics/fastapi.py +++ b/apilytics/fastapi.py @@ -51,6 +51,7 @@ async def dispatch( with apilytics.core.ApilyticsSender( api_key=self.api_key, path=request.url.path, + query=request.url.query, method=request.method, apilytics_integration="apilytics-python-fastapi", integrated_library=f"fastapi/{fastapi.__version__}", diff --git a/tests/django/test_django.py b/tests/django/test_django.py index 53f8168..c6a451e 100644 --- a/tests/django/test_django.py +++ b/tests/django/test_django.py @@ -43,7 +43,7 @@ def test_middleware_should_call_apilytics_api( assert isinstance(data["timeMillis"], int) -def test_middleware_should_not_send_query_params( +def test_middleware_should_send_query_params( mocked_urlopen: unittest.mock.MagicMock, ) -> None: client.handler.load_middleware() @@ -55,6 +55,7 @@ def test_middleware_should_not_send_query_params( data = tests.conftest.decode_request_data(call_kwargs["data"]) assert data["method"] == "POST" assert data["path"] == "/dummy/123/path/" + assert data["query"] == "param=foo¶m2=bar" assert data["statusCode"] == 201 assert isinstance(data["timeMillis"], int) diff --git a/tests/fastapi/test_fastapi.py b/tests/fastapi/test_fastapi.py index 16f33e5..fe28758 100644 --- a/tests/fastapi/test_fastapi.py +++ b/tests/fastapi/test_fastapi.py @@ -43,7 +43,7 @@ def test_middleware_should_call_apilytics_api( assert isinstance(data["timeMillis"], int) -def test_middleware_should_not_send_query_params( +def test_middleware_should_send_query_params( mocked_urlopen: unittest.mock.MagicMock, ) -> None: response = client.post("/dummy/123/path/?param=foo¶m2=bar") @@ -54,6 +54,7 @@ def test_middleware_should_not_send_query_params( data = tests.conftest.decode_request_data(call_kwargs["data"]) assert data["method"] == "POST" assert data["path"] == "/dummy/123/path/" + assert data["query"] == "param=foo¶m2=bar" assert data["statusCode"] == 201 assert isinstance(data["timeMillis"], int) diff --git a/tests/test_core.py b/tests/test_core.py index 7c89c7c..50f03ac 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -41,6 +41,54 @@ def test_apilytics_sender_should_call_apilytics_api( assert isinstance(data["timeMillis"], int) +def test_middleware_should_send_query_params( + mocked_urlopen: unittest.mock.MagicMock, +) -> None: + with apilytics.core.ApilyticsSender( + api_key="dummy-key", + path="/path", + query="key=value?other=123", + method="PUT", + ) as sender: + sender.set_response_info(status_code=200) + + assert mocked_urlopen.call_count == 1 + __, call_kwargs = mocked_urlopen.call_args + data = tests.conftest.decode_request_data(call_kwargs["data"]) + assert data["path"] == "/path" + assert data["query"] == "key=value?other=123" + + +def test_middleware_should_not_send_empty_query_params( + mocked_urlopen: unittest.mock.MagicMock, +) -> None: + with apilytics.core.ApilyticsSender( + api_key="dummy-key", + path="/", + query="", + method="GET", + ) as sender: + sender.set_response_info(status_code=200) + + assert mocked_urlopen.call_count == 1 + __, call_kwargs = mocked_urlopen.call_args + data = tests.conftest.decode_request_data(call_kwargs["data"]) + assert "query" not in data + + with apilytics.core.ApilyticsSender( + api_key="dummy-key", + path="/", + query=None, + method="GET", + ) as sender: + sender.set_response_info(status_code=200) + + assert mocked_urlopen.call_count == 2 + __, call_kwargs = mocked_urlopen.call_args + data = tests.conftest.decode_request_data(call_kwargs["data"]) + assert "query" not in data + + @unittest.mock.patch( "apilytics.core.urllib.request.urlopen", side_effect=urllib.error.URLError("testing"),