Skip to content

Commit

Permalink
feat(backend): remove SQLAlchemy and replace with async psycopg db dr…
Browse files Browse the repository at this point in the history
…iver (#1834)

* build: add psycopg async driver (v3)

* feat: create async psycopg db pool at app startup

* refactor: test usage of psycopg + pydantic models on project get endpoint

* build: relock deps after rebase

* build: migration remove additional redundant fields from projects

* refactor: work in progress removing sqlalchemy

* build: remove project_info table, merge into projects

* refactor: work in progress removing sqlalchemy

* build: migration to add default timestamptz to tables

* refactor: continue refactor to remove sqlalchemy

* refactor: continued migration from sqlalchemy --> psycopg

* refactor: continue sqlalchemy removal, fix endpoints

* fix: updating failed state for s3 upload not awaited

* refactor: fix background tasks and mbtiles / basemaps apis

* build: remove async-lru package

* refactor: refactor out project_deps.get_odk_credentials

* refactor: use HTTPStatus enum everywhere, format

* refactor: further updates to sqlalchemy removal

* refactor: working project routes

* refactor: rename project_path_prefix --> slug, remove unused project_info refs

* refactor: final type renames before task events refactor

* fix: handle error for /statuses endpoint

* fix: add project_id query param to project get

* fix: centroid loading on home page

* fix: project outline no longer nested in Feature

* fix: update frontend enums to use strings

* fix: enum usage on split task frontend

* fix: splitting of features into task areas during proj create

* fix: sometimes project missing hashtags

* fix: qrcode loading with project name

* fix: use EntityStatus enum

* fix: add ORDER BY to all get all statements

* fix: generate project working

* refactor: remove sqlalchemy once and for all!! (bye bye)

* fix: untested fixes for submission routes

* build: upgrade postgis image --> 14-3.5-alpine as 3.4 was removed

* refactor: remove manual enums since available in in python 3.11

* build: upgrade python --> 3.12 + all dependencies to latest

* refactor: fix imports, remove refs to db_models

* build: relock deps after osm-rawdata upgrade, optional sqlalchemy

* fix: awaiting AsyncConnectionPool def is incorrect

* fix: case where string is passed to dborg one func

* test: start fixing test cases

* fix: minor fixes to api

* test: continue work fixing / updating tests

* fix: error if creating project with single task

* refactor: remove author object from project response (author_id is enough)

* build: add composite index on task_history(task_id, action_date)

* fix: exclude last active from project input model

* refactor: rename locked_by_ fields to actioned_by_

* fix: getting project before update / delete

* fix: setting slug on project + org create

* fix: exclude outline field on ProjectIn to avoid dict insert error

* test: further fixes to tests

* fix: avoid spamming nominatim during tests / DEBUG=true

* fix: hashtags not set correctly on new project

* refactor(frontend): simplify upload-task-area logic during proj create

* fix: various fixes, all working tests

* fix: set some constraints on input project_id or org_id

* test: fix conftest, set server url to FMTM_DOMAIN for ci

* build: install uvloop + httptools for performance (uvicorn uses automatically)

* refactor: use asgi lifespan state over fastapi app state

* fix: use asgi_lifespan LifepanManager correctly for async testing

* fix: add async detailed debug mode when DEBUG=True

* test: correctly setup conftest with asyncclient lifespan

* test: update conftest to use db_conn override on fastapi app

* build: move api healthcheck to compose, increase startup grace period

* fix(frontend): styling based on task action not task status

* fix(frontend): tweak frontend to use TaskAction enum for now
  • Loading branch information
spwoodcock authored Oct 25, 2024
1 parent 5e4b25f commit 43336bb
Show file tree
Hide file tree
Showing 99 changed files with 5,374 additions and 6,969 deletions.
8 changes: 7 additions & 1 deletion contrib/playwright/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ services:
"critical",
"--no-access-log",
]
healthcheck:
test: ["CMD", "curl", "-f", "http://localhost:8000/__lbheartbeat__"]
start_period: 60s
interval: 10s
timeout: 5s
retries: 10

