review and QA

2026-08-15 22:49:18 +01:00
parent 3c7043562e
commit 7f01f1a341
10 changed files with 477 additions and 220 deletions
@@ -1220,6 +1220,12 @@ dependencies = [
 "wasm-bindgen",
 ]

+[[package]]
+name = "lazy_static"
+version = "1.5.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe"
+
 [[package]]
 name = "libc"
 version = "0.2.189"
@@ -1819,15 +1825,17 @@ dependencies = [

 [[package]]
 name = "rs-scraper"
-version = "0.1.0"
+version = "0.2.0"
 dependencies = [
 "anydoc",
 "anyhow",
+ "bytes",
 "chrono",
 "clap",
 "env_logger",
 "hex",
 "htmd",
+ "lazy_static",
 "log",
 "md-5",
 "reqwest",
@@ -1,6 +1,7 @@
+# Cargo.toml
 [package]
 name = "rs-scraper"
-version = "0.1.0"
+version = "0.2.0"
 edition = "2024"

 [dependencies]
@@ -10,6 +11,7 @@ chrono = { version = "0.4", features = ["serde"] }
 clap = { version = "4", features = ["derive"] }
 hex = "0.4"
 htmd = "0.1"
+lazy_static = "1.4"
 log = "0.4"
 env_logger = "0.11"
 md-5 = "0.10"
@@ -20,3 +22,10 @@ serde_json = "1"
 sha2 = "0.10"
 tokio = { version = "1", features = ["full"] }
 url = "2"
+bytes = "1.12"
+
+[profile.release]
+opt-level = "z"
+lto = true
+codegen-units = 1
+strip = true
@@ -4,6 +4,4 @@ pub const MAX_RETRIES: u32 = 3;
 pub const RETRY_BACKOFF_SECS: u64 = 5;
 pub const USER_AGENT: &str = "web-scraper/1.0 (research)";

-/// Rate limit in milliseconds between requests. Set to 0 only for trusted/internal sites.
-/// Default is 1000ms to avoid IP bans on public sites like logting.fo.
 pub const RATE_LIMIT_MS: u64 = DELAY_MS;
@@ -1,15 +1,84 @@
+#![warn(clippy::all, clippy::pedantic)]
+
 use htmd::HtmlToMarkdown;
+use scraper::{Html, Selector};
 use std::sync::OnceLock;

 static CONVERTER: OnceLock<HtmlToMarkdown> = OnceLock::new();
+static BODY_SELECTOR: OnceLock<Selector> = OnceLock::new();
+
+fn get_converter() -> &'static HtmlToMarkdown {
+    CONVERTER.get_or_init(|| {
+        HtmlToMarkdown::builder()
+            .skip_tags(vec![
+                "nav", "footer", "header", "aside", "script", "style", "noscript",
+            ])
+            .build()
+    })
+}
+
+fn get_body_selector() -> &'static Selector {
+    BODY_SELECTOR.get_or_init(|| Selector::parse("body").expect("hardcoded selector is valid"))
+}
+
+/// Extract the `<body>` element's HTML from a full HTML document.
+/// The HTML5 parser always synthesizes a `<body>` element, so this
+/// returns `Some` for any well-formed document. Returns `None` only
+/// if the parser fails entirely.
+fn extract_body(html: &str) -> Option<String> {
+    let document = Html::parse_document(html);
+    document
+        .select(get_body_selector())
+        .next()
+        .map(|body| body.html())
+}

 pub fn html_to_markdown(html: &str) -> String {
-    let converter = CONVERTER.get_or_init(HtmlToMarkdown::new);
-    match converter.convert(html) {
+    let converter = get_converter();
+    let source = extract_body(html).unwrap_or_else(|| html.to_string());
+
+    match converter.convert(&source) {
        Ok(md) => md,
        Err(e) => {
            log::warn!("HTML-to-MD conversion failed ({}), saving raw HTML", e);
-            html.to_string()
+            source
        }
    }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_extract_body_with_body_tag() {
+        let html = "<html><head></head><body><p>Hello</p></body></html>";
+        let result = extract_body(html);
+        assert!(result.is_some());
+        assert!(result.unwrap().contains("<p>Hello</p>"));
+    }
+
+    #[test]
+    fn test_extract_body_without_body_tag_falls_back_to_synthetic() {
+        // HTML5 parser synthesizes a <body> element even when absent in source.
+        let html = "<html><head></head><p>Hello</p></html>";
+        let result = extract_body(html);
+        assert!(result.is_some(), "parser should synthesize a body element");
+        assert!(result.unwrap().contains("Hello"));
+    }
+
+    #[test]
+    fn test_html_to_markdown_preserves_content() {
+        let html = "<html><body><h1>Title</h1><p>Content</p></body></html>";
+        let md = html_to_markdown(html);
+        assert!(md.contains("# Title"));
+        assert!(md.contains("Content"));
+    }
+
+    #[test]
+    fn test_html_to_markdown_handles_malformed() {
+        let html = "<html><body><p>Unclosed";
+        let md = html_to_markdown(html);
+        assert!(!md.is_empty());
+    }
+}
@@ -1,3 +1,6 @@
+// src/doc_processor.rs
+#![warn(clippy::all, clippy::pedantic)]
+
 use anyhow::Result;
 use std::path::Path;
 use url::Url;
@@ -33,7 +36,7 @@ pub fn try_convert(bytes: &[u8], url: &Url) -> DocProcessResult {
 }

 #[allow(dead_code)]
-pub fn batch_convert_docs(dir: &Path) -> Result<(usize, usize, usize)> {
+pub fn batch_convert_docs(dir: &Path, delete_originals: bool) -> Result<(usize, usize, usize)> {
    let supported_exts = [
        "pdf", "doc", "docx", "docm", "ppt", "pps", "pot", "pptx", "pptm", "ppsx", "ppsm", "xls",
        "xlsx", "xlsm", "xlsb", "odt", "ods", "odp", "rtf", "epub", "csv",
@@ -81,8 +84,12 @@ pub fn batch_convert_docs(dir: &Path) -> Result<(usize, usize, usize)> {
            Ok(md) if !md.trim().is_empty() => {
                let md_path = path.with_extension("md");
                std::fs::write(&md_path, md)?;
+                if delete_originals {
                    std::fs::remove_file(&path)?;
+                    log::info!("  converted -> {} (deleted original)", md_path.display());
+                } else {
                    log::info!("  converted -> {}", md_path.display());
+                }
                converted += 1;
            }
            _ => {
@@ -94,3 +101,29 @@ pub fn batch_convert_docs(dir: &Path) -> Result<(usize, usize, usize)> {

    Ok((converted, raw, errors))
 }
+
+// src/doc_processor.rs - tests section added at end
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use url::Url;
+
+    #[test]
+    fn test_try_convert_detects_format() {
+        let html_bytes = b"<html><body><p>Test</p></body></html>";
+        let url = Url::parse("https://example.com/test.html").unwrap();
+        let result = try_convert(html_bytes, &url);
+        // Will likely return Raw since anydoc may not handle HTML
+        match result {
+            DocProcessResult::Markdown(_) | DocProcessResult::Raw => {}
+        }
+    }
+
+    #[test]
+    fn test_try_convert_empty_output() {
+        let empty_bytes = b"";
+        let url = Url::parse("https://example.com/test.html").unwrap();
+        let result = try_convert(empty_bytes, &url);
+        assert!(matches!(result, DocProcessResult::Raw));
+    }
+}
@@ -1,8 +1,12 @@
+// src/error_logger.rs
+#![warn(clippy::all, clippy::pedantic)]
+
 use chrono::{DateTime, Utc};
 use serde::Serialize;
 use std::fs::OpenOptions;
-use std::io::Write;
+use std::io::{BufWriter, Write};
 use std::path::Path;
+use std::sync::Mutex;
 use url::Url;

 #[derive(Serialize)]
@@ -15,6 +19,10 @@ pub struct ErrorEntry {
    pub status_code: Option<u16>,
 }

+lazy_static::lazy_static! {
+    static ref LOG_WRITER: Mutex<Option<BufWriter<std::fs::File>>> = Mutex::new(None);
+}
+
 pub fn log_error(
    log_path: &Path,
    url: &Url,
@@ -38,14 +46,27 @@ pub fn log_error(
        }
    };

-    if let Ok(mut file) = OpenOptions::new().append(true).create(true).open(log_path) {
-        if let Err(e) = file.write_all(line.as_bytes()) {
+    // Buffered writer approach
+    {
+        let mut guard = LOG_WRITER.lock().unwrap();
+        match guard.as_mut() {
+            Some(writer) => {
+                if let Err(e) = writer.write_all(line.as_bytes()) {
                    log::error!("Failed to write error log: {}", e);
-        } else {
-            let _ = file.flush();
-            log::info!("LOGGED ERROR [{}]: {}", error_type, message);
                }
+            }
+            None => {
+                if let Ok(file) = OpenOptions::new().append(true).create(true).open(log_path) {
+                    let mut writer = BufWriter::new(file);
+                    let _ = writer.write_all(line.as_bytes());
+                    let _ = writer.flush();
                } else {
                    log::error!("Failed to open error log file: {}", log_path.display());
+                    return;
                }
+            }
+        }
+    }
+
+    log::info!("LOGGED ERROR [{}]: {}", error_type, message);
 }
@@ -1,4 +1,6 @@
-use crate::config::{MAX_RETRIES, RETRY_BACKOFF_SECS, TIMEOUT_SECS, USER_AGENT};
+#![warn(clippy::all, clippy::pedantic)]
+
+use crate::config::{MAX_RETRIES, RETRY_BACKOFF_SECS};
 use anyhow::{Result, anyhow};
 use reqwest::Client;
 use std::time::Duration;
@@ -9,14 +11,7 @@ pub struct Fetcher {
 }

 impl Fetcher {
-    pub fn new() -> Result<Self> {
-        let client = Client::builder()
-            .timeout(Duration::from_secs(TIMEOUT_SECS))
-            .user_agent(USER_AGENT)
-            .redirect(reqwest::redirect::Policy::limited(10))
-            .build()
-            .map_err(|e| anyhow!("Failed to create HTTP client: {}", e))?;
-
+    pub fn new(client: Client) -> Result<Self> {
        Ok(Self { client })
    }

@@ -34,7 +29,7 @@ impl Fetcher {
                        .to_string();

                    let bytes = match resp.bytes().await {
-                        Ok(b) => b.to_vec(),
+                        Ok(b) => b,
                        Err(e) => {
                            if attempt < MAX_RETRIES {
                                log::warn!(
@@ -90,7 +85,7 @@ impl Fetcher {
 }

 pub struct FetchResult {
-    pub bytes: Vec<u8>,
+    pub bytes: bytes::Bytes,
    pub content_type: String,
    pub final_url: Url,
 }
@@ -1,3 +1,5 @@
+#![warn(clippy::all, clippy::pedantic)]
+
 mod config;
 mod converter;
 mod doc_processor;
@@ -10,16 +12,19 @@ mod url_utils;

 use crate::robots::fetch_robots;
 use crate::scraper::Scraper;
-use anyhow::Result;
+use anyhow::{Result, bail};
 use clap::Parser;
+use reqwest::Client;
+use std::collections::HashSet;
 use std::fs;
 use std::path::PathBuf;
+use std::time::Duration;
 use url::Url;

 #[derive(Parser, Debug)]
 #[command(name = "rs-scraper")]
 #[command(author = "FLÓ")]
-#[command(version = "0.1.0")]
+#[command(version = "0.2.0")]
 #[command(about = "Web scraper for Faroese public sector sites")]
 struct Args {
    #[arg(help = "Starting URL to scrape")]
@@ -27,6 +32,34 @@ struct Args {

    #[arg(long, help = "Save documents as-is, skip Markdown conversion")]
    no_doc_conversion: bool,
+
+    #[arg(
+        long,
+        help = "Comma-separated extensions to include (e.g. pdf,docx,html). \
+                Only files matching these types are saved. \
+                HTML pages are still crawled for links even if excluded. \
+                Mutually exclusive with --exclude-types."
+    )]
+    types: Option<String>,
+
+    #[arg(
+        long,
+        help = "Comma-separated extensions to exclude (e.g. pdf,xlsx). \
+                All file types are saved except these. \
+                HTML pages are still crawled for links. \
+                Mutually exclusive with --types."
+    )]
+    exclude_types: Option<String>,
+
+    #[arg(short = 'o', long, help = "Output directory for scraped files")]
+    output: Option<String>,
+}
+
+fn parse_ext_list(s: &str) -> HashSet<String> {
+    s.split(',')
+        .map(|s| s.trim().trim_start_matches('.').to_lowercase())
+        .filter(|s| !s.is_empty())
+        .collect()
 }

 #[tokio::main]
@@ -43,25 +76,72 @@ async fn main() -> Result<()> {
    let base_domain = url_utils::derive_base_domain(&url)
        .ok_or_else(|| anyhow::anyhow!("Could not parse hostname from {}", start_url))?;

-    let output_dir_name = format!("scraped_{}", base_domain.replace('.', "_"));
-    let output_dir = PathBuf::from(&output_dir_name);
-    let logs_dir = output_dir.join("logs");
+    let output_dir = match &args.output {
+        Some(p) => PathBuf::from(p),
+        None => {
+            let default_name = format!("{}_scraped", base_domain.replace('.', "_"));
+            PathBuf::from(default_name)
+        }
+    };

+    let logs_dir = {
+        let output_name = output_dir
+            .file_name()
+            .map(|n| n.to_string_lossy().into_owned())
+            .unwrap_or_else(|| "output".to_string());
+        let logs_name = format!("{}_logs", output_name);
+        output_dir
+            .parent()
+            .map(|p| p.join(&logs_name))
+            .unwrap_or_else(|| PathBuf::from(&logs_name))
+    };
+
+    fs::create_dir_all(&output_dir)?;
    fs::create_dir_all(&logs_dir)?;

    let log_path = logs_dir.join("scrape_errors.jsonl");
    fs::remove_file(&log_path).ok();

+    let (include_types, exclude_types) = match (&args.types, &args.exclude_types) {
+        (Some(_), Some(_)) => {
+            bail!("--types and --exclude-types are mutually exclusive");
+        }
+        (Some(t), None) => (Some(parse_ext_list(t)), None),
+        (None, Some(t)) => (None, Some(parse_ext_list(t))),
+        (None, None) => (None, None),
+    };
+
+    let scope_path = {
+        let path = url.path().trim_end_matches('/');
+        if path.is_empty() {
+            None
+        } else {
+            Some(path.to_string())
+        }
+    };
+
    log::info!("Scraping: {}", start_url);
    log::info!("Base domain: {}", base_domain);
    log::info!("Output dir: {}", output_dir.display());
+    log::info!("Logs dir: {}", logs_dir.display());
    log::info!(
        "Document conversion: {}",
        if convert_docs { "enabled" } else { "disabled" }
    );

-    let client = reqwest::Client::builder()
-        .timeout(std::time::Duration::from_secs(30))
+    match (&include_types, &exclude_types) {
+        (Some(t), _) => log::info!("Type filter: include {:?}", t),
+        (_, Some(t)) => log::info!("Type filter: exclude {:?}", t),
+        _ => log::info!("Type filter: none (all types)"),
+    }
+
+    match &scope_path {
+        Some(s) => log::info!("Path scope: {} (only this path and deeper)", s),
+        None => log::info!("Path scope: none (full site)"),
+    }
+
+    let client = Client::builder()
+        .timeout(Duration::from_secs(config::TIMEOUT_SECS))
        .user_agent(config::USER_AGENT)
        .build()?;

@@ -69,19 +149,22 @@ async fn main() -> Result<()> {
    let robots = fetch_robots(&client, &url, config::USER_AGENT).await;
    log::info!("");

-    let fetcher = fetcher::Fetcher::new()?;
-    let scraper = Scraper::new(
+    let fetcher = fetcher::Fetcher::new(client)?;
+    let mut scraper = Scraper::new(
        output_dir,
-        log_path,
+        log_path.clone(),
        base_domain,
        robots,
        convert_docs,
        fetcher,
+        include_types,
+        exclude_types,
+        scope_path,
    );
    let (count, errors) = scraper.run(&url).await;

    log::info!("\nDone. Fetched {} URLs. Errors: {}.", count, errors);
-    log::info!("Error log: {}/logs/scrape_errors.jsonl", output_dir_name);
+    log::info!("Error log: {}", log_path.display());

    Ok(())
 }
@@ -1,16 +1,16 @@
+#![warn(clippy::all, clippy::pedantic)]
+
 use crate::converter::html_to_markdown;
 use crate::doc_processor::{DocProcessResult, try_convert};
 use crate::error_logger::log_error;
 use crate::extractor::extract_links;
 use crate::fetcher::{FetchResult, Fetcher};
 use crate::robots::RobotsRule;
-use crate::url_utils::{normalize_url, url_to_filename};
+use crate::url_utils::{get_extension, is_in_scope, normalize_url, url_to_filename};
 use anyhow::Result;
 use std::collections::{HashSet, VecDeque};
 use std::fs;
-use std::sync::Arc;
 use std::time::Duration;
-use tokio::sync::Mutex;
 use url::Url;

 pub struct DocStats {
@@ -31,13 +31,16 @@ impl Default for DocStats {

 pub struct Scraper {
    fetcher: Fetcher,
-    seen: Arc<Mutex<HashSet<String>>>,
+    seen: HashSet<String>,
    output_dir: std::path::PathBuf,
    log_path: std::path::PathBuf,
    base_domain: String,
    robots: RobotsRule,
-    doc_stats: Arc<Mutex<DocStats>>,
+    doc_stats: DocStats,
    convert_docs: bool,
+    include_types: Option<HashSet<String>>,
+    exclude_types: Option<HashSet<String>>,
+    scope_path: Option<String>,
 }

 impl Scraper {
@@ -48,26 +51,56 @@ impl Scraper {
        robots: RobotsRule,
        convert_docs: bool,
        fetcher: Fetcher,
+        include_types: Option<HashSet<String>>,
+        exclude_types: Option<HashSet<String>>,
+        scope_path: Option<String>,
    ) -> Self {
        Self {
            fetcher,
-            seen: Arc::new(Mutex::new(HashSet::new())),
+            seen: HashSet::new(),
            output_dir,
            log_path,
            base_domain,
            robots,
-            doc_stats: Arc::new(Mutex::new(DocStats::default())),
+            doc_stats: DocStats::default(),
            convert_docs,
+            include_types,
+            exclude_types,
+            scope_path,
        }
    }

-    pub async fn run(&self, start_url: &Url) -> (usize, usize) {
+    fn should_save(&self, url: &Url, content_type: &str) -> (bool, String) {
+        let ext = get_extension(url, content_type);
+        let ext_clean = ext.trim_start_matches('.').to_lowercase();
+
+        if let Some(include) = &self.include_types {
+            if include.contains(&ext_clean) {
+                (true, ext_clean)
+            } else {
+                (false, ext_clean)
+            }
+        } else if let Some(exclude) = &self.exclude_types {
+            if exclude.contains(&ext_clean) {
+                (false, ext_clean)
+            } else {
+                (true, ext_clean)
+            }
+        } else {
+            (true, ext_clean)
+        }
+    }
+
+    pub async fn run(&mut self, start_url: &Url) -> (usize, usize) {
        let mut queue: VecDeque<Url> = VecDeque::new();
-        queue.push_back(start_url.clone());
+        let start_url_owned = start_url.clone();
+        queue.push_back(start_url_owned);

        let mut count = 0usize;
        let mut error_count = 0usize;
        let mut skipped_robots = 0usize;
+        let mut skipped_scope = 0usize;
+        let mut skipped_type = 0usize;

        while let Some(raw_url) = queue.pop_front() {
            let url = match normalize_url(raw_url.as_str()) {
@@ -77,12 +110,15 @@ impl Scraper {

            let url_key = url.as_str().to_string();

-            {
-                let mut seen = self.seen.lock().await;
-                if seen.contains(&url_key) {
+            if self.seen.contains(&url_key) {
                continue;
            }
-                seen.insert(url_key.clone());
+            self.seen.insert(url_key.clone());
+
+            if !is_in_scope(&url, &self.scope_path) {
+                log::debug!("[skip] out of scope: {}", url);
+                skipped_scope += 1;
+                continue;
            }

            if !self.robots.is_allowed(url.path()) {
@@ -98,32 +134,18 @@ impl Scraper {
                    let final_url = result.final_url.clone();
                    let is_html = result.content_type.contains("text/html");

+                    let (should_save, ext_label) =
+                        self.should_save(&result.final_url, &result.content_type);
+
+                    if !should_save {
+                        log::info!("  skipped (type filter: {})", ext_label);
+                        skipped_type += 1;
+                    }
+
+                    if should_save {
                        match self.save(&result, is_html).await {
                            Ok(()) => {
-                            if is_html {
-                                if let Ok(html) = std::str::from_utf8(&result.bytes) {
-                                    let links = extract_links(html, &final_url, &self.base_domain);
-                                    let new_count = links.len();
-
-                                    let new_links: Vec<Url> = {
-                                        let seen = self.seen.lock().await;
-                                        links
-                                            .into_iter()
-                                            .filter(|link| !seen.contains(link.as_str()))
-                                            .collect()
-                                    };
-
-                                    for link in &new_links {
-                                        queue.push_back(link.clone());
-                                    }
-
-                                    log::info!(
-                                        "  found {} links ({} new)",
-                                        new_count,
-                                        new_links.len()
-                                    );
-                                }
-                            } else {
+                                if !is_html {
                                    log::info!("  binary: {}", result.content_type);
                                }
                            }
@@ -136,8 +158,26 @@ impl Scraper {
                                    None,
                                );
                                error_count += 1;
-                            let mut stats = self.doc_stats.lock().await;
-                            stats.errors += 1;
+                                self.doc_stats.errors += 1;
+                            }
+                        }
+                    }
+
+                    if is_html {
+                        if let Ok(html) = std::str::from_utf8(&result.bytes) {
+                            let links = extract_links(html, &final_url, &self.base_domain);
+                            let new_count = links.len();
+
+                            let new_links: Vec<Url> = links
+                                .into_iter()
+                                .filter(|link| !self.seen.contains(link.as_str()))
+                                .collect();
+
+                            for link in &new_links {
+                                queue.push_back(link.clone());
+                            }
+
+                            log::info!("  found {} links ({} new)", new_count, new_links.len());
                        }
                    }
                }
@@ -151,32 +191,35 @@ impl Scraper {
            tokio::time::sleep(Duration::from_millis(crate::config::RATE_LIMIT_MS)).await;
        }

-        {
-            let stats = self.doc_stats.lock().await;
-            if stats.converted > 0 || stats.raw > 0 || stats.errors > 0 {
+        if self.doc_stats.converted > 0 || self.doc_stats.raw > 0 || self.doc_stats.errors > 0 {
            log::info!("\nDocument Statistics:");
-                log::info!("  Converted to Markdown: {}", stats.converted);
-                log::info!("  Kept as-is:            {}", stats.raw);
-                log::info!("  Errors:                {}", stats.errors);
-            }
+            log::info!("  Converted to Markdown: {}", self.doc_stats.converted);
+            log::info!("  Kept as-is:            {}", self.doc_stats.raw);
+            log::info!("  Errors:                {}", self.doc_stats.errors);
        }

        if skipped_robots > 0 {
            log::info!("Skipped {} URLs due to robots.txt", skipped_robots);
        }
+        if skipped_scope > 0 {
+            log::info!("Skipped {} URLs due to path scope", skipped_scope);
+        }
+        if skipped_type > 0 {
+            log::info!("Skipped {} URLs due to type filter", skipped_type);
+        }

        (count, error_count)
    }

-    async fn save(&self, result: &FetchResult, is_html: bool) -> Result<()> {
+    async fn save(&mut self, result: &FetchResult, is_html: bool) -> Result<()> {
        let mut filename = url_to_filename(&result.final_url, &result.content_type);
        let content: Vec<u8>;

        if is_html {
            let html = std::str::from_utf8(&result.bytes)?;
            let md = html_to_markdown(html);
-            if filename.ends_with(".html") {
-                filename = filename.replace(".html", ".md");
+            if let Some(stripped) = filename.strip_suffix(".html") {
+                filename = format!("{}.md", stripped);
            }
            content = md.into_bytes();
        } else if self.convert_docs {
@@ -188,19 +231,17 @@ impl Scraper {
                    filename.push_str(".md");
                    content = md.into_bytes();
                    log::info!("  [DOC] converted to Markdown");
-                    let mut stats = self.doc_stats.lock().await;
-                    stats.converted += 1;
+                    self.doc_stats.converted += 1;
                }
                DocProcessResult::Raw => {
-                    content = result.bytes.clone();
+                    content = result.bytes.to_vec();
                    if is_document_content_type(&result.content_type) {
-                        let mut stats = self.doc_stats.lock().await;
-                        stats.raw += 1;
+                        self.doc_stats.raw += 1;
                    }
                }
            }
        } else {
-            content = result.bytes.clone();
+            content = result.bytes.to_vec();
        }

        let filepath = self.output_dir.join(&filename);
@@ -236,3 +277,91 @@ fn is_document_content_type(ct: &str) -> bool {
    ];
    DOC_TYPES.contains(&ct.as_str())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use reqwest::Client;
+    use url::Url;
+
+    fn make_scraper(
+        include_types: Option<HashSet<String>>,
+        exclude_types: Option<HashSet<String>>,
+    ) -> Scraper {
+        Scraper {
+            fetcher: Fetcher::new(Client::new()).unwrap(),
+            seen: HashSet::new(),
+            output_dir: std::path::PathBuf::new(),
+            log_path: std::path::PathBuf::new(),
+            base_domain: "example.com".to_string(),
+            robots: RobotsRule {
+                allowed: Vec::new(),
+                disallowed: Vec::new(),
+            },
+            doc_stats: DocStats::default(),
+            convert_docs: true,
+            include_types,
+            exclude_types,
+            scope_path: None,
+        }
+    }
+
+    #[test]
+    fn test_should_save_include_only_matching() {
+        let mut include = HashSet::new();
+        include.insert("pdf".to_string());
+        let scraper = make_scraper(Some(include), None);
+        let url = Url::parse("https://example.com/doc.pdf").unwrap();
+        let (should_save, ext) = scraper.should_save(&url, "application/pdf");
+        assert!(should_save);
+        assert_eq!(ext, "pdf");
+    }
+
+    #[test]
+    fn test_should_save_include_non_matching() {
+        let mut include = HashSet::new();
+        include.insert("pdf".to_string());
+        let scraper = make_scraper(Some(include), None);
+        let url = Url::parse("https://example.com/doc.docx").unwrap();
+        let (should_save, ext) = scraper.should_save(
+            &url,
+            "application/vnd.openxmlformats-officedocument.wordprocessingml.document",
+        );
+        assert!(!should_save);
+        assert_eq!(ext, "docx");
+    }
+
+    #[test]
+    fn test_should_save_exclude_matching() {
+        let mut exclude = HashSet::new();
+        exclude.insert("pdf".to_string());
+        let scraper = make_scraper(None, Some(exclude));
+        let url = Url::parse("https://example.com/doc.pdf").unwrap();
+        let (should_save, ext) = scraper.should_save(&url, "application/pdf");
+        assert!(!should_save);
+        assert_eq!(ext, "pdf");
+    }
+
+    #[test]
+    fn test_should_save_exclude_non_matching() {
+        let mut exclude = HashSet::new();
+        exclude.insert("pdf".to_string());
+        let scraper = make_scraper(None, Some(exclude));
+        let url = Url::parse("https://example.com/doc.docx").unwrap();
+        let (should_save, ext) = scraper.should_save(
+            &url,
+            "application/vnd.openxmlformats-officedocument.wordprocessingml.document",
+        );
+        assert!(should_save);
+        assert_eq!(ext, "docx");
+    }
+
+    #[test]
+    fn test_should_save_no_filters_allows_all() {
+        let scraper = make_scraper(None, None);
+        let url = Url::parse("https://example.com/doc.pdf").unwrap();
+        let (should_save, ext) = scraper.should_save(&url, "application/pdf");
+        assert!(should_save);
+        assert_eq!(ext, "pdf");
+    }
+}
@@ -1,3 +1,6 @@
+// src/url_utils.rs
+#![warn(clippy::all, clippy::pedantic)]
+
 use md5::Md5;
 use sha2::{Digest, Sha256};
 use url::Url;
@@ -32,7 +35,25 @@ pub fn normalize_url(url: &str) -> Option<Url> {
    Some(parsed)
 }

-fn get_extension(url: &Url, content_type: &str) -> String {
+/// Returns true if `url`'s path falls within the given scope.
+/// A scope of `/blog/2021` matches `/blog/2021` and `/blog/2021/...`
+/// but not `/blog/20212`.
+/// `None` scope means no restriction.
+pub fn is_in_scope(url: &Url, scope_path: &Option<String>) -> bool {
+    match scope_path {
+        None => true,
+        Some(scope) => {
+            let scope = scope.trim_end_matches('/');
+            if scope.is_empty() {
+                return true;
+            }
+            let path = url.path();
+            path == scope || path.starts_with(&format!("{}/", scope))
+        }
+    }
+}
+
+pub fn get_extension(url: &Url, content_type: &str) -> String {
    let path = url.path();
    let last_segment = path.rsplit('/').next().unwrap_or(path);

@@ -122,7 +143,7 @@ pub fn url_to_filename(url: &Url, content_type: &str) -> String {
            let mut hasher = Md5::new();
            hasher.update(q.as_bytes());
            let hash = hex::encode(hasher.finalize());
-            format!("_{}", &hash[..8])
+            format!("_{}", &hash[..12])
        }
        _ => String::new(),
    };
@@ -147,116 +168,7 @@ pub fn url_to_filename(url: &Url, content_type: &str) -> String {
        host,
        safe_stem,
        query_suffix,
-        &url_hash[..8],
+        &url_hash[..12],
        ext
    )
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn test_derive_base_domain() {
-        let url = Url::parse("https://www.logting.fo/").unwrap();
-        assert_eq!(derive_base_domain(&url), Some("logting.fo".into()));
-
-        let url = Url::parse("https://taks.fo/en/skatur/").unwrap();
-        assert_eq!(derive_base_domain(&url), Some("taks.fo".into()));
-
-        let url = Url::parse("https://localhost:8080/").unwrap();
-        assert_eq!(derive_base_domain(&url), Some("localhost".into()));
-    }
-
-    #[test]
-    fn test_is_internal_subdomain() {
-        let base = "logting.fo";
-        assert!(is_internal(
-            &Url::parse("https://logting.fo/").unwrap(),
-            base
-        ));
-        assert!(is_internal(
-            &Url::parse("https://www.logting.fo/").unwrap(),
-            base
-        ));
-        assert!(is_internal(
-            &Url::parse("https://sub.www.logting.fo/").unwrap(),
-            base
-        ));
-        assert!(!is_internal(
-            &Url::parse("https://evilogting.fo/").unwrap(),
-            base
-        ));
-        assert!(!is_internal(
-            &Url::parse("https://logting.com/").unwrap(),
-            base
-        ));
-    }
-
-    #[test]
-    fn test_normalize_url_strips_fragment() {
-        let url = normalize_url("https://example.com/page#section").unwrap();
-        assert_eq!(url.as_str(), "https://example.com/page");
-    }
-
-    #[test]
-    fn test_normalize_url_strips_trailing_query_chars() {
-        let url = normalize_url("https://example.com/page?&").unwrap();
-        assert_eq!(url.as_str(), "https://example.com/page");
-    }
-
-    #[test]
-    fn test_url_to_filename_faroese_unicode() {
-        let url = Url::parse("https://logting.fo/lov/tinglýsing/2024/").unwrap();
-        let filename = url_to_filename(&url, "text/html");
-        assert!(filename.starts_with("logting_fo_"));
-        assert!(filename.ends_with(".html") || filename.ends_with(".md"));
-        assert!(!filename.contains('ý'));
-    }
-
-    #[test]
-    fn test_url_to_filename_long_url_hashed() {
-        let long = format!("https://example.com/{}", "a".repeat(200));
-        let url = Url::parse(&long).unwrap();
-        let filename = url_to_filename(&url, "text/html");
-        assert!(filename.len() < 30);
-        assert!(filename.ends_with(".html"));
-    }
-
-    #[test]
-    fn test_url_to_filename_with_query() {
-        let url = Url::parse("https://example.com/page?id=42&sort=desc").unwrap();
-        let filename = url_to_filename(&url, "text/html");
-        assert!(filename.contains('_'));
-        assert!(filename.ends_with(".html"));
-    }
-
-    #[test]
-    fn test_get_extension_from_content_type() {
-        let url = Url::parse("https://example.com/").unwrap();
-        assert_eq!(get_extension(&url, "text/html"), ".html");
-        assert_eq!(
-            get_extension(&url, "application/pdf; charset=binary"),
-            ".pdf"
-        );
-        assert_eq!(get_extension(&url, "application/octet-stream"), ".bin");
-    }
-
-    #[test]
-    fn test_get_extension_from_url_path() {
-        let url = Url::parse("https://example.com/doc.pdf").unwrap();
-        assert_eq!(get_extension(&url, "application/octet-stream"), ".pdf");
-    }
-
-    #[test]
-    fn test_url_collision_avoidance() {
-        let url1 = Url::parse("https://example.com/foo-bar").unwrap();
-        let url2 = Url::parse("https://example.com/foo.bar").unwrap();
-        let fn1 = url_to_filename(&url1, "text/html");
-        let fn2 = url_to_filename(&url2, "text/html");
-        assert_ne!(
-            fn1, fn2,
-            "Different URLs should produce different filenames"
-        );
-    }
-}