change the path history options to struct

Summary:
Rust doesn't have named arguments as with positional it's hard to keep track
of all of them if there're many. I'm planning to add one more so let's switc to
struct.

Reviewed By: krallin

Differential Revision: D22999143

fbshipit-source-id: 54dade05f860b41d18bebb52317586015a893919
This commit is contained in:
Mateusz Kwapich 2020-08-24 13:01:06 -07:00 committed by Facebook GitHub Bot
parent 0bd2b1fe28
commit e7daab0dfb
6 changed files with 78 additions and 36 deletions

View File

@ -52,6 +52,12 @@ pub struct ChangesetContext {
Shared<Pin<Box<dyn Future<Output = Result<RootUnodeManifestId, MononokeError>> + Send>>>,
}
#[derive(Default)]
pub struct ChangesetHistoryOptions {
pub until_timestamp: Option<i64>,
pub descendants_of: Option<ChangesetId>,
}
impl fmt::Debug for ChangesetContext {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(
@ -607,10 +613,11 @@ impl ChangesetContext {
/// Returns a stream of `ChangesetContext` for the history of the repository from this commit.
pub async fn history(
&self,
until_timestamp: Option<i64>,
descendants_of: Option<ChangesetId>,
opts: ChangesetHistoryOptions,
) -> impl Stream<Item = Result<ChangesetContext, MononokeError>> + '_ {
let descendants_of = descendants_of.map(|id| Self::new(self.repo().clone(), id));
let descendants_of = opts
.descendants_of
.map(|id| Self::new(self.repo().clone(), id));
if let Some(ancestor) = descendants_of.as_ref() {
// If the the start commit is not descendant of the argument exit early.
match ancestor.is_ancestor_of(self.id()).await {
@ -623,7 +630,7 @@ impl ChangesetContext {
let cs_info_enabled = self.repo.derive_changeset_info_enabled();
// Helper allowing us to terminate walk when we reach `until_timestamp`.
let terminate = until_timestamp.map(move |until_timestamp| {
let terminate = opts.until_timestamp.map(move |until_timestamp| {
move |changeset_id| async move {
let info = if cs_info_enabled {
ChangesetInfo::derive(

View File

@ -50,6 +50,13 @@ pub struct HistoryEntry {
pub changeset_id: ChangesetId,
}
#[derive(Default)]
pub struct ChangesetPathHistoryOptions {
pub until_timestamp: Option<i64>,
pub descendants_of: Option<ChangesetId>,
pub follow_history_across_deletions: bool,
}
pub enum PathEntry {
NotPresent,
Tree(TreeContext),
@ -281,16 +288,14 @@ impl ChangesetPathContext {
/// a history of the path.
pub async fn history(
&self,
until_timestamp: Option<i64>,
descendants_of: Option<ChangesetId>,
follow_history_across_deletions: bool,
opts: ChangesetPathHistoryOptions,
) -> Result<impl Stream<Item = Result<ChangesetContext, MononokeError>> + '_, MononokeError>
{
let ctx = self.changeset.ctx().clone();
let repo = self.repo().blob_repo().clone();
let mpath = self.path.as_mpath();
let descendants_of = match descendants_of {
let descendants_of = match opts.descendants_of {
Some(descendants_of) => Some((
descendants_of,
repo.get_changeset_fetcher()
@ -427,7 +432,7 @@ impl ChangesetPathContext {
};
let cs_info_enabled = self.repo().derive_changeset_info_enabled();
let history_across_deletions = if follow_history_across_deletions {
let history_across_deletions = if opts.follow_history_across_deletions {
HistoryAcrossDeletions::Track
} else {
HistoryAcrossDeletions::DontTrack
@ -439,7 +444,7 @@ impl ChangesetPathContext {
self.changeset.id(),
FilterVisitor {
cs_info_enabled,
until_timestamp,
until_timestamp: opts.until_timestamp,
descendants_of,
cache: HashMap::new(),
skiplist_index: self.repo().skiplist_index().clone(),

View File

@ -41,9 +41,10 @@ pub mod tree;
#[cfg(test)]
mod test;
pub use crate::changeset::{ChangesetContext, Generation};
pub use crate::changeset::{ChangesetContext, ChangesetHistoryOptions, Generation};
pub use crate::changeset_path::{
unified_diff, ChangesetPathContext, CopyInfo, PathEntry, UnifiedDiff, UnifiedDiffMode,
unified_diff, ChangesetPathContext, ChangesetPathHistoryOptions, CopyInfo, PathEntry,
UnifiedDiff, UnifiedDiffMode,
};
pub use crate::changeset_path_diff::ChangesetPathDiffContext;
pub use crate::errors::MononokeError;

View File

@ -15,7 +15,10 @@ use futures::stream::TryStreamExt;
use mononoke_types::DateTime;
use tests_utils::CreateCommitContext;
use crate::{ChangesetId, ChangesetSpecifier, Repo, RepoContext};
use crate::{
ChangesetHistoryOptions, ChangesetId, ChangesetPathHistoryOptions, ChangesetSpecifier, Repo,
RepoContext,
};
// Generates this commit graph:
//
@ -201,9 +204,11 @@ async fn commit_path_history(fb: FacebookInit) -> Result<()> {
// History of file "a" includes commits that modified "a".
let a_path = cs.path("a")?;
let follow_history_across_deletions = true;
let a_history: Vec<_> = a_path
.history(None, None, follow_history_across_deletions)
.history(ChangesetPathHistoryOptions {
follow_history_across_deletions: true,
..Default::default()
})
.await?
.and_then(|cs| async move { Ok(cs.id()) })
.try_collect()
@ -222,7 +227,10 @@ async fn commit_path_history(fb: FacebookInit) -> Result<()> {
// History of directory "dir2" includes commits that modified "dir2/b".
let dir2_path = cs.path("dir2")?;
let dir2_history: Vec<_> = dir2_path
.history(None, None, follow_history_across_deletions)
.history(ChangesetPathHistoryOptions {
follow_history_across_deletions: true,
..Default::default()
})
.await?
.and_then(|cs| async move { Ok(cs.id()) })
.try_collect()
@ -240,7 +248,10 @@ async fn commit_path_history(fb: FacebookInit) -> Result<()> {
// History of directory "dir3" includes some commits on all branches.
let dir3_path = cs.path("dir3")?;
let dir3_history: Vec<_> = dir3_path
.history(None, None, follow_history_across_deletions)
.history(ChangesetPathHistoryOptions {
follow_history_across_deletions: true,
..Default::default()
})
.await?
.and_then(|cs| async move { Ok(cs.id()) })
.try_collect()
@ -262,7 +273,10 @@ async fn commit_path_history(fb: FacebookInit) -> Result<()> {
// Root path history includes all commits except the empty ones.
let root_path = cs.path("")?;
let root_history: Vec<_> = root_path
.history(None, None, follow_history_across_deletions)
.history(ChangesetPathHistoryOptions {
follow_history_across_deletions: true,
..Default::default()
})
.await?
.and_then(|cs| async move { Ok(cs.id()) })
.try_collect()
@ -286,7 +300,11 @@ async fn commit_path_history(fb: FacebookInit) -> Result<()> {
// Setting until_timestamp omits some commits.
let a_history_with_time_filter: Vec<_> = a_path
.history(Some(2500), None, follow_history_across_deletions)
.history(ChangesetPathHistoryOptions {
until_timestamp: Some(2500),
follow_history_across_deletions: true,
..Default::default()
})
.await?
.and_then(|cs| async move { Ok(cs.id()) })
.try_collect()
@ -298,11 +316,11 @@ async fn commit_path_history(fb: FacebookInit) -> Result<()> {
// Setting descendants_of omits more commits.
let a_history_with_descendants_of: Vec<_> = a_path
.history(
None,
Some(changesets["b1"]),
follow_history_across_deletions,
)
.history(ChangesetPathHistoryOptions {
descendants_of: Some(changesets["b1"]),
follow_history_across_deletions: true,
..Default::default()
})
.await?
.and_then(|cs| async move { Ok(cs.id()) })
.try_collect()
@ -327,7 +345,7 @@ async fn commit_history(fb: FacebookInit) -> Result<()> {
// The commit history includes all commits, including empty ones.
let history: Vec<_> = cs
.history(None, None)
.history(Default::default())
.await
.and_then(|cs| async move { Ok(cs.id()) })
.try_collect()
@ -358,7 +376,7 @@ async fn commit_history(fb: FacebookInit) -> Result<()> {
.await?
.expect("changeset exists");
let history: Vec<_> = cs
.history(None, None)
.history(Default::default())
.await
.and_then(|cs| async move { Ok(cs.id()) })
.try_collect()
@ -378,7 +396,10 @@ async fn commit_history(fb: FacebookInit) -> Result<()> {
// Setting until_timestamp omits some commits.
let history: Vec<_> = cs
.history(Some(2500), None)
.history(ChangesetHistoryOptions {
until_timestamp: Some(2500),
..Default::default()
})
.await
.and_then(|cs| async move { Ok(cs.id()) })
.try_collect()
@ -399,7 +420,10 @@ async fn commit_history(fb: FacebookInit) -> Result<()> {
.await?
.expect("changeset exists");
let history: Vec<_> = cs
.history(Some(2500), Some(changesets["b2"]))
.history(ChangesetHistoryOptions {
until_timestamp: Some(2500),
descendants_of: Some(changesets["b2"]),
})
.await
.and_then(|cs| async move { Ok(cs.id()) })
.try_collect()

View File

@ -12,8 +12,8 @@ use context::CoreContext;
use futures::stream::{self, StreamExt, TryStreamExt};
use futures::{future, try_join};
use mononoke_api::{
unified_diff, ChangesetContext, ChangesetId, ChangesetSpecifier, CopyInfo, MononokeError,
MononokePath, UnifiedDiffMode,
unified_diff, ChangesetContext, ChangesetHistoryOptions, ChangesetId, ChangesetSpecifier,
CopyInfo, MononokeError, MononokePath, UnifiedDiffMode,
};
use source_control as thrift;
@ -358,7 +358,12 @@ impl SourceControlServiceImpl {
.into());
}
let history_stream = changeset.history(after_timestamp, descendants_of).await;
let history_stream = changeset
.history(ChangesetHistoryOptions {
until_timestamp: after_timestamp,
descendants_of,
})
.await;
let history = collect_history(
history_stream,
skip,

View File

@ -8,7 +8,7 @@
use context::CoreContext;
use dedupmap::DedupMap;
use futures::future;
use mononoke_api::{ChangesetSpecifier, MononokeError, PathEntry};
use mononoke_api::{ChangesetPathHistoryOptions, ChangesetSpecifier, MononokeError, PathEntry};
use source_control as thrift;
use std::borrow::Cow;
use std::collections::{BTreeSet, HashMap};
@ -236,11 +236,11 @@ impl SourceControlServiceImpl {
}
let history_stream = path
.history(
after_timestamp.clone(),
.history(ChangesetPathHistoryOptions {
until_timestamp: after_timestamp.clone(),
descendants_of,
params.follow_history_across_deletions,
)
follow_history_across_deletions: params.follow_history_across_deletions,
})
.await?;
let history = collect_history(
history_stream,