Add unique constraint to filter spec so that Filter.get_or_create avoids race condition as intended - fixes #312

wagtail/wagtailimages/migrations/0005_make_filter_spec_unique.py

@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('wagtailimages', '0004_make_focal_point_key_not_nullable'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='filter',
+            name='spec',
+            field=models.CharField(unique=True, max_length=255, db_index=True),
+            preserve_default=True,
+        ),
+    ]

wagtail/wagtailimages/models.py

@@ -300,7 +300,7 @@ class Filter(models.Model):
     appropriate for final display on the website. Usually this would be a resize operation,
     but could potentially involve colour processing, etc.
     """
-    spec = models.CharField(max_length=255, db_index=True)
+    spec = models.CharField(max_length=255, db_index=True, unique=True)
 
     OPERATION_NAMES = {
         'max': 'resize_to_max',

wagtail/wagtailimages/south_migrations/0005_make_filter_spec_unique.py

@@ -0,0 +1,88 @@
+# -*- coding: utf-8 -*-
+from south.utils import datetime_utils as datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding unique constraint on 'Filter', fields ['spec']
+        db.create_unique('wagtailimages_filter', ['spec'])
+
+
+    def backwards(self, orm):
+        # Removing unique constraint on 'Filter', fields ['spec']
+        db.delete_unique('wagtailimages_filter', ['spec'])
+
+
+    models = {
+        'auth.group': {
+            'Meta': {'object_name': 'Group'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+            'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+        },
+        'auth.permission': {
+            'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+            'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        'auth.user': {
+            'Meta': {'object_name': 'User'},
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'groups': ('django.db.models.fields.related.ManyToManyField', [], {'symmetrical': 'False', 'related_name': "'user_set'", 'blank': 'True', 'to': "orm['auth.Group']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'symmetrical': 'False', 'related_name': "'user_set'", 'blank': 'True', 'to': "orm['auth.Permission']"}),
+            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+        },
+        'contenttypes.contenttype': {
+            'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+            'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+        },
+        'wagtailimages.filter': {
+            'Meta': {'object_name': 'Filter'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'spec': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255', 'db_index': 'True'})
+        },
+        'wagtailimages.image': {
+            'Meta': {'object_name': 'Image'},
+            'created_at': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'file': ('django.db.models.fields.files.ImageField', [], {'max_length': '100'}),
+            'focal_point_height': ('django.db.models.fields.PositiveIntegerField', [], {'null': 'True', 'blank': 'True'}),
+            'focal_point_width': ('django.db.models.fields.PositiveIntegerField', [], {'null': 'True', 'blank': 'True'}),
+            'focal_point_x': ('django.db.models.fields.PositiveIntegerField', [], {'null': 'True', 'blank': 'True'}),
+            'focal_point_y': ('django.db.models.fields.PositiveIntegerField', [], {'null': 'True', 'blank': 'True'}),
+            'height': ('django.db.models.fields.IntegerField', [], {}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'uploaded_by_user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True', 'blank': 'True'}),
+            'width': ('django.db.models.fields.IntegerField', [], {})
+        },
+        'wagtailimages.rendition': {
+            'Meta': {'unique_together': "(('image', 'filter', 'focal_point_key'),)", 'object_name': 'Rendition'},
+            'file': ('django.db.models.fields.files.ImageField', [], {'max_length': '100'}),
+            'filter': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'+'", 'to': "orm['wagtailimages.Filter']"}),
+            'focal_point_key': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '255', 'blank': 'True'}),
+            'height': ('django.db.models.fields.IntegerField', [], {}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'image': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'renditions'", 'to': "orm['wagtailimages.Image']"}),
+            'width': ('django.db.models.fields.IntegerField', [], {})
+        }
+    }
+
+    complete_apps = ['wagtailimages']

wagtail/wagtailimages/tests/test_models.py

@@ -6,6 +6,7 @@ from django.contrib.auth.models import Group, Permission
 
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.db.utils import IntegrityError
+from django.db import connection
 
 from wagtail.tests.utils import WagtailTestUtils, unittest, test_concurrently
 from wagtail.wagtailcore.models import Page
@@ -407,7 +408,6 @@ class TestIssue312(TestCase):
         )
 
     def test_duplicate_filters(self):
-        # get renditions concurrently, using various filters that are unlikely to exist already
         @test_concurrently(10)
         def get_renditions():
             # Create an image
@@ -415,9 +415,14 @@ class TestIssue312(TestCase):
                 title="Concurrency test image",
                 file=get_test_image_file(),
             )
+            # get renditions concurrently, using various filters that are unlikely to exist already
             for width in range(10, 100, 10):
                 image.get_rendition('width-%d' % width)
 
+            # this block opens multiple database connections, which need to be closed explicitly
+            # so that we can drop the test database at the end of the test run
+            connection.close()
+
         get_renditions()
         # if the above has completed with no race conditions, there should be precisely one
         # of each of the above filters in the database