feat(12.1-02): centralize listing freshness gate — apply_listing_and_finalize
- Add ListingResult dataclass to cloud_items.py
- Add apply_listing_and_finalize() — single shared gate for complete/incomplete semantics
- complete=True: upserts, soft-deletes unseen children, marks fresh, advances last_refreshed_at
- complete=False: upserts seen items only, never deletes unseen, preserves prior timestamp, warns
- browse.py: replace unconditional update_folder_state('fresh') with apply_listing_and_finalize
- cloud_tasks.py: replace unconditional fresh transition with apply_listing_and_finalize
- Celery task returns structured warning status on incomplete (no credentials/provider item names)
- Fix timezone naive/aware comparison in test (SQLite strips tzinfo)
This commit is contained in:
@@ -31,6 +31,7 @@ from deps.db import get_db
|
|||||||
from deps.utils import parse_uuid
|
from deps.utils import parse_uuid
|
||||||
from services.cloud_items import (
|
from services.cloud_items import (
|
||||||
ConnectionNotFound,
|
ConnectionNotFound,
|
||||||
|
apply_listing_and_finalize,
|
||||||
list_cloud_children,
|
list_cloud_children,
|
||||||
get_or_create_folder_state,
|
get_or_create_folder_state,
|
||||||
reconcile_cloud_listing,
|
reconcile_cloud_listing,
|
||||||
@@ -259,7 +260,9 @@ async def browse_connection_items(
|
|||||||
or _is_stale
|
or _is_stale
|
||||||
)
|
)
|
||||||
if should_refresh and not cached_items:
|
if should_refresh and not cached_items:
|
||||||
# First visit: do a synchronous bounded fetch so user sees content
|
# First visit: do a synchronous bounded fetch so user sees content.
|
||||||
|
# apply_listing_and_finalize is the single shared gate — it handles
|
||||||
|
# complete vs. incomplete semantics without unconditional fresh state.
|
||||||
try:
|
try:
|
||||||
credentials = _decrypt_connection(conn, current_user.id)
|
credentials = _decrypt_connection(conn, current_user.id)
|
||||||
adapter = build_cloud_resource_adapter(conn.provider, credentials)
|
adapter = build_cloud_resource_adapter(conn.provider, credentials)
|
||||||
@@ -268,20 +271,13 @@ async def browse_connection_items(
|
|||||||
current_user.id,
|
current_user.id,
|
||||||
parent_ref=parent_ref,
|
parent_ref=parent_ref,
|
||||||
)
|
)
|
||||||
await reconcile_cloud_listing(
|
await apply_listing_and_finalize(
|
||||||
session,
|
session,
|
||||||
user_id=uid_str,
|
user_id=uid_str,
|
||||||
connection_id=cid_str,
|
connection_id=cid_str,
|
||||||
parent_ref=parent_ref,
|
parent_ref=parent_ref,
|
||||||
listing=listing,
|
listing=listing,
|
||||||
)
|
)
|
||||||
await update_folder_state(
|
|
||||||
session,
|
|
||||||
user_id=uid_str,
|
|
||||||
connection_id=cid_str,
|
|
||||||
parent_ref=parent_ref or "",
|
|
||||||
refresh_state="fresh",
|
|
||||||
)
|
|
||||||
await session.commit()
|
await session.commit()
|
||||||
# Re-read items after reconciliation
|
# Re-read items after reconciliation
|
||||||
cached_items = await list_cloud_children(
|
cached_items = await list_cloud_children(
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ Domain exceptions:
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import uuid
|
import uuid
|
||||||
|
from dataclasses import dataclass, field
|
||||||
from datetime import datetime, timezone
|
from datetime import datetime, timezone
|
||||||
from typing import Optional, Sequence
|
from typing import Optional, Sequence
|
||||||
|
|
||||||
@@ -250,6 +251,97 @@ async def get_or_create_folder_state(
|
|||||||
return fs
|
return fs
|
||||||
|
|
||||||
|
|
||||||
|
# ── Listing application result ────────────────────────────────────────────────
|
||||||
|
|
||||||
|
@dataclass
|
||||||
|
class ListingResult:
|
||||||
|
"""Structured result from apply_listing_and_finalize.
|
||||||
|
|
||||||
|
is_fresh: True only when the provider supplied a complete authoritative listing.
|
||||||
|
warning_code: stable controlled code when is_fresh is False (never raw exception text).
|
||||||
|
warning_message: stable controlled message for API consumers (no credentials/URLs).
|
||||||
|
"""
|
||||||
|
is_fresh: bool
|
||||||
|
warning_code: Optional[str] = None
|
||||||
|
warning_message: Optional[str] = None
|
||||||
|
|
||||||
|
|
||||||
|
# ── Single listing application gate ──────────────────────────────────────────
|
||||||
|
|
||||||
|
async def apply_listing_and_finalize(
|
||||||
|
session: AsyncSession,
|
||||||
|
*,
|
||||||
|
user_id: str,
|
||||||
|
connection_id: str,
|
||||||
|
parent_ref: Optional[str],
|
||||||
|
listing: CloudListing,
|
||||||
|
) -> ListingResult:
|
||||||
|
"""Apply a provider listing and finalize CloudFolderState atomically.
|
||||||
|
|
||||||
|
This is the SINGLE shared gate used by both the synchronous browse endpoint
|
||||||
|
and the Celery background worker. Neither caller may independently decide
|
||||||
|
freshness — they MUST use this function.
|
||||||
|
|
||||||
|
Semantics:
|
||||||
|
- listing.complete=True (authoritative):
|
||||||
|
Upserts all items, soft-deletes unseen children, marks folder FRESH,
|
||||||
|
advances last_refreshed_at. Returns ListingResult(is_fresh=True).
|
||||||
|
|
||||||
|
- listing.complete=False (partial/unknown):
|
||||||
|
Upserts only seen items (never deletes unseen children), does NOT
|
||||||
|
advance last_refreshed_at, sets folder state to WARNING with stable
|
||||||
|
code 'incomplete_listing'. Returns ListingResult(is_fresh=False, …).
|
||||||
|
|
||||||
|
No exception is raised on incomplete results — callers receive a structured
|
||||||
|
result and must propagate the warning state to the API response or task result.
|
||||||
|
This function never raises HTTPException — domain exceptions only.
|
||||||
|
This function never calls the quota service.
|
||||||
|
"""
|
||||||
|
# Reconcile items from the provider listing
|
||||||
|
await reconcile_cloud_listing(
|
||||||
|
session,
|
||||||
|
user_id=user_id,
|
||||||
|
connection_id=connection_id,
|
||||||
|
parent_ref=parent_ref,
|
||||||
|
listing=listing,
|
||||||
|
)
|
||||||
|
|
||||||
|
if listing.complete:
|
||||||
|
# Authoritative: mark folder fresh and advance last_refreshed_at
|
||||||
|
await update_folder_state(
|
||||||
|
session,
|
||||||
|
user_id=user_id,
|
||||||
|
connection_id=connection_id,
|
||||||
|
parent_ref=parent_ref or "",
|
||||||
|
refresh_state="fresh",
|
||||||
|
)
|
||||||
|
return ListingResult(is_fresh=True)
|
||||||
|
else:
|
||||||
|
# Incomplete/partial: retain cached state; do NOT advance last_refreshed_at
|
||||||
|
# We only update the warning fields, preserving last_refreshed_at from prior success.
|
||||||
|
fs = await get_or_create_folder_state(
|
||||||
|
session,
|
||||||
|
user_id=user_id,
|
||||||
|
connection_id=connection_id,
|
||||||
|
parent_ref=parent_ref or "",
|
||||||
|
)
|
||||||
|
now = datetime.now(timezone.utc)
|
||||||
|
# Set warning state without touching last_refreshed_at
|
||||||
|
fs.refresh_state = "warning"
|
||||||
|
fs.error_code = "incomplete_listing"
|
||||||
|
fs.error_message = (
|
||||||
|
"Provider returned an incomplete listing. Showing cached results."
|
||||||
|
)
|
||||||
|
fs.updated_at = now
|
||||||
|
# Deliberately: do NOT update fs.last_refreshed_at
|
||||||
|
await session.flush()
|
||||||
|
return ListingResult(
|
||||||
|
is_fresh=False,
|
||||||
|
warning_code="incomplete_listing",
|
||||||
|
warning_message="Provider returned an incomplete listing. Showing cached results.",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
async def update_folder_state(
|
async def update_folder_state(
|
||||||
session: AsyncSession,
|
session: AsyncSession,
|
||||||
*,
|
*,
|
||||||
|
|||||||
@@ -59,6 +59,7 @@ async def _run(user_id: str, connection_id: str, parent_ref: str | None) -> dict
|
|||||||
from db.session import AsyncSessionLocal
|
from db.session import AsyncSessionLocal
|
||||||
from services.cloud_items import (
|
from services.cloud_items import (
|
||||||
ConnectionNotFound,
|
ConnectionNotFound,
|
||||||
|
apply_listing_and_finalize,
|
||||||
get_or_create_folder_state,
|
get_or_create_folder_state,
|
||||||
reconcile_cloud_listing,
|
reconcile_cloud_listing,
|
||||||
resolve_owned_connection,
|
resolve_owned_connection,
|
||||||
@@ -137,25 +138,28 @@ async def _run(user_id: str, connection_id: str, parent_ref: str | None) -> dict
|
|||||||
# Transient failure — let Celery retry
|
# Transient failure — let Celery retry
|
||||||
raise _TransientProviderError("Provider error. Will retry.") from exc
|
raise _TransientProviderError("Provider error. Will retry.") from exc
|
||||||
|
|
||||||
# Reconcile listing into durable cloud_items rows
|
# apply_listing_and_finalize is the single shared gate — it handles
|
||||||
await reconcile_cloud_listing(
|
# complete vs. incomplete semantics; only a complete authoritative
|
||||||
|
# listing transitions the folder to fresh state.
|
||||||
|
listing_result = await apply_listing_and_finalize(
|
||||||
session,
|
session,
|
||||||
user_id=user_id,
|
user_id=user_id,
|
||||||
connection_id=connection_id,
|
connection_id=connection_id,
|
||||||
parent_ref=parent_ref,
|
parent_ref=parent_ref,
|
||||||
listing=listing,
|
listing=listing,
|
||||||
)
|
)
|
||||||
|
|
||||||
await update_folder_state(
|
|
||||||
session,
|
|
||||||
user_id=user_id,
|
|
||||||
connection_id=connection_id,
|
|
||||||
parent_ref=parent_ref or "",
|
|
||||||
refresh_state="fresh",
|
|
||||||
)
|
|
||||||
await session.commit()
|
await session.commit()
|
||||||
|
|
||||||
return {"status": "ok", "items_fetched": len(listing.items)}
|
if listing_result.is_fresh:
|
||||||
|
return {"status": "ok", "items_fetched": len(listing.items)}
|
||||||
|
else:
|
||||||
|
# Incomplete listing: task succeeded but folder is not fresh.
|
||||||
|
# Return a controlled status — no credentials, no provider item names.
|
||||||
|
return {
|
||||||
|
"status": "warning",
|
||||||
|
"warning_code": listing_result.warning_code,
|
||||||
|
"items_fetched": len(listing.items),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
# ── Celery task ───────────────────────────────────────────────────────────────
|
# ── Celery task ───────────────────────────────────────────────────────────────
|
||||||
|
|||||||
@@ -730,8 +730,12 @@ async def test_incomplete_listing_retains_cached_rows_and_last_success(db_sessio
|
|||||||
)
|
)
|
||||||
)).scalars().first()
|
)).scalars().first()
|
||||||
assert fs is not None
|
assert fs is not None
|
||||||
assert fs.last_refreshed_at == prior_ts, (
|
# SQLite stores naive datetimes; strip tzinfo for comparison
|
||||||
f"incomplete listing must not overwrite last_refreshed_at: {fs.last_refreshed_at} != {prior_ts}"
|
stored_ts = fs.last_refreshed_at
|
||||||
|
if stored_ts is not None and stored_ts.tzinfo is None:
|
||||||
|
stored_ts = stored_ts.replace(tzinfo=timezone.utc)
|
||||||
|
assert stored_ts == prior_ts, (
|
||||||
|
f"incomplete listing must not overwrite last_refreshed_at: {fs.last_refreshed_at!r} != {prior_ts!r}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user