Skip to content

Commit

Permalink
wip: cache the diffs if not commit was pushed
Browse files Browse the repository at this point in the history
  • Loading branch information
meysam81 committed Oct 23, 2023
1 parent 2b067f7 commit 7c6abec
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 5 deletions.
8 changes: 6 additions & 2 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,15 @@ pub(crate) struct PrioritizeConfig {
}

#[derive(PartialEq, Eq, Debug, serde::Deserialize)]
pub(crate) struct ValidateConfig {}
pub(crate) struct ValidateConfig {
diff_history_size: usize,
}

impl ValidateConfig {
fn default() -> Option<Self> {
Some(ValidateConfig {})
Some(ValidateConfig {
diff_history_size: 128,
})
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -846,11 +846,12 @@ impl Issue {
}
}

#[derive(Debug, serde::Deserialize)]
#[derive(Debug, serde::Deserialize, Clone)]
pub struct PullRequestFile {
pub sha: String,
pub filename: String,
pub blob_url: String,
pub raw_url: String,
}

#[derive(serde::Serialize)]
Expand Down
25 changes: 23 additions & 2 deletions src/handlers/validate_config.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,26 @@
//! For pull requests that have changed the triagebot.toml, validate that the
//! changes are a valid configuration file.
//! It won't validate anything unless the PR is open and has changed.
//! For every event, it will fetch the diff of the pull request which is one
//! round-trip of HTTP request to the GitHub API.
//!
//! 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.
//!

use crate::{
config::ValidateConfig,
Expand All @@ -11,6 +29,9 @@ use crate::{
};
use tracing as log;

mod lifo;
use lifo::LifoHashMap;

/// If the issue is a pull request and if among the changed files, triagebot.toml
/// is changed, then validate the triagebot.toml config file with the syntax
/// parser from the config module.
Expand Down
50 changes: 50 additions & 0 deletions src/handlers/validate_config/lifo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use std::collections::HashMap;

pub(super) struct LifoHashMap<K, V> {
capacity: usize,
entries: Vec<Option<(K, V)>>,
map: HashMap<K, usize>,
}

impl<K, V> LifoHashMap<K, V>
where
K: Eq + std::hash::Hash + std::clone::Clone,
V: std::clone::Clone,
{
pub fn new(capacity: usize) -> Self {
Self {
capacity,
entries: vec![None; capacity],
map: HashMap::with_capacity(capacity),
}
}

pub fn push(&mut self, key: K, value: V) {
if self.map.len() >= self.capacity {
if let Some(oldest) = self.entries.pop() {
if let Some((oldest_key, _)) = oldest {
self.map.remove(&oldest_key);
}
}
}

self.entries.insert(0, Some((key.clone(), value)));
self.map.insert(key, 0);
}

pub fn get(&self, key: &K) -> Option<&V> {
if let Some(index) = self.map.get(key) {
if let Some(entry) = &self.entries[*index] {
return Some(&entry.1);
}
}
None
}

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

0 comments on commit 7c6abec

Please sign in to comment.