Handle GET on zero content length failure case.

Added a fake_file class that implements minimal set of functions that
are invoked by the code in GET.

BUG: 987841

https://bugzilla.redhat.com/show_bug.cgi?id=987841

Change-Id: I5bdf5be1c0c4c8231f34c9be529e6edc83774f2e
Signed-off-by: Chetan Risbud <crisbud@redhat.com>
Reviewed-on: http://review.gluster.org/5511
Reviewed-by: Luis Pabon <lpabon@redhat.com>
Tested-by: Luis Pabon <lpabon@redhat.com>

gluster/swift/common/fs_utils.py

@@ -24,6 +24,23 @@ from gluster.swift.common.exceptions import FileOrDirNotFoundError, \
     NotDirectoryError, GlusterFileSystemOSError, GlusterFileSystemIOError
 
 
+class Fake_file(object):
+    def __init__(self, path):
+        self.path = path
+
+    def tell(self):
+        return 0
+
+    def read(self, count):
+        return 0
+
+    def fileno(self):
+        return -1
+
+    def close(self):
+        pass
+
+
 def do_walk(*args, **kwargs):
     return os.walk(*args, **kwargs)
 
@@ -205,7 +222,7 @@ def do_open(path, flags, **kwargs):
 
 
 def do_close(fd):
-    if isinstance(fd, file):
+    if isinstance(fd, file) or isinstance(fd, Fake_file):
         try:
             fd.close()
         except IOError as err:

gluster/swift/obj/diskfile.py

@@ -35,7 +35,7 @@ from swift.common.exceptions import DiskFileNotExist, DiskFileError, \
 from gluster.swift.common.exceptions import GlusterFileSystemOSError
 from gluster.swift.common.Glusterfs import mount
 from gluster.swift.common.fs_utils import do_fstat, do_open, do_close, \
-    do_unlink, do_chown, os_path, do_fsync, do_fchown, do_stat
+    do_unlink, do_chown, os_path, do_fsync, do_fchown, do_stat, Fake_file
 from gluster.swift.common.utils import read_metadata, write_metadata, \
     validate_object, create_object_metadata, rmobjdir, dir_is_object, \
     get_object_metadata
@@ -530,13 +530,20 @@ class DiskFile(SwiftDiskFile):
 
         self._filter_metadata()
 
-        if not self._is_dir and keep_data_fp:
+        if keep_data_fp:
-            # The caller has an assumption that the "fp" field of this
+            if not self._is_dir:
-            # object is an file object if keep_data_fp is set. However,
+                # The caller has an assumption that the "fp" field of this
-            # this implementation of the DiskFile object does not need to
+                # object is an file object if keep_data_fp is set. However,
-            # open the file for internal operations. So if the caller
+                # this implementation of the DiskFile object does not need to
-            # requests it, we'll just open the file for them.
+                # open the file for internal operations. So if the caller
-            self.fp = do_open(data_file, 'rb')
+                # requests it, we'll just open the file for them.
+                self.fp = do_open(data_file, 'rb')
+            else:
+                self.fp = Fake_file(data_file)
+
+    def drop_cache(self, fd, offset, length):
+        if fd >= 0:
+            super(DiskFile, self).drop_cache(fd, offset, length)
 
     def close(self, verify_file=True):
         """
@@ -545,10 +552,6 @@ class DiskFile(SwiftDiskFile):
         :param verify_file: Defaults to True. If false, will not check
                             file to see if it needs quarantining.
         """
-        # Marker directory
-        if self._is_dir:
-            assert not self.fp
-            return
         if self.fp:
             do_close(self.fp)
             self.fp = None

test/unit/obj/test_diskfile.py

@@ -238,7 +238,6 @@ class TestDiskFile(unittest.TestCase):
             assert gdf._obj == "d"
             assert gdf.data_file == the_dir
             assert gdf._is_dir
-            assert gdf.fp is None
             assert gdf.metadata == exp_md
         finally:
             shutil.rmtree(td)
@@ -272,30 +271,61 @@ class TestDiskFile(unittest.TestCase):
                        iter_hook='hook')
         assert gdf.iter_hook == 'hook'
 
-    def test_close(self):
+    def test_close_no_open_fp(self):
         assert not os.path.exists("/tmp/foo")
-        gdf = DiskFile("/tmp/foo", "vol0", "p57", "ufo47", "bar", "z", self.lg)
+        gdf = DiskFile("/tmp/foo", "vol0", "p57", "ufo47", "bar",
-        # Should be a no-op, as by default is_dir is False, but fp is None
+                               "z", self.lg, keep_data_fp=True)
-        gdf.close()
-
-        gdf._is_dir = True
-        gdf.fp = "123"
-        # Should still be a no-op as is_dir is True (marker directory)
-        self.assertRaises(AssertionError, gdf.close)
-        assert gdf.fp == "123"
-
         gdf._is_dir = False
-        saved_dc = gluster.swift.obj.diskfile.do_close
         self.called = False
+
         def our_do_close(fp):
             self.called = True
-        gluster.swift.obj.diskfile.do_close = our_do_close
+
-        try:
+        with mock.patch("gluster.swift.obj.diskfile.do_close", our_do_close):
             gdf.close()
-            assert self.called
+            assert not self.called
             assert gdf.fp is None
+
+    def test_close_dir_object(self):
+        td = tempfile.mkdtemp()
+        the_cont = os.path.join(td, "vol0", "bar")
+        the_dir = "dir"
+        self.called = False
+        try:
+            os.makedirs(os.path.join(the_cont, "dir"))
+            gdf = DiskFile(td, "vol0", "p57", "ufo47", "bar",
+                                   "dir", self.lg, keep_data_fp=True)
+
+            def our_do_close(fp):
+                self.called = True
+
+            with mock.patch("gluster.swift.obj.diskfile.do_close",
+                    our_do_close):
+                gdf.close()
+                assert self.called
         finally:
-            gluster.swift.obj.diskfile.do_close = saved_dc
+            shutil.rmtree(td)
+
+    def test_close_file_object(self):
+        td = tempfile.mkdtemp()
+        the_cont = os.path.join(td, "vol0", "bar")
+        the_file = os.path.join(the_cont, "z")
+        self.called = False
+        try:
+            os.makedirs(the_cont)
+            with open(the_file, "wb") as fd:
+                fd.write("1234")
+            gdf = DiskFile(td, "vol0", "p57", "ufo47", "bar",
+                                   "z", self.lg, keep_data_fp=True)
+            def our_do_close(fp):
+                self.called = True
+
+            with mock.patch("gluster.swift.obj.diskfile.do_close",
+                    our_do_close):
+                gdf.close()
+                assert self.called
+        finally:
+            shutil.rmtree(td)
 
     def test_is_deleted(self):
         assert not os.path.exists("/tmp/foo")