diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 02bcbe6ad9c..8261238608e 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -691,6 +691,15 @@ def save(self, *args, **kwargs): def delete(self, *args, **kwargs): from readthedocs.projects.tasks.utils import clean_project_resources + # NOTE: We use _raw_delete to avoid Django fetching all objects + # before the deletion. Be careful when using _raw_delete, signals + # won't be sent, and can cause integrity problems if the model + # has relations with other models. + qs = self.page_views.all() + qs._raw_delete(qs.db) + qs = self.search_queries.all() + qs._raw_delete(qs.db) + # Remove extra resources clean_project_resources(self) diff --git a/readthedocs/projects/tasks/utils.py b/readthedocs/projects/tasks/utils.py index 52a23882e36..92eb5345535 100644 --- a/readthedocs/projects/tasks/utils.py +++ b/readthedocs/projects/tasks/utils.py @@ -87,6 +87,13 @@ def clean_project_resources(project, version=None, version_slug=None): sometimes to clean old resources. .. note:: + + This function shouldn't delete objects that can't be recreated + by re-activating the version (e.g. page views, search queries), + as it's used when a version is deactivated. + + .. note:: + This function is usually called just before deleting project. Make sure to not depend on the project object inside the tasks. """ @@ -108,11 +115,16 @@ def clean_project_resources(project, version=None, version_slug=None): version_slug=version_slug, ) - # Remove imported files + # NOTE: We use _raw_delete to avoid Django fetching all objects + # before the deletion. Be careful when using _raw_delete, signals + # won't be sent, and can cause integrity problems if the model + # has relations with other models. if version: - version.imported_files.all().delete() + qs = version.imported_files.all() + qs._raw_delete(qs.db) else: - project.imported_files.all().delete() + qs = project.imported_files.all() + qs._raw_delete(qs.db) @app.task() diff --git a/readthedocs/projects/tests/test_models.py b/readthedocs/projects/tests/test_models.py index 6f6512a961f..2d59b289340 100644 --- a/readthedocs/projects/tests/test_models.py +++ b/readthedocs/projects/tests/test_models.py @@ -3,7 +3,10 @@ from django.test import TestCase from django_dynamic_fixture import get -from readthedocs.projects.models import Feature, Project +from readthedocs.analytics.models import PageView +from readthedocs.builds.models import Build, Version +from readthedocs.projects.models import Feature, ImportedFile, Project +from readthedocs.search.models import SearchQuery class TestURLPatternsUtils(TestCase): @@ -137,3 +140,15 @@ def test_proxied_api_prefix(self): self.assertEqual(self.project.proxied_api_url, "prefix/_/") self.assertEqual(self.project.proxied_api_host, "/prefix/_") self.assertEqual(self.project.proxied_api_prefix, "/prefix/") + + def test_number_of_queries_on_project_deletion(self): + for i in range(5): + version = get(Version, project=self.project, slug=f"subproject-{i}", active=True, built=True) + for _ in range(50): + get(PageView, project=self.project, version=version) + get(ImportedFile, project=self.project, version=version) + get(SearchQuery, project=self.project, version=version) + get(Build, project=self.project, version=version) + + with self.assertNumQueries(48): + self.project.delete()