object-storage: apply upstream DiskFile refactor

Apply the upstream DiskFile refactoring in ahead of its use to easiliy apply
the temp file optimization.

Change-Id: I2708733eed3d87759c70eb3d9e6cd74ef91d0c7b
BUG: 876660
Signed-off-by: Peter Portante <peter.portante@redhat.com>
Reviewed-on: http://review.gluster.org/4288
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Mohammed Junaid <junaid@redhat.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

2 files changed, 81 insertions, 73 deletions

diff --git a/ufo/gluster/swift/common/DiskFile.py b/ufo/gluster/swift/common/DiskFile.py
index 26abbbef6ed..5698ff72ab4 100644
--- a/ufo/gluster/swift/common/DiskFile.py
+++ b/ufo/gluster/swift/common/DiskFile.py
@@ -41,6 +41,26 @@ class AlreadyExistsAsDir(Exception):
     pass
 
 
+def _adjust_metadata(metadata):
+    # Fix up the metadata to ensure it has a proper value for the
+    # Content-Type metadata, as well as an X_TYPE and X_OBJECT_TYPE
+    # metadata values.
+    content_type = metadata['Content-Type']
+    if not content_type:
+        # FIXME: How can this be that our caller supplied us with metadata
+        # that has a content type that evaluates to False?
+        #
+        # FIXME: If the file exists, we would already know it is a
+        # directory. So why are we assuming it is a file object?
+        metadata['Content-Type'] = FILE_TYPE
+        x_object_type = FILE
+    else:
+        x_object_type = MARKER_DIR if content_type.lower() == DIR_TYPE else FILE
+    metadata[X_TYPE] = OBJECT
+    metadata[X_OBJECT_TYPE] = x_object_type
+    return metadata
+
+
 class Gluster_DiskFile(DiskFile):
     """
     Manage object files on disk.
@@ -83,6 +103,7 @@ class Gluster_DiskFile(DiskFile):
         self._container_path = os.path.join(path, device, container)
         self._is_dir = False
         self.tmpdir = os.path.join(path, device, 'tmp')
+        self.tmppath = None
         self.logger = logger
         self.metadata = {}
         self.meta_file = None
@@ -143,7 +164,7 @@ class Gluster_DiskFile(DiskFile):
         """
         return not self.data_file
 
-    def create_dir_object(self, dir_path):
+    def _create_dir_object(self, dir_path):
         #TODO: if object already exists???
         if os.path.exists(dir_path) and not os.path.isdir(dir_path):
             self.logger.error("Deleting file %s", dir_path)
@@ -153,60 +174,42 @@ class Gluster_DiskFile(DiskFile):
         do_chown(dir_path, self.uid, self.gid)
         create_object_metadata(dir_path)
 
-    def put_metadata(self, metadata):
+    def put_metadata(self, metadata, tombstone=False):
+        """
+        Short hand for putting metadata to .meta and .ts files.
+
+        :param metadata: dictionary of metadata to be written
+        :param tombstone: whether or not we are writing a tombstone
+        """
+        if tombstone:
+            # We don't write tombstone files. So do nothing.
+            return
         assert self.data_file is not None, "put_metadata: no file to put metadata into"
+        metadata = _adjust_metadata(metadata)
         write_metadata(self.data_file, metadata)
         self.metadata = metadata
+        self.filter_metadata()
 
-    def put(self, fd, tmppath, metadata, extension=''):
+    def put(self, fd, metadata, extension='.data'):
         """
         Finalize writing the file on disk, and renames it from the temp file to
         the real location.  This should be called after the data has been
         written to the temp file.
 
-        :params fd: file descriptor of the temp file
-        :param tmppath: path to the temporary file being used
+        :param fd: file descriptor of the temp file
         :param metadata: dictionary of metadata to be written
-        :param extention: extension to be used when making the file
+        :param extension: extension to be used when making the file
         """
