Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SQLAlchemy and replace with async psycopg db driver #1834

Merged
merged 69 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
a9f15d0
build: add psycopg async driver (v3)
spwoodcock Aug 6, 2024
0fc1aaa
feat: create async psycopg db pool at app startup
spwoodcock Aug 6, 2024
0648917
refactor: test usage of psycopg + pydantic models on project get endp…
spwoodcock Aug 6, 2024
9fbd346
build: relock deps after rebase
spwoodcock Oct 19, 2024
54e9f1d
build: migration remove additional redundant fields from projects
spwoodcock Oct 19, 2024
4230edc
refactor: work in progress removing sqlalchemy
spwoodcock Oct 19, 2024
70b0b14
build: remove project_info table, merge into projects
spwoodcock Oct 20, 2024
e791256
refactor: work in progress removing sqlalchemy
spwoodcock Oct 20, 2024
9498827
build: migration to add default timestamptz to tables
spwoodcock Oct 21, 2024
0ccb587
refactor: continue refactor to remove sqlalchemy
spwoodcock Oct 21, 2024
039bfb6
refactor: continued migration from sqlalchemy --> psycopg
spwoodcock Oct 22, 2024
bc11dcf
refactor: continue sqlalchemy removal, fix endpoints
spwoodcock Oct 22, 2024
8c550d6
fix: updating failed state for s3 upload not awaited
spwoodcock Oct 22, 2024
6e723a2
refactor: fix background tasks and mbtiles / basemaps apis
spwoodcock Oct 22, 2024
e8fe626
build: remove async-lru package
spwoodcock Oct 23, 2024
1be0de1
refactor: refactor out project_deps.get_odk_credentials
spwoodcock Oct 23, 2024
e3757d2
refactor: use HTTPStatus enum everywhere, format
spwoodcock Oct 23, 2024
40f5550
refactor: further updates to sqlalchemy removal
spwoodcock Oct 23, 2024
c759fd2
refactor: working project routes
spwoodcock Oct 23, 2024
25b520a
refactor: rename project_path_prefix --> slug, remove unused project_…
spwoodcock Oct 23, 2024
110f5ec
refactor: final type renames before task events refactor
spwoodcock Oct 23, 2024
cc7e5c3
fix: handle error for /statuses endpoint
spwoodcock Oct 23, 2024
904f3b3
fix: add project_id query param to project get
spwoodcock Oct 23, 2024
facccdb
fix: centroid loading on home page
spwoodcock Oct 23, 2024
e30668b
fix: project outline no longer nested in Feature
spwoodcock Oct 23, 2024
809da8d
fix: update frontend enums to use strings
spwoodcock Oct 23, 2024
618d57d
fix: enum usage on split task frontend
spwoodcock Oct 23, 2024
0195a06
fix: splitting of features into task areas during proj create
spwoodcock Oct 23, 2024
6e0e3f9
fix: sometimes project missing hashtags
spwoodcock Oct 23, 2024
35d8163
fix: qrcode loading with project name
spwoodcock Oct 23, 2024
e052289
fix: use EntityStatus enum
spwoodcock Oct 23, 2024
11f1fbc
fix: add ORDER BY to all get all statements
spwoodcock Oct 23, 2024
c3f832b
fix: generate project working
spwoodcock Oct 23, 2024
bad568d
refactor: remove sqlalchemy once and for all!! (bye bye)
spwoodcock Oct 23, 2024
ce1ba7a
fix: untested fixes for submission routes
spwoodcock Oct 23, 2024
8831844
build: upgrade postgis image --> 14-3.5-alpine as 3.4 was removed
spwoodcock Oct 23, 2024
a2846db
refactor: remove manual enums since available in in python 3.11
spwoodcock Oct 23, 2024
88167db
build: upgrade python --> 3.12 + all dependencies to latest
spwoodcock Oct 23, 2024
5bf3776
refactor: fix imports, remove refs to db_models
spwoodcock Oct 23, 2024
f44ff08
build: relock deps after osm-rawdata upgrade, optional sqlalchemy
spwoodcock Oct 24, 2024
5a7aa3e
fix: awaiting AsyncConnectionPool def is incorrect
spwoodcock Oct 24, 2024
5c9b36d
fix: case where string is passed to dborg one func
spwoodcock Oct 24, 2024
8a988f8
test: start fixing test cases
spwoodcock Oct 24, 2024
e4d84ce
fix: minor fixes to api
spwoodcock Oct 24, 2024
5c9e54a
test: continue work fixing / updating tests
spwoodcock Oct 24, 2024
7127e2f
fix: error if creating project with single task
spwoodcock Oct 24, 2024
18028de
refactor: remove author object from project response (author_id is en…
spwoodcock Oct 24, 2024
e7092d3
build: add composite index on task_history(task_id, action_date)
spwoodcock Oct 24, 2024
76b282b
fix: exclude last active from project input model
spwoodcock Oct 24, 2024
760b87a
refactor: rename locked_by_ fields to actioned_by_
spwoodcock Oct 24, 2024
5601e54
fix: getting project before update / delete
spwoodcock Oct 24, 2024
2026660
fix: setting slug on project + org create
spwoodcock Oct 24, 2024
93d4a58
fix: exclude outline field on ProjectIn to avoid dict insert error
spwoodcock Oct 24, 2024
27a9c8f
test: further fixes to tests
spwoodcock Oct 24, 2024
25bf957
fix: avoid spamming nominatim during tests / DEBUG=true
spwoodcock Oct 25, 2024
246c314
fix: hashtags not set correctly on new project
spwoodcock Oct 25, 2024
8ee394a
refactor(frontend): simplify upload-task-area logic during proj create
spwoodcock Oct 25, 2024
4698611
fix: various fixes, all working tests
spwoodcock Oct 25, 2024
1a312d4
fix: set some constraints on input project_id or org_id
spwoodcock Oct 25, 2024
cba6082
test: fix conftest, set server url to FMTM_DOMAIN for ci
spwoodcock Oct 25, 2024
923bb92
build: install uvloop + httptools for performance (uvicorn uses autom…
spwoodcock Oct 25, 2024
edef49c
refactor: use asgi lifespan state over fastapi app state
spwoodcock Oct 25, 2024
72c562b
fix: use asgi_lifespan LifepanManager correctly for async testing
spwoodcock Oct 25, 2024
2bb5234
fix: add async detailed debug mode when DEBUG=True
spwoodcock Oct 25, 2024
0c9821d
test: correctly setup conftest with asyncclient lifespan
spwoodcock Oct 25, 2024
35fb490
test: update conftest to use db_conn override on fastapi app
spwoodcock Oct 25, 2024
82a2cb8
build: move api healthcheck to compose, increase startup grace period
spwoodcock Oct 25, 2024
6175134
fix(frontend): styling based on task action not task status
spwoodcock Oct 25, 2024
94edd62
fix(frontend): tweak frontend to use TaskAction enum for now
spwoodcock Oct 25, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading