Рефакторинг сохранения конфига

[ren3104]

В одном моем проекте access tokens и refresh tokens хранятся в базе данных и используются при инициализации клиентов. Примерно так:

api = ShikimoriAPI(
    config={
        "app_name": "...",
        "client_id": "...",
        "client_secret": "...",
        "auth_code": " ",
        "redirect_uri": "...",
        "scopes": " "
    },
    logging=False
)
api._client.tokens(tokens)
async with api:
    ...

Моя проблема состоит в том, что мне не нужно сохранять конфиг в файле.

Предлагаю рефакторинг: 1) Убрать сохранение конфига в файл, либо сделать его кастомизированным, чтобы можно было выбрать не сохранять, либо сохранять в памяти (например dict переменная), файле и т.д. 2) scopes может быть пустой строкой и не пройдет эту проверку! https://github.com/SecondThundeR/shikithon/blob/0219c63dd13e37c4e11dee5a1c856903773f7461/shikithon/base_client.py#L327 3) Когда у меня уже есть access_tokens и refresh_token мне не нужен auth_code. https://github.com/SecondThundeR/shikithon/blob/0219c63dd13e37c4e11dee5a1c856903773f7461/shikithon/base_client.py#L330 4) В целях экономии я не храню token_expire, поэтому если нам неизвестно нужно ли обновить токен или нет, то можно ждать ошибку 401 и после нее обновить токен.

Как это вижу я:

# Импортируем класс клиента и готовые хранилища конфигов для авторизации
from shikithon import ShikimoriAPI, Store, NullStore, MemoryStore, JsonStore

# Конфиг можно получить из хранилища с помощью
# auth_code, либо access_token

# Можно реализовать свой способ сохранения токенов
# унаследовав базовый класс и переопределив его методы
class MyStore(Store):
    ...

api = ShikimoriAPI(
    app_name: str = "Api Test",
    store: Store = NullStore()
)

# Без авторизации
async with api:
    ...

# С авторизацией
async with api.auth(
    # Если store не NullStore и ничего из нижеперечисленного не указано, то
    # будет использован последний токен в store (store не должен быть пустым)
    app_name: Optional[str] = None, # Если None, то используем app_name из __init__
    client_id: Optional[str] = None, # Если None, то store не может быть NullStore
    client_secret: Optional[str] = None, # Если None, то store не может быть NullStore
    redirect_uri: str = "urn:ietf:wg:oauth:2.0:oob", # Имеет приоритет перед значением в store
    scopes: str = "", # Имеет приоритет перед значением в store
    auth_code: Optional[str] = None, # Если None, то access_token должен быть указан и store не NullStore
    access_token: Optional[str] = None, # Если None, то auth_code должен быть указан и store не NullStore
    refresh_token: Optional[str] = None, # Если None, то нельзя будет обновить токен
    token_expire: Optional[int] = None # Если None, то ждем ошибку 401 и после нее обновляем токен, иначе проверяем нужно ли обновить токен
):
    ...

{
    "<app_name>" : {
        "client_id": "...",
        "client_secret": "...",
        "redirect_uri": "...",
        "tokens": [
            {
                "access_token": "...",
                "refresh_token": "...",
                "token_expire": 0,
                "auth_code": "...",
                "scopes": "..."
            }
        ]
    }
}

@SecondThundeR что думаешь об этом? Если все хорошо, то я могу сделать это и кинуть pr.

[SecondThundeR]

Интересное предложение. С 1 по 3 пункты согласен и это пойдет на пользу, а насчет 4 немного сомневаюсь. Целесообразно ли делать лишний запрос чтобы узнать, нужно ли обновлять токен или нет. Хотя если это не влияет на общую работу и скорость запросов, то тогда можно сделать и так, как ты предложил.

В целом, идея мне нравится, и если не сложно тебе это сделать, то я буду признателен за пулл реквест

[ren3104]

Сейчас по умолчанию при инициализации token_expire равен -1. https://github.com/SecondThundeR/shikithon/blob/0219c63dd13e37c4e11dee5a1c856903773f7461/shikithon/base_client.py#L73 При проверке на истечение срока действия токена результат будет положительный. https://github.com/SecondThundeR/shikithon/blob/0219c63dd13e37c4e11dee5a1c856903773f7461/shikithon/base_client.py#L419 Таким образом токен будет обновлен, даже если в этом не было необходимости.

Я предлагаю при выполнении запроса, например api.animes.get() не обновлять токен, если token_expire равен None. Таким образом если access_token действителен, то запрос будет выполнен успешно, иначе к нам придет ошибка 401, тогда мы обновим токен и повторим запрос.

[SecondThundeR]

Вообще, может есть смысл проверки и работу с токенами как-то завязать с декоратором protected_method()? Потому что я пытался настроить работу API методов таким образом, что токен будет проверяться только для защищенных методов. В нем пока только базовые проверки на наличие ограниченного режима, правильных scopes и вот как раз таки, проверка на истечение токена

https://github.com/SecondThundeR/shikithon/blob/0219c63dd13e37c4e11dee5a1c856903773f7461/shikithon/decorators/protected_method.py#L93-L95

[ren3104]

Сомневаюсь, есть же такие методы, например все тот же animes.get(), который с авторизацией возвращает заполненное поле user_rate. Еще есть методы users.get() (поля in_friends, is_ignored), comments.get() (can_be_edited) и т.д.

[SecondThundeR]

И правда, забыл про разные ответы при авторизации. Возможно имеет смысл поменять логику и может получится выкинуть декоратор, если его функционал будет где-то в другом месте обрабатываться.

Попробуй тогда примерно набросать изменения и кинуть пулл реквест, а там уже посмотрим что можно будет еще доделать или поменять

[ren3104]

Возможно завтра доделаю