edenapi server: properly deserialize history requests

Summary:
While trying to repro a user report (https://fburl.com/jqvm320o), I ran into a
new hg error: P151186623, i.e.:

```
KeyError: 'Key not found HgId(Key { path: RepoPathBuf("fbcode/thrift/facebook/test/py/TARGETS"), hgid: HgId("55713728544d5955703d604299d77bb1ed50c62d") })'
```

After some investigation (and adding a lot of prints), I noticed that this
was trying to query the EdenAPI server for this filenode. That request should
succeed, given Mononoke knows about this filenode:

```
[torozco@devbig051]~/fbcode % mononoke_exec mononoke_admin fbsource --use-mysql-client filenodes by-id fbcode/thrift/facebook/test/py/TARGETS 55713728544d5955703d604299d77bb1ed50c62d
mononoke_exec: Using config stage prod (set MONONOKE_EXEC_STAGE= to override)
I1126 08:10:02.089614 3697890 [main] eden/mononoke/cmdlib/src/args/mod.rs:1097] using repo "fbsource" repoid RepositoryId(2100)
I1126 08:10:02.995172 3697890 [main] eden/mononoke/cmds/admin/filenodes.rs:137] Filenode: HgFileNodeId(HgNodeHash(Sha1(55713728544d5955703d604299d77bb1ed50c62d)))
I1126 08:10:02.995282 3697890 [main] eden/mononoke/cmds/admin/filenodes.rs:138] -- path: FilePath(MPath("fbcode/thrift/facebook/test/py/TARGETS"))
I1126 08:10:02.995341 3697890 [main] eden/mononoke/cmds/admin/filenodes.rs:139] -- p1: Some(HgFileNodeId(HgNodeHash(Sha1(ccb76adc7db0fc4a395be066fe5464873cdf57e7))))
I1126 08:10:02.995405 3697890 [main] eden/mononoke/cmds/admin/filenodes.rs:140] -- p2: None
I1126 08:10:02.995449 3697890 [main] eden/mononoke/cmds/admin/filenodes.rs:141] -- copyfrom: None
I1126 08:10:02.995486 3697890 [main] eden/mononoke/cmds/admin/filenodes.rs:142] -- linknode: HgChangesetId(HgNodeHash(Sha1(dfe46f7d6cd8bc9b03af8870aca521b1801126f0)))
```

Turns out, the success rate for this method is actually 0% — out of thousands of requests,
not a single one succeeded :(

https://fburl.com/scuba/edenapi_server/cma3c3j0

The root cause here is that the server side is not properly deserializing
requests (actually finding that was a problem of its own, I filed T80406893 for this).

If you manage to get it to print the errors, it says:

```
{"message":"Deserialization failed: missing field `path`","request_id":"f97e2c7c-a432-4696-9a4e-538ed0db0418"}
```

The reason for this is that the server side tries to deserialize the request
as if it was a `WireHistoryRequest`, but actually it's a `HistoryRequest`, so
all the fields have different names (we use numbers in `WireHistoryRequest`).

This diff fixes that. I also introduced a helper method to make this a little
less footgun-y and double-checked the other callsites. There is one callsite
right now that looks like it might be broken (the commit one), but I couldn't
find the client side interface for this (I'm guessing it's not implemented
yet), so for now I left it as-is.

Reviewed By: StanislavGlebik

Differential Revision: D25187639

fbshipit-source-id: fa993579666dda762c0d71ccb56a646c20aee606

eden/mononoke/edenapi_server/src/handlers/commit.rs

@@ -53,7 +53,7 @@ pub async fn location_to_hash(state: &mut State) -> Result<impl TryIntoResponse,
 
     let hg_repo_ctx = get_repo(&sctx, &rctx, &params.repo).await?;
 
-    let request: CommitLocationToHashRequest = parse_cbor_request(state).await?;
+    let request: CommitLocationToHashRequest = parse_cbor_request(state).await?; // TODO: Should this use parse_wire_request?
     let hgid_list = request
         .locations
         .into_iter()

eden/mononoke/edenapi_server/src/handlers/complete_trees.rs

@@ -12,7 +12,7 @@ use gotham_derive::{StateData, StaticResponseExtender};
 use serde::Deserialize;
 
 use edenapi_types::{
-    wire::{ToApi, ToWire, WireCompleteTreeRequest},
+    wire::{ToWire, WireCompleteTreeRequest},
     CompleteTreeRequest, EdenApiServerError, TreeEntry,
 };
 use gotham_ext::{error::HttpError, response::TryIntoResponse};
@@ -26,7 +26,7 @@ use types::Key;
 use crate::context::ServerContext;
 use crate::errors::ErrorKind;
 use crate::middleware::RequestContext;
-use crate::utils::{cbor_stream, get_repo, parse_cbor_request, to_hg_path, to_mononoke_path};
+use crate::utils::{cbor_stream, get_repo, parse_wire_request, to_hg_path, to_mononoke_path};
 
 use super::{EdenApiMethod, HandlerInfo};
 
@@ -44,13 +44,7 @@ pub async fn complete_trees(state: &mut State) -> Result<impl TryIntoResponse, H
     let sctx = ServerContext::borrow_from(state);
 
     let repo = get_repo(&sctx, &rctx, &params.repo).await?;
-    let request: WireCompleteTreeRequest = parse_cbor_request(state).await?;
-    let request: CompleteTreeRequest = match request.to_api() {
-        Ok(r) => r,
-        Err(e) => {
-            return Err(HttpError::e400(e));
-        }
-    };
+    let request = parse_wire_request::<WireCompleteTreeRequest>(state).await?;
 
     Ok(cbor_stream(
         rctx,

eden/mononoke/edenapi_server/src/handlers/files.rs

@@ -12,7 +12,7 @@ use gotham_derive::{StateData, StaticResponseExtender};
 use serde::Deserialize;
 
 use edenapi_types::{
-    wire::{ToApi, ToWire, WireFileRequest},
+    wire::{ToWire, WireFileRequest},
     FileEntry, FileRequest,
 };
 use gotham_ext::{error::HttpError, response::TryIntoResponse};
@@ -23,7 +23,7 @@ use types::Key;
 use crate::context::ServerContext;
 use crate::errors::ErrorKind;
 use crate::middleware::RequestContext;
-use crate::utils::{cbor_stream, get_repo, parse_cbor_request};
+use crate::utils::{cbor_stream, get_repo, parse_wire_request};
 
 use super::{EdenApiMethod, HandlerInfo};
 
@@ -45,13 +45,7 @@ pub async fn files(state: &mut State) -> Result<impl TryIntoResponse, HttpError>
     let sctx = ServerContext::borrow_from(state);
 
     let repo = get_repo(&sctx, &rctx, &params.repo).await?;
-    let request: WireFileRequest = parse_cbor_request(state).await?;
-    let request: FileRequest = match request.to_api() {
-        Ok(r) => r,
-        Err(e) => {
-            return Err(HttpError::e400(e));
-        }
-    };
+    let request = parse_wire_request::<WireFileRequest>(state).await?;
 
     Ok(cbor_stream(
         rctx,

eden/mononoke/edenapi_server/src/handlers/history.rs

@@ -17,7 +17,9 @@ use gotham_derive::{StateData, StaticResponseExtender};
 use serde::Deserialize;
 
 use cloned::cloned;
-use edenapi_types::{HistoryRequest, HistoryResponseChunk, ToWire, WireHistoryEntry};
+use edenapi_types::{
+    wire::WireHistoryRequest, HistoryRequest, HistoryResponseChunk, ToWire, WireHistoryEntry,
+};
 use gotham_ext::{error::HttpError, response::TryIntoResponse};
 use mercurial_types::{HgFileNodeId, HgNodeHash};
 use mononoke_api::hg::HgRepoContext;
@@ -26,7 +28,7 @@ use types::Key;
 use crate::context::ServerContext;
 use crate::errors::ErrorKind;
 use crate::middleware::RequestContext;
-use crate::utils::{cbor_stream, get_repo, parse_cbor_request, to_mpath};
+use crate::utils::{cbor_stream, get_repo, parse_wire_request, to_mpath};
 
 use super::{EdenApiMethod, HandlerInfo};
 
@@ -49,7 +51,7 @@ pub async fn history(state: &mut State) -> Result<impl TryIntoResponse, HttpErro
     let sctx = ServerContext::borrow_from(state);
 
     let repo = get_repo(&sctx, &rctx, &params.repo).await?;
-    let request = parse_cbor_request(state).await?;
+    let request = parse_wire_request::<WireHistoryRequest>(state).await?;
 
     Ok(cbor_stream(
         rctx,

eden/mononoke/edenapi_server/src/handlers/trees.rs

@@ -12,7 +12,7 @@ use gotham_derive::{StateData, StaticResponseExtender};
 use serde::Deserialize;
 
 use edenapi_types::{
-    wire::{ToApi, ToWire, WireTreeRequest},
+    wire::{ToWire, WireTreeRequest},
     EdenApiServerError, FileMetadata, TreeChildEntry, TreeEntry, TreeRequest,
 };
 use gotham_ext::{error::HttpError, response::TryIntoResponse};
@@ -24,7 +24,7 @@ use types::{Key, RepoPathBuf};
 use crate::context::ServerContext;
 use crate::errors::ErrorKind;
 use crate::middleware::RequestContext;
-use crate::utils::{cbor_stream, get_repo, parse_cbor_request};
+use crate::utils::{cbor_stream, get_repo, parse_wire_request};
 
 use super::{EdenApiMethod, HandlerInfo};
 
@@ -47,13 +47,7 @@ pub async fn trees(state: &mut State) -> Result<impl TryIntoResponse, HttpError>
     let sctx = ServerContext::borrow_from(state);
 
     let repo = get_repo(&sctx, &rctx, &params.repo).await?;
-    let request: WireTreeRequest = parse_cbor_request(state).await?;
-    let request: TreeRequest = match request.to_api() {
-        Ok(r) => r,
-        Err(e) => {
-            return Err(HttpError::e400(e));
-        }
-    };
+    let request = parse_wire_request::<WireTreeRequest>(state).await?;
 
     Ok(cbor_stream(
         rctx,

eden/mononoke/edenapi_server/src/utils/cbor.rs

@@ -9,6 +9,7 @@
 
 use anyhow::{Context, Error};
 use bytes::Bytes;
+use edenapi_types::ToApi;
 use futures::prelude::*;
 use gotham::state::State;
 use mime::Mime;
@@ -60,3 +61,13 @@ pub async fn parse_cbor_request<R: DeserializeOwned>(state: &mut State) -> Resul
         .context(ErrorKind::DeserializationFailed)
         .map_err(HttpError::e400)
 }
+
+pub async fn parse_wire_request<R: DeserializeOwned + ToApi>(
+    state: &mut State,
+) -> Result<<R as ToApi>::Api, HttpError>
+where
+    <R as ToApi>::Error: Send + Sync + 'static + std::error::Error,
+{
+    let cbor = parse_cbor_request::<R>(state).await?;
+    cbor.to_api().map_err(HttpError::e400)
+}

eden/mononoke/edenapi_server/src/utils/mod.rs

@@ -21,7 +21,7 @@ use crate::middleware::RequestContext;
 pub mod cbor;
 pub mod convert;
 
-pub use cbor::{cbor_mime, cbor_stream, parse_cbor_request, to_cbor_bytes};
+pub use cbor::{cbor_mime, cbor_stream, parse_cbor_request, parse_wire_request, to_cbor_bytes};
 pub use convert::{to_hg_path, to_mononoke_path, to_mpath};
 
 pub async fn get_repo(

eden/mononoke/tests/integration/test-edenapi-server-history.t

@@ -48,19 +48,19 @@ Create and send file data request.
   > }
   > EOF
   Reading from stdin
-  Generated request: HistoryRequest {
+  Generated request: WireHistoryRequest {
       keys: [
-          Key {
-              path: RepoPathBuf(
+          WireKey {
+              path: WireRepoPathBuf(
                   "test.txt",
               ),
-              hgid: HgId("596c909aab726d7f8b3766795239cd20ede8e125"),
+              hgid: WireHgId("596c909aab726d7f8b3766795239cd20ede8e125"),
           },
-          Key {
-              path: RepoPathBuf(
+          WireKey {
+              path: WireRepoPathBuf(
                   "copy.txt",
               ),
-              hgid: HgId("672343a6daad357b926cd84a5a44a011ad029e5f"),
+              hgid: WireHgId("672343a6daad357b926cd84a5a44a011ad029e5f"),
           },
       ],
       length: None,

eden/scm/lib/edenapi/tools/make_req/src/main.rs

@@ -52,7 +52,7 @@ fn main() -> Result<()> {
     match Command::from_args() {
         Command::File(args) => make_req::<FileRequest>(args),
         Command::Tree(args) => make_req::<TreeRequest>(args),
-        Command::History(args) => make_req_wire::<HistoryRequest>(args),
+        Command::History(args) => make_req::<HistoryRequest>(args),
         Command::CompleteTree(args) => make_req::<CompleteTreeRequest>(args),
         Command::CommitRevlogData(args) => make_req_wire::<CommitRevlogDataRequest>(args),
         Command::CommitLocationToHash(args) => make_req_wire::<CommitLocationToHashRequest>(args),