From 7ca694c7e95be22c824eaf0e9a028a01b6167a6c 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, 54 insertions(+), 53 deletions(-) diff --git a/src/sparse.rs b/src/sparse.rs index fe8eda26..c20fbf7b 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,19 +216,20 @@ 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(); @@ -236,14 +237,10 @@ impl SparseIndex { match parts.status { // The server responded with the full contents of the config StatusCode::OK => { - if write_cache_entry { + 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) @@ -261,7 +258,7 @@ impl SparseIndex { "the server responded with status code '{other}', which is not supported in the current protocol" ), ) - .into()), + .into()), } } @@ -332,7 +329,7 @@ impl SparseIndex { "the server responded with status code '{other}', which is not supported in the current protocol" ), ) - .into()), + .into()), } } } @@ -350,7 +347,7 @@ mod tests { .join("tests/fixtures/sparse_registry_cache/cargo_home"), crate::sparse::URL, ) - .unwrap() + .unwrap() } // curl -v -H 'accept-encoding: gzip,identity' https://index.crates.io/cr/at/crates-index 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);