From b46451d02d2660cdf46338b2e535467bf39e5164 Mon Sep 17 00:00:00 2001
From: Prashanth Pai <ppai@redhat.com>
Date: Mon, 13 Jun 2016 15:55:49 +0530
Subject: Expose glfs_readdirplus_r

This patch does the following:
* Implements Volume.listdir_with_stat() API which internally invokes
  glfs_readdirplus_r to return directory entries along with stat for
  each entry.
* Implements Volume.scandir() which is similar to os.scandir() present
  in Python 3.5
* Makes Dir class iterable.
* Enables Dir class to raise OSError when glfs_readdir* calls fail.
  Previously, these failures were silently being ignored and treated as
  a case of EOF.

Change-Id: Id918c39a7ef3882553e9bcd3fbf9455ee1c25a83
Signed-off-by: Prashanth Pai <ppai@redhat.com>
---
 doc/api-reference.rst                    |   5 +
 gluster/api.py                           |   6 +
 gluster/gfapi.py                         | 207 ++++++++++++++++++++++++++++---
 test/functional/libgfapi-python-tests.py |  35 +++++-
 test/unit/gluster/test_gfapi.py          |  94 +++++++++++++-
 5 files changed, 328 insertions(+), 19 deletions(-)

diff --git a/doc/api-reference.rst b/doc/api-reference.rst
index 98b0427..124096e 100644
--- a/doc/api-reference.rst
+++ b/doc/api-reference.rst
@@ -11,6 +11,11 @@ Volume Class
 
     .. automethod:: gluster.gfapi.Volume.__init__
 
+.. autoclass:: gluster.gfapi.DirEntry
+    :members:
+    :undoc-members:
+    :noindex:
+
 File Class
 ----------
 
diff --git a/gluster/api.py b/gluster/api.py
index e2e2966..c440de6 100755
--- a/gluster/api.py
+++ b/gluster/api.py
@@ -419,6 +419,12 @@ glfs_readdir_r = gfapi_prototype('glfs_readdir_r', ctypes.c_int,
                                  ctypes.POINTER(Dirent),
                                  ctypes.POINTER(ctypes.POINTER(Dirent)))
 
+glfs_readdirplus_r = gfapi_prototype('glfs_readdirplus_r', ctypes.c_int,
+                                     ctypes.c_void_p,
+                                     ctypes.POINTER(Stat),
+                                     ctypes.POINTER(Dirent),
+                                     ctypes.POINTER(ctypes.POINTER(Dirent)))
+
 glfs_closedir = gfapi_prototype('glfs_closedir', ctypes.c_int,
                                 ctypes.c_void_p)
 
diff --git a/gluster/gfapi.py b/gluster/gfapi.py
index 1765791..b346d8d 100755
--- a/gluster/gfapi.py
+++ b/gluster/gfapi.py
@@ -14,6 +14,8 @@ import math
 import time
 import stat
 import errno
+from collections import Iterator
+
 from gluster import api
 from gluster.exceptions import LibgfapiException, Error
 from gluster.utils import validate_mount, validate_glfd
@@ -471,13 +473,14 @@ class File(object):
             raise OSError(err, os.strerror(err))
 
 
-class Dir(object):
+class Dir(Iterator):
 
-    def __init__(self, fd):
+    def __init__(self, fd, readdirplus=False):
         # Add a reference so the module-level variable "api" doesn't
         # get yanked out from under us (see comment above File def'n).
         self._api = api
         self.fd = fd
+        self.readdirplus = readdirplus
         self.cursor = ctypes.POINTER(api.Dirent)()
 
     def __del__(self):
@@ -487,13 +490,136 @@ class Dir(object):
     def next(self):
         entry = api.Dirent()
         entry.d_reclen = 256
-        rc = api.glfs_readdir_r(self.fd, ctypes.byref(entry),
-                                ctypes.byref(self.cursor))
 
-        if (rc < 0) or (not self.cursor) or (not self.cursor.contents):
-            return rc
+        if self.readdirplus:
+            stat_info = api.Stat()
+            ret = api.glfs_readdirplus_r(self.fd, ctypes.byref(stat_info),
+                                         ctypes.byref(entry),
+                                         ctypes.byref(self.cursor))
+        else:
+            ret = api.glfs_readdir_r(self.fd, ctypes.byref(entry),
+                                     ctypes.byref(self.cursor))
+
+        if ret != 0:
+            err = ctypes.get_errno()
+            raise OSError(err, os.strerror(err))
+
+        if (not self.cursor) or (not self.cursor.contents):
+            # Reached end of directory stream
+            raise StopIteration
+
+        if self.readdirplus:
+            return (entry, stat_info)
+        else:
+            return entry
+
+
+class DirEntry(object):
+    """
+    Object yielded by scandir() to expose the file path and other file
+    attributes of a directory entry. scandir() will provide stat information
+    without making additional calls. DirEntry instances are not intended to be
+    stored in long-lived data structures; if you know the file metadata has
+    changed or if a long time has elapsed since calling scandir(), call
+    Volume.stat(entry.path) to fetch up-to-date information.
+
+    DirEntry() instances from Python 3.5 have follow_symlinks set to True by
+    default. In this implementation, follow_symlinks is set to False by
+    default as it incurs an additional stat call over network.
+    """
+
+    __slots__ = ('_name', '_vol', '_lstat', '_stat', '_path')
+
+    def __init__(self, vol, scandir_path, name, lstat):
+        self._name = name
+        self._vol = vol
+        self._lstat = lstat
+        self._stat = None
+        self._path = os.path.join(scandir_path, name)
+
+    @property
+    def name(self):
+        """
+        The entry's base filename, relative to the scandir() path argument.
+        """
+        return self._name
+
+    @property
+    def path(self):
+        """
+        The entry's full path name: equivalent to os.path.join(scandir_path,
+        entry.name) where scandir_path is the scandir() path argument. The path
+        is only absolute if the scandir() path argument was absolute.
+        """
+        return self._path
+
+    def stat(self, follow_symlinks=False):
+        """
+        Returns information equivalent of a lstat() system call on the entry.
+        This does not follow symlinks.
+        """
+        if follow_symlinks:
+            if self._stat is None:
+                if self.is_symlink():
+                    self._stat = self._vol.stat(self.path)
+                else:
+                    self._stat = self._lstat
+            return self._stat
+        else:
+            return self._lstat
+
+    def is_dir(self, follow_symlinks=False):
+        """
+        Return True if this entry is a directory; return False if the entry is
+        any other kind of file, or if it doesn't exist anymore.
+        """
+        if follow_symlinks and self.is_symlink():
+            try:
+                st = self.stat(follow_symlinks=follow_symlinks)
+            except OSError as err:
+                if err.errno != errno.ENOENT:
+                    raise
+                return False
+            else:
+                return stat.S_ISDIR(st.st_mode)
+        else:
+            return stat.S_ISDIR(self._lstat.st_mode)
+
+    def is_file(self, follow_symlinks=False):
+        """
+        Return True if this entry is a file; return False if the entry is a
+        directory or other non-file entry, or if it doesn't exist anymore.
+        """
+        if follow_symlinks and self.is_symlink():
+            try:
+                st = self.stat(follow_symlinks=follow_symlinks)
+            except OSError as err:
+                if err.errno != errno.ENOENT:
+                    raise
+                return False
+            else:
+                return stat.S_ISREG(st.st_mode)
+        else:
+            return stat.S_ISREG(self._lstat.st_mode)
 
-        return entry
+    def is_symlink(self):
+        """
+        Return True if this entry is a symbolic link (even if broken); return
+        False if the entry points to a directory or any kind of file, or if it
+        doesn't exist anymore.
+        """
+        return stat.S_ISLNK(self._lstat.st_mode)
+
+    def inode(self):
+        """
+        Return the inode number of the entry.
+        """
+        return self._lstat.st_ino
+
+    def __str__(self):
+        return '<{0}: {1!r}>'.format(self.__class__.__name__, self.name)
+
+    __repr__ = __str__
 
 
 class Volume(object):
@@ -822,18 +948,69 @@ class Volume(object):
         given by path. The list is in arbitrary order. It does not include
         the special entries '.' and '..' even if they are present in the
         directory.
+
+        :param path: Path to directory
+        :raises: OSError on failure
+        :returns: List of names of directory entries
         """
         dir_list = []
-        d = self.opendir(path)
-        while True:
-            ent = d.next()
-            if not isinstance(ent, api.Dirent):
+        for entry in self.opendir(path):
+            if not isinstance(entry, api.Dirent):
                 break
-            name = ent.d_name[:ent.d_reclen]
+            name = entry.d_name[:entry.d_reclen]
             if name not in (".", ".."):
                 dir_list.append(name)
         return dir_list
 
+    def listdir_with_stat(self, path):
+        """
+        Return a list containing the name and stat of the entries in the
+        directory given by path. The list is in arbitrary order. It does
+        not include the special entries '.' and '..' even if they are present
+        in the directory.
+
+        :param path: Path to directory
+        :raises: OSError on failure
+        :returns: A list of tuple. The tuple is of the form (name, stat) where
+                  name is a string indicating name of the directory entry and
+                  stat contains stat info of the entry.
+        """
+        # List of tuple. Each tuple is of the form (name, stat)
+        entries_with_stat = []
+        for (entry, stat_info) in self.opendir(path, readdirplus=True):
+            if not (isinstance(entry, api.Dirent) and
+                    isinstance(stat_info, api.Stat)):
+                break
+            name = entry.d_name[:entry.d_reclen]
+            if name not in (".", ".."):
+                entries_with_stat.append((name, stat_info))
+        return entries_with_stat
+
+    def scandir(self, path):
+        """
+        Return an iterator of :class:`DirEntry` objects corresponding to the
+        entries in the directory given by path. The entries are yielded in
+        arbitrary order, and the special entries '.' and '..' are not
+        included.
+
+        Using scandir() instead of listdir() can significantly increase the
+        performance of code that also needs file type or file attribute
+        information, because :class:`DirEntry` objects expose this
+        information.
+
+        scandir() provides same functionality as listdir_with_stat() except
+        that scandir() does not return a list and is an iterator. Hence scandir
+        is less memory intensive on large directories.
+
+        :param path: Path to directory
+        :raises: OSError on failure
+        :yields: Instance of :class:`DirEntry` class.
+        """
+        for (entry, lstat) in self.opendir(path, readdirplus=True):
+            name = entry.d_name[:entry.d_reclen]
+            if name not in (".", ".."):
+                yield DirEntry(self, path, name, lstat)
+
     @validate_mount
     def listxattr(self, path, size=0):
         """
