Save config on build model

readthedocs/builds/migrations/0005_add_config_field.py

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# Generated by Django 1.9.13 on 2018-10-11 14:00
+# Generated by Django 1.9.13 on 2018-10-22 19:37
 from __future__ import unicode_literals
 
 from django.db import migrations, models
@@ -17,7 +17,7 @@ class Migration(migrations.Migration):
         migrations.AddField(
             model_name='build',
             name='config',
-            field=jsonfield.fields.JSONField(default={}, verbose_name='Configuration used in the build'),
+            field=jsonfield.fields.JSONField(default=dict, verbose_name='Configuration used in the build'),
         ),
         migrations.AlterField(
             model_name='build',

readthedocs/builds/models.py

@@ -17,6 +17,7 @@
 from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.db import models
+from django.utils import timezone
 from django.utils.encoding import python_2_unicode_compatible
 from django.utils.translation import ugettext
 from django.utils.translation import ugettext_lazy as _
@@ -137,7 +138,7 @@ def config(self):
             .order_by('-date')
             .first()
         )
-        return last_build.config
+        return last_build.get_config()
 
     @property
     def commit_name(self):
@@ -478,7 +479,7 @@ class Build(models.Model):
     exit_code = models.IntegerField(_('Exit code'), null=True, blank=True)
     commit = models.CharField(
         _('Commit'), max_length=255, null=True, blank=True)
-    config = JSONField(_('Configuration used in the build'), default={})
+    config = JSONField(_('Configuration used in the build'), default=dict)
 
     length = models.IntegerField(_('Build Length'), null=True, blank=True)
 
@@ -497,6 +498,47 @@ class Meta(object):
         get_latest_by = 'date'
         index_together = [['version', 'state', 'type']]
 
