Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

[1.7.x] Fixed #22436: More careful checking on method ref'ce serializ…

…ation
  • Loading branch information...
commit 98949e3b108100a40cb6ad59be78f33580439fef 1 parent 2b13576
@andrewgodwin andrewgodwin authored
View
28 django/db/migrations/writer.py
@@ -12,7 +12,7 @@
from django.apps import apps
from django.db import models
from django.db.migrations.loader import MigrationLoader
-from django.utils import datetime_safe, six
+from django.utils import datetime_safe, six, importlib
from django.utils.encoding import force_text
from django.utils.functional import Promise
@@ -284,13 +284,29 @@ def serialize(cls, value):
klass = value.__self__
module = klass.__module__
return "%s.%s.%s" % (module, klass.__name__, value.__name__), set(["import %s" % module])
- elif value.__name__ == '<lambda>':
+ # Further error checking
+ if value.__name__ == '<lambda>':
raise ValueError("Cannot serialize function: lambda")
- elif value.__module__ is None:
+ if value.__module__ is None:
raise ValueError("Cannot serialize function %r: No module" % value)
- else:
- module = value.__module__
- return "%s.%s" % (module, value.__name__), set(["import %s" % module])
+ # Python 3 is a lot easier, and only uses this branch if it's not local.
+ if getattr(value, "__qualname__", None) and getattr(value, "__module__", None):
+ if "<" not in value.__qualname__: # Qualname can include <locals>
+ return "%s.%s" % (value.__module__, value.__qualname__), set(["import %s" % value.__module__])
+ # Python 2/fallback version
+ module_name = value.__module__
+ # Make sure it's actually there and not an unbound method
+ module = importlib.import_module(module_name)
+ if not hasattr(module, value.__name__):
+ raise ValueError(
+ "Could not find function %s in %s.\nPlease note that "
@cwood
cwood added a note

This line is missing the strings that are to be interpolated here. Itll just print out %s in %s.

@cwood
cwood added a note

I made a PR here #2871

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ "due to Python 2 limitations, you cannot serialize "
+ "unbound method functions (e.g. a method declared\n"
+ "and used in the same class body). Please move the "
+ "function into the main module body to use migrations.\n"
+ "For more information, see https://docs.djangoproject.com/en/1.7/topics/migrations/#serializing-values"
+ )
+ return "%s.%s" % (module_name, value.__name__), set(["import %s" % module_name])
# Classes
elif isinstance(value, type):
special_cases = [
View
19 docs/topics/migrations.txt
@@ -491,11 +491,30 @@ Django can serialize the following:
- Any class reference
- Anything with a custom ``deconstruct()`` method (:ref:`see below <custom-deconstruct-method>`)
+Django can serialize the following on Python 3 only:
+
+- Unbound methods used from within the class body (see below)
+
Django cannot serialize:
- Arbitrary class instances (e.g. ``MyClass(4.3, 5.7)``)
- Lambdas
+Due to the fact ``__qualname__`` was only introduced in Python 3, Django can only
+serialize the following pattern (an unbound method used within the class body)
+on Python 3, and will fail to serialize a reference to it on Python 2::
+
+ class MyModel(models.Model):
+
+ def upload_to(self):
+ return "something dynamic"
+
+ my_file = models.FileField(upload_to=upload_to)
+
+If you are using Python 2, we recommend you move your methods for upload_to
+and similar arguments that accept callables (e.g. ``default``) to live in
+the main module body, rather than the class body.
+
.. _custom-deconstruct-method:
Adding a deconstruct() method
View
27 tests/migrations/test_writer.py
@@ -4,6 +4,7 @@
import datetime
import os
import tokenize
+import unittest
from django.core.validators import RegexValidator, EmailValidator
from django.db import models, migrations
@@ -16,6 +17,12 @@
from django.utils.timezone import get_default_timezone
+class TestModel1(object):
+ def upload_to(self):
+ return "somewhere dynamic"
+ thing = models.FileField(upload_to=upload_to)
+
+
class WriterTests(TestCase):
"""
Tests the migration writer (makes migration files from Migration instances)
@@ -137,6 +144,26 @@ def test_serialize_empty_nonempty_tuple(self):
self.assertSerializedEqual(one_item_tuple)
self.assertSerializedEqual(many_items_tuple)
+ @unittest.skipUnless(six.PY2, "Only applies on Python 2")
+ def test_serialize_direct_function_reference(self):
+ """
+ Ticket #22436: You cannot use a function straight from its body
+ (e.g. define the method and use it in the same body)
+ """
+ with self.assertRaises(ValueError):
+ self.serialize_round_trip(TestModel1.thing)
+
+ def test_serialize_local_function_reference(self):
+ """
+ Neither py2 or py3 can serialize a reference in a local scope.
+ """
+ class TestModel2(object):
+ def upload_to(self):
+ return "somewhere dynamic"
+ thing = models.FileField(upload_to=upload_to)
+ with self.assertRaises(ValueError):
+ self.serialize_round_trip(TestModel2.thing)
+
def test_simple_migration(self):
"""
Tests serializing a simple migration.

0 comments on commit 98949e3

Please sign in to comment.
Something went wrong with that request. Please try again.