refactor(xlineapi): add tests for KeyRange conflict check

Signed-off-by: lxl66566 <lxl66566@gmail.com> refactor(xlineapi): KeyRange refactor Signed-off-by: lxl66566 <lxl66566@gmail.com> refactor(xlineapi): refactor success, test pass Signed-off-by: lxl66566 <lxl66566@gmail.com> refactor(xlineapi): use KeyRange instead of directly pass &[u8] to avoid clone Signed-off-by: lxl66566 <lxl66566@gmail.com> refactor(xlineapi): simplify all keys judgement Signed-off-by: lxl66566 <lxl66566@gmail.com>
xline-kv · Nov 4, 2024 · bdd59e4 · bdd59e4
1 parent 080be1e
commit bdd59e4
Show file tree

Hide file tree

Showing 25 changed files with 652 additions and 498 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/utils/Cargo.toml b/crates/utils/Cargo.toml
@@ -25,7 +25,9 @@ derive_builder = "0.20.0"
 event-listener = "5.3.1"
 futures = "0.3.30"
 getset = "0.1"
-interval_map = { version = "0.1", package = "rb-interval-map" }
+interval_map = { version = "0.1", package = "rb-interval-map", features = [
+  "serde",
+] }
 opentelemetry = { version = "0.24.0", features = ["trace"] }
 opentelemetry_sdk = { version = "0.24.1", features = ["trace"] }
 parking_lot = { version = "0.12.3", optional = true }

diff --git a/crates/utils/src/parser.rs b/crates/utils/src/parser.rs
@@ -70,7 +70,8 @@ pub fn parse_members(s: &str) -> Result<HashMap<String, Vec<String>>, ConfigPars
     Ok(map)
 }
 
-/// Parse `ClusterRange` from the given string
+/// Parse `ClusterRange` from the given string. The string should be in the
+/// format of `start..end`, and start and end should be u64.
 ///
 /// # Errors
 ///
@@ -185,7 +186,7 @@ pub fn parse_log_file(s: &str) -> Result<PathBuf, ConfigParseError> {
     }
     // This regular expression matches file paths in a specific format.
     // Explanation of the regex pattern:
-    // ^(\.|\.\.)?            - Matches an optional prefix consisting of a dot (.), two dots (..), or a tilde (~).
+    // ^(\.|\.\.)?              - Matches an optional prefix consisting of a dot (.), two dots (..), or a tilde (~).
     // (/[a-zA-Z0-9._-]+)+/?    - Matches one or more occurrences of a forward slash (/) followed by one or more alphanumeric characters, dots (.), underscores (_), or hyphens (-).
     // /?$                      - Matches an optional trailing forward slash (/) at the end of the string.
     // Overall, this regex pattern is used to validate file paths that follow a specific format commonly used in Linux filesystem

