Merge branch 'master' into embed-test-def-lists

Author: stsewd

readthedocs/projects/migrations/0070_make_md5_field_nullable.py

@@ -0,0 +1,18 @@
+# Generated by Django 2.2.19 on 2021-03-17 15:23
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('projects', '0069_migrate_protected_projects'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='importedfile',
+            name='md5',
+            field=models.CharField(max_length=255, null=True, verbose_name='MD5 checksum'),
+        ),
+    ]

readthedocs/projects/models.py

@@ -1366,7 +1366,7 @@ class ImportedFile(models.Model):
     # of 4096 characters for most filesystems (including EXT4).
     # https://github.com/rtfd/readthedocs.org/issues/5061
     path = models.CharField(_('Path'), max_length=4096)
-    md5 = models.CharField(_('MD5 checksum'), max_length=255)
+    md5 = models.CharField(_('MD5 checksum'), max_length=255, null=True)
     commit = models.CharField(_('Commit'), max_length=255)
     build = models.IntegerField(_('Build id'), null=True)
     modified_date = models.DateTimeField(_('Modified date'), auto_now=True)

readthedocs/projects/signals.py

@@ -2,7 +2,6 @@
 
 import django.dispatch
 
-
 before_vcs = django.dispatch.Signal(providing_args=['version', 'environmemt'])
 after_vcs = django.dispatch.Signal(providing_args=['version'])
 
@@ -12,4 +11,4 @@
 project_import = django.dispatch.Signal(providing_args=['project'])
 
 # Used to purge files from the CDN
-files_changed = django.dispatch.Signal(providing_args=['project', 'version', 'files'])
+files_changed = django.dispatch.Signal(providing_args=['project', 'version'])

readthedocs/projects/tasks.py

@@ -1340,15 +1340,14 @@ def fileify(version_pk, commit, build, search_ranking, search_ignore):
         },
     )
     try:
