Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.

Commit b731bdf

Browse files
committed
perf: replace previous query for GQL test results
this commit replaces the previous django orm code with an invocation to do a raw sql query that performs much better for repos with a large amount of data. one worry with this approach is the risk for sql injection in this specific case, we're passing most of the dynamic user provided values used in the sql through the parameters which the django docs say are sanitized and should be used to protect against sql injection for the user provided values that are not passed through the sql parameters they're checked to be in a strict set of values, so we shouldn't be substituting any unexpected strings into the query
1 parent 3ee723e commit b731bdf

File tree

7 files changed

+367
-181
lines changed

7 files changed

+367
-181
lines changed

graphql_api/tests/test_test_analytics.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ def test_failed_filter_on_test_results(self) -> None:
167167
assert res["testResults"] == {"edges": [{"node": {"name": test2.name}}]}
168168

169169
def test_skipped_filter_on_test_results(self) -> None:
170+
# note - this test guards against division by zero errors for the failure/flake rate
170171
repo = RepositoryFactory(author=self.owner, active=True, private=True)
171172
test = TestFactory(repository=repo)
172173
test2 = TestFactory(repository=repo)
@@ -367,8 +368,8 @@ def test_last_duration_ordering_on_test_results(self) -> None:
367368
)
368369
assert res["testResults"] == {
369370
"edges": [
370-
{"node": {"name": test.name, "lastDuration": 0.0}},
371-
{"node": {"name": test_2.name, "lastDuration": 0.0}},
371+
{"node": {"name": test.name, "lastDuration": 2.0}},
372+
{"node": {"name": test_2.name, "lastDuration": 3.0}},
372373
]
373374
}
374375

@@ -402,8 +403,8 @@ def test_desc_last_duration_ordering_on_test_results(self) -> None:
402403
)
403404
assert res["testResults"] == {
404405
"edges": [
405-
{"node": {"name": test_2.name, "lastDuration": 0.0}},
406-
{"node": {"name": test.name, "lastDuration": 0.0}},
406+
{"node": {"name": test_2.name, "lastDuration": 3.0}},
407+
{"node": {"name": test.name, "lastDuration": 2.0}},
407408
]
408409
}
409410

@@ -924,7 +925,7 @@ def test_flake_aggregates_7_days(self) -> None:
924925

925926
res = self.fetch_test_analytics(
926927
repo.name,
927-
"""flakeAggregates(history: INTERVAL_7_DAY) { flakeCount, flakeRate, flakeCountPercentChange, flakeRatePercentChange }""",
928+
"""flakeAggregates(interval: INTERVAL_7_DAY) { flakeCount, flakeRate, flakeCountPercentChange, flakeRatePercentChange }""",
928929
)
929930

930931
assert res["flakeAggregates"] == {

graphql_api/tests/test_test_result.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ def test_fetch_test_result_last_duration(self) -> None:
197197
result["owner"]["repository"]["testAnalytics"]["testResults"]["edges"][0][
198198
"node"
199199
]["lastDuration"]
200-
== 0.0
200+
== 1.0
201201
)
202202

203203
def test_fetch_test_result_avg_duration(self) -> None:

graphql_api/types/inputs/test_results_filters.graphql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ input TestResultsFilters {
33
parameter: TestResultsFilterParameter
44
test_suites: [String!]
55
flags: [String!]
6-
history: MeasurementInterval
6+
interval: MeasurementInterval
77
term: String
88
}
99

graphql_api/types/test_analytics/test_analytics.graphql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ type TestAnalytics {
1313
): TestResultConnection! @cost(complexity: 10, multipliers: ["first", "last"])
1414

1515
"Test results aggregates are analytics data totals across all tests"
16-
testResultsAggregates(history: MeasurementInterval): TestResultsAggregates
16+
testResultsAggregates(interval: MeasurementInterval): TestResultsAggregates
1717

1818
"Flake aggregates are flake totals across all tests"
19-
flakeAggregates(history: MeasurementInterval): FlakeAggregates
19+
flakeAggregates(interval: MeasurementInterval): FlakeAggregates
2020
}
2121

