Skip to content

Commit adc4b15

Browse files
pmcfadinclaude
andcommitted
fix: replace $inc with read-modify-write $set for views counter
The Astra DB Table API does not support $inc (only $set, $unset, $push, $pullAll are supported). The previous code attempted $inc, failed with UNSUPPORTED_UPDATE_OPERATIONS_FOR_TABLE, logged a misleading "view tracking disabled" warning, then fell through to a $set fallback — a wasted round trip and a noisy false alarm on every view. Fix: go directly to read-modify-write with $set. Removes the $inc attempt, the complex two-if exception handler, and the _logged_views_disabled global. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 69a2179 commit adc4b15

2 files changed

Lines changed: 42 additions & 68 deletions

File tree

app/services/video_service.py

Lines changed: 13 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,6 @@
8888
logging.getLogger().getEffectiveLevel(),
8989
)
9090

91-
# Flag to track if we've logged the view tracking limitation warning
92-
_logged_views_disabled = False
93-
94-
9591
# ---------------------------------------------------------------------------
9692
# Helpers
9793
# ---------------------------------------------------------------------------
@@ -388,67 +384,24 @@ async def record_video_view(
388384
) -> None:
389385
"""Increment the view counter stored directly in the *videos* table.
390386
391-
NOTE: View tracking is currently disabled. The 'views' column exists in the
392-
CQL schema but is not yet exposed via the Astra DB Table API. This function
393-
will gracefully no-op until API support is added.
394-
395-
The dedicated ``video_playback_stats`` counter table is no longer updated –
396-
we instead mutate the new ``views`` bigint column in the primary table so
397-
the entire workflow remains Data-API-only.
387+
The Astra DB Table API does not support ``$inc``, so we use a read-modify-write
388+
cycle: fetch the current ``views`` value and write back with ``$set``.
398389
"""
399390

400391
if db_table is None:
401392
db_table = await get_table(VIDEOS_TABLE_NAME)
402393

403-
try:
404-
# Fast path – $inc is accepted on normal bigint columns
405-
await db_table.update_one(
406-
filter={"videoid": _uuid_for_db(video_id, db_table)},
407-
update={"$inc": {"views": 1}},
408-
upsert=True,
409-
)
410-
except DataAPIResponseException as exc:
411-
global _logged_views_disabled
412-
error_str = str(exc)
413-
414-
# Check if this is the known Table API limitation where the 'views' column
415-
# exists in CQL schema but isn't exposed via the Table API yet
416-
if (
417-
"UNKNOWN_TABLE_COLUMNS" in error_str
418-
or "UNSUPPORTED_UPDATE_OPERATIONS" in error_str
419-
):
420-
# Log warning once per process lifecycle to avoid log spam
421-
if not _logged_views_disabled:
422-
logger.warning(
423-
"View tracking is currently disabled. The 'views' column exists in "
424-
"the CQL schema (docs/schema-astra.cql:95) but is not yet exposed "
425-
"via the Astra DB Table API. Views will not be tracked until API "
426-
"support is added. Error codes: UNKNOWN_TABLE_COLUMNS / "
427-
"UNSUPPORTED_UPDATE_OPERATIONS_FOR_TABLE"
428-
)
429-
_logged_views_disabled = True
430-
pass # Gracefully no-op for views counter; continue to activity tracking below
431-
432-
# Some deployments (Astra *tables*) currently reject $inc on bigint –
433-
# fall back to a manual read-modify-write cycle.
434-
if (
435-
"Update operation not supported" in error_str
436-
or "unsupported operations" in error_str
437-
):
438-
current = (
439-
await db_table.find_one(
440-
filter={"videoid": _uuid_for_db(video_id, db_table)}
441-
)
442-
or {}
443-
)
444-
new_count = int(current.get("views", 0)) + 1
445-
await db_table.update_one(
446-
filter={"videoid": _uuid_for_db(video_id, db_table)},
447-
update={"$set": {"views": new_count}},
448-
upsert=True,
449-
)
450-
else:
451-
raise
394+
# The Astra DB Table API does not support $inc. Use read-modify-write with $set.
395+
current = (
396+
await db_table.find_one(filter={"videoid": _uuid_for_db(video_id, db_table)})
397+
or {}
398+
)
399+
new_count = int(current.get("views", 0)) + 1
400+
await db_table.update_one(
401+
filter={"videoid": _uuid_for_db(video_id, db_table)},
402+
update={"$set": {"views": new_count}},
403+
upsert=True,
404+
)
452405

453406
from app.services.user_activity_service import (
454407
record_user_activity,

tests/services/test_video_service.py

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -227,13 +227,11 @@ async def test_update_video_details_no_changes():
227227

228228
@pytest.mark.asyncio
229229
async def test_record_video_view_success():
230+
"""View counter uses read-modify-write ($set) — the Table API does not support $inc."""
230231
vid = uuid4()
231232

232-
# Mock the playback stats table passed explicitly
233233
mock_stats_table = AsyncMock()
234-
mock_stats_table.update_one.return_value = AsyncMock()
235-
236-
# Mock the activity table returned via get_table
234+
mock_stats_table.find_one.return_value = {"views": 5}
237235
mock_activity_table = AsyncMock()
238236

239237
with (
@@ -245,15 +243,14 @@ async def test_record_video_view_success():
245243
new_callable=AsyncMock,
246244
) as mock_record_user_activity,
247245
):
248-
# First call inside record_video_view is for VIDEO_PLAYBACK_STATS_TABLE_NAME
249-
# but we already pass mock_stats_table, so get_table will be used only once
250246
mock_get_table.return_value = mock_activity_table
251247

252248
await video_service.record_video_view(vid, db_table=mock_stats_table)
253249

254-
# Validate stats table increment
250+
# Must read current value first, then write the incremented value
251+
mock_stats_table.find_one.assert_awaited_once()
255252
mock_stats_table.update_one.assert_called_once_with(
256-
filter={"videoid": vid}, update={"$inc": {"views": 1}}, upsert=True
253+
filter={"videoid": vid}, update={"$set": {"views": 6}}, upsert=True
257254
)
258255

259256
# Validate activity table log
@@ -263,12 +260,34 @@ async def test_record_video_view_success():
263260
mock_record_user_activity.assert_awaited_once()
264261

265262

263+
@pytest.mark.asyncio
264+
async def test_record_video_view_no_existing_row():
265+
"""When the video has no existing views row, count starts at 1."""
266+
vid = uuid4()
267+
268+
mock_stats_table = AsyncMock()
269+
mock_stats_table.find_one.return_value = None # no row yet
270+
mock_activity_table = AsyncMock()
271+
272+
with (
273+
patch("app.services.video_service.get_table", new_callable=AsyncMock) as mock_get_table,
274+
patch("app.services.user_activity_service.record_user_activity", new_callable=AsyncMock),
275+
):
276+
mock_get_table.return_value = mock_activity_table
277+
await video_service.record_video_view(vid, db_table=mock_stats_table)
278+
279+
mock_stats_table.update_one.assert_called_once_with(
280+
filter={"videoid": vid}, update={"$set": {"views": 1}}, upsert=True
281+
)
282+
283+
266284
@pytest.mark.asyncio
267285
async def test_record_video_view_authenticated_user_activity():
268286
"""Authenticated view calls record_user_activity with real user ID."""
269287
vid = uuid4()
270288
viewer_id = uuid4()
271289
mock_stats_table = AsyncMock()
290+
mock_stats_table.find_one.return_value = None
272291
mock_activity_table = AsyncMock()
273292

274293
with (
@@ -301,6 +320,7 @@ async def test_record_video_view_anonymous_user_activity():
301320

302321
vid = uuid4()
303322
mock_stats_table = AsyncMock()
323+
mock_stats_table.find_one.return_value = None
304324
mock_activity_table = AsyncMock()
305325

306326
with (
@@ -331,6 +351,7 @@ async def test_record_video_view_user_activity_failure_does_not_break():
331351
"""If record_user_activity raises, the video view still succeeds."""
332352
vid = uuid4()
333353
mock_stats_table = AsyncMock()
354+
mock_stats_table.find_one.return_value = None
334355
mock_activity_table = AsyncMock()
335356

336357
with (

0 commit comments

Comments
 (0)