fix metadata overwrite during a post request

During a post request, the object-server is ovewriting
the existing object metadata. This fix prevents the overwrite
of the system metadata while it allows for the user to
add/remove user metadata

Change-Id: Ic62cd064589b625ee425a9934be8766650622c13
Signed-off-by: Thiago da Silva <thiago@redhat.com>
Reviewed-on: http://review.gluster.org/6254
Reviewed-by: Luis Pabon <lpabon@redhat.com>
Tested-by: Luis Pabon <lpabon@redhat.com>
Signed-off-by: Thiago da Silva <thiago@redhat.com>
Reviewed-on: http://review.gluster.org/6315

3 files changed, 140 insertions, 43 deletions

diff --git a/gluster/swift/common/middleware/gswauth/swauth/middleware.py b/gluster/swift/common/middleware/gswauth/swauth/middleware.py
index 6388aa2..996228d 100644
--- a/gluster/swift/common/middleware/gswauth/swauth/middleware.py
+++ b/gluster/swift/common/middleware/gswauth/swauth/middleware.py
@@ -40,13 +40,9 @@ from swift.common.utils import cache_from_env, get_logger, get_remote_client, \
 import swift.common.wsgi
 
 
-from gluster.swift.common.middleware.gswauth.swauth import swift_version
 from gluster.swift.common.middleware.gswauth.swauth import authtypes
 
 
-MEMCACHE_TIME = swift_version.newer_than('1.7.7-dev')
-
-
 class Swauth(object):
     """
     Scalable authentication and authorization system that uses Swift as its
@@ -349,14 +345,9 @@ class Swauth(object):
                 expires_from_now = float(resp.getheader('x-auth-ttl'))
                 groups = resp.getheader('x-auth-groups')
                 if memcache_client:
-                    if MEMCACHE_TIME:
-                        memcache_client.set(
-                            memcache_key, (time() + expires_from_now, groups),
-                            time=expires_from_now)
-                    else:
-                        memcache_client.set(
-                            memcache_key, (time() + expires_from_now, groups),
-                            timeout=expires_from_now)
+                    memcache_client.set(
+                        memcache_key, (time() + expires_from_now, groups),
+                        timeout=expires_from_now)
             else:
                 path = quote('/v1/%s/.token_%s/%s' %
                              (self.auth_account, token[-1], token))
@@ -375,16 +366,10 @@ class Swauth(object):
                     groups.append(detail['account_id'])
                 groups = ','.join(groups)
                 if memcache_client:
-                    if MEMCACHE_TIME:
-                        memcache_client.set(
-                            memcache_key,
-                            (detail['expires'], groups),
-                            time=float(detail['expires'] - time()))
-                    else:
-                        memcache_client.set(
-                            memcache_key,
-                            (detail['expires'], groups),
-                            timeout=float(detail['expires'] - time()))
+                    memcache_client.set(
+                        memcache_key,
+                        (detail['expires'], groups),
+                        timeout=float(detail['expires'] - time()))
         return groups
 
     def authorize(self, req):
@@ -1369,18 +1354,11 @@ class Swauth(object):
             if not memcache_client:
                 raise Exception(
                     'No memcache set up; required for Swauth middleware')
-            if MEMCACHE_TIME:
-                memcache_client.set(
-                    memcache_key,
-                    (self.itoken_expires,
-                     '.auth,.reseller_admin,%s.auth' % self.reseller_prefix),
-                    time=self.token_life)
-            else:
-                memcache_client.set(
-                    memcache_key,
-                    (self.itoken_expires,
-                     '.auth,.reseller_admin,%s.auth' % self.reseller_prefix),
-                    timeout=self.token_life)
+            memcache_client.set(
+                memcache_key,
+                (self.itoken_expires,
+                 '.auth,.reseller_admin,%s.auth' % self.reseller_prefix),
+                timeout=self.token_life)
         return self.itoken
 
     def get_admin_detail(self, req):
diff --git a/gluster/swift/obj/diskfile.py b/gluster/swift/obj/diskfile.py
index 6e6856d..cccd5e6 100644
--- a/gluster/swift/obj/diskfile.py
+++ b/gluster/swift/obj/diskfile.py
@@ -44,7 +44,8 @@ from gluster.swift.common.utils import read_metadata, write_metadata, \
     get_object_metadata
 from gluster.swift.common.utils import X_CONTENT_TYPE, \
     X_TIMESTAMP, X_TYPE, X_OBJECT_TYPE, FILE, OBJECT, DIR_TYPE, \
-    FILE_TYPE, DEFAULT_UID, DEFAULT_GID, DIR_NON_OBJECT, DIR_OBJECT
+    FILE_TYPE, DEFAULT_UID, DEFAULT_GID, DIR_NON_OBJECT, DIR_OBJECT, \
+    X_ETAG, X_CONTENT_LENGTH
 from ConfigParser import ConfigParser, NoSectionError, NoOptionError
 
 # FIXME: Hopefully we'll be able to move to Python 2.7+ where O_CLOEXEC will
@@ -260,7 +261,8 @@ 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[X_CONTENT_TYPE]
+    content_type = metadata.get(X_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?
@@ -983,12 +985,39 @@ class DiskFile(object):
         :raises DiskFileError: this implementation will raise the same
                             errors as the `create()` method.
         """
