-
Notifications
You must be signed in to change notification settings - Fork 1
feat: 거대한 뿔피리 OPEN API 스케쥴러 구현 #90
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
Conversation
✅ 테스트 결과 for PRBuild: success 🧪 테스트 실행 with Gradle |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
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.
Pull request overview
This pull request adds a new horn bugle history feature that collects and stores "거대한 외침의 뿔피리" (Horn of Great Cry) data from the Nexon Open API. The feature includes a scheduled job that fetches data every 5 minutes across multiple game servers, implements duplicate detection, and provides REST APIs for querying the history. Additionally, the PR includes extensive import statement reordering across the entire codebase to follow Java conventions (java imports first, then third-party, then project imports).
Changes:
- New horn bugle feature with complete domain model, services, scheduler, and REST APIs
- Database migration adding server_name and date_register columns with indexes
- Import statement reorganization across 100+ files following Java conventions
- Code formatting improvements (line breaks in javadocs, method parameters)
Reviewed changes
Copilot reviewed 122 out of 122 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/until/the/eternity/hornBugle/** | Complete horn bugle feature implementation with domain, service, scheduler, and API layers |
| src/main/resources/db/migration/V14__*.sql | Database migration for horn_bugle_world_history table enhancements |
| src/main/resources/application.yml | Configuration for horn bugle scheduler (cron, retries, delays) |
| src/main/java/until/the/eternity/config/SecurityConfig.java | Added /horn-bugle/** endpoint to permitted paths |
| Multiple test and source files | Import statement reordering and code formatting improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| package until.the.eternity.hornBugle.application.service; | ||
|
|
||
| import java.time.Instant; | ||
| import java.util.List; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.springframework.data.domain.Page; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
| import until.the.eternity.common.response.PageResponseDto; | ||
| import until.the.eternity.hornBugle.domain.entity.HornBugleWorldHistory; | ||
| import until.the.eternity.hornBugle.domain.enums.HornBugleServer; | ||
| import until.the.eternity.hornBugle.domain.mapper.HornBugleMapper; | ||
| import until.the.eternity.hornBugle.domain.repository.HornBugleRepositoryPort; | ||
| import until.the.eternity.hornBugle.domain.service.HornBugleDuplicateChecker; | ||
| import until.the.eternity.hornBugle.interfaces.external.dto.OpenApiHornBugleHistoryResponse; | ||
| import until.the.eternity.hornBugle.interfaces.rest.dto.request.HornBuglePageRequestDto; | ||
| import until.the.eternity.hornBugle.interfaces.rest.dto.response.HornBugleHistoryResponse; | ||
|
|
||
| @Slf4j | ||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class HornBugleService { | ||
|
|
||
| private final HornBugleRepositoryPort repository; | ||
| private final HornBugleDuplicateChecker duplicateChecker; | ||
| private final HornBugleMapper mapper; | ||
|
|
||
| /** | ||
| * API 응답 데이터를 중복 제거 후 저장한다. | ||
| * | ||
| * @param server 서버 정보 | ||
| * @param responses API 응답 데이터 | ||
| * @return 저장된 건수 | ||
| */ | ||
| @Transactional | ||
| public int saveAll(HornBugleServer server, List<OpenApiHornBugleHistoryResponse> responses) { | ||
| if (responses == null || responses.isEmpty()) { | ||
| log.debug("[HornBugle] [{}] No data to save.", server.getServerName()); | ||
| return 0; | ||
| } | ||
|
|
||
| // 중복 제거 | ||
| List<OpenApiHornBugleHistoryResponse> filtered = | ||
| duplicateChecker.filterDuplicates(server, responses); | ||
|
|
||
| if (filtered.isEmpty()) { | ||
| log.debug( | ||
| "[HornBugle] [{}] All data is duplicated. Nothing to save.", | ||
| server.getServerName()); | ||
| return 0; | ||
| } | ||
|
|
||
| // Entity 변환 및 저장 | ||
| Instant registerTime = Instant.now(); | ||
| List<HornBugleWorldHistory> entities = | ||
| filtered.stream().map(dto -> mapper.toEntity(dto, server, registerTime)).toList(); | ||
|
|
||
| repository.saveAll(entities); | ||
|
|
||
| log.info("[HornBugle] [{}] Saved {} new records.", server.getServerName(), entities.size()); | ||
|
|
||
| return entities.size(); | ||
| } | ||
|
|
||
| /** | ||
| * 서버별 최신 N건 조회 (페이징) | ||
| * | ||
| * @param serverName 서버 이름 (선택 사항, null이면 전체 조회) | ||
| * @param pageRequest 페이지 요청 정보 | ||
| * @return 페이징 응답 | ||
| */ | ||
| @Transactional(readOnly = true) | ||
| public PageResponseDto<HornBugleHistoryResponse> search( | ||
| String serverName, HornBuglePageRequestDto pageRequest) { | ||
|
|
||
| Page<HornBugleWorldHistory> page; | ||
|
|
||
| if (serverName != null && !serverName.isBlank()) { | ||
| page = repository.findByServerName(serverName, pageRequest.toPageable()); | ||
| } else { | ||
| page = repository.findAll(pageRequest.toPageable()); | ||
| } | ||
|
|
||
| Page<HornBugleHistoryResponse> responsePage = page.map(mapper::toResponse); | ||
|
|
||
| return PageResponseDto.of(responsePage); | ||
| } | ||
| } |
Copilot
AI
Jan 21, 2026
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.
There is no test coverage for the new HornBugle feature. The repository contains comprehensive test coverage for similar features (e.g., AuctionHistoryServiceTest, ItemInfoServiceTest), but tests are missing for HornBugleService, HornBugleScheduler, HornBugleDuplicateChecker, and HornBugleController. Add test classes to maintain consistency with the project's testing standards.
| ALTER TABLE horn_bugle_world_history | ||
| ADD COLUMN server_name VARCHAR(20) NOT NULL DEFAULT '' COMMENT '서버 이름 (류트, 만돌린, 하프, 울프)'; |
Copilot
AI
Jan 21, 2026
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.
The migration adds columns with 'NOT NULL DEFAULT' constraints. The DEFAULT value for 'server_name' is an empty string '', which may not be semantically correct for existing data if the table already has records. If the table has existing data, consider a two-step migration: first add the column as nullable, update existing records with appropriate values, then add the NOT NULL constraint.
| maxRetries, | ||
| delay); | ||
|
|
||
| Thread.sleep(delay); |
Copilot
AI
Jan 21, 2026
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.
The method uses Thread.sleep() in a Spring-managed component which can block the thread pool. While acceptable for a scheduler with controlled concurrency, this pattern should be documented or replaced with more resilient retry mechanisms from libraries like Spring Retry or Resilience4j that provide better thread management and don't block threads unnecessarily.
| @GetMapping | ||
| @Operation(summary = "뿔피리 히스토리 조회", description = "거대한 외침의 뿔피리 내역을 조회합니다. 서버별 또는 전체 조회가 가능합니다.") | ||
| public ResponseEntity<PageResponseDto<HornBugleHistoryResponse>> search( | ||
| @Parameter(description = "서버 이름 (류트, 만돌린, 하프, 울프). 미입력시 전체 조회") | ||
| @RequestParam(required = false) | ||
| String serverName, | ||
| @ParameterObject @ModelAttribute @Valid HornBuglePageRequestDto pageRequest) { | ||
| PageResponseDto<HornBugleHistoryResponse> result = service.search(serverName, pageRequest); | ||
| return ResponseEntity.ok(result); | ||
| } |
Copilot
AI
Jan 21, 2026
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.
Missing validation for the 'serverName' parameter. The method accepts any string value but the system only supports specific servers (류트, 만돌린, 하프, 울프) as defined in HornBugleServer enum. Invalid server names will return empty results without informing the user of the error. Consider validating the server name against the enum values and returning a 400 Bad Request with a clear error message for invalid values.
| @@ -1,30 +1,41 @@ | |||
| package until.the.eternity.hornBugle.domain.entity; | |||
Copilot
AI
Jan 21, 2026
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.
The package name 'hornBugle' uses camelCase, which is inconsistent with Java package naming conventions. All other packages in this project use lowercase (e.g., 'auctionhistory', 'iteminfo', 'metalwareinfo'). The package should be renamed to 'hornbugle' to maintain consistency and follow Java conventions.
| @PostMapping("/batch") | ||
| @Operation(summary = "뿔피리 히스토리 배치 실행", description = "모든 서버의 거대한 외침의 뿔피리 내역을 수집하여 저장합니다.") | ||
| public ResponseEntity<Void> triggerBatch() { | ||
| scheduler.fetchAndSaveHornBugleHistoryAll(); | ||
| return ResponseEntity.ok().build(); | ||
| } |
Copilot
AI
Jan 21, 2026
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.
The scheduler method 'fetchAndSaveHornBugleHistoryAll' is directly exposed via a POST endpoint without any authentication, authorization, or rate limiting checks. This could allow unauthorized users to trigger expensive batch operations repeatedly. Consider adding proper security controls, such as requiring admin privileges or implementing rate limiting at the endpoint level.
| private String buildDuplicateKey(String characterName, String message) { | ||
| return characterName + "|" + message; | ||
| } |
Copilot
AI
Jan 21, 2026
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.
The duplicate detection key is constructed by concatenating characterName and message with a pipe separator. If either field contains the pipe character '|', this could lead to false duplicates or missed duplicates. Consider using a more robust approach such as creating a composite key object or using a delimiter that is guaranteed not to appear in the data.
| -- 2. date_register 컬럼 추가 (수집 시각) | ||
| ALTER TABLE horn_bugle_world_history | ||
| ADD COLUMN date_register DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP COMMENT '해당 뿔피리 내역을 수집한 시각'; |
Copilot
AI
Jan 21, 2026
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.
The DATETIME column type is used for 'date_register', but the entity uses java.time.Instant which typically maps to TIMESTAMP. This mismatch could lead to timezone-related issues or precision loss. Consider using TIMESTAMP in the migration to match the entity field type, or change the entity to use LocalDateTime if DATETIME is intentional.
| // 동일한 date_send를 가진 기존 데이터들의 (character_name + message) 조합을 조회 | ||
| Set<String> existingKeys = buildExistingKeysForDateSend(serverName, latestDateSend); |
Copilot
AI
Jan 21, 2026
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.
The duplicate checker performs a database query inside a transaction that may contain hundreds of entities to save. This could lead to performance issues as the 'findByServerNameAndDateSend' query is executed for every batch save operation. Consider caching this result or moving the duplicate check outside the transaction boundary to improve performance.
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.
Pull request overview
Copilot reviewed 37 out of 37 changed files in this pull request and generated 14 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Scheduled(cron = "${openapi.horn-bugle.cron:0 */5 * * * *}", zone = "Asia/Seoul") | ||
| public void fetchAndSaveHornBugleHistoryAll() { | ||
| log.info("[HornBugle] Starting Horn Bugle World History scheduler"); | ||
|
|
||
| HornBugleServer[] servers = HornBugleServer.values(); | ||
| int totalSavedCount = 0; | ||
| List<HornBugleServer> failedServers = new ArrayList<>(); | ||
|
|
||
| for (int i = 0; i < servers.length; i++) { | ||
| HornBugleServer server = servers[i]; | ||
| int savedCount = fetchAndSaveForServer(server); | ||
|
|
||
| if (savedCount < 0) { | ||
| failedServers.add(server); | ||
| } else { | ||
| totalSavedCount += savedCount; | ||
| } | ||
|
|
||
| // Rate Limit: 마지막 서버가 아니면 1초 대기 | ||
| if (i < servers.length - 1) { | ||
| waitForRateLimit(); | ||
| } | ||
| } | ||
|
|
||
| // 실패한 서버들 재시도 | ||
| if (!failedServers.isEmpty()) { | ||
| log.info( | ||
| "[HornBugle] Retrying {} failed servers: {}", | ||
| failedServers.size(), | ||
| failedServers); | ||
| totalSavedCount += retryFailedServers(failedServers); | ||
| } | ||
|
|
||
| log.info( | ||
| "[HornBugle] Horn Bugle World History scheduler completed. Total saved: {}", | ||
| totalSavedCount); | ||
| } |
Copilot
AI
Jan 21, 2026
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.
The scheduler method is not annotated with @transactional or @async, which means it runs on the default Spring scheduler thread pool. The method blocks with Thread.sleep() calls totaling potentially several seconds (4 servers * 1 second rate limit + retry delays). If other @scheduled methods exist with similar timing, they could be blocked. Consider using @async with a dedicated thread pool for this scheduler or refactoring to use non-blocking reactive patterns.
| private int fetchAndSaveForServer(HornBugleServer server) { | ||
| try { | ||
| OpenApiHornBugleHistoryListResponse response = | ||
| client.fetchHornBugleHistory(server).block(); |
Copilot
AI
Jan 21, 2026
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.
The reactive WebClient call is being blocked with .block(), which defeats the purpose of using reactive programming. This blocking call could lead to thread exhaustion under high load. Since the scheduler already uses blocking patterns (Thread.sleep), consider either fully committing to reactive patterns with proper error handling (returning Mono/Flux and using reactive operators) or using a blocking HTTP client (RestClient) for consistency and clarity.
| client.fetchHornBugleHistory(server).block(); | |
| client.fetchHornBugleHistory(server).toFuture().join(); |
| package until.the.eternity.hornBugle.application.scheduler; | ||
|
|
||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.springframework.beans.factory.annotation.Value; | ||
| import org.springframework.scheduling.annotation.Scheduled; | ||
| import org.springframework.stereotype.Component; | ||
| import until.the.eternity.hornBugle.application.service.HornBugleService; | ||
| import until.the.eternity.hornBugle.domain.enums.HornBugleServer; | ||
| import until.the.eternity.hornBugle.infrastructure.client.HornBugleClient; | ||
| import until.the.eternity.hornBugle.interfaces.external.dto.OpenApiHornBugleHistoryListResponse; | ||
| import until.the.eternity.hornBugle.interfaces.external.dto.OpenApiHornBugleHistoryResponse; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| @Slf4j | ||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class HornBugleScheduler { | ||
|
|
||
| private final HornBugleClient client; | ||
| private final HornBugleService service; | ||
|
|
||
| private static final long RATE_LIMIT_DELAY_MS = 1000L; | ||
|
|
||
| @Value("${openapi.horn-bugle.max-retries:3}") | ||
| private int maxRetries; | ||
|
|
||
| @Value("${openapi.horn-bugle.retry-delay-ms:2000}") | ||
| private long retryDelayMs; | ||
|
|
||
| @Scheduled(cron = "${openapi.horn-bugle.cron:0 */5 * * * *}", zone = "Asia/Seoul") | ||
| public void fetchAndSaveHornBugleHistoryAll() { | ||
| log.info("[HornBugle] Starting Horn Bugle World History scheduler"); | ||
|
|
||
| HornBugleServer[] servers = HornBugleServer.values(); | ||
| int totalSavedCount = 0; | ||
| List<HornBugleServer> failedServers = new ArrayList<>(); | ||
|
|
||
| for (int i = 0; i < servers.length; i++) { | ||
| HornBugleServer server = servers[i]; | ||
| int savedCount = fetchAndSaveForServer(server); | ||
|
|
||
| if (savedCount < 0) { | ||
| failedServers.add(server); | ||
| } else { | ||
| totalSavedCount += savedCount; | ||
| } | ||
|
|
||
| // Rate Limit: 마지막 서버가 아니면 1초 대기 | ||
| if (i < servers.length - 1) { | ||
| waitForRateLimit(); | ||
| } | ||
| } | ||
|
|
||
| // 실패한 서버들 재시도 | ||
| if (!failedServers.isEmpty()) { | ||
| log.info( | ||
| "[HornBugle] Retrying {} failed servers: {}", | ||
| failedServers.size(), | ||
| failedServers); | ||
| totalSavedCount += retryFailedServers(failedServers); | ||
| } | ||
|
|
||
| log.info( | ||
| "[HornBugle] Horn Bugle World History scheduler completed. Total saved: {}", | ||
| totalSavedCount); | ||
| } | ||
|
|
||
| /** | ||
| * 서버별 API 호출 및 저장 | ||
| * | ||
| * @param server 서버 | ||
| * @return 저장된 건수 (-1: 실패) | ||
| */ | ||
| private int fetchAndSaveForServer(HornBugleServer server) { | ||
| try { | ||
| OpenApiHornBugleHistoryListResponse response = | ||
| client.fetchHornBugleHistory(server).block(); | ||
|
|
||
| if (response == null || response.hornBugleWorldHistory() == null) { | ||
| log.warn("[HornBugle] [{}] Empty response from API", server.getServerName()); | ||
| return 0; | ||
| } | ||
|
|
||
| List<OpenApiHornBugleHistoryResponse> histories = response.hornBugleWorldHistory(); | ||
| int savedCount = service.saveAll(server, histories); | ||
|
|
||
| log.info( | ||
| "[HornBugle] [{}] Fetched {} records, saved {} new records", | ||
| server.getServerName(), | ||
| histories.size(), | ||
| savedCount); | ||
|
|
||
| return savedCount; | ||
| } catch (Exception e) { | ||
| log.error( | ||
| "[HornBugle] [{}] Failed to fetch and save: {}", | ||
| server.getServerName(), | ||
| e.getMessage(), | ||
| e); | ||
| return -1; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * 실패한 서버들 재시도 | ||
| * | ||
| * @param failedServers 실패한 서버 목록 | ||
| * @return 재시도로 저장된 총 건수 | ||
| */ | ||
| private int retryFailedServers(List<HornBugleServer> failedServers) { | ||
| int totalSavedCount = 0; | ||
|
|
||
| for (HornBugleServer server : failedServers) { | ||
| int savedCount = retryForServer(server); | ||
| if (savedCount >= 0) { | ||
| totalSavedCount += savedCount; | ||
| } | ||
| } | ||
|
|
||
| return totalSavedCount; | ||
| } | ||
|
|
||
| /** | ||
| * 단일 서버 재시도 (지수 백오프) | ||
| * | ||
| * @param server 서버 | ||
| * @return 저장된 건수 (-1: 최종 실패) | ||
| */ | ||
| private int retryForServer(HornBugleServer server) { | ||
| for (int attempt = 1; attempt <= maxRetries; attempt++) { | ||
| try { | ||
| long delay = retryDelayMs * (long) Math.pow(2, attempt - 1); | ||
| log.info( | ||
| "[HornBugle] [{}] Retry attempt {}/{}, waiting {}ms", | ||
| server.getServerName(), | ||
| attempt, | ||
| maxRetries, | ||
| delay); | ||
|
|
||
| Thread.sleep(delay); | ||
|
|
||
| int savedCount = fetchAndSaveForServer(server); | ||
| if (savedCount >= 0) { | ||
| log.info( | ||
| "[HornBugle] [{}] Retry successful on attempt {}", | ||
| server.getServerName(), | ||
| attempt); | ||
| return savedCount; | ||
| } | ||
| } catch (InterruptedException e) { | ||
| log.error("[HornBugle] [{}] Retry interrupted", server.getServerName(), e); | ||
| Thread.currentThread().interrupt(); | ||
| return -1; | ||
| } | ||
| } | ||
|
|
||
| log.error( | ||
| "[HornBugle] [{}] All {} retry attempts failed", | ||
| server.getServerName(), | ||
| maxRetries); | ||
| return -1; | ||
| } | ||
|
|
||
| private void waitForRateLimit() { | ||
| try { | ||
| log.debug("[HornBugle] Waiting {}ms for rate limit", RATE_LIMIT_DELAY_MS); | ||
| Thread.sleep(RATE_LIMIT_DELAY_MS); | ||
| } catch (InterruptedException e) { | ||
| log.error("[HornBugle] Rate limit wait interrupted", e); | ||
| Thread.currentThread().interrupt(); | ||
| } | ||
| } | ||
| } |
Copilot
AI
Jan 21, 2026
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.
The new HornBugle feature lacks test coverage. Given that the repository has comprehensive tests for other features (auctionhistory, iteminfo, etc.), tests should be added for the HornBugle functionality including scheduler logic, duplicate checking, service methods, controller endpoints, and repository operations.
| public Pageable toPageable() { | ||
| int resolvedPage = this.page != null ? this.page - 1 : DEFAULT_PAGE - 1; | ||
| int resolvedSize = this.size != null ? this.size : DEFAULT_SIZE; | ||
|
|
||
| return PageRequest.of( | ||
| resolvedPage, resolvedSize, Sort.by(Sort.Direction.DESC, SORT_BY_DATE_SEND)); | ||
| } |
Copilot
AI
Jan 21, 2026
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.
The toPageable() method converts a 1-based page number to a 0-based index by subtracting 1. However, if a user passes page=0 or a negative number, it will result in an invalid page index (-1 or less), which could cause unexpected behavior in Spring Data. The @min(1) validation should prevent this, but it would be safer to add defensive validation in the method itself or document the assumption clearly.
| @Mapping(target = "id", ignore = true) | ||
| @Mapping(target = "serverName", expression = "java(server.getServerName())") | ||
| @Mapping(target = "dateRegister", source = "registerTime") | ||
| @Mapping(target = "dateSend", expression = "java(dto.dateSend().plusSeconds(32400))") |
Copilot
AI
Jan 21, 2026
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.
The time zone conversion logic appears to hardcode adding 32400 seconds (9 hours) to convert from UTC to KST. This assumes the API always returns UTC timestamps, but the conversion should ideally be done using proper timezone APIs (e.g., ZonedDateTime) rather than manual offset addition. This approach is fragile because it doesn't account for daylight saving time changes (though Korea doesn't observe DST). Consider using dateSend().atZone(ZoneId.of("UTC")).withZoneSameInstant(ZoneId.of("Asia/Seoul")).toInstant() for clarity and maintainability.
| @PostMapping("/batch") | ||
| @Operation(summary = "뿔피리 히스토리 배치 실행", description = "모든 서버의 거대한 외침의 뿔피리 내역을 수집하여 저장합니다.") | ||
| public ResponseEntity<Void> triggerBatch() { | ||
| log.info("[HornBugle] Batch API triggered"); | ||
| try { | ||
| scheduler.fetchAndSaveHornBugleHistoryAll(); | ||
| log.info("[HornBugle] Batch API completed successfully"); | ||
| } catch (Exception e) { | ||
| log.error("[HornBugle] Batch API failed: {}", e.getMessage(), e); | ||
| throw e; | ||
| } | ||
| return ResponseEntity.ok().build(); | ||
| } |
Copilot
AI
Jan 21, 2026
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.
The batch endpoint at POST /horn-bugle/batch is publicly accessible without authentication (as configured in SecurityConfig), but it triggers a potentially expensive operation that collects data from external APIs for all servers. This could be abused to cause rate limit issues or excessive resource consumption. Consider adding authentication/authorization or rate limiting for this endpoint.
| @Parameter(description = "서버 이름 (류트, 만돌린, 하프, 울프). 미입력시 전체 조회") | ||
| @RequestParam(required = false) | ||
| String serverName, |
Copilot
AI
Jan 21, 2026
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.
The serverName parameter is accepted as a plain String without validation. Invalid server names should be rejected early with a clear error message. Consider validating against the HornBugleServer enum values or using @pattern validation to ensure only valid server names are accepted.
| private final HornBugleJpaRepository jpaRepository; | ||
| private final EntityManager em; | ||
|
|
||
| @Value("${spring.jpa.properties.hibernate.jdbc.batch_size:500}") |
Copilot
AI
Jan 21, 2026
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.
The batch size is hardcoded to default to 500 but the Hibernate batch_size configuration is set to 100 in application.yml. This mismatch could lead to confusion and suboptimal batch processing. The batch size should either use the same configuration property or be documented why a different value is appropriate for this specific use case.
| @Value("${spring.jpa.properties.hibernate.jdbc.batch_size:500}") | |
| @Value("${spring.jpa.properties.hibernate.jdbc.batch_size:100}") |
| @Table( | ||
| name = "horn_bugle_world_history", | ||
| indexes = { | ||
| @Index( | ||
| name = "idx_horn_bugle_server_date_send", | ||
| columnList = "server_name, date_send DESC"), | ||
| @Index(name = "idx_horn_bugle_date_send", columnList = "date_send DESC") | ||
| }) |
Copilot
AI
Jan 21, 2026
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.
The @Index annotations in the @table definition will be ignored by JPA at runtime. These annotations only affect schema generation during bootstrap. Since the project uses Flyway for database migrations (V14__add_server_name_and_date_register_to_horn_bugle_world_history.sql), these index definitions should be removed from the entity class to avoid confusion and maintain a single source of truth for schema changes in the migration files.
| .onErrorResume( | ||
| throwable -> { | ||
| log.error( | ||
| "[HornBugle] Failed to fetch Nexon Open API Horn Bugle History API for server='{}': error='{}', message='{}'", | ||
| server.getServerName(), | ||
| throwable.getClass().getSimpleName(), | ||
| throwable.getMessage(), | ||
| throwable); | ||
| return Mono.empty(); | ||
| }); |
Copilot
AI
Jan 21, 2026
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.
When the API call fails, the error handler returns Mono.empty(), which makes it impossible for the caller to distinguish between a successful empty response and a failed request. This could lead to silent failures being treated as "no data available". Consider using Mono.error() to propagate the error or return a Result/Either type that can represent both success and failure states explicitly.
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.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ALTER TABLE horn_bugle_world_history | ||
| ADD COLUMN server_name VARCHAR(20) NOT NULL DEFAULT '' COMMENT '서버 이름 (류트, 만돌린, 하프, 울프)'; | ||
|
|
Copilot
AI
Jan 23, 2026
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.
This migration adds server_name as NOT NULL DEFAULT ''. For any existing horn_bugle_world_history rows created before this change, server_name will become an empty string and will not match any real server name, which can break server-scoped queries/dedup logic. Consider backfilling existing rows to a valid server value (if known), making the column nullable, or cleaning up legacy data as part of the migration.
| Optional<Instant> latestDate = findLatestDateAuctionBuyBySubCategory(itemCategory); | ||
| if (latestDate.isEmpty()) { | ||
| return Optional.empty(); | ||
| } | ||
| List<String> ids = | ||
| jpaRepository.findAuctionBuyIdsByLatestDateAndSubCategory( | ||
| itemCategory.getTopCategory(), itemCategory.getSubCategory()); | ||
| return Optional.of(new LatestDateWithIds(latestDate.get(), new HashSet<>(ids))); |
Copilot
AI
Jan 23, 2026
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.
findLatestDateWithIdsBySubCategory issues two separate queries: one for latestDate and another that re-computes max(dateAuctionBuy) internally. If new rows are inserted between these calls, you can return a latestDate that doesn't match the returned ID set, breaking duplicate detection. Consider returning both latestDate and IDs from a single query/transaction (or derive latestDate from the same query used to fetch IDs).
| Optional<Instant> latestDate = findLatestDateAuctionBuyBySubCategory(itemCategory); | |
| if (latestDate.isEmpty()) { | |
| return Optional.empty(); | |
| } | |
| List<String> ids = | |
| jpaRepository.findAuctionBuyIdsByLatestDateAndSubCategory( | |
| itemCategory.getTopCategory(), itemCategory.getSubCategory()); | |
| return Optional.of(new LatestDateWithIds(latestDate.get(), new HashSet<>(ids))); | |
| String topCategory = itemCategory.getTopCategory(); | |
| String subCategory = itemCategory.getSubCategory(); | |
| List<Object[]> results = | |
| em.createQuery( | |
| "select a.dateAuctionBuy, a.id " + | |
| "from AuctionHistory a " + | |
| "where a.topCategory = :topCategory " + | |
| "and a.subCategory = :subCategory " + | |
| "and a.dateAuctionBuy = (" + | |
| " select max(b.dateAuctionBuy) " + | |
| " from AuctionHistory b " + | |
| " where b.topCategory = :topCategory " + | |
| " and b.subCategory = :subCategory" + | |
| ")", | |
| Object[].class) | |
| .setParameter("topCategory", topCategory) | |
| .setParameter("subCategory", subCategory) | |
| .getResultList(); | |
| if (results.isEmpty()) { | |
| return Optional.empty(); | |
| } | |
| Instant latestDate = (Instant) results.get(0)[0]; | |
| HashSet<String> ids = new HashSet<>(); | |
| for (Object[] row : results) { | |
| ids.add((String) row[1]); | |
| } | |
| return Optional.of(new LatestDateWithIds(latestDate, ids)); |
📋 상세 설명
📊 체크리스트
이슈 미등록