Merge pull request #8934 from readthedocs/humitos/django3-json-fields

Author: humitos

readthedocs/api/v2/views/integrations.py

@@ -431,7 +431,7 @@ def handle_webhook(self):
                 (created or deleted),
         ]):
             integration = self.get_integration()
-            events = integration.provider_data.get('events', [])
+            events = integration.provider_data.get('events', []) if integration.provider_data else []  # noqa
             if any([
                     GITHUB_CREATE in events,
                     GITHUB_DELETE in events,

readthedocs/builds/migrations/0040_remove_old_jsonfields.py

@@ -0,0 +1,22 @@
+# Generated by Django 3.2.11 on 2022-01-31 12:12
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('builds', '0039_migrate_config_data'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='build',
+            name='_config',
+        ),
+        migrations.RenameField(
+            model_name='build',
+            old_name='_config_json',
+            new_name='_config',
+        ),
+    ]

readthedocs/builds/models.py

@@ -19,7 +19,6 @@
     ModificationDateTimeField,
 )
 from django_extensions.db.models import TimeStampedModel
-from jsonfield import JSONField
 from polymorphic.models import PolymorphicModel
 
 import readthedocs.builds.automation_actions as actions
@@ -662,8 +661,7 @@ class Build(models.Model):
         null=True,
         blank=True,
     )
