Skip to content

Conversation

@sjk4618
Copy link
Member

@sjk4618 sjk4618 commented Oct 31, 2025

🔥Pull requests

⛳️ 작업한 브랜치

👷 작업한 내용

  • admin에서 티켓 관련 수정할 때 redis 반영 구현

🚨 참고 사항

@sjk4618 sjk4618 self-assigned this Oct 31, 2025
@coderabbitai
Copy link

coderabbitai bot commented Oct 31, 2025

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

📥 Commits

Reviewing files that changed from the base of the PR and between 3bf24f6 and 014d8f1.

📒 Files selected for processing (1)
  • src/main/java/com/permitseoul/permitserver/global/redis/RedisTicketTypeCountInitializer.java (1 hunks)
 _________________________________________________
< Code Wars Episode VI: Return of the Unit Tests. >
 -------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ

Tip

CodeRabbit can generate a title for your PR based on the changes with custom instructions.

Add the reviews.auto_title_instructions setting in your project's settings in CodeRabbit to generate a title for your PR based on the changes in the PR with custom instructions.

📝 Walkthrough

Summary by CodeRabbit

릴리스 노트

  • 새로운 기능

    • Redis 기반 티켓 타입 카운트 동기화 기능 추가로 티켓 가용 수량 관리 성능 개선
    • 티켓 생성 및 업데이트 시 실시간 데이터 캐싱 지원
  • 개선 사항

    • 내부 트랜잭션 처리 강화
    • 에러 처리 및 로깅 체계 개선

Walkthrough

Redis 기반 티켓 타입 지속성 계층을 도입하여 어드민 티켓 서비스에 Redis 동기화 기능을 추가합니다. RedisManager 유틸리티, 초기화 컴포넌트, 새로운 에러 코드를 제공하고 기존 저장 메서드를 List 반환 방식으로 리팩토링합니다.

Changes

Cohort / File(s) 요약
Redis 기반 구조 설정
src/main/java/com/permitseoul/permitserver/global/redis/RedisManager.java, src/main/java/com/permitseoul/permitserver/global/redis/RedisProperties.java, src/main/java/com/permitseoul/permitserver/global/redis/RedisTicketTypeCountInitializer.java, src/main/java/com/permitseoul/permitserver/global/exception/RedisKeyNotFoundException.java, src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java, src/main/java/com/permitseoul/permitserver/global/config/RedisConfig.java
신규 RedisManager 서비스로 String 기반 Redis 연산(set, get, delete, increment, decrement, mSet 등) 제공. RedisProperties를 global.redis 패키지로 이동. RedisTicketTypeCountInitializer를 통한 애플리케이션 시작 시 티켓 타입 카운트 초기화. RedisKeyNotFoundException 예외 추가. 티켓 타입 Redis 관련 에러 코드 2개 신규 추가.
어드민 이벤트 서비스
src/main/java/com/permitseoul/permitserver/domain/admin/event/api/service/AdminEventService.java
AdminRedisTicketTypeSaver 의존성 추가. saveTicketTypes 반환 타입을 void에서 List<TicketType>으로 변경. 티켓 타입 생성 후 Redis 저장 로직 추가 및 에러 처리 추가.
어드민 티켓 서비스 및 DTO
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java, src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/dto/TicketTypeSplitResult.java
AdminRedisTicketTypeSaver, RedisManager, TicketTypeUpdater 의존성 추가. splitTicketTypeNewAndUpdate, syncRedisTicketCounts, rollbackRedisChanges 신규 헬퍼 메서드 추가. 신규 TicketTypeSplitResult DTO로 새로운/업데이트된 티켓 타입 분리. 티켓 라운드 생성/업데이트 시 Redis 동기화 적용.
티켓 타입 컴포넌트
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminRedisTicketTypeSaver.java, src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRetriever.java, src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeSaver.java
AdminRedisTicketTypeSaver 신규 컴포넌트 추가로 티켓 타입을 Redis에 저장. AdminTicketTypeRetriever에서 단일 ID 조회를 배치 ID 조회로 변경(getTicketTypeEntitiesByIds). AdminTicketTypeSaver.saveAllTicketTypes 반환 타입을 void에서 List<TicketType>으로 변경 및 @transactional 추가.
기타 리팩토링
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundUpdater.java, src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java, src/main/java/com/permitseoul/permitserver/domain/payment/api/service/PaymentService.java
AdminTicketRoundUpdater.updateTicketRound에 @transactional 어노테이션 추가. AuthService에서 공백 정리(기능 무변화). PaymentService의 주석 업데이트(기능 무변화).

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Admin as AdminEventService
    participant TicketSaver as AdminTicketTypeSaver
    participant Redis as AdminRedisTicketTypeSaver
    participant RedisDB as Redis
    
    Admin->>TicketSaver: saveTicketTypes(ticketTypes)
    TicketSaver->>TicketSaver: save to DB
    TicketSaver-->>Admin: List<TicketType>
    
    Admin->>Redis: saveTicketTypesInRedis(savedTicketTypes)
    Redis->>Redis: construct keys & values map
    Redis->>RedisDB: mSet(keyValues)
    RedisDB-->>Redis: success
    Redis-->>Admin: completed
    
    Note over Admin,RedisDB: On Redis error: throw INTERNAL_TICKET_TYPE_REDIS_ERROR
Loading
sequenceDiagram
    autonumber
    participant Ticket as AdminTicketService
    participant Splitter as splitTicketTypeNewAndUpdate
    participant Saver as AdminTicketTypeSaver
    participant Redis as syncRedisTicketCounts
    participant RedisDB as Redis
    
    Ticket->>Ticket: updateTicketRoundWithType(updateRequest)
    Ticket->>Splitter: split existing/new types
    Splitter-->>Ticket: TicketTypeSplitResult
    
    Ticket->>Saver: saveAllTicketTypes(newTicketTypes)
    Saver-->>Ticket: List<TicketType>
    
    Ticket->>Redis: syncRedisTicketCounts(splitResult)
    Redis->>RedisDB: increment/decrement counters
    RedisDB-->>Redis: success
    Redis-->>Ticket: completed
    
    alt Sync fails
        Redis->>Redis: rollbackRedisChanges
        Redis->>RedisDB: delete keys
        Redis-->>Ticket: throw exception
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25-30 minutes

