diff --git a/gluster/swift/obj/diskfile.py b/gluster/swift/obj/diskfile.py index cccd5e6..3e4c5af 100644 --- a/gluster/swift/obj/diskfile.py +++ b/gluster/swift/obj/diskfile.py @@ -367,6 +367,14 @@ class DiskFileWriter(object): do_fadvise64(self._fd, self._last_sync, diff) self._last_sync = self._upload_size + def close(self): + """ + Close the file descriptor + """ + if self._fd: + do_close(self._fd) + self._fd = None + def write(self, chunk): """ Write a chunk of data to disk. @@ -463,10 +471,9 @@ class DiskFileWriter(object): else: # Success! break - # Close here so the calling context does not have to perform this, - # which keeps all file system operations in the threadpool context. - do_close(self._fd) - self._fd = None + # Close here so the calling context does not have to perform this + # in a thread. + self.close() def put(self, metadata): """ @@ -966,14 +973,9 @@ class DiskFile(object): dw = DiskFileWriter(fd, tmppath, self) yield dw finally: - if dw is not None: - try: - if dw._fd: - do_close(dw._fd) - except OSError: - pass - if dw._tmppath: - do_unlink(dw._tmppath) + dw.close() + if dw._tmppath: + do_unlink(dw._tmppath) def write_metadata(self, metadata): """ diff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py index 340087f..ecec402 100644 --- a/test/unit/obj/test_diskfile.py +++ b/test/unit/obj/test_diskfile.py @@ -23,7 +23,7 @@ import tempfile import shutil import mock from eventlet import tpool -from mock import patch +from mock import Mock, patch from hashlib import md5 from copy import deepcopy @@ -35,7 +35,7 @@ from gluster.swift.common.exceptions import GlusterFileSystemOSError import gluster.swift.common.utils from gluster.swift.common.utils import normalize_timestamp import gluster.swift.obj.diskfile -from gluster.swift.obj.diskfile import DiskFile, OnDiskManager +from gluster.swift.obj.diskfile import DiskFileWriter, DiskFile, OnDiskManager from gluster.swift.common.utils import DEFAULT_UID, DEFAULT_GID, X_TYPE, \ X_OBJECT_TYPE, DIR_OBJECT @@ -94,6 +94,24 @@ class MockRenamerCalled(Exception): def _mock_renamer(a, b): raise MockRenamerCalled() +class TestDiskFileWriter(unittest.TestCase): + """ Tests for gluster.swift.obj.diskfile.DiskFileWriter """ + + def test_close(self): + dw = DiskFileWriter(100, 'a', None) + mock_close = Mock() + with patch("gluster.swift.obj.diskfile.do_close", mock_close): + # It should call do_close + self.assertEqual(100, dw._fd) + dw.close() + self.assertEqual(1, mock_close.call_count) + self.assertEqual(None, dw._fd) + + # It should not call do_close since it should + # have made fd equal to None + dw.close() + self.assertEqual(None, dw._fd) + self.assertEqual(1, mock_close.call_count) class TestDiskFile(unittest.TestCase): """ Tests for gluster.swift.obj.diskfile """ @@ -881,7 +899,7 @@ class TestDiskFile(unittest.TestCase): assert os.path.exists(saved_tmppath) dw.write("123") # Closing the fd prematurely should not raise any exceptions. - os.close(dw._fd) + dw.close() assert not os.path.exists(saved_tmppath) def test_create_err_on_unlink(self):