PYTHON-2824 Make GridOut implement full io.IOBase spec (#677)

Make GridOut inherit from io.IOBase to be a fully "file-like" object (https://docs.python.org/3/glossary.html#term-file-like-object). Implement missing methods `readlines`, `writelines`,
`writable`, `fileno`, `flush`, `isatty`, `truncate`,
and property `closed`, following the spec
(https://docs.python.org/3/library/io.html#io.IOBase.writable).

Iterating over GridOut previously returned chunks, but IOBase specifies
that lines should be returned. Thus, the `GridOutIterator` returning chunks is removed
and GridOut simply uses the existing IOBase iterator implementation (returning `self`
in `__iter__` and using `readline` in `__next__`).

Additionally, iterating over GridOut previously did not move the "file pointer" along, i.e.
`next(iter(some_grid_out_object))` always gave the same result (the first chunk of the file)
as it would create a new iterator starting at the top of the file. This is now fixed as well, so
a first call to `next(iter(some_grid_out_object))` gives the first line, and subsequent calls return
the subsequent lines.

Author: henrifroese

doc/changelog.rst

@@ -126,6 +126,10 @@ Changes in Version 3.12.0
 
 - PyMongoCrypt 1.1.0 or later is now required for client side field level
   encryption support.
+- Iterating over :class:`gridfs.grid_file.GridOut` now moves through
+  the file line by line instead of chunk by chunk, and does not
+  restart at the top for subsequent iterations on the same object.
+  Call `seek(0)` to reset the iterator.
 
 Notable improvements
 ....................

doc/contributors.rst

@@ -88,4 +88,5 @@ The following is a list of people who have contributed to
 - Terence Honles (terencehonles)
 - Paul Fisher (thetorpedodog)
 - Julius Park (juliusgeo)
-- Khanh Nguyen (KN99HN)
+- Khanh Nguyen (KN99HN)
+- Henri Froese (henrifroese)

gridfs/grid_file.py

@@ -421,7 +421,7 @@ def __exit__(self, exc_type, exc_val, exc_tb):
         return False
 
 
-class GridOut(object):
+class GridOut(io.IOBase):
     """Class to read data out of GridFS.
     """
     def __init__(self, root_collection, file_id=None, file_document=None,
@@ -465,6 +465,8 @@ def __init__(self, root_collection, file_id=None, file_document=None,
 
         root_collection = _clear_entity_type_registry(root_collection)
 
+        super().__init__()
+
         self.__chunks = root_collection.chunks
         self.__files = root_collection.files
         self.__file_id = file_id
@@ -656,33 +658,40 @@ def seekable(self):
     def __iter__(self):
         """Return an iterator over all of this file's data.
 
-        The iterator will return chunk-sized instances of
-        :class:`str` (:class:`bytes` in python 3). This can be
-        useful when serving files using a webserver that handles
-        such an iterator efficiently.
-
-        .. note::
-           This is different from :py:class:`io.IOBase` which iterates over
-           *lines* in the file. Use :meth:`GridOut.readline` to read line by
-           line instead of chunk by chunk.
+        The iterator will return lines (delimited by b'\n') of
+        :class:`bytes`. This can be useful when serving files
+        using a webserver that handles such an iterator efficiently.
 
         .. versionchanged:: 3.8
            The iterator now raises :class:`CorruptGridFile` when encountering
            any truncated, missing, or extra chunk in a file. The previous
            behavior was to only raise :class:`CorruptGridFile` on a missing
            chunk.
+
+        .. versionchanged:: 4.0
+           The iterator now iterates over *lines* in the file, instead
+           of chunks, to conform to the base class :py:class:`io.IOBase`.
+           Use :meth:`GridOut.readchunk` to read chunk by chunk instead
+           of line by line.
         """
-        return GridOutIterator(self, self.__chunks, self._session)
+        return self
 
     def close(self):
         """Make GridOut more generically file-like."""
         if self.__chunk_iter:
             self.__chunk_iter.close()
             self.__chunk_iter = None
+        super().close()
 
     def write(self, value):
         raise io.UnsupportedOperation('write')
 
+    def writelines(self, lines):
+        raise io.UnsupportedOperation('writelines')
+
+    def writable(self):
+        return False
+
     def __enter__(self):
         """Makes it possible to use :class:`GridOut` files
         with the context manager protocol.
@@ -696,6 +705,27 @@ def __exit__(self, exc_type, exc_val, exc_tb):
         self.close()
         return False
 
+    def fileno(self):
+        raise io.UnsupportedOperation('fileno')
+
+    def flush(self):
+        # GridOut is read-only, so flush does nothing.
+        pass
+
+    def isatty(self):
+        return False
+
+    def truncate(self, size=None):
+        # See https://docs.python.org/3/library/io.html#io.IOBase.writable
+        # for why truncate has to raise.
+        raise io.UnsupportedOperation('truncate')
+
+    # Override IOBase.__del__ otherwise it will lead to __getattr__ on
+    # __IOBase_closed which calls _ensure_file and potentially performs I/O.
+    # We cannot do I/O in __del__ since it can lead to a deadlock.
+    def __del__(self):
+        pass
+
 
 class _GridOutChunkIterator(object):
     """Iterates over a file's chunks using a single cursor.

test/test_grid_file.py

@@ -18,6 +18,7 @@
 """
 
 import datetime
+import io
 import sys
 import zipfile
 
@@ -323,6 +324,20 @@ def test_close(self):
         self.assertRaises(ValueError, f.write, "test")
         f.close()
 
+    def test_closed(self):
+        f = GridIn(self.db.fs, chunkSize=5)
+        f.write(b"Hello world.\nHow are you?")
+        f.close()
+
+        g = GridOut(self.db.fs, f._id)
+        self.assertFalse(g.closed)
+        g.read(1)
+        self.assertFalse(g.closed)
+        g.read(100)
+        self.assertFalse(g.closed)
+        g.close()
+        self.assertTrue(g.closed)
+
     def test_multi_chunk_file(self):
         random_string = b'a' * (DEFAULT_CHUNK_SIZE + 1000)
 
@@ -447,27 +462,85 @@ def test_readline(self):
         self.assertEqual(b"e", g.readline(1))
         self.assertEqual(b"llo world,\n", g.readline())
 
+    def test_readlines(self):
+        f = GridIn(self.db.fs, chunkSize=5)
+        f.write((b"""Hello world,
+How are you?
+Hope all is well.
+Bye"""))
+        f.close()
+
+        # Try read(), then readlines().
+        g = GridOut(self.db.fs, f._id)
+        self.assertEqual(b"He", g.read(2))
+        self.assertEqual([b"llo world,\n", b"How are you?\n"], g.readlines(11))
+        self.assertEqual([b"Hope all is well.\n", b"Bye"], g.readlines())
+        self.assertEqual([], g.readlines())
+
+        # Try readline(), then readlines().
+        g = GridOut(self.db.fs, f._id)
+        self.assertEqual(b"Hello world,\n", g.readline())
+        self.assertEqual([b"How are you?\n", b"Hope all is well.\n"], g.readlines(13))
+        self.assertEqual(b"Bye", g.readline())
+        self.assertEqual([], g.readlines())
+
+        # Only readlines().
+        g = GridOut(self.db.fs, f._id)
+        self.assertEqual(
+            [b"Hello world,\n", b"How are you?\n", b"Hope all is well.\n", b"Bye"],
+            g.readlines())
+
+        g = GridOut(self.db.fs, f._id)
+        self.assertEqual(
+            [b"Hello world,\n", b"How are you?\n", b"Hope all is well.\n", b"Bye"],
+            g.readlines(0))
+
+        g = GridOut(self.db.fs, f._id)
+        self.assertEqual([b"Hello world,\n"], g.readlines(1))
+        self.assertEqual([b"How are you?\n"], g.readlines(12))
+        self.assertEqual([b"Hope all is well.\n", b"Bye"], g.readlines(18))
+
+        # Try readlines() first, then read().
+        g = GridOut(self.db.fs, f._id)
+        self.assertEqual([b"Hello world,\n"], g.readlines(1))
+        self.assertEqual(b"H", g.read(1))
+        self.assertEqual([b"ow are you?\n", b"Hope all is well.\n"], g.readlines(29))
+        self.assertEqual([b"Bye"], g.readlines(1))
+
+        # Try readlines() first, then readline().
+        g = GridOut(self.db.fs, f._id)
+        self.assertEqual([b"Hello world,\n"], g.readlines(1))
+        self.assertEqual(b"How are you?\n", g.readline())
+        self.assertEqual([b"Hope all is well.\n"], g.readlines(17))
+        self.assertEqual(b"Bye", g.readline())
+
     def test_iterator(self):
         f = GridIn(self.db.fs)
         f.close()
         g = GridOut(self.db.fs, f._id)
         self.assertEqual([], list(g))
 
         f = GridIn(self.db.fs)
-        f.write(b"hello world")
+        f.write(b"hello world\nhere are\nsome lines.")
         f.close()
         g = GridOut(self.db.fs, f._id)
-        self.assertEqual([b"hello world"], list(g))
-        self.assertEqual(b"hello", g.read(5))
-        self.assertEqual([b"hello world"], list(g))
-        self.assertEqual(b" worl", g.read(5))
+        self.assertEqual([b"hello world\n", b"here are\n", b"some lines."], list(g))
+        self.assertEqual(b"", g.read(5))
+        self.assertEqual([], list(g))
+
+        g = GridOut(self.db.fs, f._id)
+        self.assertEqual(b"hello world\n", next(iter(g)))
+        self.assertEqual(b"here", g.read(4))
+        self.assertEqual(b" are\n", next(iter(g)))
+        self.assertEqual(b"some lines", g.read(10))
+        self.assertEqual(b".", next(iter(g)))
+        self.assertRaises(StopIteration, iter(g).__next__)
 
         f = GridIn(self.db.fs, chunk_size=2)
         f.write(b"hello world")
         f.close()
         g = GridOut(self.db.fs, f._id)
-        self.assertEqual([b"he", b"ll", b"o ",
-                          b"wo", b"rl", b"d"], list(g))
+        self.assertEqual([b"hello world"], list(g))
 
     def test_read_unaligned_buffer_size(self):
         in_data = (b"This is a text that doesn't "
@@ -665,6 +738,21 @@ def test_zip(self):
         self.assertSequenceEqual(z.namelist(), ["test.txt"])
         self.assertEqual(z.read("test.txt"), b"hello world")
 
+    def test_grid_out_unsupported_operations(self):
+        f = GridIn(self.db.fs, chunkSize=3)
+        f.write(b"hello world")
+        f.close()
+
+        g = GridOut(self.db.fs, f._id)
+
+        self.assertRaises(io.UnsupportedOperation, g.writelines, [b"some", b"lines"])
+        self.assertRaises(io.UnsupportedOperation, g.write, b"some text")
+        self.assertRaises(io.UnsupportedOperation, g.fileno)
+        self.assertRaises(io.UnsupportedOperation, g.truncate)
+
+        self.assertFalse(g.writable())
+        self.assertFalse(g.isatty())
+
 
 if __name__ == "__main__":
     unittest.main()