Skip to content
Open
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
8 changes: 8 additions & 0 deletions Doc/library/mailbox.rst
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,14 @@ Supported mailbox formats are Maildir, mbox, MH, Babyl, and MMDF.
message. Failing to lock the mailbox runs the risk of losing messages or
corrupting the entire mailbox.

The :class:`!Mailbox` class supports the :keyword:`with` statement. When used
as a context manager, :class:`!Mailbox` calls :meth:`lock` when the context is entered,
returns the mailbox object as the context object, and at context end calls :meth:`close`,
thereby releasing the lock.

.. versionchanged:: next
Support for the :keyword:`with` statement was added.

:class:`!Mailbox` instances have the following methods:


Expand Down
7 changes: 7 additions & 0 deletions Lib/mailbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ def __init__(self, path, factory=None, create=True):
self._path = os.path.abspath(os.path.expanduser(path))
self._factory = factory

def __enter__(self):
self.lock()
return self

def __exit__(self, type, value, traceback):
self.close()

def add(self, message):
"""Add message and return assigned key."""
raise NotImplementedError('Method must be implemented by subclass')
Expand Down
15 changes: 15 additions & 0 deletions Lib/test/test_mailbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,11 @@ def _test_flush_or_close(self, method, should_call_close):
self.assertIn(self._box.get_string(key), contents)
oldbox.close()

def test_use_context_manager(self):
# Mailboxes are usable as a context manager
with self._box as box:
self.assertIs(self._box, box)

Copy link
Member

Choose a reason for hiding this comment

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

Could you add a test for the lock?

Copy link
Contributor Author

@sblondon sblondon Aug 5, 2019

Choose a reason for hiding this comment

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

@matrixise I'm unsure how to do it right. For example:

  • Maildir doesn't use the lock. So the test is not useful there.
  • Mailbox uses it and stores the state in an internal boolean variable (self._lock). So the test would check the implementation.

It would be nice to have a locked attribute for Mailbox but it will not work for each case and is probably too invasive for this patch.

Copy link
Member

Choose a reason for hiding this comment

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

good question, I don't remember your code, but I am going to assign myself your PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, thank you!

Copy link
Member

Choose a reason for hiding this comment

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

This can be tested in _TestSingleFile, which tests the classes that have _locked and _file attributes.

Copy link
Member

Choose a reason for hiding this comment

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

It looks like this test is redundant now with the new test.

Copy link
Member

Choose a reason for hiding this comment

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

The other test is only for the subclasses that have _locked & _file. This one tests that all Mailboxes are usable as a context manager.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, that makes sense.

def test_dump_message(self):
# Write message representations to disk
for input in (email.message_from_string(_sample_message),
Expand Down Expand Up @@ -1122,6 +1127,16 @@ def test_ownership_after_flush(self):
self.assertEqual(st.st_gid, other_gid)
self.assertEqual(st.st_mode, mode)

def test_context_manager_locks_and_closes(self):
# Context manager locks/unlocks and closes.
# (This test uses an implementation detail to get the state.)
self.assertFalse(self._box._locked)
with self._box as context_object:
self.assertIs(self._box, context_object)
self.assertTrue(self._box._locked)
self.assertFalse(self._box._file.closed)
self.assertFalse(self._box._locked)
self.assertTrue(self._box._file.closed)

class _TestMboxMMDF(_TestSingleFile):

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
:class:`mailbox.Mailbox` instances can now be used as a context manager.
The Mailbox is locked on context entry and unlocked and closed at context exit.
Loading