Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API V3: clean version when deactivated and build version when activated #10308

Merged
merged 11 commits into from
May 22, 2023
5 changes: 5 additions & 0 deletions docs/user/api/v3.rst
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,11 @@ Version update

Update a version.

When a version is deactivated, its documentation is removed,
and when it's activated, a new build is triggered.
stsewd marked this conversation as resolved.
Show resolved Hide resolved

Updates to a version also invalidates its CDN cache.

**Example request**:

.. tabs::
Expand Down
55 changes: 54 additions & 1 deletion readthedocs/api/v3/tests/test_versions.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest import mock

import django_dynamic_fixture as fixture
from django.test import override_settings
from django.urls import reverse
Expand Down Expand Up @@ -139,6 +141,9 @@ def test_projects_versions_detail_unique(self):
)
self.assertEqual(response.status_code, 200)

@mock.patch(
"readthedocs.projects.tasks.utils.clean_project_resources", new=mock.MagicMock
)
def test_projects_versions_partial_update(self):
self.assertTrue(self.version.active)
self.assertFalse(self.version.hidden)
Expand Down Expand Up @@ -166,7 +171,7 @@ def test_projects_versions_partial_update(self):
self.assertEqual(self.version.identifier, 'a1b2c3')
self.assertFalse(self.version.active)
self.assertTrue(self.version.hidden)
self.assertTrue(self.version.built)
self.assertFalse(self.version.built)
self.assertEqual(self.version.type, TAG)

def test_projects_versions_partial_update_privacy_levels_disabled(self):
Expand Down Expand Up @@ -227,6 +232,54 @@ def test_projects_versions_partial_update_invalid_privacy_levels(self):
self.assertEqual(response.status_code, 400)
self.assertEqual(self.version.privacy_level, "public")

@mock.patch("readthedocs.builds.models.trigger_build")
@mock.patch("readthedocs.projects.tasks.utils.clean_project_resources")
def test_activate_version(self, clean_project_resources, trigger_build):
self.version.active = False
self.version.save()
self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
self.assertFalse(self.version.active)
data = {"active": True}
response = self.client.patch(
reverse(
"projects-versions-detail",
kwargs={
"parent_lookup_project__slug": self.project.slug,
"version_slug": self.version.slug,
},
),
data,
)
self.assertEqual(response.status_code, 204)
self.version.refresh_from_db()
self.assertTrue(self.version.active)
clean_project_resources.assert_not_called()
trigger_build.assert_called_once()

@mock.patch("readthedocs.builds.models.trigger_build")
@mock.patch("readthedocs.projects.tasks.utils.clean_project_resources")
def test_deactivate_version(self, clean_project_resources, trigger_build):
self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
data = {"active": False}
self.assertTrue(self.version.active)
self.assertTrue(self.version.built)
response = self.client.patch(
reverse(
"projects-versions-detail",
kwargs={
"parent_lookup_project__slug": self.project.slug,
"version_slug": self.version.slug,
},
),
data,
)
self.assertEqual(response.status_code, 204)
self.version.refresh_from_db()
self.assertFalse(self.version.active)
self.assertFalse(self.version.built)
clean_project_resources.assert_called_once()
trigger_build.assert_not_called()

def test_projects_version_external(self):
self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
self.version.type = EXTERNAL
Expand Down
11 changes: 11 additions & 0 deletions readthedocs/api/v3/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,17 @@ def get_serializer_class(self):
return VersionSerializer
return VersionUpdateSerializer

def update(self, request, *args, **kwargs):
"""Overridden to call ``post_save`` method on the updated version."""
# Get the current value before updating.
version = self.get_object()
was_active = version.active
result = super().update(request, *args, **kwargs)
# Get the updated version.
version = self.get_object()
version.post_save(was_active=was_active)
return result


class BuildsViewSet(APIv3Settings, NestedViewSetMixin, ProjectQuerySetMixin,
FlexFieldsMixin, ReadOnlyModelViewSet):
Expand Down
10 changes: 4 additions & 6 deletions readthedocs/builds/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
Version,
VersionAutomationRule,
)
from readthedocs.builds.signals import version_changed
from readthedocs.core.utils import trigger_build


class VersionForm(forms.ModelForm):
Expand Down Expand Up @@ -67,6 +65,9 @@ def __init__(self, *args, **kwargs):

