Skip to content

Commit

Permalink
Add IGNORE_URLS setting (#38)
Browse files Browse the repository at this point in the history
* Add IGNORE_URLS setting

This (optional) setting will allow a user to opt-out of the
middleware for certain URLS.

The setting is a list of strings, which are the paths of
urls to ignore. The paths can be written with prefix '/'
and/or suffix '/', or without them.

* Added tests

* Added docs, release notes and bumped version

* Fixed changelogs

* Workflow checkout broken, changing to v2 should help according to github.com/actions/isues/23

* Add Iftah to CONTRIBUTERS.rst

Thanks :)

Co-authored-by: Iftah Haimovitch <iftah.haimovitch@booking.com>
Co-authored-by: Jonas Krüger Svensson <jonas-ks@hotmail.com>
  • Loading branch information
3 people committed Apr 12, 2020
1 parent 8537555 commit 001541f
Show file tree
Hide file tree
Showing 18 changed files with 136 additions and 41 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jobs:
name: Code coverage
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@master
- uses: actions/checkout@v2
- name: Set up Python 3.8
uses: actions/setup-python@v1
with:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish_to_pypi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ jobs:
name: Build and publish
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@master
- uses: actions/checkout@v2
- name: Set up Python 3.8
uses: actions/setup-python@v1
with:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pythonpackage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jobs:
matrix:
python-version: [3.6, 3.7, 3.8]
steps:
- uses: actions/checkout@v1
- uses: actions/checkout@v2
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v1
with:
Expand Down Expand Up @@ -38,7 +38,7 @@ jobs:
matrix:
python-version: [3.6, 3.7, 3.8]
steps:
- uses: actions/checkout@v1
- uses: actions/checkout@v2
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v1
with:
Expand Down
15 changes: 14 additions & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
@@ -1,7 +1,19 @@
Changelog
=========

`2.1.0`_ - 2020-03-11

`2.2.0`_ - 2020-11-04
---------------------
**Features**

* ``IGNORE_URLS`` setting which disables the middleware on a list of URLs.

**Other**

* Added docs for the new setting


`2.1.0`_ - 2020-11-03
---------------------
**Features**

Expand Down Expand Up @@ -177,3 +189,4 @@ Changelog
.. _1.1.1: https://github.com/jonasks/django-guid/compare/1.1.0...1.1.1
.. _2.0.0: https://github.com/jonasks/django-guid/compare/1.1.1...2.0.0
.. _2.1.0: https://github.com/jonasks/django-guid/compare/2.0.0...2.1.0
.. _2.2.0: https://github.com/jonasks/django-guid/compare/2.1.0...2.2.0
1 change: 1 addition & 0 deletions CONTRIBUTORS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ Awesome Contributors

* Sondre Lillebø Gundersen | `@sondrelg <https://github.com/sondrelg>`_
* Ingvald Lorentzen | `@ingvaldlorentzen <https://github.com/ingvaldlorentzen>`_
* Iftah Haimovitch | `@Iftahh <https://github.com/Iftahh>`_
6 changes: 6 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ Package settings are added in your ``settings.py``:
RETURN_HEADER = True,
EXPOSE_HEADER = True,
INTEGRATIONS = [],
IGNORE_URLS = [],
}
Expand Down Expand Up @@ -118,6 +119,11 @@ Package settings are added in your ``settings.py``:

Default: []

* :code:`IGNORE_URLS`
URL endpoints where the middleware will be disabled. You can put your health check endpoints here.

Default: []

*************
Configuration
*************
Expand Down
3 changes: 2 additions & 1 deletion demoproj/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@
DJANGO_GUID = {
'GUID_HEADER_NAME': 'Correlation-ID',
'VALIDATE_GUID': True,
'INTEGRATIONS': []
'INTEGRATIONS': [],
'IGNORE_URLS': ['no_guid'],
}

# Set up logging for the project
Expand Down
3 changes: 2 additions & 1 deletion demoproj/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
"""
from django.urls import path

from demoproj.views import index_view, rest_view
from demoproj.views import index_view, rest_view, no_guid

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


def no_guid(request: HttpRequest) -> JsonResponse:
"""
Example view with a URL in the IGNORE_URLs list - no GUID will be in these logs
"""
logger.info('This log message should NOT have a GUID - the URL is in IGNORE_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
2 changes: 1 addition & 1 deletion django_guid/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
This file is imported by setup.py
Adding imports here will break setup.py
"""
__version__ = '2.1.0'
__version__ = '2.2.0'
default_app_config = 'django_guid.apps.DjangoGuidConfig'
63 changes: 39 additions & 24 deletions django_guid/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def __init__(self) -> None:
self.RETURN_HEADER = True
self.EXPOSE_HEADER = True
self.INTEGRATIONS = []
self.IGNORE_URLS = []
self.SKIP_CLEANUP = None # Deprecated - to be removed in the next major version

if hasattr(django_settings, 'DJANGO_GUID'):
Expand All @@ -38,32 +39,18 @@ def __init__(self) -> None:
raise ImproperlyConfigured('RETURN_HEADER must be a boolean')
if not isinstance(self.EXPOSE_HEADER, bool):
raise ImproperlyConfigured('EXPOSE_HEADER must be a boolean')
if not isinstance(self.INTEGRATIONS, list):
if not isinstance(self.INTEGRATIONS, (list, tuple)):
raise ImproperlyConfigured('INTEGRATIONS must be an array')
if not isinstance(self.IGNORE_URLS, (list, tuple)):
raise ImproperlyConfigured('IGNORE_URLS must be an array')

for integration in self.INTEGRATIONS:

# Make sure all integration methods are callable
for method, name in [
(integration.setup, 'setup'),
(integration.run, 'run'),
(integration.cleanup, 'cleanup'),
]:
# Make sure the methods are callable
if not callable(method):
raise ImproperlyConfigured(
f'Integration method `{name}` needs to be made callable for `{integration.identifier}`.'
)

# Make sure the method takes kwargs
if name in ['run', 'cleanup'] and not func_accepts_kwargs(method):
raise ImproperlyConfigured(
f'Integration method `{name}` must '
f'accept keyword arguments (**kwargs) for `{integration.identifier}`.'
)

# Run validate method
integration.setup()
if not all(isinstance(url, str) for url in self.IGNORE_URLS):
raise ImproperlyConfigured('IGNORE_URLS must be an array of strings')
# Note: stripping the '/' from the beginning and end of the path of the URLS,
# this is since some people would write a path as "/path/one/two/" while others would write "path/one/two"
self.IGNORE_URLS = {url.strip('/') for url in self.IGNORE_URLS}

self._validate_and_setup_integrations()

if 'SKIP_CLEANUP' in _settings:
warn(
Expand All @@ -72,5 +59,33 @@ def __init__(self) -> None:
DeprecationWarning,
)

def _validate_and_setup_integrations(self) -> None:
"""
Validate the INTEGRATIONS settings and verify each integration
"""
for integration in self.INTEGRATIONS:

# Make sure all integration methods are callable
for method, name in [
(integration.setup, 'setup'),
(integration.run, 'run'),
(integration.cleanup, 'cleanup'),
]:
# Make sure the methods are callable
if not callable(method):
raise ImproperlyConfigured(
f'Integration method `{name}` needs to be made callable for `{integration.identifier}`.'
)

# Make sure the method takes kwargs
if name in ['run', 'cleanup'] and not func_accepts_kwargs(method):
raise ImproperlyConfigured(
f'Integration method `{name}` must '
f'accept keyword arguments (**kwargs) for `{integration.identifier}`.'
)

# Run validate method
integration.setup()


settings = Settings()
3 changes: 3 additions & 0 deletions django_guid/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ def __call__(self, request: HttpRequest) -> Union[HttpRequest, HttpResponse]:
:param request: HttpRequest from Django
:return: Passes on the request or response to the next middleware
"""
if request.get_full_path().strip('/') in settings.IGNORE_URLS:
return self.get_response(request)

# Process request and store the GUID on the thread
self.set_guid(self._get_id_from_header(request))

Expand Down
7 changes: 7 additions & 0 deletions docs/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,10 @@ INTEGRATIONS
Whether to enable any custom or available integrations with :code:`django_guid`.
As an example, using :code:`SentryIntegration()` as an integration would set Sentry's :code:`transaction_id` to
match the GUID used by the middleware.

IGNORE_URLS
-----------
* **Default**: ``[]``
* **Type**: ``list``

URL endpoints where the middleware will be disabled. You can put your health check endpoints here.
22 changes: 21 additions & 1 deletion tests/functional/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def test_expose_header_return_header_false(client, monkeypatch, mock_uuid):
assert not response.get('Access-Control-Expose-Headers')


def test_cleanup_signal(client, caplog, monkeypatch, mock_uuid):
def test_cleanup_signal(client, caplog, monkeypatch):
"""
Tests that a request cleans up a request after finishing.
:param client: Django client
Expand Down Expand Up @@ -208,3 +208,23 @@ def test_improperly_configured_if_not_in_installed_apps(client, monkeypatch):
monkeypatch.setattr('django_guid.middleware.apps.is_installed', lambda x: False)
with pytest.raises(ImproperlyConfigured, match='django_guid must be in installed apps'):
client.get('/')


def test_url_ignored(client, caplog, monkeypatch):
"""
Test that a URL specified in IGNORE_URLS is ignored.
:param client: Django client
:param caplog: Caplog fixture
:param monkeypatch: Monkeypatch for django settings
"""
from django_guid.config import settings as guid_settings

monkeypatch.setattr(guid_settings, 'IGNORE_URLS', {'no_guid'}) # Same as it would be after config conversion
client.get('/no_guid', **{'HTTP_Correlation-ID': 'bad-guid'})
# No log message should have a GUID, aka `None` on index 1.
expected = [
('This log message should NOT have a GUID - the URL is in IGNORE_URLS', None),
('Some warning in a function', None),
('Received signal `request_finished`', None),
]
assert [(x.message, x.correlation_id) for x in caplog.records] == expected
8 changes: 4 additions & 4 deletions tests/requirements-test-base.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
pytest==5.3.5
pytest==5.4.1
pytest-cov==2.8.1
pytest-django==3.8.0
pytest-mock==2.0.0
pytest-django==3.9.0
pytest-mock==3.0.0
pytest-subtests==0.3.0
djangorestframework==3.11.0
sentry-sdk==0.14.2
sentry-sdk==0.14.3
2 changes: 1 addition & 1 deletion tests/requirements-test-django2.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Django==2.2.11
Django==2.2.12
2 changes: 1 addition & 1 deletion tests/requirements-test-django3.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
Django==3.0.4
Django==3.0.5
23 changes: 21 additions & 2 deletions tests/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,26 @@ def test_valid_settings(monkeypatch):


def test_bad_integrations_type(monkeypatch):
for item in [{}, '', 2, None, -2]:
monkeypatch.setattr(django_settings, 'DJANGO_GUID', {'INTEGRATIONS': item})
for setting in [{}, '', 2, None, -2]:
monkeypatch.setattr(django_settings, 'DJANGO_GUID', {'INTEGRATIONS': setting})
with pytest.raises(ImproperlyConfigured, match='INTEGRATIONS must be an array'):
Settings()


def test_not_array_ignore_urls(monkeypatch):
for setting in [{}, '', 2, None, -2]:
monkeypatch.setattr(django_settings, 'DJANGO_GUID', {'IGNORE_URLS': setting})
with pytest.raises(ImproperlyConfigured, match='IGNORE_URLS must be an array'):
Settings()


def test_not_string_in_igore_urls(monkeypatch):
for setting in (['api/v1/test', 'api/v1/othertest', True], [1, 2, 'yup']):
monkeypatch.setattr(django_settings, 'DJANGO_GUID', {'IGNORE_URLS': setting})
with pytest.raises(ImproperlyConfigured, match='IGNORE_URLS must be an array of strings'):
Settings()


def test_converts_correctly(monkeypatch):
monkeypatch.setattr(django_settings, 'DJANGO_GUID', {'IGNORE_URLS': ['/no_guid', '/my/api/path/']})
assert {'no_guid', 'my/api/path'} == Settings().IGNORE_URLS

0 comments on commit 001541f

Please sign in to comment.