[UPLUS-121] feat : Cloud Function API 로직 수정 및 DTO 수정#38
Conversation
…/UPLUS-121 # Conflicts: # src/main/resources/application-secret.yml # src/main/resources/logback-spring.xml
Summary of ChangesHello @paul0755, 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! 이 PR은 배치 스케줄링 시스템을 데이터베이스 기반에서 Cloud Function API 기반으로 전환하고, 배치 대시보드에 스케줄 정보를 통합하여 가시성을 높이는 것을 목표로 합니다. 또한, Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 PR은 배치 스케줄링 로직을 Cloud Function을 사용하도록 리팩터링하고, 관련 DTO와 API 로직을 수정한 내용이네요. 전반적으로 PR의 목적에 맞게 잘 구현되었지만, 코드의 유지보수성, 테스트 용이성, 안정성을 높일 수 있는 몇 가지 개선점을 발견했습니다. 특히, 하드코딩된 문자열 제거, 테스트 코드의 완전성 확보, 외부 API 호출 시 URL 생성 방식 개선 등에 대한 피드백을 포함했습니다. 해당 내용들을 반영하면 더 견고한 코드가 될 것 같습니다.
| @DisplayName("[조회] 성공 - 배치 대시보드 조회") | ||
| void getDashboard_success() { |
There was a problem hiding this comment.
이 테스트는 현재 실패할 가능성이 높습니다. InvoiceService는 BatchScheduleClient에 의존하지만, 이 테스트 클래스에서는 BatchScheduleClient를 Mock으로 만들거나 주입하지 않았습니다. @InjectMocks는 null인 필드를 주입하지 않으므로 invoiceService.batchScheduleClient는 null이 되어 getDashboard() 메서드 실행 중 NullPointerException이 발생합니다.
테스트가 정상적으로 동작하려면 BatchScheduleClient를 Mock으로 추가하고, given()을 사용하여 해당 Mock의 동작을 정의해야 합니다.
@ExtendWith(MockitoExtension.class)
class InvoiceServiceTest {
// ...
@Mock private com.project.core.util.BatchScheduleClient batchScheduleClient;
@InjectMocks private InvoiceService invoiceService;
@Test
void getDashboard_success() {
// given
// ...
given(batchScheduleClient.getSchedule(anyString())).willReturn(Optional.empty()); // 또는 mock 응답을 설정
// when
// ...
}
}| public boolean isEnabled() { | ||
| return "ENABLED".equalsIgnoreCase(state); | ||
| } |
There was a problem hiding this comment.
isEnabled() 메서드에서 "ENABLED" 문자열을 직접 사용하고 있습니다. 이런 매직 스트링은 오타를 유발하거나 일관성을 해칠 수 있습니다. 상태 값을 나타내는 상수를 선언하여 사용하는 것이 좋습니다.
public class BatchScheduleResponse {
private static final String ENABLED_STATE = "ENABLED";
// ...
public boolean isEnabled() {
return ENABLED_STATE.equalsIgnoreCase(state);
}
}| JOIN BATCH_JOB_EXECUTION je | ||
| ON ji.JOB_INSTANCE_ID = je.JOB_INSTANCE_ID | ||
| WHERE je.STATUS = 'FAILED' | ||
| AND ji.JOB_NAME IN ('invoiceJob', 'invoiceItemJob') |
There was a problem hiding this comment.
SQL 쿼리 내에 'invoiceJob', 'invoiceItemJob'과 같은 잡 이름이 하드코딩되어 있습니다. 이는 유지보수를 어렵게 만들 수 있습니다. BatchJobType enum을 활용하여 잡 이름을 동적으로 구성하거나, PreparedStatement의 파라미터로 전달하는 것이 좋습니다. 이렇게 하면 BatchJobType enum이 변경될 때 쿼리도 자동으로 반영됩니다.
예를 들어, 다음과 같이 ? 플레이스홀더를 사용하고 jdbcTemplate.query 메서드에 인자를 전달할 수 있습니다.
// 쿼리 수정
String sql =
"""
...
AND ji.JOB_NAME IN (?, ?)
...
LIMIT ?
""";
// jdbcTemplate 호출 수정
jdbcTemplate.query(sql, rowMapper, BatchJobType.INVOICE_JOB.getJobName(), BatchJobType.INVOICE_ITEM_JOB.getJobName(), limit);|
|
||
| CronExpression springCron = CronExpression.parse(springCronStr); | ||
|
|
||
| ZonedDateTime nextFire = springCron.next(ZonedDateTime.now(zoneId)); |
There was a problem hiding this comment.
ZonedDateTime.now(zoneId)를 직접 호출하면 현재 시간에 의존하게 되어 단위 테스트를 작성하기 어렵습니다. 테스트 용이성을 높이기 위해 java.time.Clock을 주입받아 사용하거나, getDashboard 메서드에서 현재 시간을 변수로 받아 하위 메서드로 전달하는 방식을 고려해 보세요.
Clock을 사용하는 예시입니다:
// In a @Configuration class
@Bean
public Clock clock() {
return Clock.systemDefaultZone();
}
// In InvoiceService
@RequiredArgsConstructor
public class InvoiceService {
private final Clock clock;
// ...
private BatchSummaryDto applySchedule(...) {
// ...
ZonedDateTime nextFire = springCron.next(ZonedDateTime.now(clock).withZoneSameInstant(zoneId));
// ...
}
}테스트에서는 Clock.fixed(...)를 주입하여 시간을 고정할 수 있습니다.
| CronParser parser = | ||
| new CronParser(CronDefinitionBuilder.instanceDefinitionFor(CronType.UNIX)); | ||
| Cron cron = parser.parse(cronStr); | ||
|
|
||
| String cronDescription = CronDescriptor.instance(Locale.KOREAN).describe(cron); |
There was a problem hiding this comment.
applySchedule 메서드가 호출될 때마다 CronParser와 CronDescriptor 인스턴스가 새로 생성됩니다. 이 객체들은 스레드에 안전하며 재사용이 가능하므로, private static final 필드로 선언하여 애플리케이션 시작 시 한 번만 생성하도록 하는 것이 성능에 더 효율적입니다.
public class InvoiceService {
private static final CronParser UNIX_CRON_PARSER = new CronParser(CronDefinitionBuilder.instanceDefinitionFor(CronType.UNIX));
private static final CronDescriptor CRON_DESCRIPTOR_KOREAN = CronDescriptor.instance(Locale.KOREAN);
// ...
private BatchSummaryDto applySchedule(...) {
// ...
try {
// ...
Cron cron = UNIX_CRON_PARSER.parse(cronStr);
String cronDescription = CRON_DESCRIPTOR_KOREAN.describe(cron);
// ...
} catch (Exception e) {
// ...
}
}
}| public Optional<BatchScheduleResponse> getSchedule(String jobName) { | ||
|
|
||
| String title = BatchJobType.getTitleByJobName(jobName); | ||
| String url = properties.getBaseUrl() + "/schedule?job=" + title; |
There was a problem hiding this comment.
URL을 문자열 연결로 생성하고 있습니다. 쿼리 파라미터에 특수 문자가 포함될 경우 문제가 발생할 수 있으므로, UriComponentsBuilder를 사용하여 URL을 안전하게 생성하고 파라미터를 인코딩하는 것이 좋습니다.
String url = org.springframework.web.util.UriComponentsBuilder.fromHttpUrl(properties.getBaseUrl())
.path("/schedule")
.queryParam("job", title)
.toUriString();
SonarQube Quality Summary (Community)❌ Quality Gate FAILED Branch: Issues
Measures
🔗 Dashboard: https://sonarqube.swthewhite.store/dashboard?id=api-core&branch=feat/UPLUS-121 Generated automatically by GitHub Actions. |
SonarQube Quality Summary (Community)✅ Quality Gate PASSED Branch: Issues
Measures
🔗 Dashboard: https://sonarqube.swthewhite.store/dashboard?id=api-core&branch=feat/UPLUS-121 Generated automatically by GitHub Actions. |
2 participants
🎫 지라 티켓
UPLUS-121
✅ 작업 사항
-크론식은 Cloud Scheduler 기준 UNIX 5필드 사용
📋 체크리스트
⌨ 기타