Skip to content

Commit 0c309fc

Browse files
authored
117 django middleware only applies to default database connections ignoring other databases (#127)
Add support for multiple connection in django sqlcommenter We used the `connections` middleware instead of `connections` to support multiple databases instead of only the default database. TESTED=unittests
1 parent 7561463 commit 0c309fc

File tree

5 files changed

+34
-6
lines changed

5 files changed

+34
-6
lines changed

python/sqlcommenter-python/google/cloud/sqlcommenter/django/middleware.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515
# limitations under the License.
1616

1717
import logging
18+
from contextlib import ExitStack
1819

1920
import django
20-
from django.db import connection
21+
from django.db import connections
22+
2123
from django.db.backends.utils import CursorDebugWrapper
2224
from google.cloud.sqlcommenter import add_sql_comment
2325
from google.cloud.sqlcommenter.opencensus import get_opencensus_values
@@ -36,7 +38,9 @@ def __init__(self, get_response):
3638
self.get_response = get_response
3739

3840
def __call__(self, request):
39-
with connection.execute_wrapper(QueryWrapper(request)):
41+
with ExitStack() as stack:
42+
for db_alias in connections:
43+
stack.enter_context(connections[db_alias].execute_wrapper(QueryWrapper(request)))
4044
return self.get_response(request)
4145

4246

@@ -88,7 +92,7 @@ def __call__(self, execute, sql, params, many, context):
8892
# * https://github.com/basecamp/marginalia/pull/80
8993

9094
# Add the query to the query log if debugging.
91-
if context['cursor'].__class__ is CursorDebugWrapper:
95+
if isinstance(context['cursor'], CursorDebugWrapper):
9296
context['connection'].queries_log.append(sql)
9397

9498
return execute(sql, params, many, context)

python/sqlcommenter-python/tests/django/settings.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@
1818
'default': {
1919
'ENGINE': 'django.db.backends.sqlite3',
2020
},
21+
22+
'other': {
23+
'ENGINE': 'django.db.backends.sqlite3',
24+
},
2125
}
2226

2327
INSTALLED_APPS = ['tests.django']

python/sqlcommenter-python/tests/django/tests.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
# limitations under the License.
1616

1717
import django
18-
from django.db import connection
18+
from django.db import connection, connections
1919
from django.http import HttpRequest
2020
from django.test import TestCase, override_settings, modify_settings
2121
from django.urls import resolve, reverse
@@ -41,7 +41,7 @@ def __call__(self, request):
4141
# Query log only active if DEBUG=True.
4242
@override_settings(DEBUG=True)
4343
class Tests(TestCase):
44-
44+
databases = '__all__'
4545
@staticmethod
4646
def get_request(path):
4747
request = HttpRequest()
@@ -55,6 +55,13 @@ def get_query(self, path='/'):
5555
self.assertEqual(len(connection.queries), 2)
5656
return connection.queries[0]
5757

58+
def get_query_other_db(self, path='/', connection_name='default'):
59+
SqlCommenter(views.home_other_db)(self.get_request(path))
60+
# Query with comment added by QueryWrapper and unaltered query added
61+
# by Django's CursorDebugWrapper.
62+
self.assertEqual(len(connections[connection_name].queries), 2)
63+
return connections[connection_name].queries[0]
64+
5865
def assertRoute(self, route, query):
5966
# route available in Django 2.2 and later.
6067
if django.VERSION < (2, 2):
@@ -69,6 +76,13 @@ def test_basic(self):
6976
self.assertIn("framework='django%%3A" + django.get_version(), query)
7077
self.assertRoute('', query)
7178

79+
def test_basic_multiple_db_support(self):
80+
query = self.get_query_other_db(path='/other/', connection_name='other')
81+
self.assertIn("/*controller='some-other-db-path'", query)
82+
# Expecting url_quoted("framework='django:'")
83+
self.assertIn("framework='django%%3A" + django.get_version(), query)
84+
self.assertRoute('other/', query)
85+
7286
def test_basic_disabled(self):
7387
with self.settings(
7488
SQLCOMMENTER_WITH_CONTROLLER=False, SQLCOMMENTER_WITH_ROUTE=False,

python/sqlcommenter-python/tests/django/urls.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,6 @@
2121
urlpatterns = [
2222
path('', views.home, name='home'),
2323
path('path/', views.home, name='some-path'),
24+
path('other/', views.home_other_db, name='some-other-db-path'),
2425
path('app-urls/', include('tests.django.app_urls')),
2526
]

python/sqlcommenter-python/tests/django/views.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,10 @@
2020

2121

2222
def home(request):
23-
list(Author.objects.all())
23+
list(Author.objects.all().using('default'))
24+
return HttpResponse()
25+
26+
27+
def home_other_db(request):
28+
list(Author.objects.all().using('other'))
2429
return HttpResponse()

0 commit comments

Comments
 (0)