blobstore: enumeration ranges are inclusive

Summary:
Manifold enumeration ranges are inclusive.  Update documentation of options that
ultimately feed into this as such.

To avoid future confusion, use Rust's inclusive ranges to initialize these, and
remove the exclusive range option.

The fileblob implementation was actually performing exclusive checks at both
ends, rather than inclusive ones.  Correct this by implementing `RangeBounds`
and using `range.contains` instead.

Reviewed By: liubov-dmitrieva

Differential Revision: D28224481

fbshipit-source-id: 7244588271d7754d6c6820790cbd76574b296d7b

eden/mononoke/blobstore/fileblob/src/lib.rs

@@ -10,6 +10,7 @@
 use std::collections::HashSet;
 use std::convert::TryFrom;
 use std::fs::create_dir_all;
+use std::ops::RangeBounds;
 use std::path::{Path, PathBuf};
 use std::time::SystemTime;
 
@@ -233,7 +234,7 @@ impl BlobstoreKeySource for Fileblob {
                         let entry = entry.path().to_str();
                         if let Some(data) = entry {
                             let key = data.to_string();
-                            if key < range.end_key && key > range.begin_key {
+                            if range.contains(&key) {
                                 enum_data.keys.insert(key);
                             }
                         }

eden/mononoke/blobstore/src/lib.rs

@@ -22,7 +22,7 @@ use serde_derive::{Deserialize, Serialize};
 use std::collections::HashSet;
 use std::fmt;
 use std::io::Cursor;
-use std::ops::{Range, RangeFrom, RangeFull, RangeTo};
+use std::ops::{Bound, RangeBounds, RangeFrom, RangeFull, RangeInclusive, RangeToInclusive};
 use strum_macros::{AsRefStr, Display, EnumIter, EnumString, IntoStaticStr};
 use thiserror::Error;
 
@@ -430,9 +430,11 @@ pub trait BlobstoreKeySource: Blobstore {
     ) -> Result<BlobstoreEnumerationData>;
 }
 
+/// Range of keys.  The range is inclusive (both start and end key are
+/// included in the range), which matches Manifold behaviour.  If the key is
+/// empty then the range is unbounded on that end.
 #[derive(Debug, Clone, Serialize, Deserialize, Eq, PartialEq)]
 pub struct BlobstoreKeyRange {
-    // Should match manifold inclusiveness rules, please check and document.
     pub begin_key: String,
     pub end_key: String,
 }
@@ -450,17 +452,18 @@ pub enum BlobstoreKeyParam {
     Continuation(BlobstoreKeyToken),
 }
 
-impl From<Range<String>> for BlobstoreKeyParam {
-    fn from(range: Range<String>) -> Self {
+impl From<RangeInclusive<String>> for BlobstoreKeyParam {
+    fn from(range: RangeInclusive<String>) -> Self {
+        let (start, end) = range.into_inner();
         BlobstoreKeyParam::Start(BlobstoreKeyRange {
-            begin_key: range.start,
-            end_key: range.end,
+            begin_key: start,
+            end_key: end,
         })
     }
 }
 
-impl From<RangeTo<String>> for BlobstoreKeyParam {
-    fn from(range: RangeTo<String>) -> Self {
+impl From<RangeToInclusive<String>> for BlobstoreKeyParam {
+    fn from(range: RangeToInclusive<String>) -> Self {
         BlobstoreKeyParam::Start(BlobstoreKeyRange {
             begin_key: String::from(""),
             end_key: range.end,
@@ -486,6 +489,24 @@ impl From<RangeFull> for BlobstoreKeyParam {
     }
 }
 
+impl RangeBounds<String> for &BlobstoreKeyRange {
+    fn start_bound(&self) -> Bound<&String> {
+        if self.begin_key.is_empty() {
+            Bound::Unbounded
+        } else {
+            Bound::Included(&self.begin_key)
+        }
+    }
+
+    fn end_bound(&self) -> Bound<&String> {
+        if self.end_key.is_empty() {
+            Bound::Unbounded
+        } else {
+            Bound::Included(&self.end_key)
+        }
+    }
+}
+
 #[derive(Debug, Clone)]
 pub struct BlobstoreEnumerationData {
     pub keys: HashSet<String>,

eden/mononoke/cmds/populate_healer.rs

@@ -281,7 +281,7 @@ async fn get_resume_state(
             config.repo_id.id(),
             config.end_key.clone().unwrap_or_else(|| "\x7f".to_string()),
         );
-        State::from_init(Arc::new(BlobstoreKeyParam::from(start..end)))
+        State::from_init(Arc::new(BlobstoreKeyParam::from(start..=end)))
     };
 
     resume_state.map(move |resume_state| {