@@ -1000,11 +1177,13 @@ class Volume(object):
         return fd
 
     @validate_mount
-    def opendir(self, path):
+    def opendir(self, path, readdirplus=False):
         """
         Open a directory.
 
         :param path: Path to the directory
+        :param readdirplus: Enable readdirplus which will also fetch stat
+                            information for each entry of directory.
         :returns: Returns a instance of Dir class
         :raises: OSError on failure
         """
@@ -1012,7 +1191,7 @@ class Volume(object):
         if not fd:
             err = ctypes.get_errno()
             raise OSError(err, os.strerror(err))
-        return Dir(fd)
+        return Dir(fd, readdirplus)
 
     @validate_mount
     def readlink(self, path):
diff --git a/test/functional/libgfapi-python-tests.py b/test/functional/libgfapi-python-tests.py
index 7dab995..1e6db3e 100644
--- a/test/functional/libgfapi-python-tests.py
+++ b/test/functional/libgfapi-python-tests.py
@@ -10,17 +10,19 @@
 
 import unittest
 import os
+import stat
 import types
 import errno
 import hashlib
 import threading
-
-from gluster.gfapi import File, Volume
-from gluster.exceptions import LibgfapiException, Error
 from test import get_test_config
 from ConfigParser import NoSectionError, NoOptionError
 from uuid import uuid4
 
+from gluster.api import Stat
+from gluster.gfapi import File, Volume, DirEntry
+from gluster.exceptions import LibgfapiException, Error
+
 config = get_test_config()
 if config:
     try:
@@ -845,6 +847,33 @@ class DirOpsTest(unittest.TestCase):
         dir_list.sort()
         self.assertEqual(dir_list, ["testfile0", "testfile1", "testfile2"])
 
+    def test_listdir_with_stat(self):
+        dir_list = self.vol.listdir_with_stat(self.dir_path)
+        dir_list_sorted = sorted(dir_list, key=lambda tup: tup[0])
+        for index, (name, stat_info) in enumerate(dir_list_sorted):
+            self.assertEqual(name, 'testfile%s' % (index))
+            self.assertTrue(isinstance(stat_info, Stat))
+            self.assertTrue(stat.S_ISREG(stat_info.st_mode))
+            self.assertEqual(stat_info.st_size, len(self.data))
+
+        # Error - path does not exist
+        self.assertRaises(OSError,
+                          self.vol.listdir_with_stat, 'non-existent-dir')
+
+    def test_scandir(self):
+        entries = []
+        for entry in self.vol.scandir(self.dir_path):
+            self.assertTrue(isinstance(entry, DirEntry))
+            entries.append(entry)
+
+        entries_sorted = sorted(entries, key=lambda e: e.name)
+        for index, entry in enumerate(entries_sorted):
+            self.assertEqual(entry.name, 'testfile%s' % (index))
+            self.assertTrue(entry.is_file())
+            self.assertFalse(entry.is_dir())
+            self.assertTrue(isinstance(entry.stat(), Stat))
+            self.assertEqual(entry.stat().st_size, len(self.data))
+
     def test_makedirs(self):
         name = self.dir_path + "/subd1/subd2/subd3"
         self.vol.makedirs(name, 0755)
diff --git a/test/unit/gluster/test_gfapi.py b/test/unit/gluster/test_gfapi.py
index a859c2f..d4a9b52 100644
--- a/test/unit/gluster/test_gfapi.py
+++ b/test/unit/gluster/test_gfapi.py
@@ -17,7 +17,7 @@ import time
 import math
 import errno
 
-from gluster.gfapi import File, Dir, Volume
+from gluster.gfapi import File, Dir, Volume, DirEntry
 from gluster import api
 from gluster.exceptions import LibgfapiException
 from nose import SkipTest
