Skip to content

Commit 3bacec5

Browse files
trinnodeopenhands-agent
trinnode
and
openhands-agent
authored
feat: API performance improvements - gzip compression, N+1 detection, query caching, DB explain endpoint (#749)
* feat: add API performance improvements - gzip compression, N+1 detection, query caching, and DB explain endpoint - Gzip compression: verified GZipMiddleware is active, added comprehensive tests for compression behavior, Accept-Encoding handling, and edge cases (issue #487) - N+1 query detection: added GraphQL extension that identifies lazy-loading patterns and logs warnings in dev mode with zero production overhead (issue #490) - Query result caching: added 30s TTL caching for GET /contracts endpoint, cache invalidation on GraphQL mutations, cache busting headers via Cache-Control/X-Cache-Bust, and cache stats endpoint (issue #488) - DB explain endpoint: added /api/admin/db/explain/ for admins to run EXPLAIN ANALYZE on SQL queries with SQL injection prevention (SELECT-only) and rate limiting (issue #491) * fix: address CI failures - add GZip/CacheBusting to test settings, fix URL routing, cache invalidation, and lint - Added GZipMiddleware and CacheBustingMiddleware to settings_test.py for CI parity - Added GRAPHQL_N1_DETECTION_ENABLED=False to test settings - Added db_explain and cache_stats endpoints to urls_test.py - Fixed contracts list cache invalidation on create/update/delete to prevent stale responses - Fixed all ruff lint issues (unused imports, lambda assignments, E402) - Moved import re to top-level in views.py - Removed unused backend_name variable in cache_stats_view - All 744 tests pass with settings_test.py (CI configuration) --------- Co-authored-by: openhands <openhands@all-hands.dev>
1 parent 6fa3e1a commit 3bacec5

13 files changed

Lines changed: 952 additions & 6 deletions

‎django-backend/soroscan/graphql_n1_detector.py‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
"""
2+
N+1 query detection for GraphQL resolvers (issue #490).
3+
4+
Logs warnings when a resolver executes more DB queries than expected,
5+
helping developers identify lazy-loading issues during development.
6+
Zero overhead in production when N1_DETECTION_ENABLED is False.
7+
"""
8+
import logging
9+
import time
10+
from typing import Any, Callable
11+
12+
from django.conf import settings
13+
from django.db import connection
14+
from strawberry.extensions import SchemaExtension
15+
16+
logger = logging.getLogger("soroscan.graphql.n1_detection")
17+
18+
19+
class N1QueryDetectorExtension(SchemaExtension):
20+
"""
21+
Strawberry extension that counts DB queries per resolver and warns
22+
when a pattern suggests N+1 query behavior.
23+
24+
Enabled only when GRAPHQL_N1_DETECTION_ENABLED=True (default: False in prod,
25+
True in DEBUG mode).
26+
"""
27+
28+
def _is_enabled(self) -> bool:
29+
return getattr(settings, "GRAPHQL_N1_DETECTION_ENABLED", settings.DEBUG)
30+
31+
def resolve(
32+
self,
33+
_next: Callable,
34+
root: Any,
35+
info: Any,
36+
*args: Any,
37+
**kwargs: Any,
38+
) -> Any:
39+
if not self._is_enabled():
40+
return _next(root, info, *args, **kwargs)
41+
42+
# Only instrument list resolvers or fields that commonly trigger N+1
43+
if info.parent_type.name not in ("Query", "Mutation"):
44+
return _next(root, info, *args, **kwargs)
45+
46+
queries_before = len(connection.queries)
47+
start = time.perf_counter()
48+
49+
result = _next(root, info, *args, **kwargs)
50+
51+
queries_after = len(connection.queries)
52+
duration_ms = (time.perf_counter() - start) * 1000
53+
query_count = queries_after - queries_before
54+
55+
if query_count > 5:
56+
logger.warning(
57+
"Potential N+1 query detected in GraphQL resolver '%s': "
58+
"%d queries executed in %.1fms. "
59+
"Consider using select_related/prefetch_related or batching.",
60+
info.field_name,
61+
query_count,
62+
duration_ms,
63+
extra={
64+
"field_name": info.field_name,
65+
"parent_type": info.parent_type.name,
66+
"query_count": query_count,
67+
"duration_ms": round(duration_ms, 2),
68+
},
69+
)
70+
71+
return result

‎django-backend/soroscan/ingest/schema.py‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,14 @@
1515
from strawberry import auto
1616
from strawberry.types import Info
1717

18-
from .cache_utils import get_or_set_json, query_cache_ttl, stable_cache_key
18+
from .cache_utils import (
19+
get_or_set_json,
20+
invalidate_contract_query_cache,
21+
invalidate_cached_contract,
22+
invalidate_event_count_cache,
23+
query_cache_ttl,
24+
stable_cache_key,
25+
)
1926
from .models import (
2027
CallGraph,
2128
ContractDependency,
@@ -33,6 +40,7 @@
3340
GraphQLResolverLoggingExtension,
3441
log_graphql_resolver,
3542
)
43+
from ..graphql_n1_detector import N1QueryDetectorExtension
3644

3745

3846
def _get_authenticated_user(info: Info):
@@ -875,6 +883,7 @@ def register_contract(
875883
team=team,
876884
metadata=metadata or {},
877885
)
886+
invalidate_cached_contract(contract_id)
878887
return contract
879888

880889
@strawberry.mutation
@@ -939,6 +948,8 @@ def set_contract_metadata(
939948
},
940949
)
941950

951+
invalidate_cached_contract(contract_id)
952+
942953
try:
943954
instance.full_clean()
944955
except ValidationError as exc:
@@ -963,6 +974,7 @@ def delete_contract_metadata(self, info: Info, contract_id: str) -> bool:
963974

964975
try:
965976
ContractMetadata.objects.get(contract__contract_id=contract_id).delete()
977+
invalidate_cached_contract(contract_id)
966978
return True
967979
except ContractMetadata.DoesNotExist:
968980
return False
@@ -1010,6 +1022,10 @@ def update_contract(
10101022

10111023
contract.save()
10121024

1025+
invalidate_cached_contract(contract_id)
1026+
invalidate_contract_query_cache(contract_id)
1027+
invalidate_event_count_cache(contract_id)
1028+
10131029
# Push notification when a contract is paused
10141030
if is_active is False:
10151031
try:
@@ -1149,5 +1165,6 @@ def get_event():
11491165
extensions=[
11501166
GraphQLRateLimitExtension,
11511167
GraphQLResolverLoggingExtension,
1168+
N1QueryDetectorExtension,
11521169
],
11531170
)

‎django-backend/soroscan/ingest/tests/test_db_explain.py‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
"""Tests for EXPLAIN ANALYZE endpoint (issue #491)."""
2+
import json
3+
4+
from django.contrib.auth.models import User
5+
from django.test import TestCase
6+
from rest_framework.test import APIRequestFactory, force_authenticate
7+
8+
from soroscan.ingest.views import db_explain_view
9+
10+
11+
class DBExplainViewTest(TestCase):
12+
"""Verify the /api/admin/db/explain/ endpoint."""
13+
14+
def setUp(self):
15+
self.factory = APIRequestFactory()
16+
self.admin_user = User.objects.create_superuser(
17+
username="admin", password="pass", email="admin@test.com"
18+
)
19+
self.regular_user = User.objects.create_user(
20+
username="regular", password="pass", email="regular@test.com"
21+
)
22+
23+
def test_admin_can_explain_simple_select(self):
24+
request = self.factory.post(
25+
"/api/admin/db/explain/",
26+
data=json.dumps({"query": "SELECT 1"}),
27+
content_type="application/json",
28+
)
29+
force_authenticate(request, user=self.admin_user)
30+
response = db_explain_view(request)
31+
32+
self.assertEqual(response.status_code, 200)
33+
data = response.data
34+
self.assertIn("query_plan", data)
35+
self.assertIsInstance(data["query_plan"], str)
36+
37+
def test_admin_can_explain_analyze(self):
38+
request = self.factory.post(
39+
"/api/admin/db/explain/",
40+
data=json.dumps({"query": "SELECT 1", "analyze": True}),
41+
content_type="application/json",
42+
)
43+
force_authenticate(request, user=self.admin_user)
44+
response = db_explain_view(request)
45+
46+
self.assertEqual(response.status_code, 200)
47+
data = response.data
48+
self.assertIn("query_plan", data)
49+
50+
def test_regular_user_rejected(self):
51+
request = self.factory.post(
52+
"/api/admin/db/explain/",
53+
data=json.dumps({"query": "SELECT 1"}),
54+
content_type="application/json",
55+
)
56+
force_authenticate(request, user=self.regular_user)
57+
response = db_explain_view(request)
58+
59+
self.assertEqual(response.status_code, 403)
60+
61+
def test_anonymous_user_rejected(self):
62+
request = self.factory.post(
63+
"/api/admin/db/explain/",
64+
data=json.dumps({"query": "SELECT 1"}),
65+
content_type="application/json",
66+
)
67+
response = db_explain_view(request)
68+
69+
self.assertIn(response.status_code, [401, 403])
70+
71+
def test_missing_query_returns_400(self):
72+
request = self.factory.post(
73+
"/api/admin/db/explain/",
74+
data=json.dumps({}),
75+
content_type="application/json",
76+
)
77+
force_authenticate(request, user=self.admin_user)
78+
response = db_explain_view(request)
79+
80+
self.assertEqual(response.status_code, 400)
81+
82+
def test_rejects_non_select_statements(self):
83+
dangerous_queries = [
84+
"DROP TABLE ingest_trackedcontract;",
85+
"DELETE FROM ingest_contractevent;",
86+
"UPDATE ingest_trackedcontract SET name='hacked';",
87+
"INSERT INTO ingest_contractevent (event_type) VALUES ('evil');",
88+
"TRUNCATE TABLE ingest_contractevent;",
89+
"ALTER TABLE ingest_trackedcontract ADD COLUMN evil BOOL;",
90+
"CREATE TABLE evil (id INT);",
91+
"GRANT ALL ON ALL TABLES IN SCHEMA public TO public;",
92+
]
93+
for sql in dangerous_queries:
94+
request = self.factory.post(
95+
"/api/admin/db/explain/",
96+
data=json.dumps({"query": sql}),
97+
content_type="application/json",
98+
)
99+
force_authenticate(request, user=self.admin_user)
100+
response = db_explain_view(request)
101+
self.assertEqual(
102+
response.status_code,
103+
400,
104+
f"Should reject: {sql[:40]}",
105+
)
106+
107+
def test_rejects_empty_string_query(self):
108+
request = self.factory.post(
109+
"/api/admin/db/explain/",
110+
data=json.dumps({"query": ""}),
111+
content_type="application/json",
112+
)
113+
force_authenticate(request, user=self.admin_user)
114+
response = db_explain_view(request)
115+
self.assertEqual(response.status_code, 400)
116+
117+
def test_select_with_rejects_mixed(self):
118+
request = self.factory.post(
119+
"/api/admin/db/explain/",
120+
data=json.dumps({"query": "SELECT 1; DROP TABLE foo"}),
121+
content_type="application/json",
122+
)
123+
force_authenticate(request, user=self.admin_user)
124+
response = db_explain_view(request)
125+
self.assertEqual(response.status_code, 400)
126+
127+
def test_with_clause_allowed(self):
128+
request = self.factory.post(
129+
"/api/admin/db/explain/",
130+
data=json.dumps({"query": "WITH cte AS (SELECT 1) SELECT * FROM cte"}),
131+
content_type="application/json",
132+
)
133+
force_authenticate(request, user=self.admin_user)
134+
response = db_explain_view(request)
135+
self.assertEqual(response.status_code, 200)

0 commit comments

Comments
 (0)