From de7f8fedf3538412186270d2bd49512d56873be3 Mon Sep 17 00:00:00 2001
From: Egor Ivkov <e.o.ivkov@gmail.com>
Date: Tue, 13 Jun 2023 19:03:25 +0300
Subject: [PATCH] fix: cas space by index and by name inconsistency

---
 src/rpc/cas.rs       | 35 +++++++++++++++++++++++++++++++++--
 test/conftest.py     | 10 ++++------
 test/int/test_cas.py | 26 +++++++++++++++++++++-----
 3 files changed, 58 insertions(+), 13 deletions(-)

diff --git a/src/rpc/cas.rs b/src/rpc/cas.rs
index 5b47a77e72..4d2c886736 100644
--- a/src/rpc/cas.rs
+++ b/src/rpc/cas.rs
@@ -227,7 +227,7 @@ pub struct PredicateInLua {
     /// CaS sender's current raft term.
     pub term: Option<RaftTerm>,
     /// Range that the CaS sender have read and expects it to be unmodified.
-    pub ranges: Option<Vec<Range>>,
+    pub ranges: Option<Vec<RangeInLua>>,
 }
 
 /// Predicate that will be checked by the leader, before accepting the proposed `op`.
@@ -259,7 +259,12 @@ impl Predicate {
         Ok(Self {
             index,
             term,
-            ranges: predicate.ranges.unwrap_or_default(),
+            ranges: predicate
+                .ranges
+                .unwrap_or_default()
+                .into_iter()
+                .map(Range::from_lua_args)
+                .collect::<traft::Result<_>>()?,
         })
     }
 
@@ -337,6 +342,18 @@ impl Predicate {
     }
 }
 
+/// Represents a lua table describing a [`Range`].
+///
+/// This is only used to parse lua arguments from lua api functions such as
+/// `pico.cas`.
+#[derive(Clone, Debug, ::serde::Serialize, ::serde::Deserialize, tlua::LuaRead)]
+pub struct RangeInLua {
+    /// Space name.
+    pub space: String,
+    pub key_min: Bound,
+    pub key_max: Bound,
+}
+
 /// A range of keys used as an argument for a [`Predicate`].
 #[derive(Clone, Debug, ::serde::Serialize, ::serde::Deserialize, tlua::LuaRead)]
 pub struct Range {
@@ -346,6 +363,20 @@ pub struct Range {
 }
 
 impl Range {
+    pub fn from_lua_args(range: RangeInLua) -> traft::Result<Self> {
+        let node = traft::node::global()?;
+        let space_id = if let Some(space) = node.storage.spaces.by_name(&range.space)? {
+            space.id
+        } else {
+            return Err(TraftError::Other("space not found".into()));
+        };
+        Ok(Self {
+            space: space_id,
+            key_min: range.key_min,
+            key_max: range.key_max,
+        })
+    }
+
     /// Creates new unbounded range in `space`. Use other methods to restrict it.
     ///
     /// # Example
diff --git a/test/conftest.py b/test/conftest.py
index fd8af98f17..98b73b72f9 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -203,13 +203,13 @@ class CasRange:
     @property
     def key_min_packed(self) -> dict:
         key = self.key_min.copy()
-        key["key"] = msgpack.packb([key["key"][0]])
+        key["key"] = msgpack.packb([key["key"][0]])  # type: ignore
         return key
 
     @property
     def key_max_packed(self) -> dict:
         key = self.key_max.copy()
-        key["key"] = msgpack.packb([key["key"][0]])
+        key["key"] = msgpack.packb([key["key"][0]])  # type: ignore
         return key
 
     def __repr__(self):
@@ -1077,7 +1077,7 @@ class Cluster:
     def cas(
         self,
         dml_kind: Literal["insert", "replace", "delete"],
-        space: str | int,
+        space: str,
         tuple: Tuple | List,
         index: int | None = None,
         term: int | None = None,
@@ -1096,14 +1096,12 @@ class Cluster:
         if instance is None:
             instance = self.instances[0]
 
-        space_id = instance.space_id(space)
-
         predicate_ranges = []
         if ranges is not None:
             for range in ranges:
                 predicate_ranges.append(
                     dict(
-                        space=space_id,
+                        space=space,
                         key_min=range.key_min,
                         key_max=range.key_max,
                     )
diff --git a/test/int/test_cas.py b/test/int/test_cas.py
index c2c6891f51..dc36d3ecbd 100644
--- a/test/int/test_cas.py
+++ b/test/int/test_cas.py
@@ -149,30 +149,46 @@ def test_cas_predicate(instance: Instance):
 
 # Previous tests use stored procedure `.proc_cas`, this one uses `pico.cas` lua api instead
 def test_cas_lua_api(cluster: Cluster):
-    def property(k: str):
+    def value(k: str):
         return cluster.instances[0].eval(
             """
-            local tuple = box.space._pico_property:get(...)
+            local tuple = box.space.some_space:get(...)
             return tuple and tuple.value
             """,
             k,
         )
 
     cluster.deploy(instance_count=3)
+
+    # We cannot use `_pico_property` here as it does not have
+    # a corresponding entry in `Spaces`
+    cluster.create_space(
+        dict(
+            id=1026,
+            name="some_space",
+            format=[
+                dict(name="key", type="string", is_nullable=False),
+                dict(name="value", type="string", is_nullable=False),
+            ],
+            primary_key=["key"],
+            distribution="global",
+        )
+    )
+
     read_index = cluster.instances[0].raft_read_index(_3_SEC)
 
     # Successful insert
-    ret = cluster.cas("insert", "_pico_property", ["fruit", "apple"], read_index)
+    ret = cluster.cas("insert", "some_space", ["fruit", "apple"], read_index)
     assert ret == read_index + 1
     cluster.raft_wait_index(ret, _3_SEC)
     assert cluster.instances[0].raft_read_index(_3_SEC) == ret
-    assert property("fruit") == "apple"
+    assert value("fruit") == "apple"
 
     # CaS rejected
     with pytest.raises(ReturnError) as e5:
         cluster.cas(
             "insert",
-            "_pico_property",
+            "some_space",
             ["fruit", "orange"],
             index=read_index,
             ranges=[CasRange(eq="fruit")],
-- 
GitLab