From b22946957d7c3ef5a46d0bb2031e54f9723825d7 Mon Sep 17 00:00:00 2001 From: Carey Metcalfe Date: Sun, 14 Nov 2021 02:02:59 -0500 Subject: [PATCH 1/5] Implement io.IOBase interface for SpooledTemporaryFile Since the underlying file-like objects (either `io.BytesIO`, `io.StringIO`, or a true file object) all implement the `io.IOBase` interface, the `SpooledTemporaryFile` should as well. Additionally, since the underlying file object will either be an instance of an `io.BufferedIOBase` (for binary mode) or an `io.TextIOBase` (for text mode), methods for these classes were also implemented. In every case, the required methods and properties are simply delegated to the underlying file object. Co-authored-by: Gary Fernie --- Doc/library/tempfile.rst | 5 ++ Lib/tempfile.py | 42 +++++++++++++--- Lib/test/test_tempfile.py | 48 +++++++++++++++++++ .../2021-11-14-01-35-04.bpo-26175.LNlOfI.rst | 2 + 4 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst diff --git a/Doc/library/tempfile.rst b/Doc/library/tempfile.rst index 3e904d04642d80..b45b9bc954df14 100644 --- a/Doc/library/tempfile.rst +++ b/Doc/library/tempfile.rst @@ -120,6 +120,11 @@ The module defines the following user-callable items: .. versionchanged:: 3.8 Added *errors* parameter. + .. versionchanged:: 3.11 + Fully implements the :class:`io.BufferedIOBase` and + :class:`io.TextIOBase` abstract base classes (depending on whether binary + or text *mode* was specified). + .. function:: TemporaryDirectory(suffix=None, prefix=None, dir=None, ignore_cleanup_errors=False) diff --git a/Lib/tempfile.py b/Lib/tempfile.py index 531cbf32f1283f..9d7f5bd393f184 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -639,7 +639,7 @@ def TemporaryFile(mode='w+b', buffering=-1, encoding=None, _os.close(fd) raise -class SpooledTemporaryFile: +class SpooledTemporaryFile(_io.IOBase): """Temporary file wrapper, specialized to switch from BytesIO or StringIO to a real file when it exceeds a certain size or when a fileno is needed. @@ -704,6 +704,16 @@ def __exit__(self, exc, value, tb): def __iter__(self): return self._file.__iter__() + def __del__(self): + if not self.closed: + _warnings.warn( + "Unclosed file {!r}".format(self), + ResourceWarning, + stacklevel=2, + source=self + ) + self.close() + def close(self): self._file.close() @@ -747,15 +757,30 @@ def name(self): def newlines(self): return self._file.newlines + def readable(self): + return self._file.readable() + def read(self, *args): return self._file.read(*args) + def read1(self, *args): + return self._file.read1(*args) + + def readinto(self, b): + return self._file.readinto(b) + + def readinto1(self, b): + return self._file.readinto1(b) + def readline(self, *args): return self._file.readline(*args) def readlines(self, *args): return self._file.readlines(*args) + def seekable(self): + return self._file.seekable() + def seek(self, *args): return self._file.seek(*args) @@ -763,12 +788,12 @@ def tell(self): return self._file.tell() def truncate(self, size=None): - if size is None: - self._file.truncate() - else: - if size > self._max_size: - self.rollover() - self._file.truncate(size) + if size is not None and size > self._max_size: + self.rollover() + return self._file.truncate(size) + + def writable(self): + return self._file.writable() def write(self, s): file = self._file @@ -782,6 +807,9 @@ def writelines(self, iterable): self._check(file) return rv + def detach(self): + return self._file.detach() + class TemporaryDirectory: """Create and return a temporary directory. This has the same diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 2b0ec46a103276..593f198e2b6eba 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1058,6 +1058,30 @@ def test_basic(self): f = self.do_create(max_size=100, pre="a", suf=".txt") self.assertFalse(f._rolled) + def test_is_iobase(self): + # SpooledTemporaryFile should implement io.IOBase + self.assertIsInstance(self.do_create(), io.IOBase) + + def test_iobase_interface(self): + # SpooledTemporaryFile should implement the io.IOBase interface. + # Ensure it has all the required methods and properties. + iobase_attrs = { + # From IOBase + 'fileno', 'seek', 'truncate', 'close', 'closed', '__enter__', + '__exit__', 'flush', 'isatty', '__iter__', '__next__', 'readable', + 'readline', 'readlines', 'seekable', 'tell', 'writable', + 'writelines', + # From BufferedIOBase (binary mode) and TextIOBase (text mode) + 'detach', 'read', 'read1', 'write', 'readinto', 'readinto1', + 'encoding', 'errors', 'newlines', + } + spooledtempfile_attrs = set(dir(tempfile.SpooledTemporaryFile)) + missing_attrs = iobase_attrs - spooledtempfile_attrs + self.assertFalse( + missing_attrs, + 'SpooledTemporaryFile missing attributes from IOBase/BufferedIOBase/TextIOBase' + ) + def test_del_on_close(self): # A SpooledTemporaryFile is deleted when closed dir = tempfile.mkdtemp() @@ -1073,6 +1097,30 @@ def test_del_on_close(self): finally: os.rmdir(dir) + def test_del_unrolled_file(self): + # The unrolled SpooledTemporaryFile should raise a ResourceWarning + # when deleted since the file was not explicitly closed. + f = self.do_create(max_size=10) + f.write(b'foo') + self.assertEqual(f.name, None) # Unrolled so no filename/fd + with self.assertWarns(ResourceWarning): + del f + + def test_del_rolled_file(self): + # The rolled file should be deleted when the SpooledTemporaryFile + # object is deleted. This should raise a ResourceWarning since the file + # was not explicitly closed. + f = self.do_create(max_size=2) + f.write(b'foo') + name = f.name # This is a fd on posix+cygwin, a filename everywhere else + self.assertTrue(os.path.exists(name)) + with self.assertWarns(ResourceWarning): + del f + self.assertFalse( + os.path.exists(name), + "Rolled SpooledTemporaryFile (name=%s) exists after delete" % name + ) + def test_rewrite_small(self): # A SpooledTemporaryFile can be written to multiple within the max_size f = self.do_create(max_size=30) diff --git a/Misc/NEWS.d/next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst b/Misc/NEWS.d/next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst new file mode 100644 index 00000000000000..f521cc5313cd84 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst @@ -0,0 +1,2 @@ +Fully implement the :class:`io.BufferedIOBase` and :class:`io.TextIOBase` +interfaces for :class:`tempfile.SpooledTemporaryFile`. From 753aeb0f903d3d4df5ae1568ed44a3b24b67576a Mon Sep 17 00:00:00 2001 From: Carey Metcalfe Date: Sun, 2 Jan 2022 21:21:13 -0500 Subject: [PATCH 2/5] Fix calling 'del' in tests --- Lib/test/test_tempfile.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 593f198e2b6eba..53f7d63dcc3b40 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1104,7 +1104,7 @@ def test_del_unrolled_file(self): f.write(b'foo') self.assertEqual(f.name, None) # Unrolled so no filename/fd with self.assertWarns(ResourceWarning): - del f + f.__del__() def test_del_rolled_file(self): # The rolled file should be deleted when the SpooledTemporaryFile @@ -1115,7 +1115,7 @@ def test_del_rolled_file(self): name = f.name # This is a fd on posix+cygwin, a filename everywhere else self.assertTrue(os.path.exists(name)) with self.assertWarns(ResourceWarning): - del f + f.__del__() self.assertFalse( os.path.exists(name), "Rolled SpooledTemporaryFile (name=%s) exists after delete" % name From d93781b8c1817f2936635ae573c6e0d61bd86595 Mon Sep 17 00:00:00 2001 From: Carey Metcalfe Date: Wed, 23 Feb 2022 03:31:29 +0000 Subject: [PATCH 3/5] Apply suggestion from @merwok --- .../next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst b/Misc/NEWS.d/next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst index f521cc5313cd84..83b1e45f9b7088 100644 --- a/Misc/NEWS.d/next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst +++ b/Misc/NEWS.d/next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst @@ -1,2 +1,3 @@ -Fully implement the :class:`io.BufferedIOBase` and :class:`io.TextIOBase` -interfaces for :class:`tempfile.SpooledTemporaryFile`. +Fully implement the :class:`io.BufferedIOBase` or :class:`io.TextIOBase` +interface for :class:`tempfile.SpooledTemporaryFile` objects. This lets +them work correctly with higher-level layers (like compression modules). From 145668affd4a2567cead63bc32a0d5649fbdb63d Mon Sep 17 00:00:00 2001 From: Carey Metcalfe Date: Thu, 24 Feb 2022 02:57:47 +0000 Subject: [PATCH 4/5] Revert improvement to truncate control flow --- Lib/tempfile.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/tempfile.py b/Lib/tempfile.py index 9d7f5bd393f184..eb870c998c251a 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -788,9 +788,12 @@ def tell(self): return self._file.tell() def truncate(self, size=None): - if size is not None and size > self._max_size: - self.rollover() - return self._file.truncate(size) + if size is None: + return self._file.truncate() + else: + if size > self._max_size: + self.rollover() + return self._file.truncate(size) def writable(self): return self._file.writable() From 9ebd545f45d9e90e4c499368206a694983d368f2 Mon Sep 17 00:00:00 2001 From: Carey Metcalfe Date: Fri, 25 Feb 2022 02:23:50 +0000 Subject: [PATCH 5/5] Added name to ACKs and patch notes --- Misc/ACKS | 1 + .../next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Misc/ACKS b/Misc/ACKS index 8baaf7304b603c..cad354db9266c1 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -1164,6 +1164,7 @@ Dimitri Merejkowsky Brian Merrell Bruce Merry Alexis Métaireau +Carey Metcalfe Luke Mewburn Carl Meyer Kyle Meyer diff --git a/Misc/NEWS.d/next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst b/Misc/NEWS.d/next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst index 83b1e45f9b7088..89072b3c04f393 100644 --- a/Misc/NEWS.d/next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst +++ b/Misc/NEWS.d/next/Library/2021-11-14-01-35-04.bpo-26175.LNlOfI.rst @@ -1,3 +1,4 @@ Fully implement the :class:`io.BufferedIOBase` or :class:`io.TextIOBase` -interface for :class:`tempfile.SpooledTemporaryFile` objects. This lets -them work correctly with higher-level layers (like compression modules). +interface for :class:`tempfile.SpooledTemporaryFile` objects. This lets them +work correctly with higher-level layers (like compression modules). Patch by +Carey Metcalfe.