Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regex support added for ignore URL list in settings.py #116

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion demoproj/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
"""
from django.urls import path

from demoproj.views.sync_views import index_view, no_guid, rest_view
from demoproj.views.sync_views import index_view, no_guid, rest_view, no_guid_regex
from demoproj.views.async_views import index_view as asgi_index_view
from demoproj.views.async_views import django_guid_api_usage

urlpatterns = [
path('', index_view, name='index'),
path('api', rest_view, name='drf'),
path('no-guid', no_guid, name='no_guid'),
path('no-guid-regex', no_guid_regex, name='no_guid_regex'),
path('asgi', asgi_index_view, name='asgi_index'),
path('api-usage', django_guid_api_usage, name='django_guid_api_usage'),
]
9 changes: 9 additions & 0 deletions demoproj/views/sync_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ def no_guid(request: 'HttpRequest') -> JsonResponse:
return JsonResponse({'detail': f'It worked also! Useless function response is {useless_response}'})


def no_guid_regex(request: 'HttpRequest') -> JsonResponse:
"""
Example view with a URL in the IGNORE_REGEX_URLS list - no GUID will be in these logs
"""
logger.info('This log message should NOT have a GUID - the URL is in IGNORE_REGEX_URLS')
useless_response = useless_function()
return JsonResponse({'detail': f'It worked also! Useless function response is {useless_response}'})


@api_view(('GET',))
def rest_view(request: 'Request') -> Response:
"""
Expand Down
4 changes: 4 additions & 0 deletions django_guid/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ def expose_header(self) -> bool:
def ignore_urls(self) -> List[str]:
return list({url.strip('/') for url in self.settings.get('IGNORE_URLS', [])})

@property
def ignore_regex_urls(self) -> List[str]:
return list({url.strip('/') for url in self.settings.get('IGNORE_REGEX_URLS', [])})

@property
def validate_guid(self) -> bool:
return self.settings.get('VALIDATE_GUID', True)
Expand Down
6 changes: 3 additions & 3 deletions django_guid/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from django.core.exceptions import ImproperlyConfigured

from django_guid.context import guid
from django_guid.utils import get_id_from_header, ignored_url
from django_guid.utils import get_id_from_header, ignored_regex_url, ignored_url

try:
from django.utils.decorators import sync_and_async_middleware
Expand All @@ -28,7 +28,7 @@ def process_incoming_request(request: 'HttpRequest') -> None:
Processes an incoming request. This function is called before the view and later middleware.
Same logic for both async and sync views.
"""
if not ignored_url(request=request):
if (not ignored_url(request=request)) and (not ignored_regex_url(request=request)):
# Process request and store the GUID in a contextvar
guid.set(get_id_from_header(request))

Expand All @@ -42,7 +42,7 @@ def process_outgoing_request(response: 'HttpResponse', request: 'HttpRequest') -
"""
Process an outgoing request. This function is called after the view and before later middleware.
"""
if not ignored_url(request=request):
if (not ignored_url(request=request)) and (not ignored_regex_url(request=request)):
if settings.return_header:
response[settings.guid_header_name] = guid.get() # Adds the GUID to the response header
if settings.expose_header:
Expand Down
18 changes: 18 additions & 0 deletions django_guid/utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
import re
import uuid
from typing import TYPE_CHECKING, Optional, Union

Expand Down Expand Up @@ -91,3 +92,20 @@ def validate_guid(original_guid: str) -> bool:
return bool(uuid.UUID(original_guid, version=4).hex)
except ValueError:
return False


def ignored_regex_url(request: Union['HttpRequest', 'HttpResponse']) -> bool:
"""
Support for Regex added
Checks if the current URL is defined in the `IGNORE_REGEX_URLS` setting.

:return: Boolean
"""
endpoint = request.path.strip('/')

IGNORE_URLS = []
for url in settings.ignore_regex_urls:
url_regex = url.replace('*', r'[\s\S]*') # noqa
url_regex = '^' + url_regex + '$'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we make this a new setting, e.g. ignore_url_regex, we can also skip this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JonasKs Thanks for the suggestions. I have moved it to a new settings. But facing errors in CI pipeline.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m on vacation, but I’ll check when I’m back. 😊

IGNORE_URLS.append(re.compile(url_regex))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will compile every regex twice every requests. It seems like a waste of processing power, and we will not get the benefits of compiled regex since they will bê compiled every time they are needed.

I wonder if it is possible to compile tem just once, maybe in the middleware setup or at the ignore_regex_urls settings.

return any(url.match(endpoint) for url in IGNORE_URLS)
Copy link
Contributor

@hartungstenio hartungstenio Oct 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return any(url.match(endpoint) for url in IGNORE_URLS)
return any(url.search(endpoint) for url in IGNORE_URLS)

Using search would remove the needs to change the user's regex a few lines above, and would make it compatible with the way django itself handle this kind of settings, like SECURE_REDIRECT_EXEMPT‎. Using search also should even allow to use a single settings.

24 changes: 24 additions & 0 deletions tests/functional/test_sync_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,3 +307,27 @@ def test_url_ignored(client, caplog):
('Received signal `request_finished`, clearing guid', None),
]
assert [(x.message, x.correlation_id) for x in caplog.records] == expected


def test_url_ignored_with_regex(client, caplog, monkeypatch):
"""
Test that a URL specified in IGNORE_URLS is ignored.
:param client: Django client
:param caplog: Caplog fixture
"""
from django.conf import settings as django_settings

mocked_settings = deepcopy(django_settings.DJANGO_GUID)
mocked_settings['IGNORE_REGEX_URLS'] = {'no-*'}
with override_settings(DJANGO_GUID=mocked_settings):
settings = Settings()
monkeypatch.setattr('django_guid.utils.settings', settings)
client.get('/no-guid-regex', **{'HTTP_Correlation-ID': 'bad-guid'})
# No log message should have a GUID, aka `None` on index 1.
expected = [
('sync middleware called', None),
('This log message should NOT have a GUID - the URL is in IGNORE_REGEX_URLS', None),
('Some warning in a function', None),
('Received signal `request_finished`, clearing guid', None),
]
assert [(x.message, x.correlation_id) for x in caplog.records] == expected
Loading