Skip to content

Simplify SQLPanel._queries data structure #1638

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

Merged
merged 1 commit into from
Jun 1, 2022
Merged
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
12 changes: 7 additions & 5 deletions debug_toolbar/panels/sql/panel.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ def current_transaction_id(self, alias):
trans_id = self.new_transaction_id(alias)
return trans_id

def record(self, alias, **kwargs):
self._queries.append((alias, kwargs))
def record(self, **kwargs):
self._queries.append(kwargs)
alias = kwargs["alias"]
if alias not in self._databases:
self._databases[alias] = {
"time_spent": kwargs["duration"],
Expand Down Expand Up @@ -193,7 +194,9 @@ def generate_stats(self, request, response):

# the last query recorded for each DB alias
last_by_alias = {}
for alias, query in self._queries:
for query in self._queries:
alias = query["alias"]

similar_query_groups[(alias, _similar_query_key(query))].append(query)
duplicate_query_groups[(alias, _duplicate_query_key(query))].append(
query
Expand All @@ -214,7 +217,6 @@ def generate_stats(self, request, response):
if trans_id is not None:
query["in_trans"] = True

query["alias"] = alias
if "iso_level" in query:
query["iso_level"] = get_isolation_level_display(
query["vendor"], query["iso_level"]
Expand Down Expand Up @@ -263,7 +265,7 @@ def generate_stats(self, request, response):
"databases": sorted(
self._databases.items(), key=lambda x: -x[1]["time_spent"]
),
"queries": [q for a, q in self._queries],
"queries": self._queries,
"sql_time": self._sql_time,
}
)
Expand Down
190 changes: 87 additions & 103 deletions tests/panels/test_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ def test_recording(self):
# ensure query was logged
self.assertEqual(len(self.panel._queries), 1)
query = self.panel._queries[0]
self.assertEqual(query[0], "default")
self.assertTrue("sql" in query[1])
self.assertTrue("duration" in query[1])
self.assertTrue("stacktrace" in query[1])
self.assertEqual(query["alias"], "default")
self.assertTrue("sql" in query)
self.assertTrue("duration" in query)
self.assertTrue("stacktrace" in query)

# ensure the stacktrace is populated
self.assertTrue(len(query[1]["stacktrace"]) > 0)
self.assertTrue(len(query["stacktrace"]) > 0)

@unittest.skipUnless(
connection.vendor == "postgresql", "Test valid only on PostgreSQL"
Expand Down Expand Up @@ -128,7 +128,7 @@ def test_generate_server_timing(self):
query = self.panel._queries[0]

expected_data = {
"sql_time": {"title": "SQL 1 queries", "value": query[1]["duration"]}
"sql_time": {"title": "SQL 1 queries", "value": query["duration"]}
}

self.assertEqual(self.panel.get_server_timing_stats(), expected_data)
Expand Down Expand Up @@ -195,7 +195,7 @@ def test_param_conversion(self):
expected_datetime = '["2017-12-22 16:07:01"]'

self.assertEqual(
tuple(q[1]["params"] for q in self.panel._queries),
tuple(query["params"] for query in self.panel._queries),
(
expected_bools,
"[10, 1]",
Expand All @@ -217,7 +217,7 @@ def test_json_param_conversion(self):
# ensure query was logged
self.assertEqual(len(self.panel._queries), 1)
self.assertEqual(
self.panel._queries[0][1]["params"],
self.panel._queries[0]["params"],
'["{\\"foo\\": \\"bar\\"}"]',
)

Expand All @@ -237,7 +237,7 @@ def test_binary_param_force_text(self):
self.assertIn(
"<strong>SELECT</strong> * <strong>FROM</strong>"
" tests_binary <strong>WHERE</strong> field =",
self.panel._queries[0][1]["sql"],
self.panel._queries[0]["sql"],
)

@unittest.skipUnless(connection.vendor != "sqlite", "Test invalid for SQLite")
Expand Down Expand Up @@ -288,7 +288,7 @@ def test_raw_query_param_conversion(self):
self.assertEqual(len(self.panel._queries), 2)

self.assertEqual(
tuple(q[1]["params"] for q in self.panel._queries),
tuple(query["params"] for query in self.panel._queries),
(
'["Foo", true, false, "2017-12-22 16:07:01"]',
" ".join(
Expand Down Expand Up @@ -375,9 +375,9 @@ def test_execute_with_psycopg2_composed_sql(self):
self.assertEqual(len(self.panel._queries), 1)

query = self.panel._queries[0]
self.assertEqual(query[0], "default")
self.assertTrue("sql" in query[1])
self.assertEqual(query[1]["sql"], 'select "username" from "auth_user"')
self.assertEqual(query["alias"], "default")
self.assertTrue("sql" in query)
self.assertEqual(query["sql"], 'select "username" from "auth_user"')

def test_disable_stacktraces(self):
self.assertEqual(len(self.panel._queries), 0)
Expand All @@ -388,13 +388,13 @@ def test_disable_stacktraces(self):
# ensure query was logged
self.assertEqual(len(self.panel._queries), 1)
query = self.panel._queries[0]
self.assertEqual(query[0], "default")
self.assertTrue("sql" in query[1])
self.assertTrue("duration" in query[1])
self.assertTrue("stacktrace" in query[1])
self.assertEqual(query["alias"], "default")
self.assertTrue("sql" in query)
self.assertTrue("duration" in query)
self.assertTrue("stacktrace" in query)

# ensure the stacktrace is empty
self.assertEqual([], query[1]["stacktrace"])
self.assertEqual([], query["stacktrace"])

@override_settings(
DEBUG=True,
Expand All @@ -418,13 +418,13 @@ def test_regression_infinite_recursion(self):
# template is loaded and basic.html extends base.html.
self.assertEqual(len(self.panel._queries), 2)
query = self.panel._queries[0]
self.assertEqual(query[0], "default")
self.assertTrue("sql" in query[1])
self.assertTrue("duration" in query[1])
self.assertTrue("stacktrace" in query[1])
self.assertEqual(query["alias"], "default")
self.assertTrue("sql" in query)
self.assertTrue("duration" in query)
self.assertTrue("stacktrace" in query)

# ensure the stacktrace is populated
self.assertTrue(len(query[1]["stacktrace"]) > 0)
self.assertTrue(len(query["stacktrace"]) > 0)

@override_settings(
DEBUG_TOOLBAR_CONFIG={"PRETTIFY_SQL": True},
Expand All @@ -439,7 +439,7 @@ def test_prettify_sql(self):

response = self.panel.process_request(self.request)
self.panel.generate_stats(self.request, response)
pretty_sql = self.panel._queries[-1][1]["sql"]
pretty_sql = self.panel._queries[-1]["sql"]
self.assertEqual(len(self.panel._queries), 1)

# Reset the queries
Expand All @@ -450,7 +450,7 @@ def test_prettify_sql(self):
response = self.panel.process_request(self.request)
self.panel.generate_stats(self.request, response)
self.assertEqual(len(self.panel._queries), 1)
self.assertNotEqual(pretty_sql, self.panel._queries[-1][1]["sql"])
self.assertNotEqual(pretty_sql, self.panel._queries[-1]["sql"])

self.panel._queries = []
# Run it again, but with prettyify back on.
Expand All @@ -461,7 +461,7 @@ def test_prettify_sql(self):
response = self.panel.process_request(self.request)
self.panel.generate_stats(self.request, response)
self.assertEqual(len(self.panel._queries), 1)
self.assertEqual(pretty_sql, self.panel._queries[-1][1]["sql"])
self.assertEqual(pretty_sql, self.panel._queries[-1]["sql"])

@override_settings(
DEBUG=True,
Expand All @@ -479,7 +479,7 @@ def test_flat_template_information(self):
self.assertEqual(len(self.panel._queries), 1)

query = self.panel._queries[0]
template_info = query[1]["template_info"]
template_info = query["template_info"]
template_name = os.path.basename(template_info["name"])
self.assertEqual(template_name, "flat.html")
self.assertEqual(template_info["context"][2]["content"].strip(), "{{ users }}")
Expand All @@ -501,7 +501,7 @@ def test_nested_template_information(self):
self.assertEqual(len(self.panel._queries), 1)

query = self.panel._queries[0]
template_info = query[1]["template_info"]
template_info = query["template_info"]
template_name = os.path.basename(template_info["name"])
self.assertEqual(template_name, "included.html")
self.assertEqual(template_info["context"][0]["content"].strip(), "{{ users }}")
Expand All @@ -524,45 +524,37 @@ def test_similar_and_duplicate_grouping(self):

queries = self.panel._queries
query = queries[0]
self.assertEqual(query[1]["similar_count"], 3)
self.assertEqual(query[1]["duplicate_count"], 2)
self.assertEqual(query["similar_count"], 3)
self.assertEqual(query["duplicate_count"], 2)

query = queries[1]
self.assertEqual(query[1]["similar_count"], 3)
self.assertEqual(query[1]["duplicate_count"], 2)
self.assertEqual(query["similar_count"], 3)
self.assertEqual(query["duplicate_count"], 2)

query = queries[2]
self.assertEqual(query[1]["similar_count"], 3)
self.assertTrue("duplicate_count" not in query[1])
self.assertEqual(query["similar_count"], 3)
self.assertTrue("duplicate_count" not in query)

query = queries[3]
self.assertEqual(query[1]["similar_count"], 2)
self.assertTrue("duplicate_count" not in query[1])
self.assertEqual(query["similar_count"], 2)
self.assertTrue("duplicate_count" not in query)

query = queries[4]
self.assertEqual(query[1]["similar_count"], 2)
self.assertTrue("duplicate_count" not in query[1])
self.assertEqual(query["similar_count"], 2)
self.assertTrue("duplicate_count" not in query)

query = queries[5]
self.assertTrue("similar_count" not in query[1])
self.assertTrue("duplicate_count" not in query[1])
self.assertTrue("similar_count" not in query)
self.assertTrue("duplicate_count" not in query)

self.assertEqual(queries[0][1]["similar_color"], queries[1][1]["similar_color"])
self.assertEqual(queries[0][1]["similar_color"], queries[2][1]["similar_color"])
self.assertEqual(
queries[0][1]["duplicate_color"], queries[1][1]["duplicate_color"]
)
self.assertNotEqual(
queries[0][1]["similar_color"], queries[0][1]["duplicate_color"]
)
self.assertEqual(queries[0]["similar_color"], queries[1]["similar_color"])
self.assertEqual(queries[0]["similar_color"], queries[2]["similar_color"])
self.assertEqual(queries[0]["duplicate_color"], queries[1]["duplicate_color"])
self.assertNotEqual(queries[0]["similar_color"], queries[0]["duplicate_color"])

self.assertEqual(queries[3][1]["similar_color"], queries[4][1]["similar_color"])
self.assertNotEqual(
queries[0][1]["similar_color"], queries[3][1]["similar_color"]
)
self.assertNotEqual(
queries[0][1]["duplicate_color"], queries[3][1]["similar_color"]
)
self.assertEqual(queries[3]["similar_color"], queries[4]["similar_color"])
self.assertNotEqual(queries[0]["similar_color"], queries[3]["similar_color"])
self.assertNotEqual(queries[0]["duplicate_color"], queries[3]["similar_color"])


class SQLPanelMultiDBTestCase(BaseMultiDBTestCase):
Expand All @@ -580,10 +572,10 @@ def test_aliases(self):
self.assertTrue(self.panel._queries)

query = self.panel._queries[0]
self.assertEqual(query[0], "default")
self.assertEqual(query["alias"], "default")

query = self.panel._queries[-1]
self.assertEqual(query[0], "replica")
self.assertEqual(query["alias"], "replica")

def test_transaction_status(self):
"""
Expand Down Expand Up @@ -614,63 +606,55 @@ def test_transaction_status(self):
self.assertEqual(len(self.panel._queries), 6)

query = self.panel._queries[0]
self.assertEqual(query[0], "default")
self.assertIsNotNone(query[1]["trans_id"])
self.assertTrue(query[1]["starts_trans"])
self.assertTrue(query[1]["in_trans"])
self.assertFalse("end_trans" in query[1])
self.assertEqual(query["alias"], "default")
self.assertIsNotNone(query["trans_id"])
self.assertTrue(query["starts_trans"])
self.assertTrue(query["in_trans"])
self.assertFalse("end_trans" in query)

query = self.panel._queries[-1]
self.assertEqual(query[0], "replica")
self.assertIsNone(query[1]["trans_id"])
self.assertFalse("starts_trans" in query[1])
self.assertFalse("in_trans" in query[1])
self.assertFalse("end_trans" in query[1])
self.assertEqual(query["alias"], "replica")
self.assertIsNone(query["trans_id"])
self.assertFalse("starts_trans" in query)
self.assertFalse("in_trans" in query)
self.assertFalse("end_trans" in query)

query = self.panel._queries[2]
self.assertEqual(query[0], "default")
self.assertIsNotNone(query[1]["trans_id"])
self.assertEqual(
query[1]["trans_id"], self.panel._queries[0][1]["trans_id"]
)
self.assertFalse("starts_trans" in query[1])
self.assertTrue(query[1]["in_trans"])
self.assertTrue(query[1]["ends_trans"])
self.assertEqual(query["alias"], "default")
self.assertIsNotNone(query["trans_id"])
self.assertEqual(query["trans_id"], self.panel._queries[0]["trans_id"])
self.assertFalse("starts_trans" in query)
self.assertTrue(query["in_trans"])
self.assertTrue(query["ends_trans"])

query = self.panel._queries[3]
self.assertEqual(query[0], "replica")
self.assertIsNotNone(query[1]["trans_id"])
self.assertNotEqual(
query[1]["trans_id"], self.panel._queries[0][1]["trans_id"]
)
self.assertTrue(query[1]["starts_trans"])
self.assertTrue(query[1]["in_trans"])
self.assertTrue(query[1]["ends_trans"])
self.assertEqual(query["alias"], "replica")
self.assertIsNotNone(query["trans_id"])
self.assertNotEqual(query["trans_id"], self.panel._queries[0]["trans_id"])
self.assertTrue(query["starts_trans"])
self.assertTrue(query["in_trans"])
self.assertTrue(query["ends_trans"])

query = self.panel._queries[4]
self.assertEqual(query[0], "default")
self.assertIsNotNone(query[1]["trans_id"])
self.assertNotEqual(
query[1]["trans_id"], self.panel._queries[0][1]["trans_id"]
)
self.assertNotEqual(
query[1]["trans_id"], self.panel._queries[3][1]["trans_id"]
)
self.assertTrue(query[1]["starts_trans"])
self.assertTrue(query[1]["in_trans"])
self.assertTrue(query[1]["ends_trans"])
self.assertEqual(query["alias"], "default")
self.assertIsNotNone(query["trans_id"])
self.assertNotEqual(query["trans_id"], self.panel._queries[0]["trans_id"])
self.assertNotEqual(query["trans_id"], self.panel._queries[3]["trans_id"])
self.assertTrue(query["starts_trans"])
self.assertTrue(query["in_trans"])
self.assertTrue(query["ends_trans"])

query = self.panel._queries[5]
self.assertEqual(query[0], "replica")
self.assertIsNone(query[1]["trans_id"])
self.assertFalse("starts_trans" in query[1])
self.assertFalse("in_trans" in query[1])
self.assertFalse("end_trans" in query[1])
self.assertEqual(query["alias"], "replica")
self.assertIsNone(query["trans_id"])
self.assertFalse("starts_trans" in query)
self.assertFalse("in_trans" in query)
self.assertFalse("end_trans" in query)
else:
# Ensure that nothing was recorded for other database engines.
self.assertTrue(self.panel._queries)
for query in self.panel._queries:
self.assertFalse("trans_id" in query[1])
self.assertFalse("starts_trans" in query[1])
self.assertFalse("in_trans" in query[1])
self.assertFalse("end_trans" in query[1])
self.assertFalse("trans_id" in query)
self.assertFalse("starts_trans" in query)
self.assertFalse("in_trans" in query)
self.assertFalse("end_trans" in query)