Skip to content

Commit

Permalink
♻️ Edit refactor db models to use UUID's instead of integer ID's (#1259)
Browse files Browse the repository at this point in the history
Co-authored-by: Sebastián Ramírez <[email protected]>
  • Loading branch information
estebanx64 and tiangolo authored Jul 22, 2024
1 parent cc480df commit e65b427
Show file tree
Hide file tree
Showing 9 changed files with 746 additions and 561 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
"""Edit replace id integers in all models to use UUID instead
Revision ID: d98dd8ec85a3
Revises: 9c0a54914c78
Create Date: 2024-07-19 04:08:04.000976
"""
from alembic import op
import sqlalchemy as sa
import sqlmodel.sql.sqltypes
from sqlalchemy.dialects import postgresql


# revision identifiers, used by Alembic.
revision = 'd98dd8ec85a3'
down_revision = '9c0a54914c78'
branch_labels = None
depends_on = None


def upgrade():
# Ensure uuid-ossp extension is available
op.execute('CREATE EXTENSION IF NOT EXISTS "uuid-ossp"')

# Create a new UUID column with a default UUID value
op.add_column('user', sa.Column('new_id', postgresql.UUID(as_uuid=True), default=sa.text('uuid_generate_v4()')))
op.add_column('item', sa.Column('new_id', postgresql.UUID(as_uuid=True), default=sa.text('uuid_generate_v4()')))
op.add_column('item', sa.Column('new_owner_id', postgresql.UUID(as_uuid=True), nullable=True))

# Populate the new columns with UUIDs
op.execute('UPDATE "user" SET new_id = uuid_generate_v4()')
op.execute('UPDATE item SET new_id = uuid_generate_v4()')
op.execute('UPDATE item SET new_owner_id = (SELECT new_id FROM "user" WHERE "user".id = item.owner_id)')

# Set the new_id as not nullable
op.alter_column('user', 'new_id', nullable=False)
op.alter_column('item', 'new_id', nullable=False)

# Drop old columns and rename new columns
op.drop_constraint('item_owner_id_fkey', 'item', type_='foreignkey')
op.drop_column('item', 'owner_id')
op.alter_column('item', 'new_owner_id', new_column_name='owner_id')

op.drop_column('user', 'id')
op.alter_column('user', 'new_id', new_column_name='id')

op.drop_column('item', 'id')
op.alter_column('item', 'new_id', new_column_name='id')

# Create primary key constraint
op.create_primary_key('user_pkey', 'user', ['id'])
op.create_primary_key('item_pkey', 'item', ['id'])

# Recreate foreign key constraint
op.create_foreign_key('item_owner_id_fkey', 'item', 'user', ['owner_id'], ['id'])

def downgrade():
# Reverse the upgrade process
op.add_column('user', sa.Column('old_id', sa.Integer, autoincrement=True))
op.add_column('item', sa.Column('old_id', sa.Integer, autoincrement=True))
op.add_column('item', sa.Column('old_owner_id', sa.Integer, nullable=True))

# Populate the old columns with default values
# Generate sequences for the integer IDs if not exist
op.execute('CREATE SEQUENCE IF NOT EXISTS user_id_seq AS INTEGER OWNED BY "user".old_id')
op.execute('CREATE SEQUENCE IF NOT EXISTS item_id_seq AS INTEGER OWNED BY item.old_id')

op.execute('SELECT setval(\'user_id_seq\', COALESCE((SELECT MAX(old_id) + 1 FROM "user"), 1), false)')
op.execute('SELECT setval(\'item_id_seq\', COALESCE((SELECT MAX(old_id) + 1 FROM item), 1), false)')

op.execute('UPDATE "user" SET old_id = nextval(\'user_id_seq\')')
op.execute('UPDATE item SET old_id = nextval(\'item_id_seq\'), old_owner_id = (SELECT old_id FROM "user" WHERE "user".id = item.owner_id)')

# Drop new columns and rename old columns back
op.drop_constraint('item_owner_id_fkey', 'item', type_='foreignkey')
op.drop_column('item', 'owner_id')
op.alter_column('item', 'old_owner_id', new_column_name='owner_id')

op.drop_column('user', 'id')
op.alter_column('user', 'old_id', new_column_name='id')

op.drop_column('item', 'id')
op.alter_column('item', 'old_id', new_column_name='id')

# Create primary key constraint
op.create_primary_key('user_pkey', 'user', ['id'])
op.create_primary_key('item_pkey', 'item', ['id'])

# Recreate foreign key constraint
op.create_foreign_key('item_owner_id_fkey', 'item', 'user', ['owner_id'], ['id'])
13 changes: 10 additions & 3 deletions backend/app/api/routes/items.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import uuid
from typing import Any

from fastapi import APIRouter, HTTPException
Expand Down Expand Up @@ -41,7 +42,7 @@ def read_items(


@router.get("/{id}", response_model=ItemPublic)
def read_item(session: SessionDep, current_user: CurrentUser, id: int) -> Any:
def read_item(session: SessionDep, current_user: CurrentUser, id: uuid.UUID) -> Any:
"""
Get item by ID.
"""
Expand Down Expand Up @@ -69,7 +70,11 @@ def create_item(

@router.put("/{id}", response_model=ItemPublic)
def update_item(
*, session: SessionDep, current_user: CurrentUser, id: int, item_in: ItemUpdate
*,
session: SessionDep,
current_user: CurrentUser,
id: uuid.UUID,
item_in: ItemUpdate,
) -> Any:
"""
Update an item.
Expand All @@ -88,7 +93,9 @@ def update_item(


@router.delete("/{id}")
def delete_item(session: SessionDep, current_user: CurrentUser, id: int) -> Message:
def delete_item(
session: SessionDep, current_user: CurrentUser, id: uuid.UUID
) -> Message:
"""
Delete an item.
"""
Expand Down
7 changes: 4 additions & 3 deletions backend/app/api/routes/users.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import uuid
from typing import Any

from fastapi import APIRouter, Depends, HTTPException
Expand Down Expand Up @@ -163,7 +164,7 @@ def register_user(session: SessionDep, user_in: UserRegister) -> Any:

@router.get("/{user_id}", response_model=UserPublic)
def read_user_by_id(
user_id: int, session: SessionDep, current_user: CurrentUser
user_id: uuid.UUID, session: SessionDep, current_user: CurrentUser
) -> Any:
"""
Get a specific user by id.
Expand All @@ -187,7 +188,7 @@ def read_user_by_id(
def update_user(
*,
session: SessionDep,
user_id: int,
user_id: uuid.UUID,
user_in: UserUpdate,
) -> Any:
"""
Expand All @@ -213,7 +214,7 @@ def update_user(

@router.delete("/{user_id}", dependencies=[Depends(get_current_active_superuser)])
def delete_user(
session: SessionDep, current_user: CurrentUser, user_id: int
session: SessionDep, current_user: CurrentUser, user_id: uuid.UUID
) -> Message:
"""
Delete a user.
Expand Down
3 changes: 2 additions & 1 deletion backend/app/crud.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import uuid
from typing import Any

from sqlmodel import Session, select
Expand Down Expand Up @@ -45,7 +46,7 @@ def authenticate(*, session: Session, email: str, password: str) -> User | None:
return db_user


def create_item(*, session: Session, item_in: ItemCreate, owner_id: int) -> Item:
def create_item(*, session: Session, item_in: ItemCreate, owner_id: uuid.UUID) -> Item:
db_item = Item.model_validate(item_in, update={"owner_id": owner_id})
session.add(db_item)
session.commit()
Expand Down
16 changes: 9 additions & 7 deletions backend/app/models.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import uuid

from pydantic import EmailStr
from sqlmodel import Field, Relationship, SQLModel

Expand Down Expand Up @@ -39,14 +41,14 @@ class UpdatePassword(SQLModel):

# Database model, database table inferred from class name
class User(UserBase, table=True):
id: int | None = Field(default=None, primary_key=True)
id: uuid.UUID = Field(default_factory=uuid.uuid4, primary_key=True)
hashed_password: str
items: list["Item"] = Relationship(back_populates="owner")


# Properties to return via API, id is always required
class UserPublic(UserBase):
id: int
id: uuid.UUID


class UsersPublic(SQLModel):
Expand All @@ -72,16 +74,16 @@ class ItemUpdate(ItemBase):

# Database model, database table inferred from class name
class Item(ItemBase, table=True):
id: int | None = Field(default=None, primary_key=True)
id: uuid.UUID = Field(default_factory=uuid.uuid4, primary_key=True)
title: str = Field(max_length=255)
owner_id: int | None = Field(default=None, foreign_key="user.id", nullable=False)
owner_id: uuid.UUID = Field(foreign_key="user.id", nullable=False)
owner: User | None = Relationship(back_populates="items")


# Properties to return via API, id is always required
class ItemPublic(ItemBase):
id: int
owner_id: int
id: uuid.UUID
owner_id: uuid.UUID


class ItemsPublic(SQLModel):
Expand All @@ -102,7 +104,7 @@ class Token(SQLModel):

# Contents of JWT token
class TokenPayload(SQLModel):
sub: int | None = None
sub: str | None = None


class NewPassword(SQLModel):
Expand Down
16 changes: 9 additions & 7 deletions backend/app/tests/api/routes/test_items.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import uuid

from fastapi.testclient import TestClient
from sqlmodel import Session

Expand Down Expand Up @@ -34,15 +36,15 @@ def test_read_item(
content = response.json()
assert content["title"] == item.title
assert content["description"] == item.description
assert content["id"] == item.id
assert content["owner_id"] == item.owner_id
assert content["id"] == str(item.id)
assert content["owner_id"] == str(item.owner_id)


def test_read_item_not_found(
client: TestClient, superuser_token_headers: dict[str, str]
) -> None:
response = client.get(
f"{settings.API_V1_STR}/items/999",
f"{settings.API_V1_STR}/items/{uuid.uuid4()}",
headers=superuser_token_headers,
)
assert response.status_code == 404
Expand Down Expand Up @@ -91,16 +93,16 @@ def test_update_item(
content = response.json()
assert content["title"] == data["title"]
assert content["description"] == data["description"]
assert content["id"] == item.id
assert content["owner_id"] == item.owner_id
assert content["id"] == str(item.id)
assert content["owner_id"] == str(item.owner_id)


def test_update_item_not_found(
client: TestClient, superuser_token_headers: dict[str, str]
) -> None:
data = {"title": "Updated title", "description": "Updated description"}
response = client.put(
f"{settings.API_V1_STR}/items/999",
f"{settings.API_V1_STR}/items/{uuid.uuid4()}",
headers=superuser_token_headers,
json=data,
)
Expand Down Expand Up @@ -141,7 +143,7 @@ def test_delete_item_not_found(
client: TestClient, superuser_token_headers: dict[str, str]
) -> None:
response = client.delete(
f"{settings.API_V1_STR}/items/999",
f"{settings.API_V1_STR}/items/{uuid.uuid4()}",
headers=superuser_token_headers,
)
assert response.status_code == 404
Expand Down
7 changes: 4 additions & 3 deletions backend/app/tests/api/routes/test_users.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import uuid
from unittest.mock import patch

from fastapi.testclient import TestClient
Expand Down Expand Up @@ -105,7 +106,7 @@ def test_get_existing_user_permissions_error(
client: TestClient, normal_user_token_headers: dict[str, str]
) -> None:
r = client.get(
f"{settings.API_V1_STR}/users/999999",
f"{settings.API_V1_STR}/users/{uuid.uuid4()}",
headers=normal_user_token_headers,
)
assert r.status_code == 403
Expand Down Expand Up @@ -371,7 +372,7 @@ def test_update_user_not_exists(
) -> None:
data = {"full_name": "Updated_full_name"}
r = client.patch(
f"{settings.API_V1_STR}/users/99999999",
f"{settings.API_V1_STR}/users/{uuid.uuid4()}",
headers=superuser_token_headers,
json=data,
)
Expand Down Expand Up @@ -468,7 +469,7 @@ def test_delete_user_not_found(
client: TestClient, superuser_token_headers: dict[str, str]
) -> None:
r = client.delete(
f"{settings.API_V1_STR}/users/99999999",
f"{settings.API_V1_STR}/users/{uuid.uuid4()}",
headers=superuser_token_headers,
)
assert r.status_code == 404
Expand Down
Loading

0 comments on commit e65b427

Please sign in to comment.