From 5e7ae54215742d8e2c07260f84ee23a93fa2a65d Mon Sep 17 00:00:00 2001 From: swve Date: Sun, 16 Mar 2025 11:34:41 +0100 Subject: [PATCH] refactor: optimize functions queries --- .../services/courses/activities/activities.py | 53 ++++----- apps/api/src/services/courses/courses.py | 104 ++++++++++++------ apps/api/src/services/orgs/orgs.py | 64 +++++------ 3 files changed, 115 insertions(+), 106 deletions(-) diff --git a/apps/api/src/services/courses/activities/activities.py b/apps/api/src/services/courses/activities/activities.py index 0d738530..d758358b 100644 --- a/apps/api/src/services/courses/activities/activities.py +++ b/apps/api/src/services/courses/activities/activities.py @@ -92,24 +92,21 @@ async def get_activity( current_user: PublicUser, db_session: Session, ): - statement = select(Activity).where(Activity.activity_uuid == activity_uuid) - activity = db_session.exec(statement).first() + # Optimize by joining Activity with Course in a single query + statement = ( + select(Activity, Course) + .join(Course) + .where(Activity.activity_uuid == activity_uuid) + ) + result = db_session.exec(statement).first() - if not activity: + if not result: raise HTTPException( status_code=404, detail="Activity not found", ) - - # Get course from that activity - statement = select(Course).where(Course.id == activity.course_id) - course = db_session.exec(statement).first() - - if not course: - raise HTTPException( - status_code=404, - detail="Course not found", - ) + + activity, course = result # RBAC check await rbac_check(request, course.course_uuid, current_user, "read", db_session) @@ -124,9 +121,8 @@ async def get_activity( activity_read = ActivityRead.model_validate(activity) activity_read.content = activity_read.content if has_paid_access else { "paid_access": False } - activity = activity_read - return activity + return activity_read async def get_activityby_id( request: Request, @@ -134,31 +130,26 @@ async def get_activityby_id( current_user: PublicUser, db_session: Session, ): - statement = select(Activity).where(Activity.id == activity_id) - activity = db_session.exec(statement).first() + # Optimize by joining Activity with Course in a single query + statement = ( + select(Activity, Course) + .join(Course) + .where(Activity.id == activity_id) + ) + result = db_session.exec(statement).first() - if not activity: + if not result: raise HTTPException( status_code=404, detail="Activity not found", ) - - # Get course from that activity - statement = select(Course).where(Course.id == activity.course_id) - course = db_session.exec(statement).first() - - if not course: - raise HTTPException( - status_code=404, - detail="Course not found", - ) + + activity, course = result # RBAC check await rbac_check(request, course.course_uuid, current_user, "read", db_session) - activity = ActivityRead.model_validate(activity) - - return activity + return ActivityRead.model_validate(activity) async def update_activity( diff --git a/apps/api/src/services/courses/courses.py b/apps/api/src/services/courses/courses.py index 42b9b49d..afbd53c9 100644 --- a/apps/api/src/services/courses/courses.py +++ b/apps/api/src/services/courses/courses.py @@ -27,6 +27,7 @@ from src.security.rbac.rbac import ( from src.services.courses.thumbnails import upload_thumbnail from fastapi import HTTPException, Request, UploadFile from datetime import datetime +import asyncio async def get_course( @@ -106,6 +107,7 @@ async def get_course_meta( # Avoid circular import from src.services.courses.chapters import get_course_chapters + # Get course with a single query course_statement = select(Course).where(Course.course_uuid == course_uuid) course = db_session.exec(course_statement).first() @@ -118,36 +120,51 @@ async def get_course_meta( # RBAC check await rbac_check(request, course.course_uuid, current_user, "read", db_session) - # Get course authors - authors_statement = ( - select(User) - .join(ResourceAuthor) - .where(ResourceAuthor.resource_uuid == course.course_uuid) - ) - authors = db_session.exec(authors_statement).all() - - # convert from User to UserRead - authors = [UserRead.model_validate(author) for author in authors] - - course = CourseRead(**course.model_dump(), authors=authors) - - # Get course chapters - chapters = await get_course_chapters(request, course.id, db_session, current_user) - - # Trail - trail = None - - if isinstance(current_user, AnonymousUser): - trail = None - else: - trail = await get_user_trail_with_orgid( + # Start async tasks concurrently + tasks = [] + + # Task 1: Get course authors + async def get_authors(): + authors_statement = ( + select(User) + .join(ResourceAuthor) + .where(ResourceAuthor.resource_uuid == course.course_uuid) + ) + return db_session.exec(authors_statement).all() + + # Task 2: Get course chapters + async def get_chapters(): + # Ensure course.id is not None + if course.id is None: + return [] + return await get_course_chapters(request, course.id, db_session, current_user) + + # Task 3: Get user trail (only for authenticated users) + async def get_trail(): + if isinstance(current_user, AnonymousUser): + return None + return await get_user_trail_with_orgid( request, current_user, course.org_id, db_session ) - + + # Add tasks to the list + tasks.append(get_authors()) + tasks.append(get_chapters()) + tasks.append(get_trail()) + + # Run all tasks concurrently + authors_raw, chapters, trail = await asyncio.gather(*tasks) + + # Convert authors from User to UserRead + authors = [UserRead.model_validate(author) for author in authors_raw] + + # Create course read model + course_read = CourseRead(**course.model_dump(), authors=authors) + return FullCourseReadWithTrail( - **course.model_dump(), + **course_read.model_dump(), chapters=chapters, - trail=trail if trail else None, + trail=trail, ) async def get_courses_orgslug( @@ -196,19 +213,34 @@ async def get_courses_orgslug( query = query.offset(offset).limit(limit).distinct() courses = db_session.exec(query).all() - - # Fetch authors for each course + + if not courses: + return [] + + # Get all course UUIDs + course_uuids = [course.course_uuid for course in courses] + + # Fetch all authors for all courses in a single query + authors_query = ( + select(ResourceAuthor, User) + .join(User, ResourceAuthor.user_id == User.id) # type: ignore + .where(ResourceAuthor.resource_uuid.in_(course_uuids)) # type: ignore + ) + + author_results = db_session.exec(authors_query).all() + + # Create a dictionary mapping course_uuid to list of authors + course_authors = {} + for resource_author, user in author_results: + if resource_author.resource_uuid not in course_authors: + course_authors[resource_author.resource_uuid] = [] + course_authors[resource_author.resource_uuid].append(UserRead.model_validate(user)) + + # Create CourseRead objects with authors course_reads = [] for course in courses: - authors_query = ( - select(User) - .join(ResourceAuthor, ResourceAuthor.user_id == User.id) # type: ignore - .where(ResourceAuthor.resource_uuid == course.course_uuid) - ) - authors = db_session.exec(authors_query).all() - course_read = CourseRead.model_validate(course) - course_read.authors = [UserRead.model_validate(author) for author in authors] + course_read.authors = course_authors.get(course.course_uuid, []) course_reads.append(course_read) return course_reads diff --git a/apps/api/src/services/orgs/orgs.py b/apps/api/src/services/orgs/orgs.py index 9f580b28..bdfcd156 100644 --- a/apps/api/src/services/orgs/orgs.py +++ b/apps/api/src/services/orgs/orgs.py @@ -529,39 +529,31 @@ async def get_orgs_by_user_admin( page: int = 1, limit: int = 10, ) -> list[OrganizationRead]: - + # Join Organization, UserOrganization and OrganizationConfig in a single query statement = ( - select(Organization) + select(Organization, OrganizationConfig) .join(UserOrganization) + .outerjoin(OrganizationConfig) .where( UserOrganization.user_id == user_id, UserOrganization.role_id == 1, # Only where the user is admin + UserOrganization.org_id == Organization.id, + OrganizationConfig.org_id == Organization.id ) .offset((page - 1) * limit) .limit(limit) ) - # Get organizations where the user is an admin + # Execute single query to get all data result = db_session.exec(statement) - orgs = result.all() + org_data = result.all() + # Process results in memory orgsWithConfig = [] - - for org in orgs: - - # Get org config - statement = select(OrganizationConfig).where( - OrganizationConfig.org_id == org.id - ) - result = db_session.exec(statement) - - org_config = result.first() - + for org, org_config in org_data: config = OrganizationConfig.model_validate(org_config) if org_config else {} - - org = OrganizationRead(**org.model_dump(), config=config) - - orgsWithConfig.append(org) + org_read = OrganizationRead(**org.model_dump(), config=config) + orgsWithConfig.append(org_read) return orgsWithConfig @@ -573,36 +565,30 @@ async def get_orgs_by_user( page: int = 1, limit: int = 10, ) -> list[OrganizationRead]: - + # Join Organization, UserOrganization and OrganizationConfig in a single query statement = ( - select(Organization) + select(Organization, OrganizationConfig) .join(UserOrganization) - .where(UserOrganization.user_id == user_id) + .outerjoin(OrganizationConfig) + .where( + UserOrganization.user_id == user_id, + UserOrganization.org_id == Organization.id, + OrganizationConfig.org_id == Organization.id + ) .offset((page - 1) * limit) .limit(limit) ) - # Get organizations where the user is an admin + # Execute single query to get all data result = db_session.exec(statement) - orgs = result.all() + org_data = result.all() + # Process results in memory orgsWithConfig = [] - - for org in orgs: - - # Get org config - statement = select(OrganizationConfig).where( - OrganizationConfig.org_id == org.id - ) - result = db_session.exec(statement) - - org_config = result.first() - + for org, org_config in org_data: config = OrganizationConfig.model_validate(org_config) if org_config else {} - - org = OrganizationRead(**org.model_dump(), config=config) - - orgsWithConfig.append(org) + org_read = OrganizationRead(**org.model_dump(), config=config) + orgsWithConfig.append(org_read) return orgsWithConfig