nearle_daily/catalogue_backend
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

sheet upload fix

Browse Source
This commit is contained in:
Suriyakumarvijayanayagam
2026-08-28 11:22:53 +05:30
parent 0e75d32f61
commit 52f5d3be1d
41 changed files with 2056 additions and 1507 deletions
Download Patch File Download Diff File Expand all files Collapse all files

390
tests/test_inbox_upload.py
View File

@@ -1,390 +0,0 @@
"""Two-actor flow: a colleague drops files, an admin decides what runs.
Follows test_batch_catalog_ingest.py: point the directories at tmp_path,
monkeypatch the storage/embedding boundary, and stub the background worker by
default. Nothing here loads sentence-transformers or torch, nothing reaches the
network, and nothing writes into the repository's data directory.
The most important tests in this file are the ones asserting what an `uploader`
credential CANNOT do. That role exists specifically so an outside contributor
does not get the `user` role's catalog write access, and an assertion is the
only thing that keeps it true as endpoints are added.
"""
from __future__ import annotations
import io
import pytest
from app.core import batch_ingest, inbox
from app.core import store_catalog_pipeline as pipeline
openpyxl = pytest.importorskip("openpyxl")
HEADERS = ["Product Name", "Category", "Brand"]
ROWS = [["Amul Butter 100g", "Butter", "Amul"]]
UPLOAD_KEY = "k" * 43
UPLOAD = "/api/uploads/catalog"
INBOX = "/api/admin/catalog-batch/inbox"
FROM_INBOX = "/api/admin/catalog-batch/from-inbox"
DISMISS = "/api/admin/catalog-batch/inbox/dismiss"
def _csv(rows=ROWS) -> bytes:
return ("\n".join([",".join(HEADERS)] + [",".join(r) for r in rows])).encode()
def _sheet(rows=ROWS) -> bytes:
wb = openpyxl.Workbook()
ws = wb.active
ws.append(HEADERS)
for row in rows:
ws.append(row)
buf = io.BytesIO()
wb.save(buf)
return buf.getvalue()
def _files(*pairs):
return [("files", (n, io.BytesIO(c), "application/octet-stream"))
for n, c in pairs]
@pytest.fixture(autouse=True)
def _isolate_sku_counter(tmp_path, monkeypatch):
from app.services import sku_service
monkeypatch.setattr(sku_service, "_data_dir", tmp_path / "sku_sequences")
@pytest.fixture(autouse=True)
def dirs(tmp_path, monkeypatch):
"""Both staging directories under tmp_path. Read at call time by design."""
monkeypatch.setattr(inbox, "INBOX_UPLOAD_DIR", tmp_path / "inbox")
monkeypatch.setattr(batch_ingest, "BATCH_UPLOAD_DIR", tmp_path / "batch_uploads")
return tmp_path
@pytest.fixture(autouse=True)
def no_background_worker(monkeypatch):
"""Same trap as in test_batch_catalog_ingest: a worker still running after
teardown resolves the directory setting again and writes into the real
data/ directory. Stub it; the tests here are about routing and state."""
from app.core import batch_worker
submitted: list = []
monkeypatch.setattr(batch_worker, "submit", submitted.append)
return submitted
@pytest.fixture(autouse=True)
def upload_key(monkeypatch):
"""One uploader API key.
Patched on `security`, not on `settings`: security.py does
`from ...settings import API_KEYS` at import, binding the dict object, so
rebinding the name in `settings` leaves `principal_for_api_key` still
looking at the original and every request comes back 401.
"""
from app.infrastructure import security
monkeypatch.setattr(
security, "API_KEYS", {UPLOAD_KEY: ("catalog-drop", "uploader")}
)
return {"X-API-Key": UPLOAD_KEY}
@pytest.fixture
def store(monkeypatch):
table: dict = {}
def fake_upsert(brand, rows, cleanup=False):
assert cleanup is False
for row in rows:
table[row["image_id"]] = dict(row)
return len(rows)
monkeypatch.setattr(pipeline, "upsert_brand_products", fake_upsert)
monkeypatch.setattr(pipeline, "get_products_by_brand", lambda b, **kw: list(table.values()))
monkeypatch.setattr(pipeline, "embed_texts", lambda texts: [[0.0] * 384 for _ in texts])
return table
def _submit(client, upload_key, *pairs):
return client.post(UPLOAD, files=_files(*pairs), headers=upload_key)
# ---------------------------------------------------------------------------
# The security boundary - the reason the `uploader` role exists
# ---------------------------------------------------------------------------
def test_uploader_key_can_submit(client, upload_key):
r = _submit(client, upload_key, ("a.csv", _csv()))
assert r.status_code == 202
body = r.json()
assert body["files_accepted"] == 1
assert body["submitted_by"] == "catalog-drop"
@pytest.mark.parametrize("method,path", [
("get", "/api/admin/catalog-batch/inbox"),
("post", "/api/admin/catalog-batch/from-inbox"),
("post", "/api/admin/catalog-batch/inbox/dismiss"),
("post", "/api/admin/catalog-batch/preview"),
("post", "/api/admin/catalog-batch/ingest"),
("get", "/api/admin/catalog-batch/batches"),
("get", "/api/admin/catalog-batch/batches/abc"),
("post", "/api/admin/catalog-batch/batches/abc/resume"),
("post", "/api/admin/catalog-batch/batches/abc/cancel"),
("post", "/api/admin/store-catalog/preview"),
("post", "/api/admin/store-catalog/ingest"),
("get", "/api/admin/store-catalog/jobs/abc"),
])
def test_uploader_key_is_refused_on_every_admin_route(client, upload_key, method, path):
"""An uploader credential must reach nothing but its own endpoint."""
r = getattr(client, method)(path, headers=upload_key)
assert r.status_code == 403, f"{method.upper()} {path} returned {r.status_code}"
@pytest.mark.parametrize("path", [
"/api/user/products/add",
"/api/user/products/upload-file",
"/api/upload/stores",
])
def test_uploader_key_has_none_of_the_user_roles_write_access(client, upload_key, path):
"""This is the specific over-grant avoided by NOT reusing the `user` role.
A `user` key would carry add_product / upload_batch_products /
upload_store_inventory - real catalog writes - to solve a problem that
needed one verb.
"""
r = client.post(path, headers=upload_key)
assert r.status_code == 403, f"{path} returned {r.status_code}"
def test_upload_endpoint_rejects_anonymous(client):
r = client.post(UPLOAD, files=_files(("a.csv", _csv())))
assert r.status_code == 401
def test_admin_can_still_use_the_upload_endpoint(client, admin_headers):
"""admin bypasses every permission check (Principal.has_permission)."""
r = client.post(UPLOAD, files=_files(("a.csv", _csv())), headers=admin_headers)
assert r.status_code == 202
def test_role_allow_lists_stay_in_step(client):
"""settings._parse_api_keys duplicates the role names from security.py
because importing back would be a cycle. Pin the duplication."""
from app.infrastructure import settings
from app.infrastructure.security import VALID_ROLES
for role in VALID_ROLES:
parsed = settings._parse_api_keys(f"n:{role}:{'x' * 43}")
assert parsed[("x" * 43)] == ("n", role)
with pytest.raises(RuntimeError):
settings._parse_api_keys(f"n:wizard:{'x' * 43}")
# ---------------------------------------------------------------------------
# Intake
# ---------------------------------------------------------------------------
def test_a_broken_sheet_is_refused_at_upload_and_never_enters_the_inbox(client, upload_key):
r = _submit(client, upload_key, ("good.csv", _csv()), ("broken.pdf", b"%PDF-1.4"))
assert r.status_code == 202
body = r.json()
assert body["files_accepted"] == 1
assert body["files_rejected"] == 1
bad = [f for f in body["files"] if f["filename"] == "broken.pdf"][0]
assert bad["accepted"] is False and bad["error"]
# Only the good one is waiting.
assert inbox.pending_count() == 1
def test_a_drop_where_nothing_parses_stages_no_submission(client, upload_key):
r = _submit(client, upload_key, ("broken.pdf", b"%PDF-1.4"))
assert r.status_code == 202
assert r.json()["submission_id"] is None
assert inbox.pending_count() == 0
assert inbox.list_submissions() == []
def test_uploaded_filenames_cannot_escape_the_inbox(client, upload_key, dirs):
r = _submit(client, upload_key, ("../../evil.csv", _csv()))
assert r.status_code == 202
submission = inbox.list_submissions()[0]
directory = inbox.submission_dir(submission.submission_id)
for entry in submission.files:
written = directory / entry.stored_name
assert written.resolve().parent == directory.resolve()
assert not (dirs / "evil.csv").exists()
def test_a_full_inbox_refuses_further_uploads(client, upload_key, monkeypatch):
from app.api.routers import uploads
monkeypatch.setattr(uploads, "INBOX_MAX_PENDING_FILES", 1)
assert _submit(client, upload_key, ("a.csv", _csv())).status_code == 202
r = _submit(client, upload_key, ("b.csv", _csv()))
assert r.status_code == 429
assert "awaiting review" in r.json()["detail"]
def test_too_many_files_in_one_drop_is_413(client, upload_key, monkeypatch):
from app.api.routers import uploads
monkeypatch.setattr(uploads, "BATCH_MAX_FILES", 2)
r = _submit(client, upload_key,
("a.csv", _csv()), ("b.csv", _csv()), ("c.csv", _csv()))
assert r.status_code == 413
def test_uploading_starts_no_work(client, upload_key, no_background_worker):
"""The whole safety property: an uploader can spend disk, never CPU."""
_submit(client, upload_key, ("a.csv", _csv()))
assert no_background_worker == []
# ---------------------------------------------------------------------------
# The admin side
# ---------------------------------------------------------------------------
def test_inbox_lists_pending_grouped_by_drop(client, upload_key, admin_headers):
_submit(client, upload_key, ("a.csv", _csv()), ("b.csv", _csv()))
_submit(client, upload_key, ("c.csv", _csv()))
r = client.get(INBOX, headers=admin_headers)
assert r.status_code == 200
body = r.json()
assert body["pending_count"] == 3
assert len(body["submissions"]) == 2
assert all(s["submitted_by"] == "catalog-drop" for s in body["submissions"])
assert body["submissions"][0]["files"][0]["rows_total"] == 1
def test_selecting_files_across_two_drops_makes_one_batch(
client, upload_key, admin_headers, store, no_background_worker
):
"""The requirement that ruled out reusing batch-resume: files from
different drops, composed into a single run."""
_submit(client, upload_key, ("a.csv", _csv()), ("b.csv", _csv()))
_submit(client, upload_key, ("c.csv", _csv()))
listing = client.get(INBOX, headers=admin_headers).json()
first = listing["submissions"][0]["files"][0]["file_id"]
second = listing["submissions"][1]["files"][0]["file_id"]
r = client.post(FROM_INBOX, json={"file_ids": [first, second]}, headers=admin_headers)
assert r.status_code == 202
batch = r.json()
assert batch["files_total"] == 2
assert batch["submitted_by"] == "catalog-drop"
assert no_background_worker == [batch["batch_id"]]
# The unselected file is untouched.
after = client.get(INBOX, headers=admin_headers).json()
assert after["pending_count"] == 1
def test_a_started_file_cannot_be_started_twice(client, upload_key, admin_headers, store):
"""Two admin tabs on the same inbox must not double-run a file."""
_submit(client, upload_key, ("a.csv", _csv()))
file_id = client.get(INBOX, headers=admin_headers).json()["submissions"][0]["files"][0]["file_id"]
assert client.post(FROM_INBOX, json={"file_ids": [file_id]}, headers=admin_headers).status_code == 202
again = client.post(FROM_INBOX, json={"file_ids": [file_id]}, headers=admin_headers)
assert again.status_code == 409
assert "already consumed" in again.json()["detail"]
def test_selecting_an_unknown_file_is_404(client, admin_headers):
r = client.post(FROM_INBOX, json={"file_ids": ["nope"]}, headers=admin_headers)
assert r.status_code == 404
def test_selecting_nothing_is_400(client, admin_headers):
assert client.post(FROM_INBOX, json={"file_ids": []}, headers=admin_headers).status_code == 400
def test_dismiss_clears_the_badge_without_running_anything(
client, upload_key, admin_headers, no_background_worker
):
_submit(client, upload_key, ("a.csv", _csv()))
file_id = client.get(INBOX, headers=admin_headers).json()["submissions"][0]["files"][0]["file_id"]
r = client.post(DISMISS, json={"file_ids": [file_id]}, headers=admin_headers)
assert r.status_code == 200
assert r.json() == {"dismissed": 1, "pending_count": 0}
assert no_background_worker == [], "dismiss must not start work"
assert client.get(INBOX, headers=admin_headers).json()["pending_count"] == 0
def test_dismissing_an_already_decided_file_is_409(client, upload_key, admin_headers):
_submit(client, upload_key, ("a.csv", _csv()))
file_id = client.get(INBOX, headers=admin_headers).json()["submissions"][0]["files"][0]["file_id"]
client.post(DISMISS, json={"file_ids": [file_id]}, headers=admin_headers)
again = client.post(DISMISS, json={"file_ids": [file_id]}, headers=admin_headers)
assert again.status_code == 409
# ---------------------------------------------------------------------------
# End to end, and retention
# ---------------------------------------------------------------------------
def test_selected_files_actually_reach_the_catalog(client, upload_key, admin_headers, store):
"""Run the composed batch for real (worker stubbed, pipeline not)."""
_submit(client, upload_key, ("a.csv", _csv()), ("b.xlsx", _sheet()))
listing = client.get(INBOX, headers=admin_headers).json()
ids = [f["file_id"] for f in listing["submissions"][0]["files"]]
started = client.post(FROM_INBOX, json={"file_ids": ids}, headers=admin_headers).json()
result = batch_ingest.run_batch(started["batch_id"])
assert result.status == "done"
assert result.files_done == 2
assert result.submitted_by == "catalog-drop"
assert store, "no rows reached the fake brand table"
def test_purge_removes_decided_submissions_and_spares_pending(client, upload_key, admin_headers, monkeypatch):
import time
monkeypatch.setattr(inbox, "INBOX_RETENTION_DAYS", 7)
_submit(client, upload_key, ("old.csv", _csv()))
old = inbox.list_submissions()[0]
file_id = old.files[0].file_id
client.post(DISMISS, json={"file_ids": [file_id]}, headers=admin_headers)
aged = inbox.read_record(old.submission_id)
aged.created_at = time.time() - 30 * 86400
inbox.write_record(aged)
_submit(client, upload_key, ("pending.csv", _csv()))
still_pending = [s for s in inbox.list_submissions() if s.pending_files][0]
stale_but_unreviewed = inbox.read_record(still_pending.submission_id)
stale_but_unreviewed.created_at = time.time() - 30 * 86400
inbox.write_record(stale_but_unreviewed)
removed = inbox.purge_expired()
assert removed == [old.submission_id]
assert not inbox.submission_dir(old.submission_id).exists()
assert inbox.submission_dir(still_pending.submission_id).exists(), \
"a file nobody has reviewed must never be purged"
def test_purge_is_disabled_when_retention_is_zero(client, upload_key, monkeypatch):
monkeypatch.setattr(inbox, "INBOX_RETENTION_DAYS", 0)
_submit(client, upload_key, ("a.csv", _csv()))
assert inbox.purge_expired() == []
def test_unreadable_record_is_skipped_not_raised(client, upload_key):
_submit(client, upload_key, ("a.csv", _csv()))
sid = inbox.list_submissions()[0].submission_id
inbox.record_path(sid).write_text("{ not json", encoding="utf-8")
assert inbox.read_record(sid) is None
assert inbox.list_submissions() == []
def test_submission_dir_rejects_a_traversing_id():
with pytest.raises(ValueError):
inbox.submission_dir("../escape")

