Skip to content

Commit

Permalink
Placate Clippy (#126)
Browse files Browse the repository at this point in the history
  • Loading branch information
itowlson committed Oct 27, 2021
1 parent c48bdd6 commit abecd21
Show file tree
Hide file tree
Showing 15 changed files with 147 additions and 57 deletions.
16 changes: 13 additions & 3 deletions src/bindle_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ impl InvoiceUnderstander {
})
})
}

pub fn parse_wagi_handlers(&self) -> Vec<WagiHandlerInfo> {
self
.top_modules().iter()
.filter_map(|parcel| self.classify_parcel(parcel))
.map(|parcel| match parcel { // If there are other cases of InterestingParcel this may need to become a filter_map, but right now that makes Clippy mad
InterestingParcel::WagiHandler(h) => h,
})
.collect()
}
}

pub enum InterestingParcel {
Expand Down Expand Up @@ -101,7 +111,7 @@ pub fn is_file(parcel: &Parcel) -> bool {
pub fn parcels_required_for(parcel: &Parcel, full_dep_map: &HashMap<String, Vec<Parcel>>) -> Vec<Parcel> {
let mut required = HashSet::new();
for group in parcel.directly_requires() {
required.extend(full_dep_map.get(&group).unwrap_or(&NO_PARCELS).iter().map(|p| p.clone()));
required.extend(full_dep_map.get(&group).unwrap_or(&NO_PARCELS).iter().cloned());
}
Vec::from_iter(required)
}
Expand Down Expand Up @@ -132,7 +142,7 @@ pub fn build_full_memberships(invoice: &Invoice) -> HashMap<String, Vec<Parcel>>
for group in direct_memberships.keys() {
let mut all_members = HashSet::new();
for dep_group in gg_deps.get(group).unwrap() {
all_members.extend(direct_memberships.get(dep_group).unwrap_or(&NO_PARCELS).iter().map(|p| p.clone()));
all_members.extend(direct_memberships.get(dep_group).unwrap_or(&NO_PARCELS).iter().cloned());
}
full_memberships.insert(group.to_owned(), Vec::from_iter(all_members));
}
Expand All @@ -150,7 +160,7 @@ fn group_to_group_direct_dependencies(direct_memberships: &HashMap<String, Vec<P
ggd
}

fn direct_deps_not_already_in_list(list: &Vec<String>, direct_dep_map: &HashMap<String, Vec<String>>) -> Vec<String> {
fn direct_deps_not_already_in_list(list: &[String], direct_dep_map: &HashMap<String, Vec<String>>) -> Vec<String> {
let mut new_dds = vec![];
for group in list {
if let Some(child_groups) = direct_dep_map.get(group) {
Expand Down
18 changes: 10 additions & 8 deletions src/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::emplacer::Bits;
use crate::handlers::{RouteHandler, WasmRouteHandler};
use crate::http_util::{not_found};
use crate::module_loader::Loaded;
use crate::request::{RequestContext, RequestGlobalContext, RequestRouteContext};
use crate::request::{RequestContext, RequestGlobalContext};

use crate::bindle_util::{WagiHandlerInfo};
use crate::wagi_config::{LoadedHandlerConfiguration, ModuleMapConfigurationEntry};
Expand Down Expand Up @@ -159,8 +159,7 @@ impl RoutingTableEntry {
match &self.handler_info {
RouteHandler::HealthCheck => Response::new(Body::from("OK")),
RouteHandler::Wasm(w) => {
let route_context = RequestRouteContext { entrypoint: w.entrypoint.clone() };
let response = w.handle_request(&self.route_pattern, req, body, request_context, &route_context, global_context, self.unique_key());
let response = w.handle_request(&self.route_pattern, req, body, request_context, global_context, self.unique_key());
match response {
Ok(res) => res,
Err(e) => {
Expand Down Expand Up @@ -259,15 +258,15 @@ impl RoutingTable {
})
}

fn build_from_modules_toml(module_map_entries: &Vec<Loaded<ModuleMapConfigurationEntry>>) -> anyhow::Result<Vec<RoutingTableEntry>> {
fn build_from_modules_toml(module_map_entries: &[Loaded<ModuleMapConfigurationEntry>]) -> anyhow::Result<Vec<RoutingTableEntry>> {
// TODO: look for `_routes` function
module_map_entries
.iter()
.map(|e| RoutingTableEntry::build_from_modules_toml(e))
.collect()
}

fn build_from_bindle_entries(bindle_entries: &Vec<(WagiHandlerInfo, Bits)>) -> anyhow::Result<Vec<RoutingTableEntry>> {
fn build_from_bindle_entries(bindle_entries: &[(WagiHandlerInfo, Bits)]) -> anyhow::Result<Vec<RoutingTableEntry>> {
bindle_entries
.iter()
.filter_map(|e| RoutingTableEntry::build_from_bindle_entry(e))
Expand Down Expand Up @@ -302,14 +301,14 @@ fn augment_one_wasm_with_dynamic_routes(routing_table_entry: &RoutingTableEntry,
let (store, instance) = prepare_wasm_instance(global_context, ctx, &wasm_route_handler.wasm_module_source, |_| Ok(()))?;

match run_prepared_wasm_instance_if_present(instance, store, "_routes") {
RunWasmResult::WasmError(e) => return Err(anyhow::Error::from(e)),
RunWasmResult::WasmError(e) => Err(e),
RunWasmResult::EntrypointNotFound => Ok(vec![routing_table_entry.clone()]),
RunWasmResult::Ok(_) => {
let out = redirects.stdout_mutex.read().unwrap();
let dynamic_routes_text = std::str::from_utf8(&*out)?;
let dynamic_routes = interpret_routes(dynamic_routes_text)?;

let mut dynamic_route_entries = append_all_dynamic_routes(&routing_table_entry, wasm_route_handler, dynamic_routes);
let mut dynamic_route_entries = append_all_dynamic_routes(routing_table_entry, wasm_route_handler, dynamic_routes);
dynamic_route_entries.reverse();
dynamic_route_entries.push(routing_table_entry.clone());
Ok(dynamic_route_entries)
Expand All @@ -318,7 +317,10 @@ fn augment_one_wasm_with_dynamic_routes(routing_table_entry: &RoutingTableEntry,
}

fn append_all_dynamic_routes(routing_table_entry: &RoutingTableEntry, wasm_route_handler: &WasmRouteHandler, dynamic_routes: DynamicRoutes) -> Vec<RoutingTableEntry> {
dynamic_routes.subpath_entrypoints.iter().map(|dr| append_one_dynamic_route(&routing_table_entry, wasm_route_handler, &dr.0, &dr.1)).collect()
dynamic_routes
.subpath_entrypoints.iter()
.map(|dr| append_one_dynamic_route(routing_table_entry, wasm_route_handler, &dr.0, &dr.1))
.collect()
}

fn append_one_dynamic_route(routing_table_entry: &RoutingTableEntry, wasm_route_handler: &WasmRouteHandler, dynamic_route_pattern: &RoutePattern, entrypoint: &str) -> RoutingTableEntry {
Expand Down
26 changes: 10 additions & 16 deletions src/emplacer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use anyhow::Context;
use sha2::{Digest, Sha256};
use url::Url;

use crate::{bindle_util::{InterestingParcel, InvoiceUnderstander, WagiHandlerInfo}, wagi_config::{HandlerConfigurationSource, WagiConfiguration}};
use crate::{bindle_util::{InvoiceUnderstander, WagiHandlerInfo}, wagi_config::{HandlerConfigurationSource, WagiConfiguration}};

pub struct Emplacer {
cache_path: PathBuf,
Expand All @@ -24,8 +24,8 @@ impl Emplacer {
).await
}

async fn new_from_settings(asset_cache_dir: &PathBuf, handlers: &HandlerConfigurationSource) -> anyhow::Result<Self> {
let cache_path = asset_cache_dir.clone();
async fn new_from_settings(asset_cache_dir: &Path, handlers: &HandlerConfigurationSource) -> anyhow::Result<Self> {
let cache_path = asset_cache_dir.to_owned();
tokio::fs::create_dir_all(&cache_path).await
.with_context(|| format!("Can't create asset cache directory {}", cache_path.display()))?;
Ok(Self {
Expand All @@ -40,7 +40,7 @@ impl Emplacer {
HandlerConfigurationSource::StandaloneBindle(bindle_base_dir, id) =>
self.emplace_standalone_bindle(bindle_base_dir, id).await,
HandlerConfigurationSource::RemoteBindle(bindle_base_url, id) =>
self.emplace_remote_bindle(&bindle_base_url, id).await,
self.emplace_remote_bindle(bindle_base_url, id).await,
}.with_context(|| "Error caching assets from bindle")
}

Expand All @@ -63,7 +63,7 @@ impl Emplacer {
Ok(invoice)
}

async fn emplace_standalone_bindle(&self, bindle_base_dir: &PathBuf, id: &bindle::Id) -> anyhow::Result<()> {
async fn emplace_standalone_bindle(&self, bindle_base_dir: &Path, id: &bindle::Id) -> anyhow::Result<()> {
let reader = bindle::standalone::StandaloneRead::new(bindle_base_dir, id).await
.with_context(|| format!("Error constructing bindle reader for {} in {}", id, bindle_base_dir.display()))?;

Expand Down Expand Up @@ -91,15 +91,9 @@ impl Emplacer {

let invoice = InvoiceUnderstander::new(&invoice);

let module_parcels: Vec<_> = invoice
.top_modules().iter()
.filter_map(|parcel| invoice.classify_parcel(parcel))
.filter_map(|parcel| match parcel {
InterestingParcel::WagiHandler(h) => Some(h),
})
.collect();
let module_parcels = invoice.parse_wagi_handlers();

let module_placements = module_parcels.iter().map(|h| self.emplace_module_and_assets(reader, &id, &h));
let module_placements = module_parcels.iter().map(|h| self.emplace_module_and_assets(reader, id, h));
let all_module_placements = futures::future::join_all(module_placements).await;

match all_module_placements.into_iter().find_map(|e| e.err()) {
Expand All @@ -120,7 +114,7 @@ impl Emplacer {
return Ok(());
}

let parcel_data = reader.get_parcel(invoice_id, &parcel).await?;
let parcel_data = reader.get_parcel(invoice_id, parcel).await?;
safely_write(&parcel_path, parcel_data).await
.with_context(|| format!("Error caching parcel {} at {}", parcel.label.name, parcel_path.display()))
}
Expand All @@ -131,13 +125,13 @@ impl Emplacer {
return Ok(());
}

let parcel_data = reader.get_parcel(invoice_id, &parcel).await?;
let parcel_data = reader.get_parcel(invoice_id, parcel).await?;
safely_write(&parcel_path, parcel_data).await
.with_context(|| format!("Error caching parcel {} at {}", parcel.label.name, parcel_path.display()))?;
Ok(())
}

async fn emplace_as_assets(&self, reader: &impl BindleReader, invoice_id: &bindle::Id, parcels: &Vec<bindle::Parcel>) -> anyhow::Result<()> {
async fn emplace_as_assets(&self, reader: &impl BindleReader, invoice_id: &bindle::Id, parcels: &[bindle::Parcel]) -> anyhow::Result<()> {
let placement_futures = parcels.iter().map(|parcel| self.emplace_as_asset(reader, invoice_id, parcel));
let all_placements = futures::future::join_all(placement_futures).await;
let first_error = all_placements.into_iter().find(|p| p.is_err());
Expand Down
5 changes: 2 additions & 3 deletions src/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use wasmtime_wasi::*;

use crate::dispatcher::RoutePattern;
use crate::http_util::{internal_error, parse_cgi_headers};
use crate::request::{RequestContext, RequestGlobalContext, RequestRouteContext};
use crate::request::{RequestContext, RequestGlobalContext};

use crate::wasm_module::WasmModuleSource;
use crate::wasm_runner::{prepare_stdio_streams, prepare_wasm_instance, run_prepared_wasm_instance};
Expand Down Expand Up @@ -42,7 +42,6 @@ impl WasmRouteHandler {
req: &Parts,
body: Vec<u8>,
request_context: &RequestContext,
route_context: &RequestRouteContext,
global_context: &RequestGlobalContext,
logging_key: String,
) -> Result<Response<Body>, anyhow::Error> {
Expand All @@ -66,7 +65,7 @@ impl WasmRouteHandler {
// Drop manually to get instantiation time
drop(startup_span);

run_prepared_wasm_instance(instance, store, &route_context.entrypoint, &self.wasm_module_name)?;
run_prepared_wasm_instance(instance, store, &self.entrypoint, &self.wasm_module_name)?;

compose_response(redirects.stdout_mutex)
}
Expand Down
45 changes: 45 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ mod test {
const TEST1_MODULE_MAP_FILE: &str = "test1.toml";
#[cfg(target_os = "windows")]
const TEST2_MODULE_MAP_FILE: &str = "test2.toml";
const TEST3_MODULE_MAP_FILE: &str = "test3.toml";
const TEST_HEALTHZ_MODULE_MAP_FILE: &str = "test_healthz_override.toml";

async fn build_routing_table_for_standalone_bindle(bindle_id: &str) -> RoutingTable {
Expand Down Expand Up @@ -144,6 +145,26 @@ mod test {
.expect("Error producing HTTP response")
}

async fn get_plain_text_response_from_module_map(map_file: &str, custom_subs: Option<HashMap<String, String>>, route: &str) -> String {
let empty_body = hyper::body::Body::empty();
let uri = format!("http://127.0.0.1:3000{}", route);
let request = hyper::Request::get(&uri).body(empty_body);

let response = send_request_to_module_map(map_file, custom_subs, request).await;

assert_eq!(hyper::StatusCode::OK, response.status());

// Content-Type could include a charset
let content_type = response.headers().get("Content-Type").expect("Expected Content-Type header").to_str().unwrap();
assert!(content_type.starts_with("text/plain"));

let response_body = hyper::body::to_bytes(response.into_body()).await
.expect("Could bot get bytes from response body");
let response_text = std::str::from_utf8(&response_body)
.expect("Could not read body as string");
response_text.to_owned()
}

async fn get_plain_text_response_from_standalone_bindle(bindle_id: &str, route: &str) -> String {
let empty_body = hyper::body::Body::empty();
let uri = format!("http://127.0.0.1:3000{}", route);
Expand Down Expand Up @@ -279,6 +300,30 @@ mod test {
res
}

#[tokio::test]
pub async fn can_serve_multiple_static_entrypoints() {
{
let route = "/defaultep";

let response = get_plain_text_response_from_module_map(TEST3_MODULE_MAP_FILE, None, route).await;
assert_eq!("Default entrypoint\n", response);
}

{
let route = "/ep1";

let response = get_plain_text_response_from_module_map(TEST3_MODULE_MAP_FILE, None, route).await;
assert_eq!("Entrypoint 1\n", response);
}

{
let route = "/ep2";

let response = get_plain_text_response_from_module_map(TEST3_MODULE_MAP_FILE, None, route).await;
assert_eq!("Entrypoint 2\n", response);
}
}

fn parse_ev_line(line: &str) -> Option<(String, String)> {
line.find('=').and_then(|index| {
let left = &line[..index];
Expand Down
9 changes: 3 additions & 6 deletions src/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use anyhow::Context;
use oci_distribution::client::{Client, ClientConfig};
use oci_distribution::secrets::RegistryAuth;
use oci_distribution::Reference;
use docker_credential;
use docker_credential::DockerCredential;
use sha2::{Digest, Sha256};
use url::Url;
Expand Down Expand Up @@ -69,10 +68,8 @@ async fn load_from_oci(

let mut auth = RegistryAuth::Anonymous;

if let Ok(credential) = docker_credential::get_credential(uri.as_str()) {
if let DockerCredential::UsernamePassword(user_name, password) = credential {
auth = RegistryAuth::Basic(user_name, password);
};
if let Ok(DockerCredential::UsernamePassword(user_name, password)) = docker_credential::get_credential(uri.as_str()) {
auth = RegistryAuth::Basic(user_name, password);
};

let img = url_to_oci(uri).map_err(|e| {
Expand Down Expand Up @@ -234,7 +231,7 @@ fn hash_name(url: &Url) -> String {
//
// *Except I changed it to take an &Vec instead of a Vec but I am sure our mighty
// brains can reconcile that if and when the moment comes.
async fn safely_write(path: impl AsRef<Path>, content: &Vec<u8>) -> std::io::Result<()> {
async fn safely_write(path: impl AsRef<Path>, content: &[u8]) -> std::io::Result<()> {
let path = path.as_ref();
let dir = path.parent().ok_or_else(||
std::io::Error::new(std::io::ErrorKind::Other, format!("cache location {} has no parent directory", path.display()))
Expand Down
5 changes: 0 additions & 5 deletions src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,6 @@ pub struct RequestContext {
pub client_addr: SocketAddr,
}

#[derive(Clone, Debug)]
pub struct RequestRouteContext {
pub entrypoint: String,
}

#[derive(Clone, Debug)]
pub struct RequestGlobalContext {
pub cache_config_path: PathBuf,
Expand Down
20 changes: 5 additions & 15 deletions src/wagi_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use anyhow::Context;
use bindle::{Invoice, standalone::StandaloneRead};
use serde::Deserialize;

use crate::{bindle_util::{InterestingParcel, InvoiceUnderstander, WagiHandlerInfo}, emplacer::{Emplacer}, module_loader::{Loaded}, request::RequestGlobalContext};
use crate::{bindle_util::{InvoiceUnderstander, WagiHandlerInfo}, emplacer::{Emplacer}, module_loader::{Loaded}, request::RequestGlobalContext};

#[derive(Clone, Debug)]
pub struct WagiConfiguration {
Expand Down Expand Up @@ -56,6 +56,7 @@ pub struct ModuleMapConfigurationEntry {
pub http_max_concurrency: Option<u32>,
}

#[allow(clippy::large_enum_variant)]
pub enum HandlerConfiguration {
ModuleMapFile(ModuleMapConfiguration),
Bindle(Invoice),
Expand Down Expand Up @@ -103,7 +104,7 @@ impl WagiConfiguration {
}
}

async fn read_module_map_configuration(path: &PathBuf) -> anyhow::Result<ModuleMapConfiguration> {
async fn read_module_map_configuration(path: &Path) -> anyhow::Result<ModuleMapConfiguration> {
tracing::info!(?path, "Loading modules config file");
if !tokio::fs::metadata(&path)
.await
Expand Down Expand Up @@ -144,24 +145,13 @@ async fn handlers_for_module_map(module_map: &ModuleMapConfiguration, configurat

let loadeds: anyhow::Result<Vec<_>> = futures::future::join_all(loaders).await.into_iter().collect();

loadeds.map(|entries| LoadedHandlerConfiguration::ModuleMapFile(entries))
loadeds.map(LoadedHandlerConfiguration::ModuleMapFile)
}

async fn handlers_for_bindle(invoice: &bindle::Invoice, emplacer: &Emplacer) -> anyhow::Result<LoadedHandlerConfiguration> {
let invoice = InvoiceUnderstander::new(invoice);

let top = invoice.top_modules();
tracing::debug!(
default_modules = top.len(),
"Loaded modules from the default group (parcels that do not have conditions.memberOf set)"
);

let interesting_parcels = top.iter().filter_map(|p| invoice.classify_parcel(p));
let wagi_handlers: Vec<_> = interesting_parcels.filter_map(|p|
match p {
InterestingParcel::WagiHandler(h) => Some(h),
}
).collect();
let wagi_handlers = invoice.parse_wagi_handlers();

let loaders = wagi_handlers.iter().map(|h| emplacer.get_bits_for(h));
let loadeds: anyhow::Result<Vec<_>> = futures::future::join_all(loaders).await.into_iter().collect();
Expand Down
2 changes: 1 addition & 1 deletion src/wagi_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ impl WagiServer {
Ok(Self {
routing_table,
tls: configuration.http_configuration.tls.clone(),
address: configuration.http_configuration.listen_on.clone(),
address: configuration.http_configuration.listen_on,
})
}

Expand Down
Binary file added testdata/module-maps/multiple-entrypoints.wasm
Binary file not shown.

0 comments on commit abecd21

Please sign in to comment.