Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions Lib/test/test_uuid.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
import contextlib
import io
import os
import pickle
import shutil
import subprocess
import sys

py_uuid = support.import_fresh_module('uuid', blocked=['_uuid'])
c_uuid = support.import_fresh_module('uuid', fresh=['_uuid'])
Expand Down Expand Up @@ -311,6 +313,64 @@ def test_getnode(self):
node2 = self.uuid.getnode()
self.assertEqual(node1, node2, '%012x != %012x' % (node1, node2))

def _setup_for_pickle(self):
orig_uuid = sys.modules.get('uuid')
sys.modules['uuid'] = self.uuid

def restore_uuid_module():
if orig_uuid is not None:
sys.modules['uuid'] = orig_uuid
else:
del sys.modules['uuid']
self.addCleanup(restore_uuid_module)

def test_pickle_roundtrip(self):
self._setup_for_pickle()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't be easier to use with support.swap_item(sys.modules, 'uuid', self.uuid):?


u = self.uuid.UUID('12345678123456781234567812345678')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to test with different is_safe values. And please used UUID with less regular hexadecimal representation.

self.assertEqual(u, pickle.loads(pickle.dumps(u)))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to test with all pickle protocols. Need to test shallow and deep copying too, since they use the same protocol.


def test_unpickle_previous_python_versions(self):
self._setup_for_pickle()

u = self.uuid.UUID('12345678123456781234567812345678')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you use UUID that doesn't contain regular patterns?


# Python 2.7 protocol 0-2 pickles of u

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to pass pickles through pickletools.optimize() for removing unneeded memoization. This will make the input data shorter.

py27_pickles = [
b'ccopy_reg\n_reconstructor\np0\n(cuuid\nUUID\np1\nc__builtin__\nob'
b'ject\np2\nNtp3\nRp4\n(dp5\nS\'int\'\np6\nL24197857161011715162171'
b'839636988778104L\nsb.',
b'ccopy_reg\n_reconstructor\nq\x00(cuuid\nUUID\nq\x01c__builtin__\n'
b'object\nq\x02Ntq\x03Rq\x04}q\x05U\x03intq\x06L2419785716101171516'
b'2171839636988778104L\nsb.',
b'\x80\x02cuuid\nUUID\nq\x00)\x81q\x01}q\x02U\x03intq\x03\x8a\x10xV'
b'4\x12xV4\x12xV4\x12xV4\x12sb.',
]
# Python 3.6 protocol 0-4 pickles of u
py36_pickles = [
b'ccopy_reg\n_reconstructor\np0\n(cuuid\nUUID\np1\nc__builtin__\nob'
b'ject\np2\nNtp3\nRp4\n(dp5\nVint\np6\nL241978571610117151621718396'
b'36988778104L\nsb.',
b'ccopy_reg\n_reconstructor\nq\x00(cuuid\nUUID\nq\x01c__builtin__\n'
b'object\nq\x02Ntq\x03Rq\x04}q\x05X\x03\x00\x00\x00intq\x06L2419785'
b'7161011715162171839636988778104L\nsb.',
b'\x80\x02cuuid\nUUID\nq\x00)\x81q\x01}q\x02X\x03\x00\x00\x00intq'
b'\x03\x8a\x10xV4\x12xV4\x12xV4\x12xV4\x12sb.',
b'\x80\x03cuuid\nUUID\nq\x00)\x81q\x01}q\x02X\x03\x00\x00\x00intq'
b'\x03\x8a\x10xV4\x12xV4\x12xV4\x12xV4\x12sb.',
b'\x80\x04\x950\x00\x00\x00\x00\x00\x00\x00\x8c\x04uuid\x94\x8c\x04'
b'UUID\x94\x93\x94)\x81\x94}\x94\x8c\x03int\x94\x8a\x10xV4\x12xV4'
b'\x12xV4\x12xV4\x12sb.',
]

for pickled in py27_pickles + py36_pickles:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why two lists are used?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clearly differentiate between those generated with different Python versions, using variable names rather than comments.

unpickled = pickle.loads(pickled)
self.assertEqual(unpickled, u)
# is_safe was added in 3.7. When unpickling values from older
# versions, is_safe will be missing, so it should be set to
# SafeUUID.unknown.
self.assertEqual(unpickled.is_safe, self.uuid.SafeUUID.unknown)

# bpo-32502: UUID1 requires a 48-bit identifier, but hardware identifiers
# need not necessarily be 48 bits (e.g., EUI-64).
def test_uuid1_eui64(self):
Expand Down
28 changes: 26 additions & 2 deletions Lib/uuid.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ class UUID:
uuid_generate_time_safe(3).
"""

__slots__ = ('int', 'is_safe')

def __init__(self, hex=None, bytes=None, bytes_le=None, fields=None,
int=None, version=None,
*, is_safe=SafeUUID.unknown):
Expand Down Expand Up @@ -201,8 +203,30 @@ def __init__(self, hex=None, bytes=None, bytes_le=None, fields=None,
# Set the version number.
int &= ~(0xf000 << 64)
int |= version << 76
self.__dict__['int'] = int
self.__dict__['is_safe'] = is_safe
object.__setattr__(self, 'int', int)
object.__setattr__(self, 'is_safe', is_safe)

def __getstate__(self):
d = {attr: getattr(self, attr) for attr in self.__slots__}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use self.__slots__ instead of just hardcoding two attribute names? In any case this code doesn't work in case of subclasses with additional attributes.

If is_safe is SafeUUID.unknown, it can be omitted for saving space.

# is_safe is a SafeUUID instance. Return just its value, so that

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the value portable? Seems there is a problem in 3.7 which should be fixed (this is a different issue).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean. The value is one of None, 0 or -1, so it should be "portable" for any meaning of that word.

We have to keep just the value to avoid errors when unpickling in Python versions <3.7, where the SafeUUID enum isn't defined. Using the .value, is_safe is ignored and unpickling succeeds.

# it can be unpickled in older Python versions without SafeUUID.
d['is_safe'] = d['is_safe'].value
return d

def __setstate__(self, state):
# is_safe was added in 3.7
state.setdefault('is_safe', SafeUUID.unknown.value)

for attr in self.__slots__:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code would be simpler if just set int and is_safe attributes.

value = state[attr]

# for is_safe, restore the SafeUUID from the stored value
if attr == 'is_safe':
try:
value = SafeUUID(value)
except ValueError:
value = SafeUUID.unknown

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this error is silenced?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You brought up the need to support pickles from future Python versions; those could potentially define new values in the SafeUUID enum.

object.__setattr__(self, attr, value)

def __eq__(self, other):
if isinstance(other, UUID):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Make uuid.UUID use ``__slots__`` to reduce its memory footprint. Based on
original patch by Wouter Bolsterlee.