주의 깊게 검토할 영역:

  • AdminTicketService의 Redis 트랜잭션 롤백 로직: syncRedisTicketCounts 및 rollbackRedisChanges 메서드의 원자성 및 예외 처리 일관성 검증 필요
  • AdminTicketTypeRetriever 배치 조회 검증: getTicketTypeEntitiesByIds의 ID 일치 검증 로직이 모든 호출 사이트에서 올바르게 동작하는지 확인
  • RedisTicketTypeCountInitializer 초기화 순서: 애플리케이션 시작 시 TicketTypeRepository 로드 및 Redis 저장의 순서 및 실패 시 롤백 동작 확인
  • 에러 코드 매핑: 새로운 에러 코드(INTERNAL_TICKET_TYPE_REDIS_ERROR, INTERNAL_TICKET_TYPE_NOT_FOUND_REDIS_ERROR)가 모든 Redis 연산 실패 경로에서 적절히 사용되는지 확인
  • TicketTypeSplitResult의 일관성: 새/기존 타입 분리 및 diff 계산 로직의 정확성

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning 대부분의 변경 사항은 이슈 #150의 Redis 적용 목표와 관련이 있지만, 두 개의 변경 사항이 범위를 벗어나 있습니다. AuthService.java의 메서드 호출에서 공백 제거는 Redis 티켓 로직과 무관한 코드 스타일 수정이고, PaymentService.java의 catch 블록 내 주석 업데이트도 이 이슈와 무관합니다. 이들은 의도치 않게 포함된 것으로 보이는 부수적 변경 사항입니다.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed 풀 리퀘스트 제목 "feat: admin 티켓 로직들 redis 반영 - #150"은 변경 사항의 주요 목표를 명확하게 설명하고 있습니다. 제목은 간결하고 명확하며, admin 영역에서 티켓 관련 로직에 Redis를 적용한다는 핵심 변경 사항을 효과적으로 전달합니다. 제목은 구체적이고 의미 있는 정보를 담고 있으며, 팀 동료가 커밋 히스토리를 훑어볼 때 주요 변경 사항을 쉽게 파악할 수 있습니다.
Linked Issues Check ✅ Passed 이슈 #150 "feat: admin 티켓 로직들 redis 반영"의 목표는 admin 티켓 관련 로직에 Redis 업데이트를 적용하는 것입니다. 이 풀 리퀘스트는 AdminRedisTicketTypeSaver, RedisManager, RedisTicketTypeCountInitializer, 그리고 AdminEventService/AdminTicketService의 수정을 통해 Redis 기반 티켓 타입 지속성 및 동기화를 구현하고 있습니다. AdminTicketTypeSaver의 반환 타입 변경, 새로운 TicketTypeSplitResult DTO 추가, Redis 연동을 위한 에러 코드 추가 등이 모두 이슈의 목표와 일치합니다.
Description Check ✅ Passed 풀 리퀨스트 설명 "admin에서 티켓 관련 수정할 때 redis 반영 구현"은 변경 사항과 관련이 있습니다. 설명이 간단하고 상세도가 낮지만, 변경 사항의 핵심(admin 티켓 로직에 Redis 적용)을 설명하고 있으며 완전히 주제를 벗어나지 않습니다. 평가 기준에 따르면 설명이 어떤 방식이든 변경 사항과 관련된 것이면 통과하며, 상세도 수준은 중요하지 않습니다.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (4)
src/main/java/com/permitseoul/permitserver/domain/payment/api/service/PaymentService.java (1)

195-201: LGTM! TODO 주석이 중요한 모니터링 요구사항을 명확히 표시합니다.

결제는 성공했지만 티켓 발급이 실패하는 중요한 시나리오에 대한 TODO 주석이 적절하게 추가되었습니다. 이러한 케이스는 고객 불만과 수익 손실로 이어질 수 있으므로 알림 시스템 구축이 필요합니다.

이 TODO들을 추적하기 위한 별도 이슈를 생성하거나, 알림 시스템 구현을 위한 코드 작성을 도와드릴까요?

src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminRedisTicketTypeSaver.java (1)

24-26: 로그 메시지 개선을 권장합니다.

현재 로그 메시지가 [Redis Init]로 시작하는데, 이 컴포넌트는 초기화뿐만 아니라 관리자의 수정 시에도 호출될 수 있습니다. 호출 컨텍스트를 구분할 수 있도록 로그 메시지를 개선하거나 로그 레벨을 debug로 낮추는 것을 고려하세요.

src/main/java/com/permitseoul/permitserver/global/exception/RedisKeyNotFoundException.java (1)

1-4: 예외 클래스에 생성자 추가를 고려하세요.

예외 클래스가 비어있어 에러 메시지나 원인을 전달할 수 없습니다. 디버깅과 에러 추적을 위해 메시지를 받는 생성자를 추가하는 것이 좋습니다.

다음과 같이 생성자를 추가할 수 있습니다:

 public class RedisKeyNotFoundException extends PermitGlobalException {
+    public RedisKeyNotFoundException(String message) {
+        super(message);
+    }
+    
+    public RedisKeyNotFoundException(String message, Throwable cause) {
+        super(message, cause);
+    }
 }
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/dto/TicketTypeSplitResult.java (1)

13-19: 팩토리 메서드가 불필요할 수 있습니다.

of 메서드가 단순히 생성자를 호출하기만 합니다. Record는 이미 public 생성자를 제공하므로, 추가적인 검증이나 로직이 없다면 팩토리 메서드는 중복입니다. 특별한 이유가 없다면 제거하거나, 향후 확장을 위한 것이라면 주석으로 의도를 명시하세요.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 10a28c3 and f695242.

📒 Files selected for processing (16)
  • src/main/java/com/permitseoul/permitserver/domain/admin/event/api/service/AdminEventService.java (5 hunks)
  • src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/dto/TicketTypeSplitResult.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java (7 hunks)
  • src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundUpdater.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminRedisTicketTypeSaver.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRetriever.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeSaver.java (2 hunks)
  • src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/domain/payment/api/service/PaymentService.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/domain/ticket/api/service/TicketService.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/global/config/RedisConfig.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/global/exception/RedisKeyNotFoundException.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/global/redis/RedisManager.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/global/redis/RedisProperties.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/global/redis/RedisTicketTypeCountInitializer.java (2 hunks)
  • src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sjk4618
Repo: PERMIT-SEOUL/permit-server PR: 48
File: src/main/java/com/permitseoul/permitserver/domain/guest/core/domain/entity/GuestEntity.java:31-37
Timestamp: 2025-07-15T09:37:32.765Z
Learning: sjk4618 prefers to implement factory methods or public constructors for entities when they are actually needed, rather than creating them proactively.
🧬 Code graph analysis (6)
src/main/java/com/permitseoul/permitserver/global/redis/RedisManager.java (2)
src/main/java/com/permitseoul/permitserver/global/exception/RedisKeyNotFoundException.java (1)
  • RedisKeyNotFoundException (3-4)
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java (1)
  • Service (43-378)
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundUpdater.java (4)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeSaver.java (1)
  • Component (14-25)
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundRetriever.java (1)
  • Component (12-35)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeUpdater.java (1)
  • Component (9-20)
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundSaver.java (1)
  • Component (11-24)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminRedisTicketTypeSaver.java (1)
src/main/java/com/permitseoul/permitserver/global/Constants.java (1)
  • Constants (3-24)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeSaver.java (3)
src/main/java/com/permitseoul/permitserver/global/redis/RedisTicketTypeCountInitializer.java (1)
  • Component (17-39)
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundUpdater.java (1)
  • Component (9-19)
src/main/java/com/permitseoul/permitserver/domain/admin/eventimage/core/component/AdminEventImageSaver.java (1)
  • Component (10-18)
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java (2)
src/main/java/com/permitseoul/permitserver/global/Constants.java (1)
  • Constants (3-24)
src/main/java/com/permitseoul/permitserver/global/redis/RedisManager.java (1)
  • Service (12-63)
src/main/java/com/permitseoul/permitserver/global/redis/RedisTicketTypeCountInitializer.java (1)
src/main/java/com/permitseoul/permitserver/global/Constants.java (1)
  • Constants (3-24)
🔇 Additional comments (6)
src/main/java/com/permitseoul/permitserver/domain/auth/api/service/AuthService.java (1)

182-184: 파일 범위 확인 필요: PR 목표와의 정렬성

이 파일(AuthService.java)은 인증 도메인의 리프레시 토큰 관련 서비스로, 현재 PR 목표인 "admin 티켓 로직 Redis 반영"과 직접적인 관련이 없어 보입니다. 183번 줄의 변경사항은 순수 포매팅(공백 제거)으로 기능상 변화가 없습니다.

  • 이 파일의 변경이 현재 PR 범위에 의도적으로 포함된 것인지 확인이 필요합니다. 혹은 더 넓은 Redis 리팩토링 일환으로 별도의 맥락에서 수행되었을 수 있습니다.
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundUpdater.java (2)

5-5: LGTM!

@Transactional 어노테이션 사용을 위한 적절한 import 추가입니다.


12-18: 컴포넌트 계층에 @transactional 추가는 Spring 아키텍처 패턴을 위반합니다.

@transactional 어노테이션은 Service 계층에 적용되어야 하며, 트랜잭션 경계를 정의하는 것은 Service 계층의 책임입니다. @service 어노테이션은 비즈니스 로직을 포함하는 클래스가 Service 계층에 속함을 나타내며, 비즈니스 기능을 제공하는 클래스에 사용됩니다.

AdminTicketRoundUpdater는 @component로 표시된 저수준 인프라 클래스이므로, @transactional을 여기에 추가하는 것보다 이 컴포넌트를 호출하는 Service 계층에 @transactional을 적용해야 합니다.

  • 현재 이 변경을 되돌리고, AdminTicketRoundUpdater를 호출하는 service 클래스에서 @transactional을 관리하도록 리팩토링하세요.
  • 전체 Updater 컴포넌트들에서 동일한 패턴을 제거하고 Service 계층으로 트랜잭션 관리를 이동하세요.

Likely an incorrect or invalid review comment.

src/main/java/com/permitseoul/permitserver/global/redis/RedisProperties.java (1)

1-11: 패키지 이동이 적절합니다.

Redis 관련 컴포넌트들을 global.redis 패키지로 그룹화하는 것은 좋은 구조 개선입니다.

src/main/java/com/permitseoul/permitserver/global/config/RedisConfig.java (1)

3-3: Import 경로 업데이트가 올바릅니다.

RedisProperties의 패키지 이동에 맞춰 import가 정확하게 업데이트되었습니다.

src/main/java/com/permitseoul/permitserver/global/redis/RedisTicketTypeCountInitializer.java (1)

22-36: 개선된 초기화 로직입니다.

RedisManager.setIfAbsent를 사용하여 멱등성을 보장하고, 조건부 로깅으로 Redis 키의 생성/유지 여부를 명확히 구분한 것은 좋은 개선입니다.

