fix error where the ADLS directory cannot be deleted: prod bug (#420)

* fix error where the ADLS directory cannot be deleted: prod bug

* tabs to spaces

azure-storage-cpp-lite/include/blob/blob_client.h

@@ -106,6 +106,14 @@ namespace microsoft_azure { namespace storage {
         /// <returns>A <see cref="std::future" /> object that represents the current operation.</returns>
         AZURE_STORAGE_API std::future<storage_outcome<void>> upload_block_blob_from_stream(const std::string &container, const std::string &blob, std::istream &is, const std::vector<std::pair<std::string, std::string>> &metadata);
 
+        /// <summary>
+        /// Intitiates an asynchronous operation  to delete a directory blob.
+        /// </summary>
+        /// <param name="container">The container name.</param>
+        /// <param name="blob">The directory blob name.</param>
+        /// <returns>A <see cref="std::future" /> object that represents the current operation.</returns>
+        AZURE_STORAGE_API std::future<storage_outcome<void>> delete_blobdir(const std::string &container, const std::string &blob);
+
         /// <summary>
         /// Intitiates an asynchronous operation  to delete a blob.
         /// </summary>
@@ -366,6 +374,13 @@ namespace microsoft_azure { namespace storage {
         /// <param name="blob">The blob name.</param>
         virtual void delete_blob(const std::string &container, const std::string &blob) = 0;
 
+        /// <summary>
+        /// Deletes a directory blob.
+        /// </summary>
+        /// <param name="container">The container name.</param>
+        /// <param name="blob">The directory blob name.</param>
+        virtual void delete_blobdir(const std::string &container, const std::string &blob) = 0;
+
         /// <summary>
         /// Copy a blob to another.
         /// </summary>
@@ -543,6 +558,13 @@ namespace microsoft_azure { namespace storage {
         /// <param name="blob">The blob name.</param>
         void delete_blob(const std::string &container, const std::string &blob);
 
+        /// <summary>
+        /// Deletes a directory blob.
+        /// </summary>
+        /// <param name="container">The container name.</param>
+        /// <param name="blob">The directory blob name.</param>
+        void delete_blobdir(const std::string &container, const std::string &blob);
+
         /// <summary>
         /// Copy a blob to another.
         /// </summary>
@@ -834,6 +856,13 @@ namespace microsoft_azure { namespace storage {
         /// <param name="blob">The blob name.</param>
         void delete_blob(const std::string &container, const std::string &blob);
 
+        /// <summary>
+        /// Deletes a directory blob.
+        /// </summary>
+        /// <param name="container">The container name.</param>
+        /// <param name="blob">The directory blob name.</param>
+        void delete_blobdir(const std::string &container, const std::string &blob);
+
         /// <summary>
         /// Copy a blob to another.
         /// </summary>

azure-storage-cpp-lite/include/blob/delete_blobdir_request.h

@@ -0,0 +1,32 @@
+#pragma once
+
+#include "delete_blob_request_base.h"
+
+namespace microsoft_azure {
+    namespace storage {
+
+        class delete_blobdir_request final : public delete_blob_request_base {
+        public:
+            delete_blobdir_request(const std::string &container, const std::string &blob)
+                : m_container(container),
+                m_blob(blob) {}
+
+            std::string container() const override {
+                return m_container;
+            }
+
+            std::string blob() const override {
+                return m_blob;
+            }
+
+            delete_snapshots ms_delete_snapshots() const override {
+                return delete_snapshots::unspecified;
+            }
+
+        private:
+            std::string m_container;
+            std::string m_blob;
+        };
+
+    }
+}

azure-storage-cpp-lite/src/blob/blob_client.cpp

@@ -5,6 +5,7 @@
 #include "blob/download_blob_request.h"
 #include "blob/create_block_blob_request.h"
 #include "blob/delete_blob_request.h"
+#include "blob/delete_blobdir_request.h"
 #include "blob/copy_blob_request.h"
 #include "blob/create_container_request.h"
 #include "blob/delete_container_request.h"
@@ -118,6 +119,14 @@ std::future<storage_outcome<void>> blob_client::delete_blob(const std::string &c
     return async_executor<void>::submit(m_account, request, http, m_context);
 }
 
+std::future<storage_outcome<void>> blob_client::delete_blobdir(const std::string &container, const std::string &blob) {
+    auto http = m_client->get_handle();
+
+    auto request = std::make_shared<delete_blobdir_request>(container, blob);
+
+    return async_executor<void>::submit(m_account, request, http, m_context);
+}
+
 std::future<storage_outcome<void>> blob_client::create_container(const std::string &container) {
     auto http = m_client->get_handle();
 
@@ -192,8 +201,8 @@ std::future<storage_outcome<list_containers_response>> blob_client::list_contain
 
     auto request = std::make_shared<list_containers_request>(prefix, include_metadata);
     request->set_maxresults(max_result);
-	
-	request->set_marker(continuation_token);
+    
+    request->set_marker(continuation_token);
 
     return async_executor<list_containers_response>::submit(m_account, request, http, m_context);
 }

azure-storage-cpp-lite/src/blob/blob_client_attr_cache_wrapper.cpp

@@ -336,6 +336,22 @@ namespace microsoft_azure {
             cache_item->m_confirmed = false;
         }
 
+        /// <summary>
+        /// Deletes a blob.
+        /// </summary>
+        /// <param name="container">The container name.</param>
+        /// <param name="blob">The blob name.</param>
+        void blob_client_attr_cache_wrapper::delete_blobdir(const std::string &container, const std::string &blob)
+        {
+            // These calls cannot be cached because we do not have a negative cache - blobs in the cache are either valid/confirmed, or unknown (which could be deleted, or not checked on the service.)
+            std::shared_ptr<boost::shared_mutex> dir_mutex = attr_cache.get_dir_item(get_parent_str(blob));
+            std::shared_ptr<blob_client_attr_cache_wrapper::blob_cache_item> cache_item = attr_cache.get_blob_item(blob);
+            boost::shared_lock<boost::shared_mutex> dirlock(*dir_mutex);
+            std::unique_lock<boost::shared_mutex> uniquelock(cache_item->m_mutex);
+            m_blob_client_wrapper->delete_blobdir(container, blob);
+            cache_item->m_confirmed = false;
+        }
+
         /// <summary>
         /// Copy a blob to another.
         /// </summary>

azure-storage-cpp-lite/src/blob/blob_client_wrapper.cpp

@@ -833,7 +833,7 @@ namespace microsoft_azure {
                 errno = client_not_init;
                 return;
             }
-            if(container.empty() || blob.empty())
+            if(container.empty() || blob.empty() )
             {
                 errno = invalid_parameters;
                 return;
@@ -862,6 +862,42 @@ namespace microsoft_azure {
             }
         }
 
+        void blob_client_wrapper::delete_blobdir(const std::string &container, const std::string &blob)
+        {
+            if(!is_valid())
+            {
+                errno = client_not_init;
+                return;
+            }
+            if(container.empty() || blob.empty() )
+            {
+                errno = invalid_parameters;
+                return;
+            }
+
+            try
+            {
+                auto task = m_blobClient->delete_blobdir(container, blob);
+                task.wait();
+                auto result = task.get();
+
+                if(!result.success())
+                {
+                    errno = std::stoi(result.error().code);
+                }
+                else
+                {
+                    errno = 0;
+                }
+            }
+            catch(std::exception& ex)
+            {
+                syslog(LOG_ERR, "Unknown failure in delete_blobdir.  ex.what() = %s, container = %s, blob = %s.", ex.what(), container.c_str(), blob.c_str());
+                errno = unknown_error;
+                return;
+            }
+        }
+
         void blob_client_wrapper::start_copy(const std::string &sourceContainer, const std::string &sourceBlob, const std::string &destContainer, const std::string &destBlob)
         {
 

blobfuse/blobfuse.cpp

@@ -43,23 +43,23 @@ float kernel_version = 0.0;
 void populate_kernel_version()
 {
     struct utsname buffer;
-	if (uname (&buffer) == 0) {
-		char *p = buffer.release;
-		int i = 0;
-		float ver[5];
+    if (uname (&buffer) == 0) {
+        char *p = buffer.release;
+        int i = 0;
+        float ver[5];
 
-		while (*p) {
-			if (isdigit(*p)) {
-				ver[i] = strtof(p, &p);
-				i++;
-			} else {
-				p++;
-			}
-			if (i >= 5) break;
-		}
-		if (i > 2)
+        while (*p) {
+            if (isdigit(*p)) {
+                ver[i] = strtof(p, &p);
+                i++;
+            } else {
+                p++;
+            }
+            if (i >= 5) break;
+        }
+        if (i > 2)
             kernel_version = ver[0];
-	}
+    }
 }
 
 #define OPTION(t, p) { t, offsetof(struct options, p), 1 }

blobfuse/directoryapis.cpp

@@ -168,7 +168,7 @@ int azs_readdir(const char *path, void *buf, fuse_fill_dir_t filler, off_t, stru
                         {
 
                             // Avoid duplicate directories - this avoids duplicate entries of legacy WASB and HNS directories
-                   	        local_list_results.push_back(prev_token_str);
+                               local_list_results.push_back(prev_token_str);
 
                             struct stat stbuf;
                             stbuf.st_mode = S_IFDIR | default_permission;
@@ -222,17 +222,25 @@ int azs_rmdir(const char *path)
         return -ENOTEMPTY;
     }
 
-    // TODO: change this to just delete blobs.
     errno = 0;
+    // delete the actual directory path if it exists
+    azure_blob_client_wrapper->delete_blobdir(str_options.containerName, pathString.substr(1));
+    int dir_blob_deletedir_errno = errno;
+    if (dir_blob_deletedir_errno == 0)
+    {
+        syslog(LOG_INFO, "Successfully deleted directory %s. ", path);
+    }
+    // now delete the directory marker
     azure_blob_client_wrapper->delete_blob(str_options.containerName, pathString.substr(1));
     int dir_blob_delete_errno = errno;
     if (dir_blob_delete_errno == 0)
     {
-        syslog(LOG_INFO, "Successfully deleted directory marker %s for path %s. ", pathString.c_str()+1, path);
+        syslog(LOG_INFO, "Successfully deleted zero-length directory marker %s for path %s. ", pathString.c_str()+1, path);
     }
-    else
+    // the below code is old and may not be needed any more.
+    if (dir_blob_deletedir_errno != 0 && dir_blob_delete_errno != 0)
     {
-        AZS_DEBUGLOGV("Failed to delete directory marker %s at path %s, errno = %d.  Checking the .directory version.\n", mntPath, pathString.c_str()+1, dir_blob_delete_errno);
+        AZS_DEBUGLOGV("Failed to delete directory marker %s at path %s and the directory, errno = %d.  Checking the .directory version.\n", mntPath, pathString.c_str()+1, dir_blob_delete_errno);
 
         pathString.append("/.directory");
 
@@ -244,12 +252,10 @@ int azs_rmdir(const char *path)
         {
             syslog(LOG_INFO, "Successfully deleted .directory-style directory marker for path %s to blob %s. ", path, pathString.c_str()+1);
         }
-        else
-        {
-            // If they both fail, dir_blob_delete_errno will be the important one in 99.99% of cases
-            syslog(LOG_ERR, "Failed I/O operation to delete directory %s.  errno = %d\n", path, dir_blob_delete_errno);
-            return 0 - map_errno(dir_blob_delete_errno);
-        }
+        // If they both fail, dir_blob_delete_errno will be the important one in 99.99% of cases
+        syslog(LOG_ERR, "Failed I/O operation to delete zero-length directory marker %s.  errno = %d\n", path, dir_blob_delete_errno);
+        syslog(LOG_ERR, "Failed I/O operation to delete directory %s.  errno = %d\n", path, dir_blob_deletedir_errno);
+        return 0 - map_errno(dir_blob_delete_errno);        
     }
 
     return 0;

test/attribcachesynchronizationtests.cpp

@@ -17,7 +17,8 @@ public:
     MOCK_METHOD5(download_blob_to_file, void(const std::string &container, const std::string &blob, const std::string &destPath, time_t &returned_last_modified, size_t parallel));
     MOCK_METHOD2(get_blob_property, blob_property(const std::string &container, const std::string &blob));
     MOCK_METHOD2(blob_exists, bool(const std::string &container, const std::string &blob));
-    MOCK_METHOD2(delete_blob, void(const std::string &container, const std::string &blob));
+    MOCK_METHOD2(delete_blob, void(const std::string &container, const std::string &blob));    
+    MOCK_METHOD2(delete_blobdir, void(const std::string &container, const std::string &blob));
     MOCK_METHOD4(start_copy, void(const std::string &sourceContainer, const std::string &sourceBlob, const std::string &destContainer, const std::string &destBlob));
 };
 

test/attribcachetests.cpp

@@ -19,6 +19,7 @@ public:
     MOCK_METHOD2(get_blob_property, blob_property(const std::string &container, const std::string &blob));
     MOCK_METHOD2(blob_exists, bool(const std::string &container, const std::string &blob));
     MOCK_METHOD2(delete_blob, void(const std::string &container, const std::string &blob));
+    MOCK_METHOD2(delete_blobdir, void(const std::string &container, const std::string &blob));
     MOCK_METHOD4(start_copy, void(const std::string &sourceContainer, const std::string &sourceBlob, const std::string &destContainer, const std::string &destBlob));
 };