Tighten api endpoints

api/deps.py

@@ -1,9 +1,9 @@
 from __future__ import annotations
 
-from typing import Annotated, AsyncGenerator, Optional
+from typing import AsyncGenerator, Optional
 
 import aiosqlite
-from fastapi import Cookie, Depends, Request, HTTPException
+from fastapi import Cookie, Depends, HTTPException, Request
 from fastapi.responses import JSONResponse
 
 import db

api/shopping.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
-from typing import Dict, List
+from datetime import datetime
+from typing import Dict, List, Literal
 
 import aiosqlite
 from fastapi import APIRouter, Depends, Request, Response
@@ -10,23 +11,103 @@ import meals
 import persons
 import recipes
 import shopping
+from shopping.models import StoreEnum
 from api.deps import cookie_person, error_response, get_db
 from common import ApiModel, Field, ProblemDetails
 
+
+# Outward-facing models to reduce unnecessary nulls in API responses
+class ListIngredientItem(ApiModel):
+    kind: Literal["ingredient"] = "ingredient"
+    id: int = -1
+    ingredient_id: int
+    person_id: int
+    created_date: datetime
+    # These may be present when the ingredient is part of a requested meal
+    list_id: int | None = None
+    meal_id: int | None = None
+    recipe_id: int | None = None
+
+
+class RequestedMealItem(ApiModel):
+    kind: Literal["requestedMeal"] = "requestedMeal"
+    id: int = -1
+    person_id: int
+    meal_id: int
+    created_date: datetime
+
+
+# Input DTOs (separate from internal DB/domain models)
+class IngredientPurchaseItemIn(ApiModel):
+    ingredient_id: int
+    person_id: int
+    created_date: datetime | None = None
+    meal_id: int | None = None
+    recipe_id: int | None = None
+
+
+class PurchaseListIn(ApiModel):
+    store_name: StoreEnum
+    items: List[IngredientPurchaseItemIn]
+
+
+# Output DTOs for purchased lists
+class ShoppingListOut(ApiModel):
+    id: int
+    created_date: datetime
+    store_name: StoreEnum
+    purchased_by_id: int
+    purchased_by: persons.Person | None = None
+    items: List[ListIngredientItem] = Field(default_factory=list)
+
+
 router = APIRouter(prefix="/shopping", tags=["shopping"])
 
 
 class CurrentShoppingList(ApiModel):
-    outstanding_items: List[shopping.ShoppingListItem]
+    outstanding_items: List[ListIngredientItem]
-    requested_meals: List[shopping.ShoppingListItem]
+    requested_meals: List[RequestedMealItem]
-    purchased_items: List[shopping.ShoppingListItem] = Field(default_factory=list)
+    purchased_items: List[ListIngredientItem] = Field(default_factory=list)
 
     ingredients_lookup: Dict[int, ingredients.Ingredient] = Field(default_factory=dict)
     meals_lookup: Dict[int, meals.Meal] = Field(default_factory=dict)
-    shopping_list_lookup: Dict[int, shopping.ShoppingList] = Field(default_factory=dict)
+    shopping_list_lookup: Dict[int, ShoppingListOut] = Field(default_factory=dict)
     recipes_lookup: Dict[int, recipes.Recipe] = Field(default_factory=dict)
 
 
+# Mapping helpers from domain -> outward API
+def _to_ingredient_item(item: shopping.ShoppingListItem) -> ListIngredientItem:
+    return ListIngredientItem(
+        id=item.id,
+        ingredient_id=item.ingredient_id if item.ingredient_id is not None else -1,
+        person_id=item.person_id,
+        created_date=item.created_date,
+        list_id=item.list_id,
+        meal_id=item.meal_id,
+        recipe_id=item.recipe_id,
+    )
+
+
+def _to_meal_item(item: shopping.ShoppingListItem) -> RequestedMealItem:
+    return RequestedMealItem(
+        id=item.id,
+        person_id=item.person_id,
+        meal_id=item.meal_id if item.meal_id is not None else -1,
+        created_date=item.created_date,
+    )
+
+
+def _to_shopping_list_out(sl: shopping.ShoppingList) -> ShoppingListOut:
+    return ShoppingListOut(
+        id=sl.id,
+        created_date=sl.created_date,
+        store_name=sl.store_name,
+        purchased_by_id=sl.purchased_by_id,
+        purchased_by=sl.purchased_by,
+        items=[_to_ingredient_item(i) for i in sl.items],
+    )
+
+
 @router.get(
     "/current",
     response_model=CurrentShoppingList,
@@ -46,24 +127,30 @@ async def get_current_shopping_list(
     ) = await shopping.get_outstanding_requests(conn)
     other_shopping_list_ids = {item.list_id for item in purchased_requests}
 
-    shopping_list_lookup = {}
+    # Load full lists for additional lookups
+    other_lists_domain: Dict[int, shopping.ShoppingList] = {}
     for list_id in other_shopping_list_ids:
         if list_id is not None:
             sl = await shopping.load_shopping_list(conn, list_id)
             if sl is not None:
-                shopping_list_lookup[list_id] = sl
+                other_lists_domain[list_id] = sl
 
     # Add any additional items from shopping lists to the existing lookups
-    additional_items = [item for sl in shopping_list_lookup.values() for item in sl.items]
+    additional_items = [item for sl in other_lists_domain.values() for item in sl.items]
     if additional_items:
         await shopping.to_lookups(
             conn, additional_items, meals_lookup, recipes_lookup, ingredients_lookup
         )
 
+    # Convert domain shopping lists to outward form for response
+    shopping_list_lookup: Dict[int, ShoppingListOut] = {
+        k: _to_shopping_list_out(v) for k, v in other_lists_domain.items()
+    }
+
     return CurrentShoppingList(
-        outstanding_items=outstanding_requests,
+        outstanding_items=[_to_ingredient_item(i) for i in outstanding_requests],
-        requested_meals=meal_requests,
+        requested_meals=[_to_meal_item(i) for i in meal_requests],
-        purchased_items=purchased_requests,
+        purchased_items=[_to_ingredient_item(i) for i in purchased_requests],
         meals_lookup=meals_lookup,
         shopping_list_lookup=shopping_list_lookup,
         ingredients_lookup=ingredients_lookup,
@@ -72,7 +159,7 @@ async def get_current_shopping_list(
 
 
 class PurchasedShoppingList(ApiModel):
-    list: shopping.ShoppingList
+    list: ShoppingListOut
     meals_lookup: Dict[int, meals.Meal] = Field(default_factory=dict)
     ingredients_lookup: Dict[int, ingredients.Ingredient] = Field(default_factory=dict)
     recipes_lookup: Dict[int, recipes.Recipe] = Field(default_factory=dict)
@@ -102,7 +189,7 @@ async def get_shopping_list(
         conn, shopping_list.items
     )
     return PurchasedShoppingList(
-        list=shopping_list,
+        list=_to_shopping_list_out(shopping_list),
         meals_lookup=meals_lookup,
         recipes_lookup=recipes_lookup,
         ingredients_lookup=ingredients_lookup,
@@ -128,7 +215,7 @@ async def get_shopping_list(
     },
 )
 async def purchase_ingredients(
-    shopping_list: shopping.ShoppingList,
+    shopping_list: PurchaseListIn,
     request: Request,
     conn: aiosqlite.Connection = Depends(get_db),
     person: persons.Person = Depends(cookie_person),
@@ -137,19 +224,32 @@ async def purchase_ingredients(
     if not person:
         return error_response(request, 401, "Unauthorized")
 
-    # Attach purchaser to ensure purchased_by_id is set via BaseLinkedModel
+    # Map outward input DTO to domain model
-    shopping_list = shopping.ShoppingList(
+    domain_items: List[shopping.ShoppingListItem] = []
-        purchased_by=person, items=shopping_list.items, store_name=shopping_list.store_name
+    for it in shopping_list.items:
+        created = it.created_date or datetime.now().astimezone()
+        domain_items.append(
+            shopping.ShoppingListItem(
+                ingredient_id=it.ingredient_id,
+                person_id=it.person_id,
+                meal_id=it.meal_id,
+                recipe_id=it.recipe_id,
+                created_date=created,
+            )
+        )
+
+    domain_list = shopping.ShoppingList(
+        purchased_by=person, items=domain_items, store_name=shopping_list.store_name
     )
     try:
-        await shopping.purchase(conn, shopping_list)
+        await shopping.purchase(conn, domain_list)
     except ValueError as e:
         # Map domain validation errors to a proper Problem Details response
         return error_response(request, 400, str(e))
-    result = PurchasedShoppingList(list=shopping_list)
+    result = PurchasedShoppingList(list=_to_shopping_list_out(domain_list))
     await shopping.to_lookups(
         conn,
-        shopping_list.items,
+        domain_list.items,
         result.meals_lookup,
         result.recipes_lookup,
         result.ingredients_lookup,
@@ -202,7 +302,7 @@ class MealIdWrapper(ApiModel):
 
 @router.post(
     "/current/meals/me",
-    response_model=shopping.ShoppingListItem,
+    response_model=RequestedMealItem,
     operation_id="requestMeal",
     summary="Request a meal for shopping",
     responses={
@@ -218,13 +318,13 @@ async def request_meal(
     request: Request,
     conn: aiosqlite.Connection = Depends(get_db),
     person: persons.Person = Depends(cookie_person),
-) -> shopping.ShoppingListItem | Response:
+) -> RequestedMealItem | Response:
     meal = await meals.find_meal_by_id(conn, r.meal_id)
     if not meal:
         return error_response(request, 404, "Meal not found")
 
     response = await shopping.request(conn, person, meal=meal)
-    return response
+    return _to_meal_item(response)
 
 
 class Ok(ApiModel):

openapi.json

@@ -859,7 +859,7 @@
           "content": {
             "application/json": {
               "schema": {
-                "$ref": "#/components/schemas/ShoppingList"
+                "$ref": "#/components/schemas/PurchaseListIn"
               }
             }
           }
@@ -1058,7 +1058,7 @@
             "content": {
               "application/json": {
                 "schema": {
-                  "$ref": "#/components/schemas/ShoppingListItem"
+                  "$ref": "#/components/schemas/RequestedMealItem"
                 }
               }
             }
@@ -1397,21 +1397,21 @@
         "properties": {
           "outstandingItems": {
             "items": {
-              "$ref": "#/components/schemas/ShoppingListItem"
+              "$ref": "#/components/schemas/ListIngredientItem"
             },
             "type": "array",
             "title": "Outstandingitems"
           },
           "requestedMeals": {
             "items": {
-              "$ref": "#/components/schemas/ShoppingListItem"
+              "$ref": "#/components/schemas/RequestedMealItem"
             },
             "type": "array",
             "title": "Requestedmeals"
           },
           "purchasedItems": {
             "items": {
-              "$ref": "#/components/schemas/ShoppingListItem"
+              "$ref": "#/components/schemas/ListIngredientItem"
             },
             "type": "array",
             "title": "Purchaseditems"
@@ -1432,7 +1432,7 @@
           },
           "shoppingListLookup": {
             "additionalProperties": {
-              "$ref": "#/components/schemas/ShoppingList"
+              "$ref": "#/components/schemas/ShoppingListOut"
             },
             "type": "object",
             "title": "Shoppinglistlookup"
@@ -1575,6 +1575,129 @@
         ],
         "title": "Ingredient"
       },
+      "IngredientPurchaseItemIn": {
+        "properties": {
+          "ingredientId": {
+            "type": "integer",
+            "title": "Ingredientid"
+          },
+          "personId": {
+            "type": "integer",
+            "title": "Personid"
+          },
+          "createdDate": {
+            "anyOf": [
+              {
+                "type": "string",
+                "format": "date-time"
+              },
+              {
+                "type": "null"
+              }
+            ],
+            "title": "Createddate"
+          },
+          "mealId": {
+            "anyOf": [
+              {
+                "type": "integer"
+              },
+              {
+                "type": "null"
+              }
+            ],
+            "title": "Mealid"
+          },
+          "recipeId": {
+            "anyOf": [
+              {
+                "type": "integer"
+              },
+              {
+                "type": "null"
+              }
+            ],
+            "title": "Recipeid"
+          }
+        },
+        "type": "object",
+        "required": [
+          "ingredientId",
+          "personId"
+        ],
+        "title": "IngredientPurchaseItemIn"
+      },
+      "ListIngredientItem": {
+        "properties": {
+          "kind": {
+            "type": "string",
+            "enum": [
+              "ingredient"
+            ],
+            "const": "ingredient",
+            "title": "Kind",
+            "default": "ingredient"
+          },
+          "id": {
+            "type": "integer",
+            "title": "Id",
+            "default": -1
+          },
+          "ingredientId": {
+            "type": "integer",
+            "title": "Ingredientid"
+          },
+          "personId": {
+            "type": "integer",
+            "title": "Personid"
+          },
+          "createdDate": {
+            "type": "string",
+            "format": "date-time",
+            "title": "Createddate"
+          },
+          "listId": {
+            "anyOf": [
+              {
+                "type": "integer"
+              },
+              {
+                "type": "null"
+              }
+            ],
+            "title": "Listid"
+          },
+          "mealId": {
+            "anyOf": [
+              {
+                "type": "integer"
+              },
+              {
+                "type": "null"
+              }
+            ],
+            "title": "Mealid"
+          },
+          "recipeId": {
+            "anyOf": [
+              {
+                "type": "integer"
+              },
+              {
+                "type": "null"
+              }
+            ],
+            "title": "Recipeid"
+          }
+        },
+        "type": "object",
+        "required": [
+          "ingredientId",
+          "personId",
+          "createdDate"
+        ],
+        "title": "ListIngredientItem"
+      },
       "LoginBody": {
         "properties": {
           "username": {
@@ -2069,10 +2192,30 @@
         ],
         "title": "ProductUrl"
       },
+      "PurchaseListIn": {
+        "properties": {
+          "storeName": {
+            "$ref": "#/components/schemas/StoreEnum"
+          },
+          "items": {
+            "items": {
+              "$ref": "#/components/schemas/IngredientPurchaseItemIn"
+            },
+            "type": "array",
+            "title": "Items"
+          }
+        },
+        "type": "object",
+        "required": [
+          "storeName",
+          "items"
+        ],
+        "title": "PurchaseListIn"
+      },
       "PurchasedShoppingList": {
         "properties": {
           "list": {
-            "$ref": "#/components/schemas/ShoppingList"
+            "$ref": "#/components/schemas/ShoppingListOut"
           },
           "mealsLookup": {
             "additionalProperties": {
@@ -2314,26 +2457,61 @@
         ],
         "title": "Recipe"
       },
-      "ShoppingList": {
+      "RequestedMealItem": {
         "properties": {
+          "kind": {
+            "type": "string",
+            "enum": [
+              "requestedMeal"
+            ],
+            "const": "requestedMeal",
+            "title": "Kind",
+            "default": "requestedMeal"
+          },
           "id": {
             "type": "integer",
             "title": "Id",
             "default": -1
           },
+          "personId": {
+            "type": "integer",
+            "title": "Personid"
+          },
+          "mealId": {
+            "type": "integer",
+            "title": "Mealid"
+          },
+          "createdDate": {
+            "type": "string",
+            "format": "date-time",
+            "title": "Createddate"
+          }
+        },
+        "type": "object",
+        "required": [
+          "personId",
+          "mealId",
+          "createdDate"
+        ],
+        "title": "RequestedMealItem"
+      },
+      "ShoppingListOut": {
+        "properties": {
+          "id": {
+            "type": "integer",
+            "title": "Id"
+          },
           "createdDate": {
             "type": "string",
             "format": "date-time",
             "title": "Createddate"
           },
           "storeName": {
-            "$ref": "#/components/schemas/StoreEnum",
+            "$ref": "#/components/schemas/StoreEnum"
-            "default": ""
           },
           "purchasedById": {
             "type": "integer",
-            "title": "Purchasedbyid",
+            "title": "Purchasedbyid"
-            "default": -1
           },
           "purchasedBy": {
             "anyOf": [
@@ -2347,79 +2525,20 @@
           },
           "items": {
             "items": {
-              "$ref": "#/components/schemas/ShoppingListItem"
+              "$ref": "#/components/schemas/ListIngredientItem"
             },
             "type": "array",
             "title": "Items"
           }
         },
         "type": "object",
-        "title": "ShoppingList"
+        "required": [
-      },
+          "id",
-      "ShoppingListItem": {
+          "createdDate",
-        "properties": {
+          "storeName",
-          "id": {
+          "purchasedById"
-            "type": "integer",
-            "title": "Id",
-            "default": -1
-          },
-          "listId": {
-            "anyOf": [
-              {
-                "type": "integer"
-              },
-              {
-                "type": "null"
-              }
         ],
-            "title": "Listid"
+        "title": "ShoppingListOut"
-          },
-          "personId": {
-            "type": "integer",
-            "title": "Personid",
-            "default": -1
-          },
-          "ingredientId": {
-            "anyOf": [
-              {
-                "type": "integer"
-              },
-              {
-                "type": "null"
-              }
-            ],
-            "title": "Ingredientid"
-          },
-          "recipeId": {
-            "anyOf": [
-              {
-                "type": "integer"
-              },
-              {
-                "type": "null"
-              }
-            ],
-            "title": "Recipeid"
-          },
-          "mealId": {
-            "anyOf": [
-              {
-                "type": "integer"
-              },
-              {
-                "type": "null"
-              }
-            ],
-            "title": "Mealid"
-          },
-          "createdDate": {
-            "type": "string",
-            "format": "date-time",
-            "title": "Createddate"
-          }
-        },
-        "type": "object",
-        "title": "ShoppingListItem"
       },
       "StoreEnum": {
         "type": "string",

products/models.py

@@ -2,9 +2,10 @@ from __future__ import annotations
 
 from typing import ClassVar, List, Optional
 
-from common import ApiModel
 from pydantic import PrivateAttr
 
+from common import ApiModel
+
 
 class Product(ApiModel):
     KEYS: ClassVar[List[str]] = [

tighten-api-spec.md

@@ -0,0 +1,113 @@
+# Tighten Public API Nullability
+
+Make the external API more consistent and predictable by eliminating unnecessary nulls (nullable fields) in models and responses. This plan lists concrete, low-risk changes, their rationale, and exact files/lines to modify. Each task is checkable and includes verification steps.
+
+Date: 2025-10-21
+
+## Principles
+
+- Prefer non-nullable types where domain requires a value (DB constraints, logic always sets it).
+- Keep optional only when a field truly may be absent by design (e.g., hiddenBy, prevCursor when first page).
+- Preserve backward compatibility where feasible. When changing response shapes, update tests and OpenAPI examples.
+- Pydantic already excludes None on serialization in some places; we still tighten model types to improve OpenAPI and client SDKs.
+
+## Quick wins (low risk)
+
+- [x] Page.total is non-nullable with default 0
+  - Why: Pagination always returns a number. Current `Optional[int]` leads to `null` in schema and potential nulls in responses.
+  - Change: in `common.py`, change `total: Optional[int]` to `total: int = Field(default=0, description="Total count")`.
+  - Verify:
+    - [ ] mypy/pyright/ruff pass.
+    - [ ] Tests for persons/recipes list remain green.
+    - [ ] OpenAPI shows `total` as `integer` (no anyOf null).
+
+- [x] Recipe.created_by_id non-nullable
+  - Why: DB enforces NOT NULL and creation flow always sets it.
+  - Change: in `recipes/models.py` set `created_by_id: int` (remove Optional). Keep `created_by: Optional[Person]` (hydrated field).
+  - Knock-on: `api/recipes.load_full_recipe` can drop the `if r.created_by_id is not None` guard.
+  - Verify:
+    - [ ] All recipe-related tests green.
+    - [ ] OpenAPI for Recipe shows `createdById` required.
+
+- [x] Product.raw_data excluded from public schema
+  - Why: Internal/testing helper currently typed as `Optional[dict]` -> visible as nullable in OpenAPI.
+  - Change: in `products/models.py` use a `PrivateAttr` (with a `raw_data` property) so it stays out of the schema without creating Input/Output variants.
+  - Verify:
+    - [ ] Product schema in OpenAPI does not include `rawData`.
+    - [ ] Tests referencing raw_data still pass (field remains available in code, excluded from schema/response).
+
+## Shopping models and endpoints
+
+`ShoppingListItem` currently represents two cases (ingredient request vs meal request), so several linking fields are nullable. We can reduce nulls in the public API by introducing outward-facing variants while keeping the DB model as-is.
+
+- [ ] Optional: Introduce discriminated union for API returns (medium change)
+  - Rationale: Return `oneOf` in OpenAPI with variant-specific required fields; eliminates irrelevant nullable properties for each variant.
+  - Approach (sketch):
+    - Define `ListIngredientItem` and `RequestedMealItem` pydantic models with a `kind` discriminator.
+    - Update `api/shopping.py` response models (CurrentShoppingList and PurchasedShoppingList) to use `Union[ListIngredientItem, RequestedMealItem]` for item arrays.
+    - Conversion helpers in `shopping` module to map from `ShoppingListItem` DB model to the outward union.
+  - Verify:
+    - [ ] Update tests in `tests/test_shopping_api.py` to accept the new shape while preserving field meanings.
+    - [ ] OpenAPI shows `oneOf` for shopping list items.
+
+- [ ] Tighten invariants without breaking shape (keep for now)
+  - Keep model but document invariants (only one of ingredient_id/meal_id required; recipe_id optional when meal request). Repository already validates; consider pydantic validators later.
+
+## Persons and Recipes listings
+
+- [ ] Ensure total is populated for Person and Recipe lists
+  - Already implemented in `api/persons.py` and `api/recipes.py` using repository `count_*` helpers. After making `Page.total` non-nullable, nothing else required.
+
+## Authentication dependency
+
+- [x] Provide strict non-null person dependency for protected endpoints
+  - Why: Many endpoints assume an authenticated user; typing as non-null simplifies signatures and docs.
+  - Change:
+    - Consolidated on a single dependency `cookie_person` (strict): `Cookie(..., alias="user_id")` and raises 401 if missing/unknown.
+    - Removed `require_cookie_person` and switched usages to `cookie_person`.
+  - Verify:
+    - [x] Endpoint signatures updated.
+    - [x] Unauthorized behavior covered by handlers; overall tests still pass.
+
+## File-by-file checklist (edits)
+
+- [x] `common.py`
+  - [x] Page.total -> `int = Field(default=0, ...)`
+
+- [x] `recipes/models.py`
+  - [x] `created_by_id: int`
+
+- [x] `api/recipes.py`
+  - [x] In `load_full_recipe`, set `r.created_by = await persons.get_by_id(conn, r.created_by_id)` unconditionally.
+
+- [x] `products/models.py`
+  - [x] `raw_data` moved to `PrivateAttr` with property; kept out of schema.
+
+- [x] `api/deps.py`
+  - [x] Added `require_cookie_person(...) -> persons.Person` that raises 401.
+  - [x] Updated protected endpoints to depend on `require_cookie_person`.
+
+- [ ] (Optional) Shopping API union types
+  - [x] Add outward-facing union models and mapping helpers.
+  - [x] Update `api/shopping.py` response models to use union.
+
+## Tests and validation
+
+- [x] Run format/lint/typecheck
+  - make format && make lint && make typecheck
+- [x] Run tests
+  - make test
+- [x] Export OpenAPI and inspect schema
+  - make openapi (confirm: Recipe.createdById required; Product has single schema; Page.total non-nullable; cookie param required on protected ops.)
+
+## Rollout notes
+
+- API change in OpenAPI (non-null total; createdById required). Client SDKs generated from the spec may need re-gen. Runtime remains compatible because server fills these fields.
+- Even without the optional union refactor, the quick wins remove several unnecessary nulls.
+
+## Acceptance criteria
+
+- OpenAPI no longer marks Page.total and Recipe.createdById as nullable.
+- Product.rawData does not appear in the public schema.
+- All tests pass; no runtime regressions.
+- Optional: Shopping list items use `oneOf` variants.