Merge pull request #10 from OpenElementsLabs/feat/005-page-fetcher

Page fetcher — robust, polite HTTP GET

Author: hendrikebbers

CLAUDE.md

@@ -34,6 +34,9 @@ extraction, Meilisearch indexing with scheduled refresh, and four MCP tools
 ├── src/main/java/com/openelements/content/
 │   ├── ContentMcpApplication.java           # entry point; imports library configs, enables scheduling
 │   ├── ContentConfig.java                   # @Configuration; ContentSourceProperties + IndexSettings bean
+│   ├── ContentHttpConfig.java               # timeout-configured RestClient + HostRateLimiter beans
+│   ├── PageFetcher.java / FetchResult.java  # robust HTTP GET (conditional, capped, retrying)
+│   ├── HostRateLimiter.java                 # per-host request throttle (injectable clock/sleeper)
 │   ├── ContentDocument.java                 # canonical indexed-document record (id() + toMap())
 │   ├── ContentSource.java / SourceType.java # typed, declarative source model (website|git)
 │   ├── ContentSourceProperties.java         # @ConfigurationProperties("open-elements.content")
@@ -73,7 +76,11 @@ SHA-256 of the URL; `toMap()` for indexing); `ContentConfig` registers the Meili
 sortable by date), which the library's initializer applies to the index at startup. `SitemapCrawler`
 (the first stage of the ingestion pipeline) discovers a source's URLs from its sitemaps (recursing
 into sitemap indexes) — or, when none are configured, via a bounded same-host fallback crawl — and
-returns `DiscoveredItem`s filtered by `UrlMatcher`; it is fault-tolerant per sitemap.
+returns `DiscoveredItem`s filtered by `UrlMatcher`; it is fault-tolerant per sitemap. `PageFetcher`
+(the fetch stage) performs polite HTTP GETs — bot `User-Agent`, conditional `If-None-Match`/
+`If-Modified-Since` (→ `304` handling), a `max-body-bytes` cap, per-host rate limiting
+(`HostRateLimiter`), and retry-with-backoff on transient failures — returning a classified
+`FetchResult` (`OK`/`NOT_MODIFIED`/`NOT_FOUND`/`ERROR`).
 
 > **Key gotcha:** the library ships no Spring Boot auto-configuration and couples MCP to a JPA
 > datasource, so `@Import({ McpConfiguration, SearchConfig })` alone does **not** boot. See

docs/specs/005-page-fetcher/steps.md

@@ -0,0 +1,65 @@
+# Implementation Steps: Page Fetcher
+
+## Step 1: `FetchResult`
+
+- [x] Record `FetchResult(Status, html, etag, lastModified, httpStatus)` with nested `Status {OK, NOT_MODIFIED, NOT_FOUND, ERROR}` and small factory helpers
+
+**Related behaviors:** all (return shape)
+
+---
+
+## Step 2: `HostRateLimiter`
+
+- [x] Per-host request spacing with injectable clock (`LongSupplier`) and sleeper (`LongConsumer`) for deterministic tests
+- [x] Independent buckets per host; non-positive rate disables throttling
+
+**Related behaviors:** Requests to the same host are throttled; Different hosts are not cross-throttled
+
+---
+
+## Step 3: `PageFetcher`
+
+- [x] `@Component`; per-request bot `User-Agent`; conditional `If-None-Match`/`If-Modified-Since` headers
+- [x] `.exchange(...)` callback classifies status without throwing: 304 → NOT_MODIFIED, 404/410 → NOT_FOUND, 2xx → OK, else ERROR
+- [x] Body read under `max-body-bytes` cap via `readNBytes` (bounded memory); oversized → aborted ERROR with a logged warning
+- [x] Retry transient failures (5xx, network/timeout) with exponential backoff (injectable sleeper); 404 and other 4xx not retried
+- [x] Per-host rate limiting via `HostRateLimiter`
+
+**Related behaviors:** 200 body+validators; bot UA; 304; oversized; timeout transient; 5xx retried then ERROR; 404 no retry; recovery
+
+---
+
+## Step 4: Wiring
+
+- [x] `ContentHttpConfig` provides the timeout-configured `contentRestClient` and the production `HostRateLimiter` (kept separate from `ContentConfig` so non-HTTP config stays isolated)
+
+**Related behaviors:** n/a (wiring)
+
+---
+
+## Step 5: Tests
+
+- [x] `PageFetcherTest` via `MockRestServiceServer` (no network): 200, UA header, conditional/304, oversized, 404 no-retry, persistent 5xx, transient network error, 503→200 recovery — with a no-op rate limiter and recorded backoffs
+- [x] `HostRateLimiterTest` with a virtual clock + recording sleeper: same-host spacing, per-host independence, disabled rate
+
+**Acceptance criteria:**
+- [x] All tests pass (`mvn test`); build green
+
+---
+
+## Behavior Coverage
+
+| Scenario | Layer | Covered in Step |
+|----------|-------|-----------------|
+| 200 returns body and validators | Backend | Step 5 (`PageFetcherTest.success200ReturnsBodyAndValidators`) |
+| Bot user-agent is sent | Backend | Step 5 (`PageFetcherTest.botUserAgentIsSent`) |
+| Unchanged page returns 304 | Backend | Step 5 (`PageFetcherTest.conditionalRequestYieldsNotModified`) |
+| Oversized body is capped | Backend | Step 5 (`PageFetcherTest.oversizedBodyIsAborted`) |
+| Request times out | Backend | Step 5 (`PageFetcherTest.transientNetworkErrorIsRetried`) |
+| Requests to the same host are throttled | Backend | Step 5 (`HostRateLimiterTest.sameHostIsThrottled`) |
+| Different hosts are not cross-throttled | Backend | Step 5 (`HostRateLimiterTest.differentHostsAreIndependent`) |
+| Transient 5xx is retried then fails | Backend | Step 5 (`PageFetcherTest.persistent5xxIsRetriedThenFails`) |
+| 404 is not retried | Backend | Step 5 (`PageFetcherTest.notFoundIsNotRetried`) |
+| Recovery after transient failure | Backend | Step 5 (`PageFetcherTest.recoversAfterTransientFailure`) |
+
+All scenarios are backend; there is no frontend in this spec.

