Loosen interface, additional proptests

liamwh · Sep 22, 2023 · bea79c1 · bea79c1
1 parent cc21e7d
commit bea79c1
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 47 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -23,3 +23,4 @@ serde = "1.0.188"
 tokio = "1.32.0"
 surrealdb = { features = ["kv-mem"], version = "~1.0" }
 proptest = "1.2.0"
+rand = "0.8.5"
diff --git a/src/new_id.rs b/src/new_id.rs
@@ -1,6 +1,5 @@
-use surrealdb::sql::Id;
-
 use crate::errors::IdError;
+use surrealdb::sql::Id;
 
 /// Defines a trait for creating a new ID with table name and inner ID.
 ///
@@ -14,46 +13,45 @@ pub trait NewId: Sized {
     const TABLE: &'static str;
 
     /// Creates a new instance of the implementing type by parsing and validating the given `id` string.
-    ///
-    /// # Errors
-    ///
-    /// Returns an error if the `id` string is empty, malformed, or doesn't match the expected table name.
-    fn new(id: &str) -> Result<Self, IdError> {
-        if id.is_empty() {
+    fn new<T: AsRef<str>>(id: T) -> Result<Self, IdError> {
+        let id_ref = id.as_ref();
+
+        let is_empty = id_ref.is_empty();
+        if is_empty {
             return Err(IdError::IdCannotBeEmpty);
         }
 
-        let mut split_at_colon = id.splitn(2, ':');
-        let table_part = split_at_colon
-            .next()
-            .ok_or(IdError::InvalidIdFormat("Missing table part".to_string()))?;
-
-        if let Some(id_part) = split_at_colon.next() {
-            if id_part.starts_with('⟨') && id_part.ends_with('⟩') {
-                let id_inner: String = id_part
-                    .chars()
-                    .skip(1)
-                    .take(id_part.chars().count() - 2)
-                    .collect();
-
-                if id_inner.is_empty() {
-                    return Err(IdError::IdCannotBeEmpty);
-                }
-
-                if table_part != Self::TABLE {
-                    return Err(IdError::InvalidTable(
-                        Self::TABLE.to_string(),
-                        table_part.to_string(),
-                    ));
-                }
-
-                return Ok(Self::from_inner_id(id_inner));
-            } else {
-                return Err(IdError::InvalidIdFormat(id.to_string()));
+        let mut split_at_colon = id_ref.splitn(2, ':');
+        let table_part = split_at_colon.next().unwrap_or(Self::TABLE);
+        let id_part = split_at_colon.next().unwrap_or(id_ref);
+        let start_bracket_count = id_ref.chars().filter(|&c| c == '⟨').count();
+        let end_bracket_count = id_ref.chars().filter(|&c| c == '⟩').count();
+        let is_valid_format = id_part.starts_with('⟨')
+            && id_part.ends_with('⟩')
+            && start_bracket_count == 1
+            && end_bracket_count == 1;
+
+        if is_valid_format {
+            let id_inner: String = id_part.chars().skip(1).take_while(|&c| c != '⟩').collect();
+            if id_inner.is_empty() {
+                return Err(IdError::IdCannotBeEmpty);
             }
+
+            if table_part != Self::TABLE {
+                return Err(IdError::InvalidTable(
+                    Self::TABLE.to_string(),
+                    table_part.to_string(),
+                ));
+            }
+
+            return Ok(Self::from_inner_id(id_inner));
+        }
+
+        if id_ref.contains(':') {
+            return Err(IdError::InvalidIdFormat(id_ref.to_string()));
         }
 
-        Ok(Self::from_inner_id(id.to_string()))
+        Ok(Self::from_inner_id(id_part.to_string()))
     }
 
     /// Constructs an instance from an inner ID of type `T`.

diff --git a/tests/proptest.rs b/tests/proptest.rs
@@ -6,45 +6,76 @@ use crate::user_id_integration::*;
 
 proptest! {
     #[test]
-    fn valid_id_and_table_always_creates_user_id(table in "users", id in "[a-f0-9\\-]{1,255}") {
+    fn valid_id_and_table_always_creates_user_id(table in "users", id in "[a-f0-9\\-:;@#$%^&*()_+={}\\[\\]~]{1,255}") {
         let full_id = format!("{}:⟨{}⟩", table, id);
-        let result = UserId::new(&full_id);
+        let result = UserId::new(full_id);
         assert!(result.is_ok());
     }
 
     #[test]
     fn invalid_table_name_returns_invalid_table_error(table in "[a-z]{5,9}", id in "[a-f0-9\\-]{1,255}") {
         if table != "users" {
             let full_id = format!("{}:⟨{}⟩", table, id);
-            let result = UserId::new(&full_id);
+            let result = UserId::new(full_id);
             assert!(matches!(result, Err(IdError::InvalidTable(_, _))));
         }
     }
 
     #[test]
-    fn empty_id_returns_id_cannot_be_empty_error(table in "users", id in "") {
-        let full_id = format!("{}:⟨{}⟩", table, id);
-        let result = UserId::new(&full_id);
-        assert!(matches!(result, Err(IdError::IdCannotBeEmpty)));
+    fn multiple_opening_brackets_should_fail(table in "users", id in "[a-f0-9\\-]{1,253}") {
+        let insert_pos = rand::random::<usize>() % id.len();
+        let full_id = format!("{}:⟨{}⟨{}⟩", table, &id[0..insert_pos], &id[insert_pos..]);
+        let result = UserId::new(full_id);
+        assert!(matches!(result, Err(IdError::InvalidIdFormat(_))));
+    }
+
+    #[test]
+    fn multiple_closing_brackets_should_fail(table in "users", id in "[a-f0-9\\-]{1,253}") {
+        let insert_pos = rand::random::<usize>() % id.len();
+        let full_id = format!("{}:⟨{}⟩{}⟩", table, &id[0..insert_pos], &id[insert_pos..]);
+        let result = UserId::new(full_id);
+        assert!(matches!(result, Err(IdError::InvalidIdFormat(_))));
+    }
+
+    #[test]
+    fn opening_backet_before_colon_should_fail(table in "users", id in "[a-f0-9\\-]{1,253}") {
+        let insert_pos = rand::random::<usize>() % table.len();
+        let full_id = format!("{}⟨{}:⟨{}⟩", &table[0..insert_pos], &table[insert_pos..], id);
+        let result = UserId::new(full_id);
+        assert!(matches!(result, Err(IdError::InvalidIdFormat(_))));
+    }
+
+    #[test]
+    fn closing_backet_before_colon_should_fail(table in "users", id in "[a-f0-9\\-]{1,253}") {
+        let insert_pos = rand::random::<usize>() % table.len();
+        let full_id = format!("{}⟩{}:{}", &table[0..insert_pos], &table[insert_pos..], id);
+        let result = UserId::new(full_id);
+        assert!(matches!(result, Err(IdError::InvalidIdFormat(_))));
     }
 
     #[test]
     fn invalid_id_format_returns_error(table in "users", id in "[a-f0-9\\-]{1,255}") {
         let full_id = format!("{}:{}", table, id);
-        let result = UserId::new(&full_id);
+        let result = UserId::new(full_id);
         assert!(matches!(result, Err(IdError::InvalidIdFormat(_))));
     }
 
     #[test]
     fn correct_table_but_invalid_id_format_returns_error(table in "users", id in "[a-f0-9\\-]{1,255}") {
         let full_id = format!("{}:{}", table, id);
-        let result = UserId::new(&full_id);
+        let result = UserId::new(full_id);
         assert!(matches!(result, Err(IdError::InvalidIdFormat(_))));
     }
 
     #[test]
     fn id_without_table_uses_default_and_succeeds(id in "[a-f0-9\\-]{1,255}") {
-        let result = UserId::new(&id);
+        let result = UserId::new(id);
         assert!(result.is_ok());
     }
+
+    #[test]
+    fn id_without_table_but_starts_with_colon_should_fail(id in ":[a-f0-9\\-]{1,255}") {
+        let result = UserId::new(id);
+        assert!(matches!(result, Err(IdError::InvalidIdFormat(_))));
+    }
 }
diff --git a/tests/user_id_integration.rs b/tests/user_id_integration.rs
@@ -55,7 +55,7 @@ mod tests {
     fn invalid_table_name_returns_invalid_table_error() {
         let fx_table_name = "invalid_table";
         let id = format!("{}:⟨fa77edc3-56ed-4208-9e0b-c0b1c32e2d34⟩", fx_table_name);
-        let result = UserId::new(&id);
+        let result = UserId::new(id);
         assert_eq!(
             result,
             Err(IdError::InvalidTable(
@@ -72,6 +72,7 @@ mod tests {
             "users:⟨fa77edc3-56ed-4208-9e0b-c0b1c32e2d34", // Missing closing bracket
             "users:fa77edc3-56ed-4208-9e0b-c0b1c32e2d34⟩", // Missing opening bracket
             "users:⟨fa77edc3-56ed-4208-9e0b-c0b1c32e2d34⟩ ", // Trailing space
+            "users:⟨fa77edc3⟩56ed⟩",                      // Multiple ⟩
         ];
 
         for id in invalid_ids {
@@ -99,6 +100,13 @@ mod tests {
         assert_eq!(result, Err(IdError::IdCannotBeEmpty));
     }
 
+    #[test]
+    fn valid_table_but_empty_id_part_returns_id_cannot_be_empty_error() {
+        let full_id = format!("{}:⟨{}⟩", USERS_TABLE, "");
+        let result = UserId::new(full_id);
+        assert!(matches!(result, Err(IdError::IdCannotBeEmpty)));
+    }
+
     #[tokio::test]
     async fn valid_user_object_saves_to_database_successfully_with_specified_table_syntax() {
         let fx_user = User {