Conversation
service/src/main/java/dmdev/mentoring/dao/querydsl/FootballClubDaoQueryDslImpl.java
Outdated
Show resolved
Hide resolved
service/src/main/java/dmdev/mentoring/dao/querydsl/FootballClubDaoQueryDslImpl.java
Show resolved
Hide resolved
| return new QPredicate(); | ||
| } | ||
|
|
||
| public <T> QPredicate add(T object, Function<T, Predicate> function) { |
There was a problem hiding this comment.
подумай как бы выглядел QPredicate если бы тебе нужно было добавить проверку на коллекцию isEmpty (если будут такие параметры в фильтре - то придется добавить)
| public class AbstractDaoTest { | ||
|
|
||
| protected SessionFactory sessionFactory; | ||
| protected Session session; |
There was a problem hiding this comment.
1 сессия на 1 метод, а не на все методы - она же хранит состояние
There was a problem hiding this comment.
так она у меня открывается и закрывается для каждого метода - BeforeEach, AfterEach
There was a problem hiding this comment.
да, все ок будет работать, если инициализировать в BeforeEach, AfterEach!
| saveFootbalClub(session, FC_NAMES[1], Warszawa); | ||
| saveFootbalClub(session, FC_NAMES[2], Kharkov); | ||
| saveFootbalClub(session, FC_NAMES[3], Kiev); | ||
| saveFootbalClub(session, FC_NAMES[4], Minsk); |
There was a problem hiding this comment.
список вместо массива? Массив вроде более лаконично выглядит, из листа пришлось бы вызывать метод гет(0), гет(1) и т.д.
| } | ||
|
|
||
| @Override | ||
| public List<FootballClub> findByCountryAndRegion(Session session, Country country, String region) { |
There was a problem hiding this comment.
уже лучше!
Но все еще нужно передавать один объект фильтр с полями, по которым будешь фильтровать. Обычно такие фильтры имеют свойства разрастаться (увеличивается кол-во полей). В таком вариант как здесь - уже 3 параметра. И уже увеличивать до 4-5 выглядит не очень.
| } | ||
|
|
||
| @Override | ||
| public List<FootballClub> findByCountryAndRegion(Session session, Country country, String region) { |
There was a problem hiding this comment.
здесь то же самое - нужно параметры представить в виде одного объекта фильтра
| sessionFactory.close(); | ||
| } | ||
|
|
||
| protected static Stream<FootballClubDao> footballClubDaoSource() { |
There was a problem hiding this comment.
абстрактные классы не могут иметь ссылка на какие-то конкретные реализации - поэтому этот метод отображает неправильное проектирование
There was a problem hiding this comment.
Этот метод вроде как утилитный - поэтому я его вынес в родителя, могу конечно обратно в наследника перенести
There was a problem hiding this comment.
даже если утилитный - он все еще находится в абстрактном классе, который не должен хранить информации про какие-то реализации, которые будут тестироваться. Это все равно, что в интерфейсе List хранить ссылки на ArrayList/LinkedList
| City.class) | ||
| .setParameter("city", "Kielce") | ||
| .list(); | ||
| actual = nativeQueryResult.get(0); |
There was a problem hiding this comment.
одни и те же ссылки не переиспользуют в тестах для разных значений - чтобы избегать flaky (хотя и в обычном коде лучше этого избегать тоже)
|
|
||
| public static void initDataBase(SessionFactory sessionFactory) { | ||
| @Cleanup Session session = sessionFactory.openSession(); | ||
|
|
There was a problem hiding this comment.
не забывай открывать транзакцию перед сохранением
There was a problem hiding this comment.
Тут с транзакцией я похоже накосячил? Почему тогда работало? Хибернейт и без транзакции все сохранил?
Как тогда? Добавить следующей строкой
session.beginTransaction();
а после сохранения всех тестовых данных закрыть транзакцию?
session.getTransaction().commit();
Или надо как-то обернуть в try-catch-finally?
There was a problem hiding this comment.
Может работать - может не работать. Hibernate все еще есть по умолчанию механизм очень похожий на auto-commit mode. Но он может не сработать во многих ситуациях (я в видео показывал). Поэтому лучше всегда явно открывать и закрывать транзакции.
В текущей реализации будет работать - все ок. Но проще вообще session передавать и вызывать этот метод перед каждым методом, а не sessionFactory, чтобы не коммитать транзакцию.
| public static FootballClub[] SWIETOKRZYSKIE_FC; | ||
| public static String[] SWIETOKRZYSKIE_FC_NAMES; | ||
| public static FootballClub[] BELARUS_FC; | ||
| public static String[] BELARUS_FC_NAMES; |
There was a problem hiding this comment.
лучше используй коллекции, а не массивы
Tasks according to Assignment