Skip to content

Commit

Permalink
Allow default pickle protocol to be specified using a setting instead.
Browse files Browse the repository at this point in the history
Bumping this value without allowing an override breaks backward compatibility
for lookups such as __exact because the string representation of the pickles
change.
  • Loading branch information
charettes committed Jun 5, 2020
1 parent fade70d commit 0d84840
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 4 deletions.
3 changes: 2 additions & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,9 @@ Changes
UNRELEASED
==========

* Allowed default pickle protocol to be overriden using the
`PICKLEFIELD_DEFAULT_PROTOCOL` setting.
* Dropped support for Python 2.
* Updated default pickle protocol to version 3.
* Added testing against Django 3.0.
* Dropped support for Django 1.11.

Expand Down
2 changes: 1 addition & 1 deletion picklefield/constants.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
DEFAULT_PROTOCOL = 3
DEFAULT_PROTOCOL = 2
22 changes: 20 additions & 2 deletions picklefield/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from pickle import dumps, loads
from zlib import compress, decompress

from django.conf import settings
from django.core import checks
from django.db import models
from django.utils.encoding import force_str
Expand Down Expand Up @@ -45,13 +46,19 @@ def wrap_conflictual_object(obj):
return obj


def dbsafe_encode(value, compress_object=False, pickle_protocol=DEFAULT_PROTOCOL, copy=True):
def get_default_protocol():
return getattr(settings, 'PICKLEFIELD_DEFAULT_PROTOCOL', DEFAULT_PROTOCOL)


def dbsafe_encode(value, compress_object=False, pickle_protocol=None, copy=True):
# We use deepcopy() here to avoid a problem with cPickle, where dumps
# can generate different character streams for same lookup value if
# they are referenced differently.
# The reason this is important is because we do all of our lookups as
# simple string matches, thus the character streams must be the same
# for the lookups to work properly. See tests.py for more information.
if pickle_protocol is None:
pickle_protocol = get_default_protocol()
if copy:
# Copy can be very expensive if users aren't going to perform lookups
# on the value anyway.
Expand Down Expand Up @@ -85,7 +92,10 @@ class PickledObjectField(models.Field):

def __init__(self, *args, **kwargs):
self.compress = kwargs.pop('compress', False)
self.protocol = kwargs.pop('protocol', DEFAULT_PROTOCOL)
protocol = kwargs.pop('protocol', None)
if protocol is None:
protocol = get_default_protocol()
self.protocol = protocol
self.copy = kwargs.pop('copy', True)
kwargs.setdefault('editable', False)
super().__init__(*args, **kwargs)
Expand Down Expand Up @@ -136,6 +146,14 @@ def check(self, **kwargs):
errors.extend(self._check_default())
return errors

def deconstruct(self):
name, path, args, kwargs = super().deconstruct()
if self.compress:
kwargs['compress'] = True
if self.protocol != get_default_protocol():
kwargs['protocol'] = self.protocol
return name, path, args, kwargs

def to_python(self, value):
"""
B64decode and unpickle the object, optionally decompressing it.
Expand Down
11 changes: 11 additions & 0 deletions tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,17 @@ def mock_decode_error(*args, **kwargs):
self.assertEqual(encoded_value, MinimalTestingModel.objects.get(pk=model.pk).pickle_field)


class PickledObjectFieldDeconstructTests(SimpleTestCase):
def test_protocol(self):
field = PickledObjectField()
self.assertNotIn('protocol', field.deconstruct()[3])
with self.settings(PICKLEFIELD_DEFAULT_PROTOCOL=3):
field = PickledObjectField(protocol=4)
self.assertEqual(field.deconstruct()[3].get('protocol'), 4)
field = PickledObjectField(protocol=3)
self.assertNotIn('protocol', field.deconstruct()[3])


@isolate_apps('tests')
class PickledObjectFieldCheckTests(SimpleTestCase):
def test_mutable_default_check(self):
Expand Down

0 comments on commit 0d84840

Please sign in to comment.