-
Notifications
You must be signed in to change notification settings - Fork 0
Создал ветку: add-bookings. #2
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
Conversation
добавил работу с базой данных, а также дал пользователям возможность бронировать вещи. 1) В файле schema.sql определены необходимые таблицы с требуемыми полями. 2) Для хранения дат используется тип TIMESTAMP WITHOUT TIME ZONE. 3) В application.properties настроены параметры доступа к базе данных PostgreSQL. 4) Созданы JPA-репозитории. 5) Созданы тесты на новую фичу, и обновлены старые.
добавил работу с базой данных, а также дал пользователям возможность бронировать вещи. 1) В файле schema.sql определены необходимые таблицы с требуемыми полями. 2) Для хранения дат используется тип TIMESTAMP WITHOUT TIME ZONE. 3) В application.properties настроены параметры доступа к базе данных PostgreSQL. 4) Созданы JPA-репозитории. 5) Созданы тесты на новую фичу, и обновлены старые. 6) Добавил файл конфигурации для тестов и перенаправил все тесты через него, чтобы пройти build проверку:
| import java.util.stream.Collectors; | ||
|
|
||
| @Component | ||
| public class BookingRepositoryImpl extends AbstractRepository<Booking, Long> implements BookingRepository { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вместо хранения данных в памяти нужно ипользовать Spring Data JPA и подключить базу данных
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вместо хранения данных в памяти использовал Spring Data JPA подключив базу данных, BookingRepositoryImpl стал не нужен.
| } | ||
|
|
||
| LocalDateTime now = LocalDateTime.now(); | ||
| if (bookingDto.getStart() == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше использовать спринговую валидаци @NotNull. Так код станет чище, а все проверку будут проходить автоматически
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В BookingServiceImpl проверки на null теперь выполняются через спринговую валидацию в контроллере.
| if (bookingDto.getStart() == null) { | ||
| throw new ShareItException.BadRequestException("Дата начала бронирования не указана"); | ||
| } | ||
| if (bookingDto.getEnd() == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нужно переписать используя сприговую валидацию
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В BookingServiceImpl проверки на null теперь выполняются через спринговую валидацию в контроллере.
| LocalDateTime now = LocalDateTime.now(); | ||
|
|
||
| switch (state.toUpperCase()) { | ||
| case "ALL": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Задание со "*"
Данный блок кода можно оптимизировать с помощью паттерна проектирования
Цепочка обязанностей (сайт может не открываться без vpn).
Предлагаю изучить этот паттерн и попробовать реализовать его.
В коммерческой разработке очень часто используются паттерны проектирования, так как код становится более чистым и структурированным. А еще его становится проще поддерживать и масштабировать.
А еще их часто спрашивают на собеседованиях 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Спасибо))
В BookingServiceImpl теперь код соответствуют паттерну "Цепочка обязанностей" он стал поддерживать уменьшение зависимости между клиентом и обработчиками, стал более чистым и структурированным.
| LocalDateTime now = LocalDateTime.now(); | ||
|
|
||
| switch (state.toUpperCase()) { | ||
| case "ALL": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь тоже поможет цепочка обязанностей
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Использовал паттерн цепочка обязанностей.
| case "ALL": | ||
| bookings = bookingRepository.findAllByBookerId(userId); | ||
| break; | ||
| case "CURRENT": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нужно представить перечисления вместо явных строк
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В BookingServiceImpl использовал перечисления вместо явных строк.
| } | ||
|
|
||
| @Override | ||
| @CachePut(value = "users", key = "#id") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Чтобы кеширование работало его нужно настроить второй уровень кэширования hibernate. Пока предлагаю удалить, а если будет время разобраться потом
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Поскольку в текущей конфигурации проекта нет настроек для кеширования, удалил лишнюю аннотацию.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Спасибо)
Добавил индексы для ускорения поиска в базе данных.
| Item update(Item item); | ||
|
|
||
| void delete(Long id); | ||
| List<Item> findByRequestId(Long requestId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Метод нигде не используется
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В ItemRepository.java удалил лишний метод findByRequestId.
| import java.util.stream.Collectors; | ||
|
|
||
| @Component | ||
| public class CommentRepositoryImpl extends AbstractRepository<Comment, Long> implements CommentRepository { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Коментарии тоже должны храниться в базе данных
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вместо хранения данных в памяти использовал Spring Data JPA подключив базу данных, CommentRepositoryImpl стал не нужен.
1) Вместо хранения данных в памяти использовал Spring Data JPA подключив базу данных, BookingRepositoryImpl стал не нужен. 2) В BookingServiceImpl проверки на null теперь выполняются через спринговую валидацию в контроллере. 3) В BookingServiceImpl использовал перечисления вместо явных строк. 4) В BookingServiceImpl теперь код соответствуют паттерну "Цепочка обязанностей" он стал поддерживать уменьшение зависимости между клиентом и обработчиками, стал более чистым и структурированным. 5) Поскольку в текущей конфигурации проекта нет настроек для кеширования, удалил лишнюю аннотацию. 6) В ItemRepository.java удалил лишний метод findByRequestId. 7) Вместо хранения данных в памяти использовал Spring Data JPA подключив базу данных, CommentRepositoryImpl стал не нужен. 8) Добавил индексы для ускорения поиска в базе данных.
1) Вместо хранения данных в памяти использовал Spring Data JPA подключив базу данных, BookingRepositoryImpl стал не нужен. 2) В BookingServiceImpl проверки на null теперь выполняются через спринговую валидацию в контроллере. 3) В BookingServiceImpl использовал перечисления вместо явных строк. 4) В BookingServiceImpl теперь код соответствуют паттерну "Цепочка обязанностей" он стал поддерживать уменьшение зависимости между клиентом и обработчиками, стал более чистым и структурированным. 5) Поскольку в текущей конфигурации проекта нет настроек для кеширования, удалил лишнюю аннотацию. 6) В ItemRepository.java удалил лишний метод findByRequestId. 7) Вместо хранения данных в памяти использовал Spring Data JPA подключив базу данных, CommentRepositoryImpl стал не нужен. 8) Добавил индексы для ускорения поиска в базе данных. 9) Добавил пустые строки в BookingStateHandler и CommentRepository, чтобы пройти проверку Build.
| @@ -22,6 +22,11 @@ public void delete(K id) { | |||
| entities.remove(id); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Данный класс больше не используется, его нужно удалить
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Удалил неиспользуемый AbstractRepository.
| this.bookingMapper = bookingMapper; | ||
|
|
||
| // Цепочки обязанностей для букера | ||
| BookingStateHandler allHandler = new AllBookingsHandler(bookingRepository); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Давай избавимся от явного определения цепочки с помощью спринга.
Для этого, нужно будет выделить интерфейс BookingStateHandler в котором будет определено два метода:
- boolean canHandle(BookingState state);
- List getBookings(Long userId, LocalDateTime now);
Затем создать абстрактный класс AbstractBookingStateHandler, который будет реализовывать методы интерфейса.
1)
@Override
public abstract boolean canHandle(BookingState state);
@Override
public abstract List<Booking> getBookings(Long userId, LocalDateTime now);
Каждый из разработчиков, должен иметь аннотацию @Component, чтобы спринг видел их как бины, а также должен реализовывать новые абстрактные методы класса AbstractBookingStateHandler.
Чтобы отделить цепочку booker от user, для всех обработчиков нужно проставить аннотацию @Qualifier с параметром "booker" или "user" соответственно.
Кстати, если нам важен порядок обработки, можно также добавить обработчикам аннотацию @Order, чтобы спринг понимал в каком порядки какой обработчик запускать.
Осталось только реализовать соборку цепочек, для этого нужно создать два отдельных сервиса:
BookerStateProcessor и OwnerStateProcessor (они тоже должны быть бинами @Service)
В каждом из них нужно объявить private final List<BookingStateHandler> handlers; а в конструкторе передать соответствующий квалификатор
public BookerStateProcessor(@Qualifier("booker") List<BookingStateHandler> handlers) {
this.handlers = handlers;
}
И создать метод обработки списка, например, с помощью фильтра:
public List<Booking> process(BookingState state, Long userId, LocalDateTime now) {
return handlers.stream()
.filter(handler -> handler.canHandle(state))
.findFirst()
.orElseThrow(() -> new IllegalArgumentException("Неизвестный статус: " + state))
.getBookings(userId, now);
}
Теперь осталось только применить процессоры в BookingServiceImpl.
Конечно, цепочка обязанностей требует больше кода и настроек, однако она помогает упросить жизнь разработчикам при добавлении новых кейсов и не раздувать огромный switch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Спасибо)
Убрал явное определение цепочки с помощью спринга, создал AbstractBookingStateHandler, обновил и разделил по папкам обработчики.
src/main/resources/schema.sql
Outdated
| -- Индекс для поиска комментариев по автору | ||
| CREATE INDEX IF NOT EXISTS idx_comments_author ON comments(author_id); | ||
| -- Индекс для сортировки комментариев по дате создания | ||
| CREATE INDEX IF NOT EXISTS idx_comments_created ON comments(created); No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Добавлять индекс почти на каждое поле — это плохая практика. Индексы занимают место, замедляют операции INSERT/UPDATE/DELETE, и чаще всего не используются, если запросы не подходят под структуру индекса.
PostgreSQL сам умеет выбирать, использовать ли индекс или нет, и если он считает, что он неэффективен — просто проигнорирует его. Но поддержка этого индекса всё равно будет тратить ресурсы.
В прошлый раз я уже скидывала ссылку на видео про индексы — рекомендую его пересмотреть. Лучше оставить только те индексы, которые:
- реально используются в WHERE, JOIN, ORDER BY,
- имеют высокую селективность (уникальные значения),
- используются в комбинации, если запрос часто фильтрует по нескольким полям.
То есть в нашем случае, индексы больше подходят для бронирований, чем для поиска каких-либо других сущностей.
Если сейчас нет времени детально разбираться — лучше вообще не добавлять лишние индексы. Всегда можно добавить нужные позже по результатам анализа запросов.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В schema удалил лишние индексы: оставив те, что больше подходят для бронирований.
1) Удалил неиспользуемый AbstractRepository. 2) Убрал явное определение цепочки с помощью спринга, обновил и разделил по папкам обработчики. 3) В schema удалил лишние индексы: оставив те, что больше подходят для бронирований. 4) Обновил тесты.
Добавил работу с базой данных, а также дал пользователям возможность бронировать вещи.