Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement review processing code #5

Merged
merged 41 commits into from
Oct 21, 2023
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
eda4929
Implement review processing code
tapishr Sep 17, 2023
9f9abec
Catch insert errors in sled db
tapishr Sep 17, 2023
68a9906
fix error reporting in hunkmap get from sled
tapishr Sep 17, 2023
db104e7
remove default in hunk to avoid potential misuse
tapishr Sep 17, 2023
d9d0117
break process_review function into separate functions
tapishr Sep 17, 2023
c707eee
fix error handling in bitbucket/user
tapishr Sep 17, 2023
b55b83b
fix error handling with timestamp parsing
tapishr Sep 17, 2023
8ec9930
fix error handling in commit_exists function
tapishr Sep 17, 2023
33c3956
fix error handling in git pull
tapishr Sep 17, 2023
670610a
fix error handling in set_git_url
tapishr Sep 17, 2023
607c850
fix line_res in process_diff function
tapishr Sep 17, 2023
d5c5c60
fix statitem deletion parse bug
tapishr Sep 17, 2023
8eb9385
fix error handling in auth and url format for commit
tapishr Sep 17, 2023
0a77eaf
fix empty option for auth timestamp
tapishr Sep 17, 2023
1834eda
fix logging in set_git_remote_url
tapishr Sep 17, 2023
a52479c
share rest api client with as many functions as possible
tapishr Sep 17, 2023
4537683
add debug log for hunkmap
tapishr Sep 17, 2023
36828f5
fixed config function naming and exception handling
tapishr Sep 17, 2023
fd5de1d
log unlogged error in bitbucket/webhook.rs
tapishr Sep 17, 2023
366cefc
fixed rpgue expect in get_commit_bb
tapishr Sep 17, 2023
61ff8e5
fix error logging in db/user.rs
tapishr Sep 17, 2023
9686369
fix db/user.rs expect for user_from_db
tapishr Sep 17, 2023
8fea72a
fix next_url naming issue
tapishr Sep 17, 2023
7e0d3f5
fix error handling in process_review and save_webhook_to_db
tapishr Sep 17, 2023
2addb3f
fix error handling for auth functions
tapishr Sep 17, 2023
ff5187b
fix error handling in send_setup_info
tapishr Sep 17, 2023
aa473ac
fix authinfo error handling in handle_install_bitbucket
tapishr Sep 17, 2023
45c3260
fix error handling in clone_git_repo
tapishr Sep 17, 2023
368a33f
clean up expect in save_workspace_to_db
tapishr Sep 17, 2023
a2a8d09
git rev-list status debugging fixed
tapishr Sep 17, 2023
7b21342
debug error codes in set_git_url
tapishr Sep 17, 2023
9a5bcf1
debug error codes in git blame
tapishr Sep 17, 2023
0893cdc
fix error handling for git commands
tapishr Sep 18, 2023
ab1696f
Merge branch 'tr/setup' into tr/review
tapishr Sep 18, 2023
c9bca44
Merge branch 'tr/setup' into tr/review
tapishr Sep 18, 2023
9d4f5b5
fix refresh_token logic and error handling
tapishr Sep 18, 2023
923d57b
Merge branch 'main' into tr/review
tapishr Oct 13, 2023
880c9fc
remove todo comment
tapishr Oct 21, 2023
b468237
add url to hunkmap publish logs
tapishr Oct 21, 2023
4a57fd3
fix git blame bug and get base head commit
tapishr Oct 21, 2023
ed7b280
replace url with db key in logs
tapishr Oct 21, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions vibi-dpu/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,6 @@ uuid = { version = "1.4.0", features = ["v4"]}
rand = "0.8.5"
chrono = "0.4.26"
tonic = "0.9.2"
once_cell = "1.18.0" # MIT

