Search: stop creating SphinxDomain objects (#10451)

We are indexing sphinx domains as sections now, we no longer need to create sphinx domains or index them. We should see some small improvement in build time, and maybe in search?

This change will require a re-index, since old projects that haven't been re-build still have sphinx domain objects indexed separately from sections (this step can be done after or before the deploy).

This also fixes a small bug, we are still indexing sphinx domains titles, without their content

![Screenshot 2023-06-20 at 14-36-48 Read the Docs documentation simplified](https://github.com/readthedocs/readthedocs.org/assets/4975310/eeb1865b-a859-4010-9cff-95a95d877882)

ref #10272

Author: stsewd

readthedocs/projects/models.py

@@ -1913,7 +1913,6 @@ def add_features(sender, **kwargs):
     RECORD_404_PAGE_VIEWS = "record_404_page_views"
     ALLOW_FORCED_REDIRECTS = "allow_forced_redirects"
     DISABLE_PAGEVIEWS = "disable_pageviews"
-    DISABLE_SPHINX_DOMAINS = "disable_sphinx_domains"
     RESOLVE_PROJECT_FROM_HEADER = "resolve_project_from_header"
     USE_UNRESOLVER_WITH_PROXITO = "use_unresolver_with_proxito"
 
@@ -1999,10 +1998,6 @@ def add_features(sender, **kwargs):
             DISABLE_PAGEVIEWS,
             _("Proxito: Disable all page views"),
         ),
-        (
-            DISABLE_SPHINX_DOMAINS,
-            _("Sphinx: Disable indexing of sphinx domains"),
-        ),
         (
             RESOLVE_PROJECT_FROM_HEADER,
             _("Proxito: Allow usage of the X-RTD-Slug header"),

readthedocs/projects/tasks/search.py

@@ -1,16 +1,12 @@
-import json
 from fnmatch import fnmatch
 
 import structlog
-from django.conf import settings
-from sphinx.ext import intersphinx
 
 from readthedocs.builds.constants import EXTERNAL
 from readthedocs.builds.models import Version
-from readthedocs.projects.models import Feature, HTMLFile, ImportedFile, Project
+from readthedocs.projects.models import HTMLFile, ImportedFile, Project
 from readthedocs.projects.signals import files_changed
 from readthedocs.search.utils import index_new_files, remove_indexed_files
-from readthedocs.sphinx_domains.models import SphinxDomain
 from readthedocs.storage import build_media_storage
 from readthedocs.worker import app
 
@@ -54,14 +50,6 @@ def fileify(version_pk, commit, build, search_ranking, search_ignore):
     except Exception:
         log.exception('Failed during ImportedFile creation')
 
-    # XXX: Don't access the sphinx domains table while we migrate the ID type
-    # https://github.com/readthedocs/readthedocs.org/pull/9482.
-    if not project.has_feature(Feature.DISABLE_SPHINX_DOMAINS):
-        try:
-            _create_intersphinx_data(version, commit, build)
-        except Exception:
-            log.exception("Failed during SphinxDomain creation")
-
     try:
         _sync_imported_files(version, build)
     except Exception:
@@ -88,18 +76,6 @@ def _sync_imported_files(version, build):
         build_id=build,
     )
 
-    # Delete SphinxDomain objects from previous versions
-    # This has to be done before deleting ImportedFiles and not with a cascade,
-    # because multiple Domain's can reference a specific HTMLFile.
-    # XXX: Don't access the sphinx domains table while we migrate the ID type
-    # https://github.com/readthedocs/readthedocs.org/pull/9482.
-    if not project.has_feature(Feature.DISABLE_SPHINX_DOMAINS):
-        (
-            SphinxDomain.objects.filter(project=project, version=version)
-            .exclude(build=build)
-            .delete()
-        )
-
     # Delete ImportedFiles objects (including HTMLFiles)
     # from the previous build of the version.
     (
@@ -119,132 +95,6 @@ def remove_search_indexes(project_slug, version_slug=None):
     )
 
 
-def _create_intersphinx_data(version, commit, build):
-    """
-    Create intersphinx data for this version.
-
-    :param version: Version instance
-    :param commit: Commit that updated path
-    :param build: Build id
-    """
-    if not version.is_sphinx_type:
-        return
-
-    html_storage_path = version.project.get_storage_path(
-        type_='html', version_slug=version.slug, include_file=False
-    )
-    json_storage_path = version.project.get_storage_path(
-        type_='json', version_slug=version.slug, include_file=False
-    )
-
-    object_file = build_media_storage.join(html_storage_path, 'objects.inv')
-    if not build_media_storage.exists(object_file):
-        log.debug('No objects.inv, skipping intersphinx indexing.')
-        return
-
-    type_file = build_media_storage.join(json_storage_path, 'readthedocs-sphinx-domain-names.json')
-    types = {}
-    titles = {}
-    if build_media_storage.exists(type_file):
-        try:
-            data = json.load(build_media_storage.open(type_file))
-            types = data['types']
-            titles = data['titles']
-        except Exception:
-            log.exception('Exception parsing readthedocs-sphinx-domain-names.json')
-
-    # These classes are copied from Sphinx
-    # https://github.com/sphinx-doc/sphinx/blob/d79d041f4f90818e0b495523fdcc28db12783caf/sphinx/ext/intersphinx.py#L400-L403  # noqa
-    class MockConfig:
-        intersphinx_timeout = None
-        tls_verify = False
-        user_agent = None
-
-    class MockApp:
-        srcdir = ''
-        config = MockConfig()
-
-        def warn(self, msg):
-            log.warning('Sphinx MockApp.', msg=msg)
-
-    # Re-create all objects from the new build of the version
-    object_file_url = build_media_storage.url(object_file)
-    if object_file_url.startswith('/'):
-        # Filesystem backed storage simply prepends MEDIA_URL to the path to get the URL
-        # This can cause an issue if MEDIA_URL is not fully qualified
-        object_file_url = settings.RTD_INTERSPHINX_URL + object_file_url
-
-    invdata = intersphinx.fetch_inventory(MockApp(), '', object_file_url)
-    for key, value in sorted(invdata.items() or {}):
-        domain, _type = key.split(':', 1)
-        for name, einfo in sorted(value.items()):
-            # project, version, url, display_name
-            # ('Sphinx', '1.7.9', 'faq.html#epub-faq', 'Epub info')
-            try:
-                url = einfo[2]
-                if '#' in url:
-                    doc_name, anchor = url.split(
-                        '#',
-                        # The anchor can contain ``#`` characters
-                        maxsplit=1
-                    )
-                else:
-                    doc_name, anchor = url, ''
-                display_name = einfo[3]
-            except Exception:
-                log.exception(
-                    'Error while getting sphinx domain information. Skipping...',
-                    project_slug=version.project.slug,
-                    version_slug=version.slug,
-                    sphinx_domain=f"{domain}->{name}",
-                )
-                continue
-
-            # HACK: This is done because the difference between
-            # ``sphinx.builders.html.StandaloneHTMLBuilder``
-            # and ``sphinx.builders.dirhtml.DirectoryHTMLBuilder``.
-            # They both have different ways of generating HTML Files,
-            # and therefore the doc_name generated is different.
-            # More info on: http://www.sphinx-doc.org/en/master/usage/builders/index.html#builders
-            # Also see issue: https://github.com/readthedocs/readthedocs.org/issues/5821
-            if doc_name.endswith('/'):
-                doc_name += 'index.html'
-
-            html_file = HTMLFile.objects.filter(
-                project=version.project, version=version,
-                path=doc_name, build=build,
-            ).first()
-
-            if not html_file:
-                log.debug(
-                    'HTMLFile object not found.',
-                    project_slug=version.project.slug,
-                    version_slug=version.slug,
-                    build_id=build,
-                    doc_name=doc_name
-                )
-
-                # Don't create Sphinx Domain objects
-                # if the HTMLFile object is not found.
-                continue
-
-            SphinxDomain.objects.create(
-                project=version.project,
-                version=version,
-                html_file=html_file,
-                domain=domain,
-                name=name,
-                display_name=display_name,
-                type=_type,
-                type_display=types.get(f'{domain}:{_type}', ''),
-                doc_name=doc_name,
-                doc_display=titles.get(doc_name, ''),
-                anchor=anchor,
-                commit=commit,
-                build=build,
-            )
-
-
 def _create_imported_files(*, version, commit, build, search_ranking, search_ignore):
     """
     Create imported files for version.

readthedocs/rtd_tests/tests/test_imported_file.py

@@ -1,5 +1,4 @@
 import os
-from unittest import mock
 
 from django.conf import settings
 from django.core.files.storage import get_storage_class
@@ -9,10 +8,8 @@
 from readthedocs.projects.models import HTMLFile, ImportedFile, Project
 from readthedocs.projects.tasks.search import (
     _create_imported_files,
-    _create_intersphinx_data,
     _sync_imported_files,
 )
-from readthedocs.sphinx_domains.models import SphinxDomain
 
 base_dir = os.path.dirname(os.path.dirname(__file__))
 
@@ -176,100 +173,3 @@ def test_update_content(self):
 
         self._manage_imported_files(self.version, 'commit02', 2)
         self.assertEqual(ImportedFile.objects.count(), 2)
-
-    @override_settings(PRODUCTION_DOMAIN="readthedocs.org")
-    @override_settings(RTD_INTERSPHINX_URL="https://readthedocs.org")
-    @mock.patch("readthedocs.doc_builder.director.os.path.exists")
-    def test_create_intersphinx_data(self, mock_exists):
-        mock_exists.return_Value = True
-
-        # Test data for objects.inv file
-        test_objects_inv = {
-            'cpp:function': {
-                'sphinx.test.function': [
-                    'dummy-proj-1',
-                    'dummy-version-1',
-                    'test.html#epub-faq',  # file generated by ``sphinx.builders.html.StandaloneHTMLBuilder``
-                    'dummy-func-name-1',
-                ]
-            },
-            'py:function': {
-                'sample.test.function': [
-                    'dummy-proj-2',
-                    'dummy-version-2',
-                    'test.html#sample-test-func',  # file generated by ``sphinx.builders.html.StandaloneHTMLBuilder``
-                    'dummy-func-name-2'
-                ]
-            },
-            'js:function': {
-                'testFunction': [
-                    'dummy-proj-3',
-                    'dummy-version-3',
-                    'api/#test-func',  # file generated by ``sphinx.builders.dirhtml.DirectoryHTMLBuilder``
-                    'dummy-func-name-3'
-                ]
-            }
-        }
-
-        with mock.patch(
-            'sphinx.ext.intersphinx.fetch_inventory',
-            return_value=test_objects_inv
-        ) as mock_fetch_inventory:
-
-            _create_imported_files(
-                version=self.version,
-                commit='commit01',
-                build=1,
-                search_ranking={},
-                search_ignore=[],
-            )
-            _create_intersphinx_data(self.version, 'commit01', 1)
-
-            # there will be two html files,
-            # `api/index.html` and `test.html`
-            self.assertEqual(
-                HTMLFile.objects.all().count(),
-                2
-            )
-            self.assertEqual(
-                HTMLFile.objects.filter(path='test.html').count(),
-                1
-            )
-            self.assertEqual(
-                HTMLFile.objects.filter(path='api/index.html').count(),
-                1
-            )
-
-            html_file_api = HTMLFile.objects.filter(path='api/index.html').first()
-
-            self.assertEqual(
-                SphinxDomain.objects.all().count(),
-                3
-            )
-            self.assertEqual(
-                SphinxDomain.objects.filter(html_file=html_file_api).count(),
-                1
-            )
-            mock_fetch_inventory.assert_called_once()
-            self.assertRegex(
-                mock_fetch_inventory.call_args[0][2],
-                r'^https://readthedocs\.org/media/.*/objects\.inv$'
-            )
-        self.assertEqual(ImportedFile.objects.count(), 2)
-
-    @override_settings(RTD_INTERSPHINX_URL="http://localhost:8080")
-    @mock.patch("readthedocs.doc_builder.director.os.path.exists")
-    def test_custom_intersphinx_url(self, mock_exists):
-        mock_exists.return_Value = True
-
-        with mock.patch(
-            'sphinx.ext.intersphinx.fetch_inventory',
-            return_value={}
-        ) as mock_fetch_inventory:
-            _create_intersphinx_data(self.version, 'commit01', 1)
-
-            mock_fetch_inventory.assert_called_once()
-            self.assertRegex(
-                mock_fetch_inventory.call_args[0][2],
-                '^http://localhost:8080/media/.*/objects.inv$'
-            )

readthedocs/search/api/v2/serializers.py

@@ -6,7 +6,6 @@
    They should be renamed in the ES index too.
 """
 
-import itertools
 import re
 from functools import namedtuple
 from operator import attrgetter
@@ -157,52 +156,16 @@ def _get_full_path(self, obj):
 
     def get_blocks(self, obj):
         """Combine and sort inner results (domains and sections)."""
-        serializers = {
-            "domain": DomainSearchSerializer,
-            "section": SectionSearchSerializer,
-        }
-
-        inner_hits = obj.meta.inner_hits
-        sections = inner_hits.sections or []
-        domains = inner_hits.domains or []
-
-        # Make them identifiable before merging them
-        for s in sections:
-            s.type = "section"
-        for d in domains:
-            d.type = "domain"
-
+        sections = obj.meta.inner_hits.sections or []
         sorted_results = sorted(
-            itertools.chain(sections, domains),
+            sections,
             key=attrgetter("meta.score"),
             reverse=True,
         )
-        sorted_results = [serializers[hit.type](hit).data for hit in sorted_results]
+        sorted_results = [SectionSearchSerializer(hit).data for hit in sorted_results]
         return sorted_results
 
 
-class DomainHighlightSerializer(serializers.Serializer):
-
-    name = serializers.SerializerMethodField()
-    content = serializers.SerializerMethodField()
-
-    def get_name(self, obj):
-        return list(getattr(obj, "domains.name", []))
-
-    def get_content(self, obj):
-        return list(getattr(obj, "domains.docstrings", []))
-
-
-class DomainSearchSerializer(serializers.Serializer):
-
-    type = serializers.CharField(default="domain", source=None, read_only=True)
-    role = serializers.CharField(source="role_name")
-    name = serializers.CharField()
-    id = serializers.CharField(source="anchor")
-    content = serializers.CharField(source="docstrings")
-    highlights = DomainHighlightSerializer(source="meta.highlight", default=dict)
-
-
 class SectionHighlightSerializer(serializers.Serializer):
 
     title = serializers.SerializerMethodField()