ui:
# This hostname is used for Playwright test internal networking
Expand All @@ -53,8 +59,8 @@ services:
api:
# Override: must be healthy before tests run
condition: service_healthy
# Override: must be healthy before tests run
ui:
# Override: must be healthy before tests run
condition: service_healthy
working_dir: /app
environment:
Expand Down
10 changes: 8 additions & 2 deletions docker-compose.development.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ services:
networks:
- fmtm-net
restart: "unless-stopped"
healthcheck:
test: ["CMD", "curl", "-f", "http://localhost:8000/__lbheartbeat__"]
start_period: 60s
interval: 10s
timeout: 5s
retries: 10
deploy:
replicas: ${API_REPLICAS:-2}
resources:
Expand Down Expand Up @@ -190,7 +196,7 @@ services:
timeout: 5s

fmtm-db:
image: "postgis/postgis:${POSTGIS_TAG:-14-3.4-alpine}"
image: "postgis/postgis:${POSTGIS_TAG:-14-3.5-alpine}"
command: -c 'max_connections=300'
volumes:
- fmtm_db_data:/var/lib/postgresql/data/
Expand All @@ -211,7 +217,7 @@ services:
retries: 3

central-db:
image: "postgis/postgis:${POSTGIS_TAG:-14-3.4-alpine}"
image: "postgis/postgis:${POSTGIS_TAG:-14-3.5-alpine}"
volumes:
- central_db_data:/var/lib/postgresql/data/
environment:
Expand Down
8 changes: 7 additions & 1 deletion docker-compose.main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ services:
networks:
- fmtm-net
restart: "unless-stopped"
healthcheck:
test: ["CMD", "curl", "-f", "http://localhost:8000/__lbheartbeat__"]
start_period: 60s
interval: 10s
timeout: 5s
retries: 10
deploy:
replicas: ${API_REPLICAS:-4}
resources:
Expand Down Expand Up @@ -130,7 +136,7 @@ services:
timeout: 5s

fmtm-db:
image: "postgis/postgis:${POSTGIS_TAG:-14-3.4-alpine}"
image: "postgis/postgis:${POSTGIS_TAG:-14-3.5-alpine}"
command: -c 'max_connections=300'
volumes:
- fmtm_db_data:/var/lib/postgresql/data/
Expand Down
4 changes: 2 additions & 2 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ services:
timeout: 5s

fmtm-db:
image: "postgis/postgis:${POSTGIS_TAG:-14-3.4-alpine}"
image: "postgis/postgis:${POSTGIS_TAG:-14-3.5-alpine}"
# Temp workaround until https://github.com/postgis/docker-postgis/issues/216
build:
context: https://github.com/postgis/docker-postgis.git#master:14-3.4/alpine
Expand Down Expand Up @@ -281,7 +281,7 @@ services:

central-db:
profiles: ["", "central"]
image: "postgis/postgis:${POSTGIS_TAG:-14-3.4-alpine}"
image: "postgis/postgis:${POSTGIS_TAG:-14-3.5-alpine}"
volumes:
- central_db_data:/var/lib/postgresql/data/
environment:
Expand Down
2 changes: 1 addition & 1 deletion docs/dev/Backend.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ just migrate

```json
{
"python.analysis.extraPaths": ["src/backend/__pypackages__/3.11/lib/"]
"python.analysis.extraPaths": ["src/backend/__pypackages__/3.12/lib/"]
}
```

Expand Down
5 changes: 1 addition & 4 deletions src/backend/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# You should have received a copy of the GNU General Public License
# along with FMTM. If not, see <https:#www.gnu.org/licenses/>.
#
ARG PYTHON_IMG_TAG=3.11
ARG PYTHON_IMG_TAG=3.12
ARG MINIO_TAG=${MINIO_TAG:-RELEASE.2024-10-13T13-34-11Z}
FROM docker.io/minio/minio:${MINIO_TAG} AS minio

Expand Down Expand Up @@ -140,9 +140,6 @@ VOLUME /opt/logs
VOLUME /opt/tiles
# Change to non-root user
USER appuser
# Add Healthcheck
HEALTHCHECK --start-period=10s --interval=5s --retries=20 --timeout=5s \
CMD curl --fail http://localhost:8000/__lbheartbeat__ || exit 1


# Add certificates to use ODK Central over SSL (HTTPS, required)
Expand Down
91 changes: 48 additions & 43 deletions src/backend/app/auth/auth_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@

"""Auth routes, to login, logout, and get user details."""

import time
from time import time
from typing import Annotated

from fastapi import APIRouter, Depends, HTTPException, Request, Response
from fastapi.responses import JSONResponse
from loguru import logger as log
from sqlalchemy import text
from sqlalchemy.orm import Session
from psycopg import Connection
from psycopg.rows import class_row

