Skip to content

Commit dee40ce

Browse files
committed
feat(spec-005): robust PageFetcher with per-host rate limiting
- FetchResult(status, html, etag, lastModified, httpStatus) with OK/NOT_MODIFIED/NOT_FOUND/ERROR. - HostRateLimiter: per-host request spacing with injectable clock + sleeper (deterministic tests); independent per-host buckets. - PageFetcher (@component): per-request bot User-Agent, conditional If-None-Match/If-Modified-Since (304 handling), .exchange status classification, max-body-bytes cap via readNBytes (bounded memory, oversized aborted), retry-with-backoff on 5xx/network errors, 404/410 as NOT_FOUND without retry. - ContentHttpConfig: timeout-configured contentRestClient + HostRateLimiter beans (split from ContentConfig to keep non-HTTP config isolated). - Tests: PageFetcherTest (MockRestServiceServer, no network) + HostRateLimiterTest (virtual clock). 52 tests green. - steps.md; refresh CLAUDE.md. Refs #9
1 parent 1e236af commit dee40ce

9 files changed

Lines changed: 606 additions & 4 deletions

File tree

CLAUDE.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ extraction, Meilisearch indexing with scheduled refresh, and four MCP tools
3434
├── src/main/java/com/openelements/content/
3535
│ ├── ContentMcpApplication.java # entry point; imports library configs, enables scheduling
3636
│ ├── ContentConfig.java # @Configuration; ContentSourceProperties + IndexSettings bean
37+
│ ├── ContentHttpConfig.java # timeout-configured RestClient + HostRateLimiter beans
38+
│ ├── PageFetcher.java / FetchResult.java # robust HTTP GET (conditional, capped, retrying)
39+
│ ├── HostRateLimiter.java # per-host request throttle (injectable clock/sleeper)
3740
│ ├── ContentDocument.java # canonical indexed-document record (id() + toMap())
3841
│ ├── ContentSource.java / SourceType.java # typed, declarative source model (website|git)
3942
│ ├── ContentSourceProperties.java # @ConfigurationProperties("open-elements.content")
@@ -73,7 +76,11 @@ SHA-256 of the URL; `toMap()` for indexing); `ContentConfig` registers the Meili
7376
sortable by date), which the library's initializer applies to the index at startup. `SitemapCrawler`
7477
(the first stage of the ingestion pipeline) discovers a source's URLs from its sitemaps (recursing
7578
into sitemap indexes) — or, when none are configured, via a bounded same-host fallback crawl — and
76-
returns `DiscoveredItem`s filtered by `UrlMatcher`; it is fault-tolerant per sitemap.
79+
returns `DiscoveredItem`s filtered by `UrlMatcher`; it is fault-tolerant per sitemap. `PageFetcher`
80+
(the fetch stage) performs polite HTTP GETs — bot `User-Agent`, conditional `If-None-Match`/
81+
`If-Modified-Since` (→ `304` handling), a `max-body-bytes` cap, per-host rate limiting
82+
(`HostRateLimiter`), and retry-with-backoff on transient failures — returning a classified
83+
`FetchResult` (`OK`/`NOT_MODIFIED`/`NOT_FOUND`/`ERROR`).
7784

