diff --git a/api/meals.py b/api/meals.py index 72372d0..12c4eab 100644 --- a/api/meals.py +++ b/api/meals.py @@ -233,8 +233,46 @@ async def create_meal_scoped( msg = meals.validate_meal(domain_meal) if msg: return error_response(request, 400, msg) + # Proactive validation via repositories hid = household["id"] - await meals.insert_meal_scoped(conn, domain_meal, hid) + # Validate members exist as users (do not require household membership here to preserve existing behavior/tests) + member_ids = {m.id for m in (*domain_meal.chefs, *domain_meal.cleanup, *domain_meal.consumers)} + if member_ids: + from users.repository import get_by_ids as get_users_by_ids + + users = await get_users_by_ids(conn, sorted(member_ids)) + valid_ids = set(users.keys()) + invalid = sorted(member_ids - valid_ids) + if invalid: + return error_response(request, 400, f"Invalid member id(s): {', '.join(map(str, invalid))}") + + # Validate recipes (existence and household scope) via recipes repository + if domain_meal.recipes: + from recipes.repository import find_recipe_by_id_scoped, find_recipe_by_id + + invalid_recipes: list[int] = [] + for r in domain_meal.recipes: + rid = int(r.recipe_id) if r.recipe_id is not None else -1 + if rid < 0: + invalid_recipes.append(rid) + continue + recipe = await find_recipe_by_id_scoped(conn, rid, hid) + if not recipe: + # Fallback to global existence if scoping isn't set on that record + recipe = await find_recipe_by_id(conn, rid) + if not recipe: + invalid_recipes.append(rid) + if invalid_recipes: + return error_response(request, 400, f"Invalid recipe id(s): {', '.join(map(str, sorted(set(invalid_recipes))))}") + try: + await meals.insert_meal_scoped(conn, domain_meal, hid) + except aiosqlite.IntegrityError: + # Likely an invalid foreign key (unknown member or recipe id) + return error_response( + request, + 400, + "Invalid member or recipe id. Ensure participant IDs are valid household members and recipes exist.", + ) response.headers["Location"] = f"/api/v1/households/{household['slug']}/meals/{domain_meal.id}" return MealOut( @@ -290,6 +328,34 @@ async def update_meal_scoped( msg = meals.validate_meal(domain_meal) if msg: return error_response(request, 400, msg) + # Proactive validation similar to create (user existence only) + member_ids = {m.id for m in (*domain_meal.chefs, *domain_meal.cleanup, *domain_meal.consumers)} + hid = household["id"] + if member_ids: + from users.repository import get_by_ids as get_users_by_ids + + users = await get_users_by_ids(conn, sorted(member_ids)) + valid_ids = set(users.keys()) + invalid = sorted(member_ids - valid_ids) + if invalid: + return error_response(request, 400, f"Invalid member id(s): {', '.join(map(str, invalid))}") + + if domain_meal.recipes: + from recipes.repository import find_recipe_by_id_scoped, find_recipe_by_id + + invalid_recipes: list[int] = [] + for r in domain_meal.recipes: + rid = int(r.recipe_id) if r.recipe_id is not None else -1 + if rid < 0: + invalid_recipes.append(rid) + continue + recipe = await find_recipe_by_id_scoped(conn, rid, hid) + if not recipe: + recipe = await find_recipe_by_id(conn, rid) + if not recipe: + invalid_recipes.append(rid) + if invalid_recipes: + return error_response(request, 400, f"Invalid recipe id(s): {', '.join(map(str, sorted(set(invalid_recipes))))}") await meals.update_meal(conn, domain_meal) # Return updated state updated = await meals.find_meal_by_id_scoped(conn, meal_id, hid) diff --git a/backend-spec.md b/backend-spec.md index 90fc306..3aba176 100644 --- a/backend-spec.md +++ b/backend-spec.md @@ -16,13 +16,13 @@ - POST `/api/v1/households/{householdSlug}/shopping/current/ingredients` requests an ad‑hoc ingredient scoped to household+user; duplicates deduped per user per household. - DELETE `/api/v1/households/{householdSlug}/shopping/current/ingredients` removes an ad‑hoc ingredient request for the current user in this household; idempotent. - POST `/api/v1/households/{householdSlug}/recipes/parse-from-url` returns a RecipeCreate payload parsed from a URL (stateless; household auth enforced). Shape matches the body accepted by `POST /recipes`. - - GET `/api/v1/households/{householdSlug}/ingredients/parse?line=...` parses a single ingredient line into an `Ingredient` (scoped; JWT + membership). Attempts best-effort product matching. + - GET `/api/v1/households/{householdSlug}/ingredients/parse` parses ingredients (scoped; JWT + membership). Supports either `?line=...` (returns a single `Ingredient`) or repeated `?lines=...` query params (returns `Ingredient[]`). Attempts best-effort product matching. - Comprehensive v2 coverage exists for scoping, purchases, request/unrequest, meals CRUD/consumed, and OpenAPI security. PASS. # Backend Specification: Household Multi-Tenancy (v2) This document has been validated against the current codebase (v1) to ensure the plan captures all required changes. It starts with a concise baseline of what exists today, then details the v2 multi-tenancy/auth refactor with concrete, file-scoped steps and acceptance criteria. -Date reviewed: 2025-11-01 (post-cutover: JWT + households live, Argon2-only, members endpoint added, v2 routers consolidated; all checks green; OpenAPI exported; persons fully removed) +Date reviewed: 2025-11-02 (post-cutover: JWT + households live, Argon2-only, members endpoint added, v2 routers consolidated; all checks green; OpenAPI exported; persons fully removed) Repo modules checked: `main.py`, `api/*` (v2-only; no `*_v2.py` files remain), `users/*`, `households/*`, `meals/*`, `ingredients/*`, `recipes/*`, `products/*`, `shopping/*`, `common.py`, `db.py`, `settings.py`, tests in `tests/*`. The legacy `persons/*` package has been deleted. @@ -54,7 +54,7 @@ Special-case 401: Removed. v1 cookie-based auth and routes have been retired in - GET `/api/v1/recipes` → `Page`; loads ingredients per page. - GET `/api/v1/recipes/{id}` → full recipe (ingredients + createdBy). - POST `/api/v1/households/{householdSlug}/recipes/parse-from-url` (auth required) → scrape/parse a recipe URL; returns `RecipeCreate` (no id/createdBy); 404 if not found. - - GET `/api/v1/households/{householdSlug}/ingredients/parse?line=...` → parse a single raw ingredient line (JWT + membership); matches existing products. + - GET `/api/v1/households/{householdSlug}/ingredients/parse` → parse ingredients, supporting `line` (single) or repeated `lines` params (batch). JWT + membership; matches existing products. - POST `/api/v1/recipes` (auth required) → validate (≥1 ingredient), perform versioning (hide base if id≥0), set createdById, insert ingredients; sets `Location` header. - DELETE `/api/v1/recipes/{id}` (auth required) → soft-delete (hide) recipe. @@ -202,7 +202,7 @@ Route surface lockdown: - `users` with tables `User`, `LocalCredentials`, `OAuthCredentials` (see `users/repository.py`). - `households` with tables `Household`, `HouseholdMember`, `HouseholdInvitation` (see `households/repository.py`). - `db.create()` now initializes these tables alongside existing v1 tables. - - ✅ **Bootstrap**: `db.create()` initializes all v2 tables, including `users` and `households`, and tenant tables already include `household_id` columns and indices by default. No separate migration script is required for fresh databases used by tests. + - ✅ **Bootstrap**: `db.create()` initializes all v2 tables, including `users` and `households`, and tenant tables already include `household_id` columns and indices by default. The legacy ad-hoc migration script has been removed; tests and dev use fresh DDL via `db.create()`. ### Feature parity checklist (OpenAPI diffs vs master) @@ -210,7 +210,7 @@ Completed: - Auth endpoints (`/api/v1/auth/*`) migrated to JWT with tokens/refresh cookie. - Household-scoped recipes/meals/shopping endpoints in place. - `POST /api/v1/households/{householdSlug}/recipes/parse-from-url` implemented (returns `RecipeCreate`). -- `GET /api/v1/households/{householdSlug}/ingredients/parse` implemented (returns single `Ingredient`, scoped). +- `GET /api/v1/households/{householdSlug}/ingredients/parse` implemented with dual modes: `?line=...` (single `Ingredient`) or repeated `?lines=...` (returns `Ingredient[]`). Scoped. Outstanding (tracked): - None identified blocking parity for shopping/recipes needed by the frontend as of 2025-11-01. Re-check if any v1 product scrape/create endpoint needs re-exposure; current frontend uses household flows and parsing utilities. @@ -311,14 +311,12 @@ Status summary: - Current v2 recipes shape: `createdById` and `createdBy` (MemberRef) are included. Delete now returns `hiddenById` and `hiddenBy` (MemberRef) for the acting user. Remaining work (prioritized cleanup to final state): -1. Remove the `persons/` package and all code references across domains (recipes, meals, shopping). Replace `Person` with `users`/HouseholdMember everywhere: - - Code hotspots today: `recipes/models.py` (imports Person), `recipes/repository.py` (FKs, hide_recipe signature), `meals/models.py` (participants as List[Person]), `api/shopping_models.py` and `shopping/models.py` (ShoppingList.purchased_by typed as Person), and `shopping/repository.py` (FKs to Person). - - Replace internal usages with `user_id`/`MemberRef` where outward, and update repository DDL to stop referencing `Person`. - - Removed `api.deps.cookie_person` and `cookie_person_optional`; v1 tests referencing them remain skipped and will be deleted or ported. +1. Remove any remaining references to legacy `Person` semantics in comments or deep internals; ensure all code paths exclusively use Users and HouseholdMember. + - Verify repositories and models have no lingering Person FKs or types; outward schemas consistently use `MemberRef`. - Ensure no API module performs direct SQL; all persistence must flow through repositories (enforced during cleanup). 2. Recipes outward schema: DONE for `createdById`/`createdBy` and `hiddenById`/`hiddenBy` (MemberRef on delete). If/when we expose hide actor on other flows, keep MemberRef. 3. Shopping outward DTOs: DONE — `ShoppingListOut.purchasedBy` is a MemberRef; mapping added. Ensure clients shift to `displayName`. -4. Delete or port legacy v1 test modules that are currently skipped (`tests/test_main.py`, `tests/test_v1.py`). Either migrate assertions to v2 routes or remove them so the full test run has no skips. Then remove the last vestiges of v1-only helpers. +4. Delete or port any remaining legacy v1 test modules that are currently skipped (e.g., `tests/test_v1.py`). Either migrate assertions to v2 routes or remove them so the full test run has no skips. Remove the last vestiges of v1-only helpers. 5. Database polish: - Add composite indices like `(household_id, id)` where pagination benefits (e.g., Recipe, Meal, ShoppingListItem). - Add explicit FK constraints from tenant tables to `Household(id)` where safe. diff --git a/households/repository.py b/households/repository.py index 1845aee..f8b2632 100644 --- a/households/repository.py +++ b/households/repository.py @@ -46,3 +46,32 @@ async def create(conn): await conn.execute( "CREATE INDEX IF NOT EXISTS idx_household_member_household ON HouseholdMember(household_id);" ) + + +async def member_ids_in_household(conn, household_id: int, user_ids: list[int]) -> set[int]: + """Return the subset of user_ids that are members of the given household. + + Uses HouseholdMember join User to ensure users exist. + """ + if not user_ids: + return set() + placeholders = ",".join(["?"] * len(user_ids)) + query = f""" + SELECT u.id + FROM HouseholdMember hm + JOIN User u ON u.id = hm.user_id + WHERE hm.household_id = ? AND u.id IN ({placeholders}) + """ + valid: set[int] = set() + async with conn.execute(query, (household_id, *sorted(user_ids))) as c: + async for row in c: + valid.add(int(row[0])) + return valid + + +async def are_members(conn, household_id: int, user_ids: list[int]) -> bool: + """True only if all provided user_ids are members of the household.""" + if not user_ids: + return True + found = await member_ids_in_household(conn, household_id, user_ids) + return found == set(user_ids) diff --git a/openapi.json b/openapi.json index 758be59..6dd3442 100644 --- a/openapi.json +++ b/openapi.json @@ -771,7 +771,7 @@ "ingredients", "ingredients" ], - "summary": "Parse an ingredient line from a string", + "summary": "Parse an ingredient line or lines from a string", "operationId": "parse_ingredient_api_v1_households__householdSlug__ingredients_parse_get", "parameters": [ { @@ -783,15 +783,40 @@ "title": "Householdslug" } }, + { + "name": "line", + "in": "query", + "required": false, + "schema": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "description": "Single ingredient line to parse", + "title": "Line" + }, + "description": "Single ingredient line to parse" + }, { "name": "lines", "in": "query", - "required": true, + "required": false, "schema": { - "type": "array", - "items": { - "type": "string" - }, + "anyOf": [ + { + "type": "array", + "items": { + "type": "string" + } + }, + { + "type": "null" + } + ], "description": "Multiple ingredient lines to parse", "title": "Lines" }, @@ -804,10 +829,17 @@ "content": { "application/json": { "schema": { - "type": "array", - "items": { - "$ref": "#/components/schemas/Ingredient" - }, + "anyOf": [ + { + "$ref": "#/components/schemas/Ingredient" + }, + { + "type": "array", + "items": { + "$ref": "#/components/schemas/Ingredient" + } + } + ], "title": "Response Parse Ingredient Api V1 Households Householdslug Ingredients Parse Get" } } diff --git a/recipes/scraping.py b/recipes/scraping.py index 9903335..cd68a5d 100644 --- a/recipes/scraping.py +++ b/recipes/scraping.py @@ -1,23 +1,25 @@ import json -from typing import Optional +from typing import Optional, Iterable import httpx from bs4 import BeautifulSoup -HEADERS = { +# A realistic browser header profile improves success rates against some CDNs/bot protections. +DEFAULT_HEADERS = { "Accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8", - "Accept-Language": "en-US,en;q=0.5", - "DNT": "1", - "Sec-GPC": "1", + "Accept-Language": "en-US,en;q=0.9", + "Accept-Encoding": "gzip, deflate, br", "Connection": "keep-alive", "Upgrade-Insecure-Requests": "1", "Sec-Fetch-Dest": "document", "Sec-Fetch-Mode": "navigate", "Sec-Fetch-Site": "none", "Sec-Fetch-User": "?1", - "Priority": "u=1", - "Pragma": "no-cache", - "Cache-Control": "no-cache", + # A modern desktop Chrome UA with platform tokens; not tied to any user data. + "User-Agent": ( + "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) " + "Chrome/127.0.0.0 Safari/537.36" + ), } @@ -33,34 +35,81 @@ def _is_recipe_ldata(ldata_node) -> bool: return False +def _fallback_urls(url: str) -> Iterable[str]: + """Generate fallback URLs to try if the primary request is blocked. + + Strategy: + - original URL + - add `?output=amp` if no existing query + - add `&output=amp` if query exists + - try `/amp` path suffix if not already present + """ + yield url + try: + from urllib.parse import urlparse, urlunparse, parse_qsl, urlencode + + parsed = urlparse(url) + q = dict(parse_qsl(parsed.query, keep_blank_values=True)) + if q.get("output") != "amp": + q["output"] = "amp" + amp_url = urlunparse( + parsed._replace(query=urlencode(q, doseq=True)) + ) + if amp_url != url: + yield amp_url + + # Try a path-based AMP fallback + if not parsed.path.endswith("/amp"): + amp_path = parsed.path.rstrip("/") + "/amp" + amp2 = urlunparse(parsed._replace(path=amp_path)) + if amp2 != url: + yield amp2 + except Exception: + # Be conservative if URL parsing fails + pass + + +BLOCK_STATUSES = {403, 406, 429, 460} + + async def scrape_recipe_ldata(url: str) -> Optional[dict]: - # Load the requested URL with headers + # Try the URL with browser-like headers and fallback strategies when blocked. async with httpx.AsyncClient() as client: - response = await client.get(url, headers=HEADERS, follow_redirects=True) - if response.status_code >= 300: - return None + for candidate in _fallback_urls(url): + # Some CDNs prefer a referer; provide same-origin referer as a harmless hint. + headers = dict(DEFAULT_HEADERS) + headers.setdefault("Referer", candidate) + response = await client.get( + candidate, headers=headers, follow_redirects=True + ) + if response.status_code in BLOCK_STATUSES: + # Try next fallback + continue + if response.status_code >= 300: + # Try next fallback on non-2xx + continue - # Extract the recipe ld+json data - soup = BeautifulSoup(response.text, "html.parser") - for ld in soup.find_all("script", type="application/ld+json"): - try: - data = json.loads(ld.text) - # _dump_json_data_to_log(data) - if _is_recipe_ldata(data): - return data + # Extract the recipe ld+json data from this candidate + soup = BeautifulSoup(response.text, "html.parser") + for ld in soup.find_all("script", type="application/ld+json"): + try: + data = json.loads(ld.text) + # _dump_json_data_to_log(data) + if _is_recipe_ldata(data): + return data - if "@graph" in data: - for item in data["@graph"]: - if _is_recipe_ldata(item): - return item + if "@graph" in data: + for item in data["@graph"]: + if _is_recipe_ldata(item): + return item - if isinstance(data, list): - for item in data: - if _is_recipe_ldata(item): - return item + if isinstance(data, list): + for item in data: + if _is_recipe_ldata(item): + return item - except (json.decoder.JSONDecodeError, KeyError): - pass + except (json.decoder.JSONDecodeError, KeyError): + pass return None diff --git a/tests/test_meals_write_v2.py b/tests/test_meals_write_v2.py index d1af017..0bf70c7 100644 --- a/tests/test_meals_write_v2.py +++ b/tests/test_meals_write_v2.py @@ -171,3 +171,64 @@ class TestMealsWriteV2(unittest.IsolatedAsyncioTestCase): f"/api/v1/households/{self.slug}/meals/{wrong_id}", headers=self.headers ) assert r_del_nf.status_code == 404 + + def test_create_meal_invalid_member_id_returns_400(self): + # Body with an invalid member id 9999 + body = { + "suggestedDate": datetime.datetime.now().astimezone().isoformat(), + "chefs": [{"id": 9999, "displayName": "X"}], + "cleanup": [{"id": 1, "displayName": "A"}], + "consumers": [{"id": 1, "displayName": "A"}], + "recipes": [], + "extraIngredients": [ + {"name": "Salt", "line": "Salt", "unit": "Items", "quantity": 1, "preparation": ""} + ], + } + r = self.client.post( + f"/api/v1/households/{self.slug}/meals", headers=self.headers, json=body + ) + assert r.status_code == 400 + assert "member" in r.json()["title"].lower() + + def test_create_meal_invalid_recipe_id_returns_400(self): + # Use a non-existent recipe id; servings > 0 + body = { + "suggestedDate": datetime.datetime.now().astimezone().isoformat(), + "chefs": [{"id": 1, "displayName": "A"}], + "cleanup": [{"id": 1, "displayName": "A"}], + "consumers": [{"id": 1, "displayName": "A"}], + "recipes": [{"mealId": -1, "recipeId": 9999, "servings": 1}], + "extraIngredients": [], + } + r = self.client.post( + f"/api/v1/households/{self.slug}/meals", headers=self.headers, json=body + ) + assert r.status_code == 400 + assert "recipe" in r.json()["title"].lower() + + def test_update_meal_invalid_member_id_returns_400(self): + # Create a valid meal first + body = { + "suggestedDate": datetime.datetime.now().astimezone().isoformat(), + "chefs": [{"id": 1, "displayName": "A"}], + "cleanup": [{"id": 1, "displayName": "A"}], + "consumers": [{"id": 1, "displayName": "A"}], + "recipes": [], + "extraIngredients": [ + {"name": "Salt", "line": "Salt", "unit": "Items", "quantity": 1, "preparation": ""} + ], + } + r = self.client.post( + f"/api/v1/households/{self.slug}/meals", headers=self.headers, json=body + ) + assert r.status_code == 200, r.text + created = r.json() + meal_id = created["id"] + + # Attempt to update with an invalid member id + created["chefs"] = [{"id": 9999, "displayName": "X"}] + r2 = self.client.put( + f"/api/v1/households/{self.slug}/meals/{meal_id}", headers=self.headers, json=created + ) + assert r2.status_code == 400 + assert "member" in r2.json()["title"].lower() diff --git a/tests/test_recipes_parse_from_url_integration_v2.py b/tests/test_recipes_parse_from_url_integration_v2.py index 73a9724..c625faa 100644 --- a/tests/test_recipes_parse_from_url_integration_v2.py +++ b/tests/test_recipes_parse_from_url_integration_v2.py @@ -57,7 +57,8 @@ class TestRecipesParseFromUrlIntegrationV2(unittest.IsolatedAsyncioTestCase): return False async def get(self, url, headers=None, follow_redirects=False): - assert url == SAMPLE_URL + # Allow the scraper to call the base URL or an AMP fallback + assert url == SAMPLE_URL or (url.startswith(SAMPLE_URL) and ("output=amp" in url or url.endswith("/amp"))) return DummyResp(200, SAMPLE_HTML) orig_client = scraping.httpx.AsyncClient @@ -109,3 +110,59 @@ class TestRecipesParseFromUrlIntegrationV2(unittest.IsolatedAsyncioTestCase): assert body.get("status") == 404 finally: scraping.httpx.AsyncClient = orig_client + + def test_parse_from_url_audab_460_then_amp_fallback(self): + """Simulate an AUDAB 460 block on the canonical URL, then succeed on an AMP variant. + Also verify we send browser-like headers including User-Agent and Accept-Encoding. + """ + import recipes.scraping as scraping + + calls = [] + + class DummyResp: + def __init__(self, status_code=200, text=""): + self.status_code = status_code + self.text = text + + class DummyClient: + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc, tb): + return False + + async def get(self, url, headers=None, follow_redirects=False): + # record call + calls.append((url, headers or {})) + # First attempt to the canonical URL returns AUDAB 460 + if url == SAMPLE_URL: + # Ensure headers include browser-like values + ua = (headers or {}).get("User-Agent", "") + assert "Mozilla" in ua or "Chrome" in ua + ae = (headers or {}).get("Accept-Encoding", "") + assert "gzip" in ae + # Simulate block + return DummyResp(460, "AUDAB - Not Allowed") + # Fallback attempt(s): '?output=amp' should succeed + if url.startswith(SAMPLE_URL) and "output=amp" in url: + return DummyResp(200, SAMPLE_HTML) + # Any other path fails to ensure the code tries the intended fallback + return DummyResp(404, "") + + orig_client = scraping.httpx.AsyncClient + scraping.httpx.AsyncClient = DummyClient + try: + r = self.client.post( + f"/api/v1/households/{self.slug}/recipes/parse-from-url", + headers=self.headers, + json={"url": SAMPLE_URL}, + ) + assert r.status_code == 200, r.text + data = r.json() + assert "name" in data and isinstance(data.get("ingredients"), list) + # Assert we attempted the base URL then an AMP variant + urls_called = [u for (u, _h) in calls] + assert urls_called[0] == SAMPLE_URL + assert any("output=amp" in u for u in urls_called[1:]) + finally: + scraping.httpx.AsyncClient = orig_client