From b932f60c2b0a020fb691b25e9f07b1701da62707 Mon Sep 17 00:00:00 2001 From: FintanH Date: Wed, 15 Jul 2020 16:59:59 +0100 Subject: [PATCH 01/13] Use typed RadUrn params --- proxy/src/http/source.rs | 57 ++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/proxy/src/http/source.rs b/proxy/src/http/source.rs index 1cc3a98116..ff4bfa46dd 100644 --- a/proxy/src/http/source.rs +++ b/proxy/src/http/source.rs @@ -70,7 +70,7 @@ where .and(http::with_peer(peer)) .and(http::with_shared(registry)) .and(http::with_store(store)) - .and(document::param::( + .and(document::param::( "project_id", "ID of the project the blob is part of", )) @@ -101,7 +101,7 @@ fn branches_filter( path("branches") .and(warp::get()) .and(http::with_peer(peer)) - .and(document::param::( + .and(document::param::( "project_id", "ID of the project the blob is part of", )) @@ -127,7 +127,7 @@ fn commit_filter( path("commit") .and(warp::get()) .and(http::with_peer(peer)) - .and(document::param::( + .and(document::param::( "project_id", "ID of the project the blob is part of", )) @@ -151,7 +151,7 @@ fn commits_filter( path("commits") .and(warp::get()) .and(http::with_peer(peer)) - .and(document::param::( + .and(document::param::( "project_id", "ID of the project the blob is part of", )) @@ -210,7 +210,7 @@ where path("revisions") .and(warp::get()) .and(http::with_peer(Arc::clone(&peer))) - .and(document::param::( + .and(document::param::( "project_id", "ID of the project the blob is part of", )) @@ -240,7 +240,7 @@ fn tags_filter( path("tags") .and(warp::get()) .and(http::with_peer(peer)) - .and(document::param::( + .and(document::param::( "project_id", "ID of the project the blob is part of", )) @@ -264,7 +264,7 @@ fn tree_filter( path("tree") .and(warp::get()) .and(http::with_peer(peer)) - .and(document::param::( + .and(document::param::( "project_id", "ID of the project the blob is part of", )) @@ -299,7 +299,6 @@ mod handler { use radicle_surf::vcs::git::{self, BranchType}; use crate::coco; - use crate::error::Error; use crate::http; use crate::registry; use crate::session; @@ -309,7 +308,7 @@ mod handler { api: Arc>, registry: http::Shared, store: http::Shared, - project_urn: String, + project_urn: coco::Urn, super::BlobQuery { path, peer_id, @@ -325,8 +324,7 @@ mod handler { let session = session::current(Arc::clone(&api), &*registry, &store).await?; let api = api.lock().await; - let urn = project_urn.parse().map_err(Error::from)?; - let project = coco::get_project(&*api, &urn)?; + let project = coco::get_project(&*api, &project_urn)?; let default_branch = match peer_id { Some(peer_id) if peer_id != *api.peer_id() => { @@ -340,7 +338,7 @@ mod handler { } else { None }; - let blob = coco::with_browser(&*api, &urn, |mut browser| { + let blob = coco::with_browser(&*api, &project_urn, |mut browser| { coco::blob(&mut browser, default_branch, revision, &path, theme) })?; @@ -350,11 +348,10 @@ mod handler { /// Fetch the list [`coco::Branch`]. pub async fn branches( peer: Arc>, - project_urn: String, + project_urn: coco::Urn, ) -> Result { let peer = peer.lock().await; - let urn = project_urn.parse().map_err(Error::from)?; - let branches = coco::with_browser(&peer, &urn, |browser| { + let branches = coco::with_browser(&peer, &project_urn, |browser| { coco::branches(browser, Some(BranchType::Local)) })?; @@ -364,13 +361,13 @@ mod handler { /// Fetch a [`coco::Commit`]. pub async fn commit( api: Arc>, - project_urn: String, + project_urn: coco::Urn, sha1: String, ) -> Result { let api = api.lock().await; - let urn = project_urn.parse().map_err(Error::from)?; - let commit = - coco::with_browser(&api, &urn, |mut browser| coco::commit(&mut browser, &sha1))?; + let commit = coco::with_browser(&api, &project_urn, |mut browser| { + coco::commit(&mut browser, &sha1) + })?; Ok(reply::json(&commit)) } @@ -378,12 +375,11 @@ mod handler { /// Fetch the list of [`coco::Commit`] from a branch. pub async fn commits( api: Arc>, - project_urn: String, + project_urn: coco::Urn, query: super::CommitsQuery, ) -> Result { let api = api.lock().await; - let urn = project_urn.parse().map_err(Error::from)?; - let commits = coco::with_browser(&api, &urn, |mut browser| { + let commits = coco::with_browser(&api, &project_urn, |mut browser| { coco::commits(&mut browser, query.into()) })?; @@ -400,12 +396,11 @@ mod handler { /// Fetch the list [`coco::Branch`] and [`coco::Tag`]. pub async fn revisions( peer: Arc>, - project_urn: String, + project_urn: coco::Urn, owner: coco::User, ) -> Result { - let urn = project_urn.parse().map_err(Error::from)?; let peer = &*peer.lock().await; - let revisions: Vec<_> = coco::revisions(peer, &owner, &urn)?.into(); + let revisions: Vec<_> = coco::revisions(peer, &owner, &project_urn)?.into(); Ok(reply::json(&revisions)) } @@ -413,11 +408,10 @@ mod handler { /// Fetch the list [`coco::Tag`]. pub async fn tags( peer: Arc>, - project_urn: String, + project_urn: coco::Urn, ) -> Result { let peer = peer.lock().await; - let urn = project_urn.parse().map_err(Error::from)?; - let tags = coco::with_browser(&peer, &urn, |browser| coco::tags(browser))?; + let tags = coco::with_browser(&peer, &project_urn, |browser| coco::tags(browser))?; Ok(reply::json(&tags)) } @@ -425,7 +419,7 @@ mod handler { /// Fetch a [`coco::Tree`]. pub async fn tree( api: Arc>, - project_urn: String, + project_urn: coco::Urn, super::TreeQuery { prefix, peer_id, @@ -439,8 +433,7 @@ mod handler { revision ); let api = api.lock().await; - let urn = project_urn.parse().map_err(Error::from)?; - let project = coco::get_project(&api, &urn)?; + let project = coco::get_project(&api, &project_urn)?; let default_branch = match peer_id { Some(peer_id) if peer_id != *api.peer_id() => { @@ -450,7 +443,7 @@ mod handler { }; log::debug!("tree.default_branch={:?}", default_branch); - let tree = coco::with_browser(&api, &urn, |mut browser| { + let tree = coco::with_browser(&api, &project_urn, |mut browser| { coco::tree(&mut browser, default_branch, revision, prefix) })?; From 6ef4d3fa0e2fb916a08e4bab80ef1a44d563d2e4 Mon Sep 17 00:00:00 2001 From: FintanH Date: Wed, 15 Jul 2020 18:14:41 +0100 Subject: [PATCH 02/13] Localise the revisions function in coco/source.rs Moving the revisions function into source so that it only operates over a Browser. The peer logic gets moved to handler and the information gets passed down. As well as this, we extend the branches endpoint so that it can take a PeerId if we want to look at a peer's list of branches. We test this in the revisions test. --- proxy/src/coco.rs | 8 ++-- proxy/src/coco/peer.rs | 79 +--------------------------------------- proxy/src/coco/source.rs | 70 +++++++++++++++++++++++++++++++++++ proxy/src/http/source.rs | 49 ++++++++++++++++++++----- 4 files changed, 115 insertions(+), 91 deletions(-) diff --git a/proxy/src/coco.rs b/proxy/src/coco.rs index 81ded197d4..8c1a8a8288 100644 --- a/proxy/src/coco.rs +++ b/proxy/src/coco.rs @@ -10,15 +10,15 @@ pub use radicle_surf::vcs::git::Stats; mod peer; pub use peer::{ create_peer_api, default_owner, get_project, get_user, init_owner, init_project, init_user, - list_projects, list_users, revisions, set_default_owner, verify_user, with_browser, PeerApi, - User, UserRevisions, + list_projects, list_users, set_default_owner, verify_user, with_browser, PeerApi, User, }; /// Module that captures all types and functions for source code. mod source; pub use source::{ - blob, branches, commit, commit_header, commits, local_state, tags, tree, Blob, BlobContent, - Branch, Commit, CommitHeader, Info, ObjectType, Person, Revision, Tag, Tree, TreeEntry, + blob, branches, commit, commit_header, commits, into_branch_type, local_state, revisions, tags, + tree, Blob, BlobContent, Branch, Commit, CommitHeader, Info, ObjectType, Person, Revision, Tag, + Tree, TreeEntry, UserRevisions, }; pub mod config; diff --git a/proxy/src/coco/peer.rs b/proxy/src/coco/peer.rs index 9c7b8b1e2f..cd0f9f8962 100644 --- a/proxy/src/coco/peer.rs +++ b/proxy/src/coco/peer.rs @@ -1,5 +1,3 @@ -use nonempty::NonEmpty; -use serde::Serialize; use std::convert::TryFrom; use std::net::SocketAddr; @@ -10,28 +8,14 @@ use librad::meta::user; use librad::net::discovery; pub use librad::net::peer::{PeerApi, PeerConfig}; use librad::uri::RadUrn; -use radicle_surf::vcs::git::{self, git2, BranchType}; +use radicle_surf::vcs::git::{self, git2}; -use super::source; use crate::error; -use crate::identity; use crate::project::Project; /// Export a verified [`user::User`] type. pub type User = user::User; -/// Bundled response to retrieve both branches and tags for a user repo. -#[derive(Debug, Serialize)] -#[serde(rename_all = "camelCase")] -pub struct UserRevisions { - /// Owner of the repo. - pub(crate) identity: identity::Identity, - /// List of [`source::Branch`]. - pub(crate) branches: Vec, - /// List of [`source::Tag`]. - pub(crate) tags: Vec, -} - /// Create a new `PeerApi` given a `PeerConfig`. /// /// # Errors @@ -123,67 +107,6 @@ pub fn list_projects(peer: &PeerApi) -> Result, error::Error> { .collect() } -/// Get all [`UserRevisions`] for a given project. -/// -/// # Parameters -/// -/// * `peer` - the peer API we're interacting through -/// * `owner` - the owner of this peer, i.e. the current user -/// * `project_urn` - the [`RadUrn`] pointing to the project we're interested in -/// -/// # Errors -/// -/// * [`error::Error::LibradLock`] -/// * [`error::Error::Git`] -pub fn revisions( - peer: &PeerApi, - owner: &User, - project_urn: &RadUrn, -) -> Result, error::Error> { - let project = get_project(peer, project_urn)?; - let storage = peer.storage().reopen()?; - let repo = storage.open_repo(project.urn())?; - let mut user_revisions = vec![]; - - let (local_branches, local_tags) = with_browser(peer, &project.urn(), |browser| { - Ok(( - source::branches(browser, Some(BranchType::Local))?, - source::tags(browser)?, - )) - })?; - - if !local_branches.is_empty() { - user_revisions.push(UserRevisions { - identity: (peer.peer_id().clone(), owner.clone()).into(), - branches: local_branches, - tags: local_tags, - }) - } - - for peer_id in repo.tracked()? { - let remote_branches = with_browser(peer, &project.urn(), |browser| { - source::branches( - browser, - Some(BranchType::Remote { - name: Some(format!("{}/heads", peer_id)), - }), - ) - })?; - - let user = repo.get_rad_self_of(peer_id.clone())?; - - user_revisions.push(UserRevisions { - identity: (peer_id, user).into(), - branches: remote_branches, - // TODO(rudolfs): implement remote peer tags once we decide how - // https://radicle.community/t/git-tags/214 - tags: vec![], - }); - } - - NonEmpty::from_vec(user_revisions).ok_or(error::Error::EmptyUserRevisions) -} - /// Returns the list of [`user::User`]s known for your peer. /// /// # Errors diff --git a/proxy/src/coco/source.rs b/proxy/src/coco/source.rs index 7d08be6424..67131a5f85 100644 --- a/proxy/src/coco/source.rs +++ b/proxy/src/coco/source.rs @@ -2,8 +2,11 @@ use serde::{Deserialize, Serialize}; use std::convert::TryFrom; use std::fmt; use std::str::FromStr; +use nonempty::NonEmpty; use librad::peer; +use librad::meta::user; +use librad::meta::entity; use radicle_surf::{ diff, file_system, vcs::git::{self, git2, BranchType, Browser, Rev}, @@ -14,7 +17,9 @@ use syntect::highlighting::ThemeSet; use syntect::parsing::SyntaxSet; use syntect::util::LinesWithEndings; +use crate::coco::User; use crate::error; +use crate::identity; use crate::session::settings::Theme; lazy_static::lazy_static! { @@ -235,6 +240,18 @@ impl TryFrom for Rev { } } +/// Bundled response to retrieve both branches and tags for a user repo. +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct UserRevisions { + /// Owner of the repo. + pub(crate) identity: identity::Identity, + /// List of [`source::Branch`]. + pub(crate) branches: Vec, + /// List of [`source::Tag`]. + pub(crate) tags: Vec, +} + /// Returns the [`Blob`] for a file at `revision` under `path`. /// /// # Errors @@ -561,6 +578,59 @@ pub fn tree<'repo>( }) } +/// Get all [`UserRevisions`] for a given project. +/// +/// # Parameters +/// +/// * `peer_id` - the PeerId of this peer +/// * `owner` - the owner of this peer, i.e. the current user +/// * `peers` - an iterator of a peer and the default self it used for this project +/// +/// # Errors +/// +/// * [`error::Error::LibradLock`] +/// * [`error::Error::Git`] +pub fn revisions( + browser: &Browser, + peer_id: peer::PeerId, + owner: &User, + peers: Vec<(user::User, peer::PeerId)>, +) -> Result, error::Error> { + let mut user_revisions = vec![]; + + let local_branches = branches(browser, Some(BranchType::Local))?; + if !local_branches.is_empty() { + user_revisions.push(UserRevisions { + identity: (peer_id, owner.clone()).into(), + branches: local_branches, + tags: tags(browser)?, + }) + } + + for (user, peer_id) in peers { + let remote_branches = branches(browser, Some(into_branch_type(Some(peer_id.clone()))))?; + + user_revisions.push(UserRevisions { + identity: (peer_id, user).into(), + branches: remote_branches, + // TODO(rudolfs): implement remote peer tags once we decide how + // https://radicle.community/t/git-tags/214 + tags: vec![], + }); + } + + NonEmpty::from_vec(user_revisions).ok_or(error::Error::EmptyUserRevisions) +} + +/// Turn an `Option` into a [`BranchType`]. If the `PeerId` is present then this is +/// set as the remote of the `BranchType`. Otherwise, it's local branch. +pub fn into_branch_type(peer_id: Option) -> BranchType { + peer_id.map_or(BranchType::Local, |peer_id| BranchType::Remote { + // We qualify the remotes as the PeerId + heads, otherwise we would grab the tags too. + name: Some(format!("{}/heads", peer_id.to_string())), + }) +} + #[cfg(test)] mod tests { use librad::keys::SecretKey; diff --git a/proxy/src/http/source.rs b/proxy/src/http/source.rs index ff4bfa46dd..3d686e4fc1 100644 --- a/proxy/src/http/source.rs +++ b/proxy/src/http/source.rs @@ -105,6 +105,10 @@ fn branches_filter( "project_id", "ID of the project the blob is part of", )) + .and(warp::filters::query::query::()) + .and(document::document( + document::query("peerId", document::string()).description("The peer identifier"), + )) .and(document::document(document::description("List Branches"))) .and(document::document(document::tag("Source"))) .and(document::document( @@ -296,9 +300,10 @@ mod handler { use warp::path::Tail; use warp::{reply, Rejection, Reply}; - use radicle_surf::vcs::git::{self, BranchType}; + use radicle_surf::vcs::git; use crate::coco; + use crate::error::Error; use crate::http; use crate::registry; use crate::session; @@ -349,10 +354,12 @@ mod handler { pub async fn branches( peer: Arc>, project_urn: coco::Urn, + super::PeerQuery { peer_id }: super::PeerQuery, ) -> Result { + log::info!("peer.id = {:?}", peer_id); let peer = peer.lock().await; let branches = coco::with_browser(&peer, &project_urn, |browser| { - coco::branches(browser, Some(BranchType::Local)) + coco::branches(browser, Some(coco::into_branch_type(peer_id))) })?; Ok(reply::json(&branches)) @@ -395,12 +402,20 @@ mod handler { /// Fetch the list [`coco::Branch`] and [`coco::Tag`]. pub async fn revisions( - peer: Arc>, + api: Arc>, project_urn: coco::Urn, owner: coco::User, ) -> Result { - let peer = &*peer.lock().await; - let revisions: Vec<_> = coco::revisions(peer, &owner, &project_urn)?.into(); + let api = &*api.lock().await; + let storage = api.storage().reopen().map_err(Error::from)?; + let repo = storage.open_repo(project_urn.clone()).map_err(Error::from)?; + let peers = repo.tracked().map_err(Error::from)?.map(|peer_id| { + repo.get_rad_self_of(peer_id.clone()).map(|user| (user, peer_id.clone())).map_err(Error::from) + }).collect::, _>>()?; + let peer_id = api.peer_id().clone(); + let revisions: Vec<_> = coco::with_browser(&api, &project_urn, |browser| { + Ok(coco::revisions(&browser, peer_id, &owner, peers)?.into()) + })?; Ok(reply::json(&revisions)) } @@ -482,6 +497,13 @@ pub struct BlobQuery { highlight: Option, } +/// A query param that only consists of a single `PeerId`. +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PeerQuery { + peer_id: Option, +} + /// Bundled query params to pass to the tree handler. #[derive(Debug, Serialize, Deserialize)] pub struct TreeQuery { @@ -1289,7 +1311,7 @@ mod test { .expect("missing target"); let _heads = platinum .reference( - &format!("{}/heads/master", prefix), + &format!("{}/heads/haptop", prefix), target, false, "remote heads", @@ -1359,14 +1381,24 @@ mod test { ] }, coco::UserRevisions { - identity: (remote, fintohaps).into(), - branches: vec![coco::Branch("master".to_string())], + identity: (remote.clone(), fintohaps).into(), + branches: vec![coco::Branch("haptop".to_string())], tags: vec![] }, ]) ) }); + let res = request() + .method("GET") + .path(&format!("/branches/{}?peerId={}", urn, remote)) + .reply(&api) + .await; + + http::test::assert_response(&res, StatusCode::OK, |have| { + assert_eq!(have, json!([coco::Branch("haptop".to_string())])); + }); + Ok(()) } @@ -1515,7 +1547,6 @@ mod test { .add(b'[') .add(b']') .add(b'='); - pretty_env_logger::init(); let tmp_dir = tempfile::tempdir()?; let key = SecretKey::new(); let registry = { From 3ed56992141498ed0a642b27e930589923ae22d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C5=ABdolfs=20O=C5=A1i=C5=86=C5=A1?= Date: Thu, 16 Jul 2020 09:23:59 +0200 Subject: [PATCH 03/13] Fmt all the code --- proxy/src/coco/source.rs | 6 +++--- proxy/src/http/source.rs | 16 ++++++++++++---- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/proxy/src/coco/source.rs b/proxy/src/coco/source.rs index 67131a5f85..0ffd98c73a 100644 --- a/proxy/src/coco/source.rs +++ b/proxy/src/coco/source.rs @@ -1,12 +1,12 @@ +use nonempty::NonEmpty; use serde::{Deserialize, Serialize}; use std::convert::TryFrom; use std::fmt; use std::str::FromStr; -use nonempty::NonEmpty; -use librad::peer; -use librad::meta::user; use librad::meta::entity; +use librad::meta::user; +use librad::peer; use radicle_surf::{ diff, file_system, vcs::git::{self, git2, BranchType, Browser, Rev}, diff --git a/proxy/src/http/source.rs b/proxy/src/http/source.rs index 3d686e4fc1..91da07c74b 100644 --- a/proxy/src/http/source.rs +++ b/proxy/src/http/source.rs @@ -408,10 +408,18 @@ mod handler { ) -> Result { let api = &*api.lock().await; let storage = api.storage().reopen().map_err(Error::from)?; - let repo = storage.open_repo(project_urn.clone()).map_err(Error::from)?; - let peers = repo.tracked().map_err(Error::from)?.map(|peer_id| { - repo.get_rad_self_of(peer_id.clone()).map(|user| (user, peer_id.clone())).map_err(Error::from) - }).collect::, _>>()?; + let repo = storage + .open_repo(project_urn.clone()) + .map_err(Error::from)?; + let peers = repo + .tracked() + .map_err(Error::from)? + .map(|peer_id| { + repo.get_rad_self_of(peer_id.clone()) + .map(|user| (user, peer_id.clone())) + .map_err(Error::from) + }) + .collect::, _>>()?; let peer_id = api.peer_id().clone(); let revisions: Vec<_> = coco::with_browser(&api, &project_urn, |browser| { Ok(coco::revisions(&browser, peer_id, &owner, peers)?.into()) From af6e090cdb1b42fe802e28ab7dddb8eb651a8352 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C5=ABdolfs=20O=C5=A1i=C5=86=C5=A1?= Date: Thu, 16 Jul 2020 09:37:15 +0200 Subject: [PATCH 04/13] Make McClippy happy --- proxy/src/coco/source.rs | 3 ++- proxy/src/http/source.rs | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/proxy/src/coco/source.rs b/proxy/src/coco/source.rs index 0ffd98c73a..a0f51ad866 100644 --- a/proxy/src/coco/source.rs +++ b/proxy/src/coco/source.rs @@ -582,7 +582,7 @@ pub fn tree<'repo>( /// /// # Parameters /// -/// * `peer_id` - the PeerId of this peer +/// * `peer_id` - the `PeerId` of this peer /// * `owner` - the owner of this peer, i.e. the current user /// * `peers` - an iterator of a peer and the default self it used for this project /// @@ -624,6 +624,7 @@ pub fn revisions( /// Turn an `Option` into a [`BranchType`]. If the `PeerId` is present then this is /// set as the remote of the `BranchType`. Otherwise, it's local branch. +#[must_use] pub fn into_branch_type(peer_id: Option) -> BranchType { peer_id.map_or(BranchType::Local, |peer_id| BranchType::Remote { // We qualify the remotes as the PeerId + heads, otherwise we would grab the tags too. diff --git a/proxy/src/http/source.rs b/proxy/src/http/source.rs index 91da07c74b..343e33da42 100644 --- a/proxy/src/http/source.rs +++ b/proxy/src/http/source.rs @@ -421,8 +421,8 @@ mod handler { }) .collect::, _>>()?; let peer_id = api.peer_id().clone(); - let revisions: Vec<_> = coco::with_browser(&api, &project_urn, |browser| { - Ok(coco::revisions(&browser, peer_id, &owner, peers)?.into()) + let revisions: Vec<_> = coco::with_browser(api, &project_urn, |browser| { + Ok(coco::revisions(browser, peer_id, &owner, peers)?.into()) })?; Ok(reply::json(&revisions)) @@ -509,6 +509,7 @@ pub struct BlobQuery { #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase")] pub struct PeerQuery { + /// PeerId to scope the query by. peer_id: Option, } From 4b1f2d556dcf288aa58c74e4f558417d60d0077f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C5=ABdolfs=20O=C5=A1i=C5=86=C5=A1?= Date: Thu, 16 Jul 2020 09:45:59 +0200 Subject: [PATCH 05/13] Fix docs --- proxy/src/coco/source.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proxy/src/coco/source.rs b/proxy/src/coco/source.rs index a0f51ad866..b5e76c3267 100644 --- a/proxy/src/coco/source.rs +++ b/proxy/src/coco/source.rs @@ -246,9 +246,9 @@ impl TryFrom for Rev { pub struct UserRevisions { /// Owner of the repo. pub(crate) identity: identity::Identity, - /// List of [`source::Branch`]. + /// List of [`git::Branch`]. pub(crate) branches: Vec, - /// List of [`source::Tag`]. + /// List of [`git::Tag`]. pub(crate) tags: Vec, } From 90411db5ea6fd0bbf2b50607dfd66ce448f16389 Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Thu, 16 Jul 2020 09:01:18 +0100 Subject: [PATCH 06/13] Remove info log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rūdolfs Ošiņš --- proxy/src/http/source.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/proxy/src/http/source.rs b/proxy/src/http/source.rs index 343e33da42..46007f9798 100644 --- a/proxy/src/http/source.rs +++ b/proxy/src/http/source.rs @@ -356,7 +356,6 @@ mod handler { project_urn: coco::Urn, super::PeerQuery { peer_id }: super::PeerQuery, ) -> Result { - log::info!("peer.id = {:?}", peer_id); let peer = peer.lock().await; let branches = coco::with_browser(&peer, &project_urn, |browser| { coco::branches(browser, Some(coco::into_branch_type(peer_id))) From 03eff2a8263d99add6dff33e90db323f3982104b Mon Sep 17 00:00:00 2001 From: FintanH Date: Fri, 17 Jul 2020 15:34:03 +0100 Subject: [PATCH 07/13] Rename PeerQuery to BranchQuery --- proxy/src/http/source.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/proxy/src/http/source.rs b/proxy/src/http/source.rs index d3015c59d5..da88d6633c 100644 --- a/proxy/src/http/source.rs +++ b/proxy/src/http/source.rs @@ -105,7 +105,7 @@ fn branches_filter( "project_id", "ID of the project the blob is part of", )) - .and(warp::filters::query::query::()) + .and(warp::filters::query::query::()) .and(document::document( document::query("peerId", document::string()).description("The peer identifier"), )) @@ -354,7 +354,7 @@ mod handler { pub async fn branches( peer: Arc>, project_urn: coco::Urn, - super::PeerQuery { peer_id }: super::PeerQuery, + super::BranchQuery { peer_id }: super::BranchQuery, ) -> Result { let peer = peer.lock().await; let branches = coco::with_browser(&peer, &project_urn, |browser| { @@ -504,10 +504,10 @@ pub struct BlobQuery { highlight: Option, } -/// A query param that only consists of a single `PeerId`. +/// A query param for [`handler::branches`]. #[derive(Debug, Clone, Deserialize)] #[serde(rename_all = "camelCase")] -pub struct PeerQuery { +pub struct BranchQuery { /// PeerId to scope the query by. peer_id: Option, } From cede7db76228b0adeb91ba072de54d2687ed3d6a Mon Sep 17 00:00:00 2001 From: FintanH Date: Mon, 20 Jul 2020 16:01:06 +0100 Subject: [PATCH 08/13] Refactor UserRevisions * Renamed UserRevisions to Revisions. * Removed Identity and replaced with peer_id, user fields. * Parameterise over P and U so that we don't import librad modules. --- proxy/src/coco.rs | 4 ++-- proxy/src/coco/source.rs | 45 ++++++++++++++++++++-------------------- proxy/src/error.rs | 2 +- proxy/src/http/source.rs | 22 ++++++++++++-------- 4 files changed, 39 insertions(+), 34 deletions(-) diff --git a/proxy/src/coco.rs b/proxy/src/coco.rs index 8c1a8a8288..bdc4978e1c 100644 --- a/proxy/src/coco.rs +++ b/proxy/src/coco.rs @@ -17,8 +17,8 @@ pub use peer::{ mod source; pub use source::{ blob, branches, commit, commit_header, commits, into_branch_type, local_state, revisions, tags, - tree, Blob, BlobContent, Branch, Commit, CommitHeader, Info, ObjectType, Person, Revision, Tag, - Tree, TreeEntry, UserRevisions, + tree, Blob, BlobContent, Branch, Commit, CommitHeader, Info, ObjectType, Person, Revision, + Revisions, Tag, Tree, TreeEntry, }; pub mod config; diff --git a/proxy/src/coco/source.rs b/proxy/src/coco/source.rs index 2e53030ce2..6ad1cd505f 100644 --- a/proxy/src/coco/source.rs +++ b/proxy/src/coco/source.rs @@ -4,8 +4,6 @@ use std::convert::TryFrom; use std::fmt; use std::str::FromStr; -use librad::meta::entity; -use librad::meta::user; use librad::peer; use radicle_surf::{ diff, file_system, @@ -17,9 +15,7 @@ use syntect::highlighting::ThemeSet; use syntect::parsing::SyntaxSet; use syntect::util::LinesWithEndings; -use crate::coco::User; use crate::error; -use crate::identity; use crate::session::settings::Theme; lazy_static::lazy_static! { @@ -242,16 +238,18 @@ impl TryFrom for Rev { } } -/// Bundled response to retrieve both branches and tags for a user repo. +/// Bundled response to retrieve both [`Branch`]es and [`Tag`]s for a [`user::User`]'s repo. #[derive(Debug, Serialize)] #[serde(rename_all = "camelCase")] -pub struct UserRevisions { +pub struct Revisions { + /// The [`peer::PeerId`] of the provided [`user::User`]. + pub peer_id: P, /// Owner of the repo. - pub(crate) identity: identity::Identity, + pub user: U, /// List of [`git::Branch`]. - pub(crate) branches: Vec, + pub branches: Vec, /// List of [`git::Tag`]. - pub(crate) tags: Vec, + pub tags: Vec, } /// Returns the [`Blob`] for a file at `revision` under `path`. @@ -596,7 +594,7 @@ pub fn tree<'repo>( }) } -/// Get all [`UserRevisions`] for a given project. +/// Get all [`Revisions`] for a given project. /// /// # Parameters /// @@ -608,28 +606,31 @@ pub fn tree<'repo>( /// /// * [`error::Error::LibradLock`] /// * [`error::Error::Git`] -pub fn revisions( +pub fn revisions( browser: &Browser, - peer_id: peer::PeerId, - owner: &User, - peers: Vec<(user::User, peer::PeerId)>, -) -> Result, error::Error> { + peer_id: P, + owner: U, + peers: Vec<(P, U)>, +) -> Result>, error::Error> +where P: Clone + ToString, { let mut user_revisions = vec![]; let local_branches = branches(browser, Some(BranchType::Local))?; if !local_branches.is_empty() { - user_revisions.push(UserRevisions { - identity: (peer_id, owner.clone()).into(), + user_revisions.push(Revisions { + peer_id, + user: owner, branches: local_branches, tags: tags(browser)?, }) } - for (user, peer_id) in peers { + for (peer_id, user) in peers { let remote_branches = branches(browser, Some(into_branch_type(Some(peer_id.clone()))))?; - user_revisions.push(UserRevisions { - identity: (peer_id, user).into(), + user_revisions.push(Revisions { + peer_id, + user, branches: remote_branches, // TODO(rudolfs): implement remote peer tags once we decide how // https://radicle.community/t/git-tags/214 @@ -637,13 +638,13 @@ pub fn revisions( }); } - NonEmpty::from_vec(user_revisions).ok_or(error::Error::EmptyUserRevisions) + NonEmpty::from_vec(user_revisions).ok_or(error::Error::EmptyRevisions) } /// Turn an `Option` into a [`BranchType`]. If the `PeerId` is present then this is /// set as the remote of the `BranchType`. Otherwise, it's local branch. #[must_use] -pub fn into_branch_type(peer_id: Option) -> BranchType { +pub fn into_branch_type

(peer_id: Option

) -> BranchType where P: ToString, { peer_id.map_or(BranchType::Local, |peer_id| BranchType::Remote { // We qualify the remotes as the PeerId + heads, otherwise we would grab the tags too. name: Some(format!("{}/heads", peer_id.to_string())), diff --git a/proxy/src/error.rs b/proxy/src/error.rs index 1441bc001c..63c595e5a1 100644 --- a/proxy/src/error.rs +++ b/proxy/src/error.rs @@ -173,7 +173,7 @@ pub enum Error { #[error( "while trying to get user revisions we could not find any, there should be at least one" )] - EmptyUserRevisions, + EmptyRevisions, } impl From for Error { diff --git a/proxy/src/http/source.rs b/proxy/src/http/source.rs index 96cc018b14..2c75cbe2ee 100644 --- a/proxy/src/http/source.rs +++ b/proxy/src/http/source.rs @@ -226,7 +226,7 @@ where document::response( 200, document::body( - document::array(coco::UserRevisions::document()) + document::array(coco::Revisions::<(), ()>::document()) .description("List of revisions per repo"), ) .mime("application/json"), @@ -415,13 +415,14 @@ mod handler { .map_err(Error::from)? .map(|peer_id| { repo.get_rad_self_of(peer_id.clone()) - .map(|user| (user, peer_id.clone())) + .map(|user| (peer_id.clone(), user)) .map_err(Error::from) }) .collect::, _>>()?; let peer_id = api.peer_id().clone(); let revisions: Vec<_> = coco::with_browser(api, &project_urn, |browser| { - Ok(coco::revisions(browser, peer_id, &owner, peers)?.into()) + let owner = owner.to_data().build()?; // TODO(finto): downgraded verified user, which should not be needed. + Ok(coco::revisions(browser, peer_id, owner, peers)?.into()) })?; Ok(reply::json(&revisions)) @@ -822,14 +823,14 @@ impl ToDocumentedType for coco::TreeEntry { } } -impl ToDocumentedType for coco::UserRevisions { +impl ToDocumentedType for coco::Revisions { fn document() -> document::DocumentedType { let mut properties = std::collections::HashMap::with_capacity(3); properties.insert("identity".into(), identity::Identity::document()); properties.insert("branches".into(), document::array(coco::Branch::document())); properties.insert("tags".into(), document::array(coco::Tag::document())); - document::DocumentedType::from(properties).description("UserRevisions") + document::DocumentedType::from(properties).description("Revisions") } } @@ -1314,12 +1315,14 @@ mod test { .reply(&api) .await; + let owner = owner.to_data().build()?; // TODO(finto): Unverify owner, unfortunately http::test::assert_response(&res, StatusCode::OK, |have| { assert_eq!( have, json!([ - coco::UserRevisions { - identity: (peer_id, owner).into(), + coco::Revisions { + peer_id, + user: owner, branches: vec![ coco::Branch("dev".to_string()), coco::Branch("master".to_string()) @@ -1332,8 +1335,9 @@ mod test { coco::Tag("v0.5.0".to_string()) ] }, - coco::UserRevisions { - identity: (remote.clone(), fintohaps).into(), + coco::Revisions { + peer_id: remote.clone(), + user: fintohaps, branches: vec![coco::Branch("master".to_string())], tags: vec![] }, From 4d28d3510d8956a970b50a4e27f163980497aac0 Mon Sep 17 00:00:00 2001 From: FintanH Date: Mon, 20 Jul 2020 16:07:54 +0100 Subject: [PATCH 09/13] Parameterise over Revision too --- proxy/src/coco/source.rs | 47 +++++++++++++++++++++++++--------------- proxy/src/http/source.rs | 4 ++-- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/proxy/src/coco/source.rs b/proxy/src/coco/source.rs index 6ad1cd505f..8e4d9363c8 100644 --- a/proxy/src/coco/source.rs +++ b/proxy/src/coco/source.rs @@ -4,7 +4,6 @@ use std::convert::TryFrom; use std::fmt; use std::str::FromStr; -use librad::peer; use radicle_surf::{ diff, file_system, vcs::git::{self, git2, BranchType, Browser, Rev}, @@ -203,7 +202,7 @@ pub struct TreeEntry { /// A revision selector for a `Browser`. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase", tag = "type")] -pub enum Revision { +pub enum Revision

{ /// Select a tag under the name provided. Tag { /// Name of the tag. @@ -214,7 +213,7 @@ pub enum Revision { /// Name of the branch. name: String, /// The remote peer, if specified. - peer_id: Option, + peer_id: Option

, }, /// Select a SHA1 under the name provided. Sha { @@ -223,10 +222,13 @@ pub enum Revision { }, } -impl TryFrom for Rev { +impl

TryFrom> for Rev +where + P: ToString, +{ type Error = error::Error; - fn try_from(other: Revision) -> Result { + fn try_from(other: Revision

) -> Result { match other { Revision::Tag { name } => Ok(git::TagName::new(&name).into()), Revision::Branch { name, peer_id } => Ok(match peer_id { @@ -242,9 +244,9 @@ impl TryFrom for Rev { #[derive(Debug, Serialize)] #[serde(rename_all = "camelCase")] pub struct Revisions { - /// The [`peer::PeerId`] of the provided [`user::User`]. + /// The peer identifier for the provided user. pub peer_id: P, - /// Owner of the repo. + /// Owner of the revision set. pub user: U, /// List of [`git::Branch`]. pub branches: Vec, @@ -257,13 +259,16 @@ pub struct Revisions { /// # Errors /// /// Will return [`error::Error`] if the project doesn't exist or a surf interaction fails. -pub fn blob( +pub fn blob

( browser: &mut Browser, default_branch: git::Branch, - maybe_revision: Option, + maybe_revision: Option>, path: &str, theme: Option<&Theme>, -) -> Result { +) -> Result +where + P: ToString, +{ let maybe_revision = maybe_revision.map(Rev::try_from).transpose()?; browser.rev(maybe_revision.unwrap_or_else(|| default_branch.into()))?; @@ -504,12 +509,15 @@ pub fn tags<'repo>(browser: &Browser<'repo>) -> Result, error::Error> { /// # Errors /// /// Will return [`error::Error`] if any of the surf interactions fail. -pub fn tree<'repo>( +pub fn tree<'repo, P>( browser: &mut Browser<'repo>, default_branch: git::Branch, - maybe_revision: Option, + maybe_revision: Option>, maybe_prefix: Option, -) -> Result { +) -> Result +where + P: ToString, +{ let maybe_revision = maybe_revision.map(Rev::try_from).transpose()?; let revision = maybe_revision.unwrap_or_else(|| default_branch.into()); let prefix = maybe_prefix.unwrap_or_default(); @@ -598,7 +606,7 @@ pub fn tree<'repo>( /// /// # Parameters /// -/// * `peer_id` - the `PeerId` of this peer +/// * `peer_id` - the identifier of this peer /// * `owner` - the owner of this peer, i.e. the current user /// * `peers` - an iterator of a peer and the default self it used for this project /// @@ -612,7 +620,9 @@ pub fn revisions( owner: U, peers: Vec<(P, U)>, ) -> Result>, error::Error> -where P: Clone + ToString, { +where + P: Clone + ToString, +{ let mut user_revisions = vec![]; let local_branches = branches(browser, Some(BranchType::Local))?; @@ -641,10 +651,13 @@ where P: Clone + ToString, { NonEmpty::from_vec(user_revisions).ok_or(error::Error::EmptyRevisions) } -/// Turn an `Option` into a [`BranchType`]. If the `PeerId` is present then this is +/// Turn an `Option

` into a [`BranchType`]. If the `P` is present then this is /// set as the remote of the `BranchType`. Otherwise, it's local branch. #[must_use] -pub fn into_branch_type

(peer_id: Option

) -> BranchType where P: ToString, { +pub fn into_branch_type

(peer_id: Option

) -> BranchType +where + P: ToString, +{ peer_id.map_or(BranchType::Local, |peer_id| BranchType::Remote { // We qualify the remotes as the PeerId + heads, otherwise we would grab the tags too. name: Some(format!("{}/heads", peer_id.to_string())), diff --git a/proxy/src/http/source.rs b/proxy/src/http/source.rs index 2c75cbe2ee..86e17c9512 100644 --- a/proxy/src/http/source.rs +++ b/proxy/src/http/source.rs @@ -500,7 +500,7 @@ pub struct BlobQuery { /// PeerId to scope the query by. peer_id: Option, /// Revision to query at. - revision: Option, + revision: Option>, /// Whether or not to syntax highlight the blob. highlight: Option, } @@ -521,7 +521,7 @@ pub struct TreeQuery { /// PeerId to scope the query by. peer_id: Option, /// Revision to query at. - revision: Option, + revision: Option>, } impl Serialize for coco::Blob { From 08ef269357d3d86a7773a9fbe44281cb44425a1e Mon Sep 17 00:00:00 2001 From: FintanH Date: Mon, 20 Jul 2020 18:21:37 +0100 Subject: [PATCH 10/13] Have a Reply version of Revisions that holds the Identity --- proxy/src/coco/source.rs | 6 +++--- proxy/src/http/source.rs | 38 ++++++++++++++++++++++++++++++-------- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/proxy/src/coco/source.rs b/proxy/src/coco/source.rs index 8e4d9363c8..bb8064ce8e 100644 --- a/proxy/src/coco/source.rs +++ b/proxy/src/coco/source.rs @@ -240,13 +240,13 @@ where } } -/// Bundled response to retrieve both [`Branch`]es and [`Tag`]s for a [`user::User`]'s repo. +/// Bundled response to retrieve both [`Branch`]es and [`Tag`]s for a user's repo. #[derive(Debug, Serialize)] #[serde(rename_all = "camelCase")] pub struct Revisions { - /// The peer identifier for the provided user. + /// The peer identifier for the user. pub peer_id: P, - /// Owner of the revision set. + /// The user who owns these revisions. pub user: U, /// List of [`git::Branch`]. pub branches: Vec, diff --git a/proxy/src/http/source.rs b/proxy/src/http/source.rs index 86e17c9512..458d2b3ea7 100644 --- a/proxy/src/http/source.rs +++ b/proxy/src/http/source.rs @@ -7,6 +7,7 @@ use tokio::sync::{Mutex, RwLock}; use warp::document::{self, ToDocumentedType}; use warp::{path, Filter, Rejection, Reply}; +use librad::meta::user; use librad::peer; use radicle_surf::vcs::git; @@ -420,9 +421,12 @@ mod handler { }) .collect::, _>>()?; let peer_id = api.peer_id().clone(); - let revisions: Vec<_> = coco::with_browser(api, &project_urn, |browser| { + let revisions: Vec = coco::with_browser(api, &project_urn, |browser| { let owner = owner.to_data().build()?; // TODO(finto): downgraded verified user, which should not be needed. - Ok(coco::revisions(browser, peer_id, owner, peers)?.into()) + Ok(coco::revisions(browser, peer_id, owner, peers)? + .into_iter() + .map(|revision| revision.into()) + .collect()) })?; Ok(reply::json(&revisions)) @@ -834,6 +838,26 @@ impl ToDocumentedType for coco::Revisions { } } +#[derive(Serialize)] +struct Revisions { + /// The [`identity::Identity`] that owns these revisions. + identity: identity::Identity, + /// The branches for this project. + branches: Vec, + /// The branches for this project. + tags: Vec, +} + +impl From>> for Revisions { + fn from(other: coco::Revisions>) -> Self { + Self { + identity: (other.peer_id, other.user).into(), + branches: other.branches, + tags: other.tags, + } + } +} + #[allow(clippy::non_ascii_literal, clippy::unwrap_used)] #[cfg(test)] mod test { @@ -1320,9 +1344,8 @@ mod test { assert_eq!( have, json!([ - coco::Revisions { - peer_id, - user: owner, + super::Revisions { + identity: (peer_id, owner).into(), branches: vec![ coco::Branch("dev".to_string()), coco::Branch("master".to_string()) @@ -1335,9 +1358,8 @@ mod test { coco::Tag("v0.5.0".to_string()) ] }, - coco::Revisions { - peer_id: remote.clone(), - user: fintohaps, + super::Revisions { + identity: (remote.clone(), fintohaps).into(), branches: vec![coco::Branch("master".to_string())], tags: vec![] }, From 8a91d32b1aab54bed017463b1808fe9cb12b0038 Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Wed, 22 Jul 2020 11:04:19 +0100 Subject: [PATCH 11/13] Update proxy/src/http/source.rs Co-authored-by: Alexander Simmerl --- proxy/src/http/source.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/proxy/src/http/source.rs b/proxy/src/http/source.rs index 458d2b3ea7..c341859261 100644 --- a/proxy/src/http/source.rs +++ b/proxy/src/http/source.rs @@ -422,7 +422,8 @@ mod handler { .collect::, _>>()?; let peer_id = api.peer_id().clone(); let revisions: Vec = coco::with_browser(api, &project_urn, |browser| { - let owner = owner.to_data().build()?; // TODO(finto): downgraded verified user, which should not be needed. + // TODO(finto): downgraded verified user, which should not be needed. + let owner = owner.to_data().build()?; Ok(coco::revisions(browser, peer_id, owner, peers)? .into_iter() .map(|revision| revision.into()) From f9e2a6b66091b2da34c3531446a5d548a46a14bb Mon Sep 17 00:00:00 2001 From: FintanH Date: Wed, 22 Jul 2020 11:09:48 +0100 Subject: [PATCH 12/13] Document Revisions output --- proxy/src/http/source.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/proxy/src/http/source.rs b/proxy/src/http/source.rs index c341859261..84dd035938 100644 --- a/proxy/src/http/source.rs +++ b/proxy/src/http/source.rs @@ -839,6 +839,7 @@ impl ToDocumentedType for coco::Revisions { } } +/// The output structure when calling the `/revisions` endpoint. #[derive(Serialize)] struct Revisions { /// The [`identity::Identity`] that owns these revisions. From 87cb0085ed661f66e61935c2bc02a2264859f14b Mon Sep 17 00:00:00 2001 From: FintanH Date: Wed, 22 Jul 2020 18:34:54 +0100 Subject: [PATCH 13/13] Fix docs --- proxy/src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/src/error.rs b/proxy/src/error.rs index 63c595e5a1..d09331b36a 100644 --- a/proxy/src/error.rs +++ b/proxy/src/error.rs @@ -168,7 +168,7 @@ pub enum Error { #[error("while calculating the number of confirmed transactions, we encountered an overflow")] TransactionConfirmationOverflow, - /// We expect at least one [`coco::UserRevisions`] when looking at a project, however the + /// We expect at least one [`coco::Revisions`] when looking at a project, however the /// computation found none. #[error( "while trying to get user revisions we could not find any, there should be at least one"