Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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{}.md", i));
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