[datalake] Fix path rename operations (#653)

* fix apth rename operation and improve tests

* remove commented code

* pr comments

Author: roeap

sdk/storage_datalake/examples/file_rename.rs

@@ -43,8 +43,14 @@ async fn main() -> Result<(), Box<dyn Error + Send + Sync>> {
     );
 
     println!("renaming file '{}' to '{}'...", file_path1, file_path2);
-    let rename_file_response = file_client1.rename(file_path2).into_future().await?;
-    println!("rename file response == {:?}\n", rename_file_response);
+    let file_client3 = file_client1.rename(file_path2).into_future().await?;
+    let renamed_file_properties = file_client3.get_properties().into_future().await?;
+    println!("renamed file properties == {:?}\n", renamed_file_properties);
+
+    // getting properties for the source file should fail, when the file no longer exists
+    // Eventually we will implement the `exists` method, which internally employs a similar check
+    let source_file_properties_result = file_client1.get_properties().into_future().await;
+    assert!(source_file_properties_result.is_err());
 
     println!("deleting file system...");
     let delete_fs_response = file_system_client.delete().into_future().await?;

sdk/storage_datalake/src/clients/directory_client.rs

@@ -81,7 +81,6 @@ impl DirectoryClient {
             .if_match_condition(IfMatchCondition::NotMatch("*".to_string()))
     }
 
-    // TODO rename seems to not delete source
     pub fn rename<P>(&self, destination_path: P) -> RenamePathBuilder<Self>
     where
         P: Into<String>,
@@ -93,7 +92,6 @@ impl DirectoryClient {
         // the path will contain a leading '/' as we extract if from the path component of the url
         let dir_path = vec![fs_url.path(), &self.dir_path].join("/");
         RenamePathBuilder::new(destination_client, self.file_system_client.context.clone())
-            .resource(ResourceType::Directory)
             .mode(PathRenameMode::Legacy)
             .rename_source(dir_path)
     }

sdk/storage_datalake/src/clients/file_client.rs

@@ -89,7 +89,6 @@ impl FileClient {
         GetFileBuilder::new(self.clone(), self.file_system_client.context.clone())
     }
 
-    // TODO rename seems to not delete source
     pub fn rename<P>(&self, destination_path: P) -> RenamePathBuilder<Self>
     where
         P: Into<String>,
@@ -99,7 +98,6 @@ impl FileClient {
         // the path will contain a leading '/' as we extract if from the path component of the url
         let file_path = vec![fs_url.path(), &self.file_path].join("/");
         RenamePathBuilder::new(destination_client, self.file_system_client.context.clone())
-            .resource(ResourceType::File)
             .mode(PathRenameMode::Legacy)
             .rename_source(file_path)
     }

sdk/storage_datalake/src/operations/path_put.rs

@@ -29,7 +29,6 @@ where
     if_match_condition: Option<IfMatchCondition>,
     if_modified_since: Option<IfModifiedSinceCondition>,
     client_request_id: Option<ClientRequestId>,
-    rename_source: Option<RenameSource>,
     properties: Option<Properties>,
     timeout: Option<Timeout>,
     context: Context,
@@ -45,7 +44,6 @@ impl<C: PathClient + 'static> PutPathBuilder<C> {
             if_match_condition: None,
             if_modified_since: None,
             client_request_id: None,
-            rename_source: None,
             properties: None,
             timeout: None,
             context,
@@ -59,7 +57,6 @@ impl<C: PathClient + 'static> PutPathBuilder<C> {
         if_match_condition: IfMatchCondition => Some(if_match_condition),
         if_modified_since: IfModifiedSinceCondition => Some(if_modified_since),
         client_request_id: ClientRequestId => Some(client_request_id),
-        rename_source: RenameSource => Some(rename_source),
         properties: Properties => Some(properties),
         timeout: Timeout => Some(timeout),
         context: Context => context,
@@ -85,7 +82,6 @@ impl<C: PathClient + 'static> PutPathBuilder<C> {
             add_optional_header2(&this.properties, &mut request)?;
             add_optional_header2(&this.if_match_condition, &mut request)?;
             add_optional_header2(&this.if_modified_since, &mut request)?;
-            add_optional_header2(&this.rename_source, &mut request)?;
             add_mandatory_header2(&ContentLength::new(0), &mut request)?;
 
             let response = self
@@ -106,7 +102,6 @@ where
 {
     client: C,
     mode: Option<PathRenameMode>,
-    resource: Option<ResourceType>,
     continuation: Option<NextMarker>,
     if_match_condition: Option<IfMatchCondition>,
     if_modified_since: Option<IfModifiedSinceCondition>,
@@ -123,7 +118,6 @@ impl<C: PathClient + 'static> RenamePathBuilder<C> {
             client,
             mode: None,
             continuation: None,
-            resource: None,
             if_match_condition: None,
             if_modified_since: None,
             client_request_id: None,
@@ -136,7 +130,6 @@ impl<C: PathClient + 'static> RenamePathBuilder<C> {
 
     setters! {
         mode: PathRenameMode => Some(mode),
-        resource: ResourceType => Some(resource),
         continuation: NextMarker => Some(continuation),
         if_match_condition: IfMatchCondition => Some(if_match_condition),
         if_modified_since: IfModifiedSinceCondition => Some(if_modified_since),
@@ -157,7 +150,6 @@ impl<C: PathClient + 'static> RenamePathBuilder<C> {
             if let Some(continuation) = self.continuation {
                 continuation.append_to_url_query_as_continuation(&mut url);
             };
-            self.resource.append_to_url_query(&mut url);
             self.mode.append_to_url_query(&mut url);
             self.timeout.append_to_url_query(&mut url);
 

sdk/storage_datalake/src/properties.rs

@@ -76,6 +76,14 @@ impl TryFrom<&HeaderMap> for Properties {
     fn try_from(headers: &HeaderMap) -> Result<Self, Self::Error> {
         let mut properties = Self::new();
 
+        let header_value = headers
+            .get(HEADER)
+            .ok_or_else(|| crate::Error::HeaderNotFound(HEADER.to_owned()))?
+            .to_str()?;
+        if header_value.is_empty() {
+            return Ok(properties);
+        }
+
         // this is probably too complicated. Should we split
         // it in more manageable code blocks?
         // The logic is this:
@@ -86,10 +94,7 @@ impl TryFrom<&HeaderMap> for Properties {
         //      5. For each pair:
         //          6. Base64 decode the second entry (value). If error, return error.
         //          7. Insert the key value pair in the returned struct.
-        headers
-            .get(HEADER)
-            .ok_or_else(|| crate::Error::HeaderNotFound(HEADER.to_owned()))? // HEADER must exists or we return Err
-            .to_str()?
+        header_value
             .split(',') // The list is a CSV so we split by comma
             .map(|key_value_pair| {
                 let mut key_and_value = key_value_pair.split('='); // Each entry is key and value separated by =

sdk/storage_datalake/src/request_options.rs

@@ -1,6 +1,7 @@
 //! Request properties used in datalake rest api operations
 use azure_core::AddAsHeader;
 use azure_core::AppendToUrlQuery;
+use azure_storage::core::headers::RENAME_SOURCE;
 use http::request::Builder;
 
 #[derive(Debug, Clone)]
@@ -197,10 +198,10 @@ impl AddAsHeader for RenameSource {
         &self,
         request: &mut azure_core::Request,
     ) -> Result<(), azure_core::HttpHeaderError> {
-        request.headers_mut().append(
-            http::header::CONTENT_LANGUAGE,
-            http::HeaderValue::from_str(&self.0)?,
-        );
+        let encoded: String = url::form_urlencoded::byte_serialize(self.0.as_bytes()).collect();
+        request
+            .headers_mut()
+            .append(RENAME_SOURCE, http::HeaderValue::from_str(&encoded)?);
 
         Ok(())
     }

sdk/storage_datalake/tests/files.rs

@@ -1,4 +1,5 @@
 #![cfg(feature = "mock_transport_framework")]
+use azure_storage_datalake::Properties;
 use std::error::Error;
 
 mod setup;
@@ -138,16 +139,38 @@ async fn file_rename() -> Result<(), Box<dyn Error + Send + Sync>> {
     let file_path2 = "some/path/e2etest-file2.txt";
     let file_client2 = file_system_client.get_file_client(file_path2);
 
-    file_client1.create().into_future().await?;
+    let mut file_properties = Properties::new();
+    file_properties.insert("AddedVia", "Azure SDK for Rust");
+
+    file_client1
+        .create()
+        .properties(file_properties.clone())
+        .into_future()
+        .await?;
     file_client2.create().into_future().await?;
 
+    // original file properties
+    let original_target_file_properties = file_client2.get_properties().into_future().await?;
+
     let rename_file_if_not_exists_result = file_client1
         .rename_if_not_exists(file_path2)
         .into_future()
         .await;
     assert!(rename_file_if_not_exists_result.is_err());
 
-    file_client1.rename(file_path2).into_future().await?;
+    let file_client3 = file_client1.rename(file_path2).into_future().await?;
+    let renamed_file_properties = file_client3.get_properties().into_future().await?;
+
+    // when renaming a file, the source file properties should be propagated
+    assert_eq!(renamed_file_properties.properties, file_properties);
+    assert_ne!(
+        renamed_file_properties.properties,
+        original_target_file_properties.properties
+    );
+
+    // getting properties for the source file should fail, when the file no longer exists
+    let source_file_properties_result = file_client1.get_properties().into_future().await;
+    assert!(source_file_properties_result.is_err());
 
     file_system_client.delete().into_future().await?;
 

test/transactions/datalake_file_create_delete/0_request.json

@@ -5,7 +5,7 @@
     "authorization": "<<STRIPPED>>",
     "content-length": "0",
     "user-agent": "azsdk-rust-storage_datalake/0.1.0 (1.58.0; linux; x86_64)",
-    "x-ms-date": "Fri, 21 Jan 2022 19:04:17 GMT",
+    "x-ms-date": "Tue, 08 Feb 2022 19:19:33 GMT",
     "x-ms-version": "2019-12-12"
   },
   "body": ""

test/transactions/datalake_file_create_delete/0_response.json

@@ -2,12 +2,12 @@
   "status": 201,
   "headers": {
     "content-length": "0",
-    "date": "Fri, 21 Jan 2022 19:04:19 GMT",
-    "etag": "\"0x8D9DD10D460A632\"",
-    "last-modified": "Fri, 21 Jan 2022 19:04:20 GMT",
+    "date": "Tue, 08 Feb 2022 19:19:34 GMT",
+    "etag": "\"0x8D9EB37F12127AD\"",
+    "last-modified": "Tue, 08 Feb 2022 19:19:35 GMT",
     "server": "Windows-Azure-HDFS/1.0 Microsoft-HTTPAPI/2.0",
     "x-ms-namespace-enabled": "true",
-    "x-ms-request-id": "9c6cdfe8-801f-0007-34f9-0e3678000000",
+    "x-ms-request-id": "c47a931d-801f-0017-7920-1d865a000000",
     "x-ms-version": "2019-12-12"
   },
   "body": ""

test/transactions/datalake_file_create_delete/1_request.json

@@ -5,7 +5,7 @@
     "authorization": "<<STRIPPED>>",
     "content-length": "0",
     "user-agent": "azsdk-rust-storage_datalake/0.1.0 (1.58.0; linux; x86_64)",
-    "x-ms-date": "Fri, 21 Jan 2022 19:04:18 GMT",
+    "x-ms-date": "Tue, 08 Feb 2022 19:19:33 GMT",
     "x-ms-version": "2019-12-12"
   },
   "body": ""