resolve bookmarks

Summary: Apiserver now resolves bookmarks to the correct hash.

Reviewed By: StanislavGlebik

Differential Revision: D13860603

fbshipit-source-id: 5f7eef407db01bdd3e89211f647704faec473991

apiserver/client/src/lib.rs

@@ -40,10 +40,17 @@ impl MononokeAPIClient {
         &self,
         revision: String,
         path: String,
+        bookmark: bool,
     ) -> BoxFuture<Vec<u8>, apiserver_thrift::errors::Error> {
+        let rev = if bookmark {
+            MononokeRevision::bookmark(revision)
+        } else {
+            MononokeRevision::commit_hash(revision)
+        };
+
         self.inner.get_raw(&MononokeGetRawParams {
             repo: self.repo.clone(),
-            revision: MononokeRevision::commit_hash(revision),
+            revision: rev,
             path: path.into_bytes(),
         })
     }

apiserver/client/src/main.rs

@@ -21,9 +21,10 @@ use client::MononokeAPIClient;
 fn cat(client: MononokeAPIClient, matches: &ArgMatches) -> BoxFuture<(), ()> {
     let revision = matches.value_of("revision").expect("must provide revision");
     let path = matches.value_of("path").expect("must provide path");
+    let bookmark = matches.is_present("bookmark");
 
     client
-        .get_raw(revision.to_string(), path.to_string())
+        .get_raw(revision.to_string(), path.to_string(), bookmark)
         .map_err(|e| eprintln!("error: {}", e))
         .and_then(|r| String::from_utf8(r).map_err(|_| eprintln!("error: utf8 conversion failed.")))
         .map(|res| {
@@ -111,6 +112,12 @@ fn main() -> Result<(), ()> {
                         .value_name("PATH")
                         .help("path to the file you want to get")
                         .required(true),
+                )
+                .arg(
+                    Arg::with_name("bookmark")
+                        .short("b")
+                        .long("bookmark")
+                        .help("Revision is a bookmark"),
                 ),
         )
         .subcommand(

apiserver/if/apiserver.thrift

@@ -7,6 +7,7 @@ enum MononokeAPIExceptionKind {
   InvalidInput = 1,
   NotFound = 2,
   InternalError = 3,
+  BookmarkNotFound = 4,
 }
 
 exception MononokeAPIException {

apiserver/src/actor/repo.rs

@@ -20,13 +20,16 @@ use tokio::runtime::TaskExecutor;
 
 use api;
 use blobrepo::{get_sha256_alias, get_sha256_alias_key, BlobRepo};
+use bookmarks::Bookmark;
 use context::CoreContext;
 use mercurial_types::manifest::Content;
-use mercurial_types::HgManifestId;
+
+use mercurial_types::{HgChangesetId, HgManifestId};
 use metaconfig_types::RepoConfig;
 use metaconfig_types::RepoType::{BlobFiles, BlobRemote, BlobRocks, BlobSqlite};
-use mononoke_types::{FileContents, RepositoryId};
+
 use genbfs::GenerationNumberBFS;
+use mononoke_types::{FileContents, RepositoryId};
 use reachabilityindex::ReachabilityIndex;
 
 use errors::ErrorKind;
@@ -90,11 +93,26 @@ impl MononokeRepo {
         repo.map(|repo| Self { repo, executor })
     }
 
-    fn get_hash_from_revision(&self, revision: Revision) -> String {
+    fn get_hgchangesetid_from_revision(
+        &self,
+        ctx: CoreContext,
+        revision: Revision,
+    ) -> impl Future<Item = HgChangesetId, Error = Error> {
+        let repo = self.repo.clone();
         match revision {
-            Revision::CommitHash(hash) => hash,
-            //TODO: T39372106 resolve bookmark to hash
-            Revision::Bookmark(_) => panic!("Bookmarks not yet supported"),
+            Revision::CommitHash(hash) => FS::get_changeset_id(hash)
+                .into_future()
+                .from_err()
+                .left_future(),
+            Revision::Bookmark(bookmark) => Bookmark::new(bookmark)
+                .into_future()
+                .from_err()
+                .and_then(move |bookmark| {
+                    repo.get_bookmark(ctx, &bookmark).and_then(move |opt| {
+                        opt.ok_or_else(|| ErrorKind::BookmarkNotFound(bookmark.to_string()).into())
+                    })
+                })
+                .right_future(),
         }
     }
 
@@ -104,13 +122,11 @@ impl MononokeRepo {
         revision: Revision,
         path: String,
     ) -> BoxFuture<MononokeRepoResponse, ErrorKind> {
-        let hash = self.get_hash_from_revision(revision);
-
         let mpath = try_boxfuture!(FS::get_mpath(path.clone()));
-        let changesetid = try_boxfuture!(FS::get_changeset_id(hash));
-        let repo = self.repo.clone();
 
-        api::get_content_by_path(ctx, repo, changesetid, Some(mpath))
+        let repo = self.repo.clone();
+        self.get_hgchangesetid_from_revision(ctx.clone(), revision)
+            .and_then(|changesetid| api::get_content_by_path(ctx, repo, changesetid, Some(mpath)))
             .and_then(move |content| match content {
                 Content::File(content)
                 | Content::Executable(content)
@@ -208,19 +224,18 @@ impl MononokeRepo {
         revision: Revision,
         path: String,
     ) -> BoxFuture<MononokeRepoResponse, ErrorKind> {
-        let hash = self.get_hash_from_revision(revision);
         let mpath = if path.is_empty() {
             None
         } else {
             Some(try_boxfuture!(FS::get_mpath(path.clone())))
         };
-        let changesetid = try_boxfuture!(FS::get_changeset_id(hash));
-        let repo = self.repo.clone();
 
-        api::get_content_by_path(ctx, repo, changesetid, mpath)
+        let repo = self.repo.clone();
+        self.get_hgchangesetid_from_revision(ctx.clone(), revision)
+            .and_then(move |changesetid| api::get_content_by_path(ctx, repo, changesetid, mpath))
             .and_then(move |content| match content {
                 Content::Tree(tree) => Ok(tree),
-                _ => Err(ErrorKind::NotADirectory(path.to_string()).into())
+                _ => Err(ErrorKind::NotADirectory(path.to_string()).into()),
             })
             .map(|tree| {
                 tree.list()
@@ -258,11 +273,9 @@ impl MononokeRepo {
         ctx: CoreContext,
         revision: Revision,
     ) -> BoxFuture<MononokeRepoResponse, ErrorKind> {
-        let hash = self.get_hash_from_revision(revision);
-        let changesetid = try_boxfuture!(FS::get_changeset_id(hash));
-
-        self.repo
-            .get_changeset_by_changesetid(ctx, &changesetid)
+        let repo = self.repo.clone();
+        self.get_hgchangesetid_from_revision(ctx.clone(), revision)
+            .and_then(move |changesetid| repo.get_changeset_by_changesetid(ctx, &changesetid))
             .and_then(|changeset| changeset.try_into().map_err(From::from))
             .map(|changeset| MononokeRepoResponse::GetChangeset { changeset })
             .from_err()

apiserver/src/errors.rs

@@ -43,6 +43,7 @@ pub enum ErrorKind {
     InternalError(Error),
     LFSNotFound(String),
     NotADirectory(String),
+    BookmarkNotFound(String),
 }
 
 impl ErrorKind {
@@ -55,6 +56,7 @@ impl ErrorKind {
             InternalError(_) => StatusCode::INTERNAL_SERVER_ERROR,
             LFSNotFound(_) => StatusCode::NOT_FOUND,
             NotADirectory(_) => StatusCode::BAD_REQUEST,
+            BookmarkNotFound(_) => StatusCode::BAD_REQUEST,
         }
     }
 
@@ -63,16 +65,15 @@ impl ErrorKind {
         use errors::ErrorKind::*;
 
         match &self {
-            NotFound(..) | InvalidInput(..) | InternalError(_) | NotADirectory(_) => {
-                ErrorResponse::APIErrorResponse(APIErrorResponse {
-                    message: self.to_string(),
-                    causes: self
-                        .causes()
-                        .skip(1)
-                        .map(|cause| cause.to_string())
-                        .collect(),
-                })
-            }
+            NotFound(..) | InvalidInput(..) | InternalError(_) | NotADirectory(_)
+            | BookmarkNotFound(_) => ErrorResponse::APIErrorResponse(APIErrorResponse {
+                message: self.to_string(),
+                causes: self
+                    .causes()
+                    .skip(1)
+                    .map(|cause| cause.to_string())
+                    .collect(),
+            }),
             LFSNotFound(_) => ErrorResponse::LFSErrorResponse(LFSErrorResponse {
                 message: self.to_string(),
             }),
@@ -100,7 +101,7 @@ impl Fail for ErrorKind {
         match self {
             NotFound(_, cause) | InvalidInput(_, cause) => cause.as_ref().map(|e| e.as_fail()),
             InternalError(err) => Some(err.as_fail()),
-            LFSNotFound(_) | NotADirectory(_) => None,
+            LFSNotFound(_) | NotADirectory(_) | BookmarkNotFound(_) => None,
         }
     }
 }
@@ -115,6 +116,7 @@ impl fmt::Display for ErrorKind {
             InternalError(_0) => write!(f, "internal server error: {}", _0),
             LFSNotFound(_0) => write!(f, "{} is not found on LFS request", _0),
             NotADirectory(_0) => write!(f, "{} is not a directory", _0),
+            BookmarkNotFound(_0) => write!(f, "{} is not a valid bookmark", _0),
         }
     }
 }
@@ -223,6 +225,10 @@ impl From<ErrorKind> for MononokeAPIException {
                 kind: MononokeAPIExceptionKind::InvalidInput,
                 reason: e.to_string(),
             },
+            e @ BookmarkNotFound(_) => MononokeAPIException {
+                kind: MononokeAPIExceptionKind::BookmarkNotFound,
+                reason: e.to_string(),
+            },
         }
     }
 }