docs/specs/INDEX.md

@@ -10,7 +10,7 @@ roadmap step, to be implemented sequentially (each builds on the previous).
 | 002 | 002-content-source-config | Content source config | backend, architecture, configuration | `ContentSource` abstraction + `@ConfigurationProperties` + Ant-glob URL matching | #3 | done |
 | 003 | 003-content-document-index | Content document & index | backend, search, data-model | `ContentDocument` record + Meilisearch `IndexSettings` bean | #5 | done |
 | 004 | 004-sitemap-crawler | Sitemap crawler | backend, crawler | `SitemapCrawler` — sitemap discovery of URLs + lastmod | #7 | done |
-| 005 | 005-page-fetcher | Page fetcher | backend, crawler | `PageFetcher` — robust HTTP fetch (ETag/IMS, rate limit, retry) | — | open |
+| 005 | 005-page-fetcher | Page fetcher | backend, crawler | `PageFetcher` — robust HTTP fetch (ETag/IMS, rate limit, retry) | #9 | done |
 | 006 | 006-content-extractor | Content extractor | backend, crawler | `ContentExtractor` — jsoup content + metadata extraction | — | open |
 | 007 | 007-source-strategy | Source strategy | backend, architecture | `ContentSourceStrategy` interface + `WebsiteSourceStrategy` | — | open |
 | 008 | 008-content-indexer | Content indexer | backend, search, crawler | `ContentIndexer` — orchestration, diff, upsert/delete | — | open |

src/main/java/com/openelements/content/ContentConfig.java

@@ -13,9 +13,10 @@
  *
  * <p>Enables {@link ContentSourceProperties} binding and is component-scanned together with the rest
  * of the {@code com.openelements.content} package (e.g. {@link UrlMatcher},
- * {@link ContentLocaleResolver}). It also declares the Meilisearch {@link IndexSettings} for the
- * content index. Later specs extend it with further {@code @Bean} definitions (crawler, indexer,
- * search service, MCP tools).
+ * {@link ContentLocaleResolver}, {@link SitemapCrawler}, {@link PageFetcher}). It also declares the
+ * Meilisearch {@link IndexSettings} for the content index. The HTTP client and rate limiter used by
+ * {@link PageFetcher} live in {@link ContentHttpConfig}. Later specs extend it with further
+ * {@code @Bean} definitions (indexer, search service, MCP tools).
  */
 @Configuration
 @EnableConfigurationProperties(ContentSourceProperties.class)

src/main/java/com/openelements/content/ContentHttpConfig.java

@@ -0,0 +1,47 @@
+package com.openelements.content;
+
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.http.client.SimpleClientHttpRequestFactory;
+import org.springframework.web.client.RestClient;
+
+/**
+ * HTTP-related beans for the content pipeline: the timeout-configured {@link RestClient} and the
+ * {@link HostRateLimiter} used by {@link PageFetcher}.
+ *
+ * <p>Kept separate from {@link ContentConfig} so configuration that has nothing to do with HTTP
+ * (e.g. the Meilisearch index settings) can be exercised in isolation without providing an HTTP
+ * client.
+ */
+@Configuration
+public class ContentHttpConfig {
+
+    /**
+     * The {@link RestClient} used by {@link PageFetcher}, configured with the source's request
+     * timeout for both connect and read. The bot {@code User-Agent} is applied per request by the
+     * fetcher rather than as a client default.
+     *
+     * @param builder    the Spring-provided builder
+     * @param properties content properties supplying the request timeout
+     * @return a timeout-configured client
+     */
+    @Bean
+    RestClient contentRestClient(RestClient.Builder builder, ContentSourceProperties properties) {
+        SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory();
+        factory.setConnectTimeout(properties.requestTimeout());
+        factory.setReadTimeout(properties.requestTimeout());
+        return builder.requestFactory(factory).build();
+    }
+
+    /**
+     * The per-host rate limiter used by {@link PageFetcher}, wired with the real wall clock and
+     * {@link Thread#sleep(long)}.
+     *
+     * @param properties content properties supplying the per-host request rate
+     * @return the rate limiter
+     */
+    @Bean
+    HostRateLimiter hostRateLimiter(ContentSourceProperties properties) {
+        return new HostRateLimiter(properties.rateLimitPerHost(), System::nanoTime, HostRateLimiter::sleepMillis);
+    }
+}

