Skip to content

Commit 485c200

Browse files
committed
Search: refactor serializer's context
We were generating the context of the serializer from outside, but we can do this from the serializer itself, this way we don't have to duplicate this logic.
1 parent 9f219b8 commit 485c200

File tree

4 files changed

+42
-93
lines changed

4 files changed

+42
-93
lines changed

readthedocs/search/api/v2/serializers.py

+27-3
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,11 @@ class PageSearchSerializer(serializers.Serializer):
6161
"""
6262
Page serializer.
6363
64-
If ``projects_data`` is passed into the context, the serializer
65-
will try to use that to generate the link before querying the database.
66-
It's a dictionary mapping the project slug to a ProjectData object.
64+
If ``projects`` is passed in the constructor, the serializer
65+
will pre-generate a cache with that information,
66+
this is to avoid querying the database again for each result.
67+
68+
:param projects: A list of tuples of project and version.
6769
"""
6870

6971
type = serializers.CharField(default="page", source=None, read_only=True)
@@ -76,6 +78,28 @@ class PageSearchSerializer(serializers.Serializer):
7678
highlights = PageHighlightSerializer(source="meta.highlight", default=dict)
7779
blocks = serializers.SerializerMethodField()
7880

81+
def __init__(self, *args, projects=None, **kwargs):
82+
if projects:
83+
context = kwargs.setdefault("context", {})
84+
context["projects_data"] = {
85+
project.slug: self._build_project_data(project, version.slug)
86+
for project, version in projects
87+
}
88+
super().__init__(*args, **kwargs)
89+
90+
def _build_project_data(self, project, version_slug):
91+
"""Build a `ProjectData` object given a project and its version."""
92+
url = project.get_docs_url(version_slug=version_slug)
93+
project_alias = project.superprojects.values_list("alias", flat=True).first()
94+
version_data = VersionData(
95+
slug=version_slug,
96+
docs_url=url,
97+
)
98+
return ProjectData(
99+
alias=project_alias,
100+
version=version_data,
101+
)
102+
79103
def _get_project_data(self, obj):
80104
"""
81105
Get and cache the project data.

readthedocs/search/api/v2/views.py

+12-64
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,7 @@
1414
from readthedocs.projects.models import Feature, Project
1515
from readthedocs.search import tasks
1616
from readthedocs.search.api import SearchPagination
17-
from readthedocs.search.api.v2.serializers import (
18-
PageSearchSerializer,
19-
ProjectData,
20-
VersionData,
21-
)
17+
from readthedocs.search.api.v2.serializers import PageSearchSerializer
2218
from readthedocs.search.faceted_search import PageSearch
2319

2420
log = structlog.get_logger(__name__)
@@ -80,45 +76,15 @@ def _validate_query_params(self):
8076
raise ValidationError(errors)
8177

8278
@lru_cache(maxsize=1)
83-
def _get_all_projects_data(self):
84-
"""
85-
Return a dictionary of the project itself and all its subprojects.
86-
87-
Example:
88-
89-
.. code::
90-
91-
{
92-
"requests": ProjectData(
93-
alias='alias',
94-
version=VersionData(
95-
"latest",
96-
"https://requests.readthedocs.io/en/latest/",
97-
),
98-
),
99-
"requests-oauth": ProjectData(
100-
alias=None,
101-
version=VersionData(
102-
"latest",
103-
"https://requests-oauth.readthedocs.io/en/latest/",
104-
),
105-
),
106-
}
107-
108-
.. note:: The response is cached into the instance.
109-
110-
:rtype: A dictionary of project slugs mapped to a `VersionData` object.
111-
"""
79+
def _get_projects_to_search(self):
80+
"""Get all projects to search."""
11281
main_version = self._get_version()
11382
main_project = self._get_project()
11483

11584
if not self._has_permission(self.request.user, main_version):
116-
return {}
117-
118-
projects_data = {
119-
main_project.slug: self._get_project_data(main_project, main_version),
120-
}
85+
return []
12186

87+
projects_to_search = [(main_project, main_version)]
12288
subprojects = Project.objects.filter(superprojects__parent_id=main_project.id)
12389
for subproject in subprojects:
12490
version = self._get_project_version(
@@ -136,24 +102,9 @@ def _get_all_projects_data(self):
136102
)
137103

138104
if version and self._has_permission(self.request.user, version):
139-
projects_data[subproject.slug] = self._get_project_data(
140-
subproject, version
141-
)
105+
projects_to_search.append((subproject, version))
142106

143-
return projects_data
144-
145-
def _get_project_data(self, project, version):
146-
"""Build a `ProjectData` object given a project and its version."""
147-
url = project.get_docs_url(version_slug=version.slug)
148-
project_alias = project.superprojects.values_list("alias", flat=True).first()
149-
version_data = VersionData(
150-
slug=version.slug,
151-
docs_url=url,
152-
)
153-
return ProjectData(
154-
alias=project_alias,
155-
version=version_data,
156-
)
107+
return projects_to_search
157108

158109
def _get_project_version(self, project, version_slug, include_hidden=True):
159110
"""
@@ -219,8 +170,8 @@ def get_queryset(self):
219170
is compatible with DRF's paginator.
220171
"""
221172
projects = {
222-
project: project_data.version.slug
223-
for project, project_data in self._get_all_projects_data().items()
173+
project.slug: version.slug
174+
for project, version in self._get_projects_to_search()
224175
}
225176
# Check to avoid searching all projects in case it's empty.
226177
if not projects:
@@ -236,11 +187,6 @@ def get_queryset(self):
236187
)
237188
return queryset
238189

239-
def get_serializer_context(self):
240-
context = super().get_serializer_context()
241-
context["projects_data"] = self._get_all_projects_data()
242-
return context
243-
244190
def get(self, request, *args, **kwargs):
245191
self._validate_query_params()
246192
result = self.list()
@@ -255,7 +201,9 @@ def list(self):
255201
self.request,
256202
view=self,
257203
)
258-
serializer = self.get_serializer(page, many=True)
204+
serializer = self.get_serializer(
205+
page, many=True, projects=self._get_projects_to_search()
206+
)
259207
return self.paginator.get_paginated_response(serializer.data)
260208

261209

readthedocs/search/tests/test_api.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -342,9 +342,9 @@ def test_doc_search_unexisting_version(self, api_client, project):
342342
resp = self.get_search(api_client, search_params)
343343
assert resp.status_code == 404
344344

345-
@mock.patch.object(PageSearchAPIView, '_get_all_projects_data', dict)
345+
@mock.patch.object(PageSearchAPIView, "_get_projects_to_search", list)
346346
def test_get_all_projects_returns_empty_results(self, api_client, project):
347-
"""If there is a case where `_get_all_projects` returns empty, we could be querying all projects."""
347+
"""If there is a case where `_get_projects_to_search` returns empty, we could be querying all projects."""
348348

349349
# `documentation` word is present both in `kuma` and `docs` files
350350
# and not in `pipeline`, so search with this phrase but filter through project

readthedocs/search/views.py

+1-24
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,7 @@
1010
from readthedocs.projects.models import Feature, Project
1111
from readthedocs.search.api.v2.serializers import (
1212
PageSearchSerializer,
13-
ProjectData,
1413
ProjectSearchSerializer,
15-
VersionData,
1614
)
1715
from readthedocs.search.faceted_search import ALL_FACETS, PageSearch, ProjectSearch
1816

@@ -91,26 +89,6 @@ def _get_project(self, project_slug):
9189
project = get_object_or_404(queryset, slug=project_slug)
9290
return project
9391

94-
def _get_project_data(self, project, version_slug):
95-
docs_url = project.get_docs_url(version_slug=version_slug)
96-
version_data = VersionData(
97-
slug=version_slug,
98-
docs_url=docs_url,
99-
)
100-
project_data = {
101-
project.slug: ProjectData(
102-
alias=None,
103-
version=version_data,
104-
)
105-
}
106-
return project_data
107-
108-
def get_serializer_context(self, project, version_slug):
109-
context = {
110-
'projects_data': self._get_project_data(project, version_slug),
111-
}
112-
return context
113-
11492
def get(self, request, project_slug):
11593
project_obj = self._get_project(project_slug)
11694
use_advanced_query = not project_obj.has_feature(
@@ -132,8 +110,7 @@ def get(self, request, project_slug):
132110
use_advanced_query=use_advanced_query,
133111
)
134112

135-
context = self.get_serializer_context(project_obj, user_input.version)
136-
results = PageSearchSerializer(results, many=True, context=context).data
113+
results = PageSearchSerializer(results, many=True).data
137114

138115
template_context = user_input._asdict()
139116
template_context.update({

0 commit comments

Comments
 (0)