diff --git a/.env.example b/.env.example index 74ece5d..1176fc1 100644 --- a/.env.example +++ b/.env.example @@ -2,5 +2,6 @@ SMTP_ADDRESS="smtp.gmail.com" EMAIL_ADDRESS="you@example.com" EMAIL_PASSWORD="Use_an_App_Password_from_your_email_provider" -# Optional: track another product instead of the default URL -# PRODUCT_URL="https://www.amazon.com/dp/B075CYMYK6" +# Optional: the default is the Amazon product page. This value gives the +# practice page instead. +# PRODUCT_URL="https://appbrewery.github.io/instant_pot/" diff --git a/docs/artifact-registry.md b/docs/artifact-registry.md index 3b2f546..5f3b514 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 | 014 | +| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 015 | ## Languages diff --git a/docs/images/step-4-amazon-robot-check.png b/docs/images/step-4-amazon-robot-check.png new file mode 100644 index 0000000..93d7bd1 Binary files /dev/null and b/docs/images/step-4-amazon-robot-check.png differ diff --git a/docs/images/step-4-run.png b/docs/images/step-4-run.png new file mode 100644 index 0000000..ebf3aec Binary files /dev/null and b/docs/images/step-4-run.png differ diff --git a/docs/sqa/reviews/rc-014-code-mil-005.md b/docs/sqa/reviews/rc-014-code-mil-005.md new file mode 100644 index 0000000..2bf2bc2 --- /dev/null +++ b/docs/sqa/reviews/rc-014-code-mil-005.md @@ -0,0 +1,62 @@ +# RC-014 Code review of MIL-005 + +## Metadata +| Key | Value | +| --- | --- | +| ID | RC-014 | +| CrossReference | [MIL-005], [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 | [34e70b3] | + +--- + +## Artifact Under Review + +- Instance reviewed: [MIL-005] +- 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: 56 pytest tests (no network), 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: 56 pytest tests (no network), 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 logged; the invalid-setting error names the variable and never echoes the value; stderr messages only | +| 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: 56 pytest tests (no network), 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 | Selectors and markers are data in constants.py; detection is a separate function, so a new marker or selector is an extension, not an edit in several places | +| 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 | +| 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-005]: ../../milestones/mil-005-live-amazon.md +[QC-PY-001]: ../../../framework/qc/code/qc-programming-python.md +[34e70b3]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/047-amazon-price-tracker/commit/34e70b31dcfb9bfb72de0af9cea14491fe0b0d71 diff --git a/src/amazon_price_tracker/constants.py b/src/amazon_price_tracker/constants.py index a874669..7edf330 100644 --- a/src/amazon_price_tracker/constants.py +++ b/src/amazon_price_tracker/constants.py @@ -49,6 +49,22 @@ DEFAULT_HEADERS: Final[Mapping[str, str]] = MappingProxyType( } ) +## @brief CSS selectors for the price, tried in order. +# The first one skips decoy .a-offscreen spans (coupons) outside .a-price; +# the second serves the practice page. +PRICE_SELECTORS: Final[tuple[str, ...]] = (".a-price .a-offscreen", ".a-offscreen") + +## @brief CSS selectors for the product title, tried in order. +TITLE_SELECTORS: Final[tuple[str, ...]] = ("#productTitle",) + +## @brief Text found only on Amazon's robot-check page. +ROBOT_CHECK_MARKERS: Final[tuple[str, ...]] = ( + "/errors_page/validateCaptcha", + "Click the button below to continue shopping", + "Enter the characters you see below", + "api-services-support@amazon.com", +) + ## @brief SMTP submission port (STARTTLS). SMTP_PORT: Final[int] = 587 diff --git a/src/amazon_price_tracker/domain/errors.py b/src/amazon_price_tracker/domain/errors.py index d2e3eaf..7cfb8bb 100644 --- a/src/amazon_price_tracker/domain/errors.py +++ b/src/amazon_price_tracker/domain/errors.py @@ -17,5 +17,13 @@ class TitleNotFoundError(ProductParseError): """The page has no product title.""" +class RobotCheckError(ProductParseError): + """The page is a robot check, not a product page.""" + + class MissingSettingError(Exception): """A required setting is missing or empty.""" + + +class InvalidSettingError(Exception): + """A setting has a value that is not allowed.""" diff --git a/src/amazon_price_tracker/infrastructure/scraper.py b/src/amazon_price_tracker/infrastructure/scraper.py index 8971501..dfab64b 100644 --- a/src/amazon_price_tracker/infrastructure/scraper.py +++ b/src/amazon_price_tracker/infrastructure/scraper.py @@ -4,40 +4,80 @@ @brief Day 47 step 1: get the title and the price as a float. """ -from bs4 import BeautifulSoup +from bs4 import BeautifulSoup, Tag +from amazon_price_tracker.constants import ( + PRICE_SELECTORS, + ROBOT_CHECK_MARKERS, + TITLE_SELECTORS, +) from amazon_price_tracker.domain.errors import ( PriceNotFoundError, ProductParseError, + RobotCheckError, TitleNotFoundError, ) from amazon_price_tracker.domain.product import Product +def is_robot_check(html: str) -> bool: + """Tell whether html is Amazon's robot-check page. + + @param html Page HTML. + @return True when any robot-check marker is present. + """ + return any(marker in html for marker in ROBOT_CHECK_MARKERS) + + +def _ensure_not_robot_check(html: str) -> None: + if is_robot_check(html): + raise RobotCheckError( + "The page is a robot check, not a product page. " + "The bot does not try to get past it." + ) + + +def _select_first(html: str, selectors: tuple[str, ...]) -> Tag | None: + soup = BeautifulSoup(html, "html.parser") + for selector in selectors: + element = soup.select_one(selector) + if element is not None: + return element + return None + + def get_title(html: str) -> str: - """Return the text of the element with id productTitle, whitespace collapsed. + """Return the text of the product title element, whitespace collapsed. @param html Page HTML. @return The product title. + @throws RobotCheckError When the page is a robot check. @throws TitleNotFoundError When the element is missing. """ - element = BeautifulSoup(html, "html.parser").select_one("#productTitle") + _ensure_not_robot_check(html) + element = _select_first(html, TITLE_SELECTORS) if element is None: - raise TitleNotFoundError("No element with id 'productTitle' on the page") + raise TitleNotFoundError( + f"No element matching {', '.join(TITLE_SELECTORS)} on the page" + ) return " ".join(element.get_text().split()) def get_price(html: str) -> float: - """Return the price of the first element with class a-offscreen. + """Return the price of the first element matching a price selector. @param html Page HTML. @return The price as a floating point number. - @throws PriceNotFoundError When the element is missing. + @throws RobotCheckError When the page is a robot check. + @throws PriceNotFoundError When no element matches. @throws ProductParseError When the price text is not a number. """ - element = BeautifulSoup(html, "html.parser").select_one(".a-offscreen") + _ensure_not_robot_check(html) + element = _select_first(html, PRICE_SELECTORS) if element is None: - raise PriceNotFoundError("No element with class 'a-offscreen' on the page") + raise PriceNotFoundError( + f"No element matching {', '.join(PRICE_SELECTORS)} on the page" + ) text = element.get_text().strip() # Text looks like "$99.99": drop the currency sign, as the hint suggests. amount = text.split("$")[-1].replace(",", "") @@ -52,8 +92,10 @@ def parse_product(html: str) -> Product: @param html Page HTML. @return The product with title and price. + @throws RobotCheckError When the page is a robot check. @throws ProductParseError When the title or the price cannot be read. """ + _ensure_not_robot_check(html) return Product(title=get_title(html), price=get_price(html)) diff --git a/src/amazon_price_tracker/infrastructure/settings.py b/src/amazon_price_tracker/infrastructure/settings.py index b07af30..b4f5b13 100644 --- a/src/amazon_price_tracker/infrastructure/settings.py +++ b/src/amazon_price_tracker/infrastructure/settings.py @@ -8,11 +8,13 @@ from collections.abc import Mapping from dataclasses import dataclass, field from amazon_price_tracker.constants import ( + AMAZON_URL, ENV_EMAIL_ADDRESS, ENV_EMAIL_PASSWORD, + ENV_PRODUCT_URL, ENV_SMTP_ADDRESS, ) -from amazon_price_tracker.domain.errors import MissingSettingError +from amazon_price_tracker.domain.errors import InvalidSettingError, MissingSettingError @dataclass(frozen=True, slots=True) @@ -32,6 +34,22 @@ def _require(environ: Mapping[str, str], name: str) -> str: return value +def load_product_url(environ: Mapping[str, str]) -> str: + """Return the product URL from environ, or the Amazon default. + + @param environ Environment variables, for example os.environ. + @return The URL, stripped. + @throws InvalidSettingError When the value does not start with https://. + """ + value = environ.get(ENV_PRODUCT_URL, "").strip() + if not value: + return AMAZON_URL + if not value.startswith("https://"): + # The value is not echoed: it may be long or carry a token. + raise InvalidSettingError(f"Invalid setting: {ENV_PRODUCT_URL} must be https") + return value + + def load_mail_settings(environ: Mapping[str, str]) -> MailSettings: """Return the mail settings found in environ. diff --git a/src/amazon_price_tracker/main.py b/src/amazon_price_tracker/main.py index 9210817..3f04b50 100644 --- a/src/amazon_price_tracker/main.py +++ b/src/amazon_price_tracker/main.py @@ -10,12 +10,20 @@ import sys from dotenv import find_dotenv, load_dotenv from amazon_price_tracker.application.alert import check_price -from amazon_price_tracker.constants import PRACTICE_URL, TARGET_PRICE -from amazon_price_tracker.domain.errors import MissingSettingError +from amazon_price_tracker.constants import TARGET_PRICE +from amazon_price_tracker.domain.errors import ( + InvalidSettingError, + MissingSettingError, + ProductParseError, + RobotCheckError, +) from amazon_price_tracker.infrastructure.http_client import RequestsPageFetcher from amazon_price_tracker.infrastructure.mailer import SmtpNotifier from amazon_price_tracker.infrastructure.scraper import SoupProductParser -from amazon_price_tracker.infrastructure.settings import load_mail_settings +from amazon_price_tracker.infrastructure.settings import ( + load_mail_settings, + load_product_url, +) def main() -> None: @@ -23,13 +31,26 @@ def main() -> None: load_dotenv(find_dotenv(usecwd=True)) try: settings = load_mail_settings(os.environ) - except MissingSettingError as error: + url = load_product_url(os.environ) + except (MissingSettingError, InvalidSettingError) as error: print(f"{error}. Copy .env.example to .env and fill it in.", file=sys.stderr) return notifier = SmtpNotifier(settings) - result = check_price( - RequestsPageFetcher(), SoupProductParser(), notifier, PRACTICE_URL, TARGET_PRICE - ) + try: + result = check_price( + RequestsPageFetcher(), SoupProductParser(), notifier, url, TARGET_PRICE + ) + except RobotCheckError: + print( + "Amazon returned a robot check page, so there is no price to read. " + "Try again later or set PRODUCT_URL to another page. " + "The bot does not try to get past the check.", + file=sys.stderr, + ) + return + except ProductParseError as error: + print(f"Could not read the product: {error}", file=sys.stderr) + return print(result.product.title) print(result.product.price) print("Alert sent" if result.alert_sent else "Price is not below the target") diff --git a/tests/fixtures/amazon_live_page.html b/tests/fixtures/amazon_live_page.html new file mode 100644 index 0000000..4da33c2 --- /dev/null +++ b/tests/fixtures/amazon_live_page.html @@ -0,0 +1,10 @@ + + + +Amazon.com: Instant Pot + + Instant Pot Duo 7-in-1 +Save $5.00 with coupon +$79.95 + + diff --git a/tests/fixtures/amazon_robot_check.html b/tests/fixtures/amazon_robot_check.html new file mode 100644 index 0000000..b9b0788 --- /dev/null +++ b/tests/fixtures/amazon_robot_check.html @@ -0,0 +1,82 @@ + + + + + + + + + + +Amazon.com + + + + + + + + + +
+ +
+ +
+ +
+
+ +

