use cache to speed up config checking

src/config.rs

@@ -161,15 +161,11 @@ pub(crate) struct PrioritizeConfig {
 }
 
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
-pub(crate) struct ValidateConfig {
-    diff_history_size: usize,
-}
+pub(crate) struct ValidateConfig {}
 
 impl ValidateConfig {
     fn default() -> Option<Self> {
-        Some(ValidateConfig {
-            diff_history_size: 128,
-        })
+        Some(ValidateConfig {})
     }
 }
 
@@ -433,7 +429,7 @@ mod tests {
                 note: Some(NoteConfig { _empty: () }),
                 ping: Some(PingConfig { teams: ping_teams }),
                 nominate: Some(NominateConfig {
-                    teams: nominate_teams
+                    teams: nominate_teams,
                 }),
                 shortcut: Some(ShortcutConfig { _empty: () }),
                 prioritize: None,

src/github.rs

@@ -977,7 +977,7 @@ struct PullRequestEventFields {}
 
 #[derive(Clone, Debug, serde::Deserialize)]
 pub struct CommitBase {
-    sha: String,
+    pub sha: String,
     #[serde(rename = "ref")]
     pub git_ref: String,
     pub repo: Repository,

src/handlers/validate_config.rs

@@ -3,32 +3,24 @@
 //! It won't validate anything unless the PR is open and has changed.
 //!
 //! These are the implementation steps as follows:
-//! 1. Fetch the diff using the following url:
-//!     https://api.github.com/repos/rust-lang/triagebot/compare/master...meysam81:master
-//! 2. The last step will return a JSON response with the `files` field as an
-//!    array of objects, each having `filename` in their fields. We check if
-//!    `triagebot.toml` is among them, and continue to the next step. Otherwise
-//!    we break out of the validation process.
-//! 3. Fetch the raw file from the head branch with the secon HTTP request
-//!    (two network calls so far):
-//!    https://raw.githubusercontent.com/meysam81/triagebot/master/triagebot.toml
-//! 4. Use the `toml` crate and the `Config` module to parse **and** validate
-//!    the contents of the file.
-//! 5. This last step will return error if the file is not a valid config file.
-//!
-//! Considerations:
-//! - HTTP calls are network requests, prone to failure and expensive in nature
-//!   which is why we are using a fixed LIFO hash map to store the value if the
-//!   URL hasn't changed e.g. no new commits has been pushed.
-//!
+//!   1. If the issue is of type pull request and it is open, then continue.
+//!   2. Fetch the triagebot.toml from the HEAD branch of the PR.
+//!   3. If the triagebot.toml is valid, then continue.
+//!   4. If the triagebot.toml is invalid, then report the error.
+
+use crate::{ config::ValidateConfig, github::IssuesAction, handlers::{ Context, IssuesEvent } };
 
-use crate::{
-    config::ValidateConfig,
-    github::IssuesAction,
-    handlers::{Context, IssuesEvent},
-};
+use once_cell::sync::Lazy;
+use std::{ sync::Mutex };
 use tracing as log;
 
+/// Caching the content of triagebot.toml across commit SHA to save network round-trip
+/// time. The content will look like this:
+/// COMMIT_SHA => CONTENT_OF_TRIAGEBOT_TOML
+static TRIAGEBOT_CACHE: Lazy<Mutex<LifoHashMap<String, Vec<u8>>>> = Lazy::new(||
+    Mutex::new(LifoHashMap::new(128))
+);
+
 mod lifo;
 use lifo::LifoHashMap;
 
@@ -38,89 +30,89 @@ use lifo::LifoHashMap;
 pub(super) async fn parse_input(
     ctx: &Context,
     event: &IssuesEvent,
-    _config: Option<&ValidateConfig>,
+    _config: Option<&ValidateConfig>
 ) -> Result<Option<()>, String> {
     if !event.issue.is_pr() {
         log::debug!("Ignoring issue: {:?}", event.issue);
         return Ok(None);
     }
 
-    if !matches!(
-        event.action,
-        IssuesAction::Opened | IssuesAction::Synchronize | IssuesAction::ReadyForReview,
-    ) {
+    if
+        !matches!(
+            event.action,
+            IssuesAction::Opened | IssuesAction::Synchronize | IssuesAction::ReadyForReview
+        )
+    {
         log::debug!("Ignoring issue action: {:?}", event.action);
         return Ok(None);
     }
 
-    let diff = match event.issue.diff(&ctx.github).await {
-        Ok(Some(diff)) => diff,
-        wildcard => {
-            log::error!("Failed to fetch diff: {:?}", wildcard);
-            return Ok(None);
+    log::debug!("Validating triagebot config");
+    match validate_triagebot_config(ctx, event).await {
+        Ok(()) => {
+            log::debug!("Valid triagebot config");
+            Ok(Some(()))
         }
-    };
-
-    if diff.contains(crate::config::CONFIG_FILE_NAME) {
-        log::debug!("Validating triagebot config");
-        match validate_triagebot_config(ctx, event).await {
-            Ok(()) => {
-                log::debug!("Valid triagebot config");
-                return Ok(Some(()));
-            }
-            Err(e) => {
-                log::error!("Invalid triagebot config: {:?}", e);
-                return Err(e);
-            }
+        Err(e) => {
+            log::error!("Invalid triagebot config: {:?}", e);
+            Err(e)
         }
     }
-
-    Ok(None)
 }
 
-async fn get_triagebot_config_blob_url(repo: &str, git_ref: &str) -> String {
-    // https://github.com/rust-lang/triagebot/blob/master/triagebot.toml
-    format!(
-        "https://github.com/{}/blob/{}/{}",
+async fn get_triagebot_content(repo: &str, git_ref: &str, sha: &str) -> Vec<u8> {
+    // Try to hit the cache first
+    if let Some(triagebot_toml) = TRIAGEBOT_CACHE.lock().unwrap().get(&sha.to_string()) {
+        log::info!("Cache hit for triagebot.toml for repository {} and commit {}", repo, sha);
+        return triagebot_toml.clone();
+    }
+
+    log::warn!("Cache miss for triagebot.toml for repository {} and commit {}", repo, sha);
+
+    let url = format!(
+        "https://raw.githubusercontent.com/{}/{}/{}",
         repo,
         git_ref,
         crate::config::CONFIG_FILE_NAME
-    )
+    );
+
+    let triagebot_toml = reqwest::get(&url).await.unwrap().text().await.unwrap().into_bytes();
+
+    TRIAGEBOT_CACHE.lock().unwrap().push(sha.to_string(), triagebot_toml.clone());
+
+    triagebot_toml
 }
 
-async fn validate_triagebot_config(ctx: &Context, event: &IssuesEvent) -> Result<(), String> {
+async fn validate_triagebot_config(_ctx: &Context, event: &IssuesEvent) -> Result<(), String> {
     if let Some(pr_source) = &event.issue.head {
-        if let Ok(Some(triagebot_toml)) = ctx
-            .github
-            .raw_file(
-                &pr_source.repo.full_name,
-                &pr_source.git_ref,
-                crate::config::CONFIG_FILE_NAME,
-            )
-            .await
-        {
-            match toml::from_slice::<crate::handlers::Config>(&triagebot_toml) {
-                Ok(_) => return Ok(()),
-                Err(e) => {
-                    let position = e
-                        .line_col()
-                        .map(|(line, col)| format!("{}:{}", line + 1, col + 1));
-
-                    let absolute_url = format!(
-                        "{}#L{}",
-                        get_triagebot_config_blob_url(
-                            &pr_source.repo.full_name,
-                            &pr_source.git_ref
-                        )
-                        .await,
-                        position.clone().unwrap_or_default()
-                    );
-
-                    return Err(format!(
+        let triagebot_content = get_triagebot_content(
+            &pr_source.repo.full_name,
+            &pr_source.git_ref,
+            &pr_source.sha
+        ).await;
+
+        match toml::from_slice::<crate::handlers::Config>(&triagebot_content) {
+            Ok(_) => {
+                return Ok(());
+            }
+            Err(e) => {
+                let position = e.line_col().map(|(line, col)| format!("{}:{}", line + 1, col + 1));
+
+                // https://github.com/rust-lang/triagebot/blob/master/triagebot.toml
+                let absolute_url = format!(
+                    "https://github.com/{}/blob/{}/{}#L{}",
+                    pr_source.repo.full_name,
+                    pr_source.git_ref,
+                    crate::config::CONFIG_FILE_NAME,
+                    position.clone().unwrap_or_default()
+                );
+
+                return Err(
+                    format!(
                         "Invalid `triagebot.toml` at position {}",
                         format!("[{}]({})", position.unwrap_or_default(), absolute_url)
-                    ));
-                }
+                    )
+                );
             }
         }
     }
@@ -132,7 +124,7 @@ pub(super) async fn handle_input(
     _ctx: &Context,
     _config: &ValidateConfig,
     _event: &IssuesEvent,
-    _input: (),
+    _input: ()
 ) -> anyhow::Result<()> {
     Ok(())
 }

src/handlers/validate_config/lifo.rs

@@ -40,11 +40,4 @@ where
         }
         None
     }
-
-    pub fn remove(&mut self, key: &K) {
-        if let Some(index) = self.map.get(key) {
-            self.entries[*index] = None;
-            self.map.remove(key);
-        }
-    }
 }