Infinite migrations created when using regex based validators

[Sayid Munawar]

Using django.core.validators.validate_slug in a model will make the migration files on that model increment infinitely, allthough no changes has made in the model.

from django.core.validators import validate_slug

class Organization(models.Model):
    slug = models.SlugField(unique=True, validators=[validate_slug])

manage.py makemigrations

Migrations for 'organizations':
  0009_auto_20150815_2151.py:
    - Alter field slug on organization
Migrations for 'organizations':
  0010_auto_20150815_2151.py:
    - Alter field slug on organization
Migrations for 'organizations':
  0011_auto_20150815_2158.py:
    - Alter field slug on organization
Migrations for 'organizations':
  0012_auto_20150815_2158.py:
    - Alter field slug on organization

A quick fix is to workaround this issue is just copy paste the code in django.core.validators:

from django.core.validators import RegexValidator

orgslug_validator = [
    RegexValidator(regex=r'^[-a-zA-Z0-9_]+\Z',
                   message=_("Enter a valid 'slug' consisting of letters, "
                             "numbers, underscores or hyphens."),
                   code='invalid_slug')
]

class Organization(models.Model):
    slug = models.SlugField(unique=True, validators=orgslug_validator)

[Tim Graham]

Bisected to ff8a02ae0b33ee52050e22909d432a3aa060d683.

[Tim Graham]

Before the commit that introduced the regression. deep_deconstruct() returns:
[<django.core.validators.RegexValidator object at 0x7fb6ca4cd780>] for validators.

Now it returns something like:
[('django.core.validators.RegexValidator', [re.compile('^(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}\\Z'), 'Enter a valid IPv4 address.', 'invalid'].

It looks like the re.compile objects on the old and new fields are not treated as equal. I believe that's why we use ​some different comparisons in RegexValidator.__eq__().

[Tim Graham]

I attached my attempt at a regression test, unfortunately it passes so it doesn't seem to capture the issue.

[Markus Holtermann]

OK, I this is just an idea, and the current tests on master work, but I don't have a test that doesn't work on master yet ;)

django/db/migrations/autodetector.py

@@ -11 +11 @@
 from django.db.migrations.operations.models import AlterModelOptions
 from django.db.migrations.optimizer import MigrationOptimizer
 from django.db.migrations.questioner import MigrationQuestioner
+from django.db.migrations.utils import COMPILED_REGEX_TYPE, RegexObject
 from django.utils import six
 
 from .topological_sort import stable_topological_sort
@@ -62 +63 @@
                 key: self.deep_deconstruct(value)
                 for key, value in obj.items()
             }
+        elif isinstance(obj, COMPILED_REGEX_TYPE):
+            return RegexObject(obj.pattern, obj.flags)
         elif isinstance(obj, type):
             # If this is a type that implements 'deconstruct' as an instance method,
             # avoid treating this as being deconstructible itself - see #22951

new file django/db/migrations/utils.py

@@ +1 @@
+import re
+
+COMPILED_REGEX_TYPE = type(re.compile(''))
+
+
+class RegexObject(object):
+    def __init__(self, pattern, flags):
+        self.pattern = pattern
+        self.flags = flags
+
+    def __eq__(self, other):
+        return self.pattern == other.pattern and self.flags == other.flags

django/db/migrations/writer.py

@@ -14 +14 @@
 from django.db import migrations, models
 from django.db.migrations.loader import MigrationLoader
 from django.db.migrations.operations.base import Operation
+from django.db.migrations.utils import COMPILED_REGEX_TYPE, RegexObject
 from django.utils import datetime_safe, six
 from django.utils._os import upath
 from django.utils.encoding import force_text
@@ -23 +24 @@
 from django.utils.timezone import utc
 from django.utils.version import get_docs_version
 
-COMPILED_REGEX_TYPE = type(re.compile(''))
-
 
 class SettingsReference(str):
     """
@@ -506 +505 @@
             format = "(%s)" if len(strings) != 1 else "(%s,)"
             return format % (", ".join(strings)), imports
         # Compiled regex
-        elif isinstance(value, COMPILED_REGEX_TYPE):
+        elif isinstance(value, (COMPILED_REGEX_TYPE, RegexObject)):
             imports = {"import re"}
             regex_pattern, pattern_imports = cls.serialize(value.pattern)
             regex_flags, flag_imports = cls.serialize(value.flags)

[Sayid Munawar]

using MarkusH patch, i can confirm that my case is resolved

$  ./manage.py makemigrations
No changes detected
$  ./manage.py makemigrations
No changes detected
$  ./manage.py makemigrations
No changes detected

[Markus Holtermann]

Here's a PR including tests: ​https://github.com/django/django/pull/5173

Before backporting it to 1.8 I'd like to get it into 1.9 alpha and see if it actually works in the field as I don't really like the approach.

[Tim Graham]

The issue doesn't affect 1.8.

[Markus Holtermann <info@…>]

In 91f701f:

Fixed #25280 -- Properly checked regex objects for equality to prevent infinite migrations

Thanks Sayid Munawar and Tim Graham for the report, investigation and
review.