diff --git a/CLAUDE.md b/CLAUDE.md index 9f60026..7eb4dd1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,11 +10,13 @@ The **Open Elements Content MCP** is an MCP server that will crawl, index, and m searchable the content (primarily blog posts) of Open-Elements-adjacent websites, exposing that content to AI agents over the standard `/mcp` streamable-HTTP endpoint. -Current state: **project skeleton** (spec `001-project-skeleton`). The app boots and exposes -`/mcp` with no content-specific tools yet. Planned capabilities (see -[`docs/roadmap.md`](docs/roadmap.md)): sitemap crawling, HTTP page fetching, jsoup content -extraction, Meilisearch indexing with scheduled refresh, and four MCP tools -(`search_content`, `list_posts`, `get_post`, `list_categories`). +Current state: **Phase 1 complete** (specs 001–014). The app crawls configured website sources +(sitemap discovery + bounded fallback, robots.txt-aware), fetches politely (conditional GET, per-host +rate limit, retries), extracts content/metadata with jsoup, indexes into Meilisearch (startup +bootstrap + scheduled incremental refresh), and exposes four MCP tools on `/mcp` (`search_content`, +`list_posts`, `get_post`, `list_categories`) backed by a scoped Meilisearch key. Phase 2+ (see +[`docs/roadmap.md`](docs/roadmap.md)): additional website sources, Git/Markdown sources, and search +enhancements. ### Tech Stack @@ -57,7 +59,8 @@ extraction, Meilisearch indexing with scheduled refresh, and four MCP tools │ ├── ContentBootstrapStep.java # SearchIndexBootstrapStep: startup full reindex │ ├── ContentRefreshScheduler.java # @Scheduled incremental re-crawl (cron, guarded) │ ├── ContentSearchService.java # read facade: multiSearch + Highlighter (+ result records) -│ └── ContentMcpToolProvider.java # McpToolProvider: the 4 tools on /mcp +│ ├── ContentMcpToolProvider.java # McpToolProvider: the 4 tools on /mcp +│ ├── RobotsPolicy.java / HttpRobotsPolicy.java # robots.txt allow/disallow + Crawl-delay ├── src/main/resources/application.yaml # datasource, JPA, OAuth2, MCP, Meilisearch, content config ├── src/test/java/com/openelements/content/ # behavior tests (context, MCP enabled/disabled, search-down, jsoup) └── docs/ @@ -127,7 +130,11 @@ as thin adapters over `ContentSearchService`, using the house helpers for schema `ContentConfig` registers a `ScopedKeySpec` bean so the library exchanges the master key for one scoped to the content index with the minimal actions the service uses (it both reads and writes); MCP api-key auth on `/mcp` defaults to **enabled** (`MCP_API_KEY_AUTH_ENABLED`), with the `dev` profile -(`application-dev.yaml`) disabling it for local use. +(`application-dev.yaml`) disabling it for local use. Robustness (spec 014): `HttpRobotsPolicy` honors +each host's `robots.txt` (cached per host) — `SitemapCrawler` drops disallowed URLs and `PageFetcher` +respects `Crawl-delay` via the per-host rate limiter. The pipeline is fault-tolerant end-to-end (one +failing page/source never aborts a run) and logs an `IndexReport` per source; Micrometer metrics are +deferred until a metrics backend is wired. > **Build note:** `spring-services` is pinned to a specific snapshot timestamp > (`1.3.0-20260712.175350-13`) in `pom.xml`, not the floating `1.3.0-SNAPSHOT`, because a later diff --git a/docs/specs/014-ops-robustness/steps.md b/docs/specs/014-ops-robustness/steps.md new file mode 100644 index 0000000..52027a2 --- /dev/null +++ b/docs/specs/014-ops-robustness/steps.md @@ -0,0 +1,59 @@ +# Implementation Steps: Operations & Robustness + +## Step 1: robots.txt + +- [x] `RobotsPolicy` interface (`isAllowed`, `crawlDelay`) with an `ALLOW_ALL` no-op for tests +- [x] `HttpRobotsPolicy` (`@Component`): fetches `robots.txt` per host over HTTPS, caches with a TTL (injectable clock), selects the group for our user agent (falling back to `*`), longest-prefix matching with `Allow` winning ties, missing robots → allow-all, parses `Crawl-delay` +- [x] `SitemapCrawler` filters discovered URLs (sitemap + fallback crawl) through `RobotsPolicy`, logging skips +- [x] `PageFetcher` honors `Crawl-delay` via `HostRateLimiter.acquire(host, minInterval)` (stricter of config vs. crawl-delay) +- [x] Hardening: `SitemapCrawler` only follows child sitemaps on the same host (a sitemap index cannot point the crawler at arbitrary/internal hosts) — closes the SSRF surface flagged in earlier specs + +**Related behaviors:** disallowed skipped; allowed fetched; cached per host; missing → allow-all; crawl-delay tightens the rate limit + +--- + +## Step 2: Fault tolerance (audit) + +- [x] Verified end-to-end (covered by existing tests): one failing page → `SKIP` (`WebsiteSourceStrategyTest`, `ContentIndexerTest.oneFailingPageIsSkipped`); one failing source isolated (`ContentBootstrapStepTest.failingSourceDoesNotBlockOthers`, `ContentRefreshSchedulerTest.oneSourceFailingDoesNotStopOthers`); idempotent upserts (`ContentIndexerTest`) + +**Related behaviors:** failing page isolated; failing source isolated + +--- + +## Step 3: Observability (logging; metrics deferred) + +- [x] Structured logging: `IndexReport` per source (`ContentIndexer`/`ContentRefreshScheduler`); robots-skip (`SitemapCrawler`/`HttpRobotsPolicy`); non-success Meilisearch task outcomes logged (`MeilisearchContentIndexStore`) +- [ ] Micrometer metrics — **deferred** per the design's open question: no metrics backend is wired (only `micrometer-observation`/`-commons` are on the classpath, not `micrometer-core`/a `MeterRegistry`). Structured logging is shipped now; add counters/timers when a metrics backend exists. + +**Related behaviors:** IndexReport logged per source; failed Meilisearch task surfaced (logging half); metrics behaviors deferred + +--- + +## Step 4: Tests + +- [x] `HttpRobotsPolicyTest` (allow/disallow/allow-override/agent-group/cache/missing/crawl-delay via `MockRestServiceServer`) +- [x] `HostRateLimiterTest.crawlDelayTightensInterval` +- [x] `SitemapCrawlerTest.robotsDisallowedUrlsAreDropped` +- [x] Existing crawler/fetcher/strategy tests updated to `RobotsPolicy.ALLOW_ALL` + +**Acceptance criteria:** +- [x] All tests pass (`mvn test`); build green + +--- + +## Behavior Coverage + +| Scenario | Layer | Covered in Step | +|----------|-------|-----------------| +| Disallowed path is skipped | Backend | Step 4 (`HttpRobotsPolicyTest.disallowedPathIsRejected`, `SitemapCrawlerTest.robotsDisallowedUrlsAreDropped`) | +| Allowed path is fetched | Backend | Step 4 (`HttpRobotsPolicyTest.disallowedPathIsRejected` asserts allowed too) | +| robots.txt is cached per host | Backend | Step 4 (`HttpRobotsPolicyTest.robotsIsCachedPerHost`) | +| Missing robots.txt means allow-all | Backend | Step 4 (`HttpRobotsPolicyTest.missingRobotsAllowsAll`) | +| Crawl-delay tightens the rate limit | Backend | Step 4 (`HttpRobotsPolicyTest.crawlDelayIsParsed` + `HostRateLimiterTest.crawlDelayTightensInterval`) | +| Failing page is isolated | Backend | Step 2 (existing `WebsiteSourceStrategyTest`/`ContentIndexerTest`) | +| Failing source is isolated | Backend | Step 2 (existing `ContentBootstrapStepTest`/`ContentRefreshSchedulerTest`) | +| IndexReport is logged per source | Backend | Step 3 (structured logging in `ContentIndexer`/`ContentRefreshScheduler`) | +| Metrics record fetch and index activity | Backend | **Deferred** (Step 3) — no metrics backend wired; design open question | +| Failed Meilisearch task is surfaced | Backend | Step 3 (logged in `MeilisearchContentIndexStore`; metric part deferred) | + +All scenarios are backend; there is no frontend in this spec. diff --git a/docs/specs/INDEX.md b/docs/specs/INDEX.md index 114814f..453ada1 100644 --- a/docs/specs/INDEX.md +++ b/docs/specs/INDEX.md @@ -19,7 +19,7 @@ roadmap step, to be implemented sequentially (each builds on the previous). | 011 | 011-content-search-service | Content search service | backend, search | `ContentSearchService` — `multiSearch` + highlighting facade | #21 | done | | 012 | 012-content-mcp-tools | Content MCP tools | backend, mcp, api | `ContentMcpToolProvider` — the 4 MCP tools | #23 | done | | 013 | 013-scoped-search-key | Scoped search key | backend, search, security | Read-only scoped Meilisearch key for the content index | #25 | done | -| 014 | 014-ops-robustness | Ops & robustness | backend, infrastructure, observability | robots.txt handling, fault tolerance, logging/metrics | — | open | +| 014 | 014-ops-robustness | Ops & robustness | backend, infrastructure, observability | robots.txt handling, fault tolerance, logging/metrics | #27 | done | | 015 | 015-additional-web-sources | Additional web sources | backend, configuration | hiero.org/blog + Support & Care as config-only sources | — | open | | 016 | 016-git-markdown-source | Git markdown source | backend, architecture, security | `type: git` source + `GitSourceStrategy` (GitHub Markdown) | — | open | | 017 | 017-search-enhancements | Search enhancements | backend, search | Facets, synonyms/stop words, optional semantic search | — | open | diff --git a/src/main/java/com/openelements/content/HostRateLimiter.java b/src/main/java/com/openelements/content/HostRateLimiter.java index 30df71f..991c048 100644 --- a/src/main/java/com/openelements/content/HostRateLimiter.java +++ b/src/main/java/com/openelements/content/HostRateLimiter.java @@ -1,5 +1,6 @@ package com.openelements.content; +import java.time.Duration; import java.util.HashMap; import java.util.Map; import java.util.function.LongConsumer; @@ -38,7 +39,20 @@ public HostRateLimiter(double permitsPerSecond, LongSupplier clockNanos, LongCon * @param host the target host (an empty or {@code null} host shares a single bucket) */ public void acquire(String host) { - if (minIntervalNanos == 0L) { + acquire(host, Duration.ZERO); + } + + /** + * Like {@link #acquire(String)}, but enforces at least {@code minInterval} between requests to the + * host — used to honor a {@code robots.txt} {@code Crawl-delay}, taking the stricter of the + * configured rate and the crawl delay. + * + * @param host the target host + * @param minInterval a minimum interval to enforce (e.g. from {@code Crawl-delay}); may be zero + */ + public void acquire(String host, Duration minInterval) { + long effectiveIntervalNanos = Math.max(minIntervalNanos, minInterval == null ? 0L : minInterval.toNanos()); + if (effectiveIntervalNanos == 0L) { return; } String key = host == null ? "" : host; @@ -47,7 +61,7 @@ public void acquire(String host) { long now = clockNanos.getAsLong(); long start = Math.max(now, nextAllowedNanos.getOrDefault(key, now)); waitNanos = start - now; - nextAllowedNanos.put(key, start + minIntervalNanos); + nextAllowedNanos.put(key, start + effectiveIntervalNanos); } if (waitNanos > 0) { sleeperMillis.accept(Math.max(1, Math.round(waitNanos / 1_000_000.0))); diff --git a/src/main/java/com/openelements/content/HttpRobotsPolicy.java b/src/main/java/com/openelements/content/HttpRobotsPolicy.java new file mode 100644 index 0000000..bd7d6c5 --- /dev/null +++ b/src/main/java/com/openelements/content/HttpRobotsPolicy.java @@ -0,0 +1,209 @@ +package com.openelements.content; + +import java.net.URI; +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.LongSupplier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; +import org.springframework.web.client.RestClient; + +/** + * {@link RobotsPolicy} that fetches and caches each host's {@code robots.txt}. + * + *