-        # FIXME: we need to validate system metadata is preserved
-        metadata = _adjust_metadata(metadata)
+        metadata = self._keep_sys_metadata(metadata)
         data_file = os.path.join(self._put_datadir, self._obj)
         self._threadpool.run_in_thread(
             write_metadata, data_file, metadata)
 
+    def _keep_sys_metadata(self, metadata):
+        """
+        Make sure system metadata is not lost when writing new user metadata
+
+        This method will read the existing metadata and check for system
+        metadata. If there are any, it should be appended to the metadata obj
+        the user is trying to write.
+        """
+        orig_metadata = self.read_metadata()
+
+        sys_keys = [X_CONTENT_TYPE, X_ETAG, 'name', X_CONTENT_LENGTH,
+                    X_OBJECT_TYPE, X_TYPE]
+
+        for key in sys_keys:
+            if key in orig_metadata:
+                metadata[key] = orig_metadata[key]
+
+        if X_OBJECT_TYPE not in orig_metadata:
+            if metadata[X_CONTENT_TYPE].lower() == DIR_TYPE:
+                metadata[X_OBJECT_TYPE] = DIR_OBJECT
+            else:
+                metadata[X_OBJECT_TYPE] = FILE
+
+        if X_TYPE not in orig_metadata:
+            metadata[X_TYPE] = OBJECT
+
+        return metadata
+
     def _unlinkold(self):
         if self._is_dir:
             # Marker, or object, directory.
diff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py
index 8270e05..340087f 100644
--- a/test/unit/obj/test_diskfile.py
+++ b/test/unit/obj/test_diskfile.py
@@ -425,11 +425,101 @@ class TestDiskFile(unittest.TestCase):
         gdf = self._get_diskfile("vol0", "p57", "ufo47", "bar", "z")
         md = {'Content-Type': 'application/octet-stream', 'a': 'b'}
         gdf.write_metadata(md.copy())
-        on_disk_md = _metadata[_mapit(the_dir)]
-        del on_disk_md['X-Type']
-        del on_disk_md['X-Object-Type']
-        assert on_disk_md == md, "on_disk_md = %r, md = %r" % (
-            on_disk_md, md)
+        self.assertIsNone(gdf._metadata)
+        fmd = _metadata[_mapit(the_dir)]
+        md.update({'X-Object-Type': 'file', 'X-Type': 'Object'})
+        self.assertTrue(fmd['a'], md['a'])
+        self.assertTrue(fmd['Content-Type'], md['Content-Type'])
+
+    def test_add_metadata_to_existing_file(self):
+        the_path = os.path.join(self.td, "vol0", "bar")
+        the_file = os.path.join(the_path, "z")
+        os.makedirs(the_path)
+        with open(the_file, "wb") as fd:
+            fd.write("1234")
+        ini_md = {
+            'X-Type': 'Object',
+            'X-Object-Type': 'file',
+            'Content-Length': 4,
+            'ETag': 'etag',
+            'X-Timestamp': 'ts',
+            'Content-Type': 'application/loctet-stream'}
+        _metadata[_mapit(the_file)] = ini_md
+        gdf = self._get_diskfile("vol0", "p57", "ufo47", "bar", "z")
+        md = {'Content-Type': 'application/octet-stream', 'a': 'b'}
+        gdf.write_metadata(md.copy())
+        self.assertTrue(_metadata[_mapit(the_file)]['a'], 'b')
+        newmd = {'X-Object-Meta-test':'1234'}
+        gdf.write_metadata(newmd.copy())
+        on_disk_md = _metadata[_mapit(the_file)]
+        self.assertTrue(on_disk_md['Content-Length'], 4)
+        self.assertTrue(on_disk_md['X-Object-Meta-test'], '1234')
+        self.assertTrue(on_disk_md['X-Type'], 'Object')
+        self.assertTrue(on_disk_md['X-Object-Type'], 'file')
+        self.assertTrue(on_disk_md['ETag'], 'etag')
+        self.assertFalse('a' in on_disk_md)
+
+    def test_add_md_to_existing_file_with_md_in_gdf(self):
+        the_path = os.path.join(self.td, "vol0", "bar")
+        the_file = os.path.join(the_path, "z")
+        os.makedirs(the_path)
+        with open(the_file, "wb") as fd:
+            fd.write("1234")
+        ini_md = {
+            'X-Type': 'Object',
+            'X-Object-Type': 'file',
+            'Content-Length': 4,
+            'name': 'z',
+            'ETag': 'etag',
+            'X-Timestamp': 'ts'}
+        _metadata[_mapit(the_file)] = ini_md
+        gdf = self._get_diskfile("vol0", "p57", "ufo47", "bar", "z")
+
+        # make sure gdf has the _metadata
+        gdf.open()
+        md = {'a': 'b'}
+        gdf.write_metadata(md.copy())
+        self.assertTrue(_metadata[_mapit(the_file)]['a'], 'b')
+        newmd = {'X-Object-Meta-test':'1234'}
+        gdf.write_metadata(newmd.copy())
+        on_disk_md = _metadata[_mapit(the_file)]
+        self.assertTrue(on_disk_md['Content-Length'], 4)
+        self.assertTrue(on_disk_md['X-Object-Meta-test'], '1234')
+        self.assertFalse('a' in on_disk_md)
+
+    def test_add_metadata_to_existing_dir(self):
+        the_cont = os.path.join(self.td, "vol0", "bar")
+        the_dir = os.path.join(the_cont, "dir")
+        os.makedirs(the_dir)
+        gdf = self._get_diskfile("vol0", "p57", "ufo47", "bar", "dir")
+        self.assertEquals(gdf._metadata, None)
+        init_md = {
+            'X-Type': 'Object',
+            'Content-Length': 0,
+            'ETag': 'etag',
+            'X-Timestamp': 'ts',
+            'X-Object-Meta-test':'test',
+            'Content-Type': 'application/directory'}
+        _metadata[_mapit(the_dir)] = init_md
+
+        md = {'X-Object-Meta-test':'test'}
+        gdf.write_metadata(md.copy())
+        self.assertEqual(_metadata[_mapit(the_dir)]['X-Object-Meta-test'],
+                'test')
+        self.assertEqual(_metadata[_mapit(the_dir)]['Content-Type'].lower(),
+                'application/directory')
+
+        # set new metadata
+        newmd = {'X-Object-Meta-test2':'1234'}
+        gdf.write_metadata(newmd.copy())
+        self.assertEqual(_metadata[_mapit(the_dir)]['Content-Type'].lower(),
+                'application/directory')
+        self.assertEqual(_metadata[_mapit(the_dir)]["X-Object-Meta-test2"],
+                '1234')
+        self.assertEqual(_metadata[_mapit(the_dir)]['X-Object-Type'],
+                DIR_OBJECT)
+        self.assertFalse('X-Object-Meta-test' in _metadata[_mapit(the_dir)])
+
 
     def test_write_metadata_w_meta_file(self):
         the_path = os.path.join(self.td, "vol0", "bar")