From 7aa01dd0427eb13a41998ddc8959d91d0b549666 Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Fri, 26 Jul 2024 12:41:31 +0100 Subject: [PATCH 1/4] fix(backend): correctly init and close database in fastapi startup lifecycle --- src/backend/app/main.py | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/backend/app/main.py b/src/backend/app/main.py index 213ed195..64981266 100644 --- a/src/backend/app/main.py +++ b/src/backend/app/main.py @@ -1,17 +1,20 @@ import os import logging import sys +from contextlib import asynccontextmanager +from loguru import logger as log from fastapi import FastAPI, Request from fastapi.middleware.cors import CORSMiddleware +from fastapi.templating import Jinja2Templates +from fastapi.responses import RedirectResponse, JSONResponse + from app.config import settings from app.projects import project_routes from app.waypoints import waypoint_routes -from fastapi.responses import RedirectResponse, JSONResponse from app.users import oauth_routes from app.users import user_routes from app.tasks import task_routes -from loguru import logger as log -from fastapi.templating import Jinja2Templates +from app.db.database import db_connection root = os.path.dirname(os.path.abspath(__file__)) @@ -101,6 +104,21 @@ def get_application() -> FastAPI: return _app +@asynccontextmanager +async def lifespan( + app: FastAPI, +): + """FastAPI startup/shutdown event.""" + log.debug("Starting up FastAPI server.") + await db_connection.connect() + + yield + + # Shutdown events + log.debug("Shutting down FastAPI server.") + await db_connection.disconnect() + + api = get_application() From 0e2b512fb8056c0820175335c384b81cb58296c9 Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Fri, 26 Jul 2024 12:42:24 +0100 Subject: [PATCH 2/4] refactor(backend): replace sqlalchemy db init entirely with encode/databases --- src/backend/app/db/database.py | 40 +++---------- src/backend/app/db/db_models.py | 5 +- src/backend/app/tasks/task_routes.py | 6 +- src/backend/app/users/oauth_routes.py | 2 +- src/backend/app/users/user_deps.py | 81 +++++++++++++-------------- src/backend/app/users/user_routes.py | 4 +- 6 files changed, 57 insertions(+), 81 deletions(-) diff --git a/src/backend/app/db/database.py b/src/backend/app/db/database.py index fa1c2d35..fa7cce69 100644 --- a/src/backend/app/db/database.py +++ b/src/backend/app/db/database.py @@ -1,10 +1,7 @@ """Config for the DTM database connection.""" + from databases import Database from app.config import settings -from sqlalchemy import create_engine -from sqlalchemy.orm import declarative_base, sessionmaker - -Base = declarative_base() class DatabaseConnection: @@ -12,16 +9,9 @@ class DatabaseConnection: def __init__(self): self.database = Database( - settings.DTM_DB_URL.unicode_string(), min_size=5, max_size=20 - ) - # self.database = Database(settings.DTM_DB_URL.unicode_string()) - self.engine = create_engine( settings.DTM_DB_URL.unicode_string(), - pool_size=20, - max_overflow=-1, - ) - self.SessionLocal = sessionmaker( - autocommit=False, autoflush=False, bind=self.engine + min_size=5, + max_size=20, ) async def connect(self): @@ -32,27 +22,11 @@ async def disconnect(self): """Disconnect from the database.""" await self.database.disconnect() - def create_db_session(self): - """Create a new SQLAlchemy DB session.""" - db = self.SessionLocal() - try: - return db - finally: - db.close() - - -db_connection = DatabaseConnection() # Create a single instance - -def get_db(): - """Yield a new database session.""" - return db_connection.create_db_session() +db_connection = DatabaseConnection() -async def encode_db(): +async def get_db(): """Get the encode database connection""" - try: - await db_connection.connect() - yield db_connection.database - finally: - await db_connection.disconnect() + await db_connection.connect() + yield db_connection.database diff --git a/src/backend/app/db/db_models.py b/src/backend/app/db/db_models.py index f07d8d6f..1b2f2d0f 100644 --- a/src/backend/app/db/db_models.py +++ b/src/backend/app/db/db_models.py @@ -14,9 +14,9 @@ ARRAY, LargeBinary, ) +from sqlalchemy.orm import declarative_base from sqlalchemy.dialects.postgresql import UUID -from app.db.database import Base from geoalchemy2 import Geometry, WKBElement from app.models.enums import ( TaskStatus, @@ -33,6 +33,9 @@ from app.utils import timestamp +Base = declarative_base() + + class DbUser(Base): __tablename__ = "users" diff --git a/src/backend/app/tasks/task_routes.py b/src/backend/app/tasks/task_routes.py index 0520f428..5d72031b 100644 --- a/src/backend/app/tasks/task_routes.py +++ b/src/backend/app/tasks/task_routes.py @@ -21,7 +21,7 @@ @router.get("/states/{project_id}") async def task_states( - project_id: uuid.UUID, db: Database = Depends(database.encode_db) + project_id: uuid.UUID, db: Database = Depends(database.get_db) ): """Get all tasks states for a project.""" @@ -35,7 +35,7 @@ async def new_event( task_id: uuid.UUID, detail: task_schemas.NewEvent, user_data: AuthUser = Depends(login_required), - db: Database = Depends(database.encode_db), + db: Database = Depends(database.get_db), ): user_id = user_data.id @@ -205,7 +205,7 @@ async def new_event( async def get_pending_tasks( project_id: uuid.UUID, user_data: AuthUser = Depends(login_required), - db: Database = Depends(database.encode_db), + db: Database = Depends(database.get_db), ): """Get a list of pending tasks for a specific project and user.""" user_id = user_data.id diff --git a/src/backend/app/users/oauth_routes.py b/src/backend/app/users/oauth_routes.py index 942d92fd..071d1c21 100644 --- a/src/backend/app/users/oauth_routes.py +++ b/src/backend/app/users/oauth_routes.py @@ -62,7 +62,7 @@ async def update_token(user_data: AuthUser = Depends(login_required)): @router.get("/my-info/") async def my_data( - db: Database = Depends(database.encode_db), + db: Database = Depends(database.get_db), user_data: AuthUser = Depends(login_required), ): """Read access token and get user details from Google""" diff --git a/src/backend/app/users/user_deps.py b/src/backend/app/users/user_deps.py index 5f424be9..b16a5e9d 100644 --- a/src/backend/app/users/user_deps.py +++ b/src/backend/app/users/user_deps.py @@ -13,50 +13,42 @@ from app.users.user_schemas import AuthUser from loguru import logger as log -reusable_oauth2 = OAuth2PasswordBearer(tokenUrl=f"{settings.API_PREFIX}/users/login") +# TODO do we need this code anymore? +# reusable_oauth2 = OAuth2PasswordBearer(tokenUrl=f"{settings.API_PREFIX}/users/login") +# # SessionDep = Annotated[ +# # Database, +# # Depends(database.get_db), +# # ] # SessionDep = Annotated[ -# Database, -# Depends(database.encode_db), +# Session, +# Depends(database.get_sqlalchemy_db), # ] -SessionDep = Annotated[ - Session, - Depends(database.get_db), -] -TokenDep = Annotated[str, Depends(reusable_oauth2)] - - -def get_current_user(session: SessionDep, token: TokenDep): - try: - payload = jwt.decode( - token, settings.SECRET_KEY, algorithms=[user_crud.ALGORITHM] - ) - token_data = user_schemas.TokenPayload(**payload) - - except (InvalidTokenError, ValidationError): - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Could not validate credentials", - ) - - user = session.get(DbUser, token_data.sub) - - if not user: - raise HTTPException(status_code=404, detail="User not found") - if not user.is_active: - raise HTTPException(status_code=400, detail="Inactive user") - return user - - -CurrentUser = Annotated[DbUser, Depends(get_current_user)] - - -def get_current_active_superuser(current_user: CurrentUser): - if not current_user.is_superuser: - raise HTTPException( - status_code=403, detail="The user doesn't have enough privileges" - ) - return current_user +# TokenDep = Annotated[str, Depends(reusable_oauth2)] +# def get_current_user(session: SessionDep, token: TokenDep): +# try: +# payload = jwt.decode( +# token, settings.SECRET_KEY, algorithms=[user_crud.ALGORITHM] +# ) +# token_data = user_schemas.TokenPayload(**payload) +# except (InvalidTokenError, ValidationError): +# raise HTTPException( +# status_code=status.HTTP_403_FORBIDDEN, +# detail="Could not validate credentials", +# ) +# user = session.get(DbUser, token_data.sub) +# if not user: +# raise HTTPException(status_code=404, detail="User not found") +# if not user.is_active: +# raise HTTPException(status_code=400, detail="Inactive user") +# return user +# CurrentUser = Annotated[DbUser, Depends(get_current_user)] +# def get_current_active_superuser(current_user: CurrentUser): +# if not current_user.is_superuser: +# raise HTTPException( +# status_code=403, detail="The user doesn't have enough privileges" +# ) +# return current_user async def init_google_auth(): @@ -81,6 +73,13 @@ async def login_required( request: Request, access_token: str = Header(None) ) -> AuthUser: """Dependency to inject into endpoints requiring login.""" + if settings.DEBUG: + return AuthUser( + id="0", + email="admin@hotosm.org", + name="admin", + img_url="", + ) if not access_token: raise HTTPException(status_code=401, detail="No access token provided") diff --git a/src/backend/app/users/user_routes.py b/src/backend/app/users/user_routes.py index b441b3c2..ec431209 100644 --- a/src/backend/app/users/user_routes.py +++ b/src/backend/app/users/user_routes.py @@ -24,7 +24,7 @@ @router.post("/login/") async def login_access_token( form_data: Annotated[OAuth2PasswordRequestForm, Depends()], - db: Database = Depends(database.encode_db), + db: Database = Depends(database.get_db), ) -> Token: """ OAuth2 compatible token login, get an access token for future requests @@ -53,7 +53,7 @@ async def login_access_token( async def update_user_profile( user_id: str, profile_update: ProfileUpdate, - db: Database = Depends(database.encode_db), + db: Database = Depends(database.get_db), user_data: AuthUser = Depends(login_required), ): """ From 963479c2893137333f5abdfdd74e0b079fa9fbf0 Mon Sep 17 00:00:00 2001 From: spwoodcock Date: Fri, 26 Jul 2024 12:42:38 +0100 Subject: [PATCH 3/4] fix(backend): raw sql project deletion using encode/databases --- src/backend/app/projects/project_routes.py | 55 +++++++++++----------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/src/backend/app/projects/project_routes.py b/src/backend/app/projects/project_routes.py index aeca6709..d3d78af1 100644 --- a/src/backend/app/projects/project_routes.py +++ b/src/backend/app/projects/project_routes.py @@ -6,7 +6,6 @@ import geojson from datetime import timedelta from fastapi import APIRouter, HTTPException, Depends, UploadFile, File, Form -from sqlalchemy.orm import Session from loguru import logger as log from app.projects import project_schemas, project_crud from app.db import database @@ -26,9 +25,9 @@ @router.delete("/{project_id}", tags=["Projects"]) -def delete_project_by_id( +async def delete_project_by_id( project_id: uuid.UUID, - db: Session = Depends(database.get_db), + db: Database = Depends(database.get_db), user: AuthUser = Depends(login_required), ): """ @@ -36,7 +35,7 @@ def delete_project_by_id( Args: project_id (int): The ID of the project to delete. - db (Session): The database session dependency. + db (Database): The database session dependency. Returns: dict: A confirmation message. @@ -44,34 +43,34 @@ def delete_project_by_id( Raises: HTTPException: If the project is not found. """ - # Query for the project - project = ( - db.query(db_models.DbProject) - .filter(db_models.DbProject.id == project_id) - .first() - ) - if not project: - raise HTTPException(status_code=404, detail="Project not found.") - - # Query and delete associated tasks - tasks = ( - db.query(db_models.DbTask) - .filter(db_models.DbTask.project_id == project_id) - .all() + delete_query = """ + WITH deleted_project AS ( + DELETE FROM projects + WHERE id = :project_id + RETURNING id + ), deleted_tasks AS ( + DELETE FROM tasks + WHERE project_id = :project_id + RETURNING project_id + ) + SELECT id FROM deleted_project + """ + + result = await db.fetch_one( + query=delete_query, + values={"project_id": project_id} ) - for task in tasks: - db.delete(task) - - # Delete the project - db.delete(project) - db.commit() + + if not result: + raise HTTPException(status_code=404) + return {"message": f"Project ID: {project_id} is deleted successfully."} @router.post("/create_project", tags=["Projects"]) async def create_project( project_info: project_schemas.ProjectIn, - db: Database = Depends(database.encode_db), + db: Database = Depends(database.get_db), user_data: AuthUser = Depends(login_required), ): """Create a project in database.""" @@ -90,7 +89,7 @@ async def create_project( async def upload_project_task_boundaries( project_id: uuid.UUID, task_geojson: UploadFile = File(...), - db: Database = Depends(database.encode_db), + db: Database = Depends(database.get_db), user: AuthUser = Depends(login_required), ): """Set project task boundaries using split GeoJSON from frontend. @@ -209,7 +208,7 @@ async def generate_presigned_url( async def read_projects( skip: int = 0, limit: int = 100, - db: Database = Depends(database.encode_db), + db: Database = Depends(database.get_db), user_data: AuthUser = Depends(login_required), ): "Return all projects" @@ -222,7 +221,7 @@ async def read_projects( ) async def read_project( project_id: uuid.UUID, - db: Database = Depends(database.encode_db), + db: Database = Depends(database.get_db), user_data: AuthUser = Depends(login_required), ): """Get a specific project and all associated tasks by ID.""" From 9972a4e0127e683c9bd49442d82f2c81c034c38a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 26 Jul 2024 11:51:05 +0000 Subject: [PATCH 4/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .github/workflows/build_and_deploy.yml | 2 +- README.md | 5 ++--- docs/INSTALL.md | 1 - docs/about/about.md | 1 - docs/about/team.md | 2 +- mkdocs.yml | 2 +- .../mapping_approved_or_rejected.html | 8 ++++---- .../app/email_templates/mapping_requests.html | 7 +++---- src/backend/app/projects/project_routes.py | 12 ++++-------- src/backend/app/tasks/task_routes.py | 4 +--- src/backend/app/users/user_deps.py | 12 ++---------- 11 files changed, 19 insertions(+), 37 deletions(-) diff --git a/.github/workflows/build_and_deploy.yml b/.github/workflows/build_and_deploy.yml index 8c338a3c..8828c7d7 100644 --- a/.github/workflows/build_and_deploy.yml +++ b/.github/workflows/build_and_deploy.yml @@ -99,7 +99,7 @@ jobs: # Cleanup db upgrade containers docker compose --file contrib/pg-upgrade/docker-compose.yml down - + docker compose --file docker-compose.vm.yml --env-file .env up \ --detach --remove-orphans --force-recreate --pull=always env: diff --git a/README.md b/README.md index b7c08723..d4fa28f0 100644 --- a/README.md +++ b/README.md @@ -33,12 +33,11 @@ - **Drone TM** is an integrated digital public good solution designed to harness the power of the crowd to generate high-resolution aerial maps of any location. -This innovative platform provides drone pilots, particularly in developing -countries, with job opportunities while contributing to the creation of +This innovative platform provides drone pilots, particularly in developing +countries, with job opportunities while contributing to the creation of high-resolution datasets crucial for disaster response and community resilience. ## Problem Statement diff --git a/docs/INSTALL.md b/docs/INSTALL.md index c1b5a76b..96025dd9 100644 --- a/docs/INSTALL.md +++ b/docs/INSTALL.md @@ -1,2 +1 @@ # Installation Guide - diff --git a/docs/about/about.md b/docs/about/about.md index 0a2b3a34..50e08209 100644 --- a/docs/about/about.md +++ b/docs/about/about.md @@ -1,2 +1 @@ # 📖 History - diff --git a/docs/about/team.md b/docs/about/team.md index ef98dc76..a3a0ea77 100644 --- a/docs/about/team.md +++ b/docs/about/team.md @@ -1 +1 @@ -# The Drone TM Team \ No newline at end of file +# The Drone TM Team diff --git a/mkdocs.yml b/mkdocs.yml index c1a61734..b35d6689 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -72,4 +72,4 @@ nav: - Contribution Guidelines: CONTRIBUTING.md - Code of Conduct: https://docs.hotosm.org/code-of-conduct - FAQ: about/faq.md - - The Team: about/team.md \ No newline at end of file + - The Team: about/team.md diff --git a/src/backend/app/email_templates/mapping_approved_or_rejected.html b/src/backend/app/email_templates/mapping_approved_or_rejected.html index c7bf1b35..22e38e8d 100644 --- a/src/backend/app/email_templates/mapping_approved_or_rejected.html +++ b/src/backend/app/email_templates/mapping_approved_or_rejected.html @@ -115,9 +115,7 @@

{{ email_subject }}

Dear {{ drone_operator_name }},

-

- {{ email_body }} -

+

{{ email_body }}

Please find below the details of the {{ task_status }} task:

{{ task_status|capitalize }} Task Details

@@ -127,7 +125,9 @@

{{ task_status|capitalize }} Task Details

Description: {{ description }}

{% if task_status == 'approved' %} - Start Mapping + Start Mapping {% endif %}
diff --git a/src/backend/app/email_templates/mapping_requests.html b/src/backend/app/email_templates/mapping_requests.html index 6c9de9b9..d50d4706 100644 --- a/src/backend/app/email_templates/mapping_requests.html +++ b/src/backend/app/email_templates/mapping_requests.html @@ -125,11 +125,10 @@

Drone Tasking Manager Invite

Mapping Task Details

-

Drone Operator: {{drone_operator_name}}

-

Task ID: {{task_id}}

+

Drone Operator: {{drone_operator_name}}

+

Task ID: {{task_id}}

Project:{{project_name}}

-

- Description: {{description}}

+

Description: {{description}}

Start Mapping