402
tests/test_upload_data_integrity.py Normal file
View File

@@ -0,0 +1,402 @@
"""The operator upload endpoints must never invent a value.
POST /api/upload/stores
POST /api/upload/analytics
POST /api/upload/nutrition
These three used to substitute a plausible constant for every column a sheet
did not carry, so a header mismatch produced rows filed under brand "amul" in
store "store_mumbai_1" - and nutrition rows with invented calories and
`allergens = ['None']` written as `data_status = 'verified'`.
Every test here pins the opposite behaviour: a missing identifying column
SKIPS the row and reports it, and a missing value is stored as NULL. The
allergen tests are the ones that matter most - "we were not told" and "this
product is allergen-free" must never become the same database row.
No database is involved: `_connect` is replaced with a recorder, so the
assertions are about the exact SQL and parameters the handlers would send.
"""
from __future__ import annotations
import io
import pytest
from app.api.routers import upload as upload_router
openpyxl = pytest.importorskip("openpyxl")
class FakeCursor:
def __init__(self, calls):
self.calls = calls
def execute(self, sql, params=None):
self.calls.append((" ".join(str(sql).split()), params))
def __enter__(self):
return self
def __exit__(self, *exc):
return False
class FakeConn:
def __init__(self):
self.calls = []
self.closed = False
def cursor(self):
return FakeCursor(self.calls)
def __enter__(self):
return self
def __exit__(self, *exc):
return False
def close(self):
self.closed = True
@pytest.fixture
def db(monkeypatch):
"""Swap the connection and the two schema bootstraps for recorders."""
conn = FakeConn()
monkeypatch.setattr(upload_router, "_connect", lambda: conn)
monkeypatch.setattr(
upload_router.store_db, "ensure_store_intelligence_schema", lambda: None
)
monkeypatch.setattr(
upload_router.nutrition_db, "ensure_nutrition_schema", lambda: None
)
return conn
def _csv(text: str):
return {"file": ("sheet.csv", io.BytesIO(text.encode()), "text/csv")}
def _find(conn, needle):
"""Every recorded statement containing `needle`, with its parameters."""
return [(sql, params) for sql, params in conn.calls if needle in sql]
# ---------------------------------------------------------------------------
# Stores
# ---------------------------------------------------------------------------
STORES_OK = (
"store_id,brand,product_name,category,available_stock,mrp,cost_price,selling_price\n"
"store_mumbai_1,Amul,Amul Butter 500ml,Dairy,120,250,200,235\n"
)
def test_a_store_row_without_brand_is_skipped_not_filed_under_amul(client, admin_headers, db):
body = "store_id,product_name,available_stock\nstore_1,Butter,10\n"
response = client.post("/api/upload/stores", files=_csv(body), headers=admin_headers)
assert response.status_code == 422
detail = response.json()["detail"]
assert "brand" in detail["errors"][0]["error"]
assert detail["errors"][0]["row"] == 2 # header is row 1
# The old code would have inserted with brand 'amul'.
assert _find(db, "INSERT INTO store_inventory") == []
assert "amul" not in response.text.lower()
def test_a_store_row_without_a_store_id_is_skipped(client, admin_headers, db):
body = "brand,product_name\namul,Butter\n"
response = client.post("/api/upload/stores", files=_csv(body), headers=admin_headers)
assert response.status_code == 422
assert "store_id" in response.json()["detail"]["errors"][0]["error"]
def test_a_complete_store_row_is_imported_with_its_own_values(client, admin_headers, db):
response = client.post("/api/upload/stores", files=_csv(STORES_OK), headers=admin_headers)
assert response.status_code == 200, response.text
body = response.json()
assert body["rows_imported"] == 1
assert body["rows_skipped"] == 0
assert body["stores_affected"] == ["store_mumbai_1"]
_sql, params = _find(db, "INSERT INTO store_inventory")[0]
assert params[0] == "store_mumbai_1"
assert params[1] == "amul"
assert params[3] == "Amul Butter 500ml"
assert params[4] == "Dairy"
assert params[5] == 120
def test_absent_category_and_city_are_stored_as_null(client, admin_headers, db):
body = "store_id,brand,product_name\nstore_1,amul,Butter\n"
assert client.post("/api/upload/stores", files=_csv(body),
headers=admin_headers).status_code == 200
_sql, inv = _find(db, "INSERT INTO store_inventory")[0]
assert inv[4] is None, "category was invented"
_sql, store = _find(db, "INSERT INTO stores")[0]
assert store[2] is None, "city was invented ('Mumbai')"
def test_absent_stock_is_zero_not_fifty(client, admin_headers, db):
body = "store_id,brand,product_name\nstore_1,amul,Butter\n"
client.post("/api/upload/stores", files=_csv(body), headers=admin_headers)
_sql, inv = _find(db, "INSERT INTO store_inventory")[0]
assert inv[5] == 0, "available_stock was invented"
assert inv[6] == 0 and inv[7] == 0 and inv[8] == 0
def test_prices_are_written_only_when_the_whole_triple_is_present(client, admin_headers, db):
response = client.post("/api/upload/stores", files=_csv(STORES_OK), headers=admin_headers)
assert response.json()["prices_written"] == 1
_sql, params = _find(db, "INSERT INTO store_prices")[0]
assert params[3] == 250.0 and params[4] == 200.0 and params[5] == 235.0
def test_a_missing_cost_price_skips_prices_but_keeps_the_inventory(client, admin_headers, db):
"""The old code invented cost = mrp * 0.7 and a selling price to match."""
body = "store_id,brand,product_name,mrp,selling_price\nstore_1,amul,Butter,250,235\n"
response = client.post("/api/upload/stores", files=_csv(body), headers=admin_headers)
assert response.status_code == 200
assert response.json()["prices_written"] == 0
assert response.json()["prices_skipped"] == 1
assert _find(db, "INSERT INTO store_prices") == []
assert len(_find(db, "INSERT INTO store_inventory")) == 1
def test_a_partial_file_imports_the_good_rows_and_reports_the_rest(client, admin_headers, db):
body = (
"store_id,brand,product_name\n"
"store_1,amul,Butter\n"
",,\n"
"store_1,nestle,Milk Powder\n"
)
response = client.post("/api/upload/stores", files=_csv(body), headers=admin_headers)
assert response.status_code == 200
assert response.json()["status"] == "partial"
assert response.json()["rows_imported"] == 2
assert response.json()["rows_skipped"] == 1
# ---------------------------------------------------------------------------
# Analytics - the endpoint that never worked
# ---------------------------------------------------------------------------
ANALYTICS_OK = (
"order_id,store_id,brand,image_id,order_date,customer_id,quantity,unit_price,total_price\n"
"ORD_1,store_1,amul,amul_butter,2026-08-01 10:30:00,cust_101,2,235.00,470.00\n"
)
def test_order_items_uses_the_columns_the_table_actually_has(client, admin_headers, db):
"""REGRESSION: the insert named `total_price` (not a column) and omitted
`store_id` (NOT NULL), so every analytics upload raised and returned 500."""
response = client.post("/api/upload/analytics", files=_csv(ANALYTICS_OK),
headers=admin_headers)
assert response.status_code == 200, response.text
sql, params = _find(db, "INSERT INTO order_items")[0]
assert "line_total" in sql
assert "total_price" not in sql
assert "store_id" in sql
# (order_id, store_id, brand, image_id, quantity, unit_price, line_total)
assert params == ("ORD_1", "store_1", "amul", "amul_butter", 2, 235.0, 470.0)
def test_line_total_is_derived_when_the_sheet_omits_it(client, admin_headers, db):
body = ("store_id,brand,image_id,customer_id,quantity,unit_price\n"
"store_1,amul,amul_butter,cust_1,3,100.00\n")
client.post("/api/upload/analytics", files=_csv(body), headers=admin_headers)
_sql, params = _find(db, "INSERT INTO order_items")[0]
assert params[6] == 300.0, "quantity * unit_price is arithmetic, not invention"
def test_a_sale_without_a_customer_is_skipped_not_merged_into_one_buyer(client, admin_headers, db):
"""'cust_imported' collapsed every buyer in a file into a single customer."""
body = "store_id,brand,image_id,quantity,unit_price\nstore_1,amul,x,1,10\n"
response = client.post("/api/upload/analytics", files=_csv(body), headers=admin_headers)
assert response.status_code == 422
assert "customer_id" in response.json()["detail"]["errors"][0]["error"]
assert _find(db, "INSERT INTO order_items") == []
def test_a_sale_without_quantity_or_price_is_skipped(client, admin_headers, db):
body = "store_id,brand,image_id,customer_id\nstore_1,amul,x,cust_1\n"
response = client.post("/api/upload/analytics", files=_csv(body), headers=admin_headers)
assert response.status_code == 422
err = response.json()["detail"]["errors"][0]["error"]
assert "quantity" in err and "unit_price" in err
def test_order_value_is_recomputed_from_its_lines(client, admin_headers, db):
"""A two-line order must not end up carrying only its last line's value."""
body = (
"order_id,store_id,brand,image_id,customer_id,quantity,unit_price\n"
"ORD_1,store_1,amul,a,cust_1,1,100\n"
"ORD_1,store_1,amul,b,cust_1,1,50\n"
)
response = client.post("/api/upload/analytics", files=_csv(body), headers=admin_headers)
assert response.status_code == 200
updates = _find(db, "UPDATE orders")
assert len(updates) == 1
assert "SUM(line_total)" in updates[0][0]
assert updates[0][1] == (["ORD_1"],)
def test_an_undated_row_is_counted_as_such(client, admin_headers, db):
body = ("store_id,brand,image_id,customer_id,quantity,unit_price\n"
"store_1,amul,x,cust_1,1,10\n")
response = client.post("/api/upload/analytics", files=_csv(body), headers=admin_headers)
assert response.json()["dates_defaulted_to_now"] == 1
# ---------------------------------------------------------------------------
# Nutrition - the data-integrity rule this module exists for
# ---------------------------------------------------------------------------
NUTRITION_FULL = (
"brand,image_id,product_name,calories_kcal,protein_g,carbohydrates_g,"
"total_sugar_g,dietary_fiber_g,total_fat_g,sodium_mg,allergens\n"
"amul,amul_butter,Amul Butter,717,0.8,0.1,0.0,0.0,81.0,650,Milk\n"
)
def _facts(db):
return _find(db, "INSERT INTO nutrition_facts")[0][1]
def _insights(db):
hits = _find(db, "INSERT INTO nutrition_insights")
return hits[0][1] if hits else None
def test_an_absent_allergen_column_is_null_never_none(client, admin_headers, db):
"""THE test. 'We were not told' must not become 'contains no allergens'."""
body = "brand,image_id,product_name,protein_g\namul,x,Butter,5.0\n"
assert client.post("/api/upload/nutrition", files=_csv(body),
headers=admin_headers).status_code == 200
params = _insights(db)
assert params is not None
allergens, allergen_source = params[8], params[9]
assert allergens is None, "allergens were invented"
assert allergen_source == "unavailable"
def test_an_absent_diet_tag_column_is_null(client, admin_headers, db):
"""It used to default to ['High Protein', 'Gluten Free'] for everything."""
body = "brand,image_id,product_name,protein_g\namul,x,Butter,5.0\n"
client.post("/api/upload/nutrition", files=_csv(body), headers=admin_headers)
assert _insights(db)[7] is None
def test_absent_nutrients_are_null_not_plausible_numbers(client, admin_headers, db):
body = "brand,image_id,product_name\namul,x,Butter\n"
client.post("/api/upload/nutrition", files=_csv(body), headers=admin_headers)
params = _facts(db)
# (brand, image_id, product_name, category, data_status, then 10 nutrients)
assert params[3] is None # category
assert all(v is None for v in params[5:]), "a nutrient was invented"
def test_a_row_with_no_nutrients_is_unavailable_not_verified(client, admin_headers, db):
body = "brand,image_id,product_name\namul,x,Butter\n"
response = client.post("/api/upload/nutrition", files=_csv(body), headers=admin_headers)
assert _facts(db)[4] == "unavailable"
assert response.json()["data_status_counts"]["unavailable"] == 1
def test_a_row_with_some_nutrients_is_partial(client, admin_headers, db):
body = "brand,image_id,product_name,protein_g,total_sugar_g\namul,x,Butter,5.0,2.0\n"
response = client.post("/api/upload/nutrition", files=_csv(body), headers=admin_headers)
assert _facts(db)[4] == "partial"
assert response.json()["data_status_counts"]["partial"] == 1
def test_only_a_complete_core_set_is_verified(client, admin_headers, db):
response = client.post("/api/upload/nutrition", files=_csv(NUTRITION_FULL),
headers=admin_headers)
assert _facts(db)[4] == "verified"
assert response.json()["data_status_counts"]["verified"] == 1
params = _insights(db)
assert params[8] == ["Milk"]
assert params[9] == "upload"
def test_insights_are_built_only_from_supplied_values(client, admin_headers, db):
body = "brand,image_id,product_name,protein_g\namul,x,Butter,5.0\n"
client.post("/api/upload/nutrition", files=_csv(body), headers=admin_headers)
params = _insights(db)
positives, cautions = params[5], params[6]
assert positives == ["Contains 5.0g protein per 100g"]
assert cautions is None, "a sugar caution was invented from no sugar value"
def test_no_insight_row_at_all_when_nothing_was_supplied(client, admin_headers, db):
body = "brand,image_id,product_name\namul,x,Butter\n"
client.post("/api/upload/nutrition", files=_csv(body), headers=admin_headers)
assert _find(db, "INSERT INTO nutrition_insights") == []
def test_the_score_breakdown_is_no_longer_a_hardcoded_constant(client, admin_headers, db):
client.post("/api/upload/nutrition", files=_csv(NUTRITION_FULL), headers=admin_headers)
sql, _params = _find(db, "INSERT INTO nutrition_insights")[0]
assert "score_breakdown" not in sql
for _sql, params in db.calls:
assert '"protein": 85' not in str(params)
def test_a_nutrition_row_without_a_brand_is_skipped(client, admin_headers, db):
body = "image_id,product_name,protein_g\nx,Butter,5.0\n"
response = client.post("/api/upload/nutrition", files=_csv(body), headers=admin_headers)
assert response.status_code == 422
assert "brand" in response.json()["detail"]["errors"][0]["error"]
assert _find(db, "INSERT INTO nutrition_facts") == []
def test_upserts_never_blank_a_value_an_earlier_source_established(client, admin_headers, db):
"""A thinner sheet must not overwrite verified data with NULL."""
client.post("/api/upload/nutrition", files=_csv(NUTRITION_FULL), headers=admin_headers)
sql, _ = _find(db, "INSERT INTO nutrition_facts")[0]
for column in ("protein_g", "total_sugar_g", "sodium_mg", "calories_kcal"):
assert f"{column} = COALESCE(EXCLUDED.{column}, nutrition_facts.{column})" in sql
assert "WHEN nutrition_facts.data_status = 'verified' THEN 'verified'" in sql
# ---------------------------------------------------------------------------
# Shared behaviour
# ---------------------------------------------------------------------------
@pytest.mark.parametrize("path", ["/api/upload/stores", "/api/upload/analytics",
"/api/upload/nutrition"])
def test_an_empty_sheet_is_refused(client, admin_headers, db, path):
assert client.post(path, files=_csv("brand,product_name\n"),
headers=admin_headers).status_code == 400
@pytest.mark.parametrize("path", ["/api/upload/stores", "/api/upload/analytics",
"/api/upload/nutrition"])
def test_uploads_require_a_credential(client, path):
assert client.post(path, files=_csv("a,b\n1,2\n")).status_code == 401
@pytest.mark.parametrize("tab", ["stores", "analytics", "nutrition"])
def test_the_template_matches_what_the_handler_requires(client, tab):
response = client.get(f"/api/upload/template/{tab}")
assert response.status_code == 200
header = response.text.splitlines()[0]
assert "brand" in header
# The nutrition template must not teach people to write "None" for
# allergens - that is the exact claim the endpoint must never store.
if tab == "nutrition":
assert ",None" not in response.text

