Pydantic V2 full support + massive refactoring #88
Conversation
| can_be_none = check_can_be_none(fields) | ||
|
|
||
| if value is None: | ||
| if can_be_none: | ||
| return getattr(model_column, operator)(value) | ||
|
|
||
| raise InvalidFilters(detail=f"The field `{schema_field.title}` can't be null") | ||
| raise InvalidFilters(detail=f"The field `{field_name}` can't be null") |
There was a problem hiding this comment.
В сообщении об ошибке имя поля должно быть то же, что и в сваггере. Там может быть alias, это оно?
| value: Any, | ||
| schema_field: FieldInfo, | ||
| ) -> Tuple[Optional[Any], List[str]]: | ||
| ) -> tuple[Any | None, list[str]]: | ||
| result_value, errors = None, [] | ||
|
|
||
| for type_to_cast in types: | ||
| try: | ||
| # Создаем экземпляр схемы для валидации значения |
There was a problem hiding this comment.
Коммент соответствует действительности? Создание экземпляра схемы довольно дорогая операция, а в либе и так есть тормоза. Если нам нужно кастануть несериализованное значение в объект python надо найти другой способ это сделать
|
|
||
| if SPLIT_REL in field: | ||
| value = {"field": SPLIT_REL.join(field.split(SPLIT_REL)[1:]), "order": self.sort_["order"]} | ||
| alias = aliased(self.related_model) |
There was a problem hiding this comment.
Поскольку планируем выпускать v3 версию либы есть некоторые мысли.
Я считаю, что использование рекурсивной логики и модели Node в данном репо антипаттерн. Если бы где-то строили AST и с ним работали еще имело бы смысл (не факт), а так код получается сложный, непонятный, и слабо расширяемый. Метод resolve становится точкой входа логику, которая будет бесконечно обрастать if условиями. Визуально может выглядеть привлекательно, но простота работы и производительность ниже чем конструирование фильтров через набор независимых функций.
В модуле fastapi_jsonapi/data_layers/filtering/sqlalchemy.py набор функций создан таким образом, чтобы они были максимально простыми и тестуруемыми.
Конкретно с алиасингом вида
alias = aliased(self.related_model)уже натерпелись проблем когда создавались некорректные join конструкции.
Я предлагаю отказаться от этого класса и сделать следующее
- Определить интерфейсы для набора функций фильтрации и сортировки. Чтобы выглядели наподобие сингатуры build_filter_expressions
- Вынести эти интерфейсы в shared модули filtering/sorgting общие для различных DataLayer
- Вынести специфичные для sqla вещи в filterint/sorting модули в data_layers/sqlalchemy
На уровне DataLayer предлагаю делать вызовы как-то так. Псевдокод
from data_layers.shared.filteting import build_filter_expressions
from data_layers.shared.sorting import build_sort_expressions
from data_layers.sqla.filtering import specific_filter_handler
from data_layers.sqla.sorting import specific_sorting_getter
class SqlaDataLayer:
async def build_filters(query_filter_conditions):
return build_filter_expressions(
dl_specific_callback=specific_filter_handler,
... # other args
)
async def build_sorts(query_sort_conditions):
return build_sorts_expressions(
dl_specific_callback=specific_sorting_getter,
... # other args
)
Очень абстрактно без привязки к реальному коду, но думаю ты понял о чем я.
| # don't allow arbitrary types, we don't know their behaviour | ||
| return TypeAdapter(field_type).validate_python | ||
| except PydanticSchemaGenerationError: | ||
| return field_type |
There was a problem hiding this comment.
Я бы добавил сюда log.error
| return None | ||
|
|
||
|
|
||
| def validate_relationship_name( |
There was a problem hiding this comment.
Я бы назвал check_relationship_exists поскольку проверка тут именно на существование
| target_schema: type[TypeSchema], | ||
| target_model: type[TypeModel], | ||
| relationships_info: dict[RelationshipPath, RelationshipFilteringInfo], | ||
| ) -> BinaryExpression: |
There was a problem hiding this comment.
Над аннотацией return значения надо подумать. Возврат BinaryExpression специфично для sqla, по-хорошему было бы обобщить эту функцию превратив в публичный интерфейс и вынести её в разделяемый различными DataLayer модуль
TODO: