From c17f6f8606b93959f285094f68117dae2141931f Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 8 Apr 2024 07:24:35 +0200 Subject: [PATCH] refactor --- src/sparse.rs | 43 ++++++++++++-------------- tests/sparse_index/mod.rs | 64 +++++++++++++++++++++------------------ 2 files changed, 53 insertions(+), 54 deletions(-) diff --git a/src/sparse.rs b/src/sparse.rs index fe8eda26..00b4cd87 100644 --- a/src/sparse.rs +++ b/src/sparse.rs @@ -186,26 +186,26 @@ impl SparseIndex { } /// Creates an HTTP request that can be sent via your HTTP client of choice - /// to retrieve the config for this index + /// to retrieve the config for this index. /// - /// See [`Self::parse_config_response`] processing the response from the remote - /// index + /// See [`Self::parse_config_response()`] processing the response from the remote + /// index. /// /// It is highly recommended to assume HTTP/2 when making requests to remote - /// indices, at least crates.io + /// indices, at least crates.io. #[cfg(feature = "sparse")] pub fn make_config_request(&self) -> Result { self.make_request(&format!("{}config.json", self.url()), None) } /// Creates an HTTP request that can be sent via your HTTP client of choice - /// to retrieve the current metadata for the specified crate + /// to retrieve the current metadata for the specified crate `namw`. /// - /// See [`Self::parse_cache_response`] processing the response from the remote - /// index + /// See [`Self::parse_cache_response()`] processing the response from the remote + /// index. /// /// It is highly recommended to assume HTTP/2 when making requests to remote - /// indices, at least crates.io + /// indices, at least crates.io. #[cfg(feature = "sparse")] pub fn make_cache_request(&self, name: &str) -> Result { self.make_request( @@ -216,39 +216,34 @@ impl SparseIndex { ) } - /// Process the response to a request created by [`Self::make_config_request`] + /// Process the response to a request created by [`Self::make_config_request()`]. /// /// You may specify whether an updated config file is written locally to the - /// cache or not + /// cache or not, but if `write_config` is `true`, the write operation may fail, + /// and you will not receive the parsed configuration. /// - /// Note that responses from sparse HTTP indices, at least crates.io, may + /// Note that the `response` from sparse HTTP indices, at least crates.io, may /// send responses with `gzip` compression, it is your responsibility to - /// decompress it before sending to this function + /// decompress it before sending to this function. #[cfg(feature = "sparse")] pub fn parse_config_response( &self, response: http::Response>, - write_cache_entry: bool, + write_config: bool, ) -> Result { use http::StatusCode; let (parts, body) = response.into_parts(); match parts.status { - // The server responded with the full contents of the config StatusCode::OK => { - if write_cache_entry { + let res = serde_json::from_slice(&body).map_err(Error::Json); + if write_config { let path = self.path.join("config.json"); - if std::fs::create_dir_all(path.parent().unwrap()).is_ok() { - // It's unfortunate if this fails for some reason, but - // not writing the cache entry shouldn't stop the user - // from getting the config - let _ = std::fs::write(&path, &body); - } + std::fs::create_dir_all(path.parent().unwrap())?; + std::fs::write(&path, &body)?; } - - serde_json::from_slice(&body).map_err(Error::Json) + res } - // The server requires authorization but the user didn't provide it StatusCode::UNAUTHORIZED => { Err(io::Error::new(io::ErrorKind::PermissionDenied, "the request was not authorized").into()) } diff --git a/tests/sparse_index/mod.rs b/tests/sparse_index/mod.rs index ee15a999..5395892d 100644 --- a/tests/sparse_index/mod.rs +++ b/tests/sparse_index/mod.rs @@ -36,7 +36,7 @@ mod with_sparse_http_feature { // Validates that a valid request is generated when there is no cache entry // for a crate #[test] - fn generates_request_for_missing_cache_entry() { + fn generate_request_for_missing_cache_entry() { let index = crates_io(); let builder = index.make_cache_request("serde").unwrap(); let req: Request> = builder.body(vec![]).unwrap(); @@ -57,7 +57,7 @@ mod with_sparse_http_feature { // Validates that a valid request is generated when there is a local cache // entry for a crate #[test] - fn generates_request_for_local_cache_entry() { + fn generate_request_for_local_cache_entry() { let index = crates_io(); let builder = index.make_cache_request("autocfg").unwrap(); let req: Request> = builder.body(vec![]).unwrap(); @@ -81,7 +81,7 @@ mod with_sparse_http_feature { // Validates that a response with the full index contents are properly parsed #[test] - fn parses_modified_response() { + fn modified_response() { let index = crates_io(); let response = http::Response::builder() .status(http::StatusCode::OK) @@ -96,7 +96,7 @@ mod with_sparse_http_feature { // Validates that a response for an index entry that has not been modified is // parsed correctly #[test] - fn parses_unmodified_response() { + fn unmodified_response() { let index = crates_io(); let response = http::Response::builder() .status(http::StatusCode::NOT_MODIFIED) @@ -114,7 +114,7 @@ mod with_sparse_http_feature { // Validates that a response for an index entry that does not exist is // parsed correcty #[test] - fn parses_missing_response() { + fn missing_response() { let index = crates_io(); let response = http::Response::builder() .status(http::StatusCode::NOT_FOUND) @@ -125,28 +125,26 @@ mod with_sparse_http_feature { } } - mod make_config_request { + #[test] + fn make_config_request() { use crate::sparse_index::with_sparse_http_feature::crates_io; use http::{header, Request}; - #[test] - fn generates_request() { - let index = crates_io(); - let builder = index.make_config_request().unwrap(); - let req: Request> = builder.body(vec![]).unwrap(); - - assert_eq!(req.uri(), format!("{}config.json", index.url()).as_str()); - assert!(req.headers().get(header::IF_NONE_MATCH).is_none()); - assert!(req.headers().get(header::IF_MODIFIED_SINCE).is_none()); - assert_eq!(req.headers().get(header::ACCEPT_ENCODING).unwrap(), "gzip,identity"); - assert_eq!( - req.headers() - .get(header::HeaderName::from_static("cargo-protocol")) - .unwrap(), - "version=1" - ); - assert_eq!(req.headers().get(header::ACCEPT).unwrap(), "text/plain"); - } + let index = crates_io(); + let builder = index.make_config_request().unwrap(); + let req: Request> = builder.body(vec![]).unwrap(); + + assert_eq!(req.uri(), format!("{}config.json", index.url()).as_str()); + assert!(req.headers().get(header::IF_NONE_MATCH).is_none()); + assert!(req.headers().get(header::IF_MODIFIED_SINCE).is_none()); + assert_eq!(req.headers().get(header::ACCEPT_ENCODING).unwrap(), "gzip,identity"); + assert_eq!( + req.headers() + .get(header::HeaderName::from_static("cargo-protocol")) + .unwrap(), + "version=1" + ); + assert_eq!(req.headers().get(header::ACCEPT).unwrap(), "text/plain"); } mod parse_config_response { @@ -170,26 +168,32 @@ mod with_sparse_http_feature { } #[test] - fn parses_response() { + fn parse() { let (_dir, index) = crates_io_tmp(); let config = index.parse_config_response(make_response(), false).unwrap(); assert_eq!(config.dl, "https://static.crates.io/crates"); assert_eq!(config.api.as_deref(), Some("https://crates.io")); + + assert!( + matches!(index.index_config(), Err(Error::Io(_))), + "the configuration shouldn't exist, hence we cannot query the index configuration" + ); } #[test] - fn stores_response() { + fn parse_and_store() { let (_dir, index) = crates_io_tmp(); - let Err(Error::Io(err)) = index.index_config() else { - panic!("expected to get an io error") + match index.index_config() { + Err(Error::Io(err)) => { + assert_eq!(err.kind(), io::ErrorKind::NotFound); + } + _ => unreachable!("precondition: there is no configuration and this triggers an IO error"), }; - assert!(err.kind() == io::ErrorKind::NotFound); let config = index.parse_config_response(make_response(), true).unwrap(); - let stored_config = index.index_config().unwrap(); assert_eq!(config.dl, stored_config.dl);