From 9d203a9f037d6aa2f6cb1f77aebf62785ab7407e Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 7 Aug 2023 19:06:00 -0500 Subject: [PATCH 1/4] Versions: keep type of version in sync with the project So, there are a couple of problems here: - Versions have been created with the type set as unknown, and where never updated to the correct type. I was able to track down this to projects created before 2018, so probably an old bug allowed this to happen. - Stable machine versions are basically an alias for the latest stable version of the project, and they can be a branch or a tag, this means that if this version is a branch, so will be the "stable" version, same for tags. Currently, we aren't updating the version type, if the stable version was created as a branch, it will remain as a branch, even if the latest stable version is a tag, and vice versa. - Latest machine versions are basically an alias for the default branch of the project, they are always branches. Since we allow to have non-machine latest versions, they can be a branch or a tag. If this version was changed back to be a non-machine version, currently it will remain as branch or tag, this was incorrect. Our new clone/checkout pattern relies on the type of the version always being correct, so it was failing for some projects. - Fixes https://github.com/readthedocs/readthedocs.org/issues/10600 - Fixes https://github.com/readthedocs/readthedocs.org/issues/10601 After deploy, we need to clean up the invalid versions (versions with "unknown" type). We can do that by: - Updating all latest machine versions to be branches. ```python Version.objects.filter(type="unknown", machine=True).update(type="branch") ``` - Re-evaluate the stable version of all projects that have a machine stable version, so they can have the correct type. ``` for project in Project.objects.filter(versions__machine=True).distinct(): project.update_stable_version() ``` - Check all remaining active versions with "unknown" type, and update them to be branches or tags (324). We can use a simple regex to see the version identifier looks like a commit. ```python vesions = Version.objects.filter(type="unknown", active=True) commit_regex = re.compile(r"^([a-f0-9]{40})|([a-f0-9]{7})$") for version in versions: if commit_regex.match(version.identifier): version.type = "tag" else: version.type = "branch" version.save() ``` --- readthedocs/api/v2/utils.py | 2 ++ readthedocs/projects/models.py | 15 ++++++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/readthedocs/api/v2/utils.py b/readthedocs/api/v2/utils.py index f6dcfb2a90d..c9e7b80db9f 100644 --- a/readthedocs/api/v2/utils.py +++ b/readthedocs/api/v2/utils.py @@ -117,6 +117,8 @@ def sync_versions_to_db(project, versions, type): # pylint: disable=redefined-b latest_version.machine = True latest_version.identifier = project.get_default_branch() latest_version.verbose_name = LATEST_VERBOSE_NAME + # The machine created latest version always points to a branch. + latest_version.type = BRANCH latest_version.save() if added: log.info( diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 834f55fa4b5..582950c462e 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -1199,18 +1199,19 @@ def update_stable_version(self): new_stable = determine_stable_version(versions) if new_stable: if current_stable: - identifier_updated = ( + version_updated = ( new_stable.identifier != current_stable.identifier + or new_stable.type != current_stable.type ) - if identifier_updated: + if version_updated: log.info( - 'Update stable version: %(project)s:%(version)s', - { - 'project': self.slug, - 'version': new_stable.identifier, - } + "Stable version updated.", + project_slug=self.slug, + version_identifier=new_stable.identifier, + version_type=new_stable.type, ) current_stable.identifier = new_stable.identifier + current_stable.type = new_stable.type current_stable.save() return new_stable else: From 80cdd8d917c3b031b85e0133c72e3829767151a0 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Mon, 7 Aug 2023 19:33:39 -0500 Subject: [PATCH 2/4] Linter --- readthedocs/api/v2/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/readthedocs/api/v2/utils.py b/readthedocs/api/v2/utils.py index c9e7b80db9f..0c51b2f9c42 100644 --- a/readthedocs/api/v2/utils.py +++ b/readthedocs/api/v2/utils.py @@ -22,7 +22,7 @@ log = structlog.get_logger(__name__) -def sync_versions_to_db(project, versions, type): # pylint: disable=redefined-builtin +def sync_versions_to_db(project, versions, type): """ Update the database with the current versions from the repository. @@ -158,7 +158,7 @@ def _create_versions(project, type, versions): def _set_or_create_version(project, slug, version_id, verbose_name, type_): """Search or create a version and set its machine attribute to false.""" - version = (project.versions.filter(slug=slug).first()) + version = project.versions.filter(slug=slug).first() if version: version.identifier = version_id version.machine = False From 21d68a400324b0d4f3a40db9edc1e36b5a214e47 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Tue, 8 Aug 2023 12:33:15 -0500 Subject: [PATCH 3/4] Tests --- .../rtd_tests/tests/test_sync_versions.py | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/readthedocs/rtd_tests/tests/test_sync_versions.py b/readthedocs/rtd_tests/tests/test_sync_versions.py index 226d18efea8..1700d2bb9e8 100644 --- a/readthedocs/rtd_tests/tests/test_sync_versions.py +++ b/readthedocs/rtd_tests/tests/test_sync_versions.py @@ -219,6 +219,94 @@ def test_delete_version(self): Version.objects.filter(slug='external').exists(), ) + def test_update_stable_version_type(self): + self.pip.update_stable_version() + stable_version = self.pip.get_stable_version() + self.assertEqual(stable_version.type, TAG) + + branches_data = [ + { + "identifier": "master", + "verbose_name": "master", + }, + { + "identifier": "1.0", + "verbose_name": "1.0", + }, + { + "identifier": "1.1", + "verbose_name": "1.1", + }, + { + "identifier": "2.0", + "verbose_name": "2.0", + }, + ] + + # Deactivate all other versions, so we only have branches for consideration + # for the new stable version. + self.pip.versions.exclude(slug__in=[LATEST, STABLE]).update(active=False) + sync_versions_task( + self.pip.pk, + branches_data=branches_data, + tags_data=[], + ) + + self.pip.update_stable_version() + stable_version = self.pip.get_stable_version() + self.assertEqual(stable_version.type, BRANCH) + self.assertEqual(stable_version.identifier, "2.0") + self.assertEqual(stable_version.verbose_name, "stable") + + original_stable = self.pip.get_original_stable_version() + self.assertEqual(original_stable.type, BRANCH) + self.assertEqual(original_stable.slug, "2.0") + self.assertEqual(original_stable.identifier, "2.0") + self.assertEqual(original_stable.verbose_name, "2.0") + + def test_update_latest_version_type(self): + latest_version = self.pip.versions.get(slug=LATEST) + self.assertEqual(latest_version.type, BRANCH) + + branches_data = [ + { + "identifier": "master", + "verbose_name": "master", + }, + ] + tags_data = [ + { + "identifier": "abc123", + "verbose_name": "latest", + } + ] + + # Latest is created as machine=False, and as a tag. + sync_versions_task( + self.pip.pk, + branches_data=branches_data, + tags_data=tags_data, + ) + + latest_version = self.pip.versions.get(slug=LATEST) + self.assertEqual(latest_version.type, TAG) + self.assertEqual(latest_version.identifier, "abc123") + self.assertEqual(latest_version.verbose_name, "latest") + self.assertEqual(latest_version.machine, False) + + # Latest is back as machine created, and as a branch. + sync_versions_task( + self.pip.pk, + branches_data=branches_data, + tags_data=[], + ) + + latest_version = self.pip.versions.get(slug=LATEST) + self.assertEqual(latest_version.type, BRANCH) + self.assertEqual(latest_version.identifier, "master") + self.assertEqual(latest_version.verbose_name, "latest") + self.assertEqual(latest_version.machine, True) + def test_machine_attr_when_user_define_stable_tag_and_delete_it(self): """ The user creates a tag named ``stable`` on an existing repo, From 722a82fa0f0455e95cdb77dd71287dd3cd817d64 Mon Sep 17 00:00:00 2001 From: Santos Gallegos Date: Thu, 10 Aug 2023 12:46:40 -0500 Subject: [PATCH 4/4] Docstring --- readthedocs/projects/models.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 15e5aa678d2..83b311f8dc3 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -1168,7 +1168,12 @@ def get_original_stable_version(self): """ Get the original version that stable points to. - Returns None if the current stable doesn't point to a valid version. + When stable is machine created, it's basically an alias + for the latest stable version (like 2.2), + that version is the "original" one. + + Returns None if the current stable doesn't point to a valid version + or if isn't machine created. """ current_stable = self.get_stable_version() if not current_stable or not current_stable.machine: