PUT of a Directory failed

PUT of a directory fails with gluster-swift when there exists
object of a same name. This is because the objects and directories are placed on
the glusterfs. And hence the filesystem semantics are applicable there.

Exceptions raised in such situation are needed to be handled and reported
correctly back to proxy-server with HTTPConflict as a error code. Although
swift still continues to choose 503 as the best respose in such cases. No
tracebacks reported.

Fix covers the case when there exists a directory and object of the same
name is PUT.

Code changes fixes both the failure cases mentioned in the bug.

Examples of failing PUT requests:
1) curl -v  -X PUT http://127.0.0.1:8080/v1/AUTH_test/c1/dir1/obj2/anotherobject
-d'asdasdsadA'
-- obj2 was already an object.

2)curl -v -H 'Content-Length: 0' -H 'Content-Type: application/directory' -X PUT
http://127.0.0.1:8080/v1/AUTH_test/c1/obj1
-- obj1 was already and object

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

Change-Id: Id3042d920e3f99e740d4042ef5907ac8c59e04db
Signed-off-by: Chetan Risbud <crisbud@redhat.com>
Reviewed-on: http://review.gluster.org/7181
Reviewed-by: Prashanth Pai <ppai@redhat.com>
Tested-by: Prashanth Pai <ppai@redhat.com>

3 files changed, 45 insertions, 15 deletions

diff --git a/gluster/swift/obj/diskfile.py b/gluster/swift/obj/diskfile.py
index 3e4c5af..852f69f 100644
--- a/gluster/swift/obj/diskfile.py
+++ b/gluster/swift/obj/diskfile.py
@@ -29,6 +29,8 @@ from hashlib import md5
 from eventlet import sleep
 from greenlet import getcurrent
 from contextlib import contextmanager
+from gluster.swift.common.exceptions import AlreadyExistsAsFile, \
+    AlreadyExistsAsDir
 from swift.common.utils import TRUE_VALUES, ThreadPool, config_true_value
 from swift.common.exceptions import DiskFileNotExist, DiskFileError, \
     DiskFileNoSpace, DiskFileDeviceUnavailable, DiskFileNotOpen
@@ -166,17 +168,19 @@ def _make_directory_unlocked(full_path, uid, gid, metadata=None):
                 if not is_dir:
                     # FIXME: Ideally we'd want to return an appropriate error
                     # message and code in the PUT Object REST API response.
-                    raise DiskFileError("_make_directory_unlocked: os.mkdir"
-                                        " failed on path %s because it already"
-                                        " exists but not as a directory" % (
-                                            full_path))
+                    raise AlreadyExistsAsFile("_make_directory_unlocked:"
+                                              " os.mkdir failed on path %s"
+                                              " because it already exists"
+                                              " but not as a directory"
+                                              % (full_path))
             return True, metadata
         elif err.errno == errno.ENOTDIR:
             # FIXME: Ideally we'd want to return an appropriate error
             # message and code in the PUT Object REST API response.
-            raise DiskFileError("_make_directory_unlocked: os.mkdir failed"
-                                " because some part of path %s is not in fact"
-                                " a directory" % (full_path))
+            raise AlreadyExistsAsFile("_make_directory_unlocked:"
+                                      " os.mkdir failed because some "
+                                      "part of path %s is not in fact"
+                                      " a directory" % (full_path))
         elif err.errno == errno.EIO:
             # Sometimes Fuse will return an EIO error when it does not know
             # how to handle an unexpected, but transient situation. It is
@@ -482,6 +486,8 @@ class DiskFileWriter(object):
         written to the temp file.
 
         :param metadata: dictionary of metadata to be written
+        :raises AlreadyExistsAsDir : If there exists a directory of the same
+                                     name
         """
         assert self._tmppath is not None
         metadata = _adjust_metadata(metadata)
@@ -497,9 +503,9 @@ class DiskFileWriter(object):
             # system, perhaps gratuitously created when another
             # object was created, or created externally to Swift
             # REST API servicing (UFO use case).
-            raise DiskFileError('DiskFile.put(): file creation failed since'
-                                ' the target, %s, already exists as a'
-                                ' directory' % df._data_file)
+            raise AlreadyExistsAsDir('DiskFile.put(): file creation failed'
+                                     ' since the target, %s, already exists'
+                                     ' as a directory' % df._data_file)
 
         df._threadpool.force_run_in_thread(self._finalize_put, metadata)
 
@@ -897,6 +903,8 @@ class DiskFile(object):
         :param size: optional initial size of file to explicitly allocate on
                      disk
         :raises DiskFileNoSpace: if a size is specified and allocation fails
+        :raises AlreadyExistsAsFile: if path or part of a path is not a \
+                                     directory
         """
         data_file = os.path.join(self._put_datadir, self._obj)
 
@@ -917,6 +925,12 @@ class DiskFile(object):
                     # Raise DiskFileNoSpace to be handled by upper layers when
                     # there is no space on disk OR when quota is exceeded
                     raise DiskFileNoSpace()
+                if gerr.errno == errno.ENOTDIR:
+                    raise AlreadyExistsAsFile('do_open(): failed on %s,'
+                                              '  path or part of a'
+                                              ' path is not a directory'
+                                              % (tmppath))
+
                 if gerr.errno not in (errno.ENOENT, errno.EEXIST, errno.EIO):
                     # FIXME: Other cases we should handle?
                     raise
diff --git a/gluster/swift/obj/server.py b/gluster/swift/obj/server.py
index 6417475..3cdd3c0 100644
--- a/gluster/swift/obj/server.py
+++ b/gluster/swift/obj/server.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2012-2013 Red Hat, Inc.
+# Copyright (c) 2012-2014 Red Hat, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -18,6 +18,11 @@
 # Simply importing this monkey patches the constraint handling to fit our
 # needs
 import gluster.swift.common.constraints    # noqa
+from swift.common.swob import HTTPConflict
+from swift.common.utils import public, timing_stats
+from gluster.swift.common.exceptions import AlreadyExistsAsFile, \
+    AlreadyExistsAsDir
+from swift.common.request_helpers import split_and_validate_path
 
 from swift.obj import server
 
@@ -80,6 +85,16 @@ class ObjectController(server.ObjectController):
         """
         return
 
+    @public
+    @timing_stats()
+    def PUT(self, request):
+        try:
+            return server.ObjectController.PUT(self, request)
+        except (AlreadyExistsAsFile, AlreadyExistsAsDir):
+            device = \
+                split_and_validate_path(request, 1, 5, True)
+            return HTTPConflict(drive=device, request=request)
+
 
 def app_factory(global_conf, **local_conf):
     """paste.deploy app factory for creating WSGI object server apps"""
diff --git a/test/unit/obj/test_diskfile.py b/test/unit/obj/test_diskfile.py
index a767261..64fa6e7 100644
--- a/test/unit/obj/test_diskfile.py
+++ b/test/unit/obj/test_diskfile.py
@@ -26,7 +26,8 @@ from eventlet import tpool
 from mock import Mock, patch
 from hashlib import md5
 from copy import deepcopy
-
+from gluster.swift.common.exceptions import AlreadyExistsAsDir, \
+    AlreadyExistsAsFile
 from swift.common.exceptions import DiskFileNotExist, DiskFileError, \
     DiskFileNoSpace, DiskFileNotOpen
 from swift.common.utils import ThreadPool
@@ -409,7 +410,7 @@ class TestDiskFile(unittest.TestCase):
         dc = gluster.swift.obj.diskfile.do_chown
         gluster.swift.obj.diskfile.do_chown = _mock_do_chown
         self.assertRaises(
-            DiskFileError, gdf._create_dir_object, the_dir)
+            AlreadyExistsAsFile, gdf._create_dir_object, the_dir)
         gluster.swift.obj.diskfile.do_chown = dc
         self.assertFalse(os.path.isdir(the_dir))
         self.assertFalse(_mapit(the_dir) in _metadata)
@@ -431,7 +432,7 @@ class TestDiskFile(unittest.TestCase):
         dc = gluster.swift.obj.diskfile.do_chown
         gluster.swift.obj.diskfile.do_chown = _mock_do_chown
         self.assertRaises(
-            DiskFileError, gdf._create_dir_object, the_dir)
+            AlreadyExistsAsFile, gdf._create_dir_object, the_dir)
         gluster.swift.obj.diskfile.do_chown = dc
         self.assertFalse(os.path.isdir(the_dir))
         self.assertFalse(_mapit(the_dir) in _metadata)
@@ -622,7 +623,7 @@ class TestDiskFile(unittest.TestCase):
                 # directory.
                 dw.write('12345\n')
                 dw.put(newmd)
-            except DiskFileError:
+            except AlreadyExistsAsDir:
                 pass
             else:
                 self.fail("Expected to encounter"