Rules are fetched once per host (over HTTPS) and cached with a TTL. The group matching our bot + * {@code User-Agent} is used, falling back to the {@code *} group; a missing or unreachable + * {@code robots.txt} allows everything. Path matching is longest-prefix with {@code Allow} winning + * ties (the common robots semantics). The clock is injectable so cache expiry is testable. + */ +@Component +public class HttpRobotsPolicy implements RobotsPolicy { + + private static final Logger log = LoggerFactory.getLogger(HttpRobotsPolicy.class); + private static final Duration CACHE_TTL = Duration.ofHours(1); + + private final RestClient restClient; + private final String userAgent; + private final LongSupplier clockNanos; + private final Map cache = new ConcurrentHashMap<>(); + + @Autowired + public HttpRobotsPolicy(RestClient.Builder restClientBuilder, ContentSourceProperties properties) { + this(restClientBuilder, properties, System::nanoTime); + } + + HttpRobotsPolicy(RestClient.Builder restClientBuilder, ContentSourceProperties properties, LongSupplier clockNanos) { + this.restClient = restClientBuilder.build(); + this.userAgent = properties.userAgent(); + this.clockNanos = clockNanos; + } + + @Override + public boolean isAllowed(String url) { + String host = hostOf(url); + if (host.isEmpty()) { + return true; + } + boolean allowed = rulesFor(host).allows(pathOf(url)); + if (!allowed) { + log.info("robots.txt disallows {} for {}", url, userAgent); + } + return allowed; + } + + @Override + public Duration crawlDelay(String host) { + return host == null || host.isEmpty() ? Duration.ZERO : rulesFor(host).crawlDelay(); + } + + private RobotsRules rulesFor(String host) { + CachedRules cached = cache.get(host); + if (cached != null && cached.expiryNanos() > clockNanos.getAsLong()) { + return cached.rules(); + } + RobotsRules rules = fetchAndParse(host); + cache.put(host, new CachedRules(rules, clockNanos.getAsLong() + CACHE_TTL.toNanos())); + return rules; + } + + private RobotsRules fetchAndParse(String host) { + try { + String body = restClient.get() + .uri(URI.create("https://" + host + "/robots.txt")) + .header("User-Agent", userAgent) + .retrieve() + .body(String.class); + return parse(body == null ? "" : body, userAgent); + } catch (Exception e) { + // Missing/unreachable robots.txt (e.g. 404) means allow everything. + log.debug("No usable robots.txt for {} ({}); allowing all", host, e.toString()); + return RobotsRules.ALLOW_ALL; + } + } + + /** Parses robots.txt, selecting the group for our user agent (falling back to {@code *}). */ + static RobotsRules parse(String body, String userAgent) { + List groups = new ArrayList<>(); + Group current = null; + boolean lastWasRule = false; + for (String rawLine : body.split("\n")) { + String line = stripComment(rawLine).trim(); + if (line.isEmpty()) { + continue; + } + int colon = line.indexOf(':'); + if (colon < 0) { + continue; + } + String field = line.substring(0, colon).trim().toLowerCase(Locale.ROOT); + String value = line.substring(colon + 1).trim(); + if (field.equals("user-agent")) { + if (current == null || lastWasRule) { + current = new Group(); + groups.add(current); + } + current.agents.add(value.toLowerCase(Locale.ROOT)); + lastWasRule = false; + } else if (current != null) { + switch (field) { + case "disallow" -> current.disallow.add(value); + case "allow" -> current.allow.add(value); + case "crawl-delay" -> current.crawlDelay = parseDelay(value); + default -> { /* ignore other directives */ } + } + lastWasRule = true; + } + } + return selectGroup(groups, userAgent).toRules(); + } + + private static Group selectGroup(List groups, String userAgent) { + String ua = userAgent.toLowerCase(Locale.ROOT); + Group wildcard = null; + for (Group group : groups) { + for (String agent : group.agents) { + if (agent.equals("*")) { + wildcard = group; + } else if (!agent.isEmpty() && ua.contains(agent)) { + return group; // a group naming our agent wins over the wildcard + } + } + } + return wildcard == null ? new Group() : wildcard; + } + + private static Duration parseDelay(String value) { + try { + return Duration.ofMillis(Math.round(Double.parseDouble(value) * 1000)); + } catch (NumberFormatException e) { + return Duration.ZERO; + } + } + + private static String stripComment(String line) { + int hash = line.indexOf('#'); + return hash >= 0 ? line.substring(0, hash) : line; + } + + private static String hostOf(String url) { + try { + String host = URI.create(url).getHost(); + return host == null ? "" : host; + } catch (IllegalArgumentException e) { + return ""; + } + } + + private static String pathOf(String url) { + try { + String path = URI.create(url).getPath(); + return path == null || path.isEmpty() ? "/" : path; + } catch (IllegalArgumentException e) { + return "/"; + } + } + + private static final class Group { + private final List agents = new ArrayList<>(); + private final List disallow = new ArrayList<>(); + private final List allow = new ArrayList<>(); + private Duration crawlDelay = Duration.ZERO; + + private RobotsRules toRules() { + return new RobotsRules(List.copyOf(disallow), List.copyOf(allow), crawlDelay); + } + } + + private record CachedRules(RobotsRules rules, long expiryNanos) { + } + + /** Parsed robots rules for the applicable group. */ + record RobotsRules(List disallow, List allow, Duration crawlDelay) { + + static final RobotsRules ALLOW_ALL = new RobotsRules(List.of(), List.of(), Duration.ZERO); + + boolean allows(String path) { + int longestDisallow = longestMatch(disallow, path); + if (longestDisallow < 0) { + return true; + } + return longestMatch(allow, path) >= longestDisallow; // Allow wins ties + } + + private static int longestMatch(List rules, String path) { + int longest = -1; + for (String rule : rules) { + if (!rule.isEmpty() && path.startsWith(rule) && rule.length() > longest) { + longest = rule.length(); + } + } + return longest; + } + } +} diff --git a/src/main/java/com/openelements/content/PageFetcher.java b/src/main/java/com/openelements/content/PageFetcher.java index 40c2517..7424473 100644 --- a/src/main/java/com/openelements/content/PageFetcher.java +++ b/src/main/java/com/openelements/content/PageFetcher.java @@ -36,20 +36,22 @@ public class PageFetcher { private final String userAgent; private final long maxBodyBytes; private final HostRateLimiter rateLimiter; + private final RobotsPolicy robotsPolicy; private final LongConsumer backoffSleeper; @Autowired public PageFetcher(RestClient contentRestClient, ContentSourceProperties properties, - HostRateLimiter rateLimiter) { - this(contentRestClient, properties, rateLimiter, PageFetcher::sleep); + HostRateLimiter rateLimiter, RobotsPolicy robotsPolicy) { + this(contentRestClient, properties, rateLimiter, robotsPolicy, PageFetcher::sleep); } PageFetcher(RestClient restClient, ContentSourceProperties properties, - HostRateLimiter rateLimiter, LongConsumer backoffSleeper) { + HostRateLimiter rateLimiter, RobotsPolicy robotsPolicy, LongConsumer backoffSleeper) { this.restClient = restClient; this.userAgent = properties.userAgent(); this.maxBodyBytes = properties.maxBodyBytes().toBytes(); this.rateLimiter = rateLimiter; + this.robotsPolicy = robotsPolicy; this.backoffSleeper = backoffSleeper; } @@ -83,7 +85,9 @@ public FetchResult fetch(String url, String priorEtag, String priorLastmod) { } private FetchResult attemptOnce(String url, String priorEtag, String priorLastmod) { - rateLimiter.acquire(hostOf(url)); + String host = hostOf(url); + // Honor robots.txt Crawl-delay for this host, taking the stricter of it and the configured rate. + rateLimiter.acquire(host, robotsPolicy.crawlDelay(host)); return restClient.get() .uri(URI.create(url)) .header(HttpHeaders.USER_AGENT, userAgent) diff --git a/src/main/java/com/openelements/content/RobotsPolicy.java b/src/main/java/com/openelements/content/RobotsPolicy.java new file mode 100644 index 0000000..adefdd8 --- /dev/null +++ b/src/main/java/com/openelements/content/RobotsPolicy.java @@ -0,0 +1,39 @@ +package com.openelements.content; + +import java.time.Duration; + +/** + * Decides whether the crawler may fetch a URL under the target host's {@code robots.txt}, and exposes + * any {@code Crawl-delay} that host requests. + * + *

Modeled as an interface so the crawling components can be tested with the {@link #ALLOW_ALL} + * no-op policy, while production uses {@link HttpRobotsPolicy}. + */ +public interface RobotsPolicy { + + /** + * @param url an absolute URL + * @return {@code true} if {@code robots.txt} permits fetching it for our user agent (a missing or + * unreachable {@code robots.txt} allows everything) + */ + boolean isAllowed(String url); + + /** + * @param host the target host + * @return the {@code Crawl-delay} the host requests, or {@link Duration#ZERO} if none + */ + Duration crawlDelay(String host); + + /** A permissive policy that allows every URL and requests no crawl delay. */ + RobotsPolicy ALLOW_ALL = new RobotsPolicy() { + @Override + public boolean isAllowed(String url) { + return true; + } + + @Override + public Duration crawlDelay(String host) { + return Duration.ZERO; + } + }; +} diff --git a/src/main/java/com/openelements/content/SitemapCrawler.java b/src/main/java/com/openelements/content/SitemapCrawler.java index 4fb4c15..fe58430 100644 --- a/src/main/java/com/openelements/content/SitemapCrawler.java +++ b/src/main/java/com/openelements/content/SitemapCrawler.java @@ -48,10 +48,12 @@ public class SitemapCrawler { private final RestClient restClient; private final UrlMatcher urlMatcher; + private final RobotsPolicy robotsPolicy; - public SitemapCrawler(RestClient.Builder restClientBuilder, UrlMatcher urlMatcher) { + public SitemapCrawler(RestClient.Builder restClientBuilder, UrlMatcher urlMatcher, RobotsPolicy robotsPolicy) { this.restClient = restClientBuilder.build(); this.urlMatcher = urlMatcher; + this.robotsPolicy = robotsPolicy; } /** @@ -74,9 +76,18 @@ public List discover(ContentSource source) { return collected.values().stream() .filter(item -> urlMatcher.matches(source, item.url())) + .filter(this::allowedByRobots) .toList(); } + private boolean allowedByRobots(DiscoveredItem item) { + if (robotsPolicy.isAllowed(item.url())) { + return true; + } + log.info("Skipping {} — disallowed by robots.txt", item.url()); + return false; + } + private void collectSitemap(String sitemapUrl, Map collected, Set visitedSitemaps, int depth) { if (depth > MAX_SITEMAP_DEPTH || !visitedSitemaps.add(sitemapUrl)) { @@ -95,9 +106,16 @@ private void collectSitemap(String sitemapUrl, Map colle if (!childSitemaps.isEmpty()) { for (Element loc : childSitemaps) { String childUrl = loc.text().trim(); - if (!childUrl.isEmpty()) { - collectSitemap(childUrl, collected, visitedSitemaps, depth + 1); + if (childUrl.isEmpty()) { + continue; + } + // Only follow child sitemaps on the same host, so a sitemap index cannot point the + // crawler at arbitrary (e.g. internal) hosts. + if (!hostOf(childUrl).equalsIgnoreCase(hostOf(sitemapUrl))) { + log.warn("Skipping off-host child sitemap {} (parent {})", childUrl, sitemapUrl); + continue; } + collectSitemap(childUrl, collected, visitedSitemaps, depth + 1); } return; } @@ -142,7 +160,7 @@ private List fallbackCrawl(ContentSource source) { if (link.isEmpty() || !host.equalsIgnoreCase(hostOf(link))) { continue; } - if (urlMatcher.matches(source, link)) { + if (urlMatcher.matches(source, link) && allowedByRobots(new DiscoveredItem(link, null))) { results.putIfAbsent(link, new DiscoveredItem(link, null)); } if (entry.depth() < MAX_FALLBACK_DEPTH && !visited.contains(link)) { diff --git a/src/test/java/com/openelements/content/HostRateLimiterTest.java b/src/test/java/com/openelements/content/HostRateLimiterTest.java index f6066f3..f297570 100644 --- a/src/test/java/com/openelements/content/HostRateLimiterTest.java +++ b/src/test/java/com/openelements/content/HostRateLimiterTest.java @@ -2,6 +2,7 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicLong; @@ -58,4 +59,16 @@ void nonPositiveRateDisablesThrottling() { assertThat(sleeps).isEmpty(); } + + @Test + @DisplayName("a crawl-delay tightens the effective interval beyond the configured rate") + void crawlDelayTightensInterval() { + HostRateLimiter limiter = new HostRateLimiter(2.0, nowNanos::get, sleeper); // base 500 ms + + limiter.acquire("a", Duration.ofSeconds(5)); // first request: no wait + limiter.acquire("a", Duration.ofSeconds(5)); + + // 5 s crawl-delay is stricter than the 500 ms configured interval. + assertThat(sleeps).containsExactly(5000L); + } } diff --git a/src/test/java/com/openelements/content/HttpRobotsPolicyTest.java b/src/test/java/com/openelements/content/HttpRobotsPolicyTest.java new file mode 100644 index 0000000..c42f4d1 --- /dev/null +++ b/src/test/java/com/openelements/content/HttpRobotsPolicyTest.java @@ -0,0 +1,99 @@ +package com.openelements.content; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.client.ExpectedCount.once; +import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo; +import static org.springframework.test.web.client.response.MockRestResponseCreators.withStatus; +import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess; + +import java.time.Duration; +import java.util.List; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.test.web.client.MockRestServiceServer; +import org.springframework.web.client.RestClient; + +/** + * Tests for {@link HttpRobotsPolicy} using {@link MockRestServiceServer} to stub {@code robots.txt} + * (no real network). + */ +@DisplayName("HTTP robots policy") +class HttpRobotsPolicyTest { + + private static final String ROBOTS_URL = "https://ex.com/robots.txt"; + + private MockRestServiceServer server; + private HttpRobotsPolicy policy; + + @BeforeEach + void setUp() { + RestClient.Builder builder = RestClient.builder(); + server = MockRestServiceServer.bindTo(builder).ignoreExpectOrder(true).build(); + ContentSourceProperties properties = new ContentSourceProperties( + true, null, "OpenElementsContentBot/1.0", 2.0, Duration.ofSeconds(10), null, List.of()); + policy = new HttpRobotsPolicy(builder, properties); + } + + private void stubRobots(String body) { + server.expect(requestTo(ROBOTS_URL)).andRespond(withSuccess(body, MediaType.TEXT_PLAIN)); + } + + @Test + @DisplayName("a disallowed path is rejected and other paths allowed") + void disallowedPathIsRejected() { + stubRobots("User-agent: *\nDisallow: /private/\n"); + + assertThat(policy.isAllowed("https://ex.com/private/page")).isFalse(); + assertThat(policy.isAllowed("https://ex.com/posts/a")).isTrue(); + } + + @Test + @DisplayName("an allow rule overrides a broader disallow") + void allowOverridesDisallow() { + stubRobots("User-agent: *\nDisallow: /private/\nAllow: /private/public\n"); + + assertThat(policy.isAllowed("https://ex.com/private/public/page")).isTrue(); + assertThat(policy.isAllowed("https://ex.com/private/secret")).isFalse(); + } + + @Test + @DisplayName("a group naming our user agent takes precedence over the wildcard group") + void agentSpecificGroupWins() { + stubRobots("User-agent: *\nDisallow: /\n\nUser-agent: OpenElementsContentBot\nDisallow: /private/\n"); + + assertThat(policy.isAllowed("https://ex.com/posts/a")).isTrue(); // our group only blocks /private/ + assertThat(policy.isAllowed("https://ex.com/private/x")).isFalse(); + } + + @Test + @DisplayName("robots.txt is fetched once per host and cached") + void robotsIsCachedPerHost() { + server.expect(once(), requestTo(ROBOTS_URL)) + .andRespond(withSuccess("User-agent: *\nDisallow: /private/\n", MediaType.TEXT_PLAIN)); + + policy.isAllowed("https://ex.com/a"); + policy.isAllowed("https://ex.com/b"); + policy.isAllowed("https://ex.com/private/c"); + + server.verify(); // exactly one robots.txt request + } + + @Test + @DisplayName("a missing robots.txt allows everything") + void missingRobotsAllowsAll() { + server.expect(requestTo(ROBOTS_URL)).andRespond(withStatus(HttpStatus.NOT_FOUND)); + + assertThat(policy.isAllowed("https://ex.com/anything")).isTrue(); + } + + @Test + @DisplayName("Crawl-delay is parsed for the applicable group") + void crawlDelayIsParsed() { + stubRobots("User-agent: *\nCrawl-delay: 5\n"); + + assertThat(policy.crawlDelay("ex.com")).isEqualTo(Duration.ofSeconds(5)); + } +} diff --git a/src/test/java/com/openelements/content/PageFetcherTest.java b/src/test/java/com/openelements/content/PageFetcherTest.java index 59c23f5..62a9bca 100644 --- a/src/test/java/com/openelements/content/PageFetcherTest.java +++ b/src/test/java/com/openelements/content/PageFetcherTest.java @@ -48,7 +48,7 @@ private PageFetcher fetcher(long maxBytes) { ContentSourceProperties properties = new ContentSourceProperties( true, null, USER_AGENT, 2.0, Duration.ofSeconds(10), DataSize.ofBytes(maxBytes), List.of()); HostRateLimiter noThrottle = new HostRateLimiter(0, () -> 0L, millis -> { }); - return new PageFetcher(client, properties, noThrottle, backoffs::add); + return new PageFetcher(client, properties, noThrottle, RobotsPolicy.ALLOW_ALL, backoffs::add); } @Test diff --git a/src/test/java/com/openelements/content/SitemapCrawlerTest.java b/src/test/java/com/openelements/content/SitemapCrawlerTest.java index 8a3f4bd..1efeb6d 100644 --- a/src/test/java/com/openelements/content/SitemapCrawlerTest.java +++ b/src/test/java/com/openelements/content/SitemapCrawlerTest.java @@ -25,11 +25,13 @@ class SitemapCrawlerTest { private MockRestServiceServer server; private SitemapCrawler crawler; + private RestClient.Builder builder; + @BeforeEach void setUp() { - RestClient.Builder builder = RestClient.builder(); + builder = RestClient.builder(); server = MockRestServiceServer.bindTo(builder).ignoreExpectOrder(true).build(); - crawler = new SitemapCrawler(builder, new UrlMatcher()); + crawler = new SitemapCrawler(builder, new UrlMatcher(), RobotsPolicy.ALLOW_ALL); } private static ContentSource source(List sitemaps, List include, List exclude) { @@ -93,6 +95,21 @@ void sitemapIndexIsFollowed() { .containsExactlyInAnyOrder("https://ex.com/posts/a", "https://ex.com/posts/b"); } + @Test + @DisplayName("an off-host child sitemap in an index is not followed") + void offHostChildSitemapIsSkipped() { + stubXml("https://ex.com/sitemap.xml", + "" + + "https://ex.com/child.xml" + + "https://evil.internal/child.xml"); + stubXml("https://ex.com/child.xml", urlset(urlEntry("https://ex.com/posts/a", null))); + + List items = crawler.discover(source(List.of("/sitemap.xml"), List.of("/**"), List.of())); + + // The off-host child sitemap is never fetched (no stub for evil.internal) and contributes nothing. + assertThat(items).extracting(DiscoveredItem::url).containsExactly("https://ex.com/posts/a"); + } + @Test @DisplayName("a without yields a null change marker") void missingLastmodIsTolerated() { @@ -115,6 +132,30 @@ void onlyIncludedUrlsAreReturned() { assertThat(items).extracting(DiscoveredItem::url).containsExactly("https://ex.com/posts/a"); } + @Test + @DisplayName("URLs disallowed by robots.txt are dropped from discovery") + void robotsDisallowedUrlsAreDropped() { + stubXml("https://ex.com/sitemap.xml", urlset( + urlEntry("https://ex.com/posts/a", null), + urlEntry("https://ex.com/private/secret", null))); + RobotsPolicy denyPrivate = new RobotsPolicy() { + @Override + public boolean isAllowed(String url) { + return !url.contains("/private/"); + } + + @Override + public java.time.Duration crawlDelay(String host) { + return java.time.Duration.ZERO; + } + }; + SitemapCrawler robotsAwareCrawler = new SitemapCrawler(builder, new UrlMatcher(), denyPrivate); + + List items = robotsAwareCrawler.discover(source(List.of("/sitemap.xml"), List.of("/**"), List.of())); + + assertThat(items).extracting(DiscoveredItem::url).containsExactly("https://ex.com/posts/a"); + } + @Test @DisplayName("URLs matching url-exclude are dropped") void excludedUrlsAreDropped() { diff --git a/src/test/java/com/openelements/content/WebsiteSourceStrategyTest.java b/src/test/java/com/openelements/content/WebsiteSourceStrategyTest.java index 879efc6..d5eaa0e 100644 --- a/src/test/java/com/openelements/content/WebsiteSourceStrategyTest.java +++ b/src/test/java/com/openelements/content/WebsiteSourceStrategyTest.java @@ -39,12 +39,12 @@ void setUp() { RestClient.Builder fetchBuilder = RestClient.builder(); fetchServer = MockRestServiceServer.bindTo(fetchBuilder).ignoreExpectOrder(true).build(); - PageFetcher fetcher = new PageFetcher( - fetchBuilder.build(), properties, new HostRateLimiter(0, () -> 0L, millis -> { }), millis -> { }); + PageFetcher fetcher = new PageFetcher(fetchBuilder.build(), properties, + new HostRateLimiter(0, () -> 0L, millis -> { }), RobotsPolicy.ALLOW_ALL, millis -> { }); RestClient.Builder crawlBuilder = RestClient.builder(); crawlServer = MockRestServiceServer.bindTo(crawlBuilder).ignoreExpectOrder(true).build(); - SitemapCrawler crawler = new SitemapCrawler(crawlBuilder, new UrlMatcher()); + SitemapCrawler crawler = new SitemapCrawler(crawlBuilder, new UrlMatcher(), RobotsPolicy.ALLOW_ALL); ContentExtractor extractor = new ContentExtractor(new ContentLocaleResolver(), new ObjectMapper()); strategy = new WebsiteSourceStrategy(crawler, fetcher, extractor);