self.helper = FormHelper()
self.helper.layout = Layout(*field_sets)
# We need to know if the version was active before the update.
# We use this value in the save method.
self._was_active = self.instance.active if self.instance else False

def clean_active(self):
active = self.cleaned_data['active']
Expand All @@ -86,10 +87,7 @@ def _is_default_version(self):

def save(self, commit=True):
obj = super().save(commit=commit)
if obj.active and not obj.built and not obj.uploaded:
trigger_build(project=obj.project, version=obj)
if self.has_changed():
version_changed.send(sender=self.__class__, version=obj)
obj.post_save(was_active=self._was_active)
return obj


Expand Down
47 changes: 46 additions & 1 deletion readthedocs/builds/models.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""Models for the builds app."""

import datetime
import os.path
import re
Expand Down Expand Up @@ -52,6 +51,7 @@
RelatedBuildQuerySet,
VersionQuerySet,
)
from readthedocs.builds.signals import version_changed
from readthedocs.builds.utils import (
external_version_name,
get_bitbucket_username_repo,
Expand All @@ -61,6 +61,7 @@
)
from readthedocs.builds.version_slug import VersionSlugField
from readthedocs.config import LATEST_CONFIGURATION_VERSION
from readthedocs.core.utils import trigger_build
from readthedocs.projects.constants import (
BITBUCKET_COMMIT_URL,
BITBUCKET_URL,
Expand Down Expand Up @@ -381,6 +382,50 @@ def delete(self, *args, **kwargs): # pylint: disable=arguments-differ
clean_project_resources(self.project, self)
super().delete(*args, **kwargs)

def clean_resources(self):
"""
Remove all resources from this version.

This includes removing files from storage,
and removing its search index.
"""
from readthedocs.projects.tasks.utils import clean_project_resources

log.info(
"Removing files for version.",
project_slug=self.project.slug,
version_slug=self.slug,
)
clean_project_resources(project=self.project, version=self)
self.built = False
self.save()

def post_save(self, was_active=False):
"""
Run extra steps after updating a version.
stsewd marked this conversation as resolved.
Show resolved Hide resolved

This method isn't called automatically by a signal but is called explicitly
from other processes.

Useful to run after the version has been saved/updated
by the user, like from a form or API.

- When a version is deactivated, we need to clean up its
files from storage, and search index.
- When a version is activated, we need to trigger a build.
- We also need to purge the cache from the CDN,
since the version could have been activated/deactivated,
or its privacy level could have changed.
"""
# If the version is deactivated, we need to clean up the files.
if was_active and not self.active:
self.clean_resources()
# If the version is activated, we need to trigger a build.
if not was_active and self.active:
trigger_build(project=self.project, version=self)
# Purge the cache from the CDN.
version_changed.send(sender=self.__class__, version=self)

@property
def identifier_friendly(self):
"""Return display friendly identifier."""
Expand Down
14 changes: 1 addition & 13 deletions readthedocs/projects/views/private.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,19 +215,7 @@ def get_form(self, data=None, files=None, **kwargs):
return self.get_form_class()(data, files, **kwargs)

def form_valid(self, form):
version = form.save()
if form.has_changed():
if 'active' in form.changed_data and version.active is False:
log.info(
'Removing files for version.',
version_slug=version.slug,
)
clean_project_resources(
version.project,
version,
)
version.built = False
version.save()
form.save()
return HttpResponseRedirect(self.get_success_url())


Expand Down
8 changes: 5 additions & 3 deletions readthedocs/rtd_tests/tests/test_build_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,11 @@ def test_can_update_privacy_level(self):
self.assertTrue(form.is_valid())
self.assertEqual(version.privacy_level, PRIVATE)

@mock.patch('readthedocs.builds.forms.trigger_build', mock.MagicMock())
@mock.patch('readthedocs.projects.views.private.clean_project_resources')
def test_resources_are_deleted_when_version_is_inactive(self, clean_project_resources):
@mock.patch("readthedocs.builds.models.trigger_build", mock.MagicMock())
@mock.patch("readthedocs.projects.tasks.utils.clean_project_resources")
def test_resources_are_deleted_when_version_is_inactive(
self, clean_project_resources
):
version = get(
Version,
project=self.project,
Expand Down