Repository navigation
Conversation
* fix: 회원가입 이메일 락 전략 추상화 * fix: 이메일 락 안정성 보강 * fix: 이메일 락 데드락 매핑 제거
📝 WalkthroughWalkthrough이 PR은 회원가입 프로세스에 이메일 기반 동시성 제어, 신청서 필터링, 활동 단위 추적을 도입합니다. 전략 패턴으로 잠금을 추상화하고, 신청서 검색 기능을 확장하며, 데이터베이스 스키마를 확대합니다. Changes회원가입 동시성 제어 및 필터링
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/kotlin/co/yappuworld/user/client/application/usecase/SignUpExecutorConcurrencyTest.kt (1)
74-75:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExecutorService 종료 누락으로 테스트 스레드 누수 가능성이 있습니다.
고정 스레드풀을 생성한 뒤
shutdown/awaitTermination을 호출하지 않아, 테스트 반복 시 잔여 스레드가 누적될 수 있습니다. 두 시나리오 모두에서try/finally로 정리해 주세요.🔧 제안 수정안
import java.util.concurrent.CountDownLatch import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit @@ scenario("회원가입") { val threadCount = 20 val executorService = Executors.newFixedThreadPool(32) val latch = CountDownLatch(threadCount) - - repeat(threadCount) { - val application = getSignUpApplicationEntityFixture( - email = "concurrency-test" - ) - executorService.submit { - try { - signUpExecutor.signUp( - application = application, - role = UserRole.ACTIVE, - now = getCurrentDateTimeInKST() - ) - } finally { - latch.countDown() - } - } - } - - latch.await() - countUsersByEmail("concurrency-test") shouldBe 1 + try { + repeat(threadCount) { + val application = getSignUpApplicationEntityFixture( + email = "concurrency-test" + ) + executorService.submit { + try { + signUpExecutor.signUp( + application = application, + role = UserRole.ACTIVE, + now = getCurrentDateTimeInKST() + ) + } finally { + latch.countDown() + } + } + } + + latch.await() + countUsersByEmail("concurrency-test") shouldBe 1 + } finally { + executorService.shutdown() + executorService.awaitTermination(5, TimeUnit.SECONDS) + } } @@ scenario("가입 신청") { val threadCount = 20 val executorService = Executors.newFixedThreadPool(32) val latch = CountDownLatch(threadCount) - - repeat(threadCount) { - val application = getSignUpApplicationEntityFixture( - email = "abcde@gmail.com" - ) - executorService.submit { - try { - signUpExecutor.submit(application = application) - } finally { - latch.countDown() - } - } - } - - latch.await() - countSignUpApplicationsByEmail("abcde@gmail.com") shouldBe 1 + try { + repeat(threadCount) { + val application = getSignUpApplicationEntityFixture( + email = "abcde@gmail.com" + ) + executorService.submit { + try { + signUpExecutor.submit(application = application) + } finally { + latch.countDown() + } + } + } + + latch.await() + countSignUpApplicationsByEmail("abcde@gmail.com") shouldBe 1 + } finally { + executorService.shutdown() + executorService.awaitTermination(5, TimeUnit.SECONDS) + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/kotlin/co/yappuworld/user/client/application/usecase/SignUpExecutorConcurrencyTest.kt` around lines 74 - 75, The test creates a fixed thread pool with Executors.newFixedThreadPool and assigns it to executorService but never shuts it down; wrap the threading logic that uses executorService and CountDownLatch in a try/finally so you call executorService.shutdown() in finally and then await termination (e.g., executorService.awaitTermination with a timeout) to ensure threads are cleaned up between test runs; update both scenarios in SignUpExecutorConcurrencyTest.kt to use this try/finally pattern around executorService usage so no thread leakage occurs.
🧹 Nitpick comments (1)
src/main/kotlin/co/yappuworld/user/infrastructure/lock/OracleSignUpEmailLockExecutor.kt (1)
96-105: 💤 Low value
touch()메서드가 필요한지 검토해주세요.락을 획득한 이후
UPDATED_AT을 업데이트하는 것이 감사(auditing) 목적이라면 문제없지만, 락 메커니즘 자체에는 불필요해 보입니다. 이 업데이트가 의도적인 것인지 확인해주세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/co/yappuworld/user/infrastructure/lock/OracleSignUpEmailLockExecutor.kt` around lines 96 - 105, The touch(email) method in OracleSignUpEmailLockExecutor updates SIGN_UP_EMAIL_LOCK.UPDATED_AT after acquiring a lock but may be unnecessary for the lock mechanism; confirm intent and either (A) remove the touch(email) function and any calls to it (and associated SQL) if it serves no locking purpose, or (B) if it is intended for auditing, rename it to something like auditTouch/emailTouch and add a comment explaining its auditing purpose and keep the UPDATE statement as-is so future readers understand it is not part of the lock algorithm.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/kotlin/co/yappuworld/user/infrastructure/lock/PostgresqlSignUpEmailLockExecutor.kt`:
- Around line 34-39: The current tryLock implementation using
pg_try_advisory_xact_lock(hashtext(...)) (see tryLock,
pg_try_advisory_xact_lock, hashtext, jdbcTemplate.queryForObject) risks hash
collisions; replace it with an explicit lock-table approach: create a dedicated
table (e.g., signup_email_locks with email as primary key) and implement tryLock
to obtain a row-level lock for the specific email (INSERT ... ON CONFLICT DO
NOTHING then SELECT ... FOR UPDATE NOWAIT or attempt to lock the existing row
with SELECT FOR UPDATE NOWAIT) so locks are per-email string without relying on
hashtext collisions, and adjust tryLock to return true only when the row lock is
obtained and false otherwise.
- Around line 19-32: Postgresql 구현의 executeWithLockReturning이 현재 즉시 실패하는
tryLock만 호출해 timeoutSeconds를 무시하므로, Oracle의 FOR UPDATE WAIT와 동등한 동작을 위해
timeoutSeconds를 존중하도록 수정하세요: executeWithLockReturning에서 tryLock(email)을 단번에 호출하는
대신 시작시간을 기록하고 timeoutSeconds만큼 반복 시도(짧은 sleep 간격, 예: 50-200ms)하며 성공하면 action()을
호출하고, 시간이 초과되면 BusinessException(UserError.ALREADY_PROCESSED_EMAIL)를 던지도록
구현하거나(대신 영속적 블로킹을 원하면 pg_advisory_xact_lock 사용과 statement_timeout 설정을 고려) 클래스의
timeoutSeconds 필드/파라미터를 참조하도록 보장하세요; 관련 심볼: executeWithLockReturning, tryLock,
timeoutSeconds, BusinessException(UserError.ALREADY_PROCESSED_EMAIL),
pg_try_advisory_xact_lock.
In
`@src/main/kotlin/co/yappuworld/user/infrastructure/SignUpApplicationFindService.kt`:
- Around line 113-117: The current implementation in
SignUpApplicationFindService builds a full in-memory list named applications and
then uses applications.drop(...).take(...) to page, which causes OOM/perf
issues; replace this with a database-level paged query (e.g., use Kotlin JDSL
listQuery or the JPA Query/QueryDSL used in this class) to fetch only the page
by applying setFirstResult(pageRequest.offset.toInt()) and
setMaxResults(pageRequest.pageSize) (or the equivalent listQuery paging API),
execute a separate count query for the total, and return PageImpl with the
pagedResults, pageRequest, and the count instead of slicing the full
applications list.
- Around line 92-93: The query only searches
SignUpApplicationEntity::applicantName, so records with null applicantName (but
with a fallback name in details.name as returned by getUserName()) are excluded;
update the search predicate to include an OR that also matches the fallback
(e.g., add a condition checking the JSON/details name or use COALESCE-like
behavior) so that name searches match either applicantName or details.name when
applicantName is null; locate the predicate construction around the
path(SignUpApplicationEntity::applicantName).like("%$it%") and extend it to
include the alternative field used by getUserName().
In `@src/main/resources/schema.sql`:
- Around line 35-49: The schema.sql DDL uses varchar lengths for applicant_name
and position that differ from Flyway V1, causing environment-specific storage
differences; update the column definitions in this file (applicant_name in the
first table and position in sign_up_application_activity_unit) to use the exact
varchar length values defined in Flyway V1 so both migrations and the checked-in
schema stay consistent (ensure applicant_name and position match Flyway V1's
lengths exactly).
In `@src/test/kotlin/co/yappuworld/support/lock/FakeSignUpEmailLockExecutor.kt`:
- Line 24: locks가 ConcurrentHashMap에 계속 쌓여 메모리 누수가 발생하니
FakeSignUpEmailLockExecutor의 locks 맵을 약한 참조 또는 정리 메커니즘으로 변경해 주세요; 예를 들어
ConcurrentHashMap 대신 WeakHashMap(또는 ConcurrentMap<WeakReference<String>,
ReentrantLock> 패턴)을 사용하거나, unlock 이후에 해당 키를 제거하도록 unlock 처리 로직에 제거 코드를 추가하고 테스트
종료 시 호출할 수 있는 cleanup() 메서드(예: fun cleanup() { locks.clear() })를 구현해 메모리 누수를
방지하세요.
---
Outside diff comments:
In
`@src/test/kotlin/co/yappuworld/user/client/application/usecase/SignUpExecutorConcurrencyTest.kt`:
- Around line 74-75: The test creates a fixed thread pool with
Executors.newFixedThreadPool and assigns it to executorService but never shuts
it down; wrap the threading logic that uses executorService and CountDownLatch
in a try/finally so you call executorService.shutdown() in finally and then
await termination (e.g., executorService.awaitTermination with a timeout) to
ensure threads are cleaned up between test runs; update both scenarios in
SignUpExecutorConcurrencyTest.kt to use this try/finally pattern around
executorService usage so no thread leakage occurs.
---
Nitpick comments:
In
`@src/main/kotlin/co/yappuworld/user/infrastructure/lock/OracleSignUpEmailLockExecutor.kt`:
- Around line 96-105: The touch(email) method in OracleSignUpEmailLockExecutor
updates SIGN_UP_EMAIL_LOCK.UPDATED_AT after acquiring a lock but may be
unnecessary for the lock mechanism; confirm intent and either (A) remove the
touch(email) function and any calls to it (and associated SQL) if it serves no
locking purpose, or (B) if it is intended for auditing, rename it to something
like auditTouch/emailTouch and add a comment explaining its auditing purpose and
keep the UPDATE statement as-is so future readers understand it is not part of
the lock algorithm.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d87a01be-4beb-4ae2-b39f-fc65b3b27a63
📒 Files selected for processing (36)
build.gradle.ktssrc/main/kotlin/co/yappuworld/global/config/AppConfig.ktsrc/main/kotlin/co/yappuworld/user/client/application/AdminSignUpService.ktsrc/main/kotlin/co/yappuworld/user/client/application/usecase/SignUpExecutor.ktsrc/main/kotlin/co/yappuworld/user/client/dto/request/AdminSignUpApplicationPageRequest.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/DiscordUserSystemNotifier.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/SignUpApplicationCommandService.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/SignUpApplicationFindService.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/UserCommandService.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/entity/SignUpApplicationActivityUnitEntity.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/entity/SignUpApplicationEntity.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/jpa/SignUpApplicationActivityUnitRepository.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/jpa/SignUpApplicationRepository.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/jpa/UserRepository.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/lock/NoopSignUpEmailLockExecutor.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/lock/OracleSignUpEmailLockExecutor.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/lock/PostgresqlSignUpEmailLockExecutor.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/lock/SignUpEmailLockExecutor.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/lock/SignUpEmailLockProperties.ktsrc/main/kotlin/co/yappuworld/user/infrastructure/model/SignUpApplicationSearchParam.ktsrc/main/resources/application-dev-oci.yamlsrc/main/resources/application-local.yamlsrc/main/resources/application-prod-oci.yamlsrc/main/resources/application-test.yamlsrc/main/resources/db/migration/V1__add_applicant_name_and_activity_unit_table.sqlsrc/main/resources/db/migration/V2__migrate_signup_application_data.sqlsrc/main/resources/db/migration/V3__add_sign_up_email_lock_table.sqlsrc/main/resources/schema.sqlsrc/test/kotlin/co/yappuworld/support/environment/SpringBootTestFeatureSpec.ktsrc/test/kotlin/co/yappuworld/support/fixture/UserFixture.ktsrc/test/kotlin/co/yappuworld/support/lock/FakeSignUpEmailLockExecutor.ktsrc/test/kotlin/co/yappuworld/user/client/application/usecase/SignUpExecutorConcurrencyTest.ktsrc/test/kotlin/co/yappuworld/user/client/application/usecase/SignUpExecutorTest.ktsrc/test/kotlin/co/yappuworld/user/client/application/usecase/SignUpFacadeUnitTest.ktsrc/test/kotlin/co/yappuworld/user/infrastructure/SignUpApplicationCommandServiceTest.ktsrc/test/kotlin/co/yappuworld/user/infrastructure/SignUpApplicationFindServiceTest.kt
💤 Files with no reviewable changes (3)
- src/main/kotlin/co/yappuworld/user/infrastructure/UserCommandService.kt
- src/main/kotlin/co/yappuworld/user/infrastructure/jpa/UserRepository.kt
- src/main/kotlin/co/yappuworld/user/infrastructure/jpa/SignUpApplicationRepository.kt
Comment on lines
+19
to
+32
| override fun <T> executeWithLockReturning( | ||
| email: String, | ||
| action: () -> T | ||
| ): T { | ||
| check(TransactionSynchronizationManager.isActualTransactionActive()) { | ||
| "PostgresqlSignUpEmailLockExecutor must be invoked within an active transaction; " + | ||
| "otherwise the transaction-scoped advisory lock is released immediately." | ||
| } | ||
|
|
||
| if (!tryLock(email)) { | ||
| throw BusinessException(UserError.ALREADY_PROCESSED_EMAIL) | ||
| } | ||
| return action() | ||
| } |
There was a problem hiding this comment.
timeoutSeconds 설정이 무시됩니다.
Oracle 구현체는 FOR UPDATE WAIT 구문으로 timeoutSeconds만큼 대기하지만, PostgreSQL 구현체는 pg_try_advisory_xact_lock을 사용하여 즉시 성공/실패를 반환합니다. 이는 전략 간 동작 불일치를 초래합니다.
일관성을 위해 PostgreSQL에서도 타임아웃을 구현하거나, 문서화를 통해 이 차이를 명시해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/kotlin/co/yappuworld/user/infrastructure/lock/PostgresqlSignUpEmailLockExecutor.kt`
around lines 19 - 32, Postgresql 구현의 executeWithLockReturning이 현재 즉시 실패하는
tryLock만 호출해 timeoutSeconds를 무시하므로, Oracle의 FOR UPDATE WAIT와 동등한 동작을 위해
timeoutSeconds를 존중하도록 수정하세요: executeWithLockReturning에서 tryLock(email)을 단번에 호출하는
대신 시작시간을 기록하고 timeoutSeconds만큼 반복 시도(짧은 sleep 간격, 예: 50-200ms)하며 성공하면 action()을
호출하고, 시간이 초과되면 BusinessException(UserError.ALREADY_PROCESSED_EMAIL)를 던지도록
구현하거나(대신 영속적 블로킹을 원하면 pg_advisory_xact_lock 사용과 statement_timeout 설정을 고려) 클래스의
timeoutSeconds 필드/파라미터를 참조하도록 보장하세요; 관련 심볼: executeWithLockReturning, tryLock,
timeoutSeconds, BusinessException(UserError.ALREADY_PROCESSED_EMAIL),
pg_try_advisory_xact_lock.
Comment on lines
+34
to
+39
| private fun tryLock(email: String): Boolean = | ||
| jdbcTemplate.queryForObject( | ||
| "SELECT pg_try_advisory_xact_lock(hashtext(CAST(? AS text)))", | ||
| Boolean::class.java, | ||
| ) ?: false |
There was a problem hiding this comment.
hashtext 함수의 해시 충돌 위험이 있습니다.
hashtext는 정수 해시를 반환하므로, 서로 다른 이메일이 동일한 해시값을 가질 수 있습니다. 이 경우 관련 없는 이메일 간에 불필요한 락 경합이 발생할 수 있습니다.
충돌 가능성이 낮지만, 프로덕션 환경에서는 이메일을 직접 키로 사용하는 명시적인 락 테이블 방식(Oracle과 유사)을 고려하는 것이 더 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/kotlin/co/yappuworld/user/infrastructure/lock/PostgresqlSignUpEmailLockExecutor.kt`
around lines 34 - 39, The current tryLock implementation using
pg_try_advisory_xact_lock(hashtext(...)) (see tryLock,
pg_try_advisory_xact_lock, hashtext, jdbcTemplate.queryForObject) risks hash
collisions; replace it with an explicit lock-table approach: create a dedicated
table (e.g., signup_email_locks with email as primary key) and implement tryLock
to obtain a row-level lock for the specific email (INSERT ... ON CONFLICT DO
NOTHING then SELECT ... FOR UPDATE NOWAIT or attempt to lock the existing row
with SELECT FOR UPDATE NOWAIT) so locks are per-email string without relying on
hashtext collisions, and adjust tryLock to return true only when the row lock is
obtained and false otherwise.
Comment on lines
+92
to
+93
| path(SignUpApplicationEntity::applicantName).like("%$it%") | ||
| ) |
There was a problem hiding this comment.
검색 로직 불일치: applicantName null 처리 누락
applicantName 컬럼만 검색하고 있어, applicantName이 null인 레코드는 검색되지 않습니다. 하지만 getUserName() 메서드는 applicantName이 null일 때 details.name으로 폴백합니다.
사용자가 이름으로 검색할 때 applicantName이 null인 신청서(마이그레이션 이전 데이터 등)도 찾을 수 있어야 한다면, 검색 조건을 조정해야 합니다.
의도가 명확하다면 이 코멘트는 무시하셔도 됩니다.
대안: JSON 필드 검색 또는 OR 조건 추가
만약 details.name도 검색 대상에 포함하려면, JSON 컬럼 검색 또는 OR 조건을 추가하는 것을 고려하세요. 다만 JSON 검색은 데이터베이스 지원에 따라 다르므로 확인이 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/kotlin/co/yappuworld/user/infrastructure/SignUpApplicationFindService.kt`
around lines 92 - 93, The query only searches
SignUpApplicationEntity::applicantName, so records with null applicantName (but
with a fallback name in details.name as returned by getUserName()) are excluded;
update the search predicate to include an OR that also matches the fallback
(e.g., add a condition checking the JSON/details name or use COALESCE-like
behavior) so that name searches match either applicantName or details.name when
applicantName is null; locate the predicate construction around the
path(SignUpApplicationEntity::applicantName).like("%$it%") and extend it to
include the alternative field used by getUserName().
Comment on lines
+113
to
+117
| return PageImpl( | ||
| applications.drop(pageRequest.offset.toInt()).take(pageRequest.pageSize), | ||
| pageRequest, | ||
| applications.size.toLong() | ||
| ) |
There was a problem hiding this comment.
메모리 기반 페이징으로 인한 성능 문제
전체 검색 결과를 메모리에 로드한 후 drop()과 take()로 수동 페이징하고 있습니다. 검색 결과가 수천~수만 건인 경우 모든 데이터를 메모리에 적재하므로 심각한 성능 저하와 메모리 부족이 발생할 수 있습니다.
데이터베이스 레벨에서 LIMIT/OFFSET을 적용하는 페이징으로 변경해야 합니다.
권장 개선 방안
Kotlin JDSL의 listQuery나 쿼리 DSL에서 제공하는 페이징 기능을 사용하여 데이터베이스 레벨 페이징을 구현하세요. 예:
val query = jpql {
select(entity(SignUpApplicationEntity::class))
.from(...)
.where(...)
.orderBy(...)
}
val pagedResults = query
.setFirstResult(pageRequest.offset.toInt())
.setMaxResults(pageRequest.pageSize)
.resultList
val count = // count query 실행
return PageImpl(pagedResults, pageRequest, count)또는 Spring Data JPA의 Specification이나 QueryDSL로 리팩터링하는 것도 고려할 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/kotlin/co/yappuworld/user/infrastructure/SignUpApplicationFindService.kt`
around lines 113 - 117, The current implementation in
SignUpApplicationFindService builds a full in-memory list named applications and
then uses applications.drop(...).take(...) to page, which causes OOM/perf
issues; replace this with a database-level paged query (e.g., use Kotlin JDSL
listQuery or the JPA Query/QueryDSL used in this class) to fetch only the page
by applying setFirstResult(pageRequest.offset.toInt()) and
setMaxResults(pageRequest.pageSize) (or the equivalent listQuery paging API),
execute a separate count query for the total, and return PageImpl with the
pagedResults, pageRequest, and the count instead of slicing the full
applications list.
Comment on lines
+35
to
+49
| applicant_name varchar(64), | ||
| status varchar(16) NOT NULL, | ||
| reject_reason varchar(128) | ||
| ); | ||
|
|
||
| DROP TABLE IF EXISTS sign_up_application_activity_unit; | ||
| CREATE TABLE sign_up_application_activity_unit | ||
| ( | ||
| id binary(16) PRIMARY KEY, | ||
| created_at datetime(6), | ||
| updated_at datetime(6), | ||
| application_id binary(16) NOT NULL, | ||
| generation int NOT NULL, | ||
| position varchar(16) NOT NULL | ||
| ); |
There was a problem hiding this comment.
Flyway DDL과 schema.sql 컬럼 길이가 불일치합니다.
Line 35(applicant_name)와 Line 48(position)의 길이가 Flyway V1과 달라 로컬/테스트와 운영의 데이터 허용 범위가 분리됩니다. 이 상태면 환경별로 저장 성공/실패가 달라질 수 있습니다.
🔧 제안 수정안
- applicant_name varchar(64),
+ applicant_name varchar(100),
@@
- position varchar(16) NOT NULL
+ position varchar(100) NOT NULL📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| applicant_name varchar(64), | |
| status varchar(16) NOT NULL, | |
| reject_reason varchar(128) | |
| ); | |
| DROP TABLE IF EXISTS sign_up_application_activity_unit; | |
| CREATE TABLE sign_up_application_activity_unit | |
| ( | |
| id binary(16) PRIMARY KEY, | |
| created_at datetime(6), | |
| updated_at datetime(6), | |
| application_id binary(16) NOT NULL, | |
| generation int NOT NULL, | |
| position varchar(16) NOT NULL | |
| ); | |
| applicant_name varchar(100), | |
| status varchar(16) NOT NULL, | |
| reject_reason varchar(128) | |
| ); | |
| DROP TABLE IF EXISTS sign_up_application_activity_unit; | |
| CREATE TABLE sign_up_application_activity_unit | |
| ( | |
| id binary(16) PRIMARY KEY, | |
| created_at datetime(6), | |
| updated_at datetime(6), | |
| application_id binary(16) NOT NULL, | |
| generation int NOT NULL, | |
| position varchar(100) NOT NULL | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/resources/schema.sql` around lines 35 - 49, The schema.sql DDL uses
varchar lengths for applicant_name and position that differ from Flyway V1,
causing environment-specific storage differences; update the column definitions
in this file (applicant_name in the first table and position in
sign_up_application_activity_unit) to use the exact varchar length values
defined in Flyway V1 so both migrations and the checked-in schema stay
consistent (ensure applicant_name and position match Flyway V1's lengths
exactly).
| private val properties: SignUpEmailLockProperties | ||
| ) : SignUpEmailLockExecutor { | ||
|
|
||
| private val locks = ConcurrentHashMap<String, ReentrantLock>() |
There was a problem hiding this comment.
locks 맵이 무제한으로 증가하는 메모리 누수가 있습니다.
ConcurrentHashMap에 이메일별 락이 추가되지만 제거되지 않아, 테스트가 진행될수록 메모리가 계속 증가합니다. 테스트 환경에서는 영향이 제한적일 수 있지만, 장기 실행 테스트나 많은 테스트 케이스가 있을 경우 문제가 될 수 있습니다.
약한 참조(WeakHashMap) 또는 주기적 정리 메커니즘을 고려해보세요.
제안하는 수정
- private val locks = ConcurrentHashMap<String, ReentrantLock>()
+ private val locks = Collections.synchronizedMap(WeakHashMap<String, ReentrantLock>())또는 테스트 종료 시 명시적으로 정리하는 메서드를 추가:
fun cleanup() {
locks.clear()
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/kotlin/co/yappuworld/support/lock/FakeSignUpEmailLockExecutor.kt` at
line 24, locks가 ConcurrentHashMap에 계속 쌓여 메모리 누수가 발생하니
FakeSignUpEmailLockExecutor의 locks 맵을 약한 참조 또는 정리 메커니즘으로 변경해 주세요; 예를 들어
ConcurrentHashMap 대신 WeakHashMap(또는 ConcurrentMap<WeakReference<String>,
ReentrantLock> 패턴)을 사용하거나, unlock 이후에 해당 키를 제거하도록 unlock 처리 로직에 제거 코드를 추가하고 테스트
종료 시 호출할 수 있는 cleanup() 메서드(예: fun cleanup() { locks.clear() })를 구현해 메모리 누수를
방지하세요.
eeun1225
dismissed
coderabbitai[bot]’s stale review
May 15, 2026 06:11
Automated review dismissed after manual verification. Build checks passed and migration changes were intentionally adjusted for the release branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Related Issue
🚨 해결하려는 문제가 무엇인가요?
⭐️ 어떻게 해결했나요?
🤔 어떤 부분에 집중하여 리뷰해야 할까요?
🗒️ 참고자료
RCA 룰
Summary by CodeRabbit
릴리스 노트
새 기능
테스트
설정