Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8047fe6
feat: add BaseEntity
stefanotroncaro Jan 24, 2026
22ded1e
refactor: move entity to app/common/domain
stefanotroncaro Jan 26, 2026
64f070e
feat: add base repository interface
stefanotroncaro Jan 26, 2026
0ec008f
feat: add non-criteria methods to base repository
stefanotroncaro Jan 26, 2026
6244314
feat: add base translator
stefanotroncaro Jan 29, 2026
1f823a9
feat: common infrastructure facade
stefanotroncaro Jan 29, 2026
710f134
refactor: remove old repo and base model
stefanotroncaro Jan 29, 2026
1e33fda
feat: adapt user submodule (WIP)
stefanotroncaro Jan 31, 2026
ff99d7d
feat: user repository dependency
stefanotroncaro Feb 2, 2026
64a7e7d
fix: type annotations
stefanotroncaro Feb 2, 2026
f3bd0a8
feat: paginator criteria
stefanotroncaro Feb 2, 2026
7db5a7c
refactor: restore old functionality for gradual refactor
stefanotroncaro Feb 2, 2026
24932e7
refactor: session dependency filename
stefanotroncaro Feb 2, 2026
eece5a4
refactor: restore auth endpoints to encapsulate refactor to user package
stefanotroncaro Feb 2, 2026
5a75255
refactor: user email tasks to use new architecture
stefanotroncaro Feb 2, 2026
7a1b6df
fix: tests
stefanotroncaro Feb 2, 2026
eee98e5
refactor: restore main to make incremental refactors
stefanotroncaro Feb 2, 2026
4a62b9d
chore: update todo comments
stefanotroncaro Feb 2, 2026
d45f7cc
refactor: errors
stefanotroncaro Feb 2, 2026
00451a1
refactor: rename paginator to pagination and make criteria explicit
stefanotroncaro Feb 3, 2026
78df8d0
refactor: add criteria suffix to UserEmailFilter
stefanotroncaro Feb 3, 2026
4962d2a
refactor: remove TODO
stefanotroncaro Feb 3, 2026
35da134
feat: sqlalchemy repository error if rowcount is not found
stefanotroncaro Feb 3, 2026
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
2 changes: 1 addition & 1 deletion app/auth/api/endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from fastapi import APIRouter, HTTPException, Header, status, Response, Request