from app.auth.auth_schemas import AuthUser, AuthUserWithToken, FMTMUser
from app.auth.osm import (
Expand All @@ -37,8 +38,9 @@
verify_token,
)
from app.config import settings
from app.db import database
from app.models.enums import HTTPStatus, UserRole
from app.db.database import db_conn
from app.db.enums import HTTPStatus, UserRole
from app.db.models import DbUser

router = APIRouter(
prefix="/auth",
Expand All @@ -64,11 +66,13 @@ async def login_url(osm_auth=Depends(init_osm_auth)):
"""
login_url = osm_auth.login()
log.debug(f"Login URL returned: {login_url}")
return JSONResponse(content=login_url, status_code=200)
return JSONResponse(content=login_url, status_code=HTTPStatus.OK)


@router.get("/callback/")
async def callback(request: Request, osm_auth=Depends(init_osm_auth)) -> JSONResponse:
async def callback(
request: Request, osm_auth: Annotated[AuthUser, Depends(init_osm_auth)]
) -> JSONResponse:
"""Performs oauth token exchange with OpenStreetMap.
Provides an access token that can be used for authenticating other endpoints.
Expand Down Expand Up @@ -96,8 +100,8 @@ async def callback(request: Request, osm_auth=Depends(init_osm_auth)) -> JSONRes
user_data = {
"sub": f"fmtm|{osm_user['id']}",
"aud": settings.FMTM_DOMAIN,
"iat": int(time.time()),
"exp": int(time.time()) + 86400, # expiry set to 1 day
"iat": int(time()),
"exp": int(time()) + 86400, # expiry set to 1 day
"username": osm_user["username"],
"email": osm_user.get("email"),
"picture": osm_user.get("img_url"),
Expand Down Expand Up @@ -134,7 +138,7 @@ async def callback(request: Request, osm_auth=Depends(init_osm_auth)) -> JSONRes
@router.get("/logout/")
async def logout():
"""Reset httpOnly cookie to sign out user."""
response = Response(status_code=200)
response = Response(status_code=HTTPStatus.OK)
# Reset all cookies (logout)
fmtm_cookie_name = settings.FMTM_DOMAIN.replace(".", "_")
refresh_cookie_name = f"{fmtm_cookie_name}_refresh"
Expand All @@ -157,21 +161,17 @@ async def logout():


async def get_or_create_user(
db: Session,
db: Connection,
user_data: AuthUser,
):
"""Get user from User table if exists, else create."""
try:
upsert_sql = text(
"""
upsert_sql = """
WITH upserted_user AS (
INSERT INTO users (
id, username, profile_img, role, mapping_level,
is_email_verified, is_expert, tasks_mapped, tasks_validated,
tasks_invalidated, registered_at
id, username, profile_img, role, registered_at
) VALUES (
:user_id, :username, :profile_img, :role,
'BEGINNER', FALSE, FALSE, 0, 0, 0, NOW()
%(user_id)s, %(username)s, %(profile_img)s, %(role)s, NOW()
)
ON CONFLICT (id)
DO UPDATE SET
Expand All @@ -191,19 +191,20 @@ async def get_or_create_user(
LEFT JOIN user_roles ur ON u.id = ur.user_id
LEFT JOIN organisation_managers om ON u.id = om.user_id
GROUP BY u.id, u.username, u.profile_img, u.role;
"""
)

parameters = {
"user_id": user_data.id,
"username": user_data.username,
"profile_img": user_data.picture or "",
"role": UserRole(user_data.role).name,
}
result = db.execute(upsert_sql, parameters)
db.commit()
"""

async with db.cursor(row_factory=class_row(DbUser)) as cur:
await cur.execute(
upsert_sql,
{
"user_id": user_data.id,
"username": user_data.username,
"profile_img": user_data.picture or "",
"role": UserRole(user_data.role).name,
},
)
db_user_details = await cur.fetchone()

db_user_details = result.first()
if not db_user_details:
raise HTTPException(
status_code=HTTPStatus.NOT_FOUND,
Expand All @@ -213,7 +214,7 @@ async def get_or_create_user(
return db_user_details

except Exception as e:
db.rollback()
await db.rollback()
log.error(f"Exception occurred: {e}")
if 'duplicate key value violates unique constraint "users_username_key"' in str(
e
Expand All @@ -230,38 +231,42 @@ async def get_or_create_user(

@router.get("/me/", response_model=FMTMUser)
async def my_data(
db: Session = Depends(database.get_db),
user_data: AuthUser = Depends(login_required),
db: Annotated[Connection, Depends(db_conn)],
current_user: Annotated[AuthUser, Depends(login_required)],
):
"""Read access token and get user details from OSM.
Args:
db: The db session.
user_data: User data provided by osm-login-python Auth.
db (Connection): The db connection.
current_user (AuthUser): User data provided by osm-login-python Auth.
Returns:
user_data(dict): The dict of user data.
FMTMUser: The dict of user data.
"""
return await get_or_create_user(db, user_data)
return await get_or_create_user(db, current_user)


@router.get("/refresh", response_model=AuthUserWithToken)
async def refresh_token(
request: Request, user_data: AuthUser = Depends(login_required)
request: Request,
current_user: Annotated[AuthUser, Depends(login_required)],
):
"""Uses the refresh token to generate a new access token."""
if settings.DEBUG:
return JSONResponse(
status_code=HTTPStatus.OK,
content={
"token": "debugtoken",
**user_data.model_dump(),
**current_user.model_dump(),
},
)
try:
refresh_token = extract_refresh_token_from_cookie(request)
if not refresh_token:
raise HTTPException(status_code=401, detail="No refresh token provided")
raise HTTPException(
status_code=HTTPStatus.UNAUTHORIZED,
detail="No refresh token provided",
)

token_data = verify_token(refresh_token)
access_token = refresh_access_token(token_data)
Expand All @@ -270,7 +275,7 @@ async def refresh_token(
status_code=HTTPStatus.OK,
content={
"token": access_token,
**user_data.model_dump(),
**current_user.model_dump(),
},
)
cookie_name = settings.FMTM_DOMAIN.replace(".", "_")
Expand Down Expand Up @@ -314,8 +319,8 @@ async def temp_login(
jwt_data = {
"sub": "fmtm|20386219",
"aud": settings.FMTM_DOMAIN,
"iat": int(time.time()),
"exp": int(time.time()) + 86400, # set token expiry to 1 day
"iat": int(time()),
"exp": int(time()) + 86400, # set token expiry to 1 day
"username": username,
"picture": None,
"role": UserRole.MAPPER,
Expand Down
5 changes: 2 additions & 3 deletions src/backend/app/auth/auth_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,15 @@
from pydantic import BaseModel, ConfigDict, PrivateAttr, computed_field
from pydantic.functional_validators import field_validator

from app.db.db_models import DbOrganisation, DbProject, DbUser
from app.models.enums import ProjectRole, UserRole
from app.db.enums import ProjectRole, UserRole
from app.db.models import DbOrganisation, DbProject, DbUser


class OrgUserDict(TypedDict):
"""Dict of both DbOrganisation & DbUser."""

user: DbUser
org: DbOrganisation
project: Optional[DbProject]


class ProjectUserDict(TypedDict):
Expand Down
20 changes: 15 additions & 5 deletions src/backend/app/auth/osm.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

from app.auth.auth_schemas import AuthUser
from app.config import settings
from app.models.enums import HTTPStatus, UserRole
from app.db.enums import HTTPStatus, UserRole

if settings.DEBUG:
# Required as callback url is http during dev
Expand Down Expand Up @@ -64,14 +64,20 @@ async def login_required(
access_token = extract_token_from_cookie(request)

if not access_token:
raise HTTPException(status_code=401, detail="No access token provided")
raise HTTPException(
status_code=HTTPStatus.UNAUTHORIZED,
detail="No access token provided",
)

try:
token_data = verify_token(access_token)
except ValueError as e:
log.error(e)
log.error("Failed to deserialise access token")
raise HTTPException(status_code=401, detail="Access token not valid") from e
raise HTTPException(
status_code=HTTPStatus.UNAUTHORIZED,
detail="Access token not valid",
) from e

return AuthUser(**token_data)

Expand Down Expand Up @@ -148,10 +154,14 @@ def verify_token(token: str):
audience=settings.FMTM_DOMAIN,
)
except jwt.ExpiredSignatureError as e:
raise HTTPException(status_code=401, detail="Refresh token has expired") from e
raise HTTPException(
status_code=HTTPStatus.UNAUTHORIZED,
detail="Refresh token has expired",
) from e
except Exception as e:
raise HTTPException(
status_code=401, detail="Could not validate refresh token"
status_code=HTTPStatus.UNAUTHORIZED,
detail="Could not validate refresh token",
) from e


Expand Down
Loading

0 comments on commit 43336bb

Please sign in to comment.