Skip to content

Commit f3b2759

Browse files
lukaszmachajlukaszmachaj
committed
add code review adjustments
1 parent 0adf50f commit f3b2759

File tree

9 files changed

+192
-149
lines changed

9 files changed

+192
-149
lines changed

‎src/main/java/com/capgemini/training/appointmentbooking/dataaccess/repository/criteria/AppointmentCriteria.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package com.capgemini.training.appointmentbooking.dataaccess.repository.criteria;
22

33
import com.capgemini.training.appointmentbooking.common.datatype.AppointmentStatus;
4+
import lombok.Builder;
45

56
import java.time.Instant;
67

8+
@Builder
79
public record AppointmentCriteria(String treatmentName, Instant startDate, Instant endDate, AppointmentStatus status,
810
Long clientId, Long specialistId) {
911
}

‎src/main/java/com/capgemini/training/appointmentbooking/dataaccess/repository/criteria/TreatmentCriteria.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.capgemini.training.appointmentbooking.dataaccess.repository.criteria;
22

33
import com.capgemini.training.appointmentbooking.common.datatype.Specialization;
4+
import lombok.Builder;
45

6+
@Builder
57
public record TreatmentCriteria(String name, Specialization specialization) {
68
}

‎src/main/java/com/capgemini/training/appointmentbooking/dataaccess/repository/criteria/UserCriteria.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
package com.capgemini.training.appointmentbooking.dataaccess.repository.criteria;
22

3+
import lombok.Builder;
4+
5+
@Builder
36
public record UserCriteria(String firstName, String lastName, String email) {
47
}

‎src/test/java/com/capgemini/training/appointmentbooking/common/BaseTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package com.capgemini.training.appointmentbooking.common;
2+
3+
import org.assertj.core.api.WithAssertions;
4+
5+
public class BaseTest implements WithAssertions {
6+
}

‎src/test/java/com/capgemini/training/appointmentbooking/dataaccess/repository/AppointmentRepositoryTest.java‎

Lines changed: 68 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.capgemini.training.appointmentbooking.dataaccess.repository;
22

3+
import com.capgemini.training.appointmentbooking.common.BaseTest;
34
import com.capgemini.training.appointmentbooking.common.datatype.AppointmentStatus;
45
import com.capgemini.training.appointmentbooking.dataaccess.entity.AppointmentEntity;
56
import com.capgemini.training.appointmentbooking.dataaccess.entity.ClientEntity;
@@ -19,11 +20,8 @@
1920
import java.util.List;
2021
import java.util.Optional;
2122