from app.auth.use_cases.reset_password_use_case import ResetPasswordUseCase
from app.common.api.dependencies.get_session import SessionDependency
from app.common.api.dependencies import SessionDependency
from app.auth.schemas.auth_schema import PasswordResetRequest, UserLogin
from app.auth.use_cases.auth_user_use_case import AuthUserUseCase
from app.common.exceptions.model_not_found_exception import (
Expand Down
45 changes: 18 additions & 27 deletions app/celery/tasks/emails.py
Original file line number Diff line number Diff line change
@@ -1,29 +1,24 @@
from uuid import UUID
from app.common.domain import PaginationCriteria
from app.core.config import settings
from app.db.session import SessionLocal
from app.emails.exceptions.email_client_exception import EmailClientException
from app.common.schemas.pagination_schema import ListFilter
from app.emails.services.emails_service import EmailService
from app.db.session import SessionLocal
from app.main import celery


from app.core.config import get_settings
from app.users.schemas.user_schema import UserInDB
from app.users.services.users_service import UsersService

settings = get_settings()
from app.users.domain import UserId
from app.users.infrastructure import SQLAlchemyUserRepository


@celery.task
def send_reminder_email() -> None:
session = SessionLocal()
try:
users = UsersService(session).list(ListFilter(page=1, page_size=100))
for user in users.data:
EmailService().send_user_remind_email(
UserInDB.model_validate(user)
)
finally:
session.close()
email_service = EmailService()

with SessionLocal() as session:
user_repository = SQLAlchemyUserRepository(session)
users = user_repository.where(
PaginationCriteria(page=1, page_size=100)
)
for user in users:
email_service.send_user_remind_email(user)


@celery.task(
Expand All @@ -32,11 +27,7 @@ def send_reminder_email() -> None:
max_retries=settings.SEND_WELCOME_EMAIL_MAX_RETRIES,
retry_jitter=False,
)
def send_welcome_email(user_id: UUID) -> None:
session = SessionLocal()
try:
user = UsersService(session).get_by_id(user_id)
if user:
EmailService().send_new_user_email(UserInDB.model_validate(user))
finally:
session.close()
def send_welcome_email(user_id: UserId) -> None:
with SessionLocal() as session:
user = SQLAlchemyUserRepository(session).find_or_fail(user_id)
EmailService().send_new_user_email(user)
1 change: 1 addition & 0 deletions app/common/api/dependencies/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
from .session_dependency import *
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
from typing import Annotated, Generator
__all__ = (
"get_session",
"SessionDependency",
)

from fastapi import Depends
import typing as t

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any benefit on importing it like this? I know the import is clearer but I think the code is uglier 😄 . This is not a fix, just a preference.

@stefanotroncaro stefanotroncaro Feb 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, this is code style preference, yes, so it's very difficult (if not impossible) to have programmers agree on these kinds of discussions 😁.

Anyways, in short, for imports I follow the following pattern for (IMO) maximum readability:

  • for external builtin and external libraries imports, prefer importing modules instead of symbols
  • if importing from another internal submodule, prefer importing the submodule
  • create a facade for each submodule in the __init__.py, similar to typescripts barrel files, that both exposes a clean interface, hides implementation details of the submodule that should not be imported outside of it, and allows other modules to have cleaner imports
  • if importing from the same submodule, import symbols from their specific file

Reasoning for these rules:

  1. the "import modules preference" ensures the source of each symbol does not get obfuscated in large files, as it is now explicit, so the code gains clarity
  2. prevents symbol name collisions
  3. importing modules over symbols has no performance hit, as the python interpreter must parse a module entirely before allowing symbols to be imported from it
  4. the "one import per line" works better with the restrictive settings of the linter (that make an import take 3 lines if it exceeds 80 characters, because it splits it with parenthesis, creating a lot of visual noise that hurt readability and inflate the line count)
  5. also, makes the imports of the file have a consistent "shape", making it faster to read source -- symbol on every line and get a clear picture at a glance.
  6. the "do import symbols if they belong to the same submodule" has to do with preventing circular imports, the explanation for this can be long, I can elaborate if you wish

Regarding the import typing as t, I generally do it like that to have a short alias for a module that is (or should) be used a lot. It's the same reasoning why alembic imports sqlalchemy as sa. It keeps the clarity of the above principles, helps with keeping line length short (again, important so the linter does not introduce weird artifacts that hurt readability), and makes typing the code easier. I generally always import it like that even if I require only one symbol from the typing module, in case the file grows later and more are needed, which creates better git diffs (as the imports are not shuffled around so much).

Anyways, this is a summary of the benefits I considered when I came up with these. For me, in conjunction they make the code easier to work with. That said, If import typing as t annoys you I can remove it.

What do you think?


import fastapi
from sqlalchemy.orm import Session

from app.db.session import SessionLocal


def get_session() -> Generator:
def get_session() -> t.Generator[Session, None, None]:
session = SessionLocal()
try:
yield session
Expand All @@ -18,4 +23,7 @@ def get_session() -> Generator:
session.close()


SessionDependency = Annotated[Session, Depends(get_session)]
SessionDependency = t.Annotated[
Session,
fastapi.Depends(get_session),
Comment thread
DanTcheche marked this conversation as resolved.
]
4 changes: 4 additions & 0 deletions app/common/domain/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
from .entities import *
from .criteria import *
from .pagination_criteria import *
from .repository import *
5 changes: 5 additions & 0 deletions app/common/domain/criteria.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
__all__ = ("Criteria",)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense tu put all here if we only have 1 class to import?

@stefanotroncaro stefanotroncaro Feb 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good python practice to have an __all__ in every file regardless of how many symbols it exports. Besides, the amount of symbols a file exports can change over the project lifetime, so it's easier to already have it there.

Also, having an __all__ defined makes the __init__.pys require less maintenance.

If a change is made in this file, the __all__ is updated, and the __init__.py (which imports with from .criteria import *) already gets the change. So, changes are contained to one file. Otherwise, to keep the __init__.py updated, you would need to make a change in this file and then go and modify the explicit imports in the __init__.py.

So, IMO, maintaining a simple __all__ on each file is more maintainable, and adheres to python good practices.



class Criteria[TEntity]:
pass
3 changes: 3 additions & 0 deletions app/common/domain/entities/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
__all__ = ("Entity",)

from .entity import Entity
77 changes: 77 additions & 0 deletions app/common/domain/entities/_exc.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
from __future__ import annotations

__all__ = (
"EntityDefinitionError",
"MissingDtoError",
"InvalidDtoError",
)

import typing as t

from app.common.exceptions import BaseApplicationError

from . import _utils

if t.TYPE_CHECKING:
from .entity import Entity
from .entity import _DtoConstraint


class EntityDefinitionError(BaseApplicationError):
pass


class MissingDtoError(EntityDefinitionError):
def __init__(
self,
entity_cls: type[Entity],
missing_dto_constraint: _DtoConstraint,
) -> None:
dto_name, dto_cls = missing_dto_constraint
dto_bases = (dto_cls.__name__,)
model_name, model_bases = _utils.extract_class_info(entity_cls)

message_lines = (
f"Missing {dto_name} definition from {model_name} declaration.",
"",
"Example:",
_utils.build_nested_class_definition_example(
outer_class_name=model_name,
outer_class_bases=model_bases,
inner_class_name=dto_name,
inner_class_bases=dto_bases,
inner_class_statements=(
"# fields required to perform operation via repository",
),
),
)
message = "\n".join(message_lines)
super().__init__(message)


class InvalidDtoError(EntityDefinitionError):
def __init__(
self,
entity_cls: type[Entity],
invalid_dto_constraint: _DtoConstraint,
) -> None:
dto_name, dto_cls = invalid_dto_constraint
dto_bases = (dto_cls.__name__,)
model_name, model_bases = _utils.extract_class_info(entity_cls)

message_lines = (
f"Invalid {dto_name} schema definition, must be child of {dto_cls.__name__}",
"",
"Example:",
_utils.build_nested_class_definition_example(
outer_class_name=model_name,
outer_class_bases=model_bases,
inner_class_name=dto_name,
inner_class_bases=dto_bases,
inner_class_statements=(
"# fields required to perform operation via repository",
),
),
)
message = "\n".join(message_lines)
super().__init__(message)
54 changes: 54 additions & 0 deletions app/common/domain/entities/_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
__all__ = (
"build_nested_class_definition_example",
"extract_class_info",
)

import typing as t


def build_nested_class_definition_example(
*,
outer_class_name: str = "Outer",
outer_class_bases: t.Sequence[str] | None = None,
inner_class_name: str = "Inner",
inner_class_bases: t.Sequence[str] | None = None,
inner_class_statements: t.Sequence[str] = ("# ...",),
) -> str:
output_lines = (
_build_class_header(outer_class_name, outer_class_bases),
_indent(1, _build_class_header(inner_class_name, inner_class_bases)),
*_indent(2, inner_class_statements),
)

return "\n".join(output_lines)


def _build_class_header(name: str, bases: t.Sequence[str] | None) -> str:
return f"class {name}{f'({", ".join(bases)})' if bases else ''}:"


@t.overload
def _indent(level: int, val: str) -> str: ...
@t.overload
def _indent(level: int, val: t.Sequence[str]) -> t.Sequence[str]: ...


def _indent(level: int, val: str | t.Sequence[str]) -> str | t.Sequence[str]:
space = " " * level

if isinstance(val, str):
return f"{space}{val}"

return tuple(f"{space}{line}" for line in val)


class _ClassInfo(t.NamedTuple):
class_name: str
class_bases: t.Sequence[str]


def extract_class_info(cls: type) -> _ClassInfo:
return _ClassInfo(
class_name=cls.__name__,
class_bases=tuple(base.__name__ for base in cls.__bases__),
)
98 changes: 98 additions & 0 deletions app/common/domain/entities/entity.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
__all__ = ("Entity",)

import typing as t

import pydantic

from . import _exc


class _DtoConstraint(t.NamedTuple):
name: str
parent_cls: type


type _DtoConstraints = t.Collection[_DtoConstraint]
type _DtoConstraintEvaluation = _exc.EntityDefinitionError | None
type _EntityDefinitionErrors = t.Sequence[_exc.EntityDefinitionError]


# TODO: schema registry and automatic model rebuilding (fix circular imports for good)
# TODO: sentinel for unloaded relationships (fix shape of data problem)
class Entity(pydantic.BaseModel):
"""
Base model of the application, representing an object in the db.

:var CreateDto: (required) Subschema for creating an object
:var UpdateDto: (required) Subschema for updating an object

## Example:
```
class User(Entity):
id: UserId
name: str
last_name: str

class CreateDto(pydantic.BaseModel):
name: str
last_name: str

class UpdateDto(pydantic.BaseModel):
name: str | None = None
last_name: str | None = None
```
"""

model_config = pydantic.ConfigDict(
from_attributes=True,
frozen=True,
)

CreateDto: t.ClassVar[type[pydantic.BaseModel]]
UpdateDto: t.ClassVar[type[pydantic.BaseModel]]
Comment on lines +51 to +52

Copilot AI Feb 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CreateDto and UpdateDto ClassVars lack docstrings explaining their purpose and requirements. Add documentation to clarify that these must be defined in subclasses as nested classes for entity creation and update operations.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already explained in the Entity docstring. No need for redundancy here.


@classmethod
def __pydantic_init_subclass__(cls, **kw: t.Any) -> None:
super().__pydantic_init_subclass__(**kw)
cls._validate_entity_definition()

__required_dtos__: t.Final[_DtoConstraints] = (
_DtoConstraint("CreateDto", pydantic.BaseModel),
_DtoConstraint("UpdateDto", pydantic.BaseModel),
)

@classmethod
def _validate_entity_definition(cls) -> None:
errors = cls._collect_entity_definition_errors()

if errors:
msg = f"Can't create {cls.__name__} entity class"
raise ExceptionGroup(msg, errors)

@classmethod
def _collect_entity_definition_errors(cls) -> _EntityDefinitionErrors:
errors = (
cls._evaluate_dto_constraint(dto_constraint)
for dto_constraint in cls.__required_dtos__
)

return tuple(error for error in errors if error is not None)

@classmethod
def _evaluate_dto_constraint(
cls,
dto_constraint: _DtoConstraint,
) -> _DtoConstraintEvaluation:
dto_name, dto_parent_cls = dto_constraint

if not hasattr(cls, dto_name):
return _exc.MissingDtoError(cls, dto_constraint)

dto = getattr(cls, dto_name)
is_type = isinstance(dto, type)
is_subclass = is_type and issubclass(dto, dto_parent_cls)

if not is_subclass:
return _exc.InvalidDtoError(cls, dto_constraint)

return None
23 changes: 23 additions & 0 deletions app/common/domain/pagination_criteria.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
__all__ = ("PaginationCriteria",)

import typing as t

import pydantic

from .criteria import Criteria

_PageField = t.Annotated[
int,
pydantic.Field(ge=1),
]
_PageSizeField = t.Annotated[
int,
pydantic.Field(ge=1, le=100),
]


class PaginationCriteria(pydantic.BaseModel, Criteria[t.Any]):
model_config = pydantic.ConfigDict(frozen=True)

page: _PageField = 1
page_size: _PageSizeField = 10
Loading