Comment on lines +203 to +214
if (newRemain <= 0) {
redisManager.set(key, "0", null);
log.warn("[Redis] ticketType 품절 key={}, ",key);
} else {
log.info("[Redis] 기존 ticketType update complete, key={}, diff={}, remain={}", key, diff, newRemain);
}
}
} catch (Exception e) {
log.error("[Redis] 오류 발생 → Redis 롤백 수행 시작", e);
rollbackRedisChanges(ticketTypeSplitResult, existRedisTicketType, newRedisTicketTypeKeys);
throw new AdminApiException(ErrorCode.INTERNAL_TICKET_TYPE_REDIS_ERROR);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

롤백 시 Redis 잔여 수량 복구가 잘못됩니다

Line 204에서 남은 수량이 0 이하일 때 값을 강제로 "0"으로 덮어쓴 뒤, Line 229에서 diff 기반 increment로만 복구를 시도하면 롤백 시점의 Redis 값이 원래 remain보다 크게 부풀어집니다(예: 기존 remain=3, diff=-5 → 실패 후 롤백하면 0에서 +5로 복구되어 remain=5). 트랜잭션 자체는 롤백되지만 Redis 잔여 수량이 실제보다 커져 과판매 위험이 생깁니다. existRedisTicketType에 저장해 둔 이전 값을 그대로 복구하도록 수정해야 합니다.

-                redisManager.increment(key, -diff);
-                log.info("[RedisRollback] 기존 key diff 기반 복구 key={}, rollbackDiff={}", key, -diff);
+                final String previousValue = successExistRedisTicketType.get(key);
+                if (previousValue != null) {
+                    redisManager.set(key, previousValue, null);
+                    log.info("[RedisRollback] 기존 key 값 복구 key={}, remain={}", key, previousValue);
+                } else {
+                    redisManager.delete(key);
+                    log.info("[RedisRollback] 기존 key 삭제 key={}", key);
+                }

Also applies to: 228-233

🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java
around lines 203-214 (and similarly 228-233), the rollback logic incorrectly
restores Redis counts by applying diff-based increments to keys that may have
been overwritten to "0", causing inflated remain values; instead, restore each
key to the exact previous value (and TTL) recorded in existRedisTicketType and
remove any keys that were newly created (newRedisTicketTypeKeys). Update
rollbackRedisChanges to, for each entry in existRedisTicketType, call
redisManager.set(key, previousValue, previousTtl) (or the equivalent API) rather
than incrementing by diff, and for keys in newRedisTicketTypeKeys call
redisManager.delete(key); ensure null/absent TTL handling matches original state
and adjust any callers to pass the saved TTLs.

Comment on lines 18 to 28
public void saveTicketTypesInRedis(final List<TicketType> ticketTypes) {
ticketTypes.forEach(ticketType -> {
final String key = Constants.REDIS_TICKET_TYPE_KEY_NAME
+ ticketType.getTicketTypeId()
+ Constants.REDIS_TICKET_TYPE_REMAIN;
redisManager.set(key, String.valueOf(ticketType.getTotalTicketCount()), null);
log.info("[Redis Init] ticketTypeId={} totalCount={}",
ticketType.getTicketTypeId(),
ticketType.getTotalTicketCount());
});
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Redis 저장 시 에러 처리를 추가하세요.

redisManager.set() 호출이 실패할 경우에 대한 예외 처리가 없습니다. Redis 통신 실패 시 일부 티켓 타입만 저장되고 나머지는 누락될 수 있어 데이터 일관성 문제가 발생할 수 있습니다.

다음 스크립트로 RedisManager의 에러 처리 방식을 확인하세요:


🏁 Script executed:

#!/bin/bash
# RedisManager.set 메서드의 예외 처리 확인
ast-grep --pattern $'class RedisManager {
  $$$
  set($$$) {
    $$$
  }
  $$$
}'

Length of output: 0


필요한 정보를 수집하기 위해 추가 검증을 진행하겠습니다.


---

🏁 Script executed:

```shell
#!/bin/bash
# RedisManager 클래스 위치 찾기
fd RedisManager --type f

Length of output: 73


🏁 Script executed:

#!/bin/bash
# RedisManager.set() 메서드 정의 검색
rg -nA10 'public.*set\(' --type=java | grep -A10 RedisManager

Length of output: 1231


🏁 Script executed:

#!/bin/bash
# AdminRedisTicketTypeSaver에서 redisManager 사용 패턴 확인
rg -nB5 -A5 'redisManager\.set' --type=java

Length of output: 7051


🏁 Script executed:

#!/bin/bash
# 다른 코드에서 redisManager.set() 호출 시 에러 처리 패턴 확인
rg -nB3 -A3 'try.*redisManager|redisManager.*catch' --type=java

Length of output: 0


🏁 Script executed:

#!/bin/bash
# RedisManager 인터페이스 또는 클래스 검색
rg -n 'class RedisManager|interface RedisManager' --type=java -A20

Length of output: 2386


추가 검증을 진행하겠습니다.


---

🏁 Script executed:

```shell
#!/bin/bash
# AdminRedisTicketTypeSaver 전체 메서드와 클래스 레벨 어노테이션 확인
rg -nB10 'saveTicketTypesInRedis' src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminRedisTicketTypeSaver.java

Length of output: 311


🏁 Script executed:

#!/bin/bash
# saveTicketTypesInRedis 메서드를 호출하는 곳 찾기
rg -nB3 -A3 'saveTicketTypesInRedis' --type=java

Length of output: 3054


🏁 Script executed:

#!/bin/bash
# RedisManager.set() 호출 시 Spring 예외 처리 확인 (StringRedisTemplate 사용)
rg -n 'throws|RuntimeException|RedisConnectionFailure' src/main/java/com/permitseoul/permitserver/global/redis/RedisManager.java

Length of output: 0


🏁 Script executed:

#!/bin/bash
# 전체 RedisManager.java 파일 확인
cat src/main/java/com/permitseoul/permitserver/global/redis/RedisManager.java

Length of output: 2007


Redis 저장 중 부분 실패로 인한 데이터 일관성 문제 - 내부 에러 처리 필수

검증 결과, 리뷰 의견이 정당합니다. 현재 saveTicketTypesInRedis() 메서드는 forEach 루프에서 각 항목을 순차적으로 Redis에 저장하지만 개별 항목 저장 실패에 대한 예외 처리가 없습니다.

발견된 문제:

  • RedisManager.set()은 Spring의 StringRedisTemplate 호출 시 예외 처리 없음
  • N번째 항목 저장 실패 시, 1~N-1번째 항목은 이미 Redis에 저장된 상태
  • 호출자(AdminTicketService, AdminEventService)의 try-catch는 예외 발생 후에 실행되므로 이미 저장된 부분 데이터를 복구할 수 없음
  • 결과적으로 Redis에 불완전한 상태의 데이터가 남게 됨

해결 방안: forEach 루프 내부에 try-catch를 추가하여 개별 항목 저장 실패를 처리하거나, 실패한 항목을 수집하여 로깅/재시도 메커니즘을 구현해야 합니다.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
src/main/java/com/permitseoul/permitserver/global/redis/RedisManager.java (1)

73-78: 빈 맵에 대한 반환값이 혼란을 줄 수 있습니다.

Line 75에서 keyValues.isEmpty()일 때 true를 반환하는데, 이는 "아무것도 설정하지 않은 것을 성공"으로 간주합니다.

호출하는 측에서는 true가 "모든 키가 성공적으로 설정됨"을 의미한다고 오해할 수 있습니다. 특히 빈 맵이 버그로 인한 것이라면 조용히 성공 처리되어 문제를 발견하기 어렵습니다.

다음과 같은 대안을 고려하세요:

옵션 1: 빈 맵을 예외로 처리

 public boolean mSetIfAbsent(final Map<String, String> keyValues) {
     Objects.requireNonNull(keyValues, "keyValues must not be null");
-    if (keyValues.isEmpty()) return true;
+    if (keyValues.isEmpty()) {
+        throw new IllegalArgumentException("keyValues must not be empty");
+    }
     final Boolean ok = redisTemplate.opsForValue().multiSetIfAbsent(keyValues);
     return Boolean.TRUE.equals(ok);
 }

옵션 2: false 반환 (설정된 것이 없음을 명시)

-    if (keyValues.isEmpty()) return true;
+    if (keyValues.isEmpty()) return false;

옵션 3: 현재 상태 유지하되 JavaDoc으로 명확히

/**
 * @return true if all keys were set, or if keyValues is empty
 */
public boolean mSetIfAbsent(final Map<String, String> keyValues) {
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java (2)

122-142: Redis 저장 실패 시 DB 트랜잭션 롤백을 고려하세요.

현재 구현에서는 Redis 저장이 실패해도 DB 트랜잭션은 이미 커밋됩니다:

  1. Lines 129-134: DB에 티켓 차수와 타입 저장 (트랜잭션 내)
  2. Lines 137-141: Redis에 저장 시도 (트랜잭션 내)
  3. Redis 실패 시: AdminApiException 발생 → DB 롤백됨 (실제로는 롤백되지 않는 버그)

실제로는 @Transactional 어노테이션이 메서드에 있으므로, Redis 저장 실패 시 예외가 발생하면 DB도 롤백됩니다. 하지만 코드 가독성을 위해 명시적으로 DB 작업과 Redis 작업을 분리하는 것이 좋습니다.

다음과 같이 명시적으로 구분하세요:

 @Transactional
 public void createTicketRoundWithType(final long eventId,
                                       final String ticketRoundName,
                                       final LocalDateTime roundSalesStartDate,
                                       final LocalDateTime roundSalesEndDate,
                                       final List<TicketRoundWithTypeCreateRequest.TicketTypeRequest> ticketTypeRequests) {
     final List<TicketType> savedTicketTypes;
     try {
         final TicketRound savedTicketRound = adminTicketRoundSaver.saveTicketRound(eventId, ticketRoundName, roundSalesStartDate, roundSalesEndDate);
         savedTicketTypes = saveTicketTypes(ticketTypeRequests, savedTicketRound.getTicketRoundId());
-    } catch (TicketRoundIllegalArgumentException | TicketTypeIllegalException e) {
-        throw new AdminApiException(ErrorCode.BAD_REQUEST_DATE_TIME_ERROR);
-    }
-
-    //redis 등록
-    try {
+        // DB 커밋 전 Redis 등록
         adminRedisTicketTypeSaver.saveTicketTypesInRedis(savedTicketTypes);
     } catch (Exception e) {
-        throw new AdminApiException(ErrorCode.INTERNAL_TICKET_TYPE_REDIS_ERROR);
+        if (e instanceof TicketRoundIllegalArgumentException || e instanceof TicketTypeIllegalException) {
+            throw new AdminApiException(ErrorCode.BAD_REQUEST_DATE_TIME_ERROR);
+        }
+        log.error("[Redis] 티켓 타입 Redis 저장 실패", e);
+        throw new AdminApiException(ErrorCode.INTERNAL_TICKET_TYPE_REDIS_ERROR);
     }
 }

이렇게 하면 Redis 저장 실패 시 전체 트랜잭션이 롤백되어 DB와 Redis 간의 불일치를 방지할 수 있습니다.


248-305: 복잡한 로직을 더 작은 메서드로 분리하는 것을 고려하세요.

현재 메서드는 58줄로 다음 책임을 모두 수행합니다:

  1. 기존 티켓 타입 ID 추출 및 로드
  2. 신규 vs 기존 티켓 타입 분류
  3. 각 타입별 검증 및 업데이트
  4. diff 계산

로직은 올바르지만, 가독성과 유지보수성 향상을 위해 작은 메서드로 분리하는 것을 권장합니다.

리팩토링 제안:

private TicketTypeSplitResult splitTicketTypeNewAndUpdate(
        final TicketRoundEntity ticketRoundEntity,
        final List<TicketRoundWithTypeUpdateRequest.TicketTypeUpdateRequest> ticketTypeRequests
) {
    final Map<Long, TicketTypeEntity> existingEntitiesMap = loadExistingTicketTypeEntities(ticketTypeRequests);
    
    final List<TicketTypeEntity> newTicketTypeEntities = new ArrayList<>();
    final List<TicketTypeEntity> updatedTicketTypeEntities = new ArrayList<>();
    final Map<Long, Integer> updateDiffMap = new HashMap<>();

    for (TicketRoundWithTypeUpdateRequest.TicketTypeUpdateRequest ticketTypeReq : ticketTypeRequests) {
        if (ticketTypeReq.id() == null) {
            newTicketTypeEntities.add(createNewTicketType(ticketRoundEntity, ticketTypeReq));
        } else {
            processExistingTicketType(
                ticketRoundEntity, 
                ticketTypeReq, 
                existingEntitiesMap, 
                updatedTicketTypeEntities, 
                updateDiffMap
            );
        }
    }

    return TicketTypeSplitResult.of(newTicketTypeEntities, updatedTicketTypeEntities, updateDiffMap);
}

private Map<Long, TicketTypeEntity> loadExistingTicketTypeEntities(
        final List<TicketRoundWithTypeUpdateRequest.TicketTypeUpdateRequest> ticketTypeRequests
) {
    final List<Long> existingIds = ticketTypeRequests.stream()
            .map(TicketRoundWithTypeUpdateRequest.TicketTypeUpdateRequest::id)
            .filter(Objects::nonNull)
            .toList();
    
    return existingIds.isEmpty()
            ? Map.of()
            : adminTicketTypeRetriever.getTicketTypeEntitiesByIds(existingIds).stream()
                .collect(Collectors.toMap(TicketTypeEntity::getTicketTypeId, e -> e));
}

private TicketTypeEntity createNewTicketType(
        final TicketRoundEntity ticketRoundEntity,
        final TicketRoundWithTypeUpdateRequest.TicketTypeUpdateRequest request
) {
    return TicketTypeEntity.create(
            ticketRoundEntity.getTicketRoundId(),
            request.name(),
            request.price(),
            request.totalCount(),
            request.startDate(),
            request.endDate()
    );
}

private void processExistingTicketType(
        final TicketRoundEntity ticketRoundEntity,
        final TicketRoundWithTypeUpdateRequest.TicketTypeUpdateRequest request,
        final Map<Long, TicketTypeEntity> existingEntitiesMap,
        final List<TicketTypeEntity> updatedEntities,
        final Map<Long, Integer> updateDiffMap
) {
    final TicketTypeEntity existingEntity = existingEntitiesMap.get(request.id());
    if (existingEntity == null) {
        throw new AdminApiException(ErrorCode.NOT_FOUND_TICKET_TYPE);
    }
    if (!Objects.equals(existingEntity.getTicketRoundId(), ticketRoundEntity.getTicketRoundId())) {
        throw new AdminApiException(ErrorCode.BAD_REQUEST_MISMATCH_TICKET_TYPE_ROUND);
    }

    final int diff = request.totalCount() - existingEntity.getTotalTicketCount();

    adminTicketTypeUpdater.updateTicketType(
            existingEntity,
            request.name(),
            request.price(),
            request.totalCount(),
            request.startDate(),
            request.endDate()
    );

    updateDiffMap.put(existingEntity.getTicketTypeId(), diff);
    updatedEntities.add(existingEntity);
}

이렇게 분리하면:

  • 각 메서드의 책임이 명확해짐
  • 단위 테스트 작성이 용이해짐
  • 코드 이해와 수정이 쉬워짐
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f695242 and 3bf24f6.

📒 Files selected for processing (6)
  • src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java (7 hunks)
  • src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminRedisTicketTypeSaver.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRetriever.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/global/redis/RedisManager.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/global/redis/RedisTicketTypeCountInitializer.java (1 hunks)
  • src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminRedisTicketTypeSaver.java
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sjk4618
Repo: PERMIT-SEOUL/permit-server PR: 48
File: src/main/java/com/permitseoul/permitserver/domain/guest/core/domain/entity/GuestEntity.java:31-37
Timestamp: 2025-07-15T09:37:32.765Z
Learning: sjk4618 prefers to implement factory methods or public constructors for entities when they are actually needed, rather than creating them proactively.
🧬 Code graph analysis (3)
src/main/java/com/permitseoul/permitserver/global/redis/RedisManager.java (2)
src/main/java/com/permitseoul/permitserver/global/exception/RedisKeyNotFoundException.java (1)
  • RedisKeyNotFoundException (3-4)
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java (1)
  • Service (43-379)
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java (3)
src/main/java/com/permitseoul/permitserver/global/Constants.java (1)
  • Constants (3-24)
src/main/java/com/permitseoul/permitserver/global/redis/RedisManager.java (1)
  • Service (14-79)
src/main/java/com/permitseoul/permitserver/domain/admin/event/api/service/AdminEventService.java (1)
  • Service (44-354)
src/main/java/com/permitseoul/permitserver/global/redis/RedisTicketTypeCountInitializer.java (2)
src/main/java/com/permitseoul/permitserver/global/Constants.java (1)
  • Constants (3-24)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminRedisTicketTypeSaver.java (1)
  • Component (14-29)
🔇 Additional comments (9)
src/main/java/com/permitseoul/permitserver/global/response/code/ErrorCode.java (1)

129-131: LGTM! 에러 코드 중복 문제가 해결되었습니다.

이전 리뷰에서 지적된 중복 에러 코드 문제(둘 다 50015 사용)가 올바르게 수정되었습니다. 이제 각 에러 코드가 고유한 번호(50015, 50016)를 가지고 있어 에러 추적과 모니터링이 정상적으로 작동할 수 있습니다.

src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRetriever.java (1)

32-39: LGTM! 이전 리뷰 피드백이 정상적으로 반영되었습니다.

findAllById로 조회한 엔티티 수를 검증하는 로직이 올바르게 추가되었습니다. 요청한 모든 ID가 조회되지 않으면 예외를 발생시켜 데이터 불일치를 방지합니다.

단, 입력 ids 리스트에 중복된 ID가 있을 경우 검증 로직이 실패할 수 있습니다. 현재 요구사항에서 중복 ID가 허용되지 않는다면 문제없지만, 향후 중복 허용이 필요한 경우 Set 기반 검증으로 개선이 필요할 수 있습니다.

src/main/java/com/permitseoul/permitserver/global/redis/RedisTicketTypeCountInitializer.java (2)

43-50: 롤백 로직이 적절합니다.

각 키 삭제 작업을 개별 try-catch로 감싸서 일부 삭제가 실패해도 나머지 키의 롤백을 계속 시도합니다. 실패한 키는 명확히 로깅되어 운영 중 추적이 가능합니다.


26-41: 서버 초기화 시 잘못된 티켓 수량이 Redis에 저장될 수 있습니다.

Line 33에서 getRemainTicketCount()를 사용하고 있는데, 이는 서버 초기화 시점에 문제를 일으킬 수 있습니다:

  1. getRemainTicketCount()는 현재 남은 티켓 수를 반환
  2. 서버 재시작 전에 이미 판매된 티켓이 있다면, Redis 초기화 시 남은 수량이 아닌 전체 수량으로 복원됨
  3. 이로 인해 실제보다 많은 티켓이 판매 가능하게 되어 과판매 위험 발생

다음과 같이 수정하세요:

-                final boolean isCreated = redisManager.setIfAbsent(key, String.valueOf(ticketType.getRemainTicketCount()));
+                final boolean isCreated = redisManager.setIfAbsent(key, String.valueOf(ticketType.getRemainTicketCount()));
 
                 if (isCreated) {
                     createdKeys.add(key);
-                    log.info("[Redis] 서버 실행 - 초기 ticketType redis 생성, key = {}, value = {}", key, ticketType.getRemainTicketCount());
+                    log.info("[Redis] 서버 실행 - 초기 ticketType redis 생성, key = {}, value = {}", key, ticketType.getRemainTicketCount());
                 } else {
-                    log.info("[Redis] 서버 실행 - 기존 ticketType 유지(스킵), key = {}, value = {}", key, ticketType.getRemainTicketCount());
+                    log.info("[Redis] 서버 실행 - 기존 ticketType 유지(스킵), key = {}", key);
                 }

참고: setIfAbsent를 사용하고 있으므로 Redis에 키가 이미 존재하면 덮어쓰지 않습니다. 따라서 서버 재시작 시 Redis가 유지되고 있다면 문제가 발생하지 않지만, Redis가 초기화된 경우 DB의 remainTicketCount를 사용하는 것이 올바릅니다.

Likely an incorrect or invalid review comment.

src/main/java/com/permitseoul/permitserver/global/redis/RedisManager.java (2)

21-30: LGTM!

TTL의 null 여부에 따라 적절히 분기 처리하여 영구 키와 만료 키를 모두 지원합니다.


66-71: LGTM!

null 검증과 빈 맵 처리가 적절하며, 배치 연산을 효율적으로 수행합니다.

src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java (3)

145-171: 트랜잭션 경계와 Redis 동기화 순서를 재검토하세요.

Line 170에서 syncRedisTicketCounts를 호출하는데, 이는 @Transactional 메서드 내부이지만 DB 작업 try-catch 블록 외부에 있습니다:

  1. Lines 148-168: DB 업데이트 (try-catch로 감싸짐)
  2. Line 170: Redis 동기화 (메서드 레벨 트랜잭션 내부)

현재는 syncRedisTicketCounts에서 예외가 발생하면 전체 트랜잭션이 롤백되지만, syncRedisTicketCounts 내부에서 자체적으로 롤백을 시도합니다 (Lines 217-246). 이로 인해:

  • Redis 롤백은 수행되지만 DB는 이미 변경됨
  • 이중 롤백 로직이 혼란스러움

개선 제안:

 @Transactional
 public void updateTicketRoundWithType(final TicketRoundWithTypeUpdateRequest updateRequest) {
     final TicketTypeSplitResult ticketTypeSplitByOldAndNewDto;
     try {
         final TicketRoundEntity ticketRoundEntity =
                 ticketRoundRetriever.findTicketRoundEntityById(updateRequest.ticketRoundId());
         adminTicketRoundUpdater.updateTicketRound(
                 ticketRoundEntity,
                 updateRequest.ticketRoundName(),
                 updateRequest.ticketRoundSalesStartDate(),
                 updateRequest.ticketRoundSalesEndDate()
         );
 
         ticketTypeSplitByOldAndNewDto = splitTicketTypeNewAndUpdate(ticketRoundEntity, updateRequest.ticketTypes());
 
         if (!ticketTypeSplitByOldAndNewDto.newTicketTypes().isEmpty()) {
             adminTicketTypeSaver.saveAllTicketTypes(ticketTypeSplitByOldAndNewDto.newTicketTypes());
         }
+        
+        // DB 커밋 전 Redis 동기화
+        syncRedisTicketCounts(ticketTypeSplitByOldAndNewDto);
     } catch (TicketRoundNotFoundException e) {
         throw new AdminApiException(ErrorCode.NOT_FOUND_TICKET_ROUND);
     } catch (TicketRoundIllegalArgumentException | TicketTypeIllegalException e) {
         throw new AdminApiException(ErrorCode.BAD_REQUEST_DATE_TIME_ERROR);
     } catch (AdminTicketTypeNotFoundException e) {
         throw new AdminApiException(ErrorCode.NOT_FOUND_TICKET_TYPE);
     }
-
-    syncRedisTicketCounts(ticketTypeSplitByOldAndNewDto);
 }

이렇게 하면 Redis 동기화 실패 시 DB도 롤백되어 일관성이 보장됩니다.


217-246: LGTM! 이전 리뷰의 롤백 로직 문제가 올바르게 수정되었습니다.

Lines 228-231에서 이전 값을 직접 복원하는 방식으로 변경되어, 과판매 위험이 제거되었습니다. 이전에는 diff 기반 increment로 롤백을 시도했으나, 지금은 저장해둔 previousValue를 그대로 복원합니다.

개선된 점:

  1. 기존 키: previousValue를 직접 set하여 정확히 복원
  2. 신규 키: delete로 완전히 제거
  3. 각 작업의 성공/실패를 개별적으로 로깅하여 추적 가능

307-319: LGTM! 반환 타입 변경이 적절합니다.

메서드 시그니처가 void에서 List<TicketType>으로 변경되어, 호출자가 저장된 티켓 타입을 Redis에 동기화할 수 있게 되었습니다. 이는 create/update 플로우에서 필요한 변경입니다.

Comment on lines +173 to +215
private void syncRedisTicketCounts(final TicketTypeSplitResult ticketTypeSplitResult) {
final List<String> newRedisTicketTypeKeys = new ArrayList<>();
final Map<String, String> existRedisTicketType = new HashMap<>();

try {
// 새로운 ticketType 등록
for (TicketTypeEntity newTicketTypeEntity : ticketTypeSplitResult.newTicketTypes()) {
final String key = Constants.REDIS_TICKET_TYPE_KEY_NAME + newTicketTypeEntity.getTicketTypeId() + Constants.REDIS_TICKET_TYPE_REMAIN;
redisManager.set(key, String.valueOf(newTicketTypeEntity.getTotalTicketCount()), null);
newRedisTicketTypeKeys.add(key);
log.info("[Redis] 신규 ticketType 등록 key={}, ticketTypeCount={}", key, newTicketTypeEntity.getTotalTicketCount());
}

// 기존에 있던 ticketType 업데이트
for (TicketTypeEntity updatedTicketTypeEntity : ticketTypeSplitResult.updatedTicketTypes()) {
final String key = Constants.REDIS_TICKET_TYPE_KEY_NAME + updatedTicketTypeEntity.getTicketTypeId() + Constants.REDIS_TICKET_TYPE_REMAIN;
final String existTicketTypeCount = redisManager.get(key);
existRedisTicketType.put(key, existTicketTypeCount);
if (existTicketTypeCount == null) {
throw new AdminApiException(ErrorCode.INTERNAL_TICKET_TYPE_NOT_FOUND_REDIS_ERROR);
}

final int diff = ticketTypeSplitResult.updateDiffMap().get(updatedTicketTypeEntity.getTicketTypeId());
if (diff == 0) continue;

final Long newRemain = redisManager.increment(key, diff);
if (newRemain == null) {
throw new AdminApiException(ErrorCode.INTERNAL_TICKET_TYPE_REDIS_ERROR);
}

if (newRemain <= 0) {
redisManager.set(key, "0", null);
log.warn("[Redis] ticketType 품절 key={}, ",key);
} else {
log.info("[Redis] 기존 ticketType update complete, key={}, diff={}, remain={}", key, diff, newRemain);
}
}
} catch (Exception e) {
log.error("[Redis] 오류 발생 → Redis 롤백 수행 시작", e);
rollbackRedisChanges(ticketTypeSplitResult, existRedisTicketType, newRedisTicketTypeKeys);
throw new AdminApiException(ErrorCode.INTERNAL_TICKET_TYPE_REDIS_ERROR);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Redis 동기화 로직이 전반적으로 잘 구현되었으나 일부 개선이 필요합니다.

전체 구조는 좋지만, Lines 203-208의 음수 처리 로직을 검토해야 합니다:

if (newRemain <= 0) {
    redisManager.set(key, "0", null);
    log.warn("[Redis] ticketType 품절 key={}, ",key);
}

문제점:

  1. incrementnewRemain이 음수가 되었다는 것은 동시성 문제나 데이터 불일치를 의미합니다.
  2. 음수를 "0"으로 덮어쓰면 실제 문제가 가려집니다.
  3. 음수가 발생한 경우 관리자가 즉시 인지하고 조치해야 합니다.

다음과 같이 개선하세요:

             final Long newRemain = redisManager.increment(key, diff);
             if (newRemain == null) {
                 throw new AdminApiException(ErrorCode.INTERNAL_TICKET_TYPE_REDIS_ERROR);
             }
 
             if (newRemain <= 0) {
-                redisManager.set(key, "0", null);
-                log.warn("[Redis] ticketType 품절 key={}, ",key);
+                if (newRemain < 0) {
+                    log.error("[Redis] ⚠️ 티켓 재고 음수 발생 key={}, remain={}, diff={} - 긴급 확인 필요", 
+                              key, newRemain, diff);
+                    // 음수는 데이터 불일치를 의미하므로 0으로 보정하되 명확히 로깅
+                    redisManager.set(key, "0", null);
+                    // TODO: 알림 발송 또는 모니터링 시스템에 전달
+                } else {
+                    log.info("[Redis] ticketType 품절 key={}, remain={}", key, newRemain);
+                }
             } else {
                 log.info("[Redis] 기존 ticketType update complete, key={}, diff={}, remain={}", key, diff, newRemain);
             }

이렇게 하면:

  • 0 이하 vs 음수를 구분하여 처리
  • 음수 발생 시 명확한 ERROR 로그 (모니터링 알람 가능)
  • 향후 알림 시스템 연동 가능 (TODO 주석)
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java
around lines 173-215, the current handling after redisManager.increment masks
negative remains by unconditionally setting them to "0"; instead, distinguish
zero vs negative: if newRemain == 0 set the redis value to "0" and log an
info/warn; if newRemain < 0 do NOT silently overwrite — log an ERROR with key,
diff, newRemain and any prior value (existRedisTicketType.get(key)), add a TODO
to trigger/emit an alert (monitoring/notification), and either throw an
AdminApiException to cause rollback or still set the redis value to "0" but
immediately throw so the negative state is visible and handled by rollback/ops;
ensure the error log contains full contextual details for debugging.

Comment on lines +52 to +64
public Long decrement(final String key, final long count) {
if(!isExists(key)) {
throw new RedisKeyNotFoundException();
}
return redisTemplate.opsForValue().decrement(key, count);
}

public Long increment(final String key, final long count) {
if(!isExists(key)) {
throw new RedisKeyNotFoundException();
}
return redisTemplate.opsForValue().increment(key, count);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

increment/decrement 메서드의 키 존재 확인 로직을 검토하세요.

Lines 53, 60에서 isExists(key) 체크 후 increment/decrement를 수행하는데, 다음 사항을 고려해야 합니다:

  1. TOCTOU 이슈: isExists 체크와 increment/decrement 호출 사이에 키가 삭제될 수 있습니다 (동시성 환경).
  2. Spring Data Redis 동작: StringRedisTemplateincrement/decrement는 키가 없으면 자동으로 0부터 시작합니다. 따라서 키 존재 체크가 없어도 작동합니다.

현재 코드의 의도가 "키가 없을 때 예외를 발생시키는 것"이라면, 다음과 같은 개선이 필요합니다.

현재 동작이 의도한 대로인지 확인하세요:

개선 제안:

존재하지 않는 키에 대한 연산을 명시적으로 방지하려면 Lua 스크립트를 사용하는 것이 더 안전합니다:

public Long increment(final String key, final long count) {
    String script = "if redis.call('exists', KEYS[1]) == 0 then return nil else return redis.call('incrby', KEYS[1], ARGV[1]) end";
    Long result = redisTemplate.execute(
        new DefaultRedisScript<>(script, Long.class),
        Collections.singletonList(key),
        String.valueOf(count)
    );
    if (result == null) {
        throw new RedisKeyNotFoundException();
    }
    return result;
}

이 방식은 존재 확인과 증감 연산을 원자적으로 수행합니다.


🏁 Script executed:

#!/bin/bash
# Redis increment/decrement 호출처를 찾아서 키가 없을 때의 예외 처리를 확인
rg -nP --type=java -C3 'redisManager\.(increment|decrement)\('

Length of output: 1107


이 부분은 핵심적인 API 계약 불일치를 확인했으므로 추가로 isExists 메서드를 확인하겠습니다:

increment/decrement 메서드의 동작 방식을 명확히 하세요.

현재 코드에서 발견된 문제점:

  1. API 계약 불일치: Redis의 INCR 명령은 키가 없으면 0부터 시작합니다. 하지만 현재 RedisManager는 키가 없을 때 예외를 던집니다. 반면 AdminTicketService 라인 199에서는 null 반환을 기대하며 체크하고 있습니다:

    final Long newRemain = redisManager.increment(key, diff);
    if (newRemain == null) {  // null 체크 예상
        throw new AdminApiException(...);
    }

    메서드는 예외를 던지므로 null 체크는 동작하지 않습니다.

  2. TOCTOU 경합 (Time-of-check-time-of-use): isExists 체크와 increment 호출 사이에 키가 삭제될 수 있습니다. 다만 Redis 자체는 원자적이므로 실제 위험은 낮습니다.

  3. redundant 체크: Redis INCR은 자동으로 존재하지 않는 키를 처리하므로, 사전 존재 여부 확인은 불필요합니다.

해결 방안:

  • 키 존재 확인을 제거하고 Redis의 자동 초기화 동작에 맡기거나
  • 메서드가 null 대신 예외를 던진다면 호출 코드를 예외 처리로 수정하세요
🤖 Prompt for AI Agents
In src/main/java/com/permitseoul/permitserver/global/redis/RedisManager.java
around lines 52 to 64, the current increment/decrement implementations pre-check
key existence and throw RedisKeyNotFoundException which contradicts Redis
semantics and callers expecting non-exceptional behavior; remove the
isExists(...) check so the methods call
redisTemplate.opsForValue().increment/decrement(...) directly (no pre-check),
thereby delegating initial-key handling to Redis and avoiding TOCTOU and
redundant checks; also audit callers (e.g., AdminTicketService) and either
remove null-based checks or adapt them to handle the returned Long
result/possible exceptions consistently.

Comment on lines 42 to 51
} catch (Exception e) {
for (String key : createdKeys) {
try {
redisManager.delete(key);
log.warn("[Redis] 서버 실행 ticketType 롤백 DEL key={}", key);
} catch (Exception ex) {
log.error("[Redis] 서버 실행 ticketType 롤백 실패 failed key={}", key, ex);
}
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Redis 초기화 실패 시 애플리케이션 시작을 중단하거나 명확히 표시하세요.

현재 구현은 Redis 초기화 중 예외가 발생해도 롤백만 수행하고 예외를 다시 던지지 않습니다. 이로 인해:

  1. Redis 초기화가 실패했음에도 애플리케이션이 정상적으로 시작됨
  2. 일부 티켓 타입만 Redis에 존재하는 불완전한 상태 발생 가능
  3. 운영 중 티켓 판매 시 INTERNAL_TICKET_TYPE_NOT_FOUND_REDIS_ERROR 발생

다음 중 하나의 방식으로 수정하세요:

옵션 1 (권장): 초기화 실패 시 애플리케이션 시작 중단

         } catch (Exception e) {
+            log.error("[Redis] 서버 실행 ticketType 초기화 실패 - 애플리케이션 종료", e);
             for (String key : createdKeys) {
                 try {
                     redisManager.delete(key);
                     log.warn("[Redis] 서버 실행 ticketType 롤백 DEL key={}", key);
                 } catch (Exception ex) {
                     log.error("[Redis] 서버 실행 ticketType 롤백 실패 failed key={}", key, ex);
                 }
             }
+            throw new IllegalStateException("Redis 티켓 타입 초기화 실패", e);
         }

옵션 2: 초기화 실패를 명확히 로깅하고 헬스체크에 반영

+    private volatile boolean initializationSuccessful = false;
+
     @Override
     public void run(ApplicationArguments args) {
         ...
         } catch (Exception e) {
+            log.error("[Redis] ⚠️⚠️⚠️ 서버 실행 ticketType 초기화 실패 - Redis 상태 불일치 주의 ⚠️⚠️⚠️", e);
+            initializationSuccessful = false;
             for (String key : createdKeys) {
                 ...
             }
+            return;
         }
+        initializationSuccessful = true;
+        log.info("[Redis] 서버 실행 ticketType 초기화 성공");
     }
+
+    public boolean isInitializationSuccessful() {
+        return initializationSuccessful;
+    }

@sjk4618 sjk4618 merged commit 802835a into dev Oct 31, 2025
1 of 2 checks passed
@sjk4618 sjk4618 deleted the feat/#150 branch October 31, 2025 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: admin 티켓 로직들 redis 반영

2 participants