Replace from_redis_value with from_owned_redis_value (#887)

from_owned_redis_value allows for some optimizations which aren't available when using a value reference.
valkey-io · Feb 4, 2024 · da08f73 · da08f73
1 parent 22ada90
commit da08f73
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 23 deletions.
diff --git a/csharp/lib/src/lib.rs b/csharp/lib/src/lib.rs
@@ -147,7 +147,7 @@ pub extern "C" fn get(client_ptr: *const c_void, callback_index: usize, key: *co
                 return;
             }
         };
-        let result = Option::<CString>::from_redis_value(&value);
+        let result = Option::<CString>::from_owned_redis_value(value);
 
         unsafe {
             match result {

diff --git a/glide-core/src/client/standalone_client.rs b/glide-core/src/client/standalone_client.rs
@@ -118,7 +118,7 @@ impl StandaloneClient {
                 Ok((connection, replication_status)) => {
                     nodes.push(connection);
                     if primary_index.is_none()
-                        && redis::from_redis_value::<String>(&replication_status)
+                        && redis::from_owned_redis_value::<String>(replication_status)
                             .is_ok_and(|val| val.contains("role:master"))
                     {
                         primary_index = Some(nodes.len() - 1);

diff --git a/glide-core/src/client/value_conversion.rs b/glide-core/src/client/value_conversion.rs
@@ -1,7 +1,9 @@
 /**
  * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0
  */
-use redis::{cluster_routing::Routable, from_redis_value, Cmd, ErrorKind, RedisResult, Value};
+use redis::{
+    cluster_routing::Routable, from_owned_redis_value, Cmd, ErrorKind, RedisResult, Value,
+};
 
 pub(crate) enum ExpectedReturnType {
     Map,
@@ -57,11 +59,13 @@ pub(crate) fn convert_to_expected_type(
             )
                 .into()),
         },
-        ExpectedReturnType::Double => Ok(Value::Double(from_redis_value::<f64>(&value)?.into())),
-        ExpectedReturnType::Boolean => Ok(Value::Boolean(from_redis_value::<bool>(&value)?)),
+        ExpectedReturnType::Double => {
+            Ok(Value::Double(from_owned_redis_value::<f64>(value)?.into()))
+        }
+        ExpectedReturnType::Boolean => Ok(Value::Boolean(from_owned_redis_value::<bool>(value)?)),
         ExpectedReturnType::DoubleOrNull => match value {
             Value::Nil => Ok(value),
-            _ => Ok(Value::Double(from_redis_value::<f64>(&value)?.into())),
+            _ => Ok(Value::Double(from_owned_redis_value::<f64>(value)?.into())),
         },
     }
 }

diff --git a/glide-core/tests/test_client.rs b/glide-core/tests/test_client.rs
@@ -96,8 +96,8 @@ pub(crate) mod shared_client_tests {
                 },
             )
             .await;
-            let hello: std::collections::HashMap<String, Value> = redis::from_redis_value(
-                &test_basics
+            let hello: std::collections::HashMap<String, Value> = redis::from_owned_redis_value(
+                test_basics
                     .client
                     .send_command(&redis::cmd("HELLO"), None)
                     .await
@@ -293,8 +293,8 @@ pub(crate) mod shared_client_tests {
             )
             .await;
             let mut client = test_basics.client;
-            let client_info: String = redis::from_redis_value(
-                &client.send_command(&client_info_cmd, None).await.unwrap(),
+            let client_info: String = redis::from_owned_redis_value(
+                client.send_command(&client_info_cmd, None).await.unwrap(),
             )
             .unwrap();
             assert!(client_info.contains(&format!("name={CLIENT_NAME}")));
@@ -314,8 +314,10 @@ pub(crate) mod shared_client_tests {
             }
             let client_info: String = repeat_try_create(|| async {
                 let mut client = client.clone();
-                redis::from_redis_value(&client.send_command(&client_info_cmd, None).await.unwrap())
-                    .ok()
+                redis::from_owned_redis_value(
+                    client.send_command(&client_info_cmd, None).await.unwrap(),
+                )
+                .ok()
             })
             .await;
 

diff --git a/glide-core/tests/test_cluster_client.rs b/glide-core/tests/test_cluster_client.rs
@@ -49,7 +49,7 @@ mod cluster_client_tests {
             let mut cmd = redis::cmd("INFO");
             cmd.arg("REPLICATION");
             let info = test_basics.client.send_command(&cmd, None).await.unwrap();
-            let info = redis::from_redis_value::<HashMap<String, String>>(&info).unwrap();
+            let info = redis::from_owned_redis_value::<HashMap<String, String>>(info).unwrap();
             let (primaries, replicas) = count_primaries_and_replicas(info);
             assert_eq!(primaries, 3);
             assert_eq!(replicas, 0);
@@ -80,7 +80,7 @@ mod cluster_client_tests {
                 )
                 .await
                 .unwrap();
-            let info = redis::from_redis_value::<HashMap<String, String>>(&info).unwrap();
+            let info = redis::from_owned_redis_value::<HashMap<String, String>>(info).unwrap();
             let (primaries, replicas) = count_primaries_and_replicas(info);
             assert_eq!(primaries, 3);
             assert_eq!(replicas, 0);
@@ -111,7 +111,7 @@ mod cluster_client_tests {
                 )
                 .await
                 .unwrap();
-            let info = redis::from_redis_value::<HashMap<String, String>>(&info).unwrap();
+            let info = redis::from_owned_redis_value::<HashMap<String, String>>(info).unwrap();
             let (primaries, replicas) = count_primaries_and_replicas(info);
             assert_eq!(primaries, 3);
             assert_eq!(replicas, 3);
@@ -141,7 +141,7 @@ mod cluster_client_tests {
                 )
                 .await
                 .unwrap();
-            let info = redis::from_redis_value::<String>(&info).unwrap();
+            let info = redis::from_owned_redis_value::<String>(info).unwrap();
             let (primaries, replicas) = count_primary_or_replica(&info);
             assert_eq!(primaries, 1);
             assert_eq!(replicas, 0);
@@ -175,7 +175,7 @@ mod cluster_client_tests {
                 )
                 .await
                 .unwrap();
-            let info = redis::from_redis_value::<String>(&info).unwrap();
+            let info = redis::from_owned_redis_value::<String>(info).unwrap();
             let (primaries, replicas) = count_primary_or_replica(&info);
             assert_eq!(primaries, 0);
             assert_eq!(replicas, 1);
@@ -209,7 +209,7 @@ mod cluster_client_tests {
                 )
                 .await
                 .unwrap();
-            let info = redis::from_redis_value::<String>(&info).unwrap();
+            let info = redis::from_owned_redis_value::<String>(info).unwrap();
             let (primaries, replicas) = count_primary_or_replica(&info);
             assert_eq!(primaries, 0);
             assert_eq!(replicas, 1);

diff --git a/glide-core/tests/test_standalone_client.rs b/glide-core/tests/test_standalone_client.rs
@@ -306,9 +306,10 @@ mod standalone_client_tests {
             .await;
             let mut client = test_basics.client;
 
-            let client_info =
-                String::from_redis_value(&client.send_command(&client_info_cmd).await.unwrap())
-                    .unwrap();
+            let client_info: String = String::from_owned_redis_value(
+                client.send_command(&client_info_cmd).await.unwrap(),
+            )
+            .unwrap();
             assert!(client_info.contains("db=4"));
 
             kill_connection(&mut client).await;
@@ -323,7 +324,8 @@ mod standalone_client_tests {
 
             let client_info = repeat_try_create(|| async {
                 let mut client = client.clone();
-                String::from_redis_value(&client.send_command(&client_info_cmd).await.unwrap()).ok()
+                String::from_owned_redis_value(client.send_command(&client_info_cmd).await.unwrap())
+                    .ok()
             })
             .await;
             assert!(client_info.contains("db=4"));

diff --git a/node/rust-client/src/lib.rs b/node/rust-client/src/lib.rs
@@ -178,7 +178,7 @@ fn redis_value_to_js(val: Value, js_env: Env) -> Result<JsUnknown> {
         Value::Map(map) => {
             let mut obj = js_env.create_object()?;
             for (key, value) in map {
-                let field_name = String::from_redis_value(&key).map_err(to_js_error)?;
+                let field_name = String::from_owned_redis_value(key).map_err(to_js_error)?;
                 let value = redis_value_to_js(value, js_env)?;
                 obj.set_named_property(&field_name, value)?;
             }