feat: 인챈트 정보 DB 테이블 생성 및 조회 API 구현#104
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.
Pull request overview
This pull request implements a new enchant information management feature for a Mabinogi game server. It creates database infrastructure to store enchant metadata extracted from auction history, and provides REST APIs for querying and synchronizing this data.
Changes:
- Added database table
enchant_infowith migrations to store enchant name, rank, affix position, and metadata - Implemented paginated query API and bulk fullname retrieval endpoint for enchant information
- Added admin-only sync endpoint that extracts enchant data from auction history using regex pattern matching
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| V23__create_enchant_info.sql | Creates the enchant_info table with columns for name, rank, position, and various metadata fields |
| V24__fix_enchant_rank_column_type.sql | Adjusts enchant_rank column type to match Hibernate validation requirements |
| V25__fix_enchant_full_option_rate_column_type.sql | Changes full_option_rate column type to INT to align with entity definition |
| EnchantInfoRepositoryPort.java | Defines repository interface for enchant data operations in domain layer |
| EnchantInfoEntity.java | JPA entity mapping for enchant_info table with all metadata fields |
| EnchantInfoJpaRepository.java | Spring Data JPA repository with custom native query for upserting from auction history |
| EnchantInfoRepositoryPortImpl.java | Implementation adapter connecting domain port to JPA repository |
| EnchantInfoService.java | Service layer implementing business logic for enchant queries and sync operation |
| EnchantInfoController.java | REST controller exposing three endpoints: paginated list, fullnames list, and admin sync |
| EnchantInfoPageRequestDto.java | Request DTO for pagination parameters with validation constraints |
| EnchantInfoResponse.java | Response DTO mapping entity data to API response format |
| EnchantInfoSyncResponse.java | Response DTO for sync operation returning count of upserted records |
Comments suppressed due to low confidence (4)
src/main/java/until/the/eternity/enchantinfo/infrastructure/persistence/EnchantInfoJpaRepository.java:25
- The ON DUPLICATE KEY UPDATE clause only updates the fullname field when a duplicate key is detected. However, if the source data (auction_history_item_option) has been updated with new values, the enchant_info table won't reflect these changes since name, enchant_rank, and affix_position (the unique key components) won't be updated. Consider whether all fields that could change should be updated in the ON DUPLICATE KEY UPDATE clause, or if this behavior is intentional.
ON DUPLICATE KEY UPDATE fullname = VALUES(fullname)
src/main/java/until/the/eternity/enchantinfo/infrastructure/persistence/EnchantInfoJpaRepository.java:11
- The JPQL query lacks pagination and could return a large result set if there are many enchant records. Since this is duplicating data that's already available through the paginated findAll endpoint, consider if this endpoint is necessary. If it is needed for a specific use case (e.g., dropdown population), consider documenting that use case and potentially adding a LIMIT clause or pagination to prevent performance issues.
@Query("SELECT e.fullname FROM EnchantInfoEntity e ORDER BY e.id ASC")
List<String> findAllFullnames();
src/main/java/until/the/eternity/enchantinfo/infrastructure/persistence/EnchantInfoEntity.java:47
- The entity class uses Boolean wrapper type for isExclusive and isCommonPart fields, which can be null. However, there's no explicit handling or documentation about what a null value means for these boolean flags. Consider either using primitive boolean type with a default value, or documenting the meaning of null vs true/false. Also consider adding @column(nullable = true) annotations to make the intent explicit in the entity definition.
@Column(name = "is_exclusive")
private Boolean isExclusive;
@Column(name = "is_common_part")
private Boolean isCommonPart;
src/main/java/until/the/eternity/enchantinfo/interfaces/rest/controller/EnchantInfoController.java:48
- The GET endpoint at /api/enchant-infos/fullnames returns a simple List<String> without pagination, which could potentially return a large amount of data if there are many enchant records. Consider either adding pagination support, implementing a maximum limit, or documenting the expected maximum size. If the number of enchants is guaranteed to be small (e.g., under 1000), this may be acceptable, but it should be documented.
@Operation(
summary = "모든 인챈트 fullname 조회",
description = "페이지네이션 없이 저장된 모든 인챈트의 fullname(이름 및 랭크)을 한 번에 조회합니다.")
@GetMapping("/fullnames")
public List<String> getAllEnchantFullnames() {
return enchantInfoService.findAllFullnames();
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| REGEXP_SUBSTR(REGEXP_SUBSTR(option_value, '랭크 [A-Za-z0-9]', 1, 1), '[A-Za-z0-9]+', 1, 1), | ||
| option_sub_type | ||
| FROM auction_history_item_option | ||
| WHERE option_type = '인챈트' |
There was a problem hiding this comment.
The REGEXP_SUBSTR function extracts the rank character but may return NULL if the pattern doesn't match. Since enchant_rank column is defined as NOT NULL in the table schema, this INSERT will fail for any option_value that doesn't contain a rank pattern. Consider adding a WHERE clause to filter out records without a valid rank pattern, or handle NULL values with COALESCE to provide a default value.
| WHERE option_type = '인챈트' | |
| WHERE option_type = '인챈트' | |
| AND REGEXP_SUBSTR(REGEXP_SUBSTR(option_value, '랭크 [A-Za-z0-9]', 1, 1), '[A-Za-z0-9]+', 1, 1) IS NOT NULL |
| id BIGINT AUTO_INCREMENT PRIMARY KEY, | ||
| fullname VARCHAR(100) NOT NULL COMMENT '인챈트 이름 및 랭크', | ||
| name VARCHAR(50) NOT NULL COMMENT '인챈트 이름', | ||
| enchant_rank VARCHAR(1) NOT NULL COMMENT '랭크', |
There was a problem hiding this comment.
The enchant_rank column is defined as VARCHAR(1), which can only store single-character ranks. If the game has multi-character ranks (e.g., "10", "SS"), this will cause data truncation errors. The REGEXP_SUBSTR pattern '[A-Za-z0-9]+' suggests that multi-character ranks are possible. Consider changing the column type to VARCHAR(2) or VARCHAR(10) to accommodate multi-character ranks, and verify what rank values actually exist in the source data.
| enchant_rank VARCHAR(1) NOT NULL COMMENT '랭크', | |
| enchant_rank VARCHAR(10) NOT NULL COMMENT '랭크', |
| @@ -0,0 +1,15 @@ | |||
| -- V21: 인챈트 정보 테이블 생성 | |||
There was a problem hiding this comment.
The comment incorrectly references "V21" but this is migration file V23. The comment should reference the correct version number.
| -- V21: 인챈트 정보 테이블 생성 | |
| -- V23: 인챈트 정보 테이블 생성 |
| package until.the.eternity.enchantinfo.application.service; | ||
|
|
||
| import java.util.List; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.data.domain.Page; | ||
| import org.springframework.data.domain.Pageable; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
| import until.the.eternity.enchantinfo.domain.repository.EnchantInfoRepositoryPort; | ||
| import until.the.eternity.enchantinfo.interfaces.rest.dto.response.EnchantInfoResponse; | ||
| import until.the.eternity.enchantinfo.interfaces.rest.dto.response.EnchantInfoSyncResponse; | ||
|
|
||
| @Service | ||
| @Transactional(readOnly = true) | ||
| @RequiredArgsConstructor | ||
| public class EnchantInfoService { | ||
|
|
||
| private final EnchantInfoRepositoryPort enchantInfoRepository; | ||
|
|
||
| public Page<EnchantInfoResponse> findAll(Pageable pageable) { | ||
| return enchantInfoRepository.findAll(pageable).map(EnchantInfoResponse::from); | ||
| } | ||
|
|
||
| public List<String> findAllFullnames() { | ||
| return enchantInfoRepository.findAllFullnames(); | ||
| } | ||
|
|
||
| @Transactional | ||
| public EnchantInfoSyncResponse syncFromAuctionHistory() { | ||
| int upserted = enchantInfoRepository.upsertFromAuctionHistory(); | ||
| return new EnchantInfoSyncResponse(upserted); | ||
| } | ||
| } |
There was a problem hiding this comment.
The EnchantInfoService class lacks unit tests, while similar service classes in the codebase (e.g., MetalwareInfoService) have comprehensive test coverage using Mockito. Consider adding unit tests to verify the behavior of findAll, findAllFullnames, and syncFromAuctionHistory methods, following the testing pattern established in the codebase.
| SELECT DISTINCT | ||
| option_value, | ||
| REGEXP_REPLACE(option_value, ' ?[(]랭크.*', '') AS name, | ||
| REGEXP_SUBSTR(REGEXP_SUBSTR(option_value, '랭크 [A-Za-z0-9]', 1, 1), '[A-Za-z0-9]+', 1, 1), |
There was a problem hiding this comment.
The REGEXP_SUBSTR pattern '랭크 [A-Za-z0-9]' has a space between '랭크' and the character class, which means it expects a space in the input string. If the source data format is inconsistent (e.g., "랭크A" without a space), the extraction will fail and return NULL, causing the insert to fail due to the NOT NULL constraint on enchant_rank. Consider making the space optional with a pattern like '랭크 ?[A-Za-z0-9]' or verifying that the source data always has the space.
| REGEXP_SUBSTR(REGEXP_SUBSTR(option_value, '랭크 [A-Za-z0-9]', 1, 1), '[A-Za-z0-9]+', 1, 1), | |
| REGEXP_SUBSTR(REGEXP_SUBSTR(option_value, '랭크 ?[A-Za-z0-9]', 1, 1), '[A-Za-z0-9]+', 1, 1), |
📋 상세 설명
enchant_info테이블 생상📊 체크리스트
이슈 미등록