-    _config = JSONField(_('Configuration used in the build'), default=dict)
-    _config_json = models.JSONField(
+    _config = models.JSONField(
         _('Configuration used in the build'),
         null=True,
         blank=True,
@@ -740,7 +738,7 @@ def config(self):
         # probably change this field to be a ForeignKey to avoid repeating the
         # config file over and over again and re-use them to save db data as
         # well
-        if self.CONFIG_KEY in self._config:
+        if self._config and self.CONFIG_KEY in self._config:
             return (
                 Build.objects
                 .only('_config')
@@ -784,10 +782,6 @@ def save(self, *args, **kwargs):  # noqa
             self.version_slug = self.version.slug
             self.version_type = self.version.type
 
-        # TODO: delete copying config after deploy
-        # Copy `_config` into the new `_config_json` JSONField
-        self._config_json = self._config
-
         super().save(*args, **kwargs)
         self._config_changed = False
 
@@ -957,7 +951,9 @@ def external_version_name(self):
         return None
 
     def using_latest_config(self):
-        return int(self.config.get('version', '1')) == LATEST_CONFIGURATION_VERSION
+        if self.config:
+            return int(self.config.get('version', '1')) == LATEST_CONFIGURATION_VERSION
+        return False
 
     def reset(self):
         """

readthedocs/builds/tasks.py

@@ -115,14 +115,17 @@ def route_for_task(self, task, args, kwargs, **__):
         last_builds = version.builds.order_by('-date')[:self.N_LAST_BUILDS]
         # Version has used conda in previous builds
         for build in last_builds.iterator():
-            build_tools_python = (
-                build.config
-                .get('build', {})
-                .get('tools', {})
-                .get('python', {})
-                .get('version', '')
-            )
-            conda = build.config.get('conda', None)
+            build_tools_python = ''
+            conda = None
+            if build.config:
+                build_tools_python = (
+                    build.config
+                    .get('build', {})
+                    .get('tools', {})
+                    .get('python', {})
+                    .get('version', '')
+                )
+                conda = build.config.get('conda', None)
 
             uses_conda = any([
                 conda,

readthedocs/integrations/migrations/0010_remove_old_jsonfields.py

@@ -0,0 +1,40 @@
+# Generated by Django 3.2.11 on 2022-01-31 12:12
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('integrations', '0009_migrate_headers_data'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='httpexchange',
+            name='request_headers',
+        ),
+        migrations.RemoveField(
+            model_name='httpexchange',
+            name='response_headers',
+        ),
+        migrations.RemoveField(
+            model_name='integration',
+            name='provider_data',
+        ),
+        migrations.RenameField(
+            model_name='httpexchange',
+            old_name='request_headers_json',
+            new_name='request_headers',
+        ),
+        migrations.RenameField(
+            model_name='httpexchange',
+            old_name='response_headers_json',
+            new_name='response_headers',
+        ),
+        migrations.RenameField(
+            model_name='integration',
+            old_name='provider_data_json',
+            new_name='provider_data',
+        ),
+    ]

readthedocs/integrations/models.py

@@ -12,7 +12,6 @@
 from django.db import models, transaction
 from django.utils.safestring import mark_safe
 from django.utils.translation import gettext_lazy as _
-from jsonfield import JSONField
 from pygments import highlight
 from pygments.formatters import HtmlFormatter
 from pygments.lexers import JsonLexer
@@ -102,12 +101,15 @@ def from_requests_exchange(self, response, related_object):
         :param related_object: Object to use for generic relationship.
         """
         request = response.request
+        # NOTE: we need to cast ``request.headers`` and ``response.headers``
+        # because it's a ``requests.structures.CaseInsensitiveDict`` which is
+        # not JSON serializable.
         obj = self.create(
             related_object=related_object,
-            request_headers=request.headers or {},
+            request_headers=dict(request.headers) or {},
             request_body=request.body or '',
             status_code=response.status_code,
-            response_headers=response.headers,
+            response_headers=dict(response.headers),
             response_body=response.text,
         )
         self.delete_limit(related_object)
@@ -144,17 +146,15 @@ class HttpExchange(models.Model):
 
     date = models.DateTimeField(_('Date'), auto_now_add=True)
 
-    request_headers = JSONField(_('Request headers'))
-    request_headers_json = models.JSONField(
+    request_headers = models.JSONField(
         _('Request headers'),
         # Delete after deploy
         null=True,
         blank=True,
     )
     request_body = models.TextField(_('Request body'))
 
-    response_headers = JSONField(_('Request headers'))
-    response_headers_json = models.JSONField(
+    response_headers = models.JSONField(
         _('Request headers'),
         # Delete after deploy
         null=True,
@@ -175,15 +175,6 @@ class Meta:
     def __str__(self):
         return _('Exchange {0}').format(self.pk)
 
-    # TODO: delete .save method after deploy
-    # Copy headers into new JSONField
-    def save(self, *args, **kwargs):
-        # NOTE: cast headers into a regular dict because Django does not know
-        # how to serialize ``requests.structures.CaseInsensitiveDict``
-        self.request_headers_json = dict(self.request_headers)
-        self.response_headers_json = dict(self.response_headers)
-        super().save(*args, **kwargs)
-
     @property
     def failed(self):
         # Assume anything that isn't 2xx level status code is an error
@@ -298,8 +289,7 @@ class Integration(models.Model):
         max_length=32,
         choices=INTEGRATIONS,
     )
-    provider_data = JSONField(_('Provider data'), default=dict)
-    provider_data_json = models.JSONField(
+    provider_data = models.JSONField(
         _('Provider data'),
         null=True,
         blank=True,
@@ -329,12 +319,6 @@ def remove_secret(self):
         self.secret = None
         self.save(update_fields=['secret'])
 
-    # TODO: delete .save method after deploy
-    # Copy `provider_data` into new JSONField
-    def save(self, *args, **kwargs):
-        self.provider_data_json = self.provider_data
-        super().save(*args, **kwargs)
-
     def __str__(self):
         return (
             _('{0} for {1}')

readthedocs/rtd_tests/tests/test_api.py

@@ -89,7 +89,7 @@ def test_make_build(self):
         self.assertEqual(resp.status_code, status.HTTP_201_CREATED)
         build = resp.data
         self.assertEqual(build['state_display'], 'Cloning')
-        self.assertEqual(build['config'], {})
+        self.assertIsNone(build['config'])
 
         resp = client.get('/api/v2/build/%s/' % build['id'])
         self.assertEqual(resp.status_code, 200)

requirements/pip.txt

@@ -21,6 +21,10 @@ drf-flex-fields==0.9.7
 drf-extensions==0.7.1
 
 django-vanilla-views==3.0.0
+
+# This module is only used on migrations. We are now using Django's official
+# JSONField. We should probably squash these migrations and remove this
+# dependency as well.
 jsonfield==3.1.0
 
 requests==2.27.1