From 42736ac8e8c31137131714013951249a09e6e7d4 Mon Sep 17 00:00:00 2001 From: Anubhav Joshi Date: Fri, 6 Jun 2014 16:40:20 +0530 Subject: [PATCH] Fixed #21430 -- Added a RuntimeWarning when unpickling Models and QuerySets from a different Django version. Thanks FunkyBob for the suggestion, prasoon2211 for the initial patch, and akaariai, loic, and charettes for helping in shaping the patch. --- django/db/__init__.py | 9 ++--- django/db/models/base.py | 21 +++++++++++- django/db/models/query.py | 23 ++++++++++++- django/db/utils.py | 1 + django/utils/version.py | 34 ++++++++++++++----- docs/ref/models/instances.txt | 22 +++++++++++++ docs/ref/models/querysets.txt | 7 ++++ docs/releases/1.8.txt | 7 ++++ tests/model_regress/test_pickle.py | 53 ++++++++++++++++++++++++++++++ tests/queryset_pickle/models.py | 20 ++++++++++- tests/queryset_pickle/tests.py | 29 ++++++++++++++++ 11 files changed, 210 insertions(+), 16 deletions(-) create mode 100644 tests/model_regress/test_pickle.py diff --git a/django/db/__init__.py b/django/db/__init__.py index a303a2c150..32da61d680 100644 --- a/django/db/__init__.py +++ b/django/db/__init__.py @@ -1,14 +1,15 @@ from django.core import signals -from django.db.utils import (DEFAULT_DB_ALIAS, DataError, OperationalError, - IntegrityError, InternalError, ProgrammingError, NotSupportedError, - DatabaseError, InterfaceError, Error, ConnectionHandler, ConnectionRouter) +from django.db.utils import (DEFAULT_DB_ALIAS, DJANGO_VERSION_PICKLE_KEY, + DataError, OperationalError, IntegrityError, InternalError, ProgrammingError, + NotSupportedError, DatabaseError, InterfaceError, Error, ConnectionHandler, + ConnectionRouter) __all__ = [ 'backend', 'connection', 'connections', 'router', 'DatabaseError', 'IntegrityError', 'InternalError', 'ProgrammingError', 'DataError', 'NotSupportedError', 'Error', 'InterfaceError', 'OperationalError', - 'DEFAULT_DB_ALIAS' + 'DEFAULT_DB_ALIAS', 'DJANGO_VERSION_PICKLE_KEY' ] connections = ConnectionHandler() diff --git a/django/db/models/base.py b/django/db/models/base.py index 11dcb5ba3e..5142f93462 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -13,7 +13,7 @@ from django.core import checks from django.core.exceptions import (ObjectDoesNotExist, MultipleObjectsReturned, FieldError, ValidationError, NON_FIELD_ERRORS) from django.db import (router, transaction, DatabaseError, - DEFAULT_DB_ALIAS) + DEFAULT_DB_ALIAS, DJANGO_VERSION_PICKLE_KEY) from django.db.models.deletion import Collector from django.db.models.fields import AutoField, FieldDoesNotExist from django.db.models.fields.related import (ForeignObjectRel, ManyToOneRel, @@ -30,6 +30,7 @@ from django.utils.functional import curry from django.utils.six.moves import zip from django.utils.text import get_text_list, capfirst from django.utils.translation import ugettext_lazy as _ +from django.utils.version import get_version def subclass_exception(name, parents, module, attached_to=None): @@ -495,6 +496,7 @@ class Model(six.with_metaclass(ModelBase)): only module-level classes can be pickled by the default path. """ data = self.__dict__ + data[DJANGO_VERSION_PICKLE_KEY] = get_version() if not self._deferred: class_id = self._meta.app_label, self._meta.object_name return model_unpickle, (class_id, [], simple_class_factory), data @@ -507,6 +509,23 @@ class Model(six.with_metaclass(ModelBase)): class_id = model._meta.app_label, model._meta.object_name return (model_unpickle, (class_id, defers, deferred_class_factory), data) + def __setstate__(self, state): + msg = None + pickled_version = state.get(DJANGO_VERSION_PICKLE_KEY) + if pickled_version: + current_version = get_version() + if current_version != pickled_version: + msg = ("Pickled model instance's Django version %s does" + " not match the current version %s." + % (pickled_version, current_version)) + else: + msg = "Pickled model instance's Django version is not specified." + + if msg: + warnings.warn(msg, RuntimeWarning, stacklevel=2) + + self.__dict__.update(state) + def _get_pk_val(self, meta=None): if not meta: meta = self._meta diff --git a/django/db/models/query.py b/django/db/models/query.py index f314fcd553..edb2cd88fb 100644 --- a/django/db/models/query.py +++ b/django/db/models/query.py @@ -5,10 +5,12 @@ The main QuerySet implementation. This provides the public API for the ORM. from collections import deque import copy import sys +import warnings from django.conf import settings from django.core import exceptions -from django.db import connections, router, transaction, IntegrityError +from django.db import (connections, router, transaction, IntegrityError, + DJANGO_VERSION_PICKLE_KEY) from django.db.models.constants import LOOKUP_SEP from django.db.models.fields import AutoField, Empty from django.db.models.query_utils import (Q, select_related_descend, @@ -19,6 +21,7 @@ from django.db.models import sql from django.utils.functional import partition from django.utils import six from django.utils import timezone +from django.utils.version import get_version # The maximum number (one less than the max to be precise) of results to fetch # in a get() query @@ -90,8 +93,26 @@ class QuerySet(object): # Force the cache to be fully populated. self._fetch_all() obj_dict = self.__dict__.copy() + obj_dict[DJANGO_VERSION_PICKLE_KEY] = get_version() return obj_dict + def __setstate__(self, state): + msg = None + pickled_version = state.get(DJANGO_VERSION_PICKLE_KEY) + if pickled_version: + current_version = get_version() + if current_version != pickled_version: + msg = ("Pickled queryset instance's Django version %s does" + " not match the current version %s." + % (pickled_version, current_version)) + else: + msg = "Pickled queryset instance's Django version is not specified." + + if msg: + warnings.warn(msg, RuntimeWarning, stacklevel=2) + + self.__dict__.update(state) + def __reduce__(self): """ Used by pickle to deal with the types that we create dynamically when diff --git a/django/db/utils.py b/django/db/utils.py index 037f34f700..b7acf8d567 100644 --- a/django/db/utils.py +++ b/django/db/utils.py @@ -14,6 +14,7 @@ from django.utils import six DEFAULT_DB_ALIAS = 'default' +DJANGO_VERSION_PICKLE_KEY = '_django_version' class Error(Exception if six.PY3 else StandardError): diff --git a/django/utils/version.py b/django/utils/version.py index 6798749d43..c680dbdc4a 100644 --- a/django/utils/version.py +++ b/django/utils/version.py @@ -7,19 +7,14 @@ import subprocess def get_version(version=None): "Returns a PEP 386-compliant version number from VERSION." - if version is None: - from django import VERSION as version - else: - assert len(version) == 5 - assert version[3] in ('alpha', 'beta', 'rc', 'final') + version = get_complete_version(version) # Now build the two parts of the version number: - # main = X.Y[.Z] + # major = X.Y[.Z] # sub = .devN - for pre-alpha releases # | {a|b|c}N - for alpha, beta and rc releases - parts = 2 if version[2] == 0 else 3 - main = '.'.join(str(x) for x in version[:parts]) + major = get_major_version(version) sub = '' if version[3] == 'alpha' and version[4] == 0: @@ -31,7 +26,28 @@ def get_version(version=None): mapping = {'alpha': 'a', 'beta': 'b', 'rc': 'c'} sub = mapping[version[3]] + str(version[4]) - return str(main + sub) + return str(major + sub) + + +def get_major_version(version=None): + "Returns major version from VERSION." + version = get_complete_version(version) + parts = 2 if version[2] == 0 else 3 + major = '.'.join(str(x) for x in version[:parts]) + return major + + +def get_complete_version(version=None): + """Returns a tuple of the django version. If version argument is non-empy, + then checks for correctness of the tuple provided. + """ + if version is None: + from django import VERSION as version + else: + assert len(version) == 5 + assert version[3] in ('alpha', 'beta', 'rc', 'final') + + return version def get_git_changeset(): diff --git a/docs/ref/models/instances.txt b/docs/ref/models/instances.txt index 45c538a782..519da0cf3d 100644 --- a/docs/ref/models/instances.txt +++ b/docs/ref/models/instances.txt @@ -410,6 +410,28 @@ For more details, including how to delete objects in bulk, see If you want customized deletion behavior, you can override the ``delete()`` method. See :ref:`overriding-model-methods` for more details. +Pickling objects +================ + +When you :mod:`pickle` a model, its current state is pickled. When you unpickle +it, it'll contain the model instance at the moment it was pickled, rather than +the data that's currently in the database. + +.. admonition:: You can't share pickles between versions + + Pickles of models are only valid for the version of Django that + was used to generate them. If you generate a pickle using Django + version N, there is no guarantee that pickle will be readable with + Django version N+1. Pickles should not be used as part of a long-term + archival strategy. + + .. versionadded:: 1.8 + + Since pickle compatibility errors can be difficult to diagnose, such as + silently corrupted objects, a ``RuntimeWarning`` is raised when you try to + unpickle a model in a Django version that is different than the one in + which it was pickled. + .. _model-instance-methods: Other model instance methods diff --git a/docs/ref/models/querysets.txt b/docs/ref/models/querysets.txt index 242a4dc2e1..70d6946922 100644 --- a/docs/ref/models/querysets.txt +++ b/docs/ref/models/querysets.txt @@ -116,6 +116,13 @@ described here. Django version N+1. Pickles should not be used as part of a long-term archival strategy. + .. versionadded:: 1.8 + + Since pickle compatibility errors can be difficult to diagnose, such as + silently corrupted objects, a ``RuntimeWarning`` is raised when you try to + unpickle a queryset in a Django version that is different than the one in + which it was pickled. + .. _queryset-api: QuerySet API diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index 426e03b87f..86f9a122c2 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -176,6 +176,13 @@ Models * Django now logs at most 9000 queries in ``connections.queries``, in order to prevent excessive memory usage in long-running processes in debug mode. +* Pickling models and querysets across different versions of Django isn't + officially supported (it may work, but there's no guarantee). An extra + variable that specifies the current Django version is now added to the + pickled state of models and querysets, and Django raises a ``RuntimeWarning`` + when these objects are unpickled in a different version than the one in + which they were pickled. + Signals ^^^^^^^ diff --git a/tests/model_regress/test_pickle.py b/tests/model_regress/test_pickle.py new file mode 100644 index 0000000000..8dc90893bf --- /dev/null +++ b/tests/model_regress/test_pickle.py @@ -0,0 +1,53 @@ +import pickle +import warnings + +from django.db import models, DJANGO_VERSION_PICKLE_KEY +from django.test import TestCase +from django.utils.encoding import force_text +from django.utils.version import get_major_version, get_version + + +class ModelPickleTestCase(TestCase): + def test_missing_django_version_unpickling(self): + """ + #21430 -- Verifies a warning is raised for models that are + unpickled without a Django version + """ + class MissingDjangoVersion(models.Model): + title = models.CharField(max_length=10) + + def __reduce__(self): + reduce_list = super(MissingDjangoVersion, self).__reduce__() + data = reduce_list[-1] + del data[DJANGO_VERSION_PICKLE_KEY] + return reduce_list + + p = MissingDjangoVersion(title="FooBar") + with warnings.catch_warnings(record=True) as recorded: + pickle.loads(pickle.dumps(p)) + msg = force_text(recorded.pop().message) + self.assertEqual(msg, + "Pickled model instance's Django version is not specified.") + + def test_unsupported_unpickle(self): + """ + #21430 -- Verifies a warning is raised for models that are + unpickled with a different Django version than the current + """ + class DifferentDjangoVersion(models.Model): + title = models.CharField(max_length=10) + + def __reduce__(self): + reduce_list = super(DifferentDjangoVersion, self).__reduce__() + data = reduce_list[-1] + data[DJANGO_VERSION_PICKLE_KEY] = str(float(get_major_version()) - 0.1) + return reduce_list + + p = DifferentDjangoVersion(title="FooBar") + with warnings.catch_warnings(record=True) as recorded: + pickle.loads(pickle.dumps(p)) + msg = force_text(recorded.pop().message) + self.assertEqual(msg, + "Pickled model instance's Django version %s does not " + "match the current version %s." + % (str(float(get_major_version()) - 0.1), get_version())) diff --git a/tests/queryset_pickle/models.py b/tests/queryset_pickle/models.py index 1e73774cd2..4c655fa980 100644 --- a/tests/queryset_pickle/models.py +++ b/tests/queryset_pickle/models.py @@ -1,7 +1,8 @@ import datetime -from django.db import models +from django.db import models, DJANGO_VERSION_PICKLE_KEY from django.utils.translation import ugettext_lazy as _ +from django.utils.version import get_major_version def standalone_number(): @@ -23,8 +24,25 @@ class Numbers(object): nn = Numbers() +class PreviousDjangoVersionQuerySet(models.QuerySet): + def __getstate__(self): + state = super(PreviousDjangoVersionQuerySet, self).__getstate__() + state[DJANGO_VERSION_PICKLE_KEY] = str(float(get_major_version()) - 0.1) # previous major version + return state + + +class MissingDjangoVersionQuerySet(models.QuerySet): + def __getstate__(self): + state = super(MissingDjangoVersionQuerySet, self).__getstate__() + del state[DJANGO_VERSION_PICKLE_KEY] + return state + + class Group(models.Model): name = models.CharField(_('name'), max_length=100) + objects = models.Manager() + previous_django_version_objects = PreviousDjangoVersionQuerySet.as_manager() + missing_django_version_objects = MissingDjangoVersionQuerySet.as_manager() class Event(models.Model): diff --git a/tests/queryset_pickle/tests.py b/tests/queryset_pickle/tests.py index 45b75dd0e9..a48b9591ab 100644 --- a/tests/queryset_pickle/tests.py +++ b/tests/queryset_pickle/tests.py @@ -2,8 +2,11 @@ from __future__ import unicode_literals import pickle import datetime +import warnings from django.test import TestCase +from django.utils.encoding import force_text +from django.utils.version import get_major_version, get_version from .models import Group, Event, Happening, Container, M2MModel @@ -108,3 +111,29 @@ class PickleabilityTestCase(TestCase): # Second pickling groups = pickle.loads(pickle.dumps(groups)) self.assertQuerysetEqual(groups, [g], lambda x: x) + + def test_missing_django_version_unpickling(self): + """ + #21430 -- Verifies a warning is raised for querysets that are + unpickled without a Django version + """ + qs = Group.missing_django_version_objects.all() + with warnings.catch_warnings(record=True) as recorded: + pickle.loads(pickle.dumps(qs)) + msg = force_text(recorded.pop().message) + self.assertEqual(msg, + "Pickled queryset instance's Django version is not specified.") + + def test_unsupported_unpickle(self): + """ + #21430 -- Verifies a warning is raised for querysets that are + unpickled with a different Django version than the current + """ + qs = Group.previous_django_version_objects.all() + with warnings.catch_warnings(record=True) as recorded: + pickle.loads(pickle.dumps(qs)) + msg = force_text(recorded.pop().message) + self.assertEqual(msg, + "Pickled queryset instance's Django version %s does not " + "match the current version %s." + % (str(float(get_major_version()) - 0.1), get_version()))