From 8737f9e92a5c9dbf0d75e0702c7ab48f287f2761 Mon Sep 17 00:00:00 2001 From: Chetan Risbud Date: Thu, 1 Aug 2013 19:36:31 +0530 Subject: 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 Reviewed-on: http://review.gluster.org/5511 Reviewed-by: Luis Pabon Tested-by: Luis Pabon --- gluster/swift/common/fs_utils.py | 19 +++++++++++- gluster/swift/obj/diskfile.py | 27 +++++++++-------- test/unit/obj/test_diskfile.py | 64 +++++++++++++++++++++++++++++----------- 3 files changed, 80 insertions(+), 30 deletions(-) diff --git a/gluster/swift/common/fs_utils.py b/gluster/swift/common/fs_utils.py index b2935d0..afc0cfe 100644 --- a/gluster/swift/common/fs_utils.py +++ b/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: diff --git a/gluster/swift/obj/diskfile.py b/gluster/swift/obj/diskfile.py index 4d50488..26852b1 100644 --- a/gluster/swift/obj/diskfile.py +++ b/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: - # The caller has an assumption that the "fp" field of this - # object is an file object if keep_data_fp is set. However, - # this implementation of the DiskFile object does not need to - # open the file for internal operations. So if the caller - # requests it, we'll just open the file for them. - self.fp = do_open(data_file, 'rb') + if keep_data_fp: + if not self._is_dir: + # The caller has an assumption that the "fp" field of this + # object is an file object if keep_data_fp is set. However, + # this implementation of the DiskFile object does not need to + # open the file for internal operations. So if the caller + # 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 diff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py index 819abe7..2eef822 100644 --- a/test/unit/obj/test_diskfile.py +++ b/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) - # Should be a no-op, as by default is_dir is False, but fp is None - 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 = DiskFile("/tmp/foo", "vol0", "p57", "ufo47", "bar", + "z", self.lg, keep_data_fp=True) 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: + 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: - gluster.swift.obj.diskfile.do_close = saved_dc + shutil.rmtree(td) def test_is_deleted(self): assert not os.path.exists("/tmp/foo") -- cgit