Cleaning up, adding constraints.

Signed-off-by: Cliff Hill <xlorep@darkhelm.org>
This commit is contained in:
2025-08-26 23:40:47 -04:00
parent 1839722777
commit 5bb775bccb
10 changed files with 474 additions and 5 deletions

View File

@@ -4,7 +4,9 @@ ignore = E203,E501,RST201,RST203,RST301,W503
max-line-length = 88
max-complexity = 10
docstring-convention = google
per-file-ignores = tests/*:S101,backend/tests/*:S101
per-file-ignores =
tests/*:S101,B101
backend/tests/*:S101,B101
rst-roles = class,const,func,meth,mod,ref
rst-directives = deprecated
exclude = backend/migrations/versions/*

View File

@@ -1,6 +1,6 @@
"""Add seed data from seed.sql with separate prepared statements
Revision ID: e6a14f4dae3d
Revision ID: 0001_initial_migration
Revises:
Create Date: 2025-08-22 23:40:52.885449
"""
@@ -13,7 +13,7 @@ from alembic import op
# revision identifiers, used by Alembic.
revision: str = "e6a14f4dae3d"
revision: str = "0001"
down_revision: Union[str, Sequence[str], None] = None
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None

View File

@@ -0,0 +1,35 @@
"""Add exclusion constraint to prevent overlapping bookings for the same room
Revision ID: 0002_add_booking_exclusion_constraint
Revises: 0001_initial_migration
Create Date: 2025-08-26
"""
from alembic import op
# revision identifiers, used by Alembic.
revision = "0002"
down_revision = "0001"
branch_labels = None
depends_on = None
def upgrade():
# Ensure btree_gist extension is enabled
op.execute("CREATE EXTENSION IF NOT EXISTS btree_gist;")
# Add exclusion constraint to prevent overlapping bookings for the same room
op.execute(
"""
ALTER TABLE bookings
ADD CONSTRAINT bookings_no_overlap
EXCLUDE USING gist (
room_id WITH =,
tsrange(start_time, end_time) WITH &&
);
"""
)
def downgrade():
op.execute("ALTER TABLE bookings DROP CONSTRAINT IF EXISTS bookings_no_overlap;")

View File

@@ -0,0 +1,25 @@
"""Add unique constraint to prevent duplicate invitees for the same booking
Revision ID: 0003_add_invitee_unique_constraint
Revises: 0002_add_booking_exclusion_constraint
Create Date: 2025-08-26
"""
from alembic import op
# revision identifiers, used by Alembic.
revision = "0003"
down_revision = "0002"
branch_labels = None
depends_on = None
def upgrade():
op.create_unique_constraint(
"uq_invitees_booking_user", "invitees", ["booking_id", "user_email"]
)
def downgrade():
op.drop_constraint("uq_invitees_booking_user", "invitees", type_="unique")

View File

@@ -0,0 +1,25 @@
"""Add start_time < end_time constraint to bookings table
Revision ID: 0004_add_start_end_check_constraint
Revises: 0003_add_invitee_unique_constraint
Create Date: 2025-08-26
"""
from alembic import op
# revision identifiers, used by Alembic.
revision = "0004"
down_revision = "0003"
branch_labels = None
depends_on = None
def upgrade():
op.create_check_constraint(
"ck_bookings_start_before_end", "bookings", "start_time < end_time"
)
def downgrade():
op.drop_constraint("ck_bookings_start_before_end", "bookings", type_="check")

View File

@@ -102,6 +102,15 @@ async def add_invitee(
status_code=status.HTTP_400_BAD_REQUEST,
detail="Database error occurred",
) from e
except ValueError as e:
logger.warning(
f"Duplicate invitee attempted for booking_id {booking_id}"
f" and user {invitee.user_email}: {str(e)}"
)
raise HTTPException(
status_code=status.HTTP_409_CONFLICT,
detail=str(e),
) from e
except Exception as e:
logger.error(
f"Unexpected error while adding invitee to booking_id {booking_id}: {str(e)}"

View File

@@ -5,18 +5,29 @@ handling queries and logging operations.
"""
import logging
from datetime import datetime
from datetime import timedelta
from datetime import timezone
from typing import Any
from typing import Awaitable
from typing import Callable
from typing import Sequence
from typing import TypedDict
from typing import Unpack
from typing import cast
from sqlalchemy import delete
from sqlalchemy import func
from sqlalchemy import select
from sqlalchemy import update
from sqlalchemy.exc import NoResultFound
from sqlalchemy.ext.asyncio import AsyncSession
from backend import config
from backend.models import Booking
from backend.models import BookingList
from backend.services.invitees import get_invitees_for_booking
from backend.services.rooms import get_room
logger = logging.getLogger(__name__)
@@ -98,10 +109,73 @@ async def new_booking(session: AsyncSession, booking: Booking) -> Booking:
Created booking.
Raises:
Exception: If any database error occurs.
ValueError: If any booking constraint is violated.
"""
logger.debug("Entering new_booking")
try:
# Constraint: Room must exist
try:
room = await get_room(session, booking.room_id)
except Exception as e:
logger.warning(f"Attempted to book non-existent room_id {booking.room_id}")
raise ValueError("The selected room does not exist.") from e
now = datetime.now(timezone.utc)
# Constraint: No bookings in the past
if booking.start_time < now or booking.end_time < now:
logger.warning("Attempted to create booking in the past.")
raise ValueError("Bookings cannot be made in the past.")
# Constraint: Start date < end date
if booking.start_time >= booking.end_time:
logger.warning("Attempted to create booking with start_time >= end_time.")
raise ValueError("The booking start time must be before the end time.")
# Constraint: Max months in future
max_months = config("BOOKING_MAX_MONTHS", default=12, cast=int)
max_future = now + timedelta(days=30 * max_months)
if booking.start_time > max_future or booking.end_time > max_future:
logger.warning("Attempted to create booking too far in the future.")
raise ValueError(
f"Bookings can only be made up to {max_months} months in advance."
)
# Check for overlapping bookings for the same room
from sqlalchemy import func
overlap_stmt = select(Booking).where(
Booking.room_id == booking.room_id,
func.tstzrange(Booking.start_time, Booking.end_time, "[]").op("&&")(
func.tstzrange(booking.start_time, booking.end_time, "[]")
),
)
result = await session.scalars(overlap_stmt)
overlapping = result.first()
if overlapping:
logger.warning(
"Attempted to create overlapping booking for room_id %s",
booking.room_id,
)
raise ValueError(
"Booking times overlap with an existing booking for this room."
)
# Constraint: Attendees <= room capacity
invitees = await get_invitees_for_booking(
session, getattr(booking, "id", None) or -1
)
num_attendees = len(invitees) if invitees else 0
if num_attendees > room.capacity:
logger.warning(
f"Attempted to create booking with {num_attendees} attendees"
f" exceeding room capacity {room.capacity}."
)
raise ValueError(
f"Number of attendees ({num_attendees}) exceeds the room"
f" capacity ({room.capacity})."
)
session.add(booking)
await session.commit()
logger.info(f"Successfully created new booking with id: {booking.id}")
@@ -114,6 +188,72 @@ async def new_booking(session: AsyncSession, booking: Booking) -> Booking:
logger.debug("Exiting new_booking")
async def _validate_room_exists(
session: AsyncSession,
room_id: int,
get_room: Callable[[AsyncSession, int], Awaitable[Any]],
) -> Any:
try:
return await get_room(session, room_id)
except Exception as e:
logger.warning(f"Attempted to update booking to non-existent room_id {room_id}")
raise ValueError("The selected room does not exist.") from e
def _validate_time_constraints(
new_start: datetime,
new_end: datetime,
now: datetime,
max_months: int,
) -> None:
if new_start < now or new_end < now:
logger.warning("Attempted to update booking to be in the past.")
raise ValueError("Bookings cannot be made in the past.")
if new_start >= new_end:
logger.warning("Attempted to update booking with start_time >= end_time.")
raise ValueError("The booking start time must be before the end time.")
max_future = now + timedelta(days=30 * max_months)
if new_start > max_future or new_end > max_future:
logger.warning("Attempted to update booking too far in the future.")
raise ValueError(
f"Bookings can only be made up to {max_months} months in advance."
)
async def _validate_no_overlap(
session: AsyncSession,
booking_id: int,
room_id: int,
new_start: datetime,
new_end: datetime,
) -> None:
overlap_stmt = select(Booking).where(
Booking.room_id == room_id,
Booking.id != booking_id,
func.tstzrange(Booking.start_time, Booking.end_time, "[]").op("&&")(
func.tstzrange(new_start, new_end, "[]")
),
)
overlapping = (await session.scalars(overlap_stmt)).first()
if overlapping:
logger.warning("Attempted to update booking to overlap for room_id %s", room_id)
raise ValueError(
"Booking times overlap with an existing booking for this room."
)
def _validate_attendee_count(invitees: Sequence[Any], room: Any) -> None:
if len(invitees or []) > room.capacity:
logger.warning(
f"Attempted to update booking with {len(invitees)} attendees"
f" exceeding room capacity {room.capacity}."
)
raise ValueError(
f"Number of attendees ({len(invitees)}) exceeds the room"
f" capacity ({room.capacity})."
)
async def update_booking(
session: AsyncSession,
booking_id: int,
@@ -136,6 +276,19 @@ async def update_booking(
f"Entering update_booking with booking_id: {booking_id}, params: {kwargs}"
)
try:
current = await get_booking(session, booking_id)
new_room_id = kwargs.get("room_id", current.room_id)
new_start = kwargs.get("start_time", current.start_time)
new_end = kwargs.get("end_time", current.end_time)
room = await _validate_room_exists(session, new_room_id, get_room)
now = datetime.now(timezone.utc)
max_months = config("BOOKING_MAX_MONTHS", default=12, cast=int)
_validate_time_constraints(new_start, new_end, now, max_months)
await _validate_no_overlap(session, booking_id, new_room_id, new_start, new_end)
invitees = await get_invitees_for_booking(session, booking_id)
_validate_attendee_count(invitees, room)
stmt = update(Booking).where(Booking.id == booking_id).values(**kwargs)
result = await session.execute(stmt)
if result.rowcount == 0:

View File

@@ -65,12 +65,44 @@ async def add_invitee_to_booking(
The created Invitee object.
Raises:
Exception: Any database error encountered during creation is logged and re-raised.
ValueError: If the invitee already exists or if adding the invitee
exceeds room capacity.
"""
logger.debug(
f"Entering add_invitee_to_booking with booking_id: {booking_id}, email: {user_email}"
)
try:
# Check for existing invitee for this booking and user
from sqlalchemy import select
stmt = select(Invitee).where(
Invitee.booking_id == booking_id, Invitee.user_email == user_email
)
result = await session.scalars(stmt)
existing = result.first()
if existing:
logger.warning(
f"User {user_email} is already invited to booking {booking_id}"
)
raise ValueError("This user is already invited to this booking.")
# Enforce attendee count does not exceed room capacity
from backend.services.bookings import get_booking
from backend.services.rooms import get_room
booking = await get_booking(session, booking_id)
room = await get_room(session, booking.room_id)
invitees = await get_invitees_for_booking(session, booking_id)
if len(invitees) + 1 > room.capacity:
logger.warning(
f"Attempted to add invitee to booking {booking_id} exceeding"
f" room capacity {room.capacity}."
)
raise ValueError(
f"Cannot add invitee: number of attendees would exceed the"
f" room capacity ({room.capacity})."
)
invitee = Invitee(booking_id=booking_id, user_email=user_email)
session.add(invitee)
await session.commit()

View File

@@ -1,9 +1,15 @@
"""Unit tests for the backend.services.bookings module."""
import sys
from collections.abc import Awaitable
from datetime import datetime
from datetime import timedelta
from datetime import timezone
from typing import Any
from typing import Callable
from unittest.mock import AsyncMock
from unittest.mock import MagicMock
from unittest.mock import patch
import pytest
from sqlalchemy import delete
@@ -234,6 +240,187 @@ async def test_new_booking_success(
async_session.commit.assert_called_once()
# --- Constraint tests ---
@pytest.mark.asyncio
@pytest.mark.parametrize(
"room_exists, start_time, end_time, max_months, expected_error",
[
# Booking in the past
(
True,
datetime.now(timezone.utc) - timedelta(days=1),
datetime.now(timezone.utc) + timedelta(hours=1),
12,
"Bookings cannot be made in the past.",
),
# Start >= end
(
True,
datetime.now(timezone.utc) + timedelta(days=1),
datetime.now(timezone.utc) + timedelta(days=1),
12,
"The booking start time must be before the end time.",
),
# Too far in future
(
True,
datetime.now(timezone.utc) + timedelta(days=365 * 2),
datetime.now(timezone.utc) + timedelta(days=365 * 2, hours=1),
12,
"Bookings can only be made up to 12 months in advance.",
),
# Non-existent room
(
False,
datetime.now(timezone.utc) + timedelta(days=1),
datetime.now(timezone.utc) + timedelta(days=2),
12,
"The selected room does not exist.",
),
],
ids=["past", "start>=end", "future", "no_room"],
)
async def test_new_booking_constraints(
async_session: AsyncSession,
mock_logger: MagicMock,
room_exists: bool,
start_time: datetime,
end_time: datetime,
max_months: int,
expected_error: str,
) -> None:
"""Test booking constraints: past, start>=end, future, non-existent room.
Args:
async_session: The asynchronous database session.
mock_logger: The mocked logger instance.
room_exists: Flag indicating if the room exists.
start_time: The proposed start time for the booking.
end_time: The proposed end time for the booking.
max_months: The maximum number of months in advance bookings can be made.
expected_error: The expected error message.
"""
from backend.models import Booking
from backend.services.bookings import new_booking
booking: Booking = Booking(room_id=1, start_time=start_time, end_time=end_time)
booking.id = 1
# Patch get_room to simulate room existence
get_room_patch: Callable[[], Awaitable[MagicMock]] = (
AsyncMock(return_value=MagicMock())
if room_exists
else AsyncMock(side_effect=Exception("no room"))
)
with patch("backend.services.bookings.get_room", new=get_room_patch):
# Patch config to set max months
with patch(
"backend.services.bookings.config", wraps=sys.modules["backend"].config
) as mock_config:
mock_config.side_effect = lambda key: ( # type: ignore
max_months if key == "BOOKING_MAX_MONTHS" else None
)
with pytest.raises(ValueError) as exc:
await new_booking(async_session, booking)
assert expected_error in str(exc.value)
@pytest.mark.asyncio
async def test_new_booking_overlap(
async_session: AsyncSession, mock_logger: MagicMock
) -> None:
"""Test overlapping booking constraint.
Args:
async_session: The asynchronous database session.
mock_logger: The mocked logger instance.
"""
from backend.models import Booking
from backend.services.bookings import new_booking
# Patch get_room to always exist
with patch(
"backend.services.bookings.get_room",
new=AsyncMock(return_value=MagicMock(capacity=10)),
):
# Patch config to allow future
with patch(
"backend.services.bookings.config", wraps=sys.modules["backend"].config
) as mock_config:
mock_config.side_effect = lambda key, **kwargs: ( # type: ignore
12
if key == "BOOKING_MAX_MONTHS"
else kwargs.get("default") # type: ignore
)
# Patch session.scalars to simulate overlap
overlap_booking: Booking = Booking(
room_id=1,
start_time=datetime.now(timezone.utc) + timedelta(days=1),
end_time=datetime.now(timezone.utc) + timedelta(days=2),
)
overlap_booking.id = 2
mock_scalars_result = AsyncMock()
mock_scalars_result.first = MagicMock(return_value=overlap_booking)
async_session.scalars = AsyncMock(return_value=mock_scalars_result)
booking: Booking = Booking(
room_id=1,
start_time=datetime.now(timezone.utc) + timedelta(days=1),
end_time=datetime.now(timezone.utc) + timedelta(days=2),
)
booking.id = 3
with pytest.raises(ValueError) as exc:
await new_booking(async_session, booking)
assert "overlap" in str(exc.value)
@pytest.mark.asyncio
async def test_new_booking_attendees_exceed_capacity(
async_session: AsyncSession, mock_logger: MagicMock
) -> None:
"""Test attendee count exceeding room capacity.
Args:
async_session: The asynchronous database session.
mock_logger: The mocked logger instance.
"""
from backend.models import Booking
from backend.services.bookings import new_booking
# Patch get_room to have capacity 1
with patch(
"backend.services.bookings.get_room",
new=AsyncMock(return_value=MagicMock(capacity=1)),
):
# Patch config to allow future
with patch(
"backend.services.bookings.config", wraps=sys.modules["backend"].config
) as mock_config:
mock_config.side_effect = lambda key, **kwargs: ( # type: ignore
12
if key == "BOOKING_MAX_MONTHS"
else kwargs.get("default") # type: ignore
)
# Patch session.scalars to simulate no overlap
mock_scalars_result = AsyncMock()
mock_scalars_result.first = MagicMock(return_value=None)
async_session.scalars = AsyncMock(return_value=mock_scalars_result)
# Patch get_invitees_for_booking to return 2 invitees (over capacity)
with patch(
"backend.services.bookings.get_invitees_for_booking",
new=AsyncMock(return_value=[MagicMock(), MagicMock()]),
):
booking: Booking = Booking(
room_id=1,
start_time=datetime.now(timezone.utc) + timedelta(days=1),
end_time=datetime.now(timezone.utc) + timedelta(days=2),
)
booking.id = 4
with pytest.raises(ValueError) as exc:
await new_booking(async_session, booking)
assert "exceeds the room capacity" in str(exc.value)
@pytest.mark.asyncio
@pytest.mark.parametrize("mock_logger", ["backend.services.bookings"], indirect=True)
async def test_new_booking_database_error(

View File

@@ -11,6 +11,7 @@ services:
ENVIRONMENT: production
POSTGRES_HOST: postgres
LOG_LEVEL: warning
BOOKING_MAX_MONTHS: 12
depends_on:
postgres:
condition: service_healthy