-        if extension == '.ts':
-            # TombStone marker (deleted)
-            return
-
-        # Fix up the metadata to ensure it has a proper value for the
-        # Content-Type metadata, as well as an X_TYPE and X_OBJECT_TYPE
-        # metadata values.
-
-        content_type = metadata['Content-Type']
-        if not content_type:
-            # FIXME: How can this be that our caller supplied us with metadata
-            # that has a content type that evaluates to False?
-            #
-            # FIXME: If the file exists, we would already know it is a
-            # directory. So why are we assuming it is a file object?
-            metadata['Content-Type'] = FILE_TYPE
-            x_object_type = FILE
-        else:
-            x_object_type = MARKER_DIR if content_type.lower() == DIR_TYPE else FILE
-        metadata[X_TYPE] = OBJECT
-        metadata[X_OBJECT_TYPE] = x_object_type
-
-        if extension == '.meta':
-            # Metadata recorded separately from the file, we just update the
-            # metadata for the file.
-            #
-            # FIXME: If the file does not exist, this call will fail.
-            self.put_metadata(metadata)
-            return
-
         # Our caller will use '.data' here; we just ignore it since we map the
         # URL directly to the file system.
         extension = ''
 
+        metadata = _adjust_metadata(metadata)
+
         if metadata[X_OBJECT_TYPE] == MARKER_DIR:
             if not self.data_file:
                 self.data_file = os.path.join(self.datadir, self._obj)
-                self.create_dir_object(self.data_file)
+                self._create_dir_object(self.data_file)
             self.put_metadata(metadata)
             return
 
@@ -218,7 +221,7 @@ class Gluster_DiskFile(DiskFile):
             raise AlreadyExistsAsDir(msg)
 
         timestamp = normalize_timestamp(metadata[X_TIMESTAMP])
-        write_metadata(tmppath, metadata)
+        write_metadata(self.tmppath, metadata)
         if X_CONTENT_LENGTH in metadata:
             self.drop_cache(fd, 0, int(metadata[X_CONTENT_LENGTH]))
         tpool.execute(os.fsync, fd)
@@ -228,13 +231,14 @@ class Gluster_DiskFile(DiskFile):
             tmp_path = self._container_path
             for dir_name in dir_objs:
                 tmp_path = os.path.join(tmp_path, dir_name)
-                self.create_dir_object(tmp_path)
+                self._create_dir_object(tmp_path)
 
         newpath = os.path.join(self.datadir, self._obj)
-        renamer(tmppath, newpath)
+        renamer(self.tmppath, newpath)
         do_chown(newpath, self.uid, self.gid)
         self.metadata = metadata
         self.data_file = newpath
+        self.filter_metadata()
         return
 
     def unlinkold(self, timestamp):
@@ -302,14 +306,15 @@ class Gluster_DiskFile(DiskFile):
 
         if not os.path.exists(self.tmpdir):
             mkdirs(self.tmpdir)
-        fd, tmppath = mkstemp(dir=self.tmpdir)
+        fd, self.tmppath = mkstemp(dir=self.tmpdir)
         try:
-            yield fd, tmppath
+            yield fd
         finally:
             try:
                 os.close(fd)
             except OSError:
                 pass
+            tmppath, self.tmppath = self.tmppath, None
             try:
                 os.unlink(tmppath)
             except OSError:
diff --git a/ufo/test/unit/common/test_diskfile.py b/ufo/test/unit/common/test_diskfile.py
index 264c656a5f1..7583fd7fa5d 100644
--- a/ufo/test/unit/common/test_diskfile.py
+++ b/ufo/test/unit/common/test_diskfile.py
@@ -315,7 +315,7 @@ class TestDiskFile(unittest.TestCase):
                                    "dir/z", self.lg)
             # Not created, dir object path is different, just checking
             assert gdf._obj == "z"
-            gdf.create_dir_object(the_dir)
+            gdf._create_dir_object(the_dir)
             assert os.path.isdir(the_dir)
             assert the_dir in _metadata
         finally:
@@ -339,7 +339,7 @@ class TestDiskFile(unittest.TestCase):
             dc = gluster.swift.common.DiskFile.do_chown
             gluster.swift.common.DiskFile.do_chown = _mock_do_chown
             try:
-                gdf.create_dir_object(the_dir)
+                gdf._create_dir_object(the_dir)
             finally:
                 gluster.swift.common.DiskFile.do_chown = dc
             assert os.path.isdir(the_dir)
@@ -355,9 +355,9 @@ class TestDiskFile(unittest.TestCase):
             os.makedirs(the_dir)
             gdf = Gluster_DiskFile(td, "vol0", "p57", "ufo47", "bar",
                                    "z", self.lg)
-            md = { 'a': 'b' }
-            gdf.put_metadata(md)
-            assert gdf.metadata == md
+            md = { 'Content-Type': 'application/octet-stream', 'a': 'b' }
+            gdf.put_metadata(md.copy())
+            assert gdf.metadata == md, "gdf.metadata = %r, md = %r" % (gdf.metadata, md)
             assert _metadata[the_dir] == md
         finally:
             shutil.rmtree(td)
@@ -368,7 +368,7 @@ class TestDiskFile(unittest.TestCase):
                                "z", self.lg)
         assert gdf.metadata == {}
 
-        gdf.put(None, '', {'x': '1'}, extension='.ts')
+        gdf.put_metadata({'x': '1'}, tombstone=True)
         assert gdf.metadata == {}
 
     def test_put_w_meta_file(self):
@@ -383,8 +383,7 @@ class TestDiskFile(unittest.TestCase):
                                    "z", self.lg)
             newmd = gdf.metadata.copy()
             newmd['X-Object-Meta-test'] = '1234'
-            with gdf.mkstemp() as (fd, tmppath):
-                gdf.put(fd, tmppath, newmd, extension='.meta')
+            gdf.put_metadata(newmd)
             assert gdf.metadata == newmd
             assert _metadata[the_file] == newmd
         finally:
@@ -403,8 +402,7 @@ class TestDiskFile(unittest.TestCase):
             newmd = gdf.metadata.copy()
             newmd['Content-Type'] = ''
             newmd['X-Object-Meta-test'] = '1234'
-            with gdf.mkstemp() as (fd, tmppath):
-                gdf.put(fd, tmppath, newmd, extension='.meta')
+            gdf.put_metadata(newmd)
             assert gdf.metadata == newmd
             assert _metadata[the_file] == newmd
         finally:
@@ -420,7 +418,7 @@ class TestDiskFile(unittest.TestCase):
                                    "dir", self.lg)
             newmd = gdf.metadata.copy()
             newmd['X-Object-Meta-test'] = '1234'
-            gdf.put(None, None, newmd, extension='.meta')
+            gdf.put_metadata(newmd)
             assert gdf.metadata == newmd
             assert _metadata[the_dir] == newmd
         finally:
@@ -436,7 +434,7 @@ class TestDiskFile(unittest.TestCase):
                                    "dir", self.lg)
             newmd = gdf.metadata.copy()
             newmd['X-Object-Meta-test'] = '1234'
-            gdf.put(None, None, newmd, extension='.data')
+            gdf.put_metadata(newmd)
             assert gdf.metadata == newmd
             assert _metadata[the_dir] == newmd
         finally:
@@ -455,7 +453,8 @@ class TestDiskFile(unittest.TestCase):
                 'ETag': 'etag',
                 'X-Timestamp': 'ts',
                 'Content-Type': 'application/directory'}
-            gdf.put(None, None, newmd)
+            gdf.put(None, newmd, extension='.dir')
+            assert gdf.data_file == the_dir
             assert gdf.metadata == newmd
             assert _metadata[the_dir] == newmd
         finally:
@@ -477,7 +476,7 @@ class TestDiskFile(unittest.TestCase):
             newmd['Content-Type'] = ''
             newmd['X-Object-Meta-test'] = '1234'
             try:
-                gdf.put(None, None, newmd, extension='.data')
+                gdf.put(None, newmd, extension='.data')
             except AlreadyExistsAsDir:
                 pass
             else:
@@ -509,9 +508,11 @@ class TestDiskFile(unittest.TestCase):
                 'Content-Length': '5',
                 }
 
-            with gdf.mkstemp() as (fd, tmppath):
+            with gdf.mkstemp() as fd:
+                assert gdf.tmppath is not None
+                tmppath = gdf.tmppath
                 os.write(fd, body)
-                gdf.put(fd, tmppath, metadata)
+                gdf.put(fd, metadata)
 
             assert gdf.data_file == os.path.join(td, "vol0", "bar", "z")
             assert os.path.exists(gdf.data_file)
@@ -543,9 +544,11 @@ class TestDiskFile(unittest.TestCase):
                 'Content-Length': '5',
                 }
 
-            with gdf.mkstemp() as (fd, tmppath):
+            with gdf.mkstemp() as fd:
+                assert gdf.tmppath is not None
+                tmppath = gdf.tmppath
                 os.write(fd, body)
-                gdf.put(fd, tmppath, metadata)
+                gdf.put(fd, metadata)
 
             assert gdf.data_file == os.path.join(td, "vol0", "bar", "b", "a", "z")
             assert os.path.exists(gdf.data_file)
@@ -844,12 +847,12 @@ class TestDiskFile(unittest.TestCase):
             gdf = Gluster_DiskFile(td, "vol0", "p57", "ufo47", "bar",
                                    "dir/z", self.lg)
             saved_tmppath = ''
-            with gdf.mkstemp() as (fd, tmppath):
+            with gdf.mkstemp() as fd:
                 assert gdf.tmpdir == os.path.join(td, "vol0", "tmp")
                 assert os.path.isdir(gdf.tmpdir)
-                assert os.path.dirname(tmppath) == gdf.tmpdir
-                assert os.path.exists(tmppath)
-                saved_tmppath = tmppath
+                saved_tmppath = gdf.tmppath
+                assert os.path.dirname(saved_tmppath) == gdf.tmpdir
+                assert os.path.exists(saved_tmppath)
                 os.write(fd, "123")
             assert not os.path.exists(saved_tmppath)
         finally:
@@ -863,12 +866,12 @@ class TestDiskFile(unittest.TestCase):
             gdf = Gluster_DiskFile(td, "vol0", "p57", "ufo47", "bar",
                                    "dir/z", self.lg)
             saved_tmppath = ''
-            with gdf.mkstemp() as (fd, tmppath):
+            with gdf.mkstemp() as fd:
                 assert gdf.tmpdir == os.path.join(td, "vol0", "tmp")
                 assert os.path.isdir(gdf.tmpdir)
-                assert os.path.dirname(tmppath) == gdf.tmpdir
-                assert os.path.exists(tmppath)
-                saved_tmppath = tmppath
+                saved_tmppath = gdf.tmppath
+                assert os.path.dirname(saved_tmppath) == gdf.tmpdir
+                assert os.path.exists(saved_tmppath)
                 os.write(fd, "123")
                 os.close(fd)
             assert not os.path.exists(saved_tmppath)
@@ -883,14 +886,14 @@ class TestDiskFile(unittest.TestCase):
             gdf = Gluster_DiskFile(td, "vol0", "p57", "ufo47", "bar",
                                    "dir/z", self.lg)
             saved_tmppath = ''
-            with gdf.mkstemp() as (fd, tmppath):
+            with gdf.mkstemp() as fd:
                 assert gdf.tmpdir == os.path.join(td, "vol0", "tmp")
                 assert os.path.isdir(gdf.tmpdir)
-                assert os.path.dirname(tmppath) == gdf.tmpdir
-                assert os.path.exists(tmppath)
-                saved_tmppath = tmppath
+                saved_tmppath = gdf.tmppath
+                assert os.path.dirname(saved_tmppath) == gdf.tmpdir
+                assert os.path.exists(saved_tmppath)
                 os.write(fd, "123")
-                os.unlink(tmppath)
+                os.unlink(saved_tmppath)
             assert not os.path.exists(saved_tmppath)
         finally:
             shutil.rmtree(td)