Merge pull request #80 from nrc/kv-ref-into

Don't implement From<&'static str> for Key and Value

Author: nrc

examples/raw.rs

@@ -1,12 +1,12 @@
 // Copyright 2018 TiKV Project Authors. Licensed under Apache-2.0.
 
 #![feature(async_await, await_macro)]
-#![type_length_limit = "3081103"]
+#![type_length_limit = "8165158"]
 
 mod common;
 
 use crate::common::parse_args;
-use tikv_client::{raw::Client, Config, Key, KvPair, Result, Value};
+use tikv_client::{raw::Client, Config, Key, KvPair, Result, ToOwnedRange, Value};
 
 const KEY: &str = "TiKV";
 const VALUE: &str = "Rust";
@@ -35,7 +35,7 @@ async fn main() -> Result<()> {
     // place.
     //
     // Here we set the key `TiKV` to have the value `Rust` associated with it.
-    client.put(KEY, VALUE).await.unwrap(); // Returns a `tikv_client::Error` on failure.
+    client.put(KEY.to_owned(), VALUE.to_owned()).await.unwrap(); // Returns a `tikv_client::Error` on failure.
     println!("Put key {:?}, value {:?}.", KEY, VALUE);
 
     // Unlike a standard Rust HashMap all calls take owned values. This is because under the hood
@@ -47,33 +47,36 @@ async fn main() -> Result<()> {
     //
     // It is best to pass a `Vec<u8>` in terms of explictness and speed. `String`s and a few other
     // types are supported as well, but it all ends up as `Vec<u8>` in the end.
-    let value: Option<Value> = client.get(KEY).await?;
-    assert_eq!(value, Some(Value::from(VALUE)));
-    println!("Get key {:?} returned value {:?}.", Key::from(KEY), value);
+    let value: Option<Value> = client.get(KEY.to_owned()).await?;
+    assert_eq!(value, Some(Value::from(VALUE.to_owned())));
+    println!("Get key `{}` returned value {:?}.", KEY, value);
 
     // You can also set the `ColumnFamily` used by the request.
     // This is *advanced usage* and should have some special considerations.
-    client.delete(KEY).await.expect("Could not delete value");
-    println!("Key: {:?} deleted", Key::from(KEY));
+    client
+        .delete(KEY.to_owned())
+        .await
+        .expect("Could not delete value");
+    println!("Key: `{}` deleted", KEY);
 
     // Here we check if the key has been deleted from the key-value store.
     let value: Option<Value> = client
-        .get(KEY)
+        .get(KEY.to_owned())
         .await
         .expect("Could not get just deleted entry");
     assert!(value.is_none());
 
     // You can ask to write multiple key-values at the same time, it is much more
     // performant because it is passed in one request to the key-value store.
     let pairs = vec![
-        KvPair::from(("k1", "v1")),
-        KvPair::from(("k2", "v2")),
-        KvPair::from(("k3", "v3")),
+        KvPair::from(("k1".to_owned(), "v1".to_owned())),
+        KvPair::from(("k2".to_owned(), "v2".to_owned())),
+        KvPair::from(("k3".to_owned(), "v3".to_owned())),
     ];
     client.batch_put(pairs).await.expect("Could not put pairs");
 
     // Same thing when you want to retrieve multiple values.
-    let keys = vec![Key::from("k1"), Key::from("k2")];
+    let keys = vec![Key::from("k1".to_owned()), Key::from("k2".to_owned())];
     let values = client
         .batch_get(keys.clone())
         .await
@@ -86,12 +89,15 @@ async fn main() -> Result<()> {
     let end = "k2";
     let pairs = client
         .with_key_only(true)
-        .scan(start..=end, 10)
+        .scan((start..=end).to_owned(), 10)
         .await
         .expect("Could not scan");
 
     let keys: Vec<_> = pairs.into_iter().map(|p| p.key().clone()).collect();
-    assert_eq!(&keys, &[Key::from("k1"), Key::from("k2")]);
+    assert_eq!(
+        &keys,
+        &[Key::from("k1".to_owned()), Key::from("k2".to_owned())]
+    );
     println!("Scaning from {:?} to {:?} gives: {:?}", start, end, keys);
 
     // Cleanly exit.

src/kv/bound_range.rs

@@ -3,6 +3,7 @@
 use super::Key;
 #[cfg(test)]
 use proptest_derive::Arbitrary;
+use std::borrow::Borrow;
 use std::cmp::{Eq, PartialEq};
 use std::convert::TryFrom;
 use std::ops::{Bound, Range, RangeFrom, RangeInclusive};
@@ -26,25 +27,25 @@ use crate::{Error, Result};
 /// use std::ops::{Range, RangeInclusive, RangeTo, RangeToInclusive, RangeFrom, RangeFull, Bound};
 /// # use std::convert::TryInto;
 ///
-/// let explict_range: Range<Key> = Range { start: Key::from("Rust"), end: Key::from("TiKV") };
+/// let explict_range: Range<Key> = Range { start: Key::from("Rust".to_owned()), end: Key::from("TiKV".to_owned()) };
 /// let from_explict_range: BoundRange = explict_range.into();
 ///
-/// let range: Range<&str> = "Rust".."TiKV";
+/// let range: Range<String> = "Rust".to_owned().."TiKV".to_owned();
 /// let from_range: BoundRange = range.into();
 /// assert_eq!(from_explict_range, from_range);
 ///
-/// let range: RangeInclusive<&str> = "Rust"..="TiKV";
+/// let range: RangeInclusive<String> = "Rust".to_owned()..="TiKV".to_owned();
 /// let from_range: BoundRange = range.into();
 /// assert_eq!(
 ///     from_range,
-///     (Bound::Included(Key::from("Rust")), Bound::Included(Key::from("TiKV"))),
+///     (Bound::Included(Key::from("Rust".to_owned())), Bound::Included(Key::from("TiKV".to_owned()))),
 /// );
 ///
-/// let range_from: RangeFrom<&str> = "Rust"..;
+/// let range_from: RangeFrom<String> = "Rust".to_owned()..;
 /// let from_range_from: BoundRange = range_from.into();
 /// assert_eq!(
 ///     from_range_from,
-///     (Bound::Included(Key::from("Rust")), Bound::Unbounded),
+///     (Bound::Included(Key::from("Rust".to_owned())), Bound::Unbounded),
 /// );
 /// ```
 ///
@@ -75,24 +76,24 @@ impl BoundRange {
     /// The **end** of a scan is exclusive, unless appended with an '\0', then it is inclusive.
     ///
     /// ```rust
-    /// use tikv_client::{BoundRange, Key};
+    /// use tikv_client::{BoundRange, Key, ToOwnedRange};
     /// // Exclusive
     /// let range = "a".."z";
     /// assert_eq!(
-    ///     BoundRange::from(range).into_keys(),
-    ///     (Key::from("a"), Some(Key::from("z"))),
+    ///     BoundRange::from(range.to_owned()).into_keys(),
+    ///     (Key::from("a".to_owned()), Some(Key::from("z".to_owned()))),
     /// );
     /// // Inclusive
     /// let range = "a"..="z";
     /// assert_eq!(
-    ///     BoundRange::from(range).into_keys(),
-    ///     (Key::from("a"), Some(Key::from("z\0"))),
+    ///     BoundRange::from(range.to_owned()).into_keys(),
+    ///     (Key::from("a".to_owned()), Some(Key::from("z\0".to_owned()))),
     /// );
     /// // Open
-    /// let range = "a"..;
+    /// let range = "a".to_owned()..;
     /// assert_eq!(
     ///     BoundRange::from(range).into_keys(),
-    ///     (Key::from("a"), None),
+    ///     (Key::from("a".to_owned()), None),
     /// );
     // ```
     pub fn into_keys(self) -> (Key, Option<Key>) {
@@ -116,6 +117,7 @@ impl BoundRange {
     }
 }
 
+// FIXME `==` should not `clone`
 impl<T: Into<Key> + Clone> PartialEq<(Bound<T>, Bound<T>)> for BoundRange {
     fn eq(&self, other: &(Bound<T>, Bound<T>)) -> bool {
         self.from == convert_to_bound_key(other.0.clone())
@@ -180,6 +182,48 @@ impl<T: Into<Key> + Eq> TryFrom<(Bound<T>, Bound<T>)> for BoundRange {
     }
 }
 
+/// A convenience trait for converting ranges of borrowed types into a `BoundRange`.
+pub trait ToOwnedRange {
+    /// Transform a borrowed range of some form into an owned `BoundRange`.
+    fn to_owned(self) -> BoundRange;
+}
+
+impl<T: Into<Key> + Borrow<U>, U: ToOwned<Owned = T> + ?Sized> ToOwnedRange for Range<&U> {
+    fn to_owned(self) -> BoundRange {
+        From::from(Range {
+            start: self.start.to_owned(),
+            end: self.end.to_owned(),
+        })
+    }
+}
+
+impl<T: Into<Key> + Borrow<U>, U: ToOwned<Owned = T> + ?Sized> ToOwnedRange for RangeFrom<&U> {
+    fn to_owned(self) -> BoundRange {
+        From::from(RangeFrom {
+            start: self.start.to_owned(),
+        })
+    }
+}
+
+impl<T: Into<Key> + Borrow<U>, U: ToOwned<Owned = T> + ?Sized> ToOwnedRange for RangeInclusive<&U> {
+    fn to_owned(self) -> BoundRange {
+        let (from, to) = self.into_inner();
+        From::from(RangeInclusive::new(from.to_owned(), to.to_owned()))
+    }
+}
+
+impl<T: Into<Key> + Borrow<U>, U: ToOwned<Owned = T> + ?Sized> ToOwnedRange for (&U, Option<&U>) {
+    fn to_owned(self) -> BoundRange {
+        From::from((self.0.to_owned(), self.1.map(|u| u.to_owned())))
+    }
+}
+
+impl<T: Into<Key> + Borrow<U>, U: ToOwned<Owned = T> + ?Sized> ToOwnedRange for (&U, &U) {
+    fn to_owned(self) -> BoundRange {
+        From::from((self.0.to_owned(), self.1.to_owned()))
+    }
+}
+
 fn convert_to_bound_key<K: Into<Key>>(b: Bound<K>) -> Bound<Key> {
     match b {
         Bound::Included(k) => Bound::Included(k.into()),

src/kv/key.rs

@@ -6,26 +6,21 @@ use proptest::{arbitrary::any_with, collection::size_range};
 #[cfg(test)]
 use proptest_derive::Arbitrary;
 use std::ops::Bound;
-use std::{fmt, str, u8};
+use std::{fmt, u8};
 
 /// The key part of a key/value pair.
 ///
 /// In TiKV, keys are an ordered sequence of bytes. This has an advantage over choosing `String` as
 /// valid `UTF-8` is not required. This means that the user is permitted to store any data they wish,
 /// as long as it can be represented by bytes. (Which is to say, pretty much anything!)
 ///
-/// This type also implements `From` for many types. With one exception, these are all done without
-/// reallocation. Using a `&'static str`, like many examples do for simplicity, has an internal
-/// allocation cost.
-///
-/// This type wraps around an owned value, so it should be treated it like `String` or `Vec<u8>`
-/// over a `&str` or `&[u8]`.
+/// This type wraps around an owned value, so it should be treated it like `String` or `Vec<u8>`.
 ///
 /// ```rust
 /// use tikv_client::Key;
 ///
 /// let static_str: &'static str = "TiKV";
-/// let from_static_str = Key::from(static_str);
+/// let from_static_str = Key::from(static_str.to_owned());
 ///
 /// let string: String = String::from(static_str);
 /// let from_string = Key::from(string);
@@ -104,12 +99,6 @@ impl From<String> for Key {
     }
 }
 
-impl From<&'static str> for Key {
-    fn from(v: &'static str) -> Key {
-        Key(v.as_bytes().to_vec())
-    }
-}
-
 impl Into<Vec<u8>> for Key {
     fn into(self) -> Vec<u8> {
         self.0

src/kv/kvpair.rs

@@ -9,9 +9,9 @@ use std::{fmt, str};
 ///
 /// ```rust
 /// # use tikv_client::{Key, Value, KvPair};
-/// let key = "key";
-/// let value = "value";
-/// let constructed = KvPair::new(key, value);
+/// let key = "key".to_owned();
+/// let value = "value".to_owned();
+/// let constructed = KvPair::new(key.clone(), value.clone());
 /// let from_tuple = KvPair::from((key, value));
 /// assert_eq!(constructed, from_tuple);
 /// ```

src/kv/mod.rs

@@ -1,14 +1,15 @@
 // Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0.
 use std::{fmt, u8};
 
+mod bound_range;
 mod key;
-pub use key::Key;
-mod value;
-pub use value::Value;
 mod kvpair;
+mod value;
+
+pub use bound_range::{BoundRange, ToOwnedRange};
+pub use key::Key;
 pub use kvpair::KvPair;
-mod bound_range;
-pub use bound_range::BoundRange;
+pub use value::Value;
 
 struct HexRepr<'a>(pub &'a [u8]);
 

src/kv/value.rs

@@ -13,18 +13,13 @@ use std::{fmt, str, u8};
 /// as valid `UTF-8` is not required. This means that the user is permitted to store any data they wish,
 /// as long as it can be represented by bytes. (Which is to say, pretty much anything!)
 ///
-/// This type also implements `From` for many types. With one exception, these are all done without
-/// reallocation. Using a `&'static str`, like many examples do for simplicity, has an internal
-/// allocation cost.
-///
-/// This type wraps around an owned value, so it should be treated it like `String` or `Vec<u8>`
-/// over a `&str` or `&[u8]`.
+/// This type wraps around an owned value, so it should be treated it like `String` or `Vec<u8>`.
 ///
 /// ```rust
 /// use tikv_client::Value;
 ///
 /// let static_str: &'static str = "TiKV";
-/// let from_static_str = Value::from(static_str);
+/// let from_static_str = Value::from(static_str.to_owned());
 ///
 /// let string: String = String::from(static_str);
 /// let from_string = Value::from(string);
@@ -73,12 +68,6 @@ impl From<String> for Value {
     }
 }
 
-impl From<&'static str> for Value {
-    fn from(v: &'static str) -> Value {
-        Value(v.as_bytes().to_vec())
-    }
-}
-
 impl Into<Vec<u8>> for Value {
     fn into(self) -> Vec<u8> {
         self.0

src/lib.rs

@@ -100,4 +100,4 @@ pub use crate::errors::ErrorKind;
 #[doc(inline)]
 pub use crate::errors::Result;
 #[doc(inline)]
-pub use crate::kv::{BoundRange, Key, KvPair, Value};
+pub use crate::kv::{BoundRange, Key, KvPair, ToOwnedRange, Value};

src/raw/client.rs

@@ -137,7 +137,7 @@ impl Client {
     /// # futures::executor::block_on(async {
     /// let connect = Client::connect(Config::default());
     /// let client = connect.await.unwrap();
-    /// let get_request = client.with_cf("write").get("foo");
+    /// let get_request = client.with_cf("write").get("foo".to_owned());
     /// # });
     /// ```
     pub fn with_cf(&self, cf: impl Into<ColumnFamily>) -> Client {
@@ -157,12 +157,12 @@ impl Client {
     ///
     /// ```rust,no_run
     /// # #![feature(async_await)]
-    /// # use tikv_client::{Config, raw::Client};
+    /// # use tikv_client::{Config, raw::Client, ToOwnedRange};
     /// # use futures::prelude::*;
     /// # futures::executor::block_on(async {
     /// let connect = Client::connect(Config::default());
     /// let client = connect.await.unwrap();
-    /// let scan_request = client.with_key_only(true).scan("TiKV"..="TiDB", 2);
+    /// let scan_request = client.with_key_only(true).scan(("TiKV"..="TiDB").to_owned(), 2);
     /// # });
     /// ```
     pub fn with_key_only(&self, key_only: bool) -> Client {