7885
> **Key gotcha:** the library ships no Spring Boot auto-configuration and couples MCP to a JPA
7986
> datasource, so `@Import({ McpConfiguration, SearchConfig })` alone does **not** boot. See
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# Implementation Steps: Page Fetcher
2+
3+
## Step 1: `FetchResult`
4+
5+
- [x] Record `FetchResult(Status, html, etag, lastModified, httpStatus)` with nested `Status {OK, NOT_MODIFIED, NOT_FOUND, ERROR}` and small factory helpers
6+
7+
**Related behaviors:** all (return shape)
8+
9+
---
10+
11+
## Step 2: `HostRateLimiter`
12+
13+
- [x] Per-host request spacing with injectable clock (`LongSupplier`) and sleeper (`LongConsumer`) for deterministic tests
14+
- [x] Independent buckets per host; non-positive rate disables throttling
15+
16+
**Related behaviors:** Requests to the same host are throttled; Different hosts are not cross-throttled
17+
18+
---
19+
20+
## Step 3: `PageFetcher`
21+
22+
- [x] `@Component`; per-request bot `User-Agent`; conditional `If-None-Match`/`If-Modified-Since` headers
23+
- [x] `.exchange(...)` callback classifies status without throwing: 304 → NOT_MODIFIED, 404/410 → NOT_FOUND, 2xx → OK, else ERROR
24+
- [x] Body read under `max-body-bytes` cap via `readNBytes` (bounded memory); oversized → aborted ERROR with a logged warning
25+
- [x] Retry transient failures (5xx, network/timeout) with exponential backoff (injectable sleeper); 404 and other 4xx not retried
26+
- [x] Per-host rate limiting via `HostRateLimiter`
27+
28+
**Related behaviors:** 200 body+validators; bot UA; 304; oversized; timeout transient; 5xx retried then ERROR; 404 no retry; recovery
29+
30+
---
31+
32+
## Step 4: Wiring
33+
34+
- [x] `ContentHttpConfig` provides the timeout-configured `contentRestClient` and the production `HostRateLimiter` (kept separate from `ContentConfig` so non-HTTP config stays isolated)
35+
36+
**Related behaviors:** n/a (wiring)
37+
38+
---
39+
40+
## Step 5: Tests
41+
42+
- [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
43+
- [x] `HostRateLimiterTest` with a virtual clock + recording sleeper: same-host spacing, per-host independence, disabled rate
44+
45+
**Acceptance criteria:**
46+
- [x] All tests pass (`mvn test`); build green
47+
48+
---
49+
50+
## Behavior Coverage
51+
52+
| Scenario | Layer | Covered in Step |
53+
|----------|-------|-----------------|
54+
| 200 returns body and validators | Backend | Step 5 (`PageFetcherTest.success200ReturnsBodyAndValidators`) |
55+
| Bot user-agent is sent | Backend | Step 5 (`PageFetcherTest.botUserAgentIsSent`) |
56+
| Unchanged page returns 304 | Backend | Step 5 (`PageFetcherTest.conditionalRequestYieldsNotModified`) |
57+
| Oversized body is capped | Backend | Step 5 (`PageFetcherTest.oversizedBodyIsAborted`) |
58+
| Request times out | Backend | Step 5 (`PageFetcherTest.transientNetworkErrorIsRetried`) |
59+
| Requests to the same host are throttled | Backend | Step 5 (`HostRateLimiterTest.sameHostIsThrottled`) |
60+
| Different hosts are not cross-throttled | Backend | Step 5 (`HostRateLimiterTest.differentHostsAreIndependent`) |
61+
| Transient 5xx is retried then fails | Backend | Step 5 (`PageFetcherTest.persistent5xxIsRetriedThenFails`) |
62+
| 404 is not retried | Backend | Step 5 (`PageFetcherTest.notFoundIsNotRetried`) |
63+
| Recovery after transient failure | Backend | Step 5 (`PageFetcherTest.recoversAfterTransientFailure`) |
64+
65+
All scenarios are backend; there is no frontend in this spec.

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@
1313
*
1414
* <p>Enables {@link ContentSourceProperties} binding and is component-scanned together with the rest
1515
* of the {@code com.openelements.content} package (e.g. {@link UrlMatcher},
16-
* {@link ContentLocaleResolver}). It also declares the Meilisearch {@link IndexSettings} for the
17-
* content index. Later specs extend it with further {@code @Bean} definitions (crawler, indexer,
18-
* search service, MCP tools).
16+
* {@link ContentLocaleResolver}, {@link SitemapCrawler}, {@link PageFetcher}). It also declares the
17+
* Meilisearch {@link IndexSettings} for the content index. The HTTP client and rate limiter used by
18+
* {@link PageFetcher} live in {@link ContentHttpConfig}. Later specs extend it with further
19+
* {@code @Bean} definitions (indexer, search service, MCP tools).
1920
*/
2021
@Configuration
2122
@EnableConfigurationProperties(ContentSourceProperties.class)
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package com.openelements.content;
2+
3+
import org.springframework.context.annotation.Bean;
4+
import org.springframework.context.annotation.Configuration;
5+
import org.springframework.http.client.SimpleClientHttpRequestFactory;
6+
import org.springframework.web.client.RestClient;
7+
8+
/**
9+
* HTTP-related beans for the content pipeline: the timeout-configured {@link RestClient} and the
10+
* {@link HostRateLimiter} used by {@link PageFetcher}.
11+
*
12+
* <p>Kept separate from {@link ContentConfig} so configuration that has nothing to do with HTTP
13+
* (e.g. the Meilisearch index settings) can be exercised in isolation without providing an HTTP
14+
* client.
15+
*/
16+
@Configuration
17+
public class ContentHttpConfig {
18+
19+
/**
20+
* The {@link RestClient} used by {@link PageFetcher}, configured with the source's request
21+
* timeout for both connect and read. The bot {@code User-Agent} is applied per request by the
22+
* fetcher rather than as a client default.
23+
*
24+
* @param builder the Spring-provided builder
25+
* @param properties content properties supplying the request timeout
26+
* @return a timeout-configured client
27+
*/
28+
@Bean
29+
RestClient contentRestClient(RestClient.Builder builder, ContentSourceProperties properties) {
30+
SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory();
31+
factory.setConnectTimeout(properties.requestTimeout());
32+
factory.setReadTimeout(properties.requestTimeout());
33+
return builder.requestFactory(factory).build();
34+
}
35+
36+
/**
37+
* The per-host rate limiter used by {@link PageFetcher}, wired with the real wall clock and
38+
* {@link Thread#sleep(long)}.
39+
*
40+
* @param properties content properties supplying the per-host request rate
41+
* @return the rate limiter
42+
*/
43+
@Bean
44+
HostRateLimiter hostRateLimiter(ContentSourceProperties properties) {
45+
return new HostRateLimiter(properties.rateLimitPerHost(), System::nanoTime, HostRateLimiter::sleepMillis);
46+
}
47+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package com.openelements.content;
2+
3+
/**
4+
* The outcome of a {@link PageFetcher} request.
5+
*
6+
* @param status the classified outcome
7+
* @param html the response body, or {@code null} unless {@link Status#OK}
8+
* @param etag the response {@code ETag}, or {@code null} if absent
9+
* @param lastModified the response {@code Last-Modified}, or {@code null} if absent
10+
* @param httpStatus the raw HTTP status code ({@code 0} if the request failed before a response)
11+
*/
12+
public record FetchResult(
13+
Status status,
14+
String html,
15+
String etag,
16+
String lastModified,
17+
int httpStatus
18+
) {
19+
20+
/** Classified fetch outcome. */
21+
public enum Status {
22+
/** 2xx with a usable body. */
23+
OK,
24+
/** 304 Not Modified — the caller's cached copy is still current. */
25+
NOT_MODIFIED,
26+
/** 404/410 — the page is gone; the indexer should delete it. */
27+
NOT_FOUND,
28+
/** A transient failure that exhausted retries, an oversized body, or a non-retryable error. */
29+
ERROR
30+
}
31+
32+
static FetchResult ok(String html, String etag, String lastModified, int httpStatus) {
33+
return new FetchResult(Status.OK, html, etag, lastModified, httpStatus);
34+
}
35+
36+
static FetchResult notModified(String etag, String lastModified, int httpStatus) {
37+
return new FetchResult(Status.NOT_MODIFIED, null, etag, lastModified, httpStatus);
38+
}
39+
40+
static FetchResult notFound(int httpStatus) {
41+
return new FetchResult(Status.NOT_FOUND, null, null, null, httpStatus);
42+
}
43+
44+
static FetchResult error(int httpStatus) {
45+
return new FetchResult(Status.ERROR, null, null, null, httpStatus);
46+
}
47+
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package com.openelements.content;
2+
3+
import java.util.HashMap;
4+
import java.util.Map;
5+
import java.util.function.LongConsumer;
6+
import java.util.function.LongSupplier;
7+
8+
/**
9+
* A simple per-host rate limiter that spaces requests to the same host to a maximum rate, while
10+
* treating different hosts independently.
11+
*
12+
* <p>The clock and the sleep operation are injected so the limiter is fully testable without real
13+
* time: production wires {@link System#nanoTime()} and {@link Thread#sleep(long)}, tests supply a
14+
* virtual clock and a recording sleeper.
15+
*/
16+
public class HostRateLimiter {
17+
18+
private final long minIntervalNanos;
19+
private final LongSupplier clockNanos;
20+
private final LongConsumer sleeperMillis;
21+
private final Map<String, Long> nextAllowedNanos = new HashMap<>();
22+
23+
/**
24+
* @param permitsPerSecond maximum requests per second per host (values {@code <= 0} disable throttling)
25+
* @param clockNanos monotonic time source in nanoseconds
26+
* @param sleeperMillis blocks the caller for the given number of milliseconds
27+
*/
28+
public HostRateLimiter(double permitsPerSecond, LongSupplier clockNanos, LongConsumer sleeperMillis) {
29+
this.minIntervalNanos = permitsPerSecond <= 0 ? 0L : (long) (1_000_000_000.0 / permitsPerSecond);
30+
this.clockNanos = clockNanos;
31+
this.sleeperMillis = sleeperMillis;
32+
}
33+
34+
/**
35+
* Blocks until a request to {@code host} is allowed under the configured rate, then reserves the
36+
* next slot for that host.
37+
*
38+
* @param host the target host (an empty or {@code null} host shares a single bucket)
39+
*/
40+
public void acquire(String host) {
41+
if (minIntervalNanos == 0L) {
42+
return;
43+
}
44+
String key = host == null ? "" : host;
45+
long waitNanos;
46+
synchronized (this) {
47+
long now = clockNanos.getAsLong();
48+
long start = Math.max(now, nextAllowedNanos.getOrDefault(key, now));
49+
waitNanos = start - now;
50+
nextAllowedNanos.put(key, start + minIntervalNanos);
51+
}
52+
if (waitNanos > 0) {
53+
sleeperMillis.accept(Math.max(1, Math.round(waitNanos / 1_000_000.0)));
54+
}
55+
}
56+
57+
/** Production sleeper: blocks the current thread, restoring the interrupt flag if interrupted. */
58+
static void sleepMillis(long millis) {
59+
try {
60+
Thread.sleep(millis);
61+
} catch (InterruptedException e) {
62+
Thread.currentThread().interrupt();
63+
}
64+
}
65+
}

0 commit comments

Comments
 (0)