+    @property
+    def previous(self):
+        """Returns the previous build to this one."""
+        date = self.date or timezone.now()
+        if self.project is not None and self.version is not None:
+            return (
+                Build.objects
+                .filter(
+                    project=self.project,
+                    version=self.version,
+                    date__lt=date,
+                )
+                .order_by('-date')
+                .first()
+            )
+        return None
+
+    def get_config(self):
+        """Get the config from correct object."""
+        if '__config' in self.config:
+            return Build.objects.get(pk=self.config['__config']).config
+        return self.config
+
+    def save(self, *args, **kwargs):  # noqa
+        """
+        Save object.
+
+        To save space on the db we only save the config if it's different
+        from the previous one.
+
+        If the config is the same, we save the pk of the object
+        that has the **real** config under the ``__config`` key.
+        """
+        previous = self.previous
+        if previous is not None and self.config == previous.get_config():
+            previous_pk = previous.pk
+            if '__config' in previous.config:
+                previous_pk = previous.config['__config']
+            self.config = {'__config': previous_pk}
+        super(Build, self).save(*args, **kwargs)
+
     def __str__(self):
         return ugettext(
             'Build {project} for {usernames} ({pk})'.format(

readthedocs/rtd_tests/tests/test_api.py

@@ -58,9 +58,8 @@ def test_make_build(self):
         build = resp.data
         self.assertEqual(build['output'], 'Test Output')
         self.assertEqual(build['state_display'], 'Cloning')
-    
+
     def test_save_config(self):
-        """Test that a superuser can use the API."""
         client = APIClient()
         client.login(username='super', password='test')
         resp = client.post(
@@ -73,10 +72,44 @@ def test_save_config(self):
             format='json',
         )
         self.assertEqual(resp.status_code, status.HTTP_201_CREATED)
+        build_one = resp.data
+        self.assertEqual(build_one['config'], {'one': 'two'})
+
+        resp = client.get('/api/v2/build/%s/' % build_one['id'])
+        self.assertEqual(resp.status_code, 200)
         build = resp.data
         self.assertEqual(build['config'], {'one': 'two'})
 
-        resp = client.get('/api/v2/build/%s/' % build['id'])
+    def test_save_same_config(self):
+        client = APIClient()
+        client.login(username='super', password='test')
+        resp = client.post(
+            '/api/v2/build/',
+            {
+                'project': 1,
+                'version': 1,
+                'config': {'one': 'two'},
+            },
+            format='json',
+        )
+        self.assertEqual(resp.status_code, status.HTTP_201_CREATED)
+        build_one = resp.data
+        self.assertEqual(build_one['config'], {'one': 'two'})
+
+        resp = client.post(
+            '/api/v2/build/',
+            {
+                'project': 1,
+                'version': 1,
+                'config': {'one': 'two'},
+            },
+            format='json',
+        )
+        self.assertEqual(resp.status_code, status.HTTP_201_CREATED)
+        build_two = resp.data
+        self.assertEqual(build_two['config'], {'__config': build_one['id']})
+
+        resp = client.get('/api/v2/build/%s/' % build_one['id'])
         self.assertEqual(resp.status_code, 200)
         build = resp.data
         self.assertEqual(build['config'], {'one': 'two'})

readthedocs/rtd_tests/tests/test_builds.py

@@ -1,17 +1,19 @@
-from __future__ import absolute_import
+from __future__ import (
+    absolute_import,
+    division,
+    print_function,
+    unicode_literals,
+)
 
 import mock
-import six
-
 from django.test import TestCase
-from django_dynamic_fixture import get
-from django_dynamic_fixture import fixture
+from django_dynamic_fixture import fixture, get
 
-from readthedocs.projects.models import Project
-from readthedocs.builds.models import Version, Build
+from readthedocs.builds.models import Build, Version
 from readthedocs.doc_builder.config import load_yaml_config
 from readthedocs.doc_builder.environments import LocalBuildEnvironment
 from readthedocs.doc_builder.python_environments import Virtualenv
+from readthedocs.projects.models import Project
 from readthedocs.projects.tasks import UpdateDocsTaskStep
 from readthedocs.rtd_tests.tests.test_config_integration import create_load
 
@@ -247,3 +249,152 @@ def test_save_config_in_build_model(self, load_config, api_v2):
         assert build_config['version'] == '1'
         assert 'sphinx' in build_config
         assert build_config['doctype'] == 'sphinx'
+
+
+class BuildModelTests(TestCase):
+
+    def setUp(self):
+        self.project = get(Project)
+        self.version = get(Version, project=self.project)
+
+    def test_get_previous_build(self):
+        build_one = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={'version': 1}
+        )
+        build_two = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={'version': 2}
+        )
+        build_three = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={'version': 3},
+            success=False,
+        )
+
+        self.assertIsNone(build_one.previous)
+        self.assertEqual(build_two.previous, build_one)
+        self.assertEqual(build_three.previous, build_two)
+        self.assertEqual(build_three.previous.previous, build_one)
+
+    def test_normal_save_config(self):
+        build = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={'version': 1}
+        )
+        self.assertEqual(build.config, {'version': 1})
+
+        build.config = {'version': 2}
+        build.save()
+        self.assertEqual(build.config, {'version': 2})
+
+    def test_save_same_config(self):
+        build_one = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={}
+        )
+
+        build_two = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={'version': 2}
+        )
+        self.assertEqual(build_two.config, {'version': 2})
+
+    def test_save_same_config_previous_empty(self):
+        build_one = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={}
+        )
+
+        build_two = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={}
+        )
+        self.assertEqual(build_two.config, {})
+        build_two.config = {'version': 2}
+        build_two.save()
+        self.assertEqual(build_two.config, {'version': 2})
+
+    def test_do_not_save_same_config(self):
+        build_one = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={'version': 1},
+        )
+
+        build_two = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={},
+        )
+        build_two.config = {'version': 1}
+        build_two.save()
+        self.assertEqual(build_two.config, {'__config': build_one.pk})
+        self.assertEqual(build_two.get_config(), {'version': 1})
+
+    def test_do_not_save_same_config_nested(self):
+        build_one = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={},
+        )
+        build_one.config = {'version': 1}
+        build_one.save()
+
+        build_two = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={},
+        )
+        build_two.config = {'version': 1}
+        build_two.save()
+
+        build_three = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={},
+        )
+        build_three.config = {'version': 1}
+        build_three.save()
+
+        build_four = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={},
+        )
+        build_four.config = {'version': 2}
+        build_four.save()
+
+        self.assertEqual(build_one.get_config(), {'version': 1})
+        self.assertEqual(build_one.config, {'version': 1})
+
+        self.assertEqual(build_two.config, {'__config': build_one.pk})
+        self.assertEqual(build_three.config, {'__config': build_one.pk})
+
+        self.assertEqual(build_two.get_config(), {'version': 1})
+        self.assertEqual(build_three.get_config(), {'version': 1})
+
+        self.assertEqual(build_four.get_config(), {'version': 2})
+        self.assertEqual(build_four.config, {'version': 2})

readthedocs/rtd_tests/tests/test_version_config.py

@@ -42,3 +42,45 @@ def test_get_correct_config(self):
         )
         config = self.version.config
         self.assertEqual(config, {'version': 2})
+
+    def test_get_correct_config_when_same_config(self):
+        build_old = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={}
+        )
+        build_old.config = {'version': 1}
+        build_old.save()
+
+        build_new = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={}
+        )
+        build_new.config = {'version': 1}
+        build_new.save()
+
+        build_new_error = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={},
+            success=False,
+        )
+        build_new_error.config = {'version': 3}
+        build_new_error.save()
+
+        build_new_unfinish = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            config={},
+            state='building',
+        )
+        build_new_unfinish.config = {'version': 1}
+        build_new_unfinish.save()
+
+        config = self.version.config
+        self.assertEqual(config, {'version': 1})