Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 2 deletions lychee-bin/src/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,14 +265,13 @@ async fn handle(
// `accepted` status codes might have changed from the previous run
// and they may have an impact on the interpretation of the status
// code.
client.host_pool().record_persistent_cache_hit(&uri);
Status::from_cache_status(v.value().status, &accept)
};

client.host_pool().record_cache_hit(&uri);
return Ok(Response::new(uri.clone(), status, request.source.into()));
}

client.host_pool().record_cache_miss(&uri);
let response = check_url(client, request).await;

// - Never cache filesystem access as it is fast already so caching has no benefit.
Expand Down
8 changes: 2 additions & 6 deletions lychee-bin/src/formatters/host_stats/compact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,8 @@ impl Compact {
}

impl HostStatsFormatter for Compact {
fn format(&self, host_stats: HashMap<String, HostStats>) -> Result<Option<String>> {
if host_stats.is_empty() {
return Ok(None);
}

fn format(&self, host_stats: HashMap<String, HostStats>) -> Result<String> {
let compact = CompactHostStats { host_stats };
Ok(Some(compact.to_string()))
Ok(compact.to_string())
}
}
8 changes: 2 additions & 6 deletions lychee-bin/src/formatters/host_stats/detailed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,8 @@ impl Detailed {
}

impl HostStatsFormatter for Detailed {
fn format(&self, host_stats: HashMap<String, HostStats>) -> Result<Option<String>> {
if host_stats.is_empty() {
return Ok(None);
}

fn format(&self, host_stats: HashMap<String, HostStats>) -> Result<String> {
let detailed = DetailedHostStats { host_stats };
Ok(Some(detailed.to_string()))
Ok(detailed.to_string())
}
}
12 changes: 3 additions & 9 deletions lychee-bin/src/formatters/host_stats/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,13 @@ pub(crate) struct Json;

impl Json {
pub(crate) const fn new() -> Self {
Self {}
Self
}
}

impl HostStatsFormatter for Json {
/// Format host stats as JSON object
fn format(&self, host_stats: HashMap<String, HostStats>) -> Result<Option<String>> {
if host_stats.is_empty() {
return Ok(None);
}

fn format(&self, host_stats: HashMap<String, HostStats>) -> Result<String> {
// Convert HostStats to a more JSON-friendly format
let json_stats: HashMap<String, serde_json::Value> = host_stats
.into_iter()
Expand Down Expand Up @@ -50,8 +46,6 @@ impl HostStatsFormatter for Json {
"host_statistics": json_stats
});

serde_json::to_string_pretty(&output)
.map(Some)
.context("Cannot format host stats as JSON")
serde_json::to_string_pretty(&output).context("Cannot format host stats as JSON")
}
}
8 changes: 2 additions & 6 deletions lychee-bin/src/formatters/host_stats/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,8 @@ impl Markdown {
}

impl HostStatsFormatter for Markdown {
fn format(&self, host_stats: HashMap<String, HostStats>) -> Result<Option<String>> {
if host_stats.is_empty() {
return Ok(None);
}

fn format(&self, host_stats: HashMap<String, HostStats>) -> Result<String> {
let markdown = MarkdownHostStats(host_stats);
Ok(Some(markdown.to_string()))
Ok(markdown.to_string())
}
}
2 changes: 1 addition & 1 deletion lychee-bin/src/formatters/host_stats/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use std::collections::HashMap;
/// Trait for formatting per-host statistics in different output formats
pub(crate) trait HostStatsFormatter {
/// Format the host statistics and return them as a string
fn format(&self, host_stats: HashMap<String, HostStats>) -> Result<Option<String>>;
fn format(&self, host_stats: HashMap<String, HostStats>) -> Result<String>;
}

/// Sort host statistics by request count (descending order)
Expand Down
19 changes: 9 additions & 10 deletions lychee-bin/src/host_stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,16 @@ pub(crate) fn output_per_host_statistics(host_pool: &HostPool, config: &Config)

let host_stats = host_pool.all_host_stats();
let host_stats_formatter = get_host_stats_formatter(&config.format, &config.mode);
let formatted_host_stats = host_stats_formatter.format(host_stats)?;

if let Some(formatted_host_stats) = host_stats_formatter.format(host_stats)? {
if let Some(output) = &config.output {
// For file output, append to the existing output
let mut file_content = std::fs::read_to_string(output).unwrap_or_default();
file_content.push_str(&formatted_host_stats);
std::fs::write(output, file_content)
.context("Cannot write host stats to output file")?;
} else {
print!("{formatted_host_stats}");
}
if let Some(output) = &config.output {
// For file output, append to the existing output
let mut file_content = std::fs::read_to_string(output).unwrap_or_default();
file_content.push_str(&formatted_host_stats);
std::fs::write(output, file_content).context("Cannot write host stats to output file")?;
} else {
print!("{formatted_host_stats}");
}

Ok(())
}
45 changes: 45 additions & 0 deletions lychee-bin/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1482,6 +1482,51 @@ The config file should contain every possible key for documentation purposes."
Ok(())
}

#[tokio::test]
async fn test_process_internal_host_caching() -> Result<()> {
// Note that this process-internal per-host caching
// has no direct relation to the lychee cache file
// where state can be persisted between multiple invocations.
let server = wiremock::MockServer::start().await;

// Return one rate-limited response to make sure that
// such a response isn't cached.
wiremock::Mock::given(wiremock::matchers::method("GET"))
.respond_with(ResponseTemplate::new(429))
.up_to_n_times(1)
.mount(&server)
.await;

wiremock::Mock::given(wiremock::matchers::method("GET"))
.respond_with(ResponseTemplate::new(200))
.expect(1)
.mount(&server)
.await;

let temp_dir = tempfile::tempdir()?;
for i in 0..9 {
let test_md1 = temp_dir.path().join(format!("test{i}.md"));
fs::write(&test_md1, server.uri())?;
}

cargo_bin_cmd!()
.arg(temp_dir.path())
.arg("--host-stats")
.assert()
.success()
.stdout(contains("9 Total"))
.stdout(contains("9 OK"))
.stdout(contains("0 Errors"))
// Per-host statistics
// 1 rate limited + 9 OK
.stdout(contains("10 reqs"))
// 1 rate limited, 1 OK, 8 cached
.stdout(contains("80.0% cached"));

server.verify().await;
Ok(())
}

#[test]
fn test_verbatim_skipped_by_default() {
let input = fixtures_path!().join("TEST_CODE_BLOCKS.md");
Expand Down
34 changes: 15 additions & 19 deletions lychee-lib/src/checker/website.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ use crate::{
BasicAuthCredentials, ErrorKind, FileType, Status, Uri,
chain::{Chain, ChainResult, ClientRequestChains, Handler, RequestChain},
quirks::Quirks,
ratelimit::HostPool,
ratelimit::{CacheableResponse, HostPool},
retry::RetryExt,
types::{redirect_history::RedirectHistory, uri::github::GithubUri},
utils::fragment_checker::{FragmentChecker, FragmentInput},
};
use async_trait::async_trait;
use http::{Method, StatusCode};
use octocrab::Octocrab;
use reqwest::{Request, Response, header::CONTENT_TYPE};
use reqwest::{Request, header::CONTENT_TYPE};
use std::{collections::HashSet, path::Path, sync::Arc, time::Duration};
use url::Url;

Expand Down Expand Up @@ -127,12 +127,12 @@ impl WebsiteChecker {
// when `accept=200,429`, `status_code=429` will be treated as success
// but we are not able the check the fragment since it's inapplicable.
if self.include_fragments
&& response.status().is_success()
&& response.status.is_success()
&& method == Method::GET
&& request_url.fragment().is_some_and(|x| !x.is_empty())
{
let Some(content_type) = response
.headers()
.headers
.get(CONTENT_TYPE)
.and_then(|header| header.to_str().ok())
else {
Expand All @@ -143,7 +143,7 @@ impl WebsiteChecker {
ct if ct.starts_with("text/html") => FileType::Html,
ct if ct.starts_with("text/markdown") => FileType::Markdown,
ct if ct.starts_with("text/plain") => {
let path = Path::new(response.url().path());
let path = Path::new(response.url.path());
match path.extension() {
Some(ext) if ext.eq_ignore_ascii_case("md") => FileType::Markdown,
_ => return status,
Expand All @@ -169,22 +169,18 @@ impl WebsiteChecker {
&self,
url: Url,
status: Status,
response: Response,
response: CacheableResponse,
file_type: FileType,
) -> Status {
match response.text().await {
Ok(content) => {
match self
.fragment_checker
.check(FragmentInput { content, file_type }, &url)
.await
{
Ok(true) => status,
Ok(false) => Status::Error(ErrorKind::InvalidFragment(url.into())),
Err(e) => Status::Error(e),
}
}
Err(e) => Status::Error(ErrorKind::ReadResponseBody(e)),
let content = response.text;
match self
.fragment_checker
.check(FragmentInput { content, file_type }, &url)
.await
{
Ok(true) => status,
Ok(false) => Status::Error(ErrorKind::InvalidFragment(url.into())),
Err(e) => Status::Error(e),
}
}

Expand Down
Loading