1. Поле uuid не уникальное. И не очень понятно, почему используется 2 идентификатора uuid и id.
2. Категория в виде строки подразумевает денормализованные данные, о чем так же нет никакой информации.
3. Поле name, в комментарии об услуге - в задаче нет никаких услуг.
4. Неверный индекс. В коде два запроса - один по UUID, для которого подошел бы хеш индекс, так как равенство и второй - по категории и активности, для которого нужен составной начиная с более селективного (категория, активность).
5. Хранение цены во float ведет к дальнейшим проблемам. Нужен int или Decimal.
1. Структура приложения запутанная и имеет высокую связанность, нарушен принцип Dependency Inversion Principle (DIP). Код не легко поддерживать, т.к. изменения в одном месте могут повлиять на другие, что в дальнейшем затруднит например замену инфраструктурных элементов.
2. Отсутствует четкая архитектура приложения. Нет внятного разделения на слои (домен, приложение, инфраструктура). Код контроллеров, репозиториев и инфраструктуры перемешан.
1. Почему-то появился класс Customer, речи о котором в задаче не было. YAGNI?
2. Отсутствует сущность Товара в доменном слое - она используется, но расположена в слое репозитория.
1. Инфраструктурный слой представлен только коннекторами к Redis, при этом и то, что находится в папке Repository и контроллеры - относятся к этому слою. Впрочем как и реализация функционала логирования.
1. Нет API-документации
1. AddToCartController.php:
- Контроллер выполняет роль сервиса, что не соответствует принципу SRP (Single Responsibility Principle).
- Напрямую создается объект JsonResponse, что нарушает Open/Closed Principle. Должен внедряться через DI как ResponseInterface.
- Нет declare(strict_types=1).
- Нет валидации входных данных. Не проверяется наличие обязательных полей `productUuid` и `quantity` в запросе.
2. GetCartController.php:
- Аналогично контроллеру AddToCartController по JsonResponse.
- Реализация use case на уровне инфраструктуры, что не соответствует принципу SRP и Open/Closed Principle.
- Ошибка в HTTP-статусе. При успешном ответе возвращается статус 404 вместо 200.
3. GetProductsController.php:
- Аналогично предыдущему контроллеру.
- Нет обработки отсутствующей категории. Не проверяется наличие параметра `category` в запросе.
4. JsonResponse.php "Класс заглушка" - не понятно зачем такой артефакт и как он коррелирует с законченной задачей.
5. Cart.php:
- `$items` - массив чего-то, хотя должен быть CartItem.
- Платежный метод и кастомер, о котором речи в задаче не было.
- Нарушение инкапсуляции. Публичный метод `addItem` позволяет изменять внутреннее состояние объекта без валидации.
6. CartItem.php:
- Использование float для цен чревато проблемами, нужно использовать либо int в виде копеек либо использовать string c BCMath или например пакет phpmoney.
7. Customer.php В задаче о нем ничего не было. Кроме того, коризна может быть от незарегистрированного пользователя.
8. Connector.php:
- В конструкторе не указан тип.
- В конструкторе return.
- В get редису в качестве ключа передается объект, хотя должно быть строкой хотя из CartManager результат session_id().
- Выбрасывается ConnectorException, а выше перехватывается просто Exception. Что может маскировать другие проблемы.
- В методах отсутствует описание типа возвращаемого значения.
- Результат получения из Redis не проверяется на null, что может привести к ошибке при десериализации.
- Использование сериализации - у редиса могут быть другие читатиели, которые ничего не знают о внутренних объектах разрабатываемого приложения, кроме того для лучшего контроля и отладки лучше иметь читаемые типы данных, например json.
9. ConnectorException.php:
- Зачем-то имплементирован Throwable с реализацией, когда можно просто наследоваться от Exception.
10. ConnectorFacade.php:
- Назван Фасадом, но по реализации какой-то кривой билдер.
- В конструкторе не указаны типы.
- build ничего не возвращает.
- В catch отсутствует сам экземпляр исключения, для которого указан класс.
- В теле catch нет обработки исключения, т.е. просто заглушка.
- Нет обработки ошибок подключения: Игнорируются исключения при подключении к Redis.
11. Product.php:
- По виду сущность продукта. Должна быть на доменном слое.
12. CartManager.php:
- Наследуется от ConnectorFacade, т.е. имеет жесткую связку, когда по идее должен имплементировать интерфейс определенный на доменном слое, сам находится на слое приложения.
- В конструкторе парента магическое число.
- В конструкторе нет типов аргументов.
- Неиспользуемый метод setLogger. Возможно он исключительно для инжектинга логера через DI контейнер, но он не вызывается. Да и в целом инжектить через что-то кроме конструктора - не очень хорошая практика, так снижает читаемость кода.
- Метод saveCart:
- Не вызывается.
- В `$this->connector->set($cart, session_id())` перепутаны аргументы и не используются именованные переменные.
- Использование session_id() - судя по тому, что пишется API, то с большой вероятностью сессии не существует и это приведет к ошибке.
- Использование session_id() - даже если бы сессия существовала и жила бы время жизни корзины или даже дольше, то пользователь может зайти с другого устройства и хранить мы её должны отдельно связанно с пользователем. Если же это решение для незарегистрированных пользователей, то лучше хранить в куках. Также судя по заданию у нас нет задачи создавать корзину, а только добавлять.
- Метод getCart:
- Опять используем session_id(), который может вернуть null и вызвать исключение. При этом мы гасим исключение и пытаемся создать Корзину с идентификатором null.
- Аргументы при создании Cart не совпадают с его сигнатурой.
- В лог пишется неинформативное сообщение "Error" без деталей исключения
- Гасим ошибки маскируя проблемы, рискуя долгое время не узнать о проблеме.
13. ProductRepository.php:
- Непонятно почему был выбран doctrine/dbal а не doctrine/orm, которая берет на себя маппинг до сущности описанной в модели. Что повышает контроль и предсказуемость.
- В коде репозитория параметры запросов (`uuid`, `category`) вставляются через конкатенацию строк, что является критической уязвимостью. Необходимо использовать подготовленные выражения.
- В use не объявлен Exception.
- Отсутствует обработка ошибок: Не обрабатывается случай, когда товар не найден.
14. CartView.php:
- Название класса CartView не соответствует его функциональности. Это скорее CartViewManager или CartViewModel.
- Логика расчета стоимости корзины не должна быть ответственностью маппера, это дело самой корзины или сервиса при анемичной модели.
- Запросы к БД в цикле - плохая практика.
15. ProductsView.php:
- Нет declare(strict_types=1).
16. Отсутствует composer.lock, а версионность с допусками определена в composer.json - лучше иметь четкое понимание, на каких версиях приложение отлаживалось в разработке, если возникнут с ним проблемы.
Ввиду описанных проблем у предоставленного кода - решил перепроектировать и полностью переделать. Некоторые моменты додумал, так исходя из задачи предположил, что id корзины приходит от сервиса, отправляющего запрос. Основываясь на предыдущем коде оставил в качестве id UIID, как универсальное решение для идентификации объектов между микросервисами.
- Спроектирована и реализована структура согласно подходам DDD, SOLID и чистой архитектуры.
- Всё приложение разложено на слои, все зависимости только выше или на одном уровне, что дает возможность замены инфраструктурных элементов, от типа ID до хранилищ и логгера с меньшими трудозатратами.
- Реализовано автоматизированное формирование документации по API в виде json
- Реализовано логирование
- Для отладки был собран стенд в docker, согласно указанным версиям стека
- Для обеспечения работоспособности кода собрана обвязка, использованы пакеты, в основном Symfony, но так же doctrine/orm и пр.
- Был реализован маппинг моделей домена на таблицы базы данных с кастомными id через слой инфраструктуры, а так же маппинг в JSON и обратно при взаимодействии с Redis
- Все рабочее, при старте создаются таблицы и заполняется фикстурами, ниже инструкция как запустить и проверить
перейти в директорию проекта
для сборки контейнеров и запуска
make init
для остановки и удаления контейнеров
make down
curl http://localhost:8080/api/doc.json
curl http://localhost:8080/api/categories/550e8400-e29b-41d4-a716-446655440001/products
curl http://localhost:8080/api/carts/550e8400-e29b-41d4-a716-446655440099
curl -X POST http://localhost:8080/api/carts/550e8400-e29b-41d4-a716-446655440099/items \
-H "Content-Type: application/json" \
-d '{
"productId": "6ba7b810-9dad-11d1-80b4-00c04fd430c1",
"quantity": 2
}'
curl http://localhost:8080/api/carts/550e8400-e29b-41d4-a716-446655440099
curl -X POST http://localhost:8080/api/carts/550e8400-e29b-41d4-a716-446655440099/items \
-H "Content-Type: application/json" \
-d '{
"productId": "6ba7b810-9dad-11d1-80b4-00c04fd430c2",
"quantity": 1
}'
curl -X GET http://localhost:8080/api/carts/550e8400-e29b-41d4-a716-446655440099
curl -X POST http://localhost:8080/api/carts/550e8400-e29b-41d4-a716-446655440090/items \
-H "Content-Type: application/json" \
-d '{
"productId": "6ba7b810-9dad-11d1-80b4-00c04fd430c2",
"quantity": 1
}'
curl -X GET http://localhost:8080/api/carts/550e8400-e29b-41d4-a716-446655440090
Логи прокинуты в var_log/dev.log
В в .env стоит уровень debug, на продкшине, соглсно задаче нужно будет поставить warning или error.