Skip to content

Commit 7677183

Browse files
Patch CursorWrapper dynamically to allow multiple base classes. (#1820)
* Patch CursorWrapper dynamically to allow multiple base classes. When the debug SQL logs are enabled, the wrapper class is CursorDebugWrapper not CursorWrapper. Since we have inspections based on that specific class they are removing the CursorDebugWrapper causing the SQL logs to not appear. This attempts to dynamically patch the CursorWrapper or CursorDebugWrapper with the functionality we need. This doesn't do a full regression test, but it may be possible to get it to work with: TEST_ARGS='--debug-sql' make test Which causes the current set of tests to fail since they are keyed to CursorWrapper. * Allow mixin as a valid word in our docs. * Support tests with --debug-sql
1 parent 6e55663 commit 7677183

File tree

4 files changed

+75
-30
lines changed

4 files changed

+75
-30
lines changed

debug_toolbar/panels/sql/tracking.py

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
from time import perf_counter
66

77
import django.test.testcases
8-
from django.db.backends.utils import CursorWrapper
98
from django.utils.encoding import force_str
109

1110
from debug_toolbar.utils import get_stack_trace, get_template_info
@@ -60,34 +59,42 @@ def cursor(*args, **kwargs):
6059
cursor = connection._djdt_cursor(*args, **kwargs)
6160
if logger is None:
6261
return cursor
63-
wrapper = NormalCursorWrapper if allow_sql.get() else ExceptionCursorWrapper
64-
return wrapper(cursor.cursor, connection, logger)
62+
mixin = NormalCursorMixin if allow_sql.get() else ExceptionCursorMixin
63+
return patch_cursor_wrapper_with_mixin(cursor.__class__, mixin)(
64+
cursor.cursor, connection, logger
65+
)
6566

6667
def chunked_cursor(*args, **kwargs):
6768
# prevent double wrapping
6869
# solves https://github.com/jazzband/django-debug-toolbar/issues/1239
6970
logger = connection._djdt_logger
7071
cursor = connection._djdt_chunked_cursor(*args, **kwargs)
71-
if logger is not None and not isinstance(cursor, DjDTCursorWrapper):
72-
if allow_sql.get():
73-
wrapper = NormalCursorWrapper
74-
else:
75-
wrapper = ExceptionCursorWrapper
76-
return wrapper(cursor.cursor, connection, logger)
72+
if logger is not None and not isinstance(cursor, DjDTCursorWrapperMixin):
73+
mixin = NormalCursorMixin if allow_sql.get() else ExceptionCursorMixin
74+
return patch_cursor_wrapper_with_mixin(cursor.__class__, mixin)(
75+
cursor.cursor, connection, logger
76+
)
7777
return cursor
7878

7979
connection.cursor = cursor
8080
connection.chunked_cursor = chunked_cursor
8181

8282

83-
class DjDTCursorWrapper(CursorWrapper):
83+
def patch_cursor_wrapper_with_mixin(base_wrapper, mixin):
84+
class DjDTCursorWrapper(mixin, base_wrapper):
85+
pass
86+
87+
return DjDTCursorWrapper
88+
89+
90+
class DjDTCursorWrapperMixin:
8491
def __init__(self, cursor, db, logger):
8592
super().__init__(cursor, db)
8693
# logger must implement a ``record`` method
8794
self.logger = logger
8895

8996

90-
class ExceptionCursorWrapper(DjDTCursorWrapper):
97+
class ExceptionCursorMixin(DjDTCursorWrapperMixin):
9198
"""
9299
Wraps a cursor and raises an exception on any operation.
93100
Used in Templates panel.
@@ -97,7 +104,7 @@ def __getattr__(self, attr):
97104
raise SQLQueryTriggered()
98105

99106

100-
class NormalCursorWrapper(DjDTCursorWrapper):
107+
class NormalCursorMixin(DjDTCursorWrapperMixin):
101108
"""
102109
Wraps a cursor and logs queries.
103110
"""

docs/changes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ Pending
1515
resolving to the wrong content type.
1616
* Fixed SQL statement recording under PostgreSQL for queries encoded as byte
1717
strings.
18+
* Patch the ``CursorWrapper`` class with a mixin class to support multiple
19+
base wrapper classes.
1820

1921
4.1.0 (2023-05-15)
2022
------------------

docs/spelling_wordlist.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ memcache
3030
memcached
3131
middleware
3232
middlewares
33+
mixin
3334
mousedown
3435
mouseup
3536
multi

tests/panels/test_sql.py

Lines changed: 53 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@
22
import datetime
33
import os
44
import unittest
5-
from unittest.mock import patch
5+
from unittest.mock import call, patch
66

77
import django
88
from asgiref.sync import sync_to_async
99
from django.contrib.auth.models import User
1010
from django.db import connection, transaction
11+
from django.db.backends.utils import CursorDebugWrapper, CursorWrapper
1112
from django.db.models import Count
1213
from django.db.utils import DatabaseError
1314
from django.shortcuts import render
@@ -68,39 +69,59 @@ def test_recording_chunked_cursor(self):
6869
self.assertEqual(len(self.panel._queries), 1)
6970

7071
@patch(
71-
"debug_toolbar.panels.sql.tracking.NormalCursorWrapper",
72-
wraps=sql_tracking.NormalCursorWrapper,
72+
"debug_toolbar.panels.sql.tracking.patch_cursor_wrapper_with_mixin",
73+
wraps=sql_tracking.patch_cursor_wrapper_with_mixin,
7374
)
74-
def test_cursor_wrapper_singleton(self, mock_wrapper):
75+
def test_cursor_wrapper_singleton(self, mock_patch_cursor_wrapper):
7576
sql_call()
76-
7777
# ensure that cursor wrapping is applied only once
78-
self.assertEqual(mock_wrapper.call_count, 1)
78+
self.assertIn(
79+
mock_patch_cursor_wrapper.mock_calls,
80+
[
81+
[call(CursorWrapper, sql_tracking.NormalCursorMixin)],
82+
# CursorDebugWrapper is used if the test is called with `--debug-sql`
83+
[call(CursorDebugWrapper, sql_tracking.NormalCursorMixin)],
84+
],
85+
)
7986

8087
@patch(
81-
"debug_toolbar.panels.sql.tracking.NormalCursorWrapper",
82-
wraps=sql_tracking.NormalCursorWrapper,
88+
"debug_toolbar.panels.sql.tracking.patch_cursor_wrapper_with_mixin",
89+
wraps=sql_tracking.patch_cursor_wrapper_with_mixin,
8390
)
84-
def test_chunked_cursor_wrapper_singleton(self, mock_wrapper):
91+
def test_chunked_cursor_wrapper_singleton(self, mock_patch_cursor_wrapper):
8592
sql_call(use_iterator=True)
8693

8794
# ensure that cursor wrapping is applied only once
88-
self.assertEqual(mock_wrapper.call_count, 1)
95+
self.assertIn(
96+
mock_patch_cursor_wrapper.mock_calls,
97+
[
98+
[call(CursorWrapper, sql_tracking.NormalCursorMixin)],
99+
# CursorDebugWrapper is used if the test is called with `--debug-sql`
100+
[call(CursorDebugWrapper, sql_tracking.NormalCursorMixin)],
101+
],
102+
)
89103

90104
@patch(
91-
"debug_toolbar.panels.sql.tracking.NormalCursorWrapper",
92-
wraps=sql_tracking.NormalCursorWrapper,
105+
"debug_toolbar.panels.sql.tracking.patch_cursor_wrapper_with_mixin",
106+
wraps=sql_tracking.patch_cursor_wrapper_with_mixin,
93107
)
94-
async def test_cursor_wrapper_async(self, mock_wrapper):
108+
async def test_cursor_wrapper_async(self, mock_patch_cursor_wrapper):
95109
await sync_to_async(sql_call)()
96110

97-
self.assertEqual(mock_wrapper.call_count, 1)
111+
self.assertIn(
112+
mock_patch_cursor_wrapper.mock_calls,
113+
[
114+
[call(CursorWrapper, sql_tracking.NormalCursorMixin)],
115+
# CursorDebugWrapper is used if the test is called with `--debug-sql`
116+
[call(CursorDebugWrapper, sql_tracking.NormalCursorMixin)],
117+
],
118+
)
98119

99120
@patch(
100-
"debug_toolbar.panels.sql.tracking.NormalCursorWrapper",
101-
wraps=sql_tracking.NormalCursorWrapper,
121+
"debug_toolbar.panels.sql.tracking.patch_cursor_wrapper_with_mixin",
122+
wraps=sql_tracking.patch_cursor_wrapper_with_mixin,
102123
)
103-
async def test_cursor_wrapper_asyncio_ctx(self, mock_wrapper):
124+
async def test_cursor_wrapper_asyncio_ctx(self, mock_patch_cursor_wrapper):
104125
self.assertTrue(sql_tracking.allow_sql.get())
105126
await sync_to_async(sql_call)()
106127

@@ -116,7 +137,21 @@ async def task():
116137
await asyncio.create_task(task())
117138
# Because it was called in another context, it should not have affected ours
118139
self.assertTrue(sql_tracking.allow_sql.get())
119-
self.assertEqual(mock_wrapper.call_count, 1)
140+
141+
self.assertIn(
142+
mock_patch_cursor_wrapper.mock_calls,
143+
[
144+
[
145+
call(CursorWrapper, sql_tracking.NormalCursorMixin),
146+
call(CursorWrapper, sql_tracking.ExceptionCursorMixin),
147+
],
148+
# CursorDebugWrapper is used if the test is called with `--debug-sql`
149+
[
150+
call(CursorDebugWrapper, sql_tracking.NormalCursorMixin),
151+
call(CursorDebugWrapper, sql_tracking.ExceptionCursorMixin),
152+
],
153+
],
154+
)
120155

121156
def test_generate_server_timing(self):
122157
self.assertEqual(len(self.panel._queries), 0)

0 commit comments

Comments
 (0)