diff --git a/README.md b/README.md index ea16521..93dc9b1 100644 --- a/README.md +++ b/README.md @@ -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 | diff --git a/docs/artifact-registry.md b/docs/artifact-registry.md index f6c14b0..3b2f546 100644 --- a/docs/artifact-registry.md +++ b/docs/artifact-registry.md @@ -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 diff --git a/docs/images/step-3-bot-headers.png b/docs/images/step-3-bot-headers.png new file mode 100644 index 0000000..3536968 Binary files /dev/null and b/docs/images/step-3-bot-headers.png differ diff --git a/docs/images/step-3-browser-headers.png b/docs/images/step-3-browser-headers.png new file mode 100644 index 0000000..9d8ef47 Binary files /dev/null and b/docs/images/step-3-browser-headers.png differ diff --git a/docs/sqa/reviews/rc-013-code-mil-004.md b/docs/sqa/reviews/rc-013-code-mil-004.md new file mode 100644 index 0000000..1b7273f --- /dev/null +++ b/docs/sqa/reviews/rc-013-code-mil-004.md @@ -0,0 +1,62 @@ +# 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 | [ec6ed21] | + +--- + +## 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 +[ec6ed21]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/047-amazon-price-tracker/commit/ec6ed213fbbc33941caf31019cee42997deb4261 diff --git a/src/amazon_price_tracker/constants.py b/src/amazon_price_tracker/constants.py index d6e182d..a874669 100644 --- a/src/amazon_price_tracker/constants.py +++ b/src/amazon_price_tracker/constants.py @@ -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 diff --git a/src/amazon_price_tracker/infrastructure/http_client.py b/src/amazon_price_tracker/infrastructure/http_client.py index 1572ce5..ffe66d2 100644 --- a/src/amazon_price_tracker/infrastructure/http_client.py +++ b/src/amazon_price_tracker/infrastructure/http_client.py @@ -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 diff --git a/tests/unit/infrastructure/test_http_client.py b/tests/unit/infrastructure/test_http_client.py index 46e03ba..e38037a 100644 --- a/tests/unit/infrastructure/test_http_client.py +++ b/tests/unit/infrastructure/test_http_client.py @@ -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 == "

hi

" - 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: diff --git a/tests/unit/test_constants.py b/tests/unit/test_constants.py index d2f6508..837b56f 100644 --- a/tests/unit/test_constants.py +++ b/tests/unit/test_constants.py @@ -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