feat: 무기 강화/합성/각성 API 및 RewardTransaction 보상 기록 추가 - #34
Conversation
Sean-mn
added
bug:버그
![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
This pull request implements weapon upgrade, synthesis, and awakening features, along with an append-only RewardTransaction audit log to track rewards and resource consumption across dungeons and weapon actions. It also introduces an xmin concurrency token to PlayerWeapon to prevent update conflicts. Feedback highlights several improvement opportunities: implementing local memory caching for static master data to avoid redundant Redis deserialization, batching database updates to prevent multiple SaveChangesAsync roundtrips within a single transaction, and handling unique constraint violations that can occur during concurrent synthesis inserts of new weapons.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| private async Task<List<WeaponEnhancementCost>> GetAllWeaponEnhancementCostsAsync() | ||
| { | ||
| var json = await _cache.GetStringAsync(WeaponEnhancementCostKey); | ||
| if (json is not null) | ||
| return JsonSerializer.Deserialize<List<WeaponEnhancementCost>>(json, CacheJsonOptions)!; | ||
|
|
||
| var data = await _repository.GetAllWeaponEnhancementCostsAsync(); | ||
| await _cache.SetStringAsync(WeaponEnhancementCostKey, JsonSerializer.Serialize(data, CacheJsonOptions), | ||
| new DistributedCacheEntryOptions { AbsoluteExpirationRelativeToNow = CacheTtl }); | ||
| return data; | ||
| } |
There was a problem hiding this comment.
[Efficiency / Performance]
GetAllWeaponEnhancementCostsAsync 및 GetAllWeaponAwakenCostsAsync 메서드는 무기 강화/각성 요청이 들어올 때마다 호출됩니다. 현재 구현에서는 매번 Redis(IDistributedCache)에서 전체 비용 목록 JSON을 가져와 역직렬화(JsonSerializer.Deserialize)하고 있습니다.
게임 마스터 데이터(기본 비용 테이블 등)는 정적이고 변경되지 않는 데이터임에도 불구하고, 매 요청마다 전체 목록을 Redis에서 조회하고 역직렬화하는 것은 다음과 같은 심각한 성능 문제를 야기할 수 있습니다:
- CPU 및 메모리 부하: 특히
UsePrivateSetters에서 리플렉션(SetValue)을 사용하여 매번 전체 목록의 객체들을 역직렬화하므로 CPU 사용량과 GC 할당량이 급증합니다. - Redis 네트워크 대역폭 낭비: 동시 접속자가 많을 때 매번 큰 JSON 데이터를 Redis로부터 전송받아야 하므로 네트워크 병목이 발생할 수 있습니다.
개선 제안:
정적 마스터 데이터는 서버 기동 시 또는 최초 조회 시 메모리(IMemoryCache 또는 단순 싱글톤 Dictionary/List 필드)에 캐싱(L1 캐시)하여 사용하고, Redis는 백업(L2 캐시) 용도로만 사용하거나 로컬 메모리 캐시를 우선 조회하도록 개선하는 것을 강력히 권장합니다.
There was a problem hiding this comment.
@gemini-code-assist 이번 PR이 추가한 2개 캐시 키(WeaponEnhancementCost, WeaponAwakenCost)는 기존 5개 캐시 키(LevelTable, WeaponData, SkillData, StageData, JobBaseStat)와 동일한 Redis-only 캐시 패턴을 그대로 재사용한 것으로, 이 PR이 새로 도입한 결함이 아니라 GameDataCacheService의 기존 아키텍처(flows.md Redis Cache Pattern)를 따른 것입니다. L1 메모리 캐시 도입은 7개 캐시 키 전체에 걸친 아키텍처 변경이라 이 PR 범위를 벗어나므로, 별도 개선 이슈로 분리해 논의하는 것을 제안합니다.
There was a problem hiding this comment.
Sean-mn 님, 의견 감사합니다. 말씀하신 대로 기존 아키텍처의 일관성을 유지하는 것이 중요하다는 점 충분히 이해했습니다. 현재 PR의 범위를 넘어서는 대규모 아키텍처 변경보다는, 제안하신 대로 L1 메모리 캐시 도입을 별도의 개선 이슈로 분리하여 논의하는 것이 타당해 보입니다. 해당 제안을 수용하여 이 건은 현재의 Redis-only 패턴을 따르는 것으로 동의합니다.
| await _transactionRunner.ExecuteAsync(async () => | ||
| { | ||
| await _playerResourceRepository.UpdateAsync(resource); | ||
| await _playerWeaponRepository.UpdateAsync(playerWeapon); | ||
| await _rewardTransactionRepository.SaveRangeAsync(rewardTransactions); | ||
| }); |
There was a problem hiding this comment.
[Efficiency / Design]
_transactionRunner.ExecuteAsync 내부에서 여러 리포지토리의 UpdateAsync 및 SaveRangeAsync 메서드를 순차적으로 호출하고 있습니다. 현재 각 리포지토리 구현(예: PlayerWeaponRepository.UpdateAsync, RewardTransactionRepository.SaveRangeAsync) 내부에서 개별적으로 _db.SaveChangesAsync()를 호출하고 있습니다.
이로 인해 하나의 트랜잭션 범위 내에서 **여러 번의 SaveChangesAsync() 호출과 데이터베이스 라운드트립(Roundtrip)**이 발생하게 됩니다. 이는 다음과 같은 문제를 일으킬 수 있습니다:
- 트랜잭션 유지 시간 증가: 불필요한 네트워크 라운드트립으로 인해 DB 트랜잭션이 잠금을 유지하는 시간이 길어져, 고부하 환경에서 데드락(Deadlock)이나 커넥션 풀 고갈이 발생할 위험이 높아집니다.
- 성능 저하: 여러 변경 사항을 한 번에 배치(Batch)로 처리하지 못하고 개별 쿼리로 나누어 실행하게 됩니다.
개선 제안:
리포지토리 메서드에서는 단순히 DbContext의 상태만 변경(Add, Update 등)하고, 실제 SaveChangesAsync()는 트랜잭션 러너의 실행 단위 마지막에 단 한 번만 호출되도록 Unit of Work 패턴을 적용하거나 리포지토리 설계를 개선하는 것이 좋습니다.
There was a problem hiding this comment.
@gemini-code-assist 리포지토리별 SaveChangesAsync 호출은 Account, Dungeon, Player 등 이 프로젝트의 모든 기존 도메인이 동일하게 사용하는 확립된 컨벤션입니다(domain-patterns.md §Repository). AppDbTransactionRunner가 명시적 DB 트랜잭션(BeginTransactionAsync)으로 감싸므로 여러 SaveChangesAsync 호출이 있어도 원자성은 보장되어 correctness 문제는 없습니다. Unit of Work 패턴 도입은 전체 리포지토리 계층을 바꾸는 대규모 리팩터링이라 이 PR 범위를 벗어나므로 반영하지 않습니다.
There was a problem hiding this comment.
Sean-mn 님, 의견 감사합니다.
제시해주신 프로젝트의 확립된 컨벤션(domain-patterns.md §Repository)과 트랜잭션 러너의 원자성 보장, 그리고 현재 PR의 범위에 대한 제약 사항을 충분히 이해했습니다. 기존 리포지토리 계층의 일관성을 유지하면서 원자성을 보장하는 현재의 설계가 프로젝트의 상황에 적합하다는 점을 확인했습니다.
제안했던 Unit of Work 패턴 도입은 향후 아키텍처 개선이 필요할 때 검토해주시면 될 것 같습니다. 답변해주셔서 감사합니다.
동일 결과 무기를 처음 합성하는 요청이 동시에 두 번 들어오면 두 요청 모두 PlayerWeapon.Create로 신규 삽입을 시도해 (PlayerId, WeaponId) 유니크 인덱스 위반이 발생한다. AppDbTransactionRunner가 DbUpdateConcurrencyException만 ConflictException으로 변환하고 있어 이 경우 예외가 그대로 전파되어 500으로 노출될 위험이 있었다. DbUpdateException 전반을 동일하게 409로 변환하도록 확장.
1 participant
📚작업 내용
무기 강화/합성/각성 API (Phase 3)
WeaponController추가 —POST /v1/weapons/{weaponId}/upgrade|synthesize|awakenUpgradeWeaponService/SynthesizeWeaponService/AwakenWeaponService구현 (비용 검증 → 재화/재료 차감 → 결과 반영 → 캐시 무효화)WeaponData에 강화/각성/합성 마스터 컬럼 추가,WeaponEnhancementCost/WeaponAwakenCost비용 마스터 테이블 및 시드 데이터 추가PlayerWeapon에xmin동시성 토큰과 단건 조회/갱신 리포지토리 메서드 추가 — 동시 강화·각성 요청 충돌 방지RewardTransaction 보상 기록 (Phase 4)
RewardTransaction엔티티/리포지토리 추가 — 재화·아이템 획득/소모 내역을 append-only로 남기는 감사 로그 (조회 API는 이번 범위 외)RewardTransaction으로 기록Amount == 0인 항목은 기록하지 않도록 가드 추가버그 수정
GrantWeaponsAsync)으로 전환 — 드랍 처리 중 동시 강화/각성이 끼어들 때 값이 덮어써지던 문제 방지기타
PlayerWeapon.Update제거AddWeaponMasterColumns,AddWeaponCostTables,AddPlayerWeaponConcurrencyToken,AddRewardTransactiondocs/TODO.md,docs/client-integration-guide.md(7장), 스펙 문서 §5.3 에러코드 매핑 정리Phase 3(무기 강화/합성/각성)과 Phase 4(RewardTransaction 보상 기록) 작업을 통합한 PR입니다(21 커밋).
신규 마이그레이션 4종이 포함되어 있어 배포 시 DB 마이그레이션 적용이 필요합니다.
RewardTransaction은 현재 기록 전용이며, 조회/집계 API는 이번 범위에 포함되지 않습니다.✅체크리스트