Skip to content

Commit 017670f

Browse files
authored
Fix/remove processed email tasks (#71)
* refactor : 데이터 합산 로직 DTO에서 `MemberAttendanceAggregator`로 분리 * refactor : MemberService에서 단순 조회 로직 Finder로 분리, Aggregator 로직 적용 및 리팩토링 * refactor : controller의 request를 service 레이어에서 사용하는 Command로 변환하여 전달하도록 변경 * test : MemberControllerTest에서 Command를 전달하도록 수정 * test : MemberServiceTest에서 Finder로직 적용 * test : MemberFinderTest 작성 * refactor : 메소드 이름 변경, 파라매터 수정 * test : `EmailService`메소드 이름 변경 테스트코드에 반영 * feat : 이메일 task 삭제 로직 변경 * test : 테스트코드 변경 반영 * refactor : 메소드 이름 변경, 파라매터 수정 * test : `EmailService`메소드 이름 변경 테스트코드에 반영 * feat : 이메일 task 삭제 로직 변경 * test : 테스트코드 변경 반영
1 parent cabc814 commit 017670f

File tree

8 files changed

+32
-33
lines changed

8 files changed

+32
-33
lines changed

src/main/java/gdsc/konkuk/platformcore/application/email/EmailService.java

+6-4
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,11 @@ public class EmailService {
2626
public List<EmailTask> getAllTaskAsList() {
2727
return emailTaskRepository.findAll();
2828
}
29+
public List<EmailTask> getTasksInIds(List<Long> emailIds) {
30+
return emailTaskRepository.findAllById(emailIds);
31+
}
2932

30-
public EmailTask getTaskDetails(Long taskId) {
33+
public EmailTask findById(Long taskId) {
3134
return findEmailTaskById(emailTaskRepository, taskId);
3235
}
3336

@@ -68,9 +71,8 @@ public void delete(Long emailId) {
6871
}
6972

7073
@Transactional
71-
public void deleteAll(List<Long> emailIds) {
72-
List<EmailTask> tasks = emailTaskRepository.findAllById(emailIds);
73-
emailTaskRepository.deleteAllInBatch(tasks);
74+
public void deleteAll(List<EmailTask> emailTasks) {
75+
emailTaskRepository.deleteAllInBatch(emailTasks);
7476
}
7577

7678
private void validateEmailTaskAlreadySent(EmailTask emailTask) {

src/main/java/gdsc/konkuk/platformcore/application/email/EmailTaskFacade.java

+18-3
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
import org.springframework.stereotype.Service;
1111
import org.springframework.transaction.annotation.Transactional;
1212

13+
import static org.hibernate.internal.util.collections.ArrayHelper.forEach;
14+
1315
@Service
1416
@RequiredArgsConstructor
1517
@Transactional
@@ -32,13 +34,26 @@ public void update(Long emailId, EmailSendRequest request) {
3234
}
3335

3436
public void cancel(Long emailId) {
35-
emailTaskScheduler.cancelTask(String.valueOf(emailId));
37+
EmailTask savedTask = emailService.findById(emailId);
38+
cancelIfTaskNotProcessed(savedTask);
3639
emailService.delete(emailId);
3740
}
3841

3942
public void cancelAll(List<Long> emailIds) {
40-
emailIds.forEach(emailId -> emailTaskScheduler.cancelTask(String.valueOf(emailId)));
41-
emailService.deleteAll(emailIds);
43+
List<EmailTask> taskList = emailService.getTasksInIds(emailIds);
44+
cancelUnProcessedTasks(taskList);
45+
emailService.deleteAll(taskList);
46+
}
47+
48+
private void cancelIfTaskNotProcessed(EmailTask emailTask) {
49+
if(emailTask.isSent()) return;
50+
emailTaskScheduler.cancelTask(String.valueOf(emailTask.getId()));
51+
}
52+
53+
private void cancelUnProcessedTasks(List<EmailTask> taskList) {
54+
for(EmailTask task : taskList) {
55+
cancelIfTaskNotProcessed(task);
56+
}
4257
}
4358

4459
private long getWaitingPeriod(EmailTask emailTask) {

src/main/java/gdsc/konkuk/platformcore/application/email/EmailTaskScheduler.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public void scheduleSyncTask(Object emailTask, long delay) {
4444
() -> {
4545
transactionTemplate.execute(status -> {
4646
try {
47-
EmailTask sendingTask = emailService.getTaskDetails(id);
47+
EmailTask sendingTask = emailService.findById(id);
4848
emailClient.sendEmailToReceivers(sendingTask);
4949
emailService.markAsCompleted(id);
5050
} catch (Exception e) {

src/main/java/gdsc/konkuk/platformcore/controller/email/EmailController.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public ResponseEntity<SuccessResponse> getAllEmailTask() {
4141
@GetMapping("/{taskId}")
4242
public ResponseEntity<SuccessResponse> getEmailTask(@PathVariable Long taskId) {
4343
EmailTaskDetailResponse emailTask = EmailTaskMapper.mapToEmailTaskDetailsResponse(
44-
emailService.getTaskDetails(taskId));
44+
emailService.findById(taskId));
4545
return ResponseEntity.ok(SuccessResponse.of(emailTask));
4646
}
4747

src/test/java/gdsc/konkuk/platformcore/application/email/EmailIntegrationTest.java

-21
Original file line numberDiff line numberDiff line change
@@ -161,27 +161,6 @@ void should_cancel_task() {
161161
() -> taskInMemoryRepository.getTask(emailTask.getId().toString()));
162162
}
163163

164-
@Test
165-
@DisplayName("이미 처리된 작업 취소 시도 시 예외 발생")
166-
void should_fail_when_cancel_already_processed_task() throws Exception {
167-
//given
168-
EmailSendRequest emailRequest = EmailSendRequestFixture.builder()
169-
.sendAt(LocalDateTime.now().plusSeconds(1)).build()
170-
.getFixture();
171-
172-
//when
173-
EmailTask scheduledTask = emailTaskFacade.register(emailRequest);
174-
sleep(2_000);
175-
176-
//then
177-
try {
178-
emailTaskFacade.cancel(scheduledTask.getId());
179-
fail("`TaskNotFoundException` or `EmailAlreadyProcessedException` should be thrown");
180-
} catch (TaskNotFoundException | EmailAlreadyProcessedException e) {
181-
// pass
182-
}
183-
}
184-
185164
@Test
186165
void should_send_discord_message_when_email_sending_error() throws InterruptedException {
187166
//given

src/test/java/gdsc/konkuk/platformcore/application/email/EmailServiceTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,14 @@ void should_success_when_getAllTaskAsList() {
6060

6161
@Test
6262
@DisplayName("getTaskDetails : 특정 이메일 전송 작업 조회 성공")
63-
void should_success_when_getTaskDetails() {
63+
void should_success_when_findById() {
6464
// given
6565
EmailTask emailTaskToFind = EmailTaskFixture.builder().build().getFixture();
6666
given(emailTaskRepository.findById(emailTaskToFind.getId()))
6767
.willReturn(java.util.Optional.of(emailTaskToFind));
6868

6969
// when
70-
EmailTask actual = subject.getTaskDetails(emailTaskToFind.getId());
70+
EmailTask actual = subject.findById(emailTaskToFind.getId());
7171

7272
// then
7373
assertEquals(emailTaskToFind.getId(), actual.getId());

src/test/java/gdsc/konkuk/platformcore/application/email/EmailTaskFacadeTest.java

+3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package gdsc.konkuk.platformcore.application.email;
22

3+
import static org.mockito.ArgumentMatchers.any;
34
import static org.mockito.BDDMockito.given;
45
import static org.mockito.BDDMockito.willDoNothing;
56
import static org.mockito.Mockito.verify;
7+
import static org.mockito.Mockito.when;
68
import static org.mockito.MockitoAnnotations.openMocks;
79

810
import gdsc.konkuk.platformcore.controller.email.dtos.EmailSendRequest;
@@ -70,6 +72,7 @@ void should_success_when_cancel_task() {
7072
EmailTask emailTaskToCancel = EmailTaskFixture.builder().build().getFixture();
7173
willDoNothing().given(emailTaskScheduler).cancelTask(emailTaskToCancel.getId().toString());
7274
willDoNothing().given(emailService).delete(emailTaskToCancel.getId());
75+
when(emailService.findById(any())).thenReturn(emailTaskToCancel);
7376

7477
//when
7578
subject.cancel(emailTaskToCancel.getId());

src/test/java/gdsc/konkuk/platformcore/controller/email/EmailControllerTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ void should_success_when_get_specific_task() throws Exception {
222222
String jwt = jwtTokenProvider.createToken(member);
223223
EmailTask emailTaskToSee = EmailTaskFixture.builder()
224224
.id(1L).build().getFixture();
225-
given(emailService.getTaskDetails(emailTaskToSee.getId()))
225+
given(emailService.findById(emailTaskToSee.getId()))
226226
.willReturn(emailTaskToSee);
227227

228228
//when

0 commit comments

Comments
 (0)