22-
import static org.assertj.core.api.Assertions.assertThat;
23-
import static org.junit.jupiter.api.Assertions.*;
24-
2523
@DataJpaTest
26-
public class AppointmentRepositoryTest {
24+
public class AppointmentRepositoryTest extends BaseTest {
2725

2826
@Autowired
2927
private AppointmentRepository appointmentRepository;
@@ -32,38 +30,44 @@ public class AppointmentRepositoryTest {
3230
private EntityManager entityManager;
3331

3432
@Test
35-
void testFindAll() {
33+
void shouldFindAll() {
3634
// given
3735
// when
3836
List<AppointmentEntity> result = appointmentRepository.findAll();
3937

4038
// then
41-
assertThat(result).isNotEmpty();
4239
assertThat(result).hasSize(20);
4340
}
4441

4542
@Test
46-
void testFindByCriteria_findAppointmentsByTreatmentName() {
43+
void shouldFindAppointmentsByTreatmentName() {
4744
// given
48-
AppointmentCriteria criteria = new AppointmentCriteria("Konsultacja pediatryczna", null, null, null, null, null);
45+
AppointmentCriteria criteria = AppointmentCriteria
46+
.builder()
47+
.treatmentName("Konsultacja pediatryczna")
48+
.build();
4949

5050
// when
5151
List<AppointmentEntity> result = appointmentRepository.findByCriteria(criteria, entityManager);
5252

5353
// then
54-
assertThat(result).isNotEmpty().hasSize(2);
54+
assertThat(result).hasSize(2);
5555
}
5656

5757
@Test
58-
void testFindByCriteria_findAppointmentsByTreatmentNameAndStartDate() {
58+
void shouldFindAppointmentsByTreatmentNameAndStartDate() {
5959
// given
60-
AppointmentCriteria criteria = new AppointmentCriteria("Konsultacja pediatryczna", toInstant("2024-03-03 14:00:00"), null, null, null, null);
60+
AppointmentCriteria criteria = AppointmentCriteria
61+
.builder()
62+
.treatmentName("Konsultacja pediatryczna")
63+
.startDate(toInstant("2024-03-03 14:00:00"))
64+
.build();
6165

6266
// when
6367
List<AppointmentEntity> result = appointmentRepository.findByCriteria(criteria, entityManager);
6468

6569
// then
66-
assertThat(result).isNotEmpty().hasSize(2);
70+
assertThat(result).hasSize(2);
6771
}
6872

6973
private Instant toInstant(String date) {
@@ -72,59 +76,74 @@ private Instant toInstant(String date) {
7276
}
7377

7478
@Test
75-
void testFindByCriteria_findAppointmentsByTreatmentNameAndBetweenStartDateAndEndDate() {
79+
void shouldFindAppointmentsByTreatmentNameAndBetweenStartDateAndEndDate() {
7680
// given
77-
AppointmentCriteria criteria = new AppointmentCriteria("Konsultacja pediatryczna", toInstant("2024-03-03 14:00:00"), toInstant("2024-03-03 15:00:00"), null, null, null);
81+
AppointmentCriteria criteria = AppointmentCriteria
82+
.builder()
83+
.treatmentName("Konsultacja pediatryczna")
84+
.startDate(toInstant("2024-03-03 14:00:00"))
85+
.endDate(toInstant("2024-03-03 15:00:00"))
86+
.build();
7887

7988
// when
8089
List<AppointmentEntity> result = appointmentRepository.findByCriteria(criteria, entityManager);
8190

8291
// then
83-
assertThat(result).isNotEmpty().hasSize(1);
92+
assertThat(result).hasSize(1);
8493
}
8594

8695
@Test
87-
void testFindByCriteria_findAppointmentsByTreatmentNameAndAppointmentStatus() {
96+
void shouldFindAppointmentsByTreatmentNameAndAppointmentStatus() {
8897
// given
89-
AppointmentCriteria criteria = new AppointmentCriteria("Konsultacja pediatryczna", null, null, AppointmentStatus.CANCELLED, null, null);
98+
AppointmentCriteria criteria = AppointmentCriteria
99+
.builder()
100+
.treatmentName("Konsultacja pediatryczna")
101+
.status(AppointmentStatus.CANCELLED)
102+
.build();
90103

91104
// when
92105
List<AppointmentEntity> result = appointmentRepository.findByCriteria(criteria, entityManager);
93106

94107
// then
95-
assertThat(result).isNotEmpty().hasSize(2);
108+
assertThat(result).hasSize(2);
96109
}
97110

98111
@Test
99-
void testFindByCriteria_findAppointmentsByClientId() {
112+
void shouldFindAppointmentsByClientId() {
100113
// given
101114
Long clientId = -1L;
102-
AppointmentCriteria criteria = new AppointmentCriteria(null, null, null, null, clientId, null);
115+
AppointmentCriteria criteria = AppointmentCriteria
116+
.builder()
117+
.clientId(clientId)
118+
.build();
103119

104120
// when
105121
List<AppointmentEntity> result = appointmentRepository.findByCriteria(criteria, entityManager);
106122

107123
// then
108-
assertThat(result).isNotEmpty().hasSize(5);
124+
assertThat(result).hasSize(5);
109125
assertThat(result).extracting(AppointmentEntity::getClient).extracting(ClientEntity::getId).containsOnly(clientId);
110126
}
111127

112128
@Test
113-
void testFindByCriteria_findAppointmentsBySpecialistId() {
129+
void shouldFindAppointmentsBySpecialistId() {
114130
// given
115131
Long specialistId = -1L;
116-
AppointmentCriteria criteria = new AppointmentCriteria(null, null, null, null, null, specialistId);
132+
AppointmentCriteria criteria = AppointmentCriteria
133+
.builder()
134+
.specialistId(specialistId)
135+
.build();
117136

118137
// when
119138
List<AppointmentEntity> result = appointmentRepository.findByCriteria(criteria, entityManager);
120139

121140
// then
122-
assertThat(result).isNotEmpty().hasSize(4);
141+
assertThat(result).hasSize(4);
123142
assertThat(result).extracting(AppointmentEntity::getTreatment).extracting(TreatmentEntity::getSpecialist).extracting(SpecialistEntity::getId).containsOnly(specialistId);
124143
}
125144

126145
@Test
127-
void testFindByDateTimeBetweenAndStatus() {
146+
void shouldFindAppointmentsByDateTimeBetweenAndStatus() {
128147
// given
129148
Instant startDate = toInstant("2024-03-10 00:00:00");
130149
Instant endDate = toInstant("2024-03-14 23:59:59");
@@ -134,58 +153,56 @@ void testFindByDateTimeBetweenAndStatus() {
134153
List<AppointmentEntity> result = appointmentRepository.findByDateTimeBetweenAndStatus(startDate, endDate, status);
135154

136155
// then
137-
assertThat(result).isNotEmpty().hasSize(3);
156+
assertThat(result).hasSize(3);
138157
assertThat(result).extracting(AppointmentEntity::getStatus).containsOnly(status);
139158
}
140159

141160
@Test
142-
void testFindById() {
161+
void shouldFindAppointmentById() {
143162
// given
144163
Long appointmentId = -1L;
145164

146165
// when
147166
Optional<AppointmentEntity> result = appointmentRepository.findById(appointmentId);
148167

149168
// then
150-
assertTrue(result.isPresent(), "AppointmentEntity should be present");
151-
152-
AppointmentEntity appointment = result.get();
153-
assertNotNull(appointment);
154-
assertEquals(appointmentId, appointment.getId());
155-
assertEquals(AppointmentStatus.SCHEDULED, appointment.getStatus());
156-
assertNotNull(appointment.getClient());
157-
assertNotNull(appointment.getTreatment());
158-
assertEquals(toInstant("2024-03-01 09:00:00"), appointment.getDateTime());
169+
assertThat(result).isPresent().hasValueSatisfying(appointment -> {
170+
assertThat(appointment.getId()).isEqualTo(appointmentId);
171+
assertThat(appointment.getStatus()).isEqualTo(AppointmentStatus.SCHEDULED);
172+
assertThat(appointment.getClient()).isNotNull();
173+
assertThat(appointment.getTreatment()).isNotNull();
174+
assertThat(appointment.getDateTime()).isEqualTo(toInstant("2024-03-01 09:00:00"));
175+
});
159176
}
160177

161178
@Test
162-
void testFindAppointmentsBySpecialistIdBeforeDate() {
163-
// Given
179+
void shouldFindAppointmentsBySpecialistIdBeforeDate() {
180+
// given
164181
Long specialistId = -1L;
165182
Instant date = toInstant("2024-03-12 09:00:00");
166183

167-
// When
184+
// when
168185
List<AppointmentEntity> appointments = appointmentRepository.findAppointmentsBySpecialistIdBeforeDate(specialistId, date);
169186

170-
// Then
171-
assertNotNull(appointments, "Appointments list should not be null");
172-
assertEquals(2, appointments.size(), "Expected 2 past appointments");
173-
appointments.forEach(a -> {
174-
assertEquals(specialistId, a.getTreatment().getSpecialist().getId(), "Appointment belongs to correct specialist");
175-
assertTrue(a.getDateTime().isBefore(date), "Appointment should be in the past");
176-
});
187+
// then
188+
assertThat(appointments)
189+
.hasSize(2)
190+
.allMatch(a -> a.getTreatment().getSpecialist().getId().equals(specialistId),
191+
"All appointments belong to the correct specialist")
192+
.allMatch(a -> a.getDateTime().isBefore(date),
193+
"All appointments should be before the given date");
177194
}
178195

179196
@Test
180-
void testFindConflictedAppointments() {
181-
// Given
197+
void shouldFindConflictedAppointment() {
198+
// given
182199
Long specialistId = -1L;
183200
Instant date = toInstant("2024-03-05 11:45:00");
184201

185-
// When
202+
// when
186203
boolean conflict = appointmentRepository.hasConflictingAppointmentBySpecialistIdAndDateTime(specialistId, date);
187204

188-
// Then
189-
assertTrue(conflict, "Should find conflicted appointments");
205+
// then
206+
assertThat(conflict).isTrue();
190207
}
191208
}

‎src/test/java/com/capgemini/training/appointmentbooking/dataaccess/repository/ClientRepositoryTest.java‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.capgemini.training.appointmentbooking.dataaccess.repository;
22

3+
import com.capgemini.training.appointmentbooking.common.BaseTest;
34
import com.capgemini.training.appointmentbooking.dataaccess.entity.ClientEntity;
45
import jakarta.persistence.EntityManager;
56
import jakarta.persistence.PersistenceContext;
@@ -9,12 +10,8 @@
910

1011
import java.util.List;
1112

12-
import static org.assertj.core.api.Assertions.assertThat;
13-
import static org.junit.jupiter.api.Assertions.assertEquals;
14-
import static org.junit.jupiter.api.Assertions.assertNotNull;
15-
1613
@DataJpaTest
17-
public class ClientRepositoryTest {
14+
public class ClientRepositoryTest extends BaseTest {
1815

1916
@Autowired
2017
private ClientRepository clientRepository;
@@ -23,7 +20,7 @@ public class ClientRepositoryTest {
2320
private EntityManager entityManager;
2421

2522
@Test
26-
void testFindByQueryDSL_testFindClientsByName() {
23+
void shouldFindClientsByName() {
2724
// given
2825
String firstName = "Stefan";
2926
String lastName = "Kowalski";
@@ -32,10 +29,12 @@ void testFindByQueryDSL_testFindClientsByName() {
3229
List<ClientEntity> clients = clientRepository.findByName(firstName, lastName, entityManager);
3330

3431
// then
35-
assertThat(clients).isNotEmpty().hasSize(1);
36-
ClientEntity client = clients.getFirst();
37-
assertNotNull(client.getUser(), "Expected client to have an associated user");
38-
assertEquals(firstName, client.getUser().getFirstname(), "Expected client to have the specified first name");
39-
assertEquals(lastName, client.getUser().getLastname(), "Expected client to have the specified last name");
32+
assertThat(clients)
33+
.hasSize(1)
34+
.first()
35+
.satisfies(client -> {
36+
assertThat(client.getUser().getFirstname()).isEqualTo(firstName).as("Expected client to have the specified first name");
37+
assertThat(client.getUser().getLastname()).isEqualTo(lastName).as("Expected client to have the specified last name");
38+
});
4039
}
4140
}

‎src/test/java/com/capgemini/training/appointmentbooking/dataaccess/repository/SpecialistRepositoryTest.java‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.capgemini.training.appointmentbooking.dataaccess.repository;
22

3+
import com.capgemini.training.appointmentbooking.common.BaseTest;
34
import com.capgemini.training.appointmentbooking.common.datatype.Specialization;
45
import com.capgemini.training.appointmentbooking.dataaccess.entity.SpecialistEntity;
56
import jakarta.persistence.EntityManager;
@@ -10,12 +11,8 @@
1011

1112
import java.util.List;
1213

13-
import static org.assertj.core.api.Assertions.assertThat;
14-
import static org.junit.jupiter.api.Assertions.assertEquals;
15-
import static org.junit.jupiter.api.Assertions.assertNotNull;
16-
1714
@DataJpaTest
18-
public class SpecialistRepositoryTest {
15+
public class SpecialistRepositoryTest extends BaseTest {
1916

2017
@Autowired
2118
private SpecialistRepository specialistRepository;
@@ -24,7 +21,7 @@ public class SpecialistRepositoryTest {
2421
private EntityManager entityManager;
2522

2623
@Test
27-
void testFindBySpecialization() {
24+
void shouldFindSpecialistsBySpecialization() {
2825
// given
2926
Specialization specialization = Specialization.DENTIST;
3027

@@ -33,14 +30,13 @@ void testFindBySpecialization() {
3330

3431
// then
3532
assertThat(specialists)
36-
.isNotEmpty()
3733
.hasSize(1)
3834
.allMatch(specialist -> specialist.getSpecialization() == specialization,
3935
"Specialists should all have specialization " + specialization);
4036
}
4137

4238
@Test
43-
void testFindSpecialistByName() {
39+
void shouldFindSpecialistsByName() {
4440
// given
4541
String firstName = "Dobromir";
4642
String lastName = "Zegula";
@@ -49,10 +45,12 @@ void testFindSpecialistByName() {
4945
List<SpecialistEntity> specialists = specialistRepository.findSpecialistByName(firstName, lastName, entityManager);
5046

5147
// then
52-
assertThat(specialists).isNotEmpty().hasSize(1);
53-
SpecialistEntity specialist = specialists.getFirst();
54-
assertNotNull(specialist.getUser(), "Expected specialist to have an associated user");
55-
assertEquals(firstName, specialist.getUser().getFirstname(), "Expected specialist to have the specified first name");
56-
assertEquals(lastName, specialist.getUser().getLastname(), "Expected specialist to have the specified last name");
48+
assertThat(specialists)
49+
.hasSize(1)
50+
.first()
51+
.satisfies(specialist -> {
52+
assertThat(specialist.getUser().getFirstname()).isEqualTo(firstName).as("Expected specialist to have the specified first name");
53+
assertThat(specialist.getUser().getLastname()).isEqualTo(lastName).as("Expected specialist to have the specified last name");
54+
});
5755
}
5856
}

0 commit comments

Comments
 (0)