# 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 v2 tests 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.