-        changed_files = _create_imported_files(
+        _create_imported_files(
             version=version,
             commit=commit,
             build=build,
             search_ranking=search_ranking,
             search_ignore=search_ignore,
         )
     except Exception:
-        changed_files = set()
         log.exception('Failed during ImportedFile creation')
 
     try:
@@ -1357,7 +1356,7 @@ def fileify(version_pk, commit, build, search_ranking, search_ignore):
         log.exception('Failed during SphinxDomain creation')
 
     try:
-        _sync_imported_files(version, build, changed_files)
+        _sync_imported_files(version, build)
     except Exception:
         log.exception('Failed during ImportedFile syncing')
 
@@ -1530,57 +1529,22 @@ def _create_imported_files(*, version, commit, build, search_ranking, search_ign
     :param version: Version instance
     :param commit: Commit that updated path
     :param build: Build id
-    :returns: paths of changed files
-    :rtype: set
     """
-    changed_files = set()
-
     # Re-create all objects from the new build of the version
     storage_path = version.project.get_storage_path(
         type_='html', version_slug=version.slug, include_file=False
     )
     for root, __, filenames in build_media_storage.walk(storage_path):
         for filename in filenames:
-            if filename.endswith('.html'):
-                model_class = HTMLFile
-            elif version.project.cdn_enabled:
-                # We need to track all files for CDN enabled projects so the files can be purged
-                model_class = ImportedFile
-            else:
-                # For projects not behind a CDN, we don't care about non-HTML
+            # We don't care about non-HTML files
+            if not filename.endswith('.html'):
                 continue
 
             full_path = build_media_storage.join(root, filename)
 
             # Generate a relative path for storage similar to os.path.relpath
             relpath = full_path.replace(storage_path, '', 1).lstrip('/')
 
-            try:
-                md5 = hashlib.md5(build_media_storage.open(full_path, 'rb').read()).hexdigest()
-            except Exception:
-                log.exception(
-                    'Error while generating md5 for %s:%s:%s. Don\'t stop.',
-                    version.project.slug,
-                    version.slug,
-                    relpath,
-                )
-                md5 = ''
-            # Keep track of changed files to be purged in the CDN
-            obj = (
-                model_class.objects
-                .filter(project=version.project, version=version, path=relpath)
-                .order_by('-modified_date')
-                .first()
-            )
-            if obj and md5 and obj.md5 != md5:
-                changed_files.add(
-                    resolve_path(
-                        version.project,
-                        filename=relpath,
-                        version_slug=version.slug,
-                    ),
-                )
-
             page_rank = 0
             # Last pattern to match takes precedence
             # XXX: see if we can implement another type of precedence,
@@ -1598,37 +1562,31 @@ def _create_imported_files(*, version, commit, build, search_ranking, search_ign
                     break
 
             # Create imported files from new build
-            model_class.objects.create(
+            HTMLFile.objects.create(
                 project=version.project,
                 version=version,
                 path=relpath,
                 name=filename,
-                md5=md5,
                 rank=page_rank,
                 commit=commit,
                 build=build,
                 ignore=ignore,
             )
 
-    # This signal is used for clearing the CDN,
-    # so send it as soon as we have the list of changed files
+    # This signal is used for purging the CDN.
     files_changed.send(
         sender=Project,
         project=version.project,
         version=version,
-        files=changed_files,
     )
 
-    return changed_files
-
 
-def _sync_imported_files(version, build, changed_files):
+def _sync_imported_files(version, build):
     """
     Sync/Update/Delete ImportedFiles objects of this version.
 
     :param version: Version instance
     :param build: Build id
-    :param changed_files: path of changed files
     """
 
     # Index new HTMLFiles to ElasticSearch

readthedocs/projects/views/public.py

@@ -26,7 +26,7 @@
 
 from readthedocs.analytics.tasks import analytics_event
 from readthedocs.analytics.utils import get_client_ip
-from readthedocs.builds.constants import LATEST
+from readthedocs.builds.constants import LATEST, BUILD_STATUS_DUPLICATED
 from readthedocs.builds.models import Version
 from readthedocs.builds.views import BuildTriggerMixin
 from readthedocs.core.permissions import AdminPermission
@@ -177,10 +177,13 @@ def get(self, request, project_slug, *args, **kwargs):
             ).first()
 
         if version:
-            last_build = version.builds.filter(
-                type='html',
-                state='finished',
-            ).order_by('-date').first()
+            last_build = (
+                version.builds
+                .filter(type='html', state='finished')
+                .exclude(status=BUILD_STATUS_DUPLICATED)
+                .order_by('-date')
+                .first()
+            )
             if last_build:
                 if last_build.success:
                     status = self.STATUS_PASSING

readthedocs/rtd_tests/tests/test_imported_file.py

@@ -47,7 +47,7 @@ def _manage_imported_files(
             search_ranking=search_ranking,
             search_ignore=search_ignore,
         )
-        _sync_imported_files(version, build, set())
+        _sync_imported_files(version, build)
 
     def _copy_storage_dir(self):
         """Copy the test directory (rtd_tests/files) to storage"""
@@ -63,18 +63,14 @@ def _copy_storage_dir(self):
     def test_properly_created(self):
         # Only 2 files in the directory is HTML (test.html, api/index.html)
         self.assertEqual(ImportedFile.objects.count(), 0)
-        self._manage_imported_files(self.version, 'commit01', 1)
+        self._manage_imported_files(version=self.version, commit='commit01', build=1)
         self.assertEqual(ImportedFile.objects.count(), 2)
-        self._manage_imported_files(self.version, 'commit01', 2)
+        self._manage_imported_files(version=self.version, commit='commit01', build=2)
         self.assertEqual(ImportedFile.objects.count(), 2)
 
         self.project.cdn_enabled = True
         self.project.save()
 
-        # CDN enabled projects => save all files
-        self._manage_imported_files(self.version, 'commit01', 3)
-        self.assertEqual(ImportedFile.objects.count(), 4)
-
     def test_update_commit(self):
         self.assertEqual(ImportedFile.objects.count(), 0)
         self._manage_imported_files(self.version, 'commit01', 1)
@@ -168,7 +164,6 @@ def test_update_content(self):
         self._copy_storage_dir()
 
         self._manage_imported_files(self.version, 'commit01', 1)
-        self.assertEqual(ImportedFile.objects.get(name='test.html').md5, 'c7532f22a052d716f7b2310fb52ad981')
         self.assertEqual(ImportedFile.objects.count(), 2)
 
         with open(os.path.join(test_dir, 'test.html'), 'w+') as f:
@@ -177,7 +172,6 @@ def test_update_content(self):
         self._copy_storage_dir()
 
         self._manage_imported_files(self.version, 'commit02', 2)
-        self.assertNotEqual(ImportedFile.objects.get(name='test.html').md5, 'c7532f22a052d716f7b2310fb52ad981')
         self.assertEqual(ImportedFile.objects.count(), 2)
 
     @override_settings(PRODUCTION_DOMAIN='readthedocs.org')

readthedocs/rtd_tests/tests/test_project_views.py

@@ -11,7 +11,7 @@
 from django.views.generic.base import ContextMixin
 from django_dynamic_fixture import get, new
 
-from readthedocs.builds.constants import EXTERNAL
+from readthedocs.builds.constants import BUILD_STATUS_DUPLICATED, EXTERNAL
 from readthedocs.builds.models import Build, Version
 from readthedocs.integrations.models import GenericAPIWebhook, GitHubWebhook
 from readthedocs.oauth.models import RemoteRepository
@@ -611,6 +611,25 @@ def test_passing_badge(self):
         self.assertContains(res, 'passing')
         self.assertEqual(res['Content-Type'], 'image/svg+xml')
 
+    def test_ignore_duplicated_build(self):
+        """Ignore builds marked as duplicate from the badge status."""
+        get(
+            Build,
+            project=self.project,
+            version=self.version,
+            success=True,
+        )
+        get(
+            Build,
+            project=self.project,
+            version=self.version,
+            success=False,
+            status=BUILD_STATUS_DUPLICATED,
+        )
+        res = self.client.get(self.badge_url, {'version': self.version.slug})
+        self.assertContains(res, 'passing')
+        self.assertEqual(res['Content-Type'], 'image/svg+xml')
+
     def test_failing_badge(self):
         get(Build, project=self.project, version=self.version, success=False)
         res = self.client.get(self.badge_url, {'version': self.version.slug})