feat(foundationdb): add more tests to directory.Move

Author: PierreZ

foundationdb/src/directory/error.rs

@@ -26,6 +26,8 @@ pub enum DirectoryError {
     ParentDirDoesNotExists,
     /// the layer is incompatible.
     IncompatibleLayer,
+    /// the destination directory cannot be a subdirectory of the source directory.
+    BadDestinationDirectory,
     Message(String),
     /// Bad directory version.
     Version(String),

foundationdb/src/directory/mod.rs

@@ -173,27 +173,43 @@ impl DirectoryLayer {
     ) -> Result<bool, DirectoryError> {
         self.check_version(trx, false).await?;
 
+        if old_path.is_empty() || new_path.is_empty() {
+            return Err(DirectoryError::NoPathProvided);
+        }
+
+        if new_path.starts_with(old_path.as_slice()) {
+            return Err(DirectoryError::BadDestinationDirectory);
+        }
+
         let mut old_nodes = self.find_nodes(&trx, old_path.to_owned()).await?;
         let last_node_from_old_path = match old_nodes.last_mut() {
             None => return Err(DirectoryError::Message(String::from("empty path"))),
             Some(node) => node,
         };
+
         let content_subspace = last_node_from_old_path
             .content_subspace
             .as_ref()
             .ok_or(DirectoryError::DirNotExists)?;
 
         let mut new_nodes = self.find_nodes(&trx, new_path.to_owned()).await?;
+
         // assert that parent of the new node exists
-        if new_nodes.get(new_nodes.len() - 2).is_none() {
-            return Err(DirectoryError::ParentDirDoesNotExists);
+        if new_nodes.len() >= 2 {
+            match new_nodes.get(new_nodes.len() - 2) {
+                None => {}
+                Some(parent_node) => match parent_node.content_subspace {
+                    None => return Err(DirectoryError::ParentDirDoesNotExists),
+                    Some(_) => {}
+                },
+            }
         }
 
         let last_node_from_new_path = match new_nodes.last_mut() {
             None => return Err(DirectoryError::Message(String::from("empty path"))),
             Some(node) => {
                 if node.content_subspace.is_some() {
-                    return Err(DirectoryError::DirNotExists);
+                    return Err(DirectoryError::DirAlreadyExists);
                 }
                 node
             }

foundationdb/tests/directory.rs

@@ -18,7 +18,7 @@ fn test_directory() {
 
     eprintln!("clearing all keys");
     let trx = db.create_trx().expect("cannot create txn");
-    trx.clear_range(b"\x00", b"\xff");
+    trx.clear_range(b"", b"\xff");
     futures::executor::block_on(trx.commit()).expect("could not clear keys");
 
     eprintln!("creating directories");
@@ -50,21 +50,97 @@ fn test_directory() {
 
     futures::executor::block_on(test_bad_layer(&db)).expect("failed to run");
 
-    futures::executor::block_on(test_move_to(
+    futures::executor::block_on(test_create_then_move_to(
         &db,
         &directory,
         vec![String::from("d"), String::from("e")],
         vec![String::from("a"), String::from("g")],
     ))
     .expect("failed to run");
+
+    // trying to move on empty path
+    match futures::executor::block_on(test_move_to(
+        &db,
+        &directory,
+        vec![String::from("dsa")],
+        vec![],
+    )) {
+        Err(DirectoryError::NoPathProvided) => {}
+        _ => panic!("should have failed"),
+    }
+
+    // trying to move on empty path
+    match futures::executor::block_on(test_move_to(
+        &db,
+        &directory,
+        vec![],
+        vec![String::from("dsa")],
+    )) {
+        Err(DirectoryError::NoPathProvided) => {}
+        Err(err) => panic!("should have NoPathProvided, got {:?}", err),
+        Ok(()) => panic!("should not be fine"),
+    }
+
+    // source path does not exists
+    match futures::executor::block_on(test_move_to(
+        &db,
+        &directory,
+        vec![String::from("e")],
+        vec![String::from("f")],
+    )) {
+        Err(DirectoryError::DirNotExists) => {}
+        Err(err) => panic!("should have NoPathProvided, got {:?}", err),
+        Ok(()) => panic!("should not be fine"),
+    }
+
+    // destination's parent does not exists
+    match futures::executor::block_on(test_create_then_move_to(
+        &db,
+        &directory,
+        vec![String::from("a"), String::from("g")],
+        vec![String::from("i-do-not-exists-yet"), String::from("z")],
+    )) {
+        Err(DirectoryError::ParentDirDoesNotExists) => {}
+        Err(err) => panic!("should have ParentDirDoesNotExists, got {:?}", err),
+        Ok(()) => panic!("should not be fine"),
+    }
+
+    // destination not empty
+    match futures::executor::block_on(test_create_then_move_to(
+        &db,
+        &directory,
+        vec![String::from("a"), String::from("g")],
+        vec![String::from("a"), String::from("g")],
+    )) {
+        Err(DirectoryError::BadDestinationDirectory) => {}
+        Err(err) => panic!("should have BadDestinationDirectory, got {:?}", err),
+        Ok(()) => panic!("should not be fine"),
+    }
+
+    // cannot move to a children of the old-path
+    match futures::executor::block_on(test_create_then_move_to(
+        &db,
+        &directory,
+        vec![String::from("a"), String::from("g")],
+        vec![String::from("a"), String::from("g"), String::from("s")],
+    )) {
+        Err(DirectoryError::BadDestinationDirectory) => {}
+        Err(err) => panic!("should have BadDestinationDirectory, got {:?}", err),
+        Ok(()) => panic!("should not be fine"),
+    }
 }
 
-async fn test_move_to(
+async fn test_create_then_move_to(
     db: &Database,
     directory: &DirectoryLayer,
     old_paths: Vec<String>,
     new_paths: Vec<String>,
 ) -> Result<(), DirectoryError> {
+    eprintln!(
+        "moving {:?} to {:?}",
+        old_paths.to_owned(),
+        new_paths.to_owned()
+    );
     let trx = db.create_trx()?;
     let create_output = directory.create_or_open(&trx, old_paths.to_owned()).await?;
 
@@ -91,6 +167,38 @@ async fn test_move_to(
     Ok(())
 }
 
+async fn test_move_to(
+    db: &Database,
+    directory: &DirectoryLayer,
+    old_paths: Vec<String>,
+    new_paths: Vec<String>,
+) -> Result<(), DirectoryError> {
+    eprintln!(
+        "moving {:?} to {:?}",
+        old_paths.to_owned(),
+        new_paths.to_owned()
+    );
+    let trx = db.create_trx()?;
+
+    let move_output = directory
+        .move_to(&trx, old_paths.to_owned(), new_paths.to_owned())
+        .await?;
+    assert!(move_output);
+
+    trx.commit().await.expect("could not commit");
+    let trx = db.create_trx()?;
+
+    directory.open(&trx, new_paths).await?;
+
+    trx.commit().await.expect("could not commit");
+    let trx = db.create_trx()?;
+
+    let open_old_path = directory.open(&trx, old_paths).await;
+    assert!(open_old_path.is_err());
+
+    Ok(())
+}
+
 async fn test_create_then_open_async(
     db: &Database,
     directory: &DirectoryLayer,