Merge pull request #69 from vohoanglong0107/refactor-move-comment-internal

refactor: move is_internal_comment to per repo

Author: Kobzol

src/bors/handlers/mod.rs

@@ -33,12 +33,14 @@ pub async fn handle_bors_event<Client: RepositoryClient>(
     match event {
         BorsEvent::Comment(comment) => {
             // We want to ignore comments made by this bot
-            if state.is_comment_internal(&comment) {
-                tracing::trace!("Ignoring comment {comment:?} because it was authored by this bot");
-                return Ok(());
-            }
-
             if let Some(repo) = get_repo_state(state, &comment.repository) {
+                if repo.client.is_comment_internal(&comment).await? {
+                    tracing::trace!(
+                        "Ignoring comment {comment:?} because it was authored by this bot"
+                    );
+                    return Ok(());
+                }
+
                 let span = tracing::info_span!(
                     "Comment",
                     pr = format!("{}#{}", comment.repository, comment.pr_number),

src/bors/mod.rs

@@ -24,6 +24,9 @@ pub use handlers::handle_bors_event;
 pub trait RepositoryClient: Send + Sync {
     fn repository(&self) -> &GithubRepoName;
 
+    /// Was the comment created by the bot?
+    async fn is_comment_internal(&self, comment: &PullRequestComment) -> anyhow::Result<bool>;
+
     /// Load repository config.
     async fn load_config(&self) -> anyhow::Result<RepositoryConfig>;
 
@@ -85,9 +88,6 @@ pub struct CheckSuite {
 /// It is behind a trait to allow easier mocking in tests.
 #[async_trait]
 pub trait BorsState<Client: RepositoryClient>: Send + Sync {
-    /// Was the comment created by the bot?
-    fn is_comment_internal(&self, comment: &PullRequestComment) -> bool;
-
     /// Get repository and database state for the given repository name.
     fn get_repo_state(&self, repo: &GithubRepoName) -> Option<Arc<RepositoryState<Client>>>;
 

src/github/api/client.rs

@@ -1,10 +1,11 @@
 use anyhow::Context;
 use axum::async_trait;
 use base64::Engine;
-use octocrab::models::{Repository, RunId};
+use octocrab::models::{App, Repository, RunId};
 use octocrab::{Error, Octocrab};
 use tracing::log;
 
+use crate::bors::event::PullRequestComment;
 use crate::bors::{CheckSuite, CheckSuiteStatus, Comment, RepositoryClient};
 use crate::config::{RepositoryConfig, CONFIG_FILE_PATH};
 use crate::github::api::operations::{merge_branches, set_branch_to_commit, MergeError};
@@ -13,6 +14,7 @@ use crate::github::{Branch, CommitSha, GithubRepoName, PullRequest, PullRequestN
 
 /// Provides access to a single app installation (repository) using the GitHub API.
 pub struct GithubRepositoryClient {
+    pub app: App,
     /// The client caches the access token for this given repository and refreshes it once it
     /// expires.
     pub client: Octocrab,
@@ -42,6 +44,11 @@ impl RepositoryClient for GithubRepositoryClient {
         self.name()
     }
 
+    /// Was the comment created by the bot?
+    async fn is_comment_internal(&self, comment: &PullRequestComment) -> anyhow::Result<bool> {
+        Ok(comment.author.html_url == self.app.html_url)
+    }
+
     /// Loads repository configuration from a file located at `[CONFIG_FILE_PATH]` in the main
     /// branch.
     async fn load_config(&self) -> anyhow::Result<RepositoryConfig> {

src/github/api/mod.rs

@@ -4,13 +4,12 @@ use std::sync::Arc;
 use anyhow::Context;
 use arc_swap::ArcSwap;
 use axum::async_trait;
-use octocrab::models::{App, AppId, InstallationRepositories, Repository};
+use octocrab::models::{AppId, InstallationRepositories, Repository};
 use octocrab::Octocrab;
 use secrecy::{ExposeSecret, SecretVec};
 
 use client::GithubRepositoryClient;
 
-use crate::bors::event::PullRequestComment;
 use crate::bors::{BorsState, RepositoryClient, RepositoryState};
 use crate::github::GithubRepoName;
 use crate::permissions::load_permissions;
@@ -32,7 +31,6 @@ fn base_github_url() -> &'static str {
 
 /// Provides access to managed GitHub repositories.
 pub struct GithubAppState {
-    app: App,
     client: Octocrab,
     repositories: ArcSwap<RepositoryMap>,
 }
@@ -48,15 +46,8 @@ impl GithubAppState {
             .build()
             .context("Could not create octocrab builder")?;
 
-        let app = client
-            .current()
-            .app()
-            .await
-            .context("Could not load Github App")?;
-
         let repositories = load_repositories(&client).await?;
         Ok(GithubAppState {
-            app,
             client,
             repositories: ArcSwap::new(Arc::new(repositories)),
         })
@@ -135,7 +126,13 @@ async fn create_repo_state(
     let name = GithubRepoName::new(&owner.login, &repo.name);
     tracing::info!("Found repository {name}");
 
+    let app = repo_client
+        .current()
+        .app()
+        .await
+        .context("Could not load Github App")?;
     let client = GithubRepositoryClient {
+        app,
         client: repo_client,
         repo_name: name.clone(),
         repository: repo,
@@ -167,10 +164,6 @@ async fn create_repo_state(
 
 #[async_trait]
 impl BorsState<GithubRepositoryClient> for GithubAppState {
-    fn is_comment_internal(&self, comment: &PullRequestComment) -> bool {
-        comment.author.html_url == self.app.html_url
-    }
-
     fn get_repo_state(
         &self,
         repo: &GithubRepoName,

src/tests/state.rs

@@ -135,10 +135,6 @@ impl TestBorsState {
 
 #[async_trait]
 impl BorsState<Arc<TestRepositoryClient>> for TestBorsState {
-    fn is_comment_internal(&self, comment: &PullRequestComment) -> bool {
-        comment.author == test_bot_user()
-    }
-
     fn get_repo_state(&self, repo: &GithubRepoName) -> Option<Arc<TestRepositoryState>> {
         self.repos.get(repo).map(|repo| Arc::clone(repo))
     }
@@ -393,6 +389,10 @@ impl RepositoryClient for Arc<TestRepositoryClient> {
         &self.name
     }
 
+    async fn is_comment_internal(&self, comment: &PullRequestComment) -> anyhow::Result<bool> {
+        Ok(comment.author == test_bot_user())
+    }
+
     async fn get_branch_sha(&self, name: &str) -> anyhow::Result<CommitSha> {
         let sha = self
             .branch_history