470
tests/test_uploads_api.py Normal file
View File

@@ -0,0 +1,470 @@
"""The catalog ingestion API given to outside API clients.
POST /api/uploads/catalog - send spreadsheets, the pipeline runs
GET /api/uploads/catalog - the batches this caller has sent
GET /api/uploads/catalog/{batch_id} - progress and result of one of them
Follows test_batch_catalog_ingest.py: point BATCH_UPLOAD_DIR at tmp_path,
monkeypatch the storage/embedding boundary, and stub the background worker by
default. Nothing here loads sentence-transformers or torch, nothing reaches the
network, and nothing writes into the repository's data directory.
Two groups of tests matter more than the rest:
* the ones asserting an upload is actually QUEUED. The endpoint previously
accepted files and started nothing, which read as a success to every client
and to every test that only checked the status code. `submitted` is
asserted on directly for that reason.
* the ones asserting what an `uploader` credential CANNOT do. That role
exists so an outside contributor does not get the `user` role's catalog
write access, and an assertion is the only thing that keeps it true as
endpoints are added.
"""
from __future__ import annotations
import io
import pytest
from app.api.batch_job_store import batch_job_store
from app.core import batch_ingest
from app.core import store_catalog_pipeline as pipeline
openpyxl = pytest.importorskip("openpyxl")
HEADERS = ["Product Name", "Category", "Brand"]
ROWS = [["Amul Butter 100g", "Butter", "Amul"]]
UPLOAD_KEY = "k" * 43
OTHER_KEY = "z" * 43
UPLOAD = "/api/uploads/catalog"
def _csv(rows=ROWS) -> bytes:
return ("\n".join([",".join(HEADERS)] + [",".join(r) for r in rows])).encode()
def _sheet(rows=ROWS) -> bytes:
"""A real .xlsx, which is the format this endpoint exists to accept."""
wb = openpyxl.Workbook()
ws = wb.active
ws.append(HEADERS)
for row in rows:
ws.append(row)
buf = io.BytesIO()
wb.save(buf)
return buf.getvalue()
def _files(*pairs):
return [("files", (n, io.BytesIO(c), "application/octet-stream"))
for n, c in pairs]
@pytest.fixture(autouse=True)
def _isolate_sku_counter(tmp_path, monkeypatch):
from app.services import sku_service
monkeypatch.setattr(sku_service, "_data_dir", tmp_path / "sku_sequences")
@pytest.fixture(autouse=True)
def batch_root(tmp_path, monkeypatch):
"""BATCH_UPLOAD_DIR under tmp_path. Read at call time by design."""
root = tmp_path / "batch_uploads"
monkeypatch.setattr(batch_ingest, "BATCH_UPLOAD_DIR", root)
return root
@pytest.fixture(autouse=True)
def submitted(monkeypatch):
"""Stub the worker and record what was handed to it.
Same trap as in test_batch_catalog_ingest: a worker still running after
teardown resolves BATCH_UPLOAD_DIR again and writes into the real data/
directory. Stubbing also turns "was this queued?" into something a test can
assert on rather than infer.
"""
from app.core import batch_worker
seen: list = []
monkeypatch.setattr(batch_worker, "submit", seen.append)
return seen
@pytest.fixture(autouse=True)
def _clean_job_store():
"""Empty the process-global batch cache around every test.
batch_job_store outlives a test - it is a module singleton - so without
this a batch staged by one test is still in the cache for the next one, and
the ownership assertions below ("this caller sees only their own") would
pass or fail depending on test order.
"""
batch_job_store._batches.clear()
batch_job_store._cancelled.clear()
yield
batch_job_store._batches.clear()
batch_job_store._cancelled.clear()
@pytest.fixture(autouse=True)
def keys(monkeypatch):
"""Two uploader keys, so "only my own batches" is testable.
Patched on `security`, not on `settings`: security.py does
`from ...settings import API_KEYS` at import, binding the dict object, so
rebinding the name in `settings` leaves `principal_for_api_key` still
looking at the original and every request comes back 401.
"""
from app.infrastructure import security
monkeypatch.setattr(security, "API_KEYS", {
UPLOAD_KEY: ("catalog-drop", "uploader"),
OTHER_KEY: ("other-drop", "uploader"),
})
@pytest.fixture
def upload_headers() -> dict:
return {"X-API-Key": UPLOAD_KEY}
@pytest.fixture
def other_headers() -> dict:
return {"X-API-Key": OTHER_KEY}
@pytest.fixture
def store(monkeypatch):
table: dict = {}
def fake_upsert(brand, rows, cleanup=False):
assert cleanup is False
for row in rows:
table[row["image_id"]] = dict(row)
return len(rows)
monkeypatch.setattr(pipeline, "upsert_brand_products", fake_upsert)
monkeypatch.setattr(pipeline, "get_products_by_brand", lambda b, **kw: list(table.values()))
monkeypatch.setattr(pipeline, "embed_texts", lambda texts: [[0.0] * 384 for _ in texts])
return table
# ---------------------------------------------------------------------------
# The requirement: a file sent here is ingested
# ---------------------------------------------------------------------------
def test_an_uploaded_spreadsheet_is_queued_for_ingestion(client, upload_headers, submitted):
"""THE regression test for this endpoint.
An earlier version parked the file and started nothing, which is
indistinguishable from working if you only assert on the status code.
"""
response = client.post(UPLOAD, files=_files(("catalog.xlsx", _sheet())),
headers=upload_headers)
assert response.status_code == 202, response.text
body = response.json()
assert body["batch_id"]
assert body["status"] == "queued"
assert body["files_total"] == 1
# The batch reached the worker. This is the assertion that was missing.
assert submitted == [body["batch_id"]]
def test_an_excel_file_reaches_the_catalog(client, upload_headers, store):
"""End to end: POST a real .xlsx, run the batch, find the row stored."""
response = client.post(UPLOAD, files=_files(("catalog.xlsx", _sheet())),
headers=upload_headers)
assert response.status_code == 202, response.text
batch_id = response.json()["batch_id"]
manifest = batch_ingest.run_batch(batch_id)
assert manifest.status == "done"
assert manifest.files[0].status == "done"
assert manifest.totals()["rows_total"] == 1
assert manifest.totals()["products_built"] >= 1
assert "Amul" in manifest.brands()
assert store, "the pipeline stored nothing"
def test_the_batch_records_which_credential_sent_it(client, upload_headers):
response = client.post(UPLOAD, files=_files(("a.csv", _csv())), headers=upload_headers)
batch_id = response.json()["batch_id"]
# The key's NAME, never its secret.
assert response.json()["submitted_by"] == "catalog-drop"
assert batch_ingest.read_manifest(batch_id).submitted_by == "catalog-drop"
assert UPLOAD_KEY not in response.text
def test_the_response_says_where_to_poll(client, upload_headers):
response = client.post(UPLOAD, files=_files(("a.csv", _csv())), headers=upload_headers)
body = response.json()
assert body["batch_id"] in body["message"]
def test_network_stages_are_off_unless_asked_for(client, upload_headers):
"""Image search spawns a Playwright subprocess; it is opt-in on this path."""
response = client.post(UPLOAD, files=_files(("a.csv", _csv())), headers=upload_headers)
assert response.json()["fetch_images"] is False
assert response.json()["use_llm"] is False
def test_network_stages_can_be_opted_into(client, upload_headers):
response = client.post(UPLOAD + "?fetch_images=true", files=_files(("a.csv", _csv())),
headers=upload_headers)
assert response.json()["fetch_images"] is True
# ---------------------------------------------------------------------------
# Who may call it
# ---------------------------------------------------------------------------
def test_upload_endpoint_rejects_anonymous(client):
assert client.post(UPLOAD, files=_files(("a.csv", _csv()))).status_code == 401
def test_admin_can_also_send_files(client, admin_headers, submitted):
"""`admin` passes every permission check, so this path stays open to them."""
response = client.post(UPLOAD, files=_files(("a.csv", _csv())), headers=admin_headers)
assert response.status_code == 202, response.text
assert submitted == [response.json()["batch_id"]]
def test_uploader_key_is_refused_on_every_admin_route(client, upload_headers):
admin_routes = [
("post", "/api/admin/catalog-batch/preview"),
("post", "/api/admin/catalog-batch/ingest"),
("get", "/api/admin/catalog-batch/batches"),
("get", "/api/admin/catalog-batch/batches/anything"),
("post", "/api/admin/catalog-batch/batches/anything/resume"),
("post", "/api/admin/catalog-batch/batches/anything/cancel"),
("post", "/api/admin/store-catalog/ingest"),
("post", "/api/system/init"),
]
for method, path in admin_routes:
response = getattr(client, method)(path, headers=upload_headers)
assert response.status_code == 403, f"{method.upper()} {path} -> {response.status_code}"
def test_uploader_key_has_none_of_the_user_roles_write_access(client, upload_headers):
"""The reason this is its own role rather than a reuse of `user`."""
blocked = [
("post", "/api/user/products/add"),
("post", "/api/user/products/batch-add"),
("post", "/api/user/products/upload-file"),
("post", "/api/upload/stores"),
]
for method, path in blocked:
response = getattr(client, method)(path, headers=upload_headers)
assert response.status_code == 403, f"{method.upper()} {path} -> {response.status_code}"
def test_uploader_role_grants_exactly_one_permission():
"""A permission added here is a permission granted to an outside party."""
from app.infrastructure.security import ROLE_PERMISSIONS
assert ROLE_PERMISSIONS["uploader"] == ["upload_catalog"]
def test_api_keys_accept_the_uploader_role():
from app.infrastructure.settings import _parse_api_keys
parsed = _parse_api_keys(f"catalog-drop:uploader:{UPLOAD_KEY}")
assert parsed[UPLOAD_KEY] == ("catalog-drop", "uploader")
# ---------------------------------------------------------------------------
# Bad input is refused while the sender is still here to be told
# ---------------------------------------------------------------------------
def test_a_broken_sheet_is_refused_and_nothing_is_queued(client, upload_headers, submitted):
response = client.post(UPLOAD, files=_files(("notes.txt", b"this is not a spreadsheet")),
headers=upload_headers)
assert response.status_code == 400
assert submitted == []
def test_a_sheet_without_the_required_columns_is_refused(client, upload_headers, submitted):
response = client.post(UPLOAD, files=_files(("wrong.csv", b"colour,size\nred,10\n")),
headers=upload_headers)
assert response.status_code == 400
assert submitted == []
def test_a_bad_file_among_good_ones_is_reported_not_fatal(client, upload_headers, submitted):
"""A partial drop is a partial success: the good files still run."""
response = client.post(
UPLOAD,
files=_files(("good.xlsx", _sheet()), ("bad.txt", b"nope")),
headers=upload_headers,
)
assert response.status_code == 202, response.text
body = response.json()
assert submitted == [body["batch_id"]]
by_name = {f["filename"]: f for f in body["files"]}
assert by_name["good.xlsx"]["status"] == "queued"
assert by_name["bad.txt"]["status"] == "failed"
assert by_name["bad.txt"]["detail"]
assert "bad.txt" not in body["message"] or "1 could not be read" in body["message"]
def test_no_files_is_refused(client, upload_headers):
# An empty multipart body fails FastAPI's own validation before the handler.
assert client.post(UPLOAD, headers=upload_headers).status_code == 422
def test_an_empty_file_is_refused(client, upload_headers, submitted):
assert client.post(UPLOAD, files=_files(("empty.csv", b"")),
headers=upload_headers).status_code == 400
assert submitted == []
def test_too_many_files_in_one_drop_is_refused(client, upload_headers, monkeypatch, submitted):
from app.api.routers import uploads
monkeypatch.setattr(uploads, "BATCH_MAX_FILES", 2)
response = client.post(
UPLOAD,
files=_files(("a.csv", _csv()), ("b.csv", _csv()), ("c.csv", _csv())),
headers=upload_headers,
)
assert response.status_code == 413
assert submitted == []
def test_a_file_over_the_size_limit_is_refused(client, upload_headers, monkeypatch, submitted):
from app.api.routers import uploads
monkeypatch.setattr(uploads, "MAX_UPLOAD_BYTES", 64)
response = client.post(UPLOAD, files=_files(("big.xlsx", _sheet())),
headers=upload_headers)
assert response.status_code == 413
assert submitted == []
def test_too_many_rows_across_the_drop_is_refused(client, upload_headers, monkeypatch, submitted):
from app.api.routers import uploads
monkeypatch.setattr(uploads, "BATCH_MAX_TOTAL_ROWS", 3)
rows = [["Amul Butter %d" % i, "Butter", "Amul"] for i in range(4)]
response = client.post(UPLOAD, files=_files(("many.csv", _csv(rows))),
headers=upload_headers)
assert response.status_code == 413
assert submitted == []
def test_uploaded_filenames_cannot_escape_the_batch_directory(client, upload_headers, batch_root):
"""UploadFile.filename is caller-controlled and is used to build a path."""
response = client.post(UPLOAD, files=_files(("../../evil.csv", _csv())),
headers=upload_headers)
assert response.status_code == 202, response.text
directory = batch_root / response.json()["batch_id"]
for entry in batch_ingest.read_manifest(response.json()["batch_id"]).files:
assert ".." not in entry.stored_name
assert (directory / entry.stored_name).resolve().parent == directory.resolve()
assert not (batch_root.parent / "evil.csv").exists()
def test_a_full_queue_is_refused_but_the_batch_is_saved(client, upload_headers, monkeypatch):
"""The caller has no Resume button, so they are told to retry - but the
files are on disk and an admin can start them."""
import queue
from app.core import batch_worker
def full(_batch_id):
raise queue.Full()
monkeypatch.setattr(batch_worker, "submit", full)
response = client.post(UPLOAD, files=_files(("a.csv", _csv())), headers=upload_headers)
assert response.status_code == 429
detail = response.json()["detail"]
assert "retry" in detail.lower()
# The id is in the message precisely so an admin can find it.
staged = batch_ingest.list_manifests()
assert len(staged) == 1
assert staged[0].batch_id in detail
assert staged[0].status == "queued"
# ---------------------------------------------------------------------------
# Polling, and seeing only your own
# ---------------------------------------------------------------------------
def test_the_sender_can_poll_their_own_batch(client, upload_headers):
batch_id = client.post(UPLOAD, files=_files(("a.csv", _csv())),
headers=upload_headers).json()["batch_id"]
response = client.get(f"{UPLOAD}/{batch_id}", headers=upload_headers)
assert response.status_code == 200
assert response.json()["batch_id"] == batch_id
assert response.json()["files"][0]["filename"] == "a.csv"
def test_progress_is_visible_after_the_batch_runs(client, upload_headers, store):
batch_id = client.post(UPLOAD, files=_files(("a.csv", _csv())),
headers=upload_headers).json()["batch_id"]
# The worker passes on_change=batch_job_store.put; calling run_batch
# directly must do the same, or the cache in front of the manifest is stale
# and the poll answers with the state as it was at staging time.
batch_ingest.run_batch(batch_id, on_change=batch_job_store.put)
body = client.get(f"{UPLOAD}/{batch_id}", headers=upload_headers).json()
assert body["status"] == "done"
assert body["files_done"] == 1
assert body["files"][0]["result"] is not None
def test_another_callers_batch_is_not_found(client, upload_headers, other_headers):
"""404 rather than 403 - whether an id exists is not their business."""
batch_id = client.post(UPLOAD, files=_files(("a.csv", _csv())),
headers=upload_headers).json()["batch_id"]
assert client.get(f"{UPLOAD}/{batch_id}", headers=other_headers).status_code == 404
def test_a_batch_an_admin_uploaded_is_not_visible_to_an_api_client(
client, admin_headers, upload_headers
):
"""Admin batches carry no submitted_by, and an empty owner matches nobody."""
batch_id = client.post("/api/admin/catalog-batch/ingest",
files=_files(("a.csv", _csv())),
headers=admin_headers).json()["batch_id"]
assert batch_ingest.read_manifest(batch_id).submitted_by is None
assert client.get(f"{UPLOAD}/{batch_id}", headers=upload_headers).status_code == 404
def test_listing_shows_only_this_callers_batches(client, upload_headers, other_headers):
mine = client.post(UPLOAD, files=_files(("mine.csv", _csv())),
headers=upload_headers).json()["batch_id"]
theirs = client.post(UPLOAD, files=_files(("theirs.csv", _csv())),
headers=other_headers).json()["batch_id"]
listed = client.get(UPLOAD, headers=upload_headers).json()["batches"]
ids = [b["batch_id"] for b in listed]
assert ids == [mine]
assert theirs not in ids
def test_an_admin_may_see_any_batch(client, admin_headers, upload_headers):
batch_id = client.post(UPLOAD, files=_files(("a.csv", _csv())),
headers=upload_headers).json()["batch_id"]
assert client.get(f"{UPLOAD}/{batch_id}", headers=admin_headers).status_code == 200
def test_an_unknown_batch_is_not_found(client, upload_headers):
assert client.get(f"{UPLOAD}/nosuchbatch", headers=upload_headers).status_code == 404
def test_a_traversing_batch_id_is_not_found(client, upload_headers):
"""batch_dir() validates the id; the route must surface that as a 404."""
assert client.get(f"{UPLOAD}/..%2F..%2Fetc", headers=upload_headers).status_code == 404
def test_reads_require_a_credential(client):
assert client.get(UPLOAD).status_code == 401
assert client.get(f"{UPLOAD}/anything").status_code == 401