Click the button below to continue shopping

+
+
+ +
+ +
+
+ +
+ + +
+ +
+ + + + + +
+ +
+
+ +
+
+ +
+ +
+ +
+ +
+ Conditions of Use + + + + + Privacy Policy +
+
+ © 1996-2025, Amazon.com, Inc. or its affiliates +
+
+ + diff --git a/tests/unit/infrastructure/test_scraper.py b/tests/unit/infrastructure/test_scraper.py index aa62e72..0290324 100644 --- a/tests/unit/infrastructure/test_scraper.py +++ b/tests/unit/infrastructure/test_scraper.py @@ -5,6 +5,7 @@ import pytest from amazon_price_tracker.domain.errors import ( PriceNotFoundError, ProductParseError, + RobotCheckError, TitleNotFoundError, ) from amazon_price_tracker.domain.product import Product @@ -12,6 +13,7 @@ from amazon_price_tracker.infrastructure.scraper import ( SoupProductParser, get_price, get_title, + is_robot_check, parse_product, ) @@ -60,6 +62,42 @@ def test_parse_product_returns_product() -> None: assert product.price == 99.99 +def test_is_robot_check_true_for_robot_check_page() -> None: + assert is_robot_check(_read("amazon_robot_check.html")) + + +@pytest.mark.parametrize("name", ["practice_page.html", "amazon_live_page.html"]) +def test_is_robot_check_false_for_product_page(name: str) -> None: + assert not is_robot_check(_read(name)) + + +def test_get_price_raises_robot_check_error_on_robot_check() -> None: + with pytest.raises(RobotCheckError): + get_price(_read("amazon_robot_check.html")) + + +def test_get_title_raises_robot_check_error_on_robot_check() -> None: + with pytest.raises(RobotCheckError): + get_title(_read("amazon_robot_check.html")) + + +def test_parse_product_raises_robot_check_error_on_robot_check() -> None: + with pytest.raises(RobotCheckError): + parse_product(_read("amazon_robot_check.html")) + + +def test_robot_check_error_is_a_product_parse_error() -> None: + assert issubclass(RobotCheckError, ProductParseError) + + +def test_get_price_skips_decoy_coupon_on_live_page() -> None: + assert get_price(_read("amazon_live_page.html")) == 79.95 + + +def test_get_title_strips_title_on_live_page() -> None: + assert get_title(_read("amazon_live_page.html")) == "Instant Pot Duo 7-in-1" + + def test_soup_product_parser_parses_html() -> None: product = SoupProductParser().parse(_read("practice_page.html")) diff --git a/tests/unit/infrastructure/test_settings.py b/tests/unit/infrastructure/test_settings.py index c21ec52..5b561e1 100644 --- a/tests/unit/infrastructure/test_settings.py +++ b/tests/unit/infrastructure/test_settings.py @@ -1,12 +1,18 @@ import pytest from amazon_price_tracker.constants import ( + AMAZON_URL, ENV_EMAIL_ADDRESS, ENV_EMAIL_PASSWORD, + ENV_PRODUCT_URL, ENV_SMTP_ADDRESS, + PRACTICE_URL, +) +from amazon_price_tracker.domain.errors import InvalidSettingError, MissingSettingError +from amazon_price_tracker.infrastructure.settings import ( + load_mail_settings, + load_product_url, ) -from amazon_price_tracker.domain.errors import MissingSettingError -from amazon_price_tracker.infrastructure.settings import load_mail_settings FULL = { ENV_SMTP_ADDRESS: "smtp.example.test", @@ -43,6 +49,30 @@ def test_load_mail_settings_names_first_missing_variable() -> None: load_mail_settings({}) +def test_load_product_url_defaults_to_amazon() -> None: + assert load_product_url({}) == AMAZON_URL + + +def test_load_product_url_defaults_when_blank() -> None: + assert load_product_url({ENV_PRODUCT_URL: " "}) == AMAZON_URL + + +def test_load_product_url_uses_override() -> None: + assert load_product_url({ENV_PRODUCT_URL: PRACTICE_URL}) == PRACTICE_URL + + +def test_load_product_url_strips_whitespace() -> None: + assert load_product_url({ENV_PRODUCT_URL: f" {PRACTICE_URL}\n"}) == PRACTICE_URL + + +@pytest.mark.parametrize("value", ["http://example.test/", "ftp://x", "example.test"]) +def test_load_product_url_rejects_non_https(value: str) -> None: + with pytest.raises(InvalidSettingError, match=ENV_PRODUCT_URL) as info: + load_product_url({ENV_PRODUCT_URL: value}) + + assert value not in str(info.value) + + def test_mail_settings_repr_hides_password() -> None: assert "s3cret-pw" not in repr(load_mail_settings(FULL)) diff --git a/tests/unit/test_constants.py b/tests/unit/test_constants.py index 837b56f..749acc2 100644 --- a/tests/unit/test_constants.py +++ b/tests/unit/test_constants.py @@ -42,6 +42,23 @@ def test_default_headers_are_non_empty() -> None: assert all(k.strip() and v.strip() for k, v in constants.DEFAULT_HEADERS.items()) +def test_selectors_and_markers_are_non_empty_tuples() -> None: + """Selectors and robot-check markers are non-empty tuples of text.""" + for group in ( + constants.PRICE_SELECTORS, + constants.TITLE_SELECTORS, + constants.ROBOT_CHECK_MARKERS, + ): + assert isinstance(group, tuple) + assert group + assert all(item.strip() for item in group) + + +def test_price_selectors_try_specific_before_generic() -> None: + """The a-price selector comes before the bare a-offscreen one.""" + assert constants.PRICE_SELECTORS == (".a-price .a-offscreen", ".a-offscreen") + + def test_smtp_port_is_starttls_port() -> None: """The SMTP port is 587.""" assert constants.SMTP_PORT == 587