@@ -622,7 +622,7 @@ class TestVolume(unittest.TestCase):
         dirent3.d_name = "."
         dirent3.d_reclen = 1
         mock_Dir_next = Mock()
-        mock_Dir_next.side_effect = [dirent1, dirent2, dirent3, None]
+        mock_Dir_next.side_effect = [dirent1, dirent2, dirent3, StopIteration]
 
         with nested(patch("gluster.gfapi.api.glfs_opendir",
                           mock_glfs_opendir),
@@ -638,6 +638,96 @@ class TestVolume(unittest.TestCase):
         with patch("gluster.gfapi.api.glfs_opendir", mock_glfs_opendir):
             self.assertRaises(OSError, self.vol.listdir, "test.txt")
 
+    def test_listdir_with_stat_success(self):
+        mock_glfs_opendir = Mock()
+        mock_glfs_opendir.return_value = 2
+
+        dirent1 = api.Dirent()
+        dirent1.d_name = "mockfile"
+        dirent1.d_reclen = 8
+        stat1 = api.Stat()
+        stat1.st_nlink = 1
+        dirent2 = api.Dirent()
+        dirent2.d_name = "mockdir"
+        dirent2.d_reclen = 7
+        stat2 = api.Stat()
+        stat2.st_nlink = 2
+        dirent3 = api.Dirent()
+        dirent3.d_name = "."
+        dirent3.d_reclen = 1
+        stat3 = api.Stat()
+        stat3.n_link = 2
+        mock_Dir_next = Mock()
+        mock_Dir_next.side_effect = [(dirent1, stat1),
+                                     (dirent2, stat2),
+                                     (dirent3, stat3),
+                                     StopIteration]
+
+        with nested(patch("gluster.gfapi.api.glfs_opendir",
+                          mock_glfs_opendir),
+                    patch("gluster.gfapi.Dir.next", mock_Dir_next)):
+            d = self.vol.listdir_with_stat("testdir")
+            self.assertEqual(len(d), 2)
+            self.assertEqual(d[0][0], 'mockfile')
+            self.assertEqual(d[0][1].st_nlink, 1)
+            self.assertEqual(d[1][0], 'mockdir')
+            self.assertEqual(d[1][1].st_nlink, 2)
+
+    def test_listdir_with_stat_fail_exception(self):
+        mock_glfs_opendir = Mock()
+        mock_glfs_opendir.return_value = None
+        with patch("gluster.gfapi.api.glfs_opendir", mock_glfs_opendir):
+            self.assertRaises(OSError, self.vol.listdir_with_stat, "dir")
+
+    def test_scandir_success(self):
+        mock_glfs_opendir = Mock()
+        mock_glfs_opendir.return_value = 2
+
+        dirent1 = api.Dirent()
+        dirent1.d_name = "mockfile"
+        dirent1.d_reclen = 8
+        stat1 = api.Stat()
+        stat1.st_nlink = 1
+        stat1.st_mode = 33188
+        dirent2 = api.Dirent()
+        dirent2.d_name = "mockdir"
+        dirent2.d_reclen = 7
+        stat2 = api.Stat()
+        stat2.st_nlink = 2
+        stat2.st_mode = 16877
+        dirent3 = api.Dirent()
+        dirent3.d_name = "."
+        dirent3.d_reclen = 1
+        stat3 = api.Stat()
+        stat3.n_link = 2
+        stat3.st_mode = 16877
+        mock_Dir_next = Mock()
+        mock_Dir_next.side_effect = [(dirent1, stat1),
+                                     (dirent2, stat2),
+                                     (dirent3, stat3),
+                                     StopIteration]
+
+        with nested(patch("gluster.gfapi.api.glfs_opendir",
+                          mock_glfs_opendir),
+                    patch("gluster.gfapi.Dir.next", mock_Dir_next)):
+            i = 0
+            for entry in self.vol.scandir("testdir"):
+                self.assertTrue(isinstance(entry, DirEntry))
+                if entry.name == 'mockfile':
+                    self.assertEqual(entry.path, 'testdir/mockfile')
+                    self.assertTrue(entry.is_file())
+                    self.assertFalse(entry.is_dir())
+                    self.assertEqual(entry.stat().st_nlink, 1)
+                elif entry.name == 'mockdir':
+                    self.assertEqual(entry.path, 'testdir/mockdir')
+                    self.assertTrue(entry.is_dir())
+                    self.assertFalse(entry.is_file())
+                    self.assertEqual(entry.stat().st_nlink, 2)
+                else:
+                    self.fail("Unexpected entry")
+                i = i + 1
+            self.assertEqual(i, 2)
+
     def test_listxattr_success(self):
         def mock_glfs_listxattr(fs, path, buf, buflen):
             if buf:
-- 
cgit