From c96b92d87dea6abf3f74b0ca2e5270dbb50d6240 Mon Sep 17 00:00:00 2001 From: Robert Yin Date: Wed, 25 May 2022 20:05:03 -0400 Subject: [PATCH 1/2] feat: changing query params to slice of key-value pairs BREAKING: removes support for using `HashMap`s as query parameters --- README.md | 18 +++++++++--------- src/client.rs | 44 ++++++++++++++++++++++++-------------------- tests/map.rs | 13 +++++++------ tests/simple.rs | 22 ++++++++-------------- tests/with_route.rs | 36 ++++++++++++------------------------ 5 files changed, 60 insertions(+), 73 deletions(-) diff --git a/README.md b/README.md index 57b53cb..2986d84 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ serde_json = "*" Simple example usage: ```rust -use std::{collections::HashMap, env}; +use std::env; use mbta_rs::Client; let client = match env::var("MBTA_TOKEN") { @@ -63,11 +63,11 @@ let client = match env::var("MBTA_TOKEN") { Err(_) => Client::without_key() }; -let query_params = HashMap::from([ - ("page[limit]".to_string(), "3".to_string()) -]); +let query_params = [ + ("page[limit]", "3") +]; -let alerts_response = client.alerts(query_params); +let alerts_response = client.alerts(&query_params); if let Ok(response) = alerts_response { for alert in response.data { println!("MBTA alert: {}", alert.attributes.header); @@ -99,7 +99,7 @@ let client = match env::var("MBTA_TOKEN") { Err(_) => Client::without_key() }; -let routes = client.routes(HashMap::from([("filter[type]".into(), "0,1".into())])).expect("failed to get routes"); +let routes = client.routes(&[("filter[type]", "0,1")]).expect("failed to get routes"); let mut map = StaticMapBuilder::new() .width(1000) .height(1000) @@ -110,9 +110,9 @@ let mut map = StaticMapBuilder::new() .expect("failed to build map"); for route in routes.data { - let query = HashMap::from([("filter[route]".into(), route.id)]); + let query_params = [("filter[route]", &route.id)]; let shapes = client - .shapes(query.clone()) + .shapes(&query_params) .expect("failed to get shapes"); for shape in shapes.data { shape @@ -120,7 +120,7 @@ for route in routes.data { .expect("failed to plot shape"); } let stops = client - .stops(query.clone()) + .stops(&query_params) .expect("failed to get stops"); for stop in stops.data { stop.plot( diff --git a/src/client.rs b/src/client.rs index 698fce9..e8e4d5b 100644 --- a/src/client.rs +++ b/src/client.rs @@ -1,6 +1,6 @@ //! The client for interacting with the V3 API. -use std::collections::{HashMap, HashSet}; +use std::collections::HashSet; use serde::de::DeserializeOwned; @@ -27,10 +27,10 @@ macro_rules! mbta_endpoint_multiple { /// /// # Arguments /// - /// * `query_params` - a [HashMap] of query parameter names to values + /// * `query_params` - a slice of pairings of query parameter names to values /// /// ``` - /// # use std::{collections::HashMap, env}; + /// # use std::env; /// # use mbta_rs::Client; /// # /// # let client = match env::var("MBTA_TOKEN") { @@ -38,23 +38,23 @@ macro_rules! mbta_endpoint_multiple { /// # Err(_) => Client::without_key() /// # }; /// # - /// # let query_params = HashMap::from([ - /// # ("page[limit]".to_string(), "3".to_string()) - /// # ]); - #[doc = concat!("let ", stringify!($func), "_response = client.", stringify!($func), "(query_params);\n")] + /// # let query_params = [ + /// # ("page[limit]", "3") + /// # ]; + #[doc = concat!("let ", stringify!($func), "_response = client.", stringify!($func), "(&query_params);\n")] #[doc = concat!("if let Ok(", stringify!($func), ") = ", stringify!($func), "_response {\n")] #[doc = concat!(" for item in ", stringify!($func), ".data {\n")] /// println!("{}", item.id); /// } /// } /// ``` - pub fn $func(&self, query_params: HashMap) -> Result, ClientError> { + pub fn $func, V: AsRef>(&self, query_params: &[(K, V)]) -> Result, ClientError> { let allowed_query_params: HashSet = $allowed_query_params.into_iter().map(|s: &str| s.to_string()).collect(); - for (k, v) in &query_params { - if !allowed_query_params.contains(&k.to_string()) { + for (k, v) in query_params { + if !allowed_query_params.contains(k.as_ref()) { return Err(ClientError::InvalidQueryParam { - name: k.to_string(), - value: v.to_string(), + name: k.as_ref().to_string(), + value: v.as_ref().to_string(), }); } } @@ -76,7 +76,7 @@ macro_rules! mbta_endpoint_single { #[doc = concat!("* `id` - the id of the ", stringify!($func), " to return")] /// /// ``` - /// # use std::{collections::HashMap, env}; + /// # use std::env; /// # use mbta_rs::Client; /// # /// # let client = match env::var("MBTA_TOKEN") { @@ -91,7 +91,7 @@ macro_rules! mbta_endpoint_single { /// } /// ``` pub fn $func(&self, id: &str) -> Result, ClientError> { - self.get(&format!("{}/{}", $endpoint, id), HashMap::new()) + self.get::<$model, String, String>(&format!("{}/{}", $endpoint, id), &[]) } } }; @@ -279,7 +279,7 @@ pub struct Client { impl Client { /// Create a [Client] without an API key. /// - /// "Without an api key in the query string or as a request header, requests will be tracked by IP address and have stricter rate limit." + /// > "Without an api key in the query string or as a request header, requests will be tracked by IP address and have stricter rate limit." - Massachusetts Bay Transportation Authority pub fn without_key() -> Self { Self { api_key: None, @@ -312,20 +312,24 @@ impl Client { } } - /// Helper method for making generalized GET requests to any endpoint with any query parameters. - /// Presumes that all query parameters given in the [HashMap] are valid. + /// Helper method for making generalized `GET` requests to any endpoint with any query parameters. + /// Presumes that all query parameters given are valid. /// /// # Arguments /// - /// * query_params - a [HashMap] of query parameter names to values - fn get(&self, endpoint: &str, query_params: HashMap) -> Result, ClientError> { + /// * query_params - a slice of pairings of query parameter names to values + fn get, V: AsRef>( + &self, + endpoint: &str, + query_params: &[(K, V)], + ) -> Result, ClientError> { let path = format!("{}/{}", self.base_url, endpoint); let request = ureq::get(&path); let request = match &self.api_key { Some(key) => request.set("x-api-key", key), None => request, }; - let request = query_params.iter().fold(request, |r, (k, v)| r.query(k, v)); + let request = query_params.iter().fold(request, |r, (k, v)| r.query(k.as_ref(), v.as_ref())); let response: Response = request.call()?.into_json()?; Ok(response) } diff --git a/tests/map.rs b/tests/map.rs index 9f173ba..5d95bba 100644 --- a/tests/map.rs +++ b/tests/map.rs @@ -1,6 +1,6 @@ //! Simple tests for tile map plotting. -use std::{collections::HashMap, fs::remove_file, path::PathBuf}; +use std::{fs::remove_file, path::PathBuf}; use mbta_rs::{map::*, *}; use raster::{compare::similar, open}; @@ -24,7 +24,8 @@ fn image_file(relative_path: &str) -> PathBuf { #[rstest] fn test_simple_map_render(client: Client) { // Arrange - let routes = client.routes(HashMap::from([("filter[type]".into(), "0,1".into())])).expect("failed to get routes"); + let route_params = [("filter[type]", "0,1")]; + let routes = client.routes(&route_params).expect("failed to get routes"); let mut map = StaticMapBuilder::new() .width(1000) .height(1000) @@ -40,19 +41,19 @@ fn test_simple_map_render(client: Client) { // Act for route in routes.data { - let query = HashMap::from([("filter[route]".into(), route.id)]); - let shapes = client.shapes(query.clone()).expect("failed to get shapes"); + let params = [("filter[route]", &route.id)]; + let shapes = client.shapes(¶ms).expect("failed to get shapes"); for shape in shapes.data { shape .plot(&mut map, true, PlotStyle::new((route.attributes.color.clone(), 3.0), Some(("#FFFFFF".into(), 1.0)))) .expect("failed to plot shape"); } - let stops = client.stops(query.clone()).expect("failed to get stops"); + let stops = client.stops(¶ms).expect("failed to get stops"); for stop in stops.data { stop.plot(&mut map, true, PlotStyle::new((route.attributes.color.clone(), 3.0), Some(("#FFFFFF".into(), 1.0)))) .expect("failed to plot stop"); } - let vehicles = client.vehicles(query).expect("failed to get vehicles"); + let vehicles = client.vehicles(¶ms).expect("failed to get vehicles"); for vehicle in vehicles.data { vehicle .plot(&mut map, true, IconStyle::new(image_file("train.png"), 12.5, 12.5)) diff --git a/tests/simple.rs b/tests/simple.rs index 09753f1..c1c59af 100644 --- a/tests/simple.rs +++ b/tests/simple.rs @@ -6,8 +6,6 @@ macro_rules! test_endpoint_plural_and_singular { (plural_func=$plural_func:ident, singular_func=$singular_func:ident) => { #[cfg(test)] mod $plural_func { - use std::collections::HashMap; - use rstest::*; use mbta_rs::*; @@ -24,11 +22,10 @@ macro_rules! test_endpoint_plural_and_singular { #[rstest] fn success_plural_models(client: Client) { // Arrange + let params = [("page[limit]", "3")]; // Act - let $plural_func = client - .$plural_func(HashMap::from([("page[limit]".into(), "3".into())])) - .expect(&format!("failed to get {}", stringify!($plural_func))); + let $plural_func = client.$plural_func(¶ms).expect(&format!("failed to get {}", stringify!($plural_func))); // Assert assert_eq!($plural_func.data.len(), 3); @@ -39,11 +36,10 @@ macro_rules! test_endpoint_plural_and_singular { #[rstest] fn request_failure_plural_models(client: Client) { // Arrange + let params = [("sort", "foobar")]; // Act - let error = client - .$plural_func(HashMap::from([("sort".into(), "foobar".into())])) - .expect_err(&format!("{} did not fail", stringify!($plural_func))); + let error = client.$plural_func(¶ms).expect_err(&format!("{} did not fail", stringify!($plural_func))); // Assert if let ClientError::ResponseError { errors } = error { @@ -56,11 +52,10 @@ macro_rules! test_endpoint_plural_and_singular { #[rstest] fn query_param_failure_plural_models(client: Client) { // Arrange + let params = [("foo", "bar")]; // Act - let error = client - .$plural_func(HashMap::from([("foo".into(), "bar".into())])) - .expect_err(&format!("{} did not fail", stringify!($plural_func))); + let error = client.$plural_func(¶ms).expect_err(&format!("{} did not fail", stringify!($plural_func))); // Assert if let ClientError::InvalidQueryParam { name, value } = error { @@ -74,9 +69,8 @@ macro_rules! test_endpoint_plural_and_singular { #[rstest] fn success_singular_model(client: Client) { // Arrange - let $plural_func = client - .$plural_func(HashMap::from([("page[limit]".into(), "3".into())])) - .expect(&format!("failed to get {}", stringify!($plural_func))); + let params = [("page[limit]", "3")]; + let $plural_func = client.$plural_func(¶ms).expect(&format!("failed to get {}", stringify!($plural_func))); // Act & Assert for $singular_func in $plural_func.data { diff --git a/tests/with_route.rs b/tests/with_route.rs index 36aba14..d78fe53 100644 --- a/tests/with_route.rs +++ b/tests/with_route.rs @@ -7,8 +7,6 @@ macro_rules! test_endpoint_plural_with_route { (plural_func=$plural_func:ident) => { #[cfg(test)] mod $plural_func { - use std::collections::HashMap; - use rstest::*; use mbta_rs::*; @@ -25,15 +23,12 @@ macro_rules! test_endpoint_plural_with_route { #[rstest] fn success_plural_models(client: Client) { // Arrange - let routes = client.routes(HashMap::from([("page[limit]".into(), "1".into())])).expect("failed to get routes"); + let route_params = [("page[limit]", "1")]; + let routes = client.routes(&route_params).expect("failed to get routes"); + let params = [("page[limit]", "3"), ("filter[route]", &routes.data[0].id.clone())]; // Act - let $plural_func = client - .$plural_func(HashMap::from([ - ("page[limit]".into(), "3".into()), - ("filter[route]".into(), routes.data[0].id.clone()), - ])) - .expect(&format!("failed to get {}", stringify!($plural_func))); + let $plural_func = client.$plural_func(¶ms).expect(&format!("failed to get {}", stringify!($plural_func))); // Assert assert_eq!($plural_func.data.len(), 3); @@ -44,11 +39,10 @@ macro_rules! test_endpoint_plural_with_route { #[rstest] fn request_failure_plural_models(client: Client) { // Arrange + let params = [("sort", "foobar")]; // Act - let error = client - .$plural_func(HashMap::from([("sort".into(), "foobar".into())])) - .expect_err(&format!("{} did not fail", stringify!($plural_func))); + let error = client.$plural_func(¶ms).expect_err(&format!("{} did not fail", stringify!($plural_func))); // Assert if let ClientError::ResponseError { errors } = error { @@ -61,11 +55,10 @@ macro_rules! test_endpoint_plural_with_route { #[rstest] fn query_param_failure_plural_models(client: Client) { // Arrange + let params = [("foo", "bar")]; // Act - let error = client - .$plural_func(HashMap::from([("foo".into(), "bar".into())])) - .expect_err(&format!("{} did not fail", stringify!($plural_func))); + let error = client.$plural_func(¶ms).expect_err(&format!("{} did not fail", stringify!($plural_func))); // Assert if let ClientError::InvalidQueryParam { name, value } = error { @@ -86,8 +79,6 @@ macro_rules! test_endpoint_singular_with_route { (plural_func=$plural_func:ident, singular_func=$singular_func:ident) => { #[cfg(test)] mod $singular_func { - use std::collections::HashMap; - use rstest::*; use mbta_rs::*; @@ -104,13 +95,10 @@ macro_rules! test_endpoint_singular_with_route { #[rstest] fn success_singular_model(client: Client) { // Arrange - let routes = client.routes(HashMap::from([("page[limit]".into(), "1".into())])).expect("failed to get routes"); - let $plural_func = client - .$plural_func(HashMap::from([ - ("page[limit]".into(), "3".into()), - ("filter[route]".into(), routes.data[0].id.clone()), - ])) - .expect(&format!("failed to get {}", stringify!($plural_func))); + let route_params = [("page[limit]", "1")]; + let routes = client.routes(&route_params).expect("failed to get routes"); + let params = [("page[limit]", "3"), ("filter[route]", &routes.data[0].id.clone())]; + let $plural_func = client.$plural_func(¶ms).expect(&format!("failed to get {}", stringify!($plural_func))); // Act & Assert for $singular_func in $plural_func.data { From b0a0f35060f43d058b868a124fa5b3876bb48cbe Mon Sep 17 00:00:00 2001 From: Robert Yin Date: Wed, 25 May 2022 20:07:26 -0400 Subject: [PATCH 2/2] chore: bump to 0.3.0 in cargo.toml --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 2802292..9a84042 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mbta-rs" -version = "0.2.1" +version = "0.3.0" edition = "2021" authors = ["Robert Yin "] description = "Simple Rust client for interacting with the MBTA V3 API."