Skip to content

Commit 140be4a

Browse files
authored
Addons: remove fields that generate unnecessary queries (#12336)
~10ms off
1 parent 127d672 commit 140be4a

File tree

3 files changed

+35
-30
lines changed

3 files changed

+35
-30
lines changed

readthedocs/proxito/tests/responses/v1.json

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,14 @@
2828
"versioning_scheme": "multiple_versions_with_translations",
2929
"slug": "project",
3030
"subproject_of": null,
31-
"tags": ["project", "tag", "test"],
3231
"translation_of": null,
3332
"urls": {
3433
"builds": "https://readthedocs.org/projects/project/builds/",
3534
"documentation": "https://project.dev.readthedocs.io/en/latest/",
3635
"home": "https://readthedocs.org/projects/project/",
3736
"versions": "https://readthedocs.org/projects/project/versions/",
3837
"downloads": null
39-
},
40-
"users": [
41-
{
42-
"username": "testuser"
43-
}
44-
]
38+
}
4539
}
4640
},
4741
"versions": {

readthedocs/proxito/tests/test_hosting.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -785,7 +785,7 @@ def test_number_of_queries_project_version_slug(self):
785785
active=True,
786786
)
787787

788-
with self.assertNumQueries(16):
788+
with self.assertNumQueries(14):
789789
r = self.client.get(
790790
reverse("proxito_readthedocs_docs_addons"),
791791
{
@@ -814,7 +814,7 @@ def test_number_of_queries_url(self):
814814
active=True,
815815
)
816816

817-
with self.assertNumQueries(17):
817+
with self.assertNumQueries(15):
818818
r = self.client.get(
819819
reverse("proxito_readthedocs_docs_addons"),
820820
{
@@ -850,7 +850,7 @@ def test_number_of_queries_url_subproject(self):
850850
active=True,
851851
)
852852

853-
with self.assertNumQueries(20):
853+
with self.assertNumQueries(18):
854854
r = self.client.get(
855855
reverse("proxito_readthedocs_docs_addons"),
856856
{
@@ -866,7 +866,7 @@ def test_number_of_queries_url_subproject(self):
866866
assert r.status_code == 200
867867

868868
# Test parent project has fewer queries
869-
with self.assertNumQueries(16):
869+
with self.assertNumQueries(14):
870870
r = self.client.get(
871871
reverse("proxito_readthedocs_docs_addons"),
872872
{
@@ -892,7 +892,7 @@ def test_number_of_queries_url_translations(self):
892892
language=language,
893893
)
894894

895-
with self.assertNumQueries(29):
895+
with self.assertNumQueries(24):
896896
r = self.client.get(
897897
reverse("proxito_readthedocs_docs_addons"),
898898
{
@@ -1042,7 +1042,7 @@ def test_file_tree_diff(self, get_manifest):
10421042
],
10431043
),
10441044
]
1045-
with self.assertNumQueries(24):
1045+
with self.assertNumQueries(22):
10461046
r = self.client.get(
10471047
reverse("proxito_readthedocs_docs_addons"),
10481048
{

readthedocs/proxito/views/hosting.py

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -223,10 +223,10 @@ def get(self, request, *args, **kwargs):
223223
return Response(data)
224224

225225

226-
class NoLinksMixin:
227-
"""Mixin to remove conflicting fields from serializers."""
226+
class RemoveFieldsMixin:
227+
"""Mixin to remove fields from serializers."""
228228

229-
FIELDS_TO_REMOVE = ("_links",)
229+
FIELDS_TO_REMOVE = []
230230

231231
def __init__(self, *args, **kwargs):
232232
super().__init__(*args, **kwargs)
@@ -245,20 +245,32 @@ def __init__(self, *args, **kwargs):
245245
# on El Proxito.
246246
#
247247
# See https://github.com/readthedocs/readthedocs-ops/issues/1323
248-
class RelatedProjectSerializerNoLinks(NoLinksMixin, RelatedProjectSerializer):
249-
pass
248+
class RelatedProjectAddonsSerializer(RemoveFieldsMixin, RelatedProjectSerializer):
249+
FIELDS_TO_REMOVE = [
250+
"_links",
251+
]
250252

251253

252-
class ProjectSerializerNoLinks(NoLinksMixin, ProjectSerializer):
253-
related_project_serializer = RelatedProjectSerializerNoLinks
254+
class ProjectAddonsSerializer(RemoveFieldsMixin, ProjectSerializer):
255+
FIELDS_TO_REMOVE = [
256+
"_links",
257+
# users and tags result in additional queries for fields that we don't use.
258+
"users",
259+
"tags",
260+
]
261+
related_project_serializer = RelatedProjectAddonsSerializer
254262

255263

256-
class VersionSerializerNoLinks(NoLinksMixin, VersionSerializer):
257-
pass
264+
class VersionAddonsSerializer(RemoveFieldsMixin, VersionSerializer):
265+
FIELDS_TO_REMOVE = [
266+
"_links",
267+
]
258268

259269

260-
class BuildSerializerNoLinks(NoLinksMixin, BuildSerializer):
261-
pass
270+
class BuildAddonsSerializer(RemoveFieldsMixin, BuildSerializer):
271+
FIELDS_TO_REMOVE = [
272+
"_links",
273+
]
262274

263275

264276
class AddonsResponseBase:
@@ -371,21 +383,21 @@ def _v1(self, project, version, build, filename, url, request):
371383
request=request, project=project, version=version, resolver=resolver
372384
),
373385
"versions": {
374-
"current": VersionSerializerNoLinks(
386+
"current": VersionAddonsSerializer(
375387
version,
376388
resolver=resolver,
377389
).data
378390
if version
379391
else None,
380392
# These are "sorted active, built, not hidden versions"
381-
"active": VersionSerializerNoLinks(
393+
"active": VersionAddonsSerializer(
382394
sorted_versions_active_built_not_hidden,
383395
resolver=resolver,
384396
many=True,
385397
).data,
386398
},
387399
"builds": {
388-
"current": BuildSerializerNoLinks(build).data if build else None,
400+
"current": BuildAddonsSerializer(build).data if build else None,
389401
},
390402
# TODO: consider creating one serializer per field here.
391403
# The resulting JSON will be the same, but maybe it's easier/cleaner?
@@ -594,14 +606,13 @@ def _get_projects_response(self, *, request, project, version, resolver):
594606
# NOTE: there is no need to prefetch superprojects,
595607
# as translations are not expected to have superprojects,
596608
# and the serializer already checks for that.
597-
.prefetch_related("tags", "domains", "users")
598609
)
599610
# NOTE: we check if there are translations first,
600611
# otherwise evaluating the queryset will be more expensive
601612
# even if there are no results. Django optimizes the queryset
602613
# if only we need to check if there are results or not.
603614
if translations_qs.exists():
604-
translations = ProjectSerializerNoLinks(
615+
translations = ProjectAddonsSerializer(
605616
translations_qs,
606617
resolver=resolver,
607618
version=version,
@@ -611,7 +622,7 @@ def _get_projects_response(self, *, request, project, version, resolver):
611622
translations = []
612623

613624
return {
614-
"current": ProjectSerializerNoLinks(
625+
"current": ProjectAddonsSerializer(
615626
project,
616627
resolver=resolver,
617628
version=version,

0 commit comments

Comments
 (0)