Skip to content

Feature/roles and ban system#32

Merged
Fl1riX merged 29 commits into
mainfrom
feature/roles-and-ban-system
Jun 2, 2026
Merged

Feature/roles and ban system#32
Fl1riX merged 29 commits into
mainfrom
feature/roles-and-ban-system

Conversation

@Fl1riX

@Fl1riX Fl1riX commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Summary by Sourcery

Введение плановых задач обслуживания и корректировка работы сессий базы данных для очистки блокировок и токенов.

Новые возможности:

  • Добавлена плановая задача для автоматического отзыва просроченных блокировок.
  • Добавлено периодическое планирование очистки блокировок и magic-токенов при запуске приложения.

Улучшения:

  • Рефакторинг управления сессиями базы данных в общую фабрику асинхронных сессий для использования во всех задачах.
  • Ослаблено уникальное ограничение на активные блокировки, чтобы оно больше не зависело от времени истечения срока, при этом по‑прежнему обеспечивается уникальность для неотозванных блокировок.
Original summary in English

Summary by Sourcery

Introduce scheduled maintenance tasks and adjust database session handling for ban and token cleanup.

New Features:

  • Add a scheduled task to automatically revoke expired bans.
  • Add periodic scheduling of ban and magic token cleanup on application startup.

Enhancements:

  • Refactor database session management into a shared async session factory for use across tasks.
  • Relax the unique constraint on active bans to no longer depend on expiration time while still enforcing non-revoked uniqueness.
Original summary in English

Summary by Sourcery

Введение плановых задач обслуживания и корректировка работы сессий базы данных для очистки блокировок и токенов.

Новые возможности:

  • Добавлена плановая задача для автоматического отзыва просроченных блокировок.
  • Добавлено периодическое планирование очистки блокировок и magic-токенов при запуске приложения.

Улучшения:

  • Рефакторинг управления сессиями базы данных в общую фабрику асинхронных сессий для использования во всех задачах.
  • Ослаблено уникальное ограничение на активные блокировки, чтобы оно больше не зависело от времени истечения срока, при этом по‑прежнему обеспечивается уникальность для неотозванных блокировок.
Original summary in English

Summary by Sourcery

Introduce scheduled maintenance tasks and adjust database session handling for ban and token cleanup.

New Features:

  • Add a scheduled task to automatically revoke expired bans.
  • Add periodic scheduling of ban and magic token cleanup on application startup.

Enhancements:

  • Refactor database session management into a shared async session factory for use across tasks.
  • Relax the unique constraint on active bans to no longer depend on expiration time while still enforcing non-revoked uniqueness.

Squ1reX and others added 28 commits May 21, 2026 21:28
…d fix date fields by adding the timezone=True property to store time zone information. Fix and add missing relationships
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…d also fix the issue of multiple bans for one user simultaneously, while preserving the ban history.
* delete cloudflared.deb"

* feat(models): Start implementing the role system, add a ban table, and fix date fields by adding the timezone=True property to store time zone information. Fix and add missing relationships

* chore(.gitignore): remove tracking of different caches

* refactor(tg_link): rename MagicTokens to MagicToken

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(bot): send login-link URL on start when telegram is not linked

Co-authored-by: Cursor <cursoragent@cursor.com>

* test(auth): add JWT unit tests

Co-authored-by: Cursor <cursoragent@cursor.com>

* refactor(models): Improve the readability of field checks in the bans table

* feat(models): Add validation of the revoked_at field at the database level

* feat(models): Add checks for the expires_at and revoked_at fields, and also fix the issue of multiple bans for one user simultaneously, while preserving the ban history.

* refactor(models): Rename MagicTokens to MagicToken across the codebase for consistency

* fix(modedls): Add foreigkey to the appointments table relationship

* fix(tasks): Fix database issues with the background task for clearing magic tokens

---------

Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…18)

* feat(ci): Add Docker image building and pushing to the ghcr registry

* feat(docker-compose): Add watchtower to query the registry and download dcoker images from there.

* feat(ci): Add a commit hash tag to Docker images
* fix: Remove the get_db file from infrastructure and remove logging of all SQL queries to reduce the IO load.

