diff --git a/sentry_sdk/integrations/django/transactions.py b/sentry_sdk/integrations/django/transactions.py index 91349c4bf9..b2e200b832 100644 --- a/sentry_sdk/integrations/django/transactions.py +++ b/sentry_sdk/integrations/django/transactions.py @@ -1,6 +1,8 @@ """ -Copied from raven-python. Used for -`DjangoIntegration(transaction_fron="raven_legacy")`. +Copied from raven-python. + +Despite being called "legacy" in some places this resolver is very much still +in use. """ from __future__ import absolute_import @@ -19,6 +21,13 @@ from typing import Union from re import Pattern +from django import VERSION as DJANGO_VERSION + +if DJANGO_VERSION >= (2, 0): + from django.urls.resolvers import RoutePattern +else: + RoutePattern = None + try: from django.urls import get_resolver except ImportError: @@ -36,6 +45,9 @@ def get_regex(resolver_or_pattern): class RavenResolver(object): + _new_style_group_matcher = re.compile( + r"<(?:([^>:]+):)?([^>]+)>" + ) # https://github.com/django/django/blob/21382e2743d06efbf5623e7c9b6dccf2a325669b/django/urls/resolvers.py#L245-L247 _optional_group_matcher = re.compile(r"\(\?\:([^\)]+)\)") _named_group_matcher = re.compile(r"\(\?P<(\w+)>[^\)]+\)+") _non_named_group_matcher = re.compile(r"\([^\)]+\)") @@ -46,7 +58,7 @@ class RavenResolver(object): _cache = {} # type: Dict[URLPattern, str] def _simplify(self, pattern): - # type: (str) -> str + # type: (Union[URLPattern, URLResolver]) -> str r""" Clean up urlpattern regexes into something readable by humans: @@ -56,11 +68,24 @@ def _simplify(self, pattern): To: > "{sport_slug}/athletes/{athlete_slug}/" """ + # "new-style" path patterns can be parsed directly without turning them + # into regexes first + if ( + RoutePattern is not None + and hasattr(pattern, "pattern") + and isinstance(pattern.pattern, RoutePattern) + ): + return self._new_style_group_matcher.sub( + lambda m: "{%s}" % m.group(2), pattern.pattern._route + ) + + result = get_regex(pattern).pattern + # remove optional params # TODO(dcramer): it'd be nice to change these into [%s] but it currently # conflicts with the other rules because we're doing regexp matches # rather than parsing tokens - result = self._optional_group_matcher.sub(lambda m: "%s" % m.group(1), pattern) + result = self._optional_group_matcher.sub(lambda m: "%s" % m.group(1), result) # handle named groups first result = self._named_group_matcher.sub(lambda m: "{%s}" % m.group(1), result) @@ -113,8 +138,8 @@ def _resolve(self, resolver, path, parents=None): except KeyError: pass - prefix = "".join(self._simplify(get_regex(p).pattern) for p in parents) - result = prefix + self._simplify(get_regex(pattern).pattern) + prefix = "".join(self._simplify(p) for p in parents) + result = prefix + self._simplify(pattern) if not result.startswith("/"): result = "/" + result self._cache[pattern] = result diff --git a/tests/integrations/django/test_transactions.py b/tests/integrations/django/test_transactions.py index 4c94a2c955..c9914c8ec5 100644 --- a/tests/integrations/django/test_transactions.py +++ b/tests/integrations/django/test_transactions.py @@ -3,47 +3,55 @@ import pytest import django +try: + from unittest import mock # python 3.3 and above +except ImportError: + import mock # python < 3.3 + + +# django<2.0 has only `url` with regex based patterns. +# django>=2.0 renames `url` to `re_path`, and additionally introduces `path` +# for new style URL patterns, e.g. . if django.VERSION >= (2, 0): - # TODO: once we stop supporting django < 2, use the real name of this - # function (re_path) - from django.urls import re_path as url + from django.urls import path, re_path + from django.urls.converters import PathConverter from django.conf.urls import include else: - from django.conf.urls import url, include + from django.conf.urls import url as re_path, include if django.VERSION < (1, 9): - included_url_conf = (url(r"^foo/bar/(?P[\w]+)", lambda x: ""),), "", "" + included_url_conf = (re_path(r"^foo/bar/(?P[\w]+)", lambda x: ""),), "", "" else: - included_url_conf = ((url(r"^foo/bar/(?P[\w]+)", lambda x: ""),), "") + included_url_conf = ((re_path(r"^foo/bar/(?P[\w]+)", lambda x: ""),), "") from sentry_sdk.integrations.django.transactions import RavenResolver example_url_conf = ( - url(r"^api/(?P[\w_-]+)/store/$", lambda x: ""), - url(r"^api/(?P(v1|v2))/author/$", lambda x: ""), - url( + re_path(r"^api/(?P[\w_-]+)/store/$", lambda x: ""), + re_path(r"^api/(?P(v1|v2))/author/$", lambda x: ""), + re_path( r"^api/(?P[^\/]+)/product/(?P(?:\d+|[A-Fa-f0-9-]{32,36}))/$", lambda x: "", ), - url(r"^report/", lambda x: ""), - url(r"^example/", include(included_url_conf)), + re_path(r"^report/", lambda x: ""), + re_path(r"^example/", include(included_url_conf)), ) -def test_legacy_resolver_no_match(): +def test_resolver_no_match(): resolver = RavenResolver() result = resolver.resolve("/foo/bar", example_url_conf) assert result is None -def test_legacy_resolver_complex_match(): +def test_resolver_re_path_complex_match(): resolver = RavenResolver() result = resolver.resolve("/api/1234/store/", example_url_conf) assert result == "/api/{project_id}/store/" -def test_legacy_resolver_complex_either_match(): +def test_resolver_re_path_complex_either_match(): resolver = RavenResolver() result = resolver.resolve("/api/v1/author/", example_url_conf) assert result == "/api/{version}/author/" @@ -51,13 +59,13 @@ def test_legacy_resolver_complex_either_match(): assert result == "/api/{version}/author/" -def test_legacy_resolver_included_match(): +def test_resolver_re_path_included_match(): resolver = RavenResolver() result = resolver.resolve("/example/foo/bar/baz", example_url_conf) assert result == "/example/foo/bar/{param}" -def test_capture_multiple_named_groups(): +def test_resolver_re_path_multiple_groups(): resolver = RavenResolver() result = resolver.resolve( "/api/myproject/product/cb4ef1caf3554c34ae134f3c1b3d605f/", example_url_conf @@ -65,21 +73,51 @@ def test_capture_multiple_named_groups(): assert result == "/api/{project_id}/product/{pid}/" -@pytest.mark.skipif(django.VERSION < (2, 0), reason="Requires Django > 2.0") -def test_legacy_resolver_newstyle_django20_urlconf(): - from django.urls import path - +@pytest.mark.skipif( + django.VERSION < (2, 0), + reason="Django>=2.0 required for patterns", +) +def test_resolver_path_group(): url_conf = (path("api/v2//store/", lambda x: ""),) resolver = RavenResolver() result = resolver.resolve("/api/v2/1234/store/", url_conf) assert result == "/api/v2/{project_id}/store/" -@pytest.mark.skipif(django.VERSION < (2, 0), reason="Requires Django > 2.0") -def test_legacy_resolver_newstyle_django20_urlconf_multiple_groups(): - from django.urls import path - - url_conf = (path("api/v2//product/", lambda x: ""),) +@pytest.mark.skipif( + django.VERSION < (2, 0), + reason="Django>=2.0 required for patterns", +) +def test_resolver_path_multiple_groups(): + url_conf = (path("api/v2//product/", lambda x: ""),) resolver = RavenResolver() - result = resolver.resolve("/api/v2/1234/product/5689", url_conf) + result = resolver.resolve("/api/v2/myproject/product/5689", url_conf) assert result == "/api/v2/{project_id}/product/{pid}" + + +@pytest.mark.skipif( + django.VERSION < (2, 0), + reason="Django>=2.0 required for patterns", +) +def test_resolver_path_complex_path(): + class CustomPathConverter(PathConverter): + regex = r"[^/]+(/[^/]+){0,2}" + + with mock.patch( + "django.urls.resolvers.get_converter", return_value=CustomPathConverter + ): + url_conf = (path("api/v3/", lambda x: ""),) + resolver = RavenResolver() + result = resolver.resolve("/api/v3/abc/def/ghi", url_conf) + assert result == "/api/v3/{my_path}" + + +@pytest.mark.skipif( + django.VERSION < (2, 0), + reason="Django>=2.0 required for patterns", +) +def test_resolver_path_no_converter(): + url_conf = (path("api/v4/", lambda x: ""),) + resolver = RavenResolver() + result = resolver.resolve("/api/v4/myproject", url_conf) + assert result == "/api/v4/{project_id}"