diff --git a/crates/xline-client/src/clients/lock.rs b/crates/xline-client/src/clients/lock.rs
@@ -6,7 +6,8 @@ use clippy_utilities::OverflowArithmetic;
 use tokio::{task::JoinHandle, time::sleep};
 use tonic::transport::Channel;
 use xlineapi::{
-    command::{Command, CommandResponse, KeyRange, SyncResponse},
+    command::{Command, CommandResponse, SyncResponse},
+    keyrange::KeyRange,
     Compare, CompareResult, CompareTarget, DeleteRangeRequest, EventType, PutRequest, RangeRequest,
     RangeResponse, Request, RequestOp, RequestWrapper, Response, ResponseHeader, SortOrder,
     SortTarget, TargetUnion, TxnRequest, TxnResponse,

diff --git a/crates/xline-client/src/types/kv.rs b/crates/xline-client/src/types/kv.rs
@@ -1,4 +1,4 @@
-use xlineapi::command::KeyRange;
+use xlineapi::keyrange::KeyRange;
 pub use xlineapi::{
     CompactionResponse, CompareResult, CompareTarget, DeleteRangeResponse, PutResponse,
     RangeResponse, Response, ResponseOp, SortOrder, SortTarget, TargetUnion, TxnResponse,

diff --git a/crates/xline-client/src/types/range_end.rs b/crates/xline-client/src/types/range_end.rs
@@ -1,4 +1,4 @@
-use xlineapi::command::KeyRange;
+use xlineapi::keyrange::KeyRange;
 
 /// Range end options, indicates how to set `range_end` from a key.
 #[derive(Clone, Debug, PartialEq, Eq, Default)]

diff --git a/crates/xline-client/src/types/watch.rs b/crates/xline-client/src/types/watch.rs
@@ -272,7 +272,7 @@ impl DerefMut for WatchStreaming {
 
 #[cfg(test)]
 mod tests {
-    use xlineapi::command::KeyRange;
+    use xlineapi::keyrange::KeyRange;
 
     use super::*;
 

diff --git a/crates/xline/src/conflict/mod.rs b/crates/xline/src/conflict/mod.rs
@@ -7,8 +7,8 @@ use curp::{
 };
 use utils::interval_map::Interval;
 use xlineapi::{
-    command::{Command, KeyRange},
-    interval::BytesAffine,
+    command::Command,
+    keyrange::{BytesAffine, KeyRange},
     RequestWrapper,
 };
 

diff --git a/crates/xline/src/conflict/spec_pool.rs b/crates/xline/src/conflict/spec_pool.rs
@@ -8,7 +8,7 @@ use std::{collections::HashMap, sync::Arc};
 use curp::rpc::PoolEntry;
 use curp_external_api::conflict::{ConflictPoolOp, EntryId, SpeculativePoolOp};
 use utils::interval_map::{Interval, IntervalMap};
-use xlineapi::{command::Command, interval::BytesAffine};
+use xlineapi::{command::Command, keyrange::BytesAffine};
 
 use crate::storage::lease_store::LeaseCollection;
 

diff --git a/crates/xline/src/conflict/uncommitted_pool.rs b/crates/xline/src/conflict/uncommitted_pool.rs
@@ -12,7 +12,7 @@ use curp::rpc::PoolEntry;
 use curp_external_api::conflict::{ConflictPoolOp, EntryId, UncommittedPoolOp};
 use itertools::Itertools;
 use utils::interval_map::{Interval, IntervalMap};
-use xlineapi::{command::Command, interval::BytesAffine};
+use xlineapi::{command::Command, keyrange::BytesAffine};
 
 use crate::storage::lease_store::LeaseCollection;
 

diff --git a/crates/xline/src/server/command.rs b/crates/xline/src/server/command.rs
@@ -34,35 +34,6 @@ use crate::{
 /// Key of applied index
 pub(crate) const APPLIED_INDEX_KEY: &str = "applied_index";
 
-/// Range start and end to get all keys
-const UNBOUNDED: &[u8] = &[0_u8];
-/// Range end to get one key
-const ONE_KEY: &[u8] = &[];
-
-/// Type of `KeyRange`
-pub(crate) enum RangeType {
-    /// `KeyRange` contains only one key
-    OneKey,
-    /// `KeyRange` contains all keys
-    AllKeys,
-    /// `KeyRange` contains the keys in the range
-    Range,
-}
-
-impl RangeType {
-    /// Get `RangeType` by given `key` and `range_end`
-    #[inline]
-    pub(crate) fn get_range_type(key: &[u8], range_end: &[u8]) -> Self {
-        if range_end == ONE_KEY {
-            RangeType::OneKey
-        } else if key == UNBOUNDED && range_end == UNBOUNDED {
-            RangeType::AllKeys
-        } else {
-            RangeType::Range
-        }
-    }
-}
-
 /// Command Executor
 #[derive(Debug)]
 pub(crate) struct CommandExecutor {

diff --git a/crates/xline/src/server/lock_server.rs b/crates/xline/src/server/lock_server.rs
@@ -10,8 +10,9 @@ use utils::build_endpoint;
 #[cfg(madsim)]
 use utils::ClientTlsConfig;
 use xlineapi::{
-    command::{Command, CommandResponse, CurpClient, KeyRange, SyncResponse},
+    command::{Command, CommandResponse, CurpClient, SyncResponse},
     execute_error::ExecuteError,
+    keyrange::KeyRange,
     AuthInfo, EventType,
 };
 

diff --git a/crates/xline/src/server/watch_server.rs b/crates/xline/src/server/watch_server.rs
@@ -9,7 +9,7 @@ use tokio::sync::mpsc;
 use tokio_stream::{wrappers::ReceiverStream, Stream, StreamExt};
 use tracing::{debug, warn};
 use utils::task_manager::{tasks::TaskName, Listener, TaskManager};
-use xlineapi::command::KeyRange;
+use xlineapi::keyrange::KeyRange;
 
 use crate::{
     header_gen::HeaderGenerator,
@@ -207,7 +207,7 @@ where
             return;
         };
 
-        let key_range = KeyRange::new(req.key, req.range_end);
+        let key_range = KeyRange::new_etcd(req.key, req.range_end);
         self.kv_watcher.watch(
             watch_id,
             key_range,

diff --git a/crates/xline/src/storage/auth_store/perms.rs b/crates/xline/src/storage/auth_store/perms.rs
@@ -6,7 +6,7 @@ use jsonwebtoken::{
 use merged_range::MergedRange;
 use serde::{Deserialize, Serialize};
 use utils::timestamp;
-use xlineapi::{command::KeyRange, AuthInfo};
+use xlineapi::{keyrange::KeyRange, AuthInfo};
 
 use crate::rpc::{Permission, Type};
 
@@ -123,18 +123,18 @@ impl UserPermissions {
 
     /// Insert a permission to `UserPermissions`
     pub(super) fn insert(&mut self, perm: Permission) {
-        let range = KeyRange::new(perm.key, perm.range_end).unpack();
+        let range = KeyRange::new_etcd(perm.key, perm.range_end);
         #[allow(clippy::unwrap_used)] // safe unwrap
         match Type::try_from(perm.perm_type).unwrap() {
             Type::Readwrite => {
-                self.read.insert(range.clone());
-                self.write.insert(range);
+                self.read.insert(range.clone().into_bounds());
+                self.write.insert(range.into_bounds());
             }
             Type::Write => {
-                self.write.insert(range);
+                self.write.insert(range.into_bounds());
             }
             Type::Read => {
-                self.read.insert(range);
+                self.read.insert(range.into_bounds());
             }
         }
     }

diff --git a/crates/xline/src/storage/auth_store/store.rs b/crates/xline/src/storage/auth_store/store.rs
@@ -18,8 +18,9 @@ use pbkdf2::{
 };
 use utils::parking_lot_lock::RwLockMap;
 use xlineapi::{
-    command::{CommandResponse, KeyRange, SyncResponse},
+    command::{CommandResponse, SyncResponse},
     execute_error::ExecuteError,
+    keyrange::KeyRange,
     AuthInfo,
 };
 
@@ -1119,7 +1120,7 @@ impl AuthStore {
         if user.has_role(ROOT_ROLE) {
             return Ok(());
         }
-        let key_range = KeyRange::new(key, range_end);
+        let key_range = KeyRange::new_etcd(key, range_end);
         if let Some(permissions) = self.permission_cache.read().user_permissions.get(username) {
             match perm_type {
                 Type::Read => {
@@ -1222,10 +1223,10 @@ mod test {
                 user_permissions: HashMap::from([(
                     "u".to_owned(),
                     UserPermissions {
-                        read: MergedRange::from_iter(vec![KeyRange::new("foo", "")]),
+                        read: MergedRange::from_iter(vec![KeyRange::new_etcd("foo", "")]),
                         write: MergedRange::from_iter(vec![
-                            KeyRange::new("foo", ""),
-                            KeyRange::new("fop", "foz")
+                            KeyRange::new_etcd("foo", ""),
+                            KeyRange::new_etcd("fop", "foz")
                         ]),
                     },
                 )]),
@@ -1382,8 +1383,8 @@ mod test {
                 user_permissions: HashMap::from([(
                     "u".to_owned(),
                     UserPermissions {
-                        read: MergedRange::from_iter(vec![KeyRange::new("foo", "")]),
-                        write: MergedRange::from_iter(vec![KeyRange::new("foo", "")]),
+                        read: MergedRange::from_iter(vec![KeyRange::new_etcd("foo", "")]),
+                        write: MergedRange::from_iter(vec![KeyRange::new_etcd("foo", "")]),
                     },
                 )]),
                 role_to_users_map: HashMap::from([("r".to_owned(), vec!["u".to_owned()])]),