Bookmark support for is_ancestor queries in the API server

Summary:
Added support for queries which use bookmark names in place of node hashes. This involved:
* Creating a method `string_to_bookmark_changeset_id`, which takes a string, treats it as a bookmark, and tries to find the corresponding changeset id in the repo.
* Modifying the `is_ancestor call` in `MononokeRepoActor` to try to interpret the query strings as bookmarks if they can't be interpretted as node hashes.
* Introducing the `cloned` crate from `//common/rust` into the API server to make the above methods cleaner.
* Modifying the integration test to add a bookmark to the test repo and attempt querying using the bookmark name.

Reviewed By: fanzeyi

Differential Revision: D8976793

fbshipit-source-id: 3a2b58cac0fb80ee18fad8529bd58af5b54f85ef

apiserver/src/actor/repo.rs

@@ -4,19 +4,18 @@
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
-use std::str::FromStr;
 use std::sync::Arc;
 
 use actix::{Actor, Context, Handler};
-use failure::{err_msg, Error, Result, ResultExt};
-use futures::Future;
+use failure::{err_msg, Error, Result};
+use futures::{Future, IntoFuture};
 use futures_ext::BoxFuture;
 use slog::Logger;
 
 use api;
 use blobrepo::BlobRepo;
 use futures_ext::FutureExt;
-use mercurial_types::{HgNodeHash, RepositoryId};
+use mercurial_types::RepositoryId;
 use mercurial_types::manifest::Content;
 use metaconfig::repoconfig::RepoConfig;
 use metaconfig::repoconfig::RepoType::{BlobManifold, BlobRocks};
@@ -80,15 +79,33 @@ impl MononokeRepoActor {
         proposed_descendent: String,
     ) -> Result<BoxFuture<MononokeRepoResponse, Error>> {
         let mut genbfs = GenerationNumberBFS::new();
-        let src_hash = HgNodeHash::from_str(&proposed_descendent)
-            .with_context(|_| ErrorKind::InvalidInput(proposed_descendent))
-            .map_err(|e| -> Error { e.into() })?;
-        let dst_hash = HgNodeHash::from_str(&proposed_ancestor)
-            .with_context(|_| ErrorKind::InvalidInput(proposed_ancestor))
-            .map_err(|e| -> Error { e.into() })?;
-        Ok(genbfs
-            .query_reachability(self.repo.clone(), src_hash, dst_hash)
-            .map(move |answer| MononokeRepoResponse::IsAncestor { answer: answer })
+        let src_hash_maybe = FS::get_nodehash(&proposed_descendent);
+        let dst_hash_maybe = FS::get_nodehash(&proposed_ancestor);
+        let src_hash_future = src_hash_maybe.into_future().or_else({
+            cloned!(self.repo);
+            move |_| {
+                FS::string_to_bookmark_changeset_id(proposed_descendent, repo)
+                    .map(|node_cs| *node_cs.as_nodehash())
+            }
+        });
+        let dst_hash_future = dst_hash_maybe.into_future().or_else({
+            cloned!(self.repo);
+            move |_| {
+                FS::string_to_bookmark_changeset_id(proposed_ancestor, repo)
+                    .map(|node_cs| *node_cs.as_nodehash())
+            }
+        });
+
+        Ok(src_hash_future
+            .and_then(|src_hash| dst_hash_future.map(move |dst_hash| (src_hash, dst_hash)))
+            .and_then({
+                cloned!(self.repo);
+                move |(src_hash, dst_hash)| {
+                    genbfs
+                        .query_reachability(repo, src_hash, dst_hash)
+                        .map(move |answer| MononokeRepoResponse::IsAncestor { answer: answer })
+                }
+            })
             .from_err()
             .boxify())
     }

apiserver/src/from_string.rs

@@ -8,10 +8,16 @@
 
 use std::convert::TryFrom;
 use std::str::FromStr;
+use std::sync::Arc;
 
-use failure::{Result, ResultExt};
+use failure::{Error, Result, ResultExt};
+use futures::{Future, IntoFuture};
+use futures_ext::{BoxFuture, FutureExt};
 
-use mercurial_types::HgChangesetId;
+use api;
+use blobrepo::BlobRepo;
+use bookmarks::Bookmark;
+use mercurial_types::{HgChangesetId, HgNodeHash};
 use mononoke_types::MPath;
 
 use errors::ErrorKind;
@@ -27,3 +33,32 @@ pub fn get_changeset_id(changesetid: String) -> Result<HgChangesetId> {
         .with_context(|_| ErrorKind::InvalidInput(changesetid))
         .map_err(From::from)
 }
+
+pub fn get_bookmark(bookmark: String) -> Result<Bookmark> {
+    Bookmark::new(bookmark.clone())
+        .with_context(|_| ErrorKind::InvalidInput(bookmark))
+        .map_err(From::from)
+}
+pub fn get_nodehash(hash: &str) -> Result<HgNodeHash> {
+    HgNodeHash::from_str(hash)
+        .with_context(|_| ErrorKind::InvalidInput(hash.to_string()))
+        .map_err(From::from)
+}
+
+// interpret a string as a bookmark and find the corresponding changeset id.
+// this method doesn't consider that the string could be a node hash, so any caller
+// should do that check themselves, and if it fails, then attempt to use this method.
+pub fn string_to_bookmark_changeset_id(
+    node_string: String,
+    repo: Arc<BlobRepo>,
+) -> BoxFuture<HgChangesetId, Error> {
+    get_bookmark(node_string.clone())
+        .into_future()
+        .and_then({ move |bookmark| api::get_changeset_by_bookmark(repo, bookmark).from_err() })
+        .map_err({
+            cloned!(node_string);
+            |_| ErrorKind::InvalidInput(node_string)
+        })
+        .from_err()
+        .boxify()
+}

apiserver/src/main.rs

@@ -15,6 +15,8 @@ extern crate bytes;
 extern crate cachelib;
 extern crate clap;
 #[macro_use]
+extern crate cloned;
+#[macro_use]
 extern crate failure_ext as failure;
 extern crate futures;
 extern crate futures_ext;

mononoke-api/src/lib.rs

@@ -6,7 +6,11 @@
 
 #![deny(warnings)]
 
+#[macro_use]
+extern crate cloned;
+
 extern crate blobrepo;
+extern crate bookmarks;
 #[macro_use]
 extern crate failure_ext as failure;
 extern crate futures;
@@ -22,6 +26,7 @@ use failure::Error;
 use futures::Future;
 
 use blobrepo::BlobRepo;
+use bookmarks::Bookmark;
 use mercurial_types::{Changeset, HgChangesetId};
 use mercurial_types::manifest::Content;
 use mononoke_types::MPath;
@@ -44,3 +49,20 @@ pub fn get_content_by_path(
             content.ok_or_else(move || ErrorKind::NotFound(path.to_string()).into())
         })
 }
