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.

Sign up for GitHub

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

Jump to bottom

[#82] Adds proxy as a flag and config option #121

Merged
merged 9 commits into from
Oct 7, 2022
Merged

[#82] Adds proxy as a flag and config option #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8693657
saving work, there is still an issue with unpacking the tarball after…
MitchellBerend Sep 25, 2022
db0bbc6
corrected Accept header in asset download method
MitchellBerend Sep 26, 2022
4bd024f
added proxy flag that overwrites config proxy if passed
MitchellBerend Sep 26, 2022
58f89c9
updated tests, these tests don't account for any proxy so this featur…
MitchellBerend Sep 26, 2022
32f4b0e
updated some comments:
MitchellBerend Sep 27, 2022
cdd8c8f
added changelog change
MitchellBerend Sep 27, 2022
32f80cd
incorporated feedback
MitchellBerend Oct 1, 2022
213e8b2
Merge branch 'main' into 82-proxy-option
MitchellBerend Oct 3, 2022
63540ae
Merge branch 'main' into 82-proxy-option
MitchellBerend Oct 6, 2022
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ available [on GitHub][2].
Adds the `tool sync <tool-name>` command to install only one tool
from the configuration file
(by [@zixuanzhang-x][zixuanzhang-x])
* [#82](https://github.com/chshersh/tool-sync/issues/82):
Adds proxy as a flag and config option
(by [@MitchellBerend][MitchellBerend])
* [#111](https://github.com/chshersh/tool-sync/issues/111):
Adds repo URLs to the output of `default-config` and `install` commands
(by [@crudiedo][crudiedo])
Expand Down
3 changes: 3 additions & 0 deletions src/config/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ pub struct Cli {
#[clap(short, long, value_name = "FILE")]
pub config: Option<PathBuf>,

#[clap(short, long, value_name = "uri")]
pub proxy: Option<String>,

#[clap(subcommand)]
pub command: Command,
}
Expand Down
7 changes: 7 additions & 0 deletions src/config/schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub struct Config {
/// Directory to store all locally downloaded tools
pub store_directory: String,

pub proxy: Option<String>,
/// Info about each individual tool
pub tools: BTreeMap<String, ConfigAsset>,
}
Expand All @@ -38,6 +39,9 @@ pub struct ConfigAsset {

/// Name of the specific asset to download
pub asset_name: AssetName,

/// Proxy which will get used for all communication
pub proxy: Option<ureq::Proxy>,
Comment on lines +43 to +44
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a particular reason we may need a different proxy for individual assets?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

tool-sync/src/sync/prefetch.rs

Lines 76 to 101 in 2b752bc

pub fn prefetch(tools: BTreeMap<String, ConfigAsset>) -> Vec<ToolAsset> {
let total_count = tools.len();
let prefetch_progress = PrefetchProgress::new(total_count);
prefetch_progress.update_message(0);
let tool_assets: Vec<ToolAsset> = tools
.iter()
.enumerate()
.filter_map(|(index, (tool_name, config_asset))| {
prefetch_tool(tool_name, config_asset, &prefetch_progress, index)
})
.collect();
prefetch_progress.finish();
let estimated_download_size: u64 = tool_assets.iter().map(|ta| ta.asset.size).sum();
let size = HumanBytes(estimated_download_size);
eprintln!(
"{emoji} Estimated total download size: {size}",
emoji = PACKAGE,
size = size
);
tool_assets
}

This method makes the first call to the github api. It only has access to the names and ConfigAsset struct.

The implementation of this was kind of messy so there could be a better way to implement this so the Client gets recycled for each api call instead of a new instance being created for every tool.

}

impl From<ToolInfo> for ConfigAsset {
Expand All @@ -53,6 +57,9 @@ impl From<ToolInfo> for ConfigAsset {
exe_name: Some(tool_info.exe_name),
tag,
asset_name: tool_info.asset_name,

/// Hardcoded tools don't supply their own proxy automatically
proxy: None,
}
}
}
Expand Down
74 changes: 52 additions & 22 deletions src/config/toml.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,12 @@ impl Display for TomlError {
}
}

pub fn with_parsed_file<F: FnOnce(Config)>(config_path: PathBuf, on_success: F) {
match parse_file(&config_path) {
pub fn with_parsed_file<F: FnOnce(Config)>(
config_path: PathBuf,
proxy: Option<String>,
on_success: F,
) {
match parse_file(&config_path, proxy) {
Ok(config) => {
on_success(config);
}
Expand All @@ -41,54 +45,70 @@ pub fn with_parsed_file<F: FnOnce(Config)>(config_path: PathBuf, on_success: F)
}
}

fn parse_file(config_path: &PathBuf) -> Result<Config, TomlError> {
fn parse_file(config_path: &PathBuf, proxy: Option<String>) -> Result<Config, TomlError> {
let contents = fs::read_to_string(config_path).map_err(|e| TomlError::IO(format!("{}", e)))?;

parse_string(&contents)
parse_string(&contents, proxy)
}

fn parse_string(contents: &str) -> Result<Config, TomlError> {
fn parse_string(contents: &str, proxy: Option<String>) -> Result<Config, TomlError> {
contents
.parse::<Value>()
.map_err(TomlError::Parse)
.and_then(|toml| match decode_config(toml) {
.and_then(|toml| match decode_config(toml, proxy) {
None => Err(TomlError::Decode),
Some(config) => Ok(config),
})
}

fn decode_config(toml: Value) -> Option<Config> {
fn decode_config(toml: Value, proxy: Option<String>) -> Option<Config> {
let str_store_directory = toml.get("store_directory")?.as_str()?;
let proxy: Option<String> = proxy.or_else(|| match toml.get("proxy") {
Some(p) => Some(p.as_str().unwrap_or("").into()),
None => None,
});

let store_directory = String::from(str_store_directory);

let mut tools = BTreeMap::new();

for (key, val) in toml.as_table()?.iter() {
if let Value::Table(table) = val {
tools.insert(key.clone(), decode_config_asset(table));
tools.insert(key.clone(), decode_config_asset(table, &proxy));
}
}

Some(Config {
store_directory,
tools,
proxy,
})
}

fn decode_config_asset(table: &Map<String, Value>) -> ConfigAsset {
fn decode_config_asset(table: &Map<String, Value>, proxy: &Option<String>) -> ConfigAsset {
let owner = str_by_key(table, "owner");
let repo = str_by_key(table, "repo");
let exe_name = str_by_key(table, "exe_name");
let asset_name = decode_asset_name(table);
let tag = str_by_key(table, "tag");

ConfigAsset {
let mut config_asset = ConfigAsset {
owner,
repo,
exe_name,
asset_name,
tag,
}
proxy: None,
};
if let Some(p) = proxy {
config_asset.proxy = Some(ureq::Proxy::new(p.clone()).unwrap_or_else(|_| {
panic!(
"Could not parse proxy address, please check the syntax: {}",
p
)
}));
};
config_asset
}

fn decode_asset_name(table: &Map<String, Value>) -> AssetName {
Expand Down Expand Up @@ -134,7 +154,7 @@ mod tests {
#[test]
fn test_toml_error_display_parse() {
let broken_toml_str: String = "broken toml".into();
match parse_string(&broken_toml_str) {
match parse_string(&broken_toml_str, None) {
Err(error) => {
assert_eq!(
String::from(
Expand All @@ -157,7 +177,7 @@ mod tests {
fn test_parse_file_correct_output() {
let result = std::panic::catch_unwind(|| {
let test_config_path = PathBuf::from("tests/sync-full.toml");
parse_file(&test_config_path).expect("This should not fail")
parse_file(&test_config_path, None).expect("This should not fail")
});

if let Ok(config) = result {
Expand All @@ -168,7 +188,7 @@ mod tests {
#[test]
fn test_parse_file_error() {
let test_config_path = PathBuf::from("src/main.rs");
match parse_file(&test_config_path) {
match parse_file(&test_config_path, None) {
Ok(_) => {
assert!(false, "Unexpected succces")
}
Expand All @@ -181,35 +201,36 @@ mod tests {
#[test]
fn empty_file() {
let toml = "";
let res = parse_string(toml);
let res = parse_string(toml, None);

assert_eq!(res, Err(TomlError::Decode));
}

#[test]
fn store_directory_is_dotted() {
let toml = "store.directory = \"pancake\"";
let res = parse_string(toml);
let res = parse_string(toml, None);

assert_eq!(res, Err(TomlError::Decode));
}

#[test]
fn store_directory_is_a_number() {
let toml = "store_directory = 42";
let res = parse_string(toml);
let res = parse_string(toml, None);

assert_eq!(res, Err(TomlError::Decode));
}

#[test]
fn only_store_directory() {
let toml = "store_directory = \"pancake\"";
let res = parse_string(toml);
let res = parse_string(toml, None);

let cfg = Config {
store_directory: String::from("pancake"),
tools: BTreeMap::new(),
proxy: None,
};

assert_eq!(res, Ok(cfg));
Expand All @@ -223,7 +244,7 @@ mod tests {
[ripgrep]
"#;

let res = parse_string(toml);
let res = parse_string(toml, None);

let cfg = Config {
store_directory: String::from("pancake"),
Expand All @@ -239,8 +260,10 @@ mod tests {
windows: None,
},
tag: None,
proxy: None,
},
)]),
proxy: None,
};

assert_eq!(res, Ok(cfg));
Expand All @@ -255,7 +278,7 @@ mod tests {
[bat]
"#;

let res = parse_string(toml);
let res = parse_string(toml, None);

let cfg = Config {
store_directory: String::from("pancake"),
Expand All @@ -272,6 +295,7 @@ mod tests {
windows: None,
},
tag: None,
proxy: None,
},
),
(
Expand All @@ -286,9 +310,11 @@ mod tests {
windows: None,
},
tag: None,
proxy: None,
},
),
]),
proxy: None,
};

assert_eq!(res, Ok(cfg));
Expand All @@ -304,7 +330,7 @@ mod tests {
asset_name.linux = "R2D2"
"#;

let res = parse_string(toml);
let res = parse_string(toml, None);

let cfg = Config {
store_directory: String::from("pancake"),
Expand All @@ -320,8 +346,10 @@ mod tests {
windows: None,
},
tag: None,
proxy: None,
},
)]),
proxy: None,
};

assert_eq!(res, Ok(cfg));
Expand All @@ -342,7 +370,7 @@ mod tests {
tag = "4.2.0"
"#;

let res = parse_string(toml);
let res = parse_string(toml, None);

let cfg = Config {
store_directory: String::from("pancake"),
Expand All @@ -358,8 +386,10 @@ mod tests {
windows: Some("IG-88".to_owned()),
},
tag: Some("4.2.0".to_owned()),
proxy: None,
},
)]),
proxy: None,
};

assert_eq!(res, Ok(cfg));
Expand Down
52 changes: 41 additions & 11 deletions src/infra/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ use std::io::Read;
use crate::model::release::{Asset, Release};

/// GitHub API client to handle all API requests
#[derive(Debug)]
pub struct Client {
pub owner: String,
pub repo: String,
pub version: String,

pub proxy: Option<ureq::Proxy>,
}

impl Client {
Expand All @@ -22,6 +25,7 @@ impl Client {
}

fn asset_url(&self, asset_id: u32) -> String {
println!("{:?}", &self.proxy);
format!(
"https://api.github.com/repos/{owner}/{repo}/releases/assets/{asset_id}",
owner = self.owner,
Expand All @@ -31,13 +35,26 @@ impl Client {
}

pub fn fetch_release_info(&self) -> Result<Release, Box<dyn Error>> {
println!("{:?}", &self.proxy);
let release_url = self.release_url();

let req = add_auth_header(
ureq::get(&release_url)
.set("Accept", "application/vnd.github+json")
.set("User-Agent", "chshersh/tool-sync-0.2.0"),
);
let req = match &self.proxy {
Some(proxy) => {
let agent = ureq::AgentBuilder::new().proxy(proxy.clone()).build();

add_auth_header(
agent
.get(&release_url)
.set("Accept", "application/vnd.github+json")
.set("User-Agent", "chshersh/tool-sync-0.2.0"),
)
}
None => add_auth_header(
ureq::get(&release_url)
.set("Accept", "application/vnd.github+json")
.set("User-Agent", "chshersh/tool-sync-0.2.0"),
),
};

let release: Release = req.call()?.into_json()?;

Expand All @@ -49,12 +66,23 @@ impl Client {
asset: &Asset,
) -> Result<Box<dyn Read + Send + Sync>, ureq::Error> {
let asset_url = self.asset_url(asset.id);

let req = add_auth_header(
ureq::get(&asset_url)
.set("Accept", "application/octet-stream")
.set("User-Agent", "chshersh/tool-sync-0.2.0"),
);
let req = match &self.proxy {
Some(proxy) => {
let agent = ureq::AgentBuilder::new().proxy(proxy.clone()).build();

add_auth_header(
agent
.get(&asset_url)
.set("Accept", "application/octet-stream")
.set("User-Agent", "chshersh/tool-sync-0.2.0"),
)
}
None => add_auth_header(
ureq::get(&asset_url)
.set("Accept", "application/octet-stream")
.set("User-Agent", "chshersh/tool-sync-0.2.0"),
),
};

Ok(req.call()?.into_reader())
}
Expand All @@ -79,6 +107,7 @@ mod tests {
owner: String::from("OWNER"),
repo: String::from("REPO"),
version: ToolInfoTag::Latest.to_str_version(),
proxy: None,
};

assert_eq!(
Expand All @@ -93,6 +122,7 @@ mod tests {
owner: String::from("OWNER"),
repo: String::from("REPO"),
version: ToolInfoTag::Specific(String::from("SPECIFIC_TAG")).to_str_version(),
proxy: None,
};

assert_eq!(
Expand Down
Loading