2026-10-28 07:45:57 +01:00
9 changed files with 165 additions and 12 deletions
@@ -21,7 +21,7 @@ The assignment is built in four lecture steps. Each step was delivered on its ow
 | --- | --- | --- |
 | 1 | Scrape the price from the practice page with BeautifulSoup | done |
 | 2 | E-mail alert below the target price, secrets in `.env` | done |
-| 3 | Browser-like request headers | planned (MIL-004) |
+| 3 | Browser-like request headers | done |
 | 4 | The live Amazon page | planned (MIL-005) |

 ### Step 1: scrape the price
@@ -42,6 +42,16 @@ The screenshots below come from a run where the SMTP connection is replaced by a

 ![The alert message](docs/images/step-2-email.png)

+### Step 3: request headers
+
+A browser sends headers with every request: the browser and system (`User-Agent`), the preferred language (`Accept-Language`) and more. A bare script sends almost none, so a server can answer in the wrong language or currency, or treat the request as a bot. `DEFAULT_HEADERS` in `constants.py` holds the browser-like set (without `Host` and `X-Amzn-Trace-Id`, as the lecture says) and `RequestsPageFetcher` passes it as `headers=` to `requests.get`.
+
+What a browser sends to [httpbin.org/headers](https://httpbin.org/headers), and what the bot sends (`Host` is added by `requests` itself):
+
+![Headers sent by a browser](docs/images/step-3-browser-headers.png)
+
+![Headers sent by the bot](docs/images/step-3-bot-headers.png)
+
 Repository layout:

 | Path | What it is for |
@@ -15,7 +15,7 @@ document of a type. `Primary File` may contain a glob (e.g.
 | SA | Stakeholder Analysis | docs/stakeholder-analysis.md | 002 |
 | PP | Project Plan | docs/project-plan.md | 002 |
 | MIL | Milestone / Gateway | docs/milestones/*.md | 007 |
-| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 013 |
+| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 014 |

 ## Languages

@@ -0,0 +1,61 @@
+# RC-013 Code review of MIL-004
+
+## Metadata
+| Key | Value |
+| --- | --- |
+| ID | RC-013 |
+| CrossReference | [MIL-004], [QC-PY-001] |
+
+## Version History
+| Date | Status | Author | Reviewer | Change | Commit |
+| --- | --- | --- | --- | --- | --- |
+| 2026-10-28 | Proposed | Jens Tirsvad Nielsen | AI as stand-in for S01 | Initial version | pending |
+
+---
+
+## Artifact Under Review
+
+- Instance reviewed: [MIL-004]
+- Checklist used: [QC-PY-001]
+- Scope: full review
+- Language and domain: n/a (technical)
+- Language reviewer: none
+- Mode: autonomous, not independent (AI as stand-in for S01); the author and the stand-in are both the AI working for S01
+
+## Checklist Results
+
+| # | Criterion | Status | Evidence/Notes |
+| --- | --- | --- | --- |
+| 1 | Packages, modules, functions, variables, classes and constants follow PEP 8 casing (`snake_case`, `PascalCase`, `UPPER_SNAKE`) | Pass | Modules and functions are snake_case, classes PascalCase, constants UPPER_SNAKE |
+| 2 | Names state purpose in the domain's language; no unexplained abbreviations, no single-letter names outside tiny scopes | Pass | Names state purpose (price, target, headers); no abbreviations left unexplained |
+| 3 | Code is produced by the project's formatter and passes its linter with no unexplained suppressions | Pass | ruff format and ruff check pass: 38 pytest tests, ruff 0 findings, mypy strict 0 errors (26 files) |
+| 4 | Every function and method signature is type-annotated, including `-> None` | Pass | All signatures annotated; mypy --strict passes: 38 pytest tests, ruff 0 findings, mypy strict 0 errors (26 files) |
+| 5 | No bare `except:`, no swallowed exceptions; specific exceptions are raised and the cause is kept (`raise ... from`) | Pass | No bare except; errors are specific and chained with `raise ... from` where a cause exists |
+| 6 | No mutable default arguments and no shadowed builtins | Pass | No mutable defaults, no shadowed builtins |
+| 7 | Files, locks and connections are managed with context managers | Pass | No open files or connections without a context manager |
+| 8 | Public modules, classes and functions have docstrings that say what, not how | Pass | Public modules and functions have docstrings with Doxygen tags |
+| 9 | Logging uses `logging`, not `print`; no secrets or personal data in log output | Pass | No secrets are involved; headers are public browser values |
+| 10 | Classes and operations trace to the Design Class Diagram they implement; deviations are recorded | N-A | No Design Class Diagram exists; the milestone's technical tasks need none (Needs use case: No) |
+| 11 | Tests exist for new behaviour, are named for the behaviour, and do not depend on order or the network | Pass | Tests exist, are named for behaviour and use no network or real mail server: 38 pytest tests, ruff 0 findings, mypy strict 0 errors (26 files) |
+| 12 | Type checker runs in strict mode without errors; `Any` is justified in a comment | Pass | mypy --strict passes; no Any |
+| 13 | Dependencies are declared and pinned in the project's dependency file, none unused | Pass | Dependencies are in pyproject.toml with lower bounds; none unused |
+| 14 | Each piece of knowledge (a business rule, constant, format, validation or query) is defined in one place; code is merged only where it expresses the same knowledge, not where it merely looks alike | Pass | Constants live only in constants.py |
+| 15 | Imports point inward: business-rule modules import no framework, database, UI or delivery-mechanism code, and there are no import cycles | Pass | Imports point inward; no cycles |
+| 16 | SOLID holds: a class or module has one reason to change; new behaviour is added by extension, not by editing a type test in several places; subclasses honour the contract of their base; protocols are narrow; high-level code depends on protocols or abstract types and receives its implementations | Pass | The fetcher takes its headers as a constructor argument (extension point); the default is one read-only mapping in constants.py |
+| 17 | Source is laid out in the layers and folders of the clean architecture folder structure (`domain`, `application`, `infrastructure`, `presentation` packages under `src/`, and `unit`, `integration` and `e2e` under `tests/`), in `snake_case`, and imports between the layers point inward | Pass | Layers and tests/unit/ mirror as before; only constants and the infrastructure adapter changed |
+| 18 | File and folder names are English, contain no space and no character outside letters, digits, `-`, `_` and `.`, and follow the casing of the language for files | Pass | File names are English snake_case |
+
+## Overall Verdict
+
+Go — all Mandatory criteria pass or are not applicable.
+
+## Action Items
+
+| Action | Owner | Due |
+| --- | --- | --- |
+| None: no Fail and no Pending criterion | S01 | 2026-10-28 |
+
+---
+
+[MIL-004]: ../../milestones/mil-004-request-headers.md
+[QC-PY-001]: ../../../framework/qc/code/qc-programming-python.md
@@ -4,6 +4,8 @@
@brief The only place that defines constants (URLs, limits, environment names).
 """

+from collections.abc import Mapping
+from types import MappingProxyType
 from typing import Final

 ## @brief Practice page used instead of Amazon while developing.
@@ -23,6 +25,30 @@ TARGET_PRICE: Final[float] = 100.0
 ## @brief Seconds to wait for an HTTP response.
 REQUEST_TIMEOUT_SECONDS: Final[int] = 10

+## @brief Browser-like request headers, read-only.
+# Copied from a real browser (see httpbin.org/headers) as the lecture says,
+# without Host and X-Amzn-Trace-Id. No br/zstd: requests may not decode them.
+DEFAULT_HEADERS: Final[Mapping[str, str]] = MappingProxyType(
+    {
+        "User-Agent": (
+            "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 "
+            "(KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36"
+        ),
+        "Accept-Language": "en-GB,en-US;q=0.9,en;q=0.8",
+        "Accept": (
+            "text/html,application/xhtml+xml,application/xml;q=0.9,"
+            "image/avif,image/webp,*/*;q=0.8"
+        ),
+        "Accept-Encoding": "gzip, deflate",
+        "Upgrade-Insecure-Requests": "1",
+        "Sec-Fetch-Dest": "document",
+        "Sec-Fetch-Mode": "navigate",
+        "Sec-Fetch-Site": "none",
+        "Sec-Fetch-User": "?1",
+        "Priority": "u=0, i",
+    }
+)
+
 ## @brief SMTP submission port (STARTTLS).
 SMTP_PORT: Final[int] = 587

@@ -4,14 +4,23 @@
@brief Downloads a page with requests.
 """

+from collections.abc import Mapping
+
 import requests

-from amazon_price_tracker.constants import REQUEST_TIMEOUT_SECONDS
+from amazon_price_tracker.constants import DEFAULT_HEADERS, REQUEST_TIMEOUT_SECONDS


 class RequestsPageFetcher:
    """PageFetcher port implemented with requests."""

+    def __init__(self, headers: Mapping[str, str] = DEFAULT_HEADERS) -> None:
+        """Keep the headers sent with every request.
+
+        @param headers Request headers; browser-like by default.
+        """
+        self._headers = headers
+
    def fetch(self, url: str) -> str:
        """Return the HTML text of the page at url.

@@ -19,6 +28,8 @@ class RequestsPageFetcher:
        @return The response body as text.
        @throws requests.HTTPError When the server answers with an error status.
        """
-        response = requests.get(url, timeout=REQUEST_TIMEOUT_SECONDS)
+        response = requests.get(
+            url, headers=dict(self._headers), timeout=REQUEST_TIMEOUT_SECONDS
+        )
        response.raise_for_status()
        return response.text
@@ -1,9 +1,11 @@
 import pytest
 import requests

-from amazon_price_tracker.constants import REQUEST_TIMEOUT_SECONDS
+from amazon_price_tracker.constants import DEFAULT_HEADERS, REQUEST_TIMEOUT_SECONDS
 from amazon_price_tracker.infrastructure.http_client import RequestsPageFetcher

+Call = tuple[str, dict[str, str], float]
+

 class FakeResponse:
    def __init__(self, text: str, status_code: int) -> None:
@@ -15,13 +17,11 @@ class FakeResponse:
            raise requests.HTTPError(f"status {self.status_code}")


-def _patch_get(
-    monkeypatch: pytest.MonkeyPatch, response: FakeResponse
-) -> list[tuple[str, float]]:
-    calls: list[tuple[str, float]] = []
+def _patch_get(monkeypatch: pytest.MonkeyPatch, response: FakeResponse) -> list[Call]:
+    calls: list[Call] = []

-    def fake_get(url: str, timeout: float) -> FakeResponse:
-        calls.append((url, timeout))
+    def fake_get(url: str, headers: dict[str, str], timeout: float) -> FakeResponse:
+        calls.append((url, headers, timeout))
        return response

    monkeypatch.setattr(requests, "get", fake_get)
@@ -36,7 +36,46 @@ def test_fetch_returns_text_using_url_and_timeout(
    html = RequestsPageFetcher().fetch("https://example.test/")

    assert html == "<p>hi</p>"
-    assert calls == [("https://example.test/", REQUEST_TIMEOUT_SECONDS)]
+    assert calls == [
+        ("https://example.test/", dict(DEFAULT_HEADERS), REQUEST_TIMEOUT_SECONDS)
+    ]
+
+
+def test_fetch_sends_user_agent_and_accept_language(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    calls = _patch_get(monkeypatch, FakeResponse("", 200))
+
+    RequestsPageFetcher().fetch("https://example.test/")
+
+    headers = calls[0][1]
+    assert headers["User-Agent"]
+    assert headers["Accept-Language"]
+
+
+def test_fetch_omits_host_and_trace_id_headers(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    calls = _patch_get(monkeypatch, FakeResponse("", 200))
+
+    RequestsPageFetcher().fetch("https://example.test/")
+
+    assert "Host" not in calls[0][1]
+    assert "X-Amzn-Trace-Id" not in calls[0][1]
+
+
+def test_fetch_uses_overriding_headers(monkeypatch: pytest.MonkeyPatch) -> None:
+    calls = _patch_get(monkeypatch, FakeResponse("", 200))
+
+    RequestsPageFetcher(headers={"User-Agent": "custom"}).fetch("https://example.test/")
+
+    assert calls[0][1] == {"User-Agent": "custom"}
+    assert calls[0][2] == REQUEST_TIMEOUT_SECONDS
+
+
+def test_default_headers_are_read_only() -> None:
+    with pytest.raises(TypeError):
+        DEFAULT_HEADERS["User-Agent"] = "x"  # type: ignore[index]


 def test_fetch_raises_on_http_error(monkeypatch: pytest.MonkeyPatch) -> None:
@@ -36,6 +36,12 @@ def test_env_names_are_unique() -> None:
    assert len(set(ENV_NAMES)) == len(ENV_NAMES)


+def test_default_headers_are_non_empty() -> None:
+    """Every default header has a name and a value."""
+    assert constants.DEFAULT_HEADERS
+    assert all(k.strip() and v.strip() for k, v in constants.DEFAULT_HEADERS.items())
+
+
 def test_smtp_port_is_starttls_port() -> None:
    """The SMTP port is 587."""
    assert constants.SMTP_PORT == 587