+
+pub fn get_changeset_by_bookmark(
+    repo: Arc<BlobRepo>,
+    bookmark: Bookmark,
+) -> impl Future<Item = HgChangesetId, Error = Error> {
+    repo.get_bookmark(&bookmark)
+        .map_err({
+            cloned!(bookmark);
+            move |_| ErrorKind::InvalidInput(bookmark.to_string()).into()
+        })
+        .and_then({
+            cloned!(bookmark);
+            move |node_cs_maybe| {
+                node_cs_maybe.ok_or_else(move || ErrorKind::NotFound(bookmark.to_string()).into())
+            }
+        })
+}

tests/integration/test-apiserver.t

@@ -29,6 +29,8 @@ setup testing repo for mononoke
   $ hg add branch2
   $ hg commit -ma
   $ COMMITB2=$(hg --debug id -i)
+  $ COMMITB2_BOOKMARK=B2
+  $ hg bookmark $COMMITB2_BOOKMARK
 
 import testing repo to mononoke
   $ cd ..
@@ -144,3 +146,16 @@ test reachability response on nonexistent nodes
   $ sslcurl -w "\n%{http_code}" $APISERVER/repo/is_ancestor/$COMMIT2/1234567890123456789012345678901234567890 2> /dev/null
   1234567890123456789012345678901234567890 not found
   404 (no-eol)
+
+test reachability on bookmarks
+  $ echo $COMMITB2_BOOKMARK
+  B2
+
+  $ sslcurl $APISERVER/repo/is_ancestor/$COMMIT2/$COMMITB2_BOOKMARK 2> /dev/null
+  true (no-eol)
+
+  $ sslcurl $APISERVER/repo/is_ancestor/$COMMITB2_BOOKMARK/$COMMITB2_BOOKMARK 2> /dev/null
+  true (no-eol)
+
+  $ sslcurl $APISERVER/repo/is_ancestor/$COMMITB2_BOOKMARK/$COMMIT2 2> /dev/null
+  false (no-eol)