radicle-surf: use specialised iterators for names

The `tag_names` and `branch_names` functions use the `Tags` and
`Branches` functions which require to go through their `TryFrom`
instances. This can be expensive, in the case of `Tags` especially.

Add `TagNames` and `BranchNames` iterators that only return the names
of the tags and branches, respectively.

This requires to change the definition of Revisions, where the
NonEmpty branch list is now foregone and there references are
collected in a BTreeSet to ensure ordering.

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett
This commit is contained in:
Fintan Halpenny
2022-11-18 11:21:06 +00:00
parent 08c311b640
commit d17cb3a8d3
4 changed files with 143 additions and 88 deletions
+7 -17
View File
@@ -23,7 +23,7 @@ use std::{
};
use directory::{Directory, FileContent};
use git_ref_format::{refspec::QualifiedPattern, Qualified, RefString};
use git_ref_format::{refspec::QualifiedPattern, Qualified};
use radicle_git_ext::Oid;
use thiserror::Error;
@@ -51,6 +51,8 @@ use crate::{
pub mod iter;
pub use iter::{Branches, Namespaces, Tags};
use self::iter::{BranchNames, TagNames};
/// Enumeration of errors that can occur in operations from [`crate::git`].
#[derive(Debug, Error)]
#[non_exhaustive]
@@ -336,25 +338,13 @@ impl<'a> RepositoryRef<'a> {
}
/// Lists branch names with `filter`.
pub fn branch_names(&self, filter: &Glob<Branch>) -> Result<Vec<RefString>, Error> {
let mut branches = self
.branches(filter)?
.map(|b| b.map(|b| b.refname().into()))
.collect::<Result<Vec<_>, _>>()?;
branches.sort();
Ok(branches)
pub fn branch_names(&self, filter: &Glob<Branch>) -> Result<BranchNames, Error> {
Ok(self.branches(filter)?.names())
}
/// Lists tag names in the local RefScope.
pub fn tag_names(&self) -> Result<Vec<RefString>, Error> {
let mut tags = self
.tags(&Glob::tags("*")?)?
.map(|t| t.map_err(Error::from).map(|t| t.refname().into()))
.collect::<Result<Vec<_>, Error>>()?;
tags.sort();
Ok(tags)
pub fn tag_names(&self) -> Result<TagNames, Error> {
Ok(self.tags(&Glob::tags("*")?)?.names())
}
/// Returns the Oid of the current HEAD
+69 -3
View File
@@ -6,19 +6,30 @@ use std::{
convert::TryFrom as _,
};
use crate::git::{Branch, Namespace, Tag};
use git_ref_format::{lit, Qualified};
/// An iterator for tags.
use crate::git::{tag, Branch, Namespace, Tag};
/// Iterator over [`Tag`]s.
#[derive(Default)]
pub struct Tags<'a> {
references: Vec<git2::References<'a>>,
current: usize,
}
/// Iterator over the [`Qualified`] names of [`Tag`]s.
pub struct TagNames<'a> {
inner: Tags<'a>,
}
impl<'a> Tags<'a> {
pub(super) fn push(&mut self, references: git2::References<'a>) {
self.references.push(references)
}
pub fn names(self) -> TagNames<'a> {
TagNames { inner: self }
}
}
impl<'a> Iterator for Tags<'a> {
@@ -43,17 +54,49 @@ impl<'a> Iterator for Tags<'a> {
}
}
/// An iterator for branches.
impl<'a> Iterator for TagNames<'a> {
type Item = Result<Qualified<'static>, error::Tag>;
fn next(&mut self) -> Option<Self::Item> {
while self.inner.current < self.inner.references.len() {
match self.inner.references.get_mut(self.inner.current) {
Some(refs) => match refs.next() {
Some(res) => {
return Some(res.map_err(error::Tag::from).and_then(|r| {
tag::reference_name(&r)
.map(|name| lit::refs_tags(name).into())
.map_err(error::Tag::from)
}))
},
None => self.inner.current += 1,
},
None => break,
}
}
None
}
}
/// Iterator over [`Branch`]es.
#[derive(Default)]
pub struct Branches<'a> {
references: Vec<git2::References<'a>>,
current: usize,
}
/// Iterator over the [`Qualified`] names of [`Branch`]es.
pub struct BranchNames<'a> {
inner: Branches<'a>,
}
impl<'a> Branches<'a> {
pub(super) fn push(&mut self, references: git2::References<'a>) {
self.references.push(references)
}
pub fn names(self) -> BranchNames<'a> {
BranchNames { inner: self }
}
}
impl<'a> Iterator for Branches<'a> {
@@ -78,6 +121,29 @@ impl<'a> Iterator for Branches<'a> {
}
}
impl<'a> Iterator for BranchNames<'a> {
type Item = Result<Qualified<'static>, error::Branch>;
fn next(&mut self) -> Option<Self::Item> {
while self.inner.current < self.inner.references.len() {
match self.inner.references.get_mut(self.inner.current) {
Some(refs) => match refs.next() {
Some(res) => {
return Some(res.map_err(error::Branch::from).and_then(|r| {
Branch::try_from(&r)
.map(|branch| branch.refname().into_owned())
.map_err(error::Branch::from)
}))
},
None => self.inner.current += 1,
},
None => break,
}
}
None
}
}
// TODO: not sure this buys us much
/// An iterator for namespaces.
pub struct Namespaces {
+34 -29
View File
@@ -117,35 +117,40 @@ impl TryFrom<&git2::Reference<'_>> for Tag {
type Error = error::FromReference;
fn try_from(reference: &git2::Reference) -> Result<Self, Self::Error> {
let name = {
let name = str::from_utf8(reference.name_bytes())?;
RefStr::try_from_str(name)?
.qualified()
.ok_or_else(|| error::FromReference::NotQualified(name.to_string()))?
};
let (_refs, tags, c, cs) = name.non_empty_components();
if tags == component::TAGS {
match reference.peel_to_tag() {
Ok(tag) => Tag::try_from(&tag).map_err(error::FromReference::from),
// If we get an error peeling to a tag _BUT_ we also have confirmed the
// reference is a tag, that means we have a lightweight tag,
// i.e. a commit SHA and name.
Err(err)
if err.class() == git2::ErrorClass::Object
&& err.code() == git2::ErrorCode::InvalidSpec =>
{
let commit = reference.peel_to_commit()?;
Ok(Tag::Light {
id: commit.id().into(),
name: refstr_join(c, cs),
})
},
Err(err) => Err(err.into()),
}
} else {
Err(error::FromReference::NotTag(name.into()))
let name = reference_name(reference)?;
match reference.peel_to_tag() {
Ok(tag) => Tag::try_from(&tag).map_err(error::FromReference::from),
// If we get an error peeling to a tag _BUT_ we also have confirmed the
// reference is a tag, that means we have a lightweight tag,
// i.e. a commit SHA and name.
Err(err)
if err.class() == git2::ErrorClass::Object
&& err.code() == git2::ErrorCode::InvalidSpec =>
{
let commit = reference.peel_to_commit()?;
Ok(Tag::Light {
id: commit.id().into(),
name,
})
},
Err(err) => Err(err.into()),
}
}
}
pub(crate) fn reference_name(
reference: &git2::Reference,
) -> Result<RefString, error::FromReference> {
let name = str::from_utf8(reference.name_bytes())?;
let name = RefStr::try_from_str(name)?
.qualified()
.ok_or_else(|| error::FromReference::NotQualified(name.to_string()))?;
let (_refs, tags, c, cs) = name.non_empty_components();
if tags == component::TAGS {
Ok(refstr_join(c, cs))
} else {
Err(error::FromReference::NotTag(name.into()))
}
}
+33 -39
View File
@@ -17,8 +17,9 @@
//! Represents revisions
use std::collections::BTreeSet;
use git_ref_format::{lit, Qualified, RefString};
use nonempty::NonEmpty;
#[cfg(feature = "serialize")]
use serde::{Deserialize, Serialize};
@@ -104,9 +105,9 @@ pub struct Revisions<P, U> {
/// The user who owns these revisions.
pub user: U,
/// List of branch reference names.
pub branches: NonEmpty<RefString>,
pub branches: BTreeSet<RefString>,
/// List of tag reference names.
pub tags: Vec<RefString>,
pub tags: BTreeSet<RefString>,
}
/// Provide the [`Revisions`] for the given `peer_id`, looking for the
@@ -117,26 +118,22 @@ pub struct Revisions<P, U> {
/// # Errors
///
/// * If we cannot get the branches from the `Browser`
pub fn remote<P, U>(
repo: &RepositoryRef,
peer_id: P,
user: U,
) -> Result<Option<Revisions<P, U>>, Error>
pub fn remote<P, U>(repo: &RepositoryRef, peer_id: P, user: U) -> Result<Revisions<P, U>, Error>
where
P: Clone + ToString,
{
let remote_branches =
repo.branch_names(&Glob::remotes(&format!("{}/*", peer_id.to_string()))?)?;
Ok(
NonEmpty::from_vec(remote_branches).map(|branches| Revisions {
peer_id,
user,
branches,
// TODO(rudolfs): implement remote peer tags once we decide how
// https://radicle.community/t/git-tags/214
tags: vec![],
}),
)
let branches = repo
.branch_names(&Glob::remotes(&format!("{}/*", peer_id.to_string()))?)?
.map(|name| name.map(RefString::from))
.collect::<Result<_, _>>()?;
Ok(Revisions {
peer_id,
user,
branches,
// TODO(rudolfs): implement remote peer tags once we decide how
// https://radicle.community/t/git-tags/214
tags: BTreeSet::new(),
})
}
/// Provide the [`Revisions`] for the given `peer_id`, looking for the
@@ -147,24 +144,24 @@ where
/// # Errors
///
/// * If we cannot get the branches from the `Browser`
pub fn local<P, U>(
repo: &RepositoryRef,
peer_id: P,
user: U,
) -> Result<Option<Revisions<P, U>>, Error>
pub fn local<P, U>(repo: &RepositoryRef, peer_id: P, user: U) -> Result<Revisions<P, U>, Error>
where
P: Clone + ToString,
{
let local_branches = repo.branch_names(&Glob::heads("*")?)?;
let tags = repo.tag_names()?;
Ok(
NonEmpty::from_vec(local_branches).map(|branches| Revisions {
peer_id,
user,
branches,
tags,
}),
)
let branches = repo
.branch_names(&Glob::heads("*")?)?
.map(|name| name.map(RefString::from))
.collect::<Result<_, _>>()?;
let tags = repo
.tag_names()?
.map(|name| name.map(RefString::from))
.collect::<Result<_, _>>()?;
Ok(Revisions {
peer_id,
user,
branches,
tags,
})
}
/// Provide the [`Revisions`] of a peer.
@@ -178,10 +175,7 @@ where
/// # Errors
///
/// * If we cannot get the branches from the `Browser`
pub fn revisions<P, U>(
repo: &RepositoryRef,
peer: Category<P, U>,
) -> Result<Option<Revisions<P, U>>, Error>
pub fn revisions<P, U>(repo: &RepositoryRef, peer: Category<P, U>) -> Result<Revisions<P, U>, Error>
where
P: Clone + ToString,
{