Skip to content

Commit

Permalink
Fixes and improvements for Django (#15)
Browse files Browse the repository at this point in the history
* Fixes

* Add test for Django 2.2

* Fix accessing response header in Django 2.2

* Fix tests

* Update djangorestframework version constraint
  • Loading branch information
itssimon authored Mar 18, 2024
1 parent 1e2e959 commit 3670539
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 30 deletions.
1 change: 1 addition & 0 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ jobs:
- djangorestframework django
- djangorestframework django==4.2.*
- djangorestframework==3.12.* django==3.2.*
- djangorestframework==3.10.* django==2.2.*
- django-ninja django
- django-ninja==0.22.* django
- django-ninja==0.18.0 django
Expand Down
23 changes: 14 additions & 9 deletions apitally/django.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from django.conf import settings
from django.core.exceptions import ViewDoesNotExist
from django.test import RequestFactory
from django.urls import URLPattern, URLResolver, get_resolver, resolve
from django.urls import Resolver404, URLPattern, URLResolver, get_resolver, resolve
from django.utils.module_loading import import_string

from apitally.client.threading import ApitallyClient
Expand Down Expand Up @@ -41,12 +41,15 @@ def __init__(self, get_response: Callable[[HttpRequest], HttpResponse]) -> None:
config = getattr(settings, "APITALLY_MIDDLEWARE", {})
self.configure(**config)
assert self.config is not None
self.views = _extract_views_from_url_patterns(get_resolver().url_patterns)
views = _extract_views_from_url_patterns(get_resolver().url_patterns)
self.view_lookup = {
view.pattern: view for view in reversed(_extract_views_from_url_patterns(get_resolver().url_patterns))
}
self.client = ApitallyClient(client_id=self.config.client_id, env=self.config.env)
self.client.start_sync_loop()
self.client.set_app_info(
app_info=_get_app_info(
views=self.views,
views=views,
app_version=self.config.app_version,
openapi_url=self.config.openapi_url,
)
Expand Down Expand Up @@ -84,7 +87,7 @@ def __call__(self, request: HttpRequest) -> HttpResponse:
status_code=response.status_code,
response_time=time.perf_counter() - start_time,
request_size=request.headers.get("Content-Length"),
response_size=response.headers.get("Content-Length") # type: ignore[attr-defined]
response_size=response["Content-Length"]
if response.has_header("Content-Length")
else (len(response.content) if not response.streaming else None),
)
Expand All @@ -108,8 +111,11 @@ def __call__(self, request: HttpRequest) -> HttpResponse:
return response

def get_view(self, request: HttpRequest) -> Optional[DjangoViewInfo]:
resolver_match = resolve(request.path_info)
return next((view for view in self.views if view.pattern == resolver_match.route), None)
try:
resolver_match = resolve(request.path_info)
return self.view_lookup.get(resolver_match.route)
except Resolver404: # pragma: no cover
return None

def get_consumer(self, request: HttpRequest) -> Optional[str]:
if hasattr(request, "consumer_identifier"):
Expand Down Expand Up @@ -165,8 +171,7 @@ def _get_app_info(
app_info: Dict[str, Any] = {}
if openapi := _get_openapi(views, openapi_url):
app_info["openapi"] = openapi
if paths := _get_paths(views):
app_info["paths"] = paths
app_info["paths"] = _get_paths(views)
app_info["versions"] = _get_versions(app_version)
app_info["client"] = "python:django"
return app_info
Expand Down Expand Up @@ -255,7 +260,7 @@ def _get_versions(app_version: Optional[str]) -> Dict[str, str]:
"django": version("django"),
}
try:
versions["django-rest-framework"] = version("django-rest-framework")
versions["djangorestframework"] = version("djangorestframework")
except PackageNotFoundError: # pragma: no cover
pass
try:
Expand Down
2 changes: 1 addition & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ backoff = ">=2.0.0"
python = ">=3.8,<4.0"

# Optional dependencies, included in extras
django = { version = ">=4.0", optional = true }
django = { version = ">=2.2", optional = true }
django-ninja = { version = ">=0.18.0", optional = true }
djangorestframework = { version = ">=3.12.0", optional = true }
djangorestframework = { version = ">=3.10.0", optional = true }
fastapi = { version = ">=0.87.0", optional = true }
flask = { version = ">=2.0.0", optional = true }
httpx = { version = ">=0.22.0", optional = true }
Expand Down
22 changes: 13 additions & 9 deletions tests/test_django_ninja.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import json
import sys
from importlib.util import find_spec
from typing import TYPE_CHECKING, Optional

Expand All @@ -22,23 +23,23 @@ def identify_consumer(request: HttpRequest) -> Optional[str]:
return None


@pytest.fixture(scope="module")
def reset_modules() -> None:
for module in list(sys.modules):
if module.startswith("django.") or module.startswith("apitally."):
del sys.modules[module]


@pytest.fixture(scope="module", autouse=True)
def setup(module_mocker: MockerFixture) -> None:
def setup(reset_modules, module_mocker: MockerFixture) -> None:
import django
from django.apps.registry import apps
from django.conf import settings
from django.utils.functional import empty

module_mocker.patch("apitally.client.threading.ApitallyClient._instance", None)
module_mocker.patch("apitally.client.threading.ApitallyClient.start_sync_loop")
module_mocker.patch("apitally.client.threading.ApitallyClient.set_app_info")
module_mocker.patch("apitally.django.ApitallyMiddleware.config", None)

settings._wrapped = empty
apps.app_configs.clear()
apps.loading = False
apps.ready = False

settings.configure(
ROOT_URLCONF="tests.django_ninja_urls",
ALLOWED_HOSTS=["testserver"],
Expand All @@ -57,9 +58,12 @@ def setup(module_mocker: MockerFixture) -> None:


@pytest.fixture(scope="module")
def client() -> Client:
def client(module_mocker: MockerFixture) -> Client:
import django
from django.test import Client

if django.VERSION[0] < 3:
module_mocker.patch("django.test.client.Client.store_exc_info") # Simulate raise_request_exception=False
return Client(raise_request_exception=False)


Expand Down
22 changes: 13 additions & 9 deletions tests/test_django_rest_framework.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import sys
from importlib.util import find_spec
from typing import TYPE_CHECKING

Expand All @@ -14,23 +15,23 @@
from rest_framework.test import APIClient


@pytest.fixture(scope="module")
def reset_modules() -> None:
for module in list(sys.modules):
if module.startswith("django.") or module.startswith("rest_framework.") or module.startswith("apitally."):
del sys.modules[module]


@pytest.fixture(scope="module", autouse=True)
def setup(module_mocker: MockerFixture) -> None:
def setup(reset_modules, module_mocker: MockerFixture) -> None:
import django
from django.apps.registry import apps
from django.conf import settings
from django.utils.functional import empty

module_mocker.patch("apitally.client.threading.ApitallyClient._instance", None)
module_mocker.patch("apitally.client.threading.ApitallyClient.start_sync_loop")
module_mocker.patch("apitally.client.threading.ApitallyClient.set_app_info")
module_mocker.patch("apitally.django.ApitallyMiddleware.config", None)

settings._wrapped = empty
apps.app_configs.clear()
apps.loading = False
apps.ready = False

settings.configure(
ROOT_URLCONF="tests.django_rest_framework_urls",
ALLOWED_HOSTS=["testserver"],
Expand All @@ -53,9 +54,12 @@ def setup(module_mocker: MockerFixture) -> None:


@pytest.fixture(scope="module")
def client() -> APIClient:
def client(module_mocker: MockerFixture) -> APIClient:
import django
from rest_framework.test import APIClient

if django.VERSION[0] < 3:
module_mocker.patch("django.test.client.Client.store_exc_info") # Simulate raise_request_exception=False
return APIClient(raise_request_exception=False)


Expand Down

0 comments on commit 3670539

Please sign in to comment.