* chore(jwt): Remove dprecated definition of the current time
Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
…al network (#22)

Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
* fix(auth): Fix a vulnerability in the process of linking a Telegram bot to an account by adding X-Bot-Secret to the header

* chore(auth): Improve logging of the create_telegram_magic_link function

* fix(auth): Fix X-Bot-Secret authentication. Eliminates timig attack.

* chore(requirements): update requrements.txt list

* fix(bot): Add sending of the X-Bot-Secret header from the Telegram bot

* fix(config): Remove os.environ, which causes an error in tests

* Update comment src/presentation/api/v1/auth/tg_link.py

Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>

* docs(services): Fix logging and doc string errors

---------

Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
* feat(docker): Isolate services by separating docker networks

* feat(docker): Add a system user app. Remove buffering and .pyc, __pycache__ files.

* feat(docker): Remove root privileges. Add cap_drop to disable root privileges. Enable write access to the /tmp folder.

* feat(logger): Remove logging to a file

* feat(docker): Write a user creation command in the api container and transfer the user change to the bot container

* docs: Correct syntax errors

---------

Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
* feat: Add a health endpoint to check if the API is online

* feat(docker): Add cleaning of outdated Docker images

* fix: Change your HealthCheck internet connection to avoid being blocked by RateLimiting

---------

Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
#29)

* feat: Bring the architecture to a clean archeticture. In the structure, repeat the project structure for easier navigation. Separate the models into different files.

* fix(models): Fix circular import

* fix(models): Fix circular import

* fix(models): correct error texts in models

---------

Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
* fix: Delete alembic.ini

* chore: add alembic.ini into .gitignore

* chore: update alembic scheme

* fix: Remove the backup key from config.py

* fix: Fix the SECRET_KEY test environment variable

* fix(auth): Fixing a missing SECRET_KEY error in .env

* fix(tests): Fix E401 [*] Multiple imports on one line

* feat(config): Add a check for the absence of DATABSE_URL in .env

* Add a database engine to lifespan's fastapi

* refactor: Move session factory to lifespan

---------

Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
Co-authored-by: Squ1reX <maksimhripinkov658@gmail.com>
@sourcery-ai

sourcery-ai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Руководство для ревьюера

Реализует плановую очистку просроченных банов вместе с рефакторингом жизненного цикла DB-сессий к централизованному асинхронному SessionLocal, переносом запуска планировщика в lifespan FastAPI и ужесточением ограничений уникальности банов и проверок безопасности вокруг инициализации БД.

Диаграмма последовательностей для старта FastAPI, инициализации DB-сессии и плановой очистки банов

sequenceDiagram
    participant FastAPIApp as FastAPI_app
    participant Lifespan as lifespan
    participant DBModule as database
    participant Scheduler as AsyncIOScheduler
    participant BanTask as remove_expired_bans
    participant TokenTask as cleanup_telegram_tokens
    participant GetDB as get_db

    FastAPIApp->>Lifespan: lifespan(app)
    activate Lifespan
    Lifespan->>DBModule: SessionLocal = async_sessionmaker(engine)
    Lifespan->>Scheduler: add_job(cleanup_telegram_tokens, interval 5m)
    Lifespan->>Scheduler: add_job(remove_expired_bans, interval 10m)
    Lifespan->>Scheduler: start()

    loop every 10 minutes
        Scheduler->>BanTask: remove_expired_bans()
        activate BanTask
        BanTask->>DBModule: SessionLocal
        BanTask->>DBModule: SessionLocal()
        DBModule-->>BanTask: AsyncSession
        BanTask->>BanTask: update(Ban).where(expires_at < now, revoked_at is None)
        BanTask->>BanTask: commit()
        deactivate BanTask
    end

    loop request handling
        FastAPIApp->>GetDB: get_db()
        activate GetDB
        alt [SessionLocal is None]
            GetDB->>GetDB: raise RuntimeError("Database is not initialied")
        else [SessionLocal is set]
            GetDB->>DBModule: SessionLocal()
            DBModule-->>GetDB: AsyncSession
            GetDB-->>FastAPIApp: AsyncSession
        end
        deactivate GetDB
    end

    Lifespan-->>FastAPIApp: shutdown
    Lifespan->>Scheduler: dispose()
    Lifespan->>DBModule: engine.dispose()
    deactivate Lifespan
Loading

Изменения на уровне файлов

Изменение Детали Файлы
Рефакторинг управления DB-сессиями к глобально инициализируемому асинхронному SessionLocal с проверками безопасности.
  • Добавить SessionLocal на уровне модуля с типом async_sessionmaker[AsyncSession] | None в модуле database.
  • Изменить зависимость get_db так, чтобы она больше не зависела от Request и вместо этого проверяла SessionLocal перед выдачей сессии.
  • Подключать связанный с engine async_sessionmaker к общему database.SessionLocal во время инициализации lifespan FastAPI вместо сохранения его в app.state.
src/infrastructure/db/database.py
main.py
Переработать жизненный цикл планировщика и добавить периодические задания для очистки magic token и обработки истечения банов.
  • Создать экземпляр AsyncIOScheduler на уровне модуля и перенести регистрацию задач в обработчик старта приложения, определённый внутри контекста lifespan.
  • Добавить новое плановое задание remove_expired_bans, запускаемое каждые 10 минут вместе с существующей задачей cleanup_telegram_tokens.
  • Запускать планировщик при настройке lifespan и освобождать ресурсы при завершении работы приложения.
main.py
Скорректировать логику уникальности модели банов и реализовать фоновую задачу для отзыва просроченных банов.
  • Ослабить условие уникального индекса для активных банов, чтобы игнорировать время истечения и требовать только, чтобы revoked_at было NULL.
  • Создать новую фоновую задачу remove_expired_bans, которая помечает просроченные, не отозванные баны как отозванные с стандартной причиной и фиксирует изменения.
  • Использовать общий SessionLocal в новой фоновой задаче для массовых обновлений таблицы Ban.
src/domain/models/ban_model.py
src/infrastructure/tasks/remove_bans.py
Привести задачу очистки токенов в соответствие с новым местом импорта общего SessionLocal.
  • Обновить задачу cleanup_telegram_tokens, чтобы импортировать SessionLocal из централизованного модуля database вместо прежнего расположения.
  • Убедиться, что задача очистки токенов использует ту же фабрику асинхронных сессий, что и остальное приложение.
src/infrastructure/tasks/cleanup_magic_tokens.py

Подсказки и команды

Взаимодействие с Sourcery

  • Запустить новое ревью: Оставьте комментарий @sourcery-ai review в pull request.
  • Продолжить обсуждения: Отвечайте напрямую на комментарии ревью от Sourcery.
  • Создать задачу GitHub из комментария к ревью: Попросите Sourcery создать
    issue из комментария к ревью, ответив на него. Вы также можете ответить на
    комментарий к ревью командой @sourcery-ai issue, чтобы создать issue на его основе.
  • Сгенерировать заголовок pull request: Напишите @sourcery-ai в любом месте
    заголовка pull request, чтобы сгенерировать заголовок в любой момент. Также можно
    оставить комментарий @sourcery-ai title в pull request, чтобы (повторно) сгенерировать заголовок в любой момент.
  • Сгенерировать краткое описание pull request: Напишите @sourcery-ai summary в любом
    месте тела pull request, чтобы сгенерировать описание PR в любой момент именно там,
    где вы хотите. Также можно оставить комментарий @sourcery-ai summary в pull request, чтобы
    (повторно) сгенерировать summary в любой момент.
  • Сгенерировать руководство для ревьюера: Оставьте комментарий @sourcery-ai guide в pull
    request, чтобы (повторно) сгенерировать руководство для ревьюера в любой момент.
  • Закрыть все комментарии Sourcery: Оставьте комментарий @sourcery-ai resolve в pull
    request, чтобы пометить как решённые все комментарии Sourcery. Полезно, если вы уже
    учли все замечания и больше не хотите их видеть.
  • Отклонить все ревью Sourcery: Оставьте комментарий @sourcery-ai dismiss в pull
    request, чтобы отклонить все существующие ревью от Sourcery. Особенно полезно, если вы
    хотите начать заново с новым ревью — не забудьте оставить комментарий
    @sourcery-ai review, чтобы запустить новое ревью!

Настройка работы

Зайдите в свою панель управления, чтобы:

  • Включать или отключать функции ревью, такие как автоматически сгенерированное
    Sourcery краткое описание pull request, руководство для ревьюера и другие.
  • Изменить язык ревью.
  • Добавлять, удалять или редактировать пользовательские инструкции для ревью.
  • Настраивать другие параметры ревью.

Получение помощи

Original review guide in English

Reviewer's Guide

Implements scheduled cleanup for expired bans alongside refactoring DB session lifecycle to a central async SessionLocal, moving scheduler startup into FastAPI lifespan, and tightening ban uniqueness constraints and safety checks around DB initialization.

Sequence diagram for FastAPI startup, DB session init, and scheduled ban cleanup

sequenceDiagram
    participant FastAPIApp as FastAPI_app
    participant Lifespan as lifespan
    participant DBModule as database
    participant Scheduler as AsyncIOScheduler
    participant BanTask as remove_expired_bans
    participant TokenTask as cleanup_telegram_tokens
    participant GetDB as get_db

    FastAPIApp->>Lifespan: lifespan(app)
    activate Lifespan
    Lifespan->>DBModule: SessionLocal = async_sessionmaker(engine)
    Lifespan->>Scheduler: add_job(cleanup_telegram_tokens, interval 5m)
    Lifespan->>Scheduler: add_job(remove_expired_bans, interval 10m)
    Lifespan->>Scheduler: start()

    loop every 10 minutes
        Scheduler->>BanTask: remove_expired_bans()
        activate BanTask
        BanTask->>DBModule: SessionLocal
        BanTask->>DBModule: SessionLocal()
        DBModule-->>BanTask: AsyncSession
        BanTask->>BanTask: update(Ban).where(expires_at < now, revoked_at is None)
        BanTask->>BanTask: commit()
        deactivate BanTask
    end

    loop request handling
        FastAPIApp->>GetDB: get_db()
        activate GetDB
        alt [SessionLocal is None]
            GetDB->>GetDB: raise RuntimeError("Database is not initialied")
        else [SessionLocal is set]
            GetDB->>DBModule: SessionLocal()
            DBModule-->>GetDB: AsyncSession
            GetDB-->>FastAPIApp: AsyncSession
        end
        deactivate GetDB
    end

    Lifespan-->>FastAPIApp: shutdown
    Lifespan->>Scheduler: dispose()
    Lifespan->>DBModule: engine.dispose()
    deactivate Lifespan
Loading

File-Level Changes

Change Details Files
Refactor DB session management to a globally initialized async SessionLocal with safety checks.
  • Introduce a module-level SessionLocal of type async_sessionmaker[AsyncSession]
None in the database module.
  • Change get_db dependency to no longer depend on Request and instead validate SessionLocal before yielding a session.
  • Wire the engine-bound async_sessionmaker into the shared database.SessionLocal during FastAPI lifespan initialization instead of storing it on app.state.
  • Rework scheduler lifecycle and attach periodic jobs for magic token cleanup and ban expiration handling.
    • Instantiate AsyncIOScheduler at module level and move job registration into an app startup handler defined inside the lifespan context.
    • Add a new scheduled job remove_expired_bans to run every 10 minutes alongside the existing cleanup_telegram_tokens job.
    • Start the scheduler during lifespan setup and dispose it when the application shuts down.
    main.py
    Adjust ban model uniqueness logic and implement a background task to revoke expired bans.
    • Relax the unique index condition on active bans to ignore expiration time and only require revoked_at to be NULL.
    • Create a new remove_expired_bans background task that marks expired, non-revoked bans as revoked with a standard reason and commits the update.
    • Use the shared SessionLocal in the new background task to perform bulk updates against the Ban table.
    src/domain/models/ban_model.py
    src/infrastructure/tasks/remove_bans.py
    Align token cleanup task with the new shared SessionLocal import location.
    • Update the cleanup_telegram_tokens task to import SessionLocal from the centralized database module instead of any previous location.
    • Ensure the token cleanup task uses the same async session factory as the rest of the application.
    src/infrastructure/tasks/cleanup_magic_tokens.py

    Tips and commands

    Interacting with Sourcery

    • Trigger a new review: Comment @sourcery-ai review on the pull request.
    • Continue discussions: Reply directly to Sourcery's review comments.
    • Generate a GitHub issue from a review comment: Ask Sourcery to create an
      issue from a review comment by replying to it. You can also reply to a
      review comment with @sourcery-ai issue to create an issue from it.
    • Generate a pull request title: Write @sourcery-ai anywhere in the pull
      request title to generate a title at any time. You can also comment
      @sourcery-ai title on the pull request to (re-)generate the title at any time.
    • Generate a pull request summary: Write @sourcery-ai summary anywhere in
      the pull request body to generate a PR summary at any time exactly where you
      want it. You can also comment @sourcery-ai summary on the pull request to
      (re-)generate the summary at any time.
    • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
      request to (re-)generate the reviewer's guide at any time.
    • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
      pull request to resolve all Sourcery comments. Useful if you've already
      addressed all the comments and don't want to see them anymore.
    • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
      request to dismiss all existing Sourcery reviews. Especially useful if you
      want to start fresh with a new review - don't forget to comment
      @sourcery-ai review to trigger a new review!

    Customizing Your Experience

    Access your dashboard to:

    • Enable or disable review features such as the Sourcery-generated pull request
      summary, the reviewer's guide, and others.
    • Change the review language.
    • Add, remove or edit custom review instructions.
    • Adjust other review settings.

    Getting Help

    @Fl1riX Fl1riX merged commit daec4fd into main Jun 2, 2026
    4 checks passed
    @coveralls

    coveralls commented Jun 2, 2026

    Copy link
    Copy Markdown

    Coverage Report for CI Build 26831015150

    Warning

    Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
    Quick fix: rebase this PR. Learn more →

    Coverage decreased (-0.1%) to 30.025%

    Details

    • Coverage decreased (-0.1%) from the base build.
    • Patch coverage: No coverable lines changed in this PR.
    • 12 coverage regressions across 2 files.

    Uncovered Changes

    No uncovered changes found.

    Coverage Regressions

    12 previously-covered lines in 2 files lost coverage.

    File Lines Losing Coverage Coverage
    domain/models/ban_model.py 9 56.0%
    infrastructure/db/database.py 3 55.56%

    Coverage Stats

    Coverage Status
    Relevant Lines: 1209
    Covered Lines: 363
    Line Coverage: 30.02%
    Coverage Strength: 0.6 hits per line

    💛 - Coveralls

    @sourcery-ai sourcery-ai Bot left a comment

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

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

    Привет — я нашёл 4 проблемы и оставил несколько общих замечаний:

    • В остановке планировщика в lifespan используется scheduler.dispose(), но у AsyncIOScheduler есть метод shutdown() для корректной остановки; вызов несуществующего метода приведёт к ошибке при завершении работы.
    • Фоновые задачи вроде remove_expired_bans и cleanup_telegram_tokens используют SessionLocal напрямую без проверки на None, поэтому, если планировщик сработает до инициализации SessionLocal или после освобождения engine, задачи упадут; подумайте о повторном использовании паттерна get_db или явных проверках.
    • В remove_expired_bans вы вызываете datetime.now(timezone.utc) дважды (в where и values), что может привести к слегка несовпадающим временным меткам; вычислите значение один раз и используйте его и в фильтре, и в revoked_at.
    Prompt for AI Agents
    Пожалуйста, исправьте замечания из этого code review:
    
    ## Общие комментарии
    - В остановке планировщика в `lifespan` используется `scheduler.dispose()`, но у `AsyncIOScheduler` есть метод `shutdown()` для корректной остановки; вызов несуществующего метода приведёт к ошибке при завершении работы.
    - Фоновые задачи вроде `remove_expired_bans` и `cleanup_telegram_tokens` используют `SessionLocal` напрямую без проверки на `None`, поэтому, если планировщик сработает до инициализации `SessionLocal` или после освобождения engine, задачи упадут; подумайте о повторном использовании паттерна `get_db` или явных проверках.
    - В `remove_expired_bans` вы вызываете `datetime.now(timezone.utc)` дважды (в `where` и `values`), что может привести к слегка несовпадающим временным меткам; вычислите значение один раз и используйте его и в фильтре, и в `revoked_at`.
    
    ## Индивидуальные комментарии
    
    ### Комментарий 1
    <location path="main.py" line_range="37-46" />
    <code_context>
         )
    
    -    app.state.SessionLocal = SessionLocal
    +    @app.on_event("startup")
    +    async def startup():
    +        scheduler.add_job(
    +            cleanup_telegram_tokens,
    +            "interval",
    +            minutes=5
    +        )
    +        scheduler.add_job(
    +            remove_expired_bans,
    +            "interval",
    +            minutes=10
    +        )
    +    scheduler.start()
    +
         # При старте выполняется ко до yiled
    </code_context>
    <issue_to_address>
    **issue (bug_risk):** Регистрация обработчика события startup внутри контекста lifespan может повлиять на то, когда именно задачи будут запланированы.
    
    Поскольку декоратор теперь выполняется при входе в контекст lifespan, а не при импорте модуля, обработчик может быть зарегистрирован слишком поздно или более одного раза, если контекст будет входиться повторно. Рассмотрите варианты:
    
    - Оставить обработчик `on_event("startup")` на этапе импорта модуля и выполнять только привязку `SessionLocal` в `lifespan`, или
    - Перенести планирование задач полностью в `lifespan` (без `on_event`), чтобы явно контролировать момент добавления задач и запуска планировщика.
    
    Также убедитесь с помощью тестов или логов, что логика startup выполняется ровно один раз в нужный момент жизненного цикла приложения.
    </issue_to_address>
    
    ### Комментарий 2
    <location path="main.py" line_range="55" />
    <code_context>
         yield
         # выполняется код после yiled и остановка
    
    +    scheduler.dispose()
         await engine.dispose()
    
    </code_context>
    <issue_to_address>
    **issue (bug_risk):** Использование `dispose()` для `AsyncIOScheduler`, скорее всего, некорректно; `shutdown()` — это ожидаемый API.
    
    `AsyncIOScheduler` в APScheduler 3.x должен останавливаться через `scheduler.shutdown(...)`, а не `dispose()`. `dispose()` — это метод SQLAlchemy engine и, скорее всего, приведёт здесь к `AttributeError`. Замените его на `scheduler.shutdown()` (при необходимости настроив параметр `wait`).
    </issue_to_address>
    
    ### Комментарий 3
    <location path="src/infrastructure/db/database.py" line_range="13" />
    <code_context>
    +
    +async def get_db():
    +    if SessionLocal is None:
    +        raise RuntimeError("Database is not initialied")
    +    async with SessionLocal() as session:
                 yield session
    </code_context>
    <issue_to_address>
    **nitpick (typo):** Опечатка в сообщении RuntimeError.
    
    Замените `initialied` на `initialized` в тексте ошибки для более понятных логов.
    </issue_to_address>
    
    ### Комментарий 4
    <location path="src/infrastructure/tasks/remove_bans.py" line_range="7-16" />
    <code_context>
    +from src.domain.models.ban_model import Ban
    +from src.infrastructure.db.database import SessionLocal
    +
    +async def remove_expired_bans():
    +    async with SessionLocal() as db:
    +        await db.execute(update(Ban).where(
    +                Ban.expires_at.is_not(None),
    +                Ban.expires_at < datetime.now(timezone.utc),
    +                Ban.revoked_at.is_(None)
    +            ).values(
    +                revoked_at = datetime.now(timezone.utc),
    +                revoked_reason="Expiration of the Term"
    +            )
    +        )
    +        
    +        await db.commit()
    \ No newline at end of file
    </code_context>
    <issue_to_address>
    **suggestion:** Дважды использовать `datetime.now(timezone.utc)` в одном и том же update может привести к слегка несовпадающим временным меткам.
    
    Здесь `datetime.now(timezone.utc)` вычисляется отдельно для условия `where` и для `revoked_at`. Под нагрузкой они могут немного отличаться, что усложняет анализ данных. Вычислите `now = datetime.now(timezone.utc)` один раз (например, в начале функции) и используйте его в обоих местах, чтобы сравнение и сохраняемая временная метка были согласованы.
    </issue_to_address>

    Sourcery бесплатен для open source — если вам нравятся наши обзоры, пожалуйста, поделитесь ими ✨
    Помогите мне стать полезнее! Пожалуйста, нажимайте 👍 или 👎 на каждом комментарии — я использую этот фидбек, чтобы улучшать обзоры.
    Original comment in English

    Hey - I've found 4 issues, and left some high level feedback:

    • The scheduler teardown in lifespan uses scheduler.dispose(), but AsyncIOScheduler exposes shutdown() for a clean stop; calling a non-existent method will raise at shutdown.
    • Background jobs like remove_expired_bans and cleanup_telegram_tokens use SessionLocal directly without a None guard, so if the scheduler fires before SessionLocal is initialized or after the engine is disposed, they will fail; consider reusing the get_db pattern or adding explicit checks.
    • In remove_expired_bans you call datetime.now(timezone.utc) twice (in the where and values), which can produce slightly inconsistent timestamps; compute it once and reuse the same value for both the filter and revoked_at.
    Prompt for AI Agents
    Please address the comments from this code review:
    
    ## Overall Comments
    - The scheduler teardown in `lifespan` uses `scheduler.dispose()`, but `AsyncIOScheduler` exposes `shutdown()` for a clean stop; calling a non-existent method will raise at shutdown.
    - Background jobs like `remove_expired_bans` and `cleanup_telegram_tokens` use `SessionLocal` directly without a `None` guard, so if the scheduler fires before `SessionLocal` is initialized or after the engine is disposed, they will fail; consider reusing the `get_db` pattern or adding explicit checks.
    - In `remove_expired_bans` you call `datetime.now(timezone.utc)` twice (in the `where` and `values`), which can produce slightly inconsistent timestamps; compute it once and reuse the same value for both the filter and `revoked_at`.
    
    ## Individual Comments
    
    ### Comment 1
    <location path="main.py" line_range="37-46" />
    <code_context>
         )
    
    -    app.state.SessionLocal = SessionLocal
    +    @app.on_event("startup")
    +    async def startup():
    +        scheduler.add_job(
    +            cleanup_telegram_tokens,
    +            "interval",
    +            minutes=5
    +        )
    +        scheduler.add_job(
    +            remove_expired_bans,
    +            "interval",
    +            minutes=10
    +        )
    +    scheduler.start()
    +
         # При старте выполняется ко до yiled
    </code_context>
    <issue_to_address>
    **issue (bug_risk):** Registering the startup event handler inside the lifespan context may affect when the jobs are actually scheduled.
    
    Because the decorator now runs when the lifespan context is entered rather than at import time, the handler may be registered too late or more than once if the context is re‑entered. Consider either:
    
    - Keeping the `on_event("startup")` handler at module import time and only wiring `SessionLocal` in `lifespan`, or
    - Moving the job scheduling entirely into `lifespan` (without `on_event`) so you explicitly control when jobs are added and the scheduler is started.
    
    Also verify via tests or logs that the startup logic runs exactly once at the intended point in the app lifecycle.
    </issue_to_address>
    
    ### Comment 2
    <location path="main.py" line_range="55" />
    <code_context>
         yield
         # выполняется код после yiled и остановка
    
    +    scheduler.dispose()
         await engine.dispose()
    
    </code_context>
    <issue_to_address>
    **issue (bug_risk):** Using `dispose()` on `AsyncIOScheduler` is likely incorrect; `shutdown()` is the intended API.
    
    `AsyncIOScheduler` in APScheduler 3.x should be stopped with `scheduler.shutdown(...)`, not `dispose()`. `dispose()` is a SQLAlchemy engine method and will likely trigger an `AttributeError` here. Replace this with `scheduler.shutdown()` (optionally configuring `wait`).
    </issue_to_address>
    
    ### Comment 3
    <location path="src/infrastructure/db/database.py" line_range="13" />
    <code_context>
    +
    +async def get_db():
    +    if SessionLocal is None:
    +        raise RuntimeError("Database is not initialied")
    +    async with SessionLocal() as session:
                 yield session
    </code_context>
    <issue_to_address>
    **nitpick (typo):** Typo in the RuntimeError message.
    
    Change `initialied` to `initialized` in the error message for clearer logs.
    </issue_to_address>
    
    ### Comment 4
    <location path="src/infrastructure/tasks/remove_bans.py" line_range="7-16" />
    <code_context>
    +from src.domain.models.ban_model import Ban
    +from src.infrastructure.db.database import SessionLocal
    +
    +async def remove_expired_bans():
    +    async with SessionLocal() as db:
    +        await db.execute(update(Ban).where(
    +                Ban.expires_at.is_not(None),
    +                Ban.expires_at < datetime.now(timezone.utc),
    +                Ban.revoked_at.is_(None)
    +            ).values(
    +                revoked_at = datetime.now(timezone.utc),
    +                revoked_reason="Expiration of the Term"
    +            )
    +        )
    +        
    +        await db.commit()
    \ No newline at end of file
    </code_context>
    <issue_to_address>
    **suggestion:** Using `datetime.now(timezone.utc)` twice inside the same update may lead to slightly inconsistent timestamps.
    
    Here, `datetime.now(timezone.utc)` is evaluated separately for the `where` clause and `revoked_at`. Under load, these can differ slightly, making data harder to reason about. Compute `now = datetime.now(timezone.utc)` once (e.g., at the start of the function) and reuse it in both places to keep the comparison and stored timestamp aligned.
    </issue_to_address>

    Sourcery is free for open source - if you like our reviews please consider sharing them ✨
    Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

    Comment thread main.py
    Comment on lines +37 to +46
    @app.on_event("startup")
    async def startup():
    scheduler.add_job(
    cleanup_telegram_tokens,
    "interval",
    minutes=5
    )
    scheduler.add_job(
    remove_expired_bans,
    "interval",

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

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

    issue (bug_risk): Регистрация обработчика события startup внутри контекста lifespan может повлиять на то, когда именно задачи будут запланированы.

    Поскольку декоратор теперь выполняется при входе в контекст lifespan, а не при импорте модуля, обработчик может быть зарегистрирован слишком поздно или более одного раза, если контекст будет входиться повторно. Рассмотрите варианты:

    • Оставить обработчик on_event("startup") на этапе импорта модуля и выполнять только привязку SessionLocal в lifespan, или
    • Перенести планирование задач полностью в lifespan (без on_event), чтобы явно контролировать момент добавления задач и запуска планировщика.

    Также убедитесь с помощью тестов или логов, что логика startup выполняется ровно один раз в нужный момент жизненного цикла приложения.

    Original comment in English

    issue (bug_risk): Registering the startup event handler inside the lifespan context may affect when the jobs are actually scheduled.

    Because the decorator now runs when the lifespan context is entered rather than at import time, the handler may be registered too late or more than once if the context is re‑entered. Consider either:

    • Keeping the on_event("startup") handler at module import time and only wiring SessionLocal in lifespan, or
    • Moving the job scheduling entirely into lifespan (without on_event) so you explicitly control when jobs are added and the scheduler is started.

    Also verify via tests or logs that the startup logic runs exactly once at the intended point in the app lifecycle.

    Comment thread main.py
    yield
    # выполняется код после yiled и остановка

    scheduler.dispose()

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

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

    issue (bug_risk): Использование dispose() для AsyncIOScheduler, скорее всего, некорректно; shutdown() — это ожидаемый API.

    AsyncIOScheduler в APScheduler 3.x должен останавливаться через scheduler.shutdown(...), а не dispose(). dispose() — это метод SQLAlchemy engine и, скорее всего, приведёт здесь к AttributeError. Замените его на scheduler.shutdown() (при необходимости настроив параметр wait).

    Original comment in English

    issue (bug_risk): Using dispose() on AsyncIOScheduler is likely incorrect; shutdown() is the intended API.

    AsyncIOScheduler in APScheduler 3.x should be stopped with scheduler.shutdown(...), not dispose(). dispose() is a SQLAlchemy engine method and will likely trigger an AttributeError here. Replace this with scheduler.shutdown() (optionally configuring wait).


    async def get_db():
    if SessionLocal is None:
    raise RuntimeError("Database is not initialied")

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

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

    nitpick (typo): Опечатка в сообщении RuntimeError.

    Замените initialied на initialized в тексте ошибки для более понятных логов.

    Original comment in English

    nitpick (typo): Typo in the RuntimeError message.

    Change initialied to initialized in the error message for clearer logs.

    Comment on lines +7 to +16
    async def remove_expired_bans():
    async with SessionLocal() as db:
    await db.execute(update(Ban).where(
    Ban.expires_at.is_not(None),
    Ban.expires_at < datetime.now(timezone.utc),
    Ban.revoked_at.is_(None)
    ).values(
    revoked_at = datetime.now(timezone.utc),
    revoked_reason="Expiration of the Term"
    )

    Copy link
    Copy Markdown
    Contributor

    Choose a reason for hiding this comment

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

    suggestion: Дважды использовать datetime.now(timezone.utc) в одном и том же update может привести к слегка несовпадающим временным меткам.

    Здесь datetime.now(timezone.utc) вычисляется отдельно для условия where и для revoked_at. Под нагрузкой они могут немного отличаться, что усложняет анализ данных. Вычислите now = datetime.now(timezone.utc) один раз (например, в начале функции) и используйте его в обоих местах, чтобы сравнение и сохраняемая временная метка были согласованы.

    Original comment in English

    suggestion: Using datetime.now(timezone.utc) twice inside the same update may lead to slightly inconsistent timestamps.

    Here, datetime.now(timezone.utc) is evaluated separately for the where clause and revoked_at. Under load, these can differ slightly, making data harder to reason about. Compute now = datetime.now(timezone.utc) once (e.g., at the start of the function) and reuse it in both places to keep the comparison and stored timestamp aligned.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    None yet

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants