From 50a56aadee435b2369a4f709ba939f838adf3216 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Wed, 10 Apr 2024 18:02:04 +0100 Subject: [PATCH] fix(server): Doesn't properly detect when dev or next are ahead Closes kemitix/git-next#23 --- src/server/actors/repo/branch.rs | 10 +++- src/server/actors/repo/mod.rs | 12 ++++- src/server/actors/repo/status.rs | 11 +++-- src/server/forge/forgejo/mod.rs | 85 +++++++++++++++++++++----------- 4 files changed, 83 insertions(+), 35 deletions(-) diff --git a/src/server/actors/repo/branch.rs b/src/server/actors/repo/branch.rs index ba3f081..6d7456a 100644 --- a/src/server/actors/repo/branch.rs +++ b/src/server/actors/repo/branch.rs @@ -44,7 +44,10 @@ pub async fn validate_positions( ); return; } - let next = next_commits[0].clone(); + let Some(next) = next_commits.first().cloned() else { + warn!("No commits on next branch '{}'", config.branches().next()); + return; + }; let dev_has_next = commit_histories .dev .iter() @@ -66,7 +69,10 @@ pub async fn validate_positions( ); return; } - let dev = commit_histories.dev[0].clone(); + let Some(dev) = commit_histories.dev.first().cloned() else { + warn!("No commits on dev branch '{}'", config.branches().dev()); + return; + }; addr.do_send(StartMonitoring { main, next, diff --git a/src/server/actors/repo/mod.rs b/src/server/actors/repo/mod.rs index d2e3dad..6f57a3b 100644 --- a/src/server/actors/repo/mod.rs +++ b/src/server/actors/repo/mod.rs @@ -80,7 +80,7 @@ impl Handler for RepoActor { } } -#[derive(Message)] +#[derive(Debug, Message)] #[rtype(result = "()")] pub struct StartMonitoring { pub main: forge::Commit, @@ -90,6 +90,7 @@ pub struct StartMonitoring { } impl Handler for RepoActor { type Result = (); + #[allow(clippy::cognitive_complexity)] // TODO: (#25) this function is complex fn handle(&mut self, msg: StartMonitoring, ctx: &mut Self::Context) -> Self::Result { info!("Monitoring started"); let Some(repo_config) = self.config.clone() else { @@ -101,13 +102,21 @@ impl Handler for RepoActor { let addr = ctx.address(); let net = self.net.clone(); + info!(%msg.main, %msg.next, %msg.dev, "Checking positions"); let next_ahead_of_main = msg.main != msg.next; let dev_ahead_of_next = msg.next != msg.dev; + info!( + ?next_ahead_of_main, + ?dev_ahead_of_next, + "Checking positions" + ); if next_ahead_of_main { + info!("Next is ahead of main"); status::check_next(msg.next, repo_details, addr, net) .into_actor(self) .wait(ctx); } else if dev_ahead_of_next { + info!("Dev is ahead of next"); branch::advance_next( msg.next, msg.dev_commit_history, @@ -118,6 +127,7 @@ impl Handler for RepoActor { .into_actor(self) .wait(ctx); } else if self.webhook_id.is_none() { + info!("No webhook registered"); webhook::register(repo_details, addr, net) .into_actor(self) .wait(ctx) diff --git a/src/server/actors/repo/status.rs b/src/server/actors/repo/status.rs index 0594f4c..fce0ae4 100644 --- a/src/server/actors/repo/status.rs +++ b/src/server/actors/repo/status.rs @@ -1,4 +1,5 @@ use actix::prelude::*; +use gix::trace::warn; use tracing::info; use crate::server::{ @@ -25,10 +26,14 @@ pub async fn check_next( Status::Pass => { addr.do_send(AdvanceMainTo(next)); } - Status::Pending => (), // TODO: (#22) wait and try again OR can webhook tell us when it's done, in + Status::Pending => { + info!("Checks are pending"); + } // TODO: (#22) wait and try again OR can webhook tell us when it's done, in // which case we can do nothing here and wait for the webhook to trigger - Status::Fail => (), // TODO: (#21) reset next and wait for dev to be updated and this - // commit removed from the commit history before trying again + Status::Fail => { + warn!("Check have failed"); + } // TODO: (#21) reset next and wait for dev to be updated and this + // commit removed from the commit history before trying again } } diff --git a/src/server/forge/forgejo/mod.rs b/src/server/forge/forgejo/mod.rs index c244811..8013747 100644 --- a/src/server/forge/forgejo/mod.rs +++ b/src/server/forge/forgejo/mod.rs @@ -1,6 +1,6 @@ use kxio::network; use terrors::OneOf; -use tracing::{debug, error, info, warn}; +use tracing::{error, info, warn}; use crate::server::{actors::repo::status::Status, config::BranchName, forge}; @@ -13,9 +13,17 @@ pub async fn get_commit_histories( config: &crate::server::config::RepoConfig, net: &kxio::network::Network, ) -> Result> { - let main = (get_commit_history(repo_details, &config.branches().main(), net).await)?; - let next = (get_commit_history(repo_details, &config.branches().next(), net).await)?; - let dev = (get_commit_history(repo_details, &config.branches().dev(), net).await)?; + let main = (get_commit_history(repo_details, &config.branches().main(), None, net).await)?; + let next = + (get_commit_history(repo_details, &config.branches().next(), Some(&main[0]), net).await)?; + let dev = + (get_commit_history(repo_details, &config.branches().dev(), Some(&next[0]), net).await)?; + info!( + main = main.len(), + next = next.len(), + dev = dev.len(), + "Commit histories" + ); let histories = CommitHistories { main, next, dev }; Ok(histories) } @@ -24,39 +32,58 @@ pub async fn get_commit_histories( async fn get_commit_history( repo_details: &crate::server::config::RepoDetails, branch_name: &BranchName, + find_commit: Option<&forge::Commit>, // INFO: (#23) if [None] then get only one commit, if [Some] then get all commits up to this one or return an error net: &kxio::network::Network, ) -> Result, OneOf<(network::NetworkError,)>> { let hostname = &repo_details.forge.hostname; let path = &repo_details.repo; let token = &repo_details.forge.token; - let url = network::NetUrl::new(format!( - "https://{hostname}/api/v1/repos/{path}/commits?sha={branch_name}&stat=false&files=false&token={token}" + let mut page = 1; + let limit = match find_commit { + Some(_) => 100, + None => 1, + }; + let mut all_commits = Vec::new(); + loop { + let url = network::NetUrl::new(format!( + "https://{hostname}/api/v1/repos/{path}/commits?sha={branch_name}&stat=false&verification=false&files=false&token={token}&page={page}&limit={limit}" )); - debug!(%url, "Fetching commits"); - let request = network::NetRequest::new( - network::RequestMethod::Get, - url, - network::NetRequestHeaders::new(), - network::RequestBody::None, - network::ResponseType::Json, - None, - network::NetRequestLogging::None, - ); - let result = net.get::>(request).await; - let response = result.map_err(|e| { - error!(?e, "Failed to get commit history"); - OneOf::new(e) - })?; - let commits = response - .response_body() - .unwrap_or_default() - .into_iter() - .map(|commit| commit.sha) - .map(|sha| forge::Commit { sha }) - .collect(); + info!(%url, "Fetching commits"); + let request = network::NetRequest::new( + network::RequestMethod::Get, + url, + network::NetRequestHeaders::new(), + network::RequestBody::None, + network::ResponseType::Json, + None, + network::NetRequestLogging::None, + ); + let result = net.get::>(request).await; + let response = result.map_err(|e| { + error!(?e, "Failed to get commit history"); + OneOf::new(e) + })?; + let commits = response + .response_body() + .unwrap_or_default() + .into_iter() + .map(|commit| commit.sha) + .map(|sha| forge::Commit { sha }) + .collect::>(); - Ok(commits) + let found = find_commit.map_or(true, |find_commit| { + commits.iter().any(|commit| commit == find_commit) + }); + let at_end = commits.len() < limit; + all_commits.extend(commits); + if found || at_end { + break; + } + page += 1; + } + + Ok(all_commits) } #[allow(dead_code)]