2222
type TestResultConnection {

graphql_api/types/test_analytics/test_analytics.py

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
from codecov.db import sync_to_async
88
from core.models import Repository
9-
from graphql_api.helpers.connection import queryset_to_connection
109
from graphql_api.types.enums import OrderingDirection, TestResultsFilterParameter
1110
from graphql_api.types.enums.enum_types import MeasurementInterval
1211
from utils.test_results import (
@@ -28,70 +27,72 @@ async def resolve_test_results(
2827
info: GraphQLResolveInfo,
2928
ordering=None,
3029
filters=None,
31-
**kwargs,
30+
first: int | None = None,
31+
after: str | None = None,
32+
last: int | None = None,
33+
before: str | None = None,
3234
):
3335
parameter = (
3436
convert_test_results_filter_parameter(filters.get("parameter"))
3537
if filters
3638
else None
3739
)
38-
history = (
39-
convert_history_to_timedelta(filters.get("history"))
40+
interval = (
41+
convert_interval_to_timedelta(filters.get("interval"))
4042
if filters
4143
else timedelta(days=30)
4244
)
4345

4446
queryset = await sync_to_async(generate_test_results)(
45-
repoid=repository.repoid,
46-
history=history,
47-
branch=filters.get("branch") if filters else None,
48-
parameter=parameter,
49-
testsuites=filters.get("test_suites") if filters else None,
50-
flags=filters.get("flags") if filters else None,
51-
term=filters.get("term") if filters else None,
52-
)
53-
54-
return await queryset_to_connection(
55-
queryset,
5647
ordering=(
57-
(ordering.get("parameter"), "name")
48+
(ordering.get("parameter").value, "name")
5849
if ordering
5950
else ("avg_duration", "name")
6051
),
6152
ordering_direction=(
6253
ordering.get("direction") if ordering else OrderingDirection.DESC
6354
),
64-
**kwargs,
55+
repoid=repository.repoid,
56+
interval=interval,
57+
first=first,
58+
after=after,
59+
last=last,
60+
before=before,
61+
branch=filters.get("branch") if filters else None,
62+
parameter=parameter,
63+
testsuites=filters.get("test_suites") if filters else None,
64+
flags=filters.get("flags") if filters else None,
65+
term=filters.get("term") if filters else None,
6566
)
6667

68+
return queryset
69+
6770

6871
@test_analytics_bindable.field("testResultsAggregates")
6972
async def resolve_test_results_aggregates(
7073
repository: Repository,
7174
info: GraphQLResolveInfo,
72-
history: MeasurementInterval | None = None,
75+
interval: MeasurementInterval | None = None,
7376
**_,
7477
):
75-
history = convert_history_to_timedelta(history)
7678
return await sync_to_async(generate_test_results_aggregates)(
77-
repoid=repository.repoid, history=history
79+
repoid=repository.repoid, interval=convert_interval_to_timedelta(interval)
7880
)
7981

8082

8183
@test_analytics_bindable.field("flakeAggregates")
8284
async def resolve_flake_aggregates(
8385
repository: Repository,
8486
info: GraphQLResolveInfo,
85-
history: MeasurementInterval | None = None,
87+
interval: MeasurementInterval | None = None,
8688
**_,
8789
):
88-
history = convert_history_to_timedelta(history)
8990
return await sync_to_async(generate_flake_aggregates)(
90-
repoid=repository.repoid, history=history
91+
repoid=repository.repoid, interval=convert_interval_to_timedelta(interval)
9192
)
9293

9394

94-
def convert_history_to_timedelta(interval: MeasurementInterval | None) -> timedelta:
95+
def convert_interval_to_timedelta(interval: MeasurementInterval | None) -> timedelta:
9596
if interval is None:
9697
return timedelta(days=30)
9798

Lines changed: 21 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,71 +1,58 @@
11
from datetime import datetime
2-
from typing import TypedDict
32

43
from ariadne import ObjectType
54
from graphql import GraphQLResolveInfo
65

7-
8-
class TestDict(TypedDict):
9-
name: str
10-
updated_at: datetime
11-
commits_where_fail: int
12-
failure_rate: float
13-
avg_duration: float
14-
last_duration: float
15-
flake_rate: float
16-
total_fail_count: int
17-
total_skip_count: int
18-
total_pass_count: int
19-
6+
from utils.test_results import TestResultsRow
207

218
test_result_bindable = ObjectType("TestResult")
229

2310

2411
@test_result_bindable.field("name")
25-
def resolve_name(test: TestDict, _: GraphQLResolveInfo) -> str:
26-
return test["name"].replace("\x1f", " ")
12+
def resolve_name(test: TestResultsRow, _: GraphQLResolveInfo) -> str:
13+
return test.name.replace("\x1f", " ")
2714

2815

2916
@test_result_bindable.field("updatedAt")
30-
def resolve_updated_at(test: TestDict, _: GraphQLResolveInfo) -> datetime:
31-
return test["updated_at"]
17+
def resolve_updated_at(test: TestResultsRow, _: GraphQLResolveInfo) -> datetime:
18+
return test.updated_at
3219

3320

3421
@test_result_bindable.field("commitsFailed")
35-
def resolve_commits_failed(test: TestDict, _: GraphQLResolveInfo) -> int:
36-
return test["commits_where_fail"]
22+
def resolve_commits_failed(test: TestResultsRow, _: GraphQLResolveInfo) -> int:
23+
return test.commits_where_fail
3724

3825

3926
@test_result_bindable.field("failureRate")
40-
def resolve_failure_rate(test: TestDict, _: GraphQLResolveInfo) -> float:
41-
return test["failure_rate"]
27+
def resolve_failure_rate(test: TestResultsRow, _: GraphQLResolveInfo) -> float:
28+
return test.failure_rate
4229

4330

4431
@test_result_bindable.field("flakeRate")
45-
def resolve_flake_rate(test: TestDict, _: GraphQLResolveInfo) -> float:
46-
return test["flake_rate"]
32+
def resolve_flake_rate(test: TestResultsRow, _: GraphQLResolveInfo) -> float:
33+
return test.flake_rate
4734

4835

4936
@test_result_bindable.field("avgDuration")
50-
def resolve_avg_duration(test: TestDict, _: GraphQLResolveInfo) -> float:
51-
return test["avg_duration"]
37+
def resolve_avg_duration(test: TestResultsRow, _: GraphQLResolveInfo) -> float:
38+
return test.avg_duration
5239

5340

5441
@test_result_bindable.field("lastDuration")
55-
def resolve_last_duration(test: TestDict, _: GraphQLResolveInfo) -> float:
56-
return test["last_duration"]
42+
def resolve_last_duration(test: TestResultsRow, _: GraphQLResolveInfo) -> float:
43+
return test.last_duration
5744

5845

5946
@test_result_bindable.field("totalFailCount")
60-
def resolve_total_fail_count(test: TestDict, _: GraphQLResolveInfo) -> int:
61-
return test["total_fail_count"]
47+
def resolve_total_fail_count(test: TestResultsRow, _: GraphQLResolveInfo) -> int:
48+
return test.total_fail_count
6249

6350

6451
@test_result_bindable.field("totalSkipCount")
65-
def resolve_total_skip_count(test: TestDict, _: GraphQLResolveInfo) -> int:
66-
return test["total_skip_count"]
52+
def resolve_total_skip_count(test: TestResultsRow, _: GraphQLResolveInfo) -> int:
53+
return test.total_skip_count
6754

6855

6956
@test_result_bindable.field("totalPassCount")
70-
def resolve_total_pass_count(test: TestDict, _: GraphQLResolveInfo) -> int:
71-
return test["total_pass_count"]
57+
def resolve_total_pass_count(test: TestResultsRow, _: GraphQLResolveInfo) -> int:
58+
return test.total_pass_count

0 commit comments

Comments
 (0)