-
Notifications
You must be signed in to change notification settings - Fork 1
prod #384
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
prod #384
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @chominju02, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request appears to be a set of urgent adjustments, possibly for a production environment, given the title 'prod'. It includes a temporary bypass of an exam date validation, an update to a cron job schedule, and the complete disabling of unit tests for the application service. The changes suggest a focus on immediate operational concerns rather than new feature development or standard bug fixes, potentially to resolve a critical issue or facilitate a specific deployment scenario. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
이 PR은 cron 작업 시간 변경, import 정리, 그리고 몇 가지 중요한 코드 비활성화를 포함하고 있습니다. 특히, 시험 신청 마감일 검증 로직과 ApplicationService의 전체 테스트 스위트가 주석 처리된 점은 매우 우려스럽습니다. 이러한 변경은 프로덕션 환경에서 심각한 오류를 유발할 수 있으므로 배포 전에 반드시 수정되어야 합니다. 또한, 와일드카드 import 사용은 코드 가독성을 저해할 수 있어 개선이 필요합니다.
| ) { | ||
| List<ExamJpaEntity> exams = examJpaRepository.findAllById(examIds); | ||
| validator.examDateNotPassed(exams); | ||
| // validator.examDateNotPassed(exams); |
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.
시험 신청 마감일 검증 로직(validator.examDateNotPassed(exams))이 주석 처리되었습니다. 이 검증을 비활성화하면 마감일이 지난 시험에도 신청이 가능해져 데이터 정합성 문제 및 심각한 비즈니스 로직 오류를 유발할 수 있습니다. 이 코드를 주석 처리한 특별한 이유가 없다면 즉시 복원해야 합니다. 만약 테스트 등의 목적으로 임시 변경한 것이라면, 프로덕션에 배포되기 전에 반드시 원상 복구되어야 합니다.
| // validator.examDateNotPassed(exams); | |
| validator.examDateNotPassed(exams); |
| //package life.mosu.mosuserver.application.application; | ||
| // | ||
| //import static org.assertj.core.api.Assertions.assertThat; | ||
| //import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
| //import static org.mockito.ArgumentMatchers.any; | ||
| //import static org.mockito.ArgumentMatchers.anyList; | ||
| //import static org.mockito.ArgumentMatchers.eq; | ||
| //import static org.mockito.BDDMockito.given; | ||
| //import static org.mockito.BDDMockito.then; | ||
| //import static org.mockito.Mockito.doThrow; | ||
| //import static org.mockito.Mockito.never; | ||
| // | ||
| //import java.time.LocalDate; | ||
| //import java.util.List; | ||
| //import java.util.Set; | ||
| //import life.mosu.mosuserver.application.application.dto.RegisterApplicationCommand; | ||
| //import life.mosu.mosuserver.application.application.processor.GetApplicationsStepProcessor; | ||
| //import life.mosu.mosuserver.application.application.processor.RegisterApplicationStepProcessor; | ||
| //import life.mosu.mosuserver.application.application.processor.SaveExamTicketStepProcessor; | ||
| //import life.mosu.mosuserver.application.application.validator.ApplicationValidator; | ||
| //import life.mosu.mosuserver.application.exam.cache.ExamQuotaCacheManager; | ||
| //import life.mosu.mosuserver.application.user.UserService; | ||
| //import life.mosu.mosuserver.domain.application.entity.ApplicationJpaEntity; | ||
| //import life.mosu.mosuserver.domain.application.repository.ApplicationJpaRepository; | ||
| //import life.mosu.mosuserver.domain.exam.entity.ExamJpaEntity; | ||
| //import life.mosu.mosuserver.domain.exam.entity.ExamJpaRepository; | ||
| //import life.mosu.mosuserver.global.exception.CustomRuntimeException; | ||
| //import life.mosu.mosuserver.global.exception.ErrorCode; | ||
| //import life.mosu.mosuserver.presentation.application.dto.AgreementRequest; | ||
| //import life.mosu.mosuserver.presentation.application.dto.ApplicationGuestRequest; | ||
| //import life.mosu.mosuserver.presentation.application.dto.ApplicationRequest; | ||
| //import life.mosu.mosuserver.presentation.application.dto.ApplicationResponse; | ||
| //import life.mosu.mosuserver.presentation.application.dto.CreateApplicationResponse; | ||
| //import life.mosu.mosuserver.presentation.application.dto.ExamApplicationRequest; | ||
| //import life.mosu.mosuserver.presentation.common.FileRequest; | ||
| //import org.junit.jupiter.api.BeforeEach; | ||
| //import org.junit.jupiter.api.DisplayName; | ||
| //import org.junit.jupiter.api.Nested; | ||
| //import org.junit.jupiter.api.Test; | ||
| //import org.junit.jupiter.api.extension.ExtendWith; | ||
| //import org.mockito.Mock; | ||
| //import org.mockito.junit.jupiter.MockitoExtension; | ||
| // | ||
| //@ExtendWith(MockitoExtension.class) | ||
| //@DisplayName("ApplicationService 단위 테스트") | ||
| //class ApplicationServiceTest { | ||
| // | ||
| // private ApplicationService applicationService; | ||
| // | ||
| // @Mock | ||
| // private UserService userService; | ||
| // | ||
| // @Mock | ||
| // private ApplicationJpaRepository applicationJpaRepository; | ||
| // | ||
| // @Mock | ||
| // private ExamJpaRepository examJpaRepository; | ||
| // | ||
| // @Mock | ||
| // private ExamQuotaCacheManager cacheManager; | ||
| // | ||
| // @Mock | ||
| // private RegisterApplicationStepProcessor registerApplicationStepProcessor; | ||
| // | ||
| // @Mock | ||
| // private SaveExamTicketStepProcessor saveExamTicketStepProcessor; | ||
| // | ||
| // @Mock | ||
| // private GetApplicationsStepProcessor getApplicationsStepProcessor; | ||
| // | ||
| // @Mock | ||
| // private ApplicationValidator validator; | ||
| // | ||
| // @BeforeEach | ||
| // void setUp() { | ||
| // applicationService = new ApplicationService( | ||
| // userService, | ||
| // applicationJpaRepository, | ||
| // examJpaRepository, | ||
| // cacheManager, | ||
| // registerApplicationStepProcessor, | ||
| // saveExamTicketStepProcessor, | ||
| // getApplicationsStepProcessor, | ||
| // validator | ||
| // ); | ||
| // } | ||
| // | ||
| // // 테스트 데이터 생성 메서드들 | ||
| // private ApplicationRequest createApplicationRequest() { | ||
| // return new ApplicationRequest( | ||
| // new FileRequest("test-file.pdf", "base64encodedcontent"), | ||
| // "010-1234-5678", | ||
| // List.of( | ||
| // new ExamApplicationRequest(1L, true), | ||
| // new ExamApplicationRequest(2L, false) | ||
| // ), | ||
| // new AgreementRequest(true, true), | ||
| // List.of("국어", "수학") | ||
| // ); | ||
| // } | ||
| // | ||
| // private ApplicationRequest createApplicationRequestWithDuplicateExams() { | ||
| // return new ApplicationRequest( | ||
| // new FileRequest("test-file.pdf", "base64encodedcontent"), | ||
| // "010-1234-5678", | ||
| // List.of( | ||
| // new ExamApplicationRequest(1L, true), | ||
| // new ExamApplicationRequest(1L, false) // 중복된 시험 ID | ||
| // ), | ||
| // new AgreementRequest(true, true), | ||
| // List.of("국어", "수학") | ||
| // ); | ||
| // } | ||
| // | ||
| // private ApplicationRequest createApplicationRequestWithWrongSubjects() { | ||
| // return new ApplicationRequest( | ||
| // new FileRequest("test-file.pdf", "base64encodedcontent"), | ||
| // "010-1234-5678", | ||
| // List.of( | ||
| // new ExamApplicationRequest(1L, true), | ||
| // new ExamApplicationRequest(2L, false) | ||
| // ), | ||
| // new AgreementRequest(true, true), | ||
| // List.of("국어") // 과목이 1개만 있음 (2개여야 함) | ||
| // ); | ||
| // } | ||
| // | ||
| // private ApplicationGuestRequest createApplicationGuestRequest() { | ||
| // return new ApplicationGuestRequest( | ||
| // "테스트 학교", | ||
| // "남자", | ||
| // "홍길동", | ||
| // LocalDate.of(2000, 1, 1), | ||
| // "010-1234-5678", | ||
| // new ExamApplicationRequest(1L, true), | ||
| // Set.of("국어", "수학"), | ||
| // new FileRequest("test-file.pdf", "base64encodedcontent") | ||
| // ); | ||
| // } | ||
| // | ||
| // private ApplicationGuestRequest createApplicationGuestRequestWithWrongSubjects() { | ||
| // return new ApplicationGuestRequest( | ||
| // "테스트 학교", | ||
| // "남자", | ||
| // "홍길동", | ||
| // LocalDate.of(2000, 1, 1), | ||
| // "010-1234-5678", | ||
| // new ExamApplicationRequest(1L, true), | ||
| // Set.of("국어"), // 과목이 1개만 있음 (2개여야 함) | ||
| // new FileRequest("test-file.pdf", "base64encodedcontent") | ||
| // ); | ||
| // } | ||
| // | ||
| // @Nested | ||
| // @DisplayName("apply 메서드 테스트") | ||
| // class ApplyTest { | ||
| // | ||
| // @Test | ||
| // @DisplayName("정상적인 시험 신청 - 성공") | ||
| // void apply_Success() { | ||
| // // given | ||
| // Long userId = 1L; | ||
| // ApplicationRequest request = createApplicationRequest(); | ||
| // ApplicationJpaEntity savedApplication = new ApplicationJpaEntity(userId, | ||
| // "010-1234-5678", true, true); | ||
| // // Reflection을 사용하여 ID 설정 | ||
| // try { | ||
| // var idField = ApplicationJpaEntity.class.getDeclaredField("id"); | ||
| // idField.setAccessible(true); | ||
| // idField.set(savedApplication, 1L); | ||
| // } catch (Exception e) { | ||
| // // 테스트용이므로 간단히 처리 | ||
| // } | ||
| // | ||
| // List<ExamJpaEntity> exams = List.of( | ||
| // ExamJpaEntity.builder().build(), | ||
| // ExamJpaEntity.builder().build() | ||
| // ); | ||
| // | ||
| // given(examJpaRepository.findAllById(anyList())).willReturn(exams); | ||
| // given(applicationJpaRepository.save(any(ApplicationJpaEntity.class))).willReturn( | ||
| // savedApplication); | ||
| // | ||
| // // when | ||
| // CreateApplicationResponse response = applicationService.apply(userId, request); | ||
| // | ||
| // // then | ||
| // assertThat(response).isNotNull(); | ||
| // assertThat(response.applicationId()).isEqualTo(1L); | ||
| // | ||
| // then(validator).should().agreedToTerms(request); | ||
| // then(validator).should().requestNoDuplicateExams(anyList()); | ||
| // then(validator).should().examDateNotPassed(exams); | ||
| // then(validator).should().examNotFull(exams); | ||
| // then(validator).should().examIdsAndLunchSelection(anyList()); | ||
| // then(validator).should().noDuplicateApplication(eq(userId), anyList()); | ||
| // then(applicationJpaRepository).should().save(any(ApplicationJpaEntity.class)); | ||
| // then(registerApplicationStepProcessor).should() | ||
| // .process(any(RegisterApplicationCommand.class)); | ||
| // then(saveExamTicketStepProcessor).should().process(any()); | ||
| // | ||
| // } | ||
| // | ||
| // @Test | ||
| // @DisplayName("약관 동의하지 않은 경우 - 실패") | ||
| // void apply_WhenNotAgreedToTerms_ThrowsException() { | ||
| // // given | ||
| // Long userId = 1L; | ||
| // ApplicationRequest request = createApplicationRequest(); | ||
| // | ||
| // doThrow(new CustomRuntimeException(ErrorCode.NOT_AGREED_TO_TERMS)) | ||
| // .when(validator).agreedToTerms(request); | ||
| // | ||
| // // when & then | ||
| // assertThatThrownBy(() -> applicationService.apply(userId, request)) | ||
| // .isInstanceOf(CustomRuntimeException.class) | ||
| // .hasMessage(ErrorCode.NOT_AGREED_TO_TERMS.getMessage()); | ||
| // | ||
| // then(validator).should().agreedToTerms(request); | ||
| // then(examJpaRepository).should(never()).findAllById(anyList()); | ||
| // } | ||
| // | ||
| // @Test | ||
| // @DisplayName("중복된 시험 ID가 있는 경우 - 실패") | ||
| // void apply_WhenDuplicateExamIds_ThrowsException() { | ||
| // // given | ||
| // Long userId = 1L; | ||
| // ApplicationRequest request = createApplicationRequestWithDuplicateExams(); | ||
| // | ||
| // doThrow(new CustomRuntimeException(ErrorCode.EXAM_DUPLICATED)) | ||
| // .when(validator).requestNoDuplicateExams(anyList()); | ||
| // | ||
| // // when & then | ||
| // assertThatThrownBy(() -> applicationService.apply(userId, request)) | ||
| // .isInstanceOf(CustomRuntimeException.class) | ||
| // .hasMessage(ErrorCode.EXAM_DUPLICATED.getMessage()); | ||
| // | ||
| // then(validator).should().agreedToTerms(request); | ||
| // then(validator).should().requestNoDuplicateExams(anyList()); | ||
| // then(examJpaRepository).should(never()).findAllById(anyList()); | ||
| // } | ||
| // | ||
| // @Test | ||
| // @DisplayName("잘못된 과목 개수 - 실패") | ||
| // void apply_WhenWrongSubjectCount_ThrowsException() { | ||
| // // given | ||
| // Long userId = 1L; | ||
| // ApplicationRequest request = createApplicationRequestWithWrongSubjects(); | ||
| // | ||
| // // when & then | ||
| // assertThatThrownBy(() -> applicationService.apply(userId, request)) | ||
| // .isInstanceOf(CustomRuntimeException.class) | ||
| // .hasMessage(ErrorCode.WRONG_SUBJECT_COUNT.getMessage()); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // @DisplayName("applyByGuest 메서드 테스트") | ||
| // class ApplyByGuestTest { | ||
| // | ||
| // @Test | ||
| // @DisplayName("게스트 신청 - 성공") | ||
| // void applyByGuest_Success() { | ||
| // // given | ||
| // ApplicationGuestRequest request = createApplicationGuestRequest(); | ||
| // Long userId = 1L; | ||
| // ApplicationJpaEntity savedApplication = ApplicationJpaEntity.builder() | ||
| // .userId(userId) | ||
| // .agreedToNotices(true) | ||
| // .agreedToRefundPolicy(true) | ||
| // .build(); | ||
| // List<ExamJpaEntity> exams = List.of(ExamJpaEntity.builder().build()); | ||
| // | ||
| // given(userService.saveOrGetUser(any())).willReturn(userId); | ||
| // given(examJpaRepository.findAllById(anyList())).willReturn(exams); | ||
| // given(applicationJpaRepository.save(any(ApplicationJpaEntity.class))).willReturn( | ||
| // savedApplication); | ||
| // | ||
| // // when | ||
| // CreateApplicationResponse response = applicationService.applyByGuest(request); | ||
| // | ||
| // // then | ||
| // assertThat(response).isNotNull(); | ||
| // assertThat(response.applicationId()).isNull(); // ID는 builder에서 설정하지 않으므로 null | ||
| // | ||
| // then(userService).should().saveOrGetUser(any()); | ||
| // then(examJpaRepository).should().findAllById(anyList()); | ||
| // then(cacheManager).should() | ||
| // .increaseCurrentApplications(request.examApplication().examId()); | ||
| // } | ||
| // | ||
| // @Test | ||
| // @DisplayName("게스트 신청 시 잘못된 과목 개수 - 실패") | ||
| // void applyByGuest_WhenWrongSubjectCount_ThrowsException() { | ||
| // // given | ||
| // ApplicationGuestRequest request = createApplicationGuestRequestWithWrongSubjects(); | ||
| // | ||
| // // when & then | ||
| // assertThatThrownBy(() -> applicationService.applyByGuest(request)) | ||
| // .isInstanceOf(CustomRuntimeException.class) | ||
| // .hasMessage(ErrorCode.WRONG_SUBJECT_COUNT.getMessage()); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // @DisplayName("getApplications 메서드 테스트") | ||
| // class GetApplicationsTest { | ||
| // | ||
| // @Test | ||
| // @DisplayName("사용자의 신청 목록 조회 - 성공") | ||
| // void getApplications_Success() { | ||
| // // given | ||
| // Long userId = 1L; | ||
| // List<ApplicationResponse> expectedApplications = List.of( | ||
| // new ApplicationResponse(1L, List.of()), | ||
| // new ApplicationResponse(2L, List.of()) | ||
| // ); | ||
| // | ||
| // given(getApplicationsStepProcessor.process(userId)) | ||
| // .willReturn(expectedApplications); | ||
| // | ||
| // // when | ||
| // List<ApplicationResponse> applications = applicationService.getApplications(userId); | ||
| // | ||
| // // then | ||
| // assertThat(applications).hasSize(2); | ||
| // assertThat(applications.get(0).applicationId()).isEqualTo(1L); | ||
| // assertThat(applications.get(1).applicationId()).isEqualTo(2L); | ||
| // | ||
| // then(getApplicationsStepProcessor).should().process(userId); | ||
| // } | ||
| // | ||
| // @Test | ||
| // @DisplayName("신청 목록이 없는 경우 - 빈 목록 반환") | ||
| // void getApplications_WhenNoApplications_ReturnsEmptyList() { | ||
| // // given | ||
| // Long userId = 1L; | ||
| // List<ApplicationResponse> expectedApplications = List.of(); | ||
| // | ||
| // given(getApplicationsStepProcessor.process(userId)) | ||
| // .willReturn(expectedApplications); | ||
| // | ||
| // // when | ||
| // List<ApplicationResponse> applications = applicationService.getApplications(userId); | ||
| // | ||
| // // then | ||
| // assertThat(applications).isEmpty(); | ||
| // | ||
| // then(getApplicationsStepProcessor).should().process(userId); | ||
| // } | ||
| // } | ||
| //} |
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.
| import life.mosu.mosuserver.presentation.application.dto.CreateApplicationResponse; | ||
| import life.mosu.mosuserver.presentation.application.dto.ExamApplicationRequest; | ||
| import life.mosu.mosuserver.presentation.application.dto.SchoolApplicationCountResponse; | ||
| import life.mosu.mosuserver.presentation.application.dto.*; |
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.
와일드카드(*) import는 사용을 지양하는 것이 좋습니다. Google Java Style Guide와 같은 여러 스타일 가이드에서는 클래스를 명시적으로 import하여 코드의 가독성을 높이고, 어떤 클래스가 사용되는지 명확하게 파악하여 유지보수를 용이하게 할 것을 권장합니다. 또한, 이름 충돌 가능성을 줄일 수 있습니다.
| import life.mosu.mosuserver.presentation.application.dto.*; | |
| import life.mosu.mosuserver.presentation.application.dto.ApplicationGuestRequest; | |
| import life.mosu.mosuserver.presentation.application.dto.ApplicationRequest; | |
| import life.mosu.mosuserver.presentation.application.dto.ApplicationResponse; | |
| import life.mosu.mosuserver.presentation.application.dto.CreateApplicationResponse; | |
| import life.mosu.mosuserver.presentation.application.dto.ExamApplicationRequest; | |
| import life.mosu.mosuserver.presentation.application.dto.SchoolApplicationCountResponse; |
✨ 구현한 기능
📢 논의하고 싶은 내용
🎸 기타