# todo - check all lib licences
132 changes: 118 additions & 14 deletions vibi-dpu/src/bitbucket/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ use std::str;
use std::process::Command;
use std::time::{SystemTime, UNIX_EPOCH};
use reqwest::Client;
use super::config::get_client;
use crate::db::auth::{save_auth_info_to_db, auth_info};
use crate::utils::auth::AuthInfo;

pub async fn get_access_token_from_bitbucket(code: &str) -> Option<AuthInfo> {
let client = Client::new();
let client = get_client();
let bitbucket_client_id = env::var("BITBUCKET_CLIENT_ID").unwrap();
let bitbucket_client_secret = env::var("BITBUCKET_CLIENT_SECRET").unwrap();
let mut params = std::collections::HashMap::new();
Expand All @@ -19,32 +20,135 @@ pub async fn get_access_token_from_bitbucket(code: &str) -> Option<AuthInfo> {
params.insert("grant_type", "authorization_code".to_owned());
params.insert("redirect_uri", redirect_uri);
println!("params = {:?}", &params);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using println! for logging in production code. Consider using a logging library like log or env_logger which provides different levels of logging (debug, info, warn, error).

let response = client
let post_res = client
.post("https://bitbucket.org/site/oauth2/access_token")
.form(&params)
.send()
.await;
if response.is_err() {
let response_err = response.expect_err("No error in access token response");
eprintln!("error in calling api : {:?}", &response_err);
if post_res.is_err() {
let e = post_res.expect_err("No error in post_res");
eprintln!("error in calling api : {:?}", e);
return None;
}
let res = response.expect("Uncaught error in response");
let res = post_res.expect("Uncaught error in post_res");
if !res.status().is_success() {
eprintln!(
"Failed to exchange code for access token. Status code: {}, Response content: {}",
println!(
"Failed to exchange code for access token. Status code: {}, Response content: {:?}",
res.status(),
res.text().await.expect("No text in response")
res.text().await
);
return None;
}
let json_res = res.json::<AuthInfo>().await;
if json_res.is_err() {
let json_error = json_res.expect_err("Error not found in json");
eprintln!("error deserializing : {:?}", json_error);
let parse_res = res.json::<AuthInfo>().await ;
if parse_res.is_err() {
let e = parse_res.expect_err("No error in parse_res for AuthInfo");
eprintln!("error deserializing AuthInfo: {:?}", e);
return None;
}
let mut response_json = json_res.expect("Uncaught error in deserializing response json");
let mut response_json = parse_res.expect("Uncaught error in parse_res for AuthInfo");
save_auth_info_to_db(&mut response_json);
return Some(response_json);
}

pub async fn refresh_git_auth(clone_url: &str, directory: &str) -> Option<String>{
let authinfo_opt = auth_info();
if authinfo_opt.is_none() {
return None;
}
let authinfo = authinfo_opt.expect("empty authinfo_opt in refresh_git_auth");
let mut access_token = authinfo.access_token().to_string();
let authinfo_opt = update_access_token(&authinfo).await;
if authinfo_opt.is_none() {
eprintln!("Empty authinfo_opt from update_access_token");
return None;
}
let mut new_auth_info = authinfo_opt
.expect("empty auhtinfo_opt from update_access_token");
println!("New auth info = {:?}", &new_auth_info);
access_token = new_auth_info.access_token().to_string();
set_git_remote_url(clone_url, directory, &access_token);
save_auth_info_to_db(&mut new_auth_info);
return Some(access_token);
}

pub async fn update_access_token(auth_info: &AuthInfo) -> Option<AuthInfo> {
let now = SystemTime::now();
let now_secs = now.duration_since(UNIX_EPOCH).expect("Time went backwards").as_secs();
let timestamp_opt = auth_info.timestamp();
if timestamp_opt.is_none() {
eprintln!("No timestamp in authinfo");
return None;
}
let timestamp = timestamp_opt.expect("Empty timestamp");
let expires_at = timestamp + auth_info.expires_in();
println!(" expires_at = {expires_at}, now_secs = {now_secs}");
tapishr marked this conversation as resolved.
Show resolved Hide resolved
if expires_at <= now_secs {
// auth info has expired
let new_auth_info_opt = bitbucket_refresh_token(auth_info.refresh_token()).await;
return new_auth_info_opt;
}
return None;
}

async fn bitbucket_refresh_token(refresh_token: &str) -> Option<AuthInfo> {
let token_url = "https://bitbucket.org/site/oauth2/access_token";
let client_id = std::env::var("BITBUCKET_CLIENT_ID")
.expect("BITBUCKET_CLIENT_ID must be set");
let client_secret = std::env::var("BITBUCKET_CLIENT_SECRET")
.expect("BITBUCKET_CLIENT_SECRET must be set");
let mut headers = reqwest::header::HeaderMap::new();
headers.insert(reqwest::header::CONTENT_TYPE,
"application/x-www-form-urlencoded".parse().expect("Invalid content-type"));
let payload = &[
("grant_type", "refresh_token"),
("refresh_token", refresh_token)
];
let client = get_client();
let post_res = client.post(token_url)
.headers(headers)
.basic_auth(client_id, Some(client_secret))
.form(payload)
.send()
.await;
if post_res.is_err() {
let e = post_res.expect_err("No error in post_err for refres token");
eprintln!("Unable to get refresh token: {}", e);
return None;
}
let response = post_res.expect("Uncaught error in post_res");
if !response.status().is_success() {
eprintln!("Failed to get refresh token, status: {} body: {:?}",
response.status(), response.text().await);
return None;
}
let parse_res = response.json().await;
if parse_res.is_err() {
let e = parse_res.expect_err("No error in parse_res refresh_token");
eprintln!("Unable to deserialize refresh token response: {}", e);
return None;
}
Comment on lines +128 to +132
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error handling can be improved by using the ? operator to propagate errors up the call stack. This will make the code more concise and easier to read.

let refresh_token_resbody = parse_res.expect("Uncaught error in parse_res");
return Some(refresh_token_resbody);
}

fn set_git_remote_url(git_url: &str, directory: &str, access_token: &str) {
let clone_url = git_url.to_string()
.replace("git@", format!("https://x-token-auth:{{{access_token}}}@").as_str())
.replace("bitbucket.org:", "bitbucket.org/");
Comment on lines +139 to +140
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The access token is being inserted into the URL in plain text. This could potentially expose sensitive information if the URL is logged or leaked. Consider using a more secure method to authenticate with the git server.

Comment on lines +138 to +140
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The access token is not being correctly inserted into the URL string. The current code is trying to replace "git@" with "https://x-token-auth:{{{access_token}}}@" but the triple braces around access_token will not interpolate the variable. You should use single braces instead.

-    let clone_url = git_url.to_string()
-        .replace("git@", format!("https://x-token-auth:{{{access_token}}}@").as_str())
-        .replace("bitbucket.org:", "bitbucket.org/");
+    let clone_url = git_url.replace("git@", &format!("https://x-token-auth:{}@", access_token))
+        .replace("bitbucket.org:", "bitbucket.org/");

let output = Command::new("git")
.arg("remote").arg("set-url").arg("origin")
.arg(clone_url)
.current_dir(directory)
.output()
.expect("failed to execute git pull");
// Only for debug purposes
match str::from_utf8(&output.stderr) {
Ok(v) => println!("set_git_url stderr = {:?}", v),
Err(e) => eprintln!("set_git_url stderr error: {}", e),
};
match str::from_utf8(&output.stdout) {
Ok(v) => println!("set_git_urll stdout = {:?}", v),
Err(e) => eprintln!("set_git_url stdout error: {}", e),
};
println!("git pull output = {:?}, {:?}", &output.stdout, &output.stderr);
tapishr marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +148 to +156
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid using println! for logging in production code. Consider using a logging library like log or env_logger which provides different levels of logging (debug, info, warn, error).

}
144 changes: 79 additions & 65 deletions vibi-dpu/src/bitbucket/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,24 @@ use std::{env, collections::HashMap};

use reqwest::{Response, header::{HeaderMap, HeaderValue}};
use serde_json::Value;
use std::sync::Arc;
use once_cell::sync::Lazy;
use reqwest::Client;

static CLIENT: Lazy<Arc<Client>> = Lazy::new(|| {
Arc::new(Client::new())
});

pub fn get_client() -> Arc<Client> {
Arc::clone(&CLIENT)
}
Comment on lines +13 to +15
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The get_client function is returning a clone of the Arc wrapping the Client. This is unnecessary as Arc is designed to be shared and can be cloned when needed by the caller. Consider returning a reference instead.


pub fn bitbucket_base_url() -> String {
env::var("BITBUCKET_BASE_URL").expect("BITBUCKET_BASE_URL must be set")
}

pub async fn get_api(url: &str, access_token: &str, params: Option<HashMap<&str, &str>> ) -> Vec<Value> {
let response_opt = call_get_api(url, access_token, &params).await;
pub async fn get_api_values(url: &str, access_token: &str, params: Option<HashMap<&str, &str>> ) -> Vec<Value> {
let response_opt = get_api(url, access_token, &params).await;
println!("response of get_api = {:?}", &response_opt);
let (mut response_values, next_url) = deserialize_response(response_opt).await;
if next_url.is_some() {
Expand All @@ -18,89 +29,92 @@ pub async fn get_api(url: &str, access_token: &str, params: Option<HashMap<&str,
return response_values;
}

pub async fn call_get_api(url: &str, access_token: &str, params: &Option<HashMap<&str, &str>> ) -> Option<Response>{
pub async fn get_api(url: &str, access_token: &str, params: &Option<HashMap<&str, &str>> ) -> Option<Response>{
println!("GET api url = {}", url);
let client = reqwest::Client::new();
let client = get_client();
let mut headers = reqwest::header::HeaderMap::new();
headers.insert( reqwest::header::AUTHORIZATION,
format!("Bearer {}", access_token).parse().expect("Invalid auth header"), );
headers.insert("Accept",
"application/json".parse().expect("Invalid Accept header"));
match params {
Some(params) => {
match client.get(url).headers(headers).query(params).send().await {
Ok(response) => {
if response.status().is_success() {
return Some(response);
}
else { eprintln!("Failed to call API {}, status: {}", url, response.status()); }
},
Err(e) => { eprintln!("Error sending GET request to {}, error: {}", url, e); },
};
},
None => {
match client.get(url).headers(headers).send().await {
Ok(response) => {
if response.status().is_success() {
return Some(response);
}
else { eprintln!("Failed to call API {}, status: {}", url, response.status()); }
},
Err(e) => { eprintln!("Error sending GET request to {}, error: {}", url, e); },
};
}
};
return None;
let res_opt = get_api_response(url, headers, access_token, params).await;
return res_opt;
}

async fn get_api_response(url: &str, headers: reqwest::header::HeaderMap, access_token: &str, params: &Option<HashMap<&str, &str>>) -> Option<Response>{
let client = get_client();
let get_res = client.get(url).headers(headers).send().await;
if get_res.is_err() {
let e = get_res.expect_err("No error in get_res");
eprintln!("Error sending GET request without params to {}, error: {}", url, e);
return None;
}
let response = get_res.expect("Uncaught error in get_res");
if !response.status().is_success() {
eprintln!("Failed to call API {}, status: {}", url, response.status());
return None;
}
return Some(response);
}

async fn deserialize_response(response_opt: Option<Response>) -> (Vec<Value>, Option<String>) {
let values_vec = Vec::new();
match response_opt {
Some(response) => {
match response.json::<serde_json::Value>().await {
Ok(response_json) => {
let mut values_vec = Vec::new();
if let Some(values) = response_json["values"].as_array() {
for value in values {
values_vec.push(value.to_owned());
}
return (values_vec, Some(response_json["next"].to_string()));
}
}
Err(e) => { eprintln!("Unable to deserialize response: {}", e); }
};
},
None => { eprintln!("Response is None");}
};
return (values_vec, None);
let mut values_vec = Vec::new();
if response_opt.is_none() {
eprintln!("Response is None, can't deserialize");
return (values_vec, None);
}
let response = response_opt.expect("Uncaught empty response_opt");
let parse_res = response.json::<serde_json::Value>().await;
if parse_res.is_err() {
let e = parse_res.expect_err("No error in parse_res");
eprintln!("Unable to deserialize response: {}", e);
return (values_vec, None);
}
let response_json = parse_res
.expect("Uncaught error in parse_res in deserialize_response");
let res_values_opt = response_json["values"].as_array();
if res_values_opt.is_none() {
eprintln!("response_json[values] is empty");
return (values_vec, None);
}
let values = res_values_opt.expect("res_values_opt is empty");
for value in values {
values_vec.push(value.to_owned());
}
return (values_vec, Some(response_json["next"].to_string()));
}

async fn get_all_pages(next_url: Option<String>, access_token: &str, params: &Option<HashMap<&str, &str>>) -> Vec<Value>{
let mut values_vec = Vec::new();
let mut next_url = next_url;
while next_url.is_some() {
let url = next_url.as_ref().expect("next_url is none").trim_matches('"');
if url != "null" {
let response_opt = call_get_api(url, access_token, params).await;
let (mut response_values, url_opt) = deserialize_response(response_opt).await;
next_url = url_opt.clone();
values_vec.append(&mut response_values);
} else {
let mut next_url_mut = next_url;
while next_url_mut.is_some() {
let url_opt = next_url_mut.as_ref();
if url_opt.is_none() {
eprintln!("next_url is none");
break;
}
let url = url_opt.expect("Empty next url_opt").trim_matches('"');
if url == "null" {
break;
}
let response_opt = get_api(url, access_token, params).await;
let (mut response_values, url_opt) = deserialize_response(response_opt).await;
next_url_mut = url_opt.clone();
values_vec.append(&mut response_values);
}
return values_vec;
}

pub fn prepare_auth_headers(access_token: &str) -> HeaderMap{
pub fn prepare_auth_headers(access_token: &str) -> Option<HeaderMap>{
let mut headers_map = HeaderMap::new();
let auth_header = format!("Bearer {}", access_token);
let headervalres = HeaderValue::from_str(&auth_header);
match headervalres {
Ok(headerval) => {
headers_map.insert("Authorization", headerval);
},
Err(e) => panic!("Could not parse header value: {}", e),
};
return headers_map;
if headervalres.is_err() {
let e = headervalres.expect_err("No error found in headervalres");
eprintln!("Could not parse header value: {}", e);
return None;
}
let headerval = headervalres.expect("Empty headervalres");
headers_map.insert("Authorization", headerval);
return Some(headers_map);
}
2 changes: 1 addition & 1 deletion vibi-dpu/src/bitbucket/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
pub mod auth;
pub mod workspace;
pub mod repo;
mod config;
pub mod config;
pub mod webhook;
pub mod user;
4 changes: 2 additions & 2 deletions vibi-dpu/src/bitbucket/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ use serde_json::Value;

use crate::db::repo::save_repo_to_db;
use crate::utils::repo::Repository;
use super::config::{bitbucket_base_url, get_api};
use super::config::{bitbucket_base_url, get_api_values};

pub async fn get_workspace_repos(workspace: &str, access_token: &str) -> Option<Vec<Repository>> {
let repos_url = format!("{}/repositories/{}", bitbucket_base_url(), workspace);
let response_json = get_api(&repos_url, access_token, None).await;
let response_json = get_api_values(&repos_url, access_token, None).await;
let mut repos_data = Vec::new();
for repo_json in response_json {
let val = Repository::new(
Expand Down
Loading