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

Исправить ошибку InvalidRequestError при регистрации / обновлении данных пользователя #468

Open
gorskyolga opened this issue Apr 7, 2024 · 10 comments · May be fixed by #551
Assignees

Comments

@gorskyolga
Copy link
Collaborator

gorskyolga commented Apr 7, 2024

Ошибка InvalidRequestError: A transaction is already begun on this Session возникает при использовании set_categories_to_user в UserRepository при попытке создать новую сессию в строке 63 (async with self._session.begin() as transaction:).
Сейчас для обхода этой ошибки при необходимости используется добавление await self._session.commit() перед этой строкой.
Данная функция используется при получении данных о пользователе внешнего сайта external_site_user (первично и при обновлении) и при привязке пользователя user к пользователю внешнего сайта external_site_user.

Идея в том, чтобы добавление категорий пользователю происходило в рамках отдельной транзакции (всё либо ничего). Отдельную транзакцию попросили добавить в рамках этого PR.

Нужно разобраться, как правильно создавать транзакцию, чтобы не пришлось использовать костыль с добавлением await self._session.commit(). О транзакциях в sqlalchemy.ext.asyncio можно почитать здесь.

В классе ContentService есть actualize_objects, где применяется конструкция async with self._session as session:. Возможно такой вариант решения тоже подойдет.

Where:

src/core/db/repository/user.py

@gorskyolga gorskyolga changed the title Устранить ошибку InvalidRequestError при регистрации нового пользователя Устранить ошибку InvalidRequestError при регистрации / обновлении данных пользователя Apr 7, 2024
@gorskyolga gorskyolga changed the title Устранить ошибку InvalidRequestError при регистрации / обновлении данных пользователя Исправить ошибку InvalidRequestError при регистрации / обновлении данных пользователя Apr 8, 2024
@Yohimbe227
Copy link

А как вообще протестить этот момент? Я закомментил await self._session.commit(), но при каких действиях будет всплывать ошибка?

@gorskyolga
Copy link
Collaborator Author

Update по заданию:
В текущей ветке develop строки, над которыми нужно работать, 67 и 68 (await self._session.commit() и async with self._session.begin():)

@gorskyolga
Copy link
Collaborator Author

А как вообще протестить этот момент? Я закомментил await self._session.commit(), но при каких действиях будет всплывать ошибка?

Чтобы получить эту ошибку, нужно зарегистрировать нового пользователя (вызвать эндпоинт и перейти с id_hash в бот).
При подключении нового пользователя из личного кабинета вызывается эндпоинт для передачи его данных как external_site_user и пользователь переходит по ссылке, содержащей start <id_hash>, так получается user и он привязывается к external_site_user.

По коду это можно посмотреть здесь:

  • при вызове эндпоинта api/auth/external_user_registration для регистрации нового волонтера / обновления данных волонтера срабатывает функция register из ExternalSiteUserService;
  • при вызове команды start <id_hash> в боте для регистрации нового пользователя срабатывает функция register_user в UserService.

@Yohimbe227
Copy link

Это я делал. И ошибки не вижу.
_```
2024-04-19T08:17:30.595577Z [info ] Запущенна функция start_command [src.core.logging.utils] args=(Update(message=Message(channel_chat_created=False, chat=Chat(first_name='Юрий', id=400425347, type=<ChatType.PRIVATE>, username='Yohimbe25'), date=datetime.datetime(2024, 4, 19, 8, 17, 32, tzinfo=datetime.timezone.utc), delete_chat_photo=False, entities=(MessageEntity(length=6, offset=0, type=<MessageEntityType.BOT_COMMAND>),), from_user=User(first_name='Юрий', id=400425347, is_bot=False, language_code='ru', username='Yohimbe25'), group_chat_created=False, message_id=122, supergroup_chat_created=False, text='/start asdasd1'), update_id=305832870), <telegram.ext._callbackcontext.CallbackContext object at 0x000001ECF94555C0>) kwargs={'volunteer_auth_url': 'http://test6.procharity.corptest.ru/volunteers/settings/', 'fund_auth_url': 'http://test6.procharity.corptest.ru/foundations/lk/settings/', 'user_service': <src.bot.services.user.UserService object at 0x000001ECF9067150>}
2024-04-19T08:17:30.602577Z [info ] Запущенна функция register_user [src.core.logging.utils] args=(<src.bot.services.user.UserService object at 0x000001ECF9067150>, <SiteUser 26>, User(first_name='Юрий', id=400425347, is_bot=False, language_code='ru', username='Yohimbe25')) kwargs={}
Мне пофиг на коммит
2024-04-19T08:17:30.631576Z [info ] Обновлены данные пользователя user=<User 400425347> [src.bot.services.user]
2024-04-19T08:17:30.646577Z [info ] Изменены категории у пользователя [src.core.db.repository.user]

async def set_categories_to_user(self, user_id: int, categories_ids: list[int] | None) -> None:
    """Присваивает или удаляет список категорий."""
    # await self._session.commit()
    print("Мне пофиг на коммит")
    async with self._session.begin():
        await self._session.execute(delete(UsersCategories).where(UsersCategories.user_id == user_id))
        if categories_ids:
            await self._session.execute(
                insert(UsersCategories).values(
                    [{"user_id": user_id, "category_id": category_id} for category_id in categories_ids]
                )
            )
    await logger.ainfo("Изменены категории у пользователя")
Может ранее где-то не было закрыто соединение с базой, из-за чего и появлялась ошибка, а теперь это пофиксили и ошибка ушла?

@gorskyolga
Copy link
Collaborator Author

Изменила код также, как у тебя, и получилось повторить ошибку.

Нужно:

  1. Закомментировать в fill_db.py заполнение таблиц external_site_user и users:
    await filling_user_and_external_site_user_in_db(session)
    await filling_unsubscribe_reason_in_db(session)
  2. Запустить fill_db.py.
  3. Создать external_site_user через эндпоинт:
    http://127.0.0.1:8000/docs#/ExternalSiteUser/external_user_registration_api_auth_external_user_registration_post
  4. В боте вызвать команду "/start <id_hash>".

В результате бот не отобразит никакого сообщения вообще и в терминале отобразится ошибка.

@StriderDunedain
Copy link
Contributor

StriderDunedain commented Jun 7, 2024

Как работает http://127.0.0.1:8000/docs#/ExternalSiteUser/external_user_registration_api_auth_external_user_registration_post? Насколько я понял, мне нужны а) пользователь и б) его id_hash, но как получить то и другое, я пока не понял.
Выше в переписке упоминается, что надо вызвать этот эндпоинт, чтобы зарегистрировать нового пользователя, но мне возвращается только null, хотя, по идее, должен вернуться id_hash (если я все правильно понимаю), который потом надо передать в бота и там уже дальше смотреть. И как посмотреть, зарегистрировал ли пользователя? По ручке /api/users/ возвращается ответ, что detail: "Unauthorized"

@gorskyolga
Copy link
Collaborator Author

gorskyolga commented Jun 7, 2024

Для своей локальной БД ты эти данные просто придумываешь. Можешь передать в теле запроса такое:

{
  "user_id": 123,
  "id_hash": "some_id_hash",
  "first_name": "string",
  "last_name": "string",
  "email": "[email protected]",
  "moderation_status": "NEW_VOL",
  "specializations": [
    11
  ]
}

moderation_status равен NEW_VOL, т.к. это одно из возможных значений, которое принимает эндпоинт.
specializations равен 11, т.к. при заполнении локальной БД тестовыми данными с помощью fill_db.py такая категория точно создается (не архивная, не родительская).

Проверить, что пользователь зарегистрировался можно через DBeaver. Должна появиться запись в таблице external_site_users.
Если отправить в бот команду /start some_id_hash, то будет сообщение об успешной авторизации.

@StriderDunedain
Copy link
Contributor

StriderDunedain commented Jun 7, 2024

  1. Получилось избежать ошибки, если убрать метод .begin(). Проводил чистый опыт - все стер и поднял проект заново, так что все работает как надо (по крайней мере, если следовать шагам, описанным выше)
  2. Есть предложение немного переписать функцию register_user в src/bot/services/user.py на 30 строчке, чтобы сделать её покороче и понятнее (разумеется, эта версия тоже работает):
...
telegram_id = telegram_user.id
user = await self._user_repository.get_by_external_id(ext_site_user.id)
user_by_telegram_id = await self._user_repository.get_by_telegram_id(telegram_id)
# `do_update_or_create` все равно становится True и в новой реализации нигде не нужен
# Проверку `telegram_id` можно сделать через and
# else пропадает вообще, потому что в if и в else тот же функционал, что касается `user` и `do_update_or_create` 
if user and user.telegram_id != telegram_id:
    await self._update_or_create(user, external_id=None)
# А раз `do_update_or_create = True`, то if тоже пропадает
user = await self._update_or_create(
                user_by_telegram_id,  # вместо user, который создается через user = user_by_telegram_id
                ...
         )
# В остальном все так же

Если добро, сделаю две ветки и создам 2 pr'а

@gorskyolga
Copy link
Collaborator Author

  1. Если получилось решить поставленную задачу, то нужно создавать PR и прикреплять его к этой задаче.

  2. Смотри, преобразования ты начинаешь с того, что вместо двух проверок делаешь одну:
    image
    Получается, что в случае, когда user существует и он привязан к переданному ext_site_user, произойдет обновление user, а оно происходить не должно.

@StriderDunedain
Copy link
Contributor

Аааа, вот оно как! Да, пропустил этот момент, спасибо! Впредь буду внимательнее

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants