Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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 @@ -76,6 +76,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
like this, the Mailbox acquires a lock when the :keyword:`with` statement
enters and releases it and :meth:`close` when the :keyword:`with` statement
exits.
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.

Suggested change
exits.
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.

Comment thread
encukou marked this conversation as resolved.
Outdated

.. versionchanged:: 3.7
Comment thread
sblondon marked this conversation as resolved.
Outdated
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 @@ -38,6 +38,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
5 changes: 5 additions & 0 deletions Lib/test/test_mailbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,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._factory(self._path) as box:
self.assertIsInstance(box, self._box.__class__)

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 add a test for the lock?

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Mailbox instances can be used in the context manager. The Mailbox is locked
Comment thread
sblondon marked this conversation as resolved.
Outdated
at enter and unlocked and closed at exit.