[FIX] 소셜 로그인 안정화 및 운영 환경 설정 개선 - #106
Conversation
📝 WalkthroughWalkthroughRedis 기반 OAuth 임시 코드 저장과 원자적 소비를 추가했습니다. 소셜 로그인 교환 상태 계산을 정리하고 Google audience 검증을 강화했습니다. Redis, Flyway, OAuth, AI 서버 설정과 데이터베이스 마이그레이션 구성을 변경했습니다. ChangesOAuth 및 운영 기반 변경
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthService
participant OAuthTempCodeStore
participant Redis
participant AuthTransactionService
Client->>AuthService: OAuth 임시 코드 교환 요청
AuthService->>OAuthTempCodeStore: 임시 코드 소비
OAuthTempCodeStore->>Redis: GETDEL 실행
Redis-->>OAuthTempCodeStore: 교환 데이터 반환
OAuthTempCodeStore-->>AuthService: TempExchangeData 반환
AuthService->>AuthTransactionService: 사용자 및 토큰 교환 요청
AuthTransactionService-->>AuthService: 로그인 응답 반환
AuthService-->>Client: 액세스 토큰 응답
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
…#101-auth # Conflicts: # src/main/resources/application.yml
p1001q
left a comment
p1001q
left a comment
There was a problem hiding this comment.
리뷰 요청하신 항목별 확인 결과 (전부 정독함)
✅ Redis TTL/일회성 소비: getAndDelete()(Redis GETDEL)로 원자적 조회+삭제 — 두 요청이 동시에 같은 코드를 소비하는 레이스 없음. 키도 원문이 아니라 SHA-256 해시로 저장해서 Redis 키 노출 방어까지 되어있어요. 잘 짜셨습니다.
✅ 동시 회원가입 충돌 처리: 이게 제일 인상 깊었는데, completeTokenExchange()가 이제 prepareSocialLogin()에서 스냅샷한 값을 그대로 안 믿고 교환 시점에 SocialAuth를 다시 fresh하게 조회해서, 그사이 다른 요청이 이미 가입을 완료했으면 그 유저를 그대로 씁니다. 그래도 진짜 동시에 INSERT가 겹치면 DataIntegrityViolationException을 그대로 전파시켜서 AuthService가 새 트랜잭션에서 executeSocialLoginForExistingUser()로 복구하는 구조까지 — 이거 제가 지난번 PR #82 리뷰 때 지적했던 "accessToken 경로엔 재시도 있는데 code-exchange 경로엔 없다"는 그 P2를 정확히 고치신 거네요. 테스트(completeTokenExchange_socialAuthCreatedAfterPrepare_usesExistingUser)도 이 시나리오 커버하고 있고요.
✅ Google aud 검증: ID Token(JWT 형태) fallback 경로에만 정확히 걸려있어요 — 일반 access_token 경로(/oauth2/v2/userinfo)나 인가코드 교환 경로는 이미 Google이 client_secret으로 검증해주니 audience 체크가 불필요한 게 맞고, 딱 필요한 곳에만 넣으셨습니다. catch (GeneralException) { throw; }로 aud mismatch 예외가 뒤쪽 catch-all에 삼켜지지 않게 처리한 것도 꼼꼼하네요.
✅ Flyway 마이그레이션: ADD CONSTRAINT UNIQUE가 중복 데이터 있으면 트랜잭션째로 실패하는 게 Postgres/Flyway 기본 동작이라 "실패하도록"이라는 정책 그대로 동작합니다. DROP INDEX IF EXISTS/CREATE INDEX IF NOT EXISTS도 재실행 안전하게 잘 짜셨고, 엔티티의 @UniqueConstraint/@Index 이름이 마이그레이션 SQL이랑 정확히 일치해서 ddl-auto: validate 전환해도 문제없을 것 같습니다.
✅ JWT 로그 노출 제거: 확인, 토큰 값 완전히 빠짐.
✅ Docker Compose: 로컬은 Redis를 127.0.0.1:6379로 호스트에 노출하고, 운영은 포트 노출 없이 내부 네트워크(SPRING_DATA_REDIS_HOST: redis)로만 접근 — 기존 analysis 서비스랑 같은 패턴이라 일관성 있고 운영 쪽이 더 안전하게 잘 되어있습니다.
결론: P2(AI 서버 base-url 기본값) 하나만 보완하면 나머지는 전부 깔끔합니다. 특히 동시성 처리는 지난 PR들에서 나온 지적사항들을 제대로 다 반영한 티가 나요.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/test/java/com/mr/domain/auth/service/OAuthClientServiceTest.java (1)
159-193: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winGoogle audience 거부 경로도 테스트하세요.
현재 테스트는 일치하는
aud의 성공 경로만 검증합니다.aud불일치 또는 누락 시OAUTH_CLIENT_ERROR가 발생하는 테스트를 추가하세요. GoogleclientId누락 시OAUTH_SERVER_ERROR가 발생하는 테스트도 추가하세요. 이 테스트는 audience 검증의 보안 동작 회귀를 방지합니다.🤖 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/java/com/mr/domain/auth/service/OAuthClientServiceTest.java` around lines 159 - 193, Extend getGoogleUserInfo_idToken_validatesAudience coverage with rejection cases: when Google tokeninfo returns a mismatched or missing aud, assert OAUTH_CLIENT_ERROR, and when the configured Google clientId is missing, assert OAUTH_SERVER_ERROR. Reuse the existing OAuthClientService setup and mock-server flow while preserving the current matching-audience success assertion.src/test/java/com/mr/domain/auth/service/OAuthTempCodeStoreTest.java (1)
33-37: 🚀 Performance & Scalability | 🔵 Trivial실제 프로퍼티 바인딩을 검증하는 테스트를 추가하는 것을 권장합니다.
이 테스트는
Duration.ofMinutes(2)를 생성자에 직접 전달합니다. 그래서application.yml의${oauth.temp-code-ttl:2m}문자열이 실제로@Value를 통해 올바른Duration으로 변환되는지는 검증하지 못합니다.
@SpringBootTest(properties = "oauth.temp-code-ttl=2m")처럼 실제 스프링 컨텍스트를 띄워 프로퍼티 바인딩 결과를 확인하는 테스트를 하나 추가하면, 이 리스크를 사전에 잡을 수 있습니다. 관련 배경은OAuthTempCodeStore.java의 생성자 리뷰 코멘트를 참고해 주세요.🤖 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/java/com/mr/domain/auth/service/OAuthTempCodeStoreTest.java` around lines 33 - 37, OAuthTempCodeStoreTest에 실제 Spring 프로퍼티 바인딩을 검증하는 테스트를 추가하세요. 기존 setUp의 직접적인 Duration.ofMinutes(2) 주입 테스트는 유지하되, `@SpringBootTest`(properties = "oauth.temp-code-ttl=2m")로 컨텍스트를 구성하고 application 프로퍼티가 OAuthTempCodeStore의 Duration 값으로 변환·주입되는지 확인하세요.
🤖 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/java/com/mr/domain/auth/service/OAuthTempCodeStore.java`:
- Around line 28-39: Update the `@Value` default in OAuthTempCodeStore to the
standard ISO-8601 duration PT2M instead of 2m, while preserving the existing TTL
validation and constructor injection behavior. Add or update Spring context
coverage to verify property parsing for the default and an injected
OAUTH_TEMP_CODE_TTL value.
In
`@src/main/resources/db/migration/V1__add_mentor_chat_constraints_and_indexes.sql`:
- Around line 3-10: Before deploying the migration, validate
mentor_chat_sessions.analysis_id for duplicate non-null values using the
specified GROUP BY/HAVING query and clean up any duplicates as needed; then
rerun the migration so the uk_mentor_chat_sessions_analysis_id constraint can be
created successfully.
- Around line 9-10: V1__add_mentor_chat_constraints_and_indexes.sql의 UNIQUE 제약조건
추가, 인덱스 삭제, 일반 인덱스 생성을 런타임 Flyway 마이그레이션과 분리하세요. application-prod.yml의 자동 Flyway
실행 경로에서 대규모 테이블 잠금과 쓰기 차단이 발생하지 않도록 해당 DDL을 배포 전 별도 운영 절차로 이동하고, 마이그레이션에는 애플리케이션
시작 시 안전하게 실행될 변경만 남기세요.
---
Nitpick comments:
In `@src/test/java/com/mr/domain/auth/service/OAuthClientServiceTest.java`:
- Around line 159-193: Extend getGoogleUserInfo_idToken_validatesAudience
coverage with rejection cases: when Google tokeninfo returns a mismatched or
missing aud, assert OAUTH_CLIENT_ERROR, and when the configured Google clientId
is missing, assert OAUTH_SERVER_ERROR. Reuse the existing OAuthClientService
setup and mock-server flow while preserving the current matching-audience
success assertion.
In `@src/test/java/com/mr/domain/auth/service/OAuthTempCodeStoreTest.java`:
- Around line 33-37: OAuthTempCodeStoreTest에 실제 Spring 프로퍼티 바인딩을 검증하는 테스트를
추가하세요. 기존 setUp의 직접적인 Duration.ofMinutes(2) 주입 테스트는 유지하되,
`@SpringBootTest`(properties = "oauth.temp-code-ttl=2m")로 컨텍스트를 구성하고 application
프로퍼티가 OAuthTempCodeStore의 Duration 값으로 변환·주입되는지 확인하세요.
🪄 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 Plus
Run ID: 27ef568f-4542-4dd7-9750-233145f9f5d3
📒 Files selected for processing (21)
MR_config/local/application.example.ymlbuild.gradledocker-compose.local.ymldocker-compose.ymlsrc/main/java/com/mr/domain/auth/dto/res/GoogleUserResponse.javasrc/main/java/com/mr/domain/auth/exception/AuthErrorStatus.javasrc/main/java/com/mr/domain/auth/service/AuthService.javasrc/main/java/com/mr/domain/auth/service/AuthTransactionService.javasrc/main/java/com/mr/domain/auth/service/OAuthClientService.javasrc/main/java/com/mr/domain/auth/service/OAuthTempCodeStore.javasrc/main/java/com/mr/global/config/AiServerProperties.javasrc/main/java/com/mr/global/security/jwt/JwtAuthenticationFilter.javasrc/main/resources/application-dev.ymlsrc/main/resources/application-prod.ymlsrc/main/resources/application.ymlsrc/main/resources/db/migration/V1__add_mentor_chat_constraints_and_indexes.sqlsrc/test/java/com/mr/domain/auth/service/AuthServiceTest.javasrc/test/java/com/mr/domain/auth/service/AuthTransactionServiceTest.javasrc/test/java/com/mr/domain/auth/service/OAuthClientServiceTest.javasrc/test/java/com/mr/domain/auth/service/OAuthTempCodeStoreTest.javasrc/test/java/com/mr/global/config/AiServerPropertiesBindingTest.java
| public OAuthTempCodeStore( | ||
| StringRedisTemplate redisTemplate, | ||
| ObjectMapper objectMapper, | ||
| @Value("${oauth.temp-code-ttl:2m}") Duration ttl | ||
| ) { | ||
| this.redisTemplate = redisTemplate; | ||
| this.objectMapper = objectMapper; | ||
| if (ttl == null || ttl.isZero() || ttl.isNegative()) { | ||
| throw new IllegalArgumentException("OAuth 임시 코드 TTL은 0보다 커야 합니다."); | ||
| } | ||
| this.ttl = ttl; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: 코드베이스에 이미 `@Value로` Duration을 주입하는 다른 사례가 있는지 확인합니다.
rg -n -B1 -A1 '`@Value`.*Duration' --type=javaRepository: Musereview/BE
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== build files =='
fd '^(pom.xml|build.gradle|build.gradle.kts|gradle.properties)$' . -x sh -c 'echo "--- $1"; sed -n "1,220p" "$1"' sh {}
echo
echo "== candidate java files =='
fd -e java . src/main/java src/test/java | rg 'OAuthTempCodeStore|Duration|Properties|ConfigurationProperties|Test' || true
echo
echo "== relevant searches =='
rg -n '`@Value`|Duration|OAuthTempCodeStore|oauth\.temp-code-ttl|OAUTH_TEMP_CODE_TTL|`@ConfigurationProperties`|Relaxed' --type=java . || trueRepository: Musereview/BE
Length of output: 422
🌐 Web query:
Spring Framework @valueDuration parsing PT10S "10s" "2m" relaxed binding@ConfigurationProperties``
💡 Result:
In Spring Boot, Duration parsing is a native feature that allows you to specify time-based properties using human-readable strings, which are then automatically converted into java.time.Duration objects [1]. Key concepts regarding Duration parsing: 1. Supported Formats: Spring Boot supports various suffixes to represent units of time, including ns (nanoseconds), us (microseconds), ms (milliseconds), s (seconds), m (minutes), h (hours), and d (days) [2][3]. For example, 10s, 2m, and 500ms are all valid [1][4][5]. If no suffix is provided, the default unit is typically milliseconds, though this can vary depending on the specific property or context [2]. 2. @ConfigurationProperties vs @Value: - @ConfigurationProperties is the recommended, type-safe way to bind properties [1][6]. When you define a field of type java.time.Duration in a class annotated with @ConfigurationProperties, Spring Boot automatically converts the string value from your application properties (e.g., '10s') into the corresponding Duration object [1]. - @Value can also be used for simple cases, but it lacks the full benefits of type-safe prefix binding and relaxed binding rules that @ConfigurationProperties provides. 3. Relaxed Binding: Spring Boot's relaxed binding rules apply to @ConfigurationProperties, allowing you to match property names in different formats (e.g., kebab-case, snake_case, or camelCase) to your Java fields [1]. This ensures that, for instance, a property defined as 'connect-timeout' in application.properties can be correctly mapped to a field named 'connectTimeout' in your configuration class [1]. Because Spring Boot handles this conversion automatically for java.time.Duration targets, you do not need to manually parse strings in your code [1]. Simply defining the target field as a Duration object is sufficient for the framework to perform the conversion [1].
Citations:
- 1: https://medium.com/@sachinverma_78701/mastering-spring-restclient-setup-timeouts-headers-params-9bfebf728497
- 2: https://marketaylor.synology.me/?p=668
- 3: https://github.com/neo4j/neo4j/blob/c68156ed/community/configuration/src/test/java/org/neo4j/configuration/SettingTest.java
- 4: https://runebook.dev/en/docs/spring_boot/application-properties/application-properties.core.spring.reactor.netty.shutdown-quiet-period
- 5: https://runebook.dev/en/docs/spring_boot/application-properties/application-properties.actuator.management.dynatrace.metrics.export.connect-timeout
- 6: https://www.pluralsight.com/courses/spring-boot-efficient-development-configuration-deployment
🌐 Web query:
"Unable to resolve duration" "@value" "10s" Duration Spring 5 @value 10s "PT10S"
💡 Result:
The error occurs because Spring's standard @Value annotation does not automatically support the direct conversion of String-based property values (like "10s") into java.time.Duration objects, unlike the @ConfigurationProperties mechanism [1][2]. To resolve this, you have three primary options: 1. Use @ConfigurationProperties (Recommended) The most robust, "Spring Boot-idiomatic" way to handle durations is to use @ConfigurationProperties. This mechanism natively supports relaxed binding and automatic conversion of strings (like "10s", "PT10S", or "1000ms") into Duration objects [3][4]. 2. Use SpEL (Spring Expression Language) If you must use @Value, you can use SpEL to parse the value manually [3][5]. For example: @Value("#{T(java.time.Duration).parse('PT10S')}") private Duration duration; Or, to handle a property with a default value: @Value("${my.property:#{T(java.time.Duration).ofSeconds(10)}}") private Duration duration; 3. Expose a ConversionService Bean You can manually register a ConversionService bean that includes Spring Boot's ApplicationConversionService. This service contains the necessary converters to handle Duration strings [3]. @Bean public ConversionService conversionService { return ApplicationConversionService.getSharedInstance; } Note: While some versions of Spring have seen pull requests to add direct support for @Value duration conversion [6], consistency and best practices in Spring Boot favor @ConfigurationProperties for type-safe, complex configuration binding [4].
Citations:
- 1: java.time.Duration properties can't be injected through @Value spring-projects/spring-boot#13237
- 2: Not able to use java.util.Duration as @Value configuration in Spring Boot spring-projects/spring-boot#11306
- 3: https://stackoverflow.com/questions/51818137/spring-boot-2-converting-duration-java-8-application-properties
- 4: https://rwinch.github.io/spring-boot/features/external-config/typesafe-configuration-properties/conversion.html
- 5: https://stackoverflow.com/questions/62062005/default-value-as-duration-at-value
- 6: Introduce DurationEditor for java.time.Duration support spring-projects/spring-framework#28084
🌐 Web query:
Spring Framework @value Duration parsing "10s" "PT10S"
💡 Result:
In the Spring Framework and Spring Boot, when injecting a java.time.Duration into a field using @Value or configuration binding, both "10s" (simplified format) and "PT10S" (ISO-8601 format) are supported, though they are handled by different underlying mechanisms [1][2]. The two formats behave as follows: 1. Simplified Format (e.g., "10s", "1m", "2h"): This is the human-readable format commonly favored in Spring Boot configuration properties (application.properties or application.yml) [1][3][2]. It is processed by Spring's conversion services, which allow developers to define durations intuitively [2]. 2. ISO-8601 Format (e.g., "PT10S"): This is the standard format required by the java.time.Duration.parse method [1][4]. Spring's converters typically support this format as well, treating it as the standard representation of a duration [1][2]. Key Technical Considerations: - Underlying Parsing: When using @Value, Spring leverages its ConversionService to transform string configuration values into Duration objects [2]. If you are using Spring Framework (without Spring Boot), support for these conversions was improved in recent versions to handle both the simplified and ISO-8601 formats directly [1]. - IDE Validation: A common issue when using the simplified "10s" format is that IDEs (like IntelliJ) may flag it as an error or warning [4]. This often occurs because the IDE's validation logic may attempt to parse the string using standard java.time.Duration.parse, which strictly requires the ISO-8601 "PT" prefix [4]. While the code will execute correctly at runtime using Spring's custom converters, the IDE might not always recognize the custom conversion rules applied by Spring [4]. - Recommendation: Use the simplified format (e.g., "10s") for better readability in your configuration files, as it is the idiomatic standard within the Spring ecosystem [1][2]. If you encounter IDE validation warnings, verify that your project settings are configured to use Spring's conversion context rather than generic ISO-8601 parsing [4].
Citations:
- 1: Introduce DurationEditor for java.time.Duration support spring-projects/spring-framework#28084
- 2: https://medium.com/@AlexanderObregon/working-with-conversionservice-for-type-safe-configuration-binding-in-spring-boot-a394bc6df1d6
- 3: Support "Spring" format for Duration values in resource YAML/JSON spinnaker/keel#521
- 4: IDE false-positive warnings for custom properties in application.properties cuioss/TokenSheriff#182
@Value의 Duration 기본값을 표준 ISO-8601로 선언하세요.
@ConfigurationProperties와 달리 @Value의 Duration 변환은 Spring/Boot 컨텍스트에 의존할 수 있어 2m 기본값이 깨질 수 있습니다. 배포 전에는 실제 OAUTH_TEMP_CODE_TTL=2m 값을 주입해 기동 검증이 필요합니다. 값 계약은 PT2M 같은 ISO-8601로 작성하고, 현재 기본값도 @Value("${oauth.temp-code-ttl:PT2M}")로 맞추는 편이 안전합니다. 테스트는 생성자 주입만 하므로, @SpringBootTest로 프로퍼티 파싱을 함께 검증하세요.
🤖 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/java/com/mr/domain/auth/service/OAuthTempCodeStore.java` around
lines 28 - 39, Update the `@Value` default in OAuthTempCodeStore to the standard
ISO-8601 duration PT2M instead of 2m, while preserving the existing TTL
validation and constructor injection behavior. Add or update Spring context
coverage to verify property parsing for the default and an injected
OAUTH_TEMP_CODE_TTL value.
| IF NOT EXISTS ( | ||
| SELECT 1 | ||
| FROM pg_constraint | ||
| WHERE conname = 'uk_mentor_chat_sessions_analysis_id' | ||
| AND conrelid = 'mentor_chat_sessions'::regclass | ||
| ) THEN | ||
| ALTER TABLE mentor_chat_sessions | ||
| ADD CONSTRAINT uk_mentor_chat_sessions_analysis_id UNIQUE (analysis_id); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repository files of interest =="
git ls-files | rg '(^|/)(V1__add_mentor_chat_constraints_and_indexes\.sql|application-prod\.ya?ml|application\.ya?ml)$' || true
echo
echo "== migration content =="
if [ -f src/main/resources/db/migration/V1__add_mentor_chat_constraints_and_indexes.sql ]; then
cat -n src/main/resources/db/migration/V1__add_mentor_chat_constraints_and_indexes.sql
fi
echo
echo "== prod yaml snippets =="
for f in $(git ls-files | rg '(^|/)application-prod\.ya?ml$'); do
echo "--- $f ---"
sed -n '1,80p' "$f" | cat -n
done
if [ -f src/main/resources/application.yml ]; then
echo "--- src/main/resources/application.yml ---"
sed -n '1,120p' src/main/resources/application.yml | cat -n
fi
echo
echo "== all Flyway config occurrences =="
rg -n "flyway|flyway-baseline|baseline|enabled" src/main/resources || true
echo
echo "== schema/table references =="
rg -n "mentor_chat_sessions|uk_mentor_chat_sessions_analysis_id|analysis_id" src/main/resources || trueRepository: Musereview/BE
Length of output: 6653
🌐 Web query:
PostgreSQL ALTER TABLE ADD UNIQUE constraint checks existing rows documentation
💡 Result:
In PostgreSQL, when you use the ALTER TABLE command to add a UNIQUE constraint, the database automatically performs a scan of the existing rows in the table to verify that the constraint is satisfied [1]. The constraint is checked immediately upon execution, and the command will fail if the table data does not satisfy the uniqueness requirement [1]. Key points regarding this process include: 1. Immediate Enforcement: Unlike some other constraint types (such as foreign keys or CHECK constraints) that support a NOT VALID option to delay or skip initial verification, a standard ADD UNIQUE constraint requires immediate validation [2][3][4]. 2. Index Creation: Adding a UNIQUE constraint inherently requires the creation of a unique index [5]. The system must scan the existing table data to build this index and ensure no duplicate values exist. 3. Performance Consideration: Because it requires a table scan, adding a UNIQUE constraint on a very large table can be time-consuming and may acquire locks that block concurrent updates for the duration of the command [3]. If you already have a unique index that covers the desired columns, you can use the syntax ALTER TABLE table_name ADD CONSTRAINT constraint_name UNIQUE USING INDEX index_name [5]. This method avoids the need for a full table scan to build a new index, as the existing index is adopted by the constraint [5].
Citations:
- 1: https://www.postgresql.org/docs/18/ddl-alter.html
- 2: https://www.postgresql.org/docs/18/sql-altertable.html
- 3: https://www.postgresql.org/docs/19/sql-altertable.html
- 4: https://www.postgresql.org/docs/16/sql-altertable.html
- 5: https://www.postgresql.org/docs/15/sql-altertable.html
mentor_chat_sessions.analysis_id의 중복 데이터를 배포 전에 정리하세요.
ALTER TABLE mentor_chat_sessions ADD CONSTRAINT ... UNIQUE (analysis_id)는 실행 시 기존 행의 중복값을 검사하고, 중복이 있으면 마이그레이션이 실패합니다. Flyway baseline은 중복 데이터 검증이나 정리를 하지 않습니다. 배포 전 다음 쿼리로 중복이 있는지 확인하고, 필요하면 정제한 뒤 마이그레이션을 재실행하세요.
SELECT analysis_id, COUNT(*)
FROM mentor_chat_sessions
GROUP BY analysis_id
HAVING COUNT(*) > 1;관련 문서: PostgreSQL ALTER TABLE UNIQUE constraint documentation
🤖 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/db/migration/V1__add_mentor_chat_constraints_and_indexes.sql`
around lines 3 - 10, Before deploying the migration, validate
mentor_chat_sessions.analysis_id for duplicate non-null values using the
specified GROUP BY/HAVING query and clean up any duplicates as needed; then
rerun the migration so the uk_mentor_chat_sessions_analysis_id constraint can be
created successfully.
| ALTER TABLE mentor_chat_sessions | ||
| ADD CONSTRAINT uk_mentor_chat_sessions_analysis_id UNIQUE (analysis_id); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
: "${DATABASE_URL:?DATABASE_URL is required}"
psql "$DATABASE_URL" -v ON_ERROR_STOP=1 <<'SQL'
SELECT relname AS table_name,
pg_size_pretty(pg_total_relation_size(oid)) AS total_size
FROM pg_class
WHERE relname IN ('mentor_chat_sessions', 'mentor_message');
SELECT schemaname, tablename, indexname, indexdef
FROM pg_indexes
WHERE tablename IN ('mentor_chat_sessions', 'mentor_message')
ORDER BY tablename, indexname;
SQLRepository: Musereview/BE
Length of output: 204
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Changed files/stat:\n'
git diff --stat || true
printf '\nMigration snippet:\n'
if [ -f src/main/resources/db/migration/V1__add_mentor_chat_constraints_and_indexes.sql ]; then
cat -n src/main/resources/db/migration/V1__add_mentor_chat_constraints_and_indexes.sql
else
fd -i 'V1__add_mentor_chat_constraints_and_indexes' . -x sh -c 'echo "--- {}"; cat -n "{}"' \;
fi
printf '\nApplication YAML references:\n'
fd -i 'application.*\.ya?ml$' . -x sh -c 'echo "--- {}"; rg -n "flyway|spring\.flyway|executeIn|enabled|ddl-auto|lock" "{}" || true' \;
printf '\nRelevant db/migration files names:\n'
git ls-files 'src/main/resources/db/migration/*' | sortRepository: Musereview/BE
Length of output: 1382
🌐 Web query:
Flyway Spring Boot transactionalExecute executeInTransaction enabled by default PostgreSQL migration transaction behavior
💡 Result:
In Flyway, the property executeInTransaction determines whether migration scripts are executed within a database transaction [1][2]. The behavior regarding this setting is distinct from how PostgreSQL manages internal locks, and understanding the difference is critical for operations like CREATE INDEX CONCURRENTLY [3][4]. 1. Transactional Execution Setting (executeInTransaction) Flyway attempts to wrap each migration script in a transaction by default [2]. In Spring Boot, you can configure this via the spring.flyway.execute-in-transaction property [5][6]. While Flyway defaults to true for transactional execution, users should be aware that Spring Boot may have historically had different alignment for this default value [7]. You can manually override this for specific scripts using a script configuration file (e.g., V1script.sql.conf) by setting executeInTransaction=false [1][2]. 2. PostgreSQL-Specific Locking Behavior It is important to distinguish executeInTransaction from Flyway's PostgreSQL transactional locking mechanism [3]. By default, Flyway uses a transactional advisory lock to ensure consistency across multiple connections [3][4]. This lock is independent of the execution of individual migration scripts [3]. 3. Special Cases (CREATE INDEX CONCURRENTLY) Certain PostgreSQL commands, such as CREATE INDEX CONCURRENTLY, cannot run inside a standard transaction block [2][4]. If you attempt to run these in a transaction, the migration will fail. For these scenarios: - Disable transactional locks: Because Flyway's default transactional lock might interfere with these commands, you must set flyway.postgresql.transactional.lock=false in your configuration [3][8][4]. This switches Flyway to using session-level locks [8][4]. - Manage transaction blocks: If you have a script containing statements that cannot run in a transaction, you may also need to set executeInTransaction=false or configure the mixed property to true to allow mixed transactional and non-transactional statements within the same migration [2][9]. Summary of Key Settings: - For per-migration control: Use executeInTransaction=false in your script configuration file [1][2]. - For global PostgreSQL lock compatibility: Use flyway.postgresql.transactional.lock=false in your Flyway configuration [8][4].
Citations:
- 1: https://documentation.red-gate.com/flyway/reference/configuration/flyway-namespace/flyway-execute-in-transaction-setting
- 2: https://documentation.red-gate.com/flyway/flyway-concepts/migrations/migration-transaction-handling
- 3: PostgresSQL transaction locks even when disabling transactions flyway/flyway#3492
- 4: https://documentation.red-gate.com/flyway/reference/database-driver-reference/postgresql-database
- 5: https://docs.spring.io/spring-boot/api/java/org/springframework/boot/flyway/autoconfigure/FlywayProperties.html
- 6: https://docs.enterprise.spring.io/spring-boot/docs/3.2.15/api/org/springframework/boot/autoconfigure/flyway/FlywayProperties.html
- 7: Default for spring.flway.execute-in-transaction is not aligned with Flyway's default spring-projects/spring-boot#35447
- 8: https://documentation.red-gate.com/flyway/reference/configuration/flyway-namespace/flyway-postgresql-namespace/flyway-postgresql-transactional-lock-setting
- 9: https://documentation.red-gate.com/flyway/reference/configuration/flyway-namespace/flyway-mixed-setting
🌐 Web query:
Flyway ALTER TABLE DROP CONSTRAINT lock PostgreSQL migration large transactions
💡 Result:
Performing an ALTER TABLE DROP CONSTRAINT operation in PostgreSQL on a large table is a metadata-intensive task that requires an ACCESS EXCLUSIVE lock [1][2][3]. While the actual execution time for the DDL command is typically very fast (often measured in milliseconds), the requirement to acquire an ACCESS EXCLUSIVE lock means it will block—and be blocked by—all other concurrent operations on that table, including simple SELECT statements [4][2]. To manage this safely in production, especially with Flyway, consider the following strategies: 1. Use lock_timeout: The most effective safety measure is to set a short lock_timeout immediately before the command [2][5]. This ensures that if the migration cannot acquire the required lock within a few seconds (e.g., because of long-running queries), it will fail immediately rather than waiting indefinitely and creating a queue of blocked queries that can degrade or crash your application [2][5]. SET lock_timeout = '5s'; ALTER TABLE my_table DROP CONSTRAINT my_constraint; 2. Migration Tooling Configuration: Flyway by default wraps migrations in a transactional lock to maintain consistency [6]. While this is generally desirable, it can interfere with certain operations if not configured correctly. For standard DROP CONSTRAINT operations, the default transactional behavior is usually acceptable [6]. However, if you are performing complex operations that require disabling transactions, be aware that you may need to manage Flyway's transactional locks globally or via configuration (e.g., flyway.postgresql.transactional.lock=false) [6][7][8]. 3. Understand Lock Contention: Even for empty tables, the command will hang if other transactions (even idle or read-only ones) hold locks on the table [4]. Always verify that there are no long-running transactions active on the target table before running the migration [4][9]. 4. Expand/Contract Pattern: If you are concerned about application impact, ensure that removing the constraint is backward-compatible [5][9]. If the constraint is a foreign key, dropping it does not require a complex multi-step process like adding one, but you should still ensure that no application code still relies on that constraint's enforcement before removal [9]. By combining a set local lock_timeout with monitoring for active transactions, you can significantly reduce the risk of downtime or performance degradation caused by lock contention [2][5].
Citations:
- 1: https://www.postgresql.org/docs/19/sql-altertable.html
- 2: https://monpg.app/blog/postgresql-alter-table-locks
- 3: https://www.postgresql.org/docs/18/sql-altertable.html
- 4: https://stackoverflow.com/questions/17605511/why-would-alter-table-drop-constraint-on-an-empty-table-take-a-long-time
- 5: https://www.michal-drozd.com/en/blog/zero-downtime-postgresql-migrations/
- 6: https://documentation.red-gate.com/flyway/reference/database-driver-reference/postgresql-database
- 7: Transactional Lock False not working anymore for PostgreSQL flyway/flyway#3684
- 8: https://stackoverflow.com/questions/20350501/how-do-i-create-indexes-concurrently-with-flyway-in-postgres
- 9: https://mdsanwarhossain.me/blog-zero-downtime-database-migration-flyway-liquibase.html
🌐 Web query:
PostgreSQL CREATE INDEX locks table writes ALTER TABLE ADD UNIQUE constraint creates index locks writes ADD CONSTRAINT NOT NULL
💡 Result:
In PostgreSQL, database operations like creating indexes or adding constraints have different locking behaviors that impact concurrent read/write operations [1][2][3]. CREATE INDEX A standard CREATE INDEX command acquires a SHARE lock [2]. This allows concurrent reads but blocks all write operations (INSERT, UPDATE, DELETE) for the duration of the index build [1][2]. To avoid this, you should use the CONCURRENTLY option [1][4]. CREATE INDEX CONCURRENTLY acquires a SHARE UPDATE EXCLUSIVE lock, which permits concurrent reads and writes, though the operation takes longer to complete and performs more total work [1][2]. ADD UNIQUE CONSTRAINT Adding a unique constraint directly (e.g., ALTER TABLE ADD CONSTRAINT UNIQUE) acquires an ACCESS EXCLUSIVE lock, which blocks all access—including SELECTs—until the operation finishes [5][6][7]. To avoid downtime on production tables, use a two-step process: 1. Build the unique index first using CREATE UNIQUE INDEX CONCURRENTLY [8][9]. 2. Add the constraint using that existing index (ALTER TABLE ADD CONSTRAINT... USING INDEX...) [5][8]. While the final ALTER TABLE step still requires an ACCESS EXCLUSIVE lock, it only performs a metadata change and completes near-instantaneously, provided there are no long-running transactions blocking the lock [5][8][9]. ADD CONSTRAINT NOT NULL Standard SET NOT NULL or ADD CONSTRAINT NOT NULL operations acquire an ACCESS EXCLUSIVE lock and perform a full table scan to verify that no NULL values exist, blocking all reads and writes [10][3][11]. To minimize disruption, you can use the NOT VALID option [3][11]: 1. Add the constraint as NOT VALID (ALTER TABLE ADD CONSTRAINT... NOT NULL... NOT VALID). This acquires a brief ACCESS EXCLUSIVE lock but does not scan the table, allowing the operation to complete quickly [11]. 2. Validate the constraint separately using VALIDATE CONSTRAINT [3][12][11]. This step scans the table to verify existing data but uses a SHARE UPDATE EXCLUSIVE lock, which allows concurrent reads and writes to continue [3][12][11].
Citations:
- 1: https://www.postgresql.org/docs/19/sql-createindex.html
- 2: https://www.bytebase.com/blog/postgres-create-index-concurrently/
- 3: https://www.postgresql.org/docs/19/sql-altertable.html
- 4: https://www.postgresql.org/docs/17/sql-createindex.html
- 5: https://stackoverflow.com/questions/76156967/postgresql-add-unique-constraint-to-existing-table-without-downtime-locking
- 6: https://dev.to/mickelsamuel/which-alter-table-operations-lock-your-postgresql-table-1082
- 7: https://neon.com/blog/postgres-locks-neon-atlas
- 8: https://marcelofern.com/posts/postgres/unique-constraints-without-downtime/index.html
- 9: https://medium.com/@raminorujov/postgres-story-how-to-efficiently-implement-a-unique-constraint-with-minimum-locks-aa3f72b8cf1b
- 10: https://www.postgresql.org/docs/18/sql-altertable.html
- 11: https://neon.com/postgresql/18/not-null-as-not-valid
- 12: https://www.postgresql.org/docs/16/sql-altertable.html
🌐 Web query:
Flyway migrations run at application startup Spring Boot application-local migration table lock behavior
💡 Result:
When a Spring Boot application starts, Flyway automatically attempts to migrate the database schema [1]. To prevent race conditions—especially in clustered or scaled environments—Flyway acquires an exclusive lock on the flyway_schema_history table before executing any migrations [1]. Key aspects of this lock behavior include: 1. Mechanism: Flyway typically uses an exclusive lock (such as SELECT... FOR UPDATE or database-specific advisory locks) on the schema history table [2][1]. This ensures that only one application instance can run migrations at a time [1]. If another instance attempts to start simultaneously, it will either wait for the lock to be released or fail, depending on the configuration [1]. 2. Transactional Locks (PostgreSQL): Since Flyway 9.1, the default behavior for PostgreSQL is to use transactional advisory locks [2][3]. This lock is held within a transaction that remains open for the duration of the migration process [2]. 3. Potential Conflicts: Because the transactional lock is held within a transaction, it can cause issues for SQL statements that cannot run inside a transaction block, such as CREATE INDEX CONCURRENTLY in PostgreSQL [2][3]. If you encounter such issues, you can disable transactional locks by setting the following property in your application.properties or application.yml [4][5][3]: spring.flyway.postgresql.transactional-lock: false 4. Configuration and Retries: If multiple instances start and compete for the lock, Flyway provides a retry mechanism [6]. You can control how many times Flyway attempts to acquire the lock before giving up using the lock-retry-count property (default is 50) [7][5][8]: spring.flyway.lock-retry-count: 50 In summary, Flyway's locking is a safety mechanism to ensure data consistency during startup [1]. While the default transactional locking is generally preferred, it can be disabled if your specific migration scripts (like concurrent index creation) require operations that are incompatible with long-running transactions [2][3].
Citations:
- 1: https://medium.com/@AlexanderObregon/using-spring-boot-with-flyway-to-manage-database-migrations-8180ce0c9230
- 2: https://goldlapel.com/grounds/spring-java/flyway-create-index-concurrently-hang
- 3: Transactional locks hanging for non-transactional migrations flyway/flyway#3497
- 4: Provide a configuration property for configuring Flyway's use of transactional locks with PostgreSQL spring-projects/spring-boot#32629
- 5: https://github.com/spring-projects/spring-boot/blob/v3.2.12/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayProperties.java
- 6: https://github.com/flyway/flyway/blob/main/flyway-core/src/main/java/org/flywaydb/core/internal/database/InsertRowLock.java
- 7: https://docs.spring.io/spring-boot/api/java/org/springframework/boot/flyway/autoconfigure/FlywayProperties.html
- 8: https://github.com/spring-projects/spring-boot/blob/v3.0.1/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayProperties.java
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\napplication-prod.yml relevant section:\n'
sed -n '1,80p' src/main/resources/application-prod.yml | cat -n
printf '\napplication.yml relevant section:\n'
sed -n '1,80p' src/main/resources/application.yml | cat -n
printf '\nSearch for Flyway transactional-lock / executeIn / transaction settings:\n'
rg -n "flyway|transactional-lock|execute-in-transaction|executeIn" src/main/resources || trueRepository: Musereview/BE
Length of output: 4183
운영 트래픽에 맞는 Flyway DDL 실행 구조로 분리하세요.
V1__add_mentor_chat_constraints_and_indexes.sql은 ALTER TABLE ... ADD CONSTRAINT UNIQUE, DROP INDEX, 일반 CREATE INDEX를 포함합니다. 이 작업들도 PostgreSQL에서 상당한 잠금 또는 쓰기 차단이 발생할 수 있습니다. 특히 Flyway는 기본값으로 런타임 마이그레이션을 트랜잭션에서 실행하므로, mentor_chat_sessions 크기가 크면 앱 시작 대기에 영향을 줄 수 있습니다. src/main/resources/application-prod.yml에 Flyway 활성화가 있어 자동 실행 경로입니다.
🤖 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/db/migration/V1__add_mentor_chat_constraints_and_indexes.sql`
around lines 9 - 10, V1__add_mentor_chat_constraints_and_indexes.sql의 UNIQUE
제약조건 추가, 인덱스 삭제, 일반 인덱스 생성을 런타임 Flyway 마이그레이션과 분리하세요. application-prod.yml의 자동
Flyway 실행 경로에서 대규모 테이블 잠금과 쓰기 차단이 발생하지 않도록 해당 DDL을 배포 전 별도 운영 절차로 이동하고, 마이그레이션에는
애플리케이션 시작 시 안전하게 실행될 변경만 남기세요.
Source: Linters/SAST tools
4 participants
📍 개요
⛓️💥 관련 이슈
🛠️ 작업 내용
aud클레임 검증 추가AI_INTERNAL_BASE_URL환경변수 필수화validate로 변경하고 dev에서만update사용analysis_id유니크 제약조건 추가🔥 리뷰 요청 사항
aud검증 및 OAuth 예외 코드 분류가 적절한지analysis_id가 존재할 경우 유니크 제약조건 마이그레이션이 실패하도록 한 정책이 적절한지✅ 체크리스트
📎 참고 사항
KAKAO_REDIRECT_URIGOOGLE_REDIRECT_URIAI_INTERNAL_BASE_URLmentor_chat_sessions.analysis_id중복 여부를 확인해야 합니다.0을 사용합니다.PlayingServiceTest의 Mockito unnecessary stubbing 문제입니다.Summary by CodeRabbit
새로운 기능
버그 수정
개선