src/main/java/com/openelements/content/FetchResult.java

@@ -0,0 +1,47 @@
+package com.openelements.content;
+
+/**
+ * The outcome of a {@link PageFetcher} request.
+ *
+ * @param status       the classified outcome
+ * @param html         the response body, or {@code null} unless {@link Status#OK}
+ * @param etag         the response {@code ETag}, or {@code null} if absent
+ * @param lastModified the response {@code Last-Modified}, or {@code null} if absent
+ * @param httpStatus   the raw HTTP status code ({@code 0} if the request failed before a response)
+ */
+public record FetchResult(
+    Status status,
+    String html,
+    String etag,
+    String lastModified,
+    int httpStatus
+) {
+
+    /** Classified fetch outcome. */
+    public enum Status {
+        /** 2xx with a usable body. */
+        OK,
+        /** 304 Not Modified — the caller's cached copy is still current. */
+        NOT_MODIFIED,
+        /** 404/410 — the page is gone; the indexer should delete it. */
+        NOT_FOUND,
+        /** A transient failure that exhausted retries, an oversized body, or a non-retryable error. */
+        ERROR
+    }
+
+    static FetchResult ok(String html, String etag, String lastModified, int httpStatus) {
+        return new FetchResult(Status.OK, html, etag, lastModified, httpStatus);
+    }
+
+    static FetchResult notModified(String etag, String lastModified, int httpStatus) {
+        return new FetchResult(Status.NOT_MODIFIED, null, etag, lastModified, httpStatus);
+    }
+
+    static FetchResult notFound(int httpStatus) {
+        return new FetchResult(Status.NOT_FOUND, null, null, null, httpStatus);
+    }
+
+    static FetchResult error(int httpStatus) {
+        return new FetchResult(Status.ERROR, null, null, null, httpStatus);
+    }
+}

src/main/java/com/openelements/content/HostRateLimiter.java

@@ -0,0 +1,65 @@
+package com.openelements.content;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.LongConsumer;
+import java.util.function.LongSupplier;
+
+/**
+ * A simple per-host rate limiter that spaces requests to the same host to a maximum rate, while
+ * treating different hosts independently.
+ *
+ * <p>The clock and the sleep operation are injected so the limiter is fully testable without real
+ * time: production wires {@link System#nanoTime()} and {@link Thread#sleep(long)}, tests supply a
+ * virtual clock and a recording sleeper.
+ */
+public class HostRateLimiter {
+
+    private final long minIntervalNanos;
+    private final LongSupplier clockNanos;
+    private final LongConsumer sleeperMillis;
+    private final Map<String, Long> nextAllowedNanos = new HashMap<>();
+
+    /**
+     * @param permitsPerSecond maximum requests per second per host (values {@code <= 0} disable throttling)
+     * @param clockNanos       monotonic time source in nanoseconds
+     * @param sleeperMillis    blocks the caller for the given number of milliseconds
+     */
+    public HostRateLimiter(double permitsPerSecond, LongSupplier clockNanos, LongConsumer sleeperMillis) {
+        this.minIntervalNanos = permitsPerSecond <= 0 ? 0L : (long) (1_000_000_000.0 / permitsPerSecond);
+        this.clockNanos = clockNanos;
+        this.sleeperMillis = sleeperMillis;
+    }
+
+    /**
+     * Blocks until a request to {@code host} is allowed under the configured rate, then reserves the
+     * next slot for that host.
+     *
+     * @param host the target host (an empty or {@code null} host shares a single bucket)
+     */
+    public void acquire(String host) {
+        if (minIntervalNanos == 0L) {
+            return;
+        }
+        String key = host == null ? "" : host;
+        long waitNanos;
+        synchronized (this) {
+            long now = clockNanos.getAsLong();
+            long start = Math.max(now, nextAllowedNanos.getOrDefault(key, now));
+            waitNanos = start - now;
+            nextAllowedNanos.put(key, start + minIntervalNanos);
+        }
+        if (waitNanos > 0) {
+            sleeperMillis.accept(Math.max(1, Math.round(waitNanos / 1_000_000.0)));
+        }
+    }
+
+    /** Production sleeper: blocks the current thread, restoring the interrupt flag if interrupted. */
+    static void sleepMillis(long millis) {
+        try {
+            Thread.sleep(millis);
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+        }
+    }
+}