diff --git a/frigate/api/event.py b/frigate/api/event.py index d37a547ba4..a007872e8a 100644 --- a/frigate/api/event.py +++ b/frigate/api/event.py @@ -386,7 +386,9 @@ def events_explore( limit: int = 10, allowed_cameras: list[str] = Depends(get_allowed_cameras_for_filter), ): - # get distinct labels for all events + if not allowed_cameras: + return JSONResponse(content=[]) + distinct_labels = ( Event.select(Event.label) .where(Event.camera << allowed_cameras) @@ -396,13 +398,31 @@ def events_explore( label_counts = {} + explore_columns = ( + Event.id, + Event.camera, + Event.label, + Event.sub_label, + Event.zones, + Event.start_time, + Event.end_time, + Event.has_clip, + Event.has_snapshot, + Event.plus_id, + Event.retain_indefinitely, + Event.top_score, + Event.false_positive, + Event.box, + Event.data, + ) + def event_generator(): for label_obj in distinct_labels.iterator(): label = label_obj.label # get most recent events for this label label_events = ( - Event.select() + Event.select(*explore_columns) .where((Event.label == label) & (Event.camera << allowed_cameras)) .order_by(Event.start_time.desc()) .limit(limit) @@ -484,22 +504,18 @@ async def event_ids(ids: str, request: Request): status_code=400, ) - for event_id in ids: - try: - event = Event.get(Event.id == event_id) - await require_camera_access(event.camera, request=request) - except DoesNotExist: - # we should not fail the entire request if an event is not found - continue - try: - events = Event.select().where(Event.id << ids).dicts().iterator() - return JSONResponse(list(events)) + events = list(Event.select().where(Event.id << ids).dicts().iterator()) except Exception: return JSONResponse( content=({"success": False, "message": "Events not found"}), status_code=400 ) + for event in events: + await require_camera_access(event["camera"], request=request) + + return JSONResponse(events) + @router.get( "/events/search", diff --git a/frigate/api/review.py b/frigate/api/review.py index 08705e3f9e..a3b1f0598d 100644 --- a/frigate/api/review.py +++ b/frigate/api/review.py @@ -9,7 +9,7 @@ import pandas as pd from fastapi import APIRouter, Request from fastapi.params import Depends from fastapi.responses import JSONResponse -from peewee import Case, DoesNotExist, IntegrityError, fn, operator +from peewee import Case, DoesNotExist, fn, operator from playhouse.shortcuts import model_to_dict from frigate.api.auth import ( @@ -171,11 +171,19 @@ async def review_ids(request: Request, ids: str): status_code=400, ) + try: + reviews = list( + ReviewSegment.select().where(ReviewSegment.id << ids).dicts().iterator() + ) + except Exception: + return JSONResponse( + content=({"success": False, "message": "Review segments not found"}), + status_code=400, + ) + + found_ids = {r["id"] for r in reviews} for review_id in ids: - try: - review = ReviewSegment.get(ReviewSegment.id == review_id) - await require_camera_access(review.camera, request=request) - except DoesNotExist: + if review_id not in found_ids: return JSONResponse( content=( {"success": False, "message": f"Review {review_id} not found"} @@ -183,16 +191,10 @@ async def review_ids(request: Request, ids: str): status_code=404, ) - try: - reviews = ( - ReviewSegment.select().where(ReviewSegment.id << ids).dicts().iterator() - ) - return JSONResponse(list(reviews)) - except Exception: - return JSONResponse( - content=({"success": False, "message": "Review segments not found"}), - status_code=400, - ) + for review in reviews: + await require_camera_access(review["camera"], request=request) + + return JSONResponse(reviews) @router.get( @@ -489,27 +491,52 @@ async def set_multiple_reviewed( user_id = current_user["username"] - for review_id in body.ids: - try: - review = ReviewSegment.get(ReviewSegment.id == review_id) - await require_camera_access(review.camera, request=request) - review_status = UserReviewStatus.get( - UserReviewStatus.user_id == user_id, - UserReviewStatus.review_segment == review_id, + reviews = list( + ReviewSegment.select(ReviewSegment.id, ReviewSegment.camera).where( + ReviewSegment.id << body.ids + ) + ) + + for review in reviews: + await require_camera_access(review.camera, request=request) + + found_ids = [r.id for r in reviews] + + if found_ids: + existing_statuses = list( + UserReviewStatus.select().where( + (UserReviewStatus.user_id == user_id) + & (UserReviewStatus.review_segment << found_ids) ) - # Update based on the reviewed parameter - if review_status.has_been_reviewed != body.reviewed: - review_status.has_been_reviewed = body.reviewed - review_status.save() - except DoesNotExist: - try: - UserReviewStatus.create( - user_id=user_id, - review_segment=ReviewSegment.get(id=review_id), - has_been_reviewed=body.reviewed, + ) + + status_by_review = {s.review_segment_id: s for s in existing_statuses} + + to_update = [] + to_create = [] + + for review_id in found_ids: + if review_id in status_by_review: + status = status_by_review[review_id] + if status.has_been_reviewed != body.reviewed: + status.has_been_reviewed = body.reviewed + to_update.append(status) + else: + to_create.append( + { + "user_id": user_id, + "review_segment_id": review_id, + "has_been_reviewed": body.reviewed, + } ) - except (DoesNotExist, IntegrityError): - pass + + if to_update: + UserReviewStatus.bulk_update( + to_update, fields=[UserReviewStatus.has_been_reviewed], batch_size=100 + ) + + if to_create: + UserReviewStatus.insert_many(to_create).on_conflict_ignore().execute() return JSONResponse( content=( diff --git a/frigate/test/http_api/test_http_review.py b/frigate/test/http_api/test_http_review.py index d13a7bd273..67b832e3f2 100644 --- a/frigate/test/http_api/test_http_review.py +++ b/frigate/test/http_api/test_http_review.py @@ -497,6 +497,43 @@ class TestHttpReview(BaseTestHttp): ) assert user_review.has_been_reviewed == True + def test_post_reviews_viewed_concurrent_duplicate_does_not_raise(self): + """Regression: concurrent requests marking the same review must not 500. + + Two requests can both SELECT and find no existing status, then both try + to INSERT, hitting the unique (user_id, review_segment) constraint. + on_conflict_ignore() must silently skip the duplicate instead of raising + an IntegrityError (which was previously caught with try/except). + """ + id = "123456.random" + with AuthTestClient(self.app): + super().insert_mock_review_segment(id) + + # Simulate the first request having already committed its insert. + self._insert_user_review_status(id, reviewed=True) + + # Simulate the second concurrent request attempting the same insert. + UserReviewStatus.insert_many( + [ + { + "user_id": self.user_id, + "review_segment_id": id, + "has_been_reviewed": True, + } + ] + ).on_conflict_ignore().execute() + + # Exactly one row should exist; no exception should have been raised. + count = ( + UserReviewStatus.select() + .where( + (UserReviewStatus.user_id == self.user_id) + & (UserReviewStatus.review_segment == id) + ) + .count() + ) + assert count == 1 + #################################################################################################################### ################################### POST reviews/delete Endpoint ################################################ #################################################################################################################### diff --git a/migrations/036_add_perf_indexes.py b/migrations/036_add_perf_indexes.py new file mode 100644 index 0000000000..9be193d6b4 --- /dev/null +++ b/migrations/036_add_perf_indexes.py @@ -0,0 +1,21 @@ +"""Peewee migrations -- 036_add_perf_indexes.py. + +Adds composite/single-column indexes to speed up single-camera queries +issued by the web UI. + +""" + +import peewee as pw + +SQL = pw.SQL + + +def migrate(migrator, database, fake=False, **kwargs): + migrator.sql( + 'CREATE INDEX IF NOT EXISTS "event_camera_start_time" ' + 'ON "event" ("camera", "start_time" DESC)' + ) + + +def rollback(migrator, database, fake=False, **kwargs): + migrator.sql('DROP INDEX IF EXISTS "event_camera_start_time"')