diff --git a/eden/mononoke/edenapi_server/src/errors.rs b/eden/mononoke/edenapi_server/src/errors.rs index 2caa1f9ee7..83067fa4b1 100644 --- a/eden/mononoke/edenapi_server/src/errors.rs +++ b/eden/mononoke/edenapi_server/src/errors.rs @@ -54,8 +54,8 @@ pub enum ErrorKind { DataFetchFailed(Key), #[error("Failed to fetch history for key: {0:?}")] HistoryFetchFailed(Key), - #[error("Subtree request failed")] - SubtreeRequestFailed, + #[error("Complete tree request failed")] + CompleteTreeRequestFailed, } /// Extension trait for converting `MononokeError`s into `HttpErrors`. diff --git a/eden/mononoke/edenapi_server/src/handlers/subtree.rs b/eden/mononoke/edenapi_server/src/handlers/complete_trees.rs similarity index 66% rename from eden/mononoke/edenapi_server/src/handlers/subtree.rs rename to eden/mononoke/edenapi_server/src/handlers/complete_trees.rs index 8307ed80d8..5745a81817 100644 --- a/eden/mononoke/edenapi_server/src/handlers/subtree.rs +++ b/eden/mononoke/edenapi_server/src/handlers/complete_trees.rs @@ -11,7 +11,7 @@ use gotham::state::{FromState, State}; use gotham_derive::{StateData, StaticResponseExtender}; use serde::Deserialize; -use edenapi_types::{DataEntry, TreeRequest}; +use edenapi_types::{CompleteTreeRequest, DataEntry}; use gotham_ext::{error::HttpError, response::TryIntoResponse}; use mercurial_types::{HgManifestId, HgNodeHash}; use mononoke_api::{ @@ -26,36 +26,43 @@ use crate::middleware::RequestContext; use crate::utils::{cbor_stream, get_repo, parse_cbor_request, to_hg_path, to_mononoke_path}; #[derive(Debug, Deserialize, StateData, StaticResponseExtender)] -pub struct SubTreeParams { +pub struct CompleteTreesParams { repo: String, } -pub async fn subtree(state: &mut State) -> Result { +pub async fn complete_trees(state: &mut State) -> Result { let rctx = RequestContext::borrow_from(state); let sctx = ServerContext::borrow_from(state); - let params = SubTreeParams::borrow_from(state); + let params = CompleteTreesParams::borrow_from(state); let repo = get_repo(&sctx, &rctx, ¶ms.repo).await?; let request = parse_cbor_request(state).await?; - Ok(cbor_stream(get_complete_subtree(&repo, request)?)) + Ok(cbor_stream(fetch_trees_under_path(&repo, request)?)) } -/// Fetch all of the nodes for the subtree under the specified -/// path for the specified root node versions of this path. -/// The client may optionally specify a list of root versions -/// for the path that it already has, and any nodes in these -/// older subtrees will be filtered out if present in the -/// requested subtrees. +/// Fetch the complete tree under the specified path. +/// +/// This function returns all tree nodes underneath (and including) +/// a given directory in the repo. Multiple versions of the +/// root directory can be specified (via their manifest IDs); +/// all tree nodes reachable from any of these root nodes will +/// be fetched. +/// +/// Optionally, the caller can specify a list of versions of +/// the root directory that are already present on the client. +/// It is assumed that the client possess the *complete tree* +/// underneath each of these versions. Any tree node reachable +/// from any of these root nodes will not be fetched. /// /// This is essentially an HTTP-based implementation of Mercurial's -/// `gettreepack` wire protocol command, and is generally considered +/// `gettreepack` wire protocol command. This is generally considered /// a fairly expensive way to request trees. When possible, clients -/// should prefer explicitly request individual tree nodes via the +/// should prefer to request individual tree nodes as needed via the /// more lightweight `/trees` endpoint. -fn get_complete_subtree( +fn fetch_trees_under_path( repo: &HgRepoContext, - request: TreeRequest, + request: CompleteTreeRequest, ) -> Result>, HttpError> { let path = to_mononoke_path(request.rootdir).map_err(HttpError::e400)?; @@ -74,7 +81,7 @@ fn get_complete_subtree( let stream = repo .trees_under_path(path, root_nodes, base_nodes, request.depth) .err_into::() - .map_err(|e| e.context(ErrorKind::SubtreeRequestFailed)) + .map_err(|e| e.context(ErrorKind::CompleteTreeRequestFailed)) .and_then(move |(tree, path)| async { data_entry_for_tree(tree, path) }); Ok(stream) diff --git a/eden/mononoke/edenapi_server/src/handlers/mod.rs b/eden/mononoke/edenapi_server/src/handlers/mod.rs index 6e582c1645..f6c0ae3069 100644 --- a/eden/mononoke/edenapi_server/src/handlers/mod.rs +++ b/eden/mononoke/edenapi_server/src/handlers/mod.rs @@ -23,10 +23,10 @@ use gotham_ext::response::build_response; use crate::context::ServerContext; +mod complete_trees; mod data; mod history; mod repos; -mod subtree; /// Macro to create a Gotham handler function from an async function. /// @@ -54,8 +54,8 @@ macro_rules! define_handler { define_handler!(repos_handler, repos::repos); define_handler!(files_handler, data::files); define_handler!(trees_handler, data::trees); +define_handler!(complete_trees_handler, complete_trees::complete_trees); define_handler!(history_handler, history::history); -define_handler!(subtree_handler, subtree::subtree); fn health_handler(state: State) -> (State, &'static str) { if ServerContext::borrow_from(&state).will_exit() { @@ -80,13 +80,13 @@ pub fn build_router(ctx: ServerContext) -> Router { .post("/:repo/trees") .with_path_extractor::() .to(trees_handler); + route + .post("/:repo/trees/complete") + .with_path_extractor::() + .to(complete_trees_handler); route .post("/:repo/history") .with_path_extractor::() .to(history_handler); - route - .post("/:repo/subtree") - .with_path_extractor::() - .to(subtree_handler); }) } diff --git a/eden/mononoke/hgproto/Cargo.toml b/eden/mononoke/hgproto/Cargo.toml index a0a36bea55..8dc44d5d99 100644 --- a/eden/mononoke/hgproto/Cargo.toml +++ b/eden/mononoke/hgproto/Cargo.toml @@ -7,7 +7,6 @@ license = "GPLv2+" include = ["src/**/*.rs"] [dependencies] -edenapi_types = { path = "../../scm/lib/edenapi/types" } mercurial_bundles = { path = "../mercurial/bundles" } mercurial_types = { path = "../mercurial/types" } mononoke_types = { path = "../mononoke_types" } diff --git a/eden/mononoke/hgproto/src/lib.rs b/eden/mononoke/hgproto/src/lib.rs index b19c33f754..41c3e5a429 100644 --- a/eden/mononoke/hgproto/src/lib.rs +++ b/eden/mononoke/hgproto/src/lib.rs @@ -13,13 +13,10 @@ #![deny(warnings)] -use anyhow::Error; use bytes_old::Bytes; -use edenapi_types::TreeRequest; use mercurial_types::{HgChangesetId, HgManifestId}; use mononoke_types::MPath; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; -use std::convert::TryFrom; use std::fmt::{self, Debug}; use std::sync::Mutex; @@ -180,31 +177,6 @@ pub struct GettreepackArgs { pub depth: Option, } -impl TryFrom for GettreepackArgs { - type Error = Error; - - fn try_from(req: TreeRequest) -> Result { - let mfnodes = req - .mfnodes - .into_iter() - .map(|node| HgManifestId::new(node.into())) - .collect(); - let basemfnodes = req - .basemfnodes - .into_iter() - .map(|node| HgManifestId::new(node.into())) - .collect(); - - Ok(Self { - rootdir: MPath::new_opt(req.rootdir)?, - mfnodes, - basemfnodes, - directories: Vec::new(), - depth: req.depth, - }) - } -} - #[derive(Debug)] pub enum Response { Batch(Vec), diff --git a/eden/mononoke/tests/integration/test-edenapi-server-subtree.t b/eden/mononoke/tests/integration/test-edenapi-server-complete-trees.t similarity index 92% rename from eden/mononoke/tests/integration/test-edenapi-server-subtree.t rename to eden/mononoke/tests/integration/test-edenapi-server-complete-trees.t index 108991c737..0dae47d328 100644 --- a/eden/mononoke/tests/integration/test-edenapi-server-subtree.t +++ b/eden/mononoke/tests/integration/test-edenapi-server-complete-trees.t @@ -15,7 +15,7 @@ Initialize test repo. $ cd repo-hg $ setup_hg_server -Create a nested directory structure. +Create a nested directory structure. $ mkdir -p a{1,2}/b{1,2}/c{1,2} $ echo "1" | tee a{1,2}/{file,b{1,2}/{file,c{1,2}/file}} > /dev/null $ tree @@ -81,7 +81,7 @@ Start up EdenAPI server. $ setup_mononoke_config $ start_edenapi_server -Create and send tree request. +Create and send complete tree request. $ edenapi_make_req tree > req.cbor < { > "rootdir": "", @@ -91,7 +91,7 @@ Create and send tree request. > } > EOF Reading from stdin - Generated request: TreeRequest { + Generated request: CompleteTreeRequest { rootdir: RepoPathBuf( "", ), @@ -106,11 +106,11 @@ Create and send tree request. 2, ), } - $ sslcurl -s "$EDENAPI_URI/repo/subtree" -d@req.cbor > res.cbor + $ sslcurl -s "$EDENAPI_URI/repo/trees/complete" -d@req.cbor > res.cbor Confirm that the response contains only directories whose files were modified, and that each directory appears twice since the -files therein were modified in both commits. +files therein were modified in both commits. $ edenapi_read_res data ls res.cbor Reading from file: "res.cbor" 3d866afaa8cdb847e3800fef742c1fe9e741f75f diff --git a/eden/scm/lib/edenapi/src/curl.rs b/eden/scm/lib/edenapi/src/curl.rs index f2096e029c..bc18cd45d5 100644 --- a/eden/scm/lib/edenapi/src/curl.rs +++ b/eden/scm/lib/edenapi/src/curl.rs @@ -21,8 +21,8 @@ use serde_cbor::Deserializer; use url::Url; use edenapi_types::{ - DataEntry, DataRequest, DataResponse, HistoryEntry, HistoryRequest, HistoryResponse, - TreeRequest, Validity, WireHistoryEntry, + CompleteTreeRequest, DataEntry, DataRequest, DataResponse, HistoryEntry, HistoryRequest, + HistoryResponse, Validity, WireHistoryEntry, }; use types::{HgId, Key, RepoPathBuf}; @@ -279,7 +279,12 @@ impl EdenApi for EdenApiCurlClient { } let creds = self.creds.as_ref(); - let requests = vec![TreeRequest::new(rootdir, mfnodes, basemfnodes, depth)]; + let requests = vec![CompleteTreeRequest::new( + rootdir, + mfnodes, + basemfnodes, + depth, + )]; let mut responses = Vec::new(); let stats = if self.stream_trees { diff --git a/eden/scm/lib/edenapi/tools/make_req/src/main.rs b/eden/scm/lib/edenapi/tools/make_req/src/main.rs index 3876af8d3c..bf8372395d 100644 --- a/eden/scm/lib/edenapi/tools/make_req/src/main.rs +++ b/eden/scm/lib/edenapi/tools/make_req/src/main.rs @@ -24,7 +24,7 @@ use anyhow::{anyhow, ensure, Result}; use serde_json::Value; use structopt::StructOpt; -use edenapi_types::{DataRequest, HistoryRequest, TreeRequest}; +use edenapi_types::{CompleteTreeRequest, DataRequest, HistoryRequest}; use types::{HgId, Key, RepoPathBuf}; #[derive(Debug, StructOpt)] @@ -118,14 +118,13 @@ fn parse_history_req(json: &Value) -> Result { Ok(HistoryRequest { keys, length }) } -/// Parse a `TreeRequest` from JSON. +/// Parse a `CompleteTreeRequest` from JSON. /// /// The request is represented as a JSON object containing the fields -/// needed for a "gettreepack"-style tree request. Note that most -/// EdenAPI tree requests are actually performed using a `DataRequest` -/// for the desired tree nodes; `TreeRequest`s are only used in situations -/// where behavior similar to Mercurial's `gettreepack` wire protocol -/// command is desired. +/// needed for a "gettreepack"-style complete tree request. Note that +/// it is generally preferred to request trees using a `DataRequest` +/// for the desired tree nodes, as this is a lot less expensive than +/// fetching complete trees. /// /// Example request: /// @@ -144,7 +143,7 @@ fn parse_history_req(json: &Value) -> Result { /// } /// ``` /// -fn parse_tree_req(json: &Value) -> Result { +fn parse_tree_req(json: &Value) -> Result { let obj = json .as_object() .ok_or_else(|| anyhow!("input must be a JSON object"))?; @@ -172,7 +171,7 @@ fn parse_tree_req(json: &Value) -> Result { .and_then(|d| d.as_u64()) .map(|d| d as usize); - Ok(TreeRequest { + Ok(CompleteTreeRequest { rootdir, mfnodes, basemfnodes, diff --git a/eden/scm/lib/edenapi/types/src/lib.rs b/eden/scm/lib/edenapi/types/src/lib.rs index bd440bc1de..56155df878 100644 --- a/eden/scm/lib/edenapi/types/src/lib.rs +++ b/eden/scm/lib/edenapi/types/src/lib.rs @@ -15,4 +15,4 @@ pub use crate::data::{DataEntry, DataRequest, DataResponse, Validity}; pub use crate::history::{ HistoryEntry, HistoryRequest, HistoryResponse, HistoryResponseChunk, WireHistoryEntry, }; -pub use crate::tree::TreeRequest; +pub use crate::tree::CompleteTreeRequest; diff --git a/eden/scm/lib/edenapi/types/src/tree.rs b/eden/scm/lib/edenapi/types/src/tree.rs index 5f070599a8..c719e63dd5 100644 --- a/eden/scm/lib/edenapi/types/src/tree.rs +++ b/eden/scm/lib/edenapi/types/src/tree.rs @@ -18,17 +18,16 @@ use types::{hgid::HgId, path::RepoPathBuf}; /// In general, trees can be requested from the API server using a `DataRequest` /// containing the keys of the desired tree nodes. /// -/// In all cases, trees will be returned in a `DataResponse`, so there is no -/// `TreeResponse` type to accompany `TreeRequest`. +/// In all cases, trees will be returned in a `DataResponse`. #[derive(Debug, Serialize, Deserialize)] -pub struct TreeRequest { +pub struct CompleteTreeRequest { pub rootdir: RepoPathBuf, pub mfnodes: Vec, pub basemfnodes: Vec, pub depth: Option, } -impl TreeRequest { +impl CompleteTreeRequest { pub fn new( rootdir: RepoPathBuf, mfnodes: Vec,