Skip to content

Commit 672e078

Browse files
committed
fix: handle a tuple value for settings.MIDDLEWARE
Django is fine with settings.MIDDLEWARE being either a tuple or a list, but we were expecting a list. Now, we can handle either.
1 parent 00381dc commit 672e078

File tree

3 files changed

+28
-5
lines changed

3 files changed

+28
-5
lines changed

appmap/django.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
"""
77

88
import json
9+
import logging
910
import re
1011
import sys
1112
import time
@@ -37,6 +38,8 @@
3738
from ._implementation.web_framework import AppmapMiddleware
3839
from ._implementation.web_framework import TemplateHandler as BaseTemplateHandler
3940

41+
logger = logging.getLogger(__name__)
42+
4043

4144
def parse_pg_version(version):
4245
"""Transform postgres version number like 120005 to a tuple like 12.0.5."""
@@ -296,10 +299,23 @@ def after_request_main(self, rec, response, start, call_event_id):
296299
def inject_middleware():
297300
"""Make sure AppMap middleware is added to the stack"""
298301
if "appmap.django.Middleware" not in settings.MIDDLEWARE:
302+
stack = list(settings.MIDDLEWARE)
303+
299304
new_middleware = ["appmap.django.Middleware"]
300305
if DetectEnabled.should_enable("remote"):
301306
new_middleware.insert(0, "appmap._implementation.django.RemoteRecording")
302-
settings.MIDDLEWARE[0:0] = new_middleware
307+
308+
stack[0:0] = new_middleware
309+
# Django is ok with settings.MIDDLEWARE being any kind iterable. Update it, without changing
310+
# its type, if we can.
311+
if isinstance(settings.MIDDLEWARE, list):
312+
settings.MIDDLEWARE = stack
313+
elif isinstance(settings.MIDDLEWARE, tuple):
314+
settings.MIDDLEWARE = tuple(stack)
315+
else:
316+
logger.warning(
317+
f"Don't know how to update settings.MIDDLEWARE of type {type(settings.MIDDLEWARE)}, recording is not enabled."
318+
)
303319

304320

305321
original_load_middleware = BaseHandler.load_middleware

appmap/test/data/django/test/test_app.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
import app
2+
import pytest
23
from django.core.handlers.base import BaseHandler
34
from django.test import Client
45

56

6-
def test_middleware(rf, settings):
7-
settings.MIDDLEWARE = ["app.middleware.hello_world"]
8-
request = rf.get("/")
7+
@pytest.mark.parametrize(
8+
"mware", [["app.middleware.hello_world"], ("app.middleware.hello_world",)]
9+
)
10+
def test_middleware(rf, settings, mware):
11+
settings.MIDDLEWARE = mware
12+
orig_type = type(settings.MIDDLEWARE)
13+
request = rf.get("/_appmap/record")
914
handler = BaseHandler()
1015
handler.load_middleware()
16+
assert type(settings.MIDDLEWARE) == orig_type
1117
response = handler.get_response(request)
1218
assert response.status_code == 200
1319

appmap/test/test_django.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,8 @@ def test_middleware_reset(pytester, monkeypatch):
191191

192192
# To really check middleware reset, the tests must run in order,
193193
# so disable randomly.
194-
pytester.runpytest("-svv", "-p", "no:randomly")
194+
result = pytester.runpytest("-svv", "-p", "no:randomly")
195+
result.assert_outcomes(passed=3, failed=0, errors=0)
195196

196197
# Look for the http_server_request event in test_app's appmap. If
197198
# middleware reset is broken, it won't be there.

0 commit comments

Comments
 (0)