Repository navigation
[Volume-1] User 도메인 구현 (TDD + DDD 적용) - #39
Merged
Merged
Conversation
chore: 테크 노트 작성용 이슈 템플릿 4종 추가
- 4개 마크다운 템플릿(.md)을 GitHub Issue Forms(.yml)로 변환 - 학습자가 빈 마크다운 페이지가 아닌 섹션별 입력 박스로 작성하도록 UX 개선 - 필수 필드(TL;DR, 대상 독자, 관련 PR, 본문 섹션) 검증 적용 - 제목 prefill '[포맷] 키워드 (N주차 · K팀 · 이름)' 유지 - 라벨 'tech-note' + 'format:*' 자동 부여 (rt는 코치가 선별 부여) - blank issue 비활성화로 분류되지 않은 이슈 차단 - README를 폼 UX 중심으로 갱신 + 제목 규칙·라벨 안내 포함
* chore: 폼 필드를 TL;DR · 본문 · Reference 3개로 단순화 - 본문 섹션을 별도 textarea로 쪼개던 구조 → 본문 하나의 큰 textarea로 통합 - 강제감 줄이기: 필수 필드를 TL;DR과 본문 2개로 축소 - 대상 독자·관련 PR 필드 제거 (변별력 낮음, 제목·과제 페이지로 대체) - 본문 prefill에 모든 섹션 골격을 마크다운 헤딩으로 박아둠. 안 쓰는 섹션은 통째로 삭제 - Reference는 optional textarea로 분리 (참고 자료, 본인 코드 블록 링크 등) * chore: 포맷별 Optional 영역 분리 - 본문(필수)에서 각 포맷의 보조 섹션을 별도 optional textarea로 추출 - Design Doc: Cross-cutting Concerns - Retrospective: Deep Dive - Challenge Story: Lessons Learned - Benchmark Report: Future Work - Optional 영역은 placeholder만 적용 (value prefill 없이 회색 힌트만) - 필수 본문은 헤딩 골격을 value로 prefill 유지 — 정책 일관성 확보
- '어떻게 작성하나' 섹션에 필드 구성(TL;DR · 본문 · Optional · Reference) 명시 - 본문 골격에서 안 쓰는 섹션은 헤딩째 삭제하면 된다는 안내 추가 - Optional 영역은 비워둬도 OK이며 제출 시 _No response_로 표시됨을 명시
- 폼 진입 시 학습자가 보는 markdown 안내 블록에 포맷별 제목 예시 추가 - 'prefill된 [포맷] 키워드 (N주차 · K팀 · 이름)을 어떻게 바꿔야 할지' 감을 즉시 잡도록 함 - 예시는 README의 예시 4개와 동일하게 맞춰 일관성 유지
본문이 체크리스트처럼 무거워져 학습자 부담이 컸음. Kev 원본 수준으로 정리. - Design Doc: Non-goals / Verification / Risks & Open Questions 제거. 본문 description의 작성 팁도 간결화. - Retrospective: 기술 결정 표를 4컬럼 → 3컬럼(원본). Before/After 지표 표, 트레이드오프 회고 항목 제거. - Challenge Story: Alternatives 섹션 제거(필요 시 핵심 기술 선택 안에 한 줄 흡수). 작성 팁 간결화. - Benchmark Report: Analysis 섹션을 Decision의 '선택 이유'로 흡수. Setup·Results 항목 핵심만 유지. '선정 기준'은 Question에 흡수. Optional 영역(Cross-cutting / Deep Dive / Lessons Learned / Future Work)과 Reference는 그대로 유지.
- .gitignore에 .claude/ 폴더 제외 추가 - gradle wrapper 버전 업데이트 - settings.gradle.kts 설정 추가 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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: 13
🧹 Nitpick comments (1)
apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java (1)
142-147: ⚡ Quick win성공 응답 검증을 2xx 범위가 아닌 200 고정으로 강화해야 한다.
운영 관점에서 2xx 범위 허용은 컨트롤러가 계약 외 상태코드(예: 204)를 반환해도 테스트가 통과해 회귀를 놓치는 문제가 있다.
assertTrue(response.getStatusCode().is2xxSuccessful())를assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK)로 변경하는 것이 안전하다. 추가로GET /me,PATCH /me/password각각에 대해 정확히 200만 허용되는지 검증을 유지해야 한다.As per coding guidelines
**/*Test*.java: 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.Also applies to: 217-219
🤖 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 `@apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java` around lines 142 - 147, Replace the loose 2xx status assertions in UserV1ApiE2ETest (the assertAll blocks that currently use response.getStatusCode().is2xxSuccessful()) with strict equality checks against HttpStatus.OK (use assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK)) so the tests only pass for a 200 response; apply the same change for the other occurrences mentioned (the GET /me and PATCH /me/password assertions around the other assertAll block) to ensure those endpoints accept only 200 OK.
🤖 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 @.github/ISSUE_TEMPLATE/README.md:
- Around line 33-35: The fenced code block in README.md lacks a language
specifier which triggers MD040; update the fence in the example block (the
triple-backtick block showing "[포맷] 키워드 (N주차 · K팀 · 이름)") to include a language
such as "text" (i.e., replace the opening ``` with ```text) and then run the
validation command `markdownlint-cli2 .github/ISSUE_TEMPLATE/README.md` to
ensure the warning count is zero; make this change in the README.md example
block and add/verify the CI/local check that runs the markdownlint-cli2 command
as described.
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java`:
- Around line 19-24: The UserModel currently stores and compares passwords in
plaintext (field password, constructor, and matchesPassword()), which must be
replaced with secure one-way hashing: change the constructor/any setter that
persists password to store a salted hash (e.g., BCrypt) instead of the raw
string and update matchesPassword() to verify the raw candidate against the
stored hash using the hash verifier (e.g., BCrypt.checkpw or a
PasswordEncoder.matches call) so no plaintext comparisons or storage occur; also
add/adjust tests to assert stored value differs from the plaintext, to verify
successful match for correct password and failure for incorrect password, and to
verify that after a password change the old password no longer matches.
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`:
- Around line 24-29: Unify authentication failures in UserService.getUser so
both missing-user and wrong-password paths return the same HTTP status and
public message: throw a CoreException with ErrorType.UNAUTHORIZED and a generic
message (e.g., "인증에 실패했습니다.") for both findUserOrThrow failures and password
mismatches; when propagating the original error, preserve the cause in the
CoreException (use the exception constructor that accepts a cause) and log the
detailed reason (including loginId) only to internal logs (not the exception
message returned to clients). Update findUserOrThrow usage or implementation to
avoid throwing a NOT_FOUND that exposes loginId in its message (wrap/translate
it to UNAUTHORIZED in getUser). Add E2E tests that assert both invalid-loginId
and invalid-password requests return identical status and response body without
the loginId present.
- Around line 16-20: The signUp method has a TOCTOU race: it checks
userRepository.findByLoginId(...) then saves, allowing concurrent inserts;
enforce uniqueness at the DB (add a UNIQUE constraint on users.login_id) and
make signUp resilient by removing the pre-check or keeping it only for fast-fail
but catching the persistence-layer unique-violation exception thrown by
userRepository.save(...) (e.g. DataIntegrityViolationException /
DuplicateKeyException) and map it to throw new CoreException(ErrorType.CONFLICT,
"[loginId = " + user.getLoginId() + "] 이미 존재하는 아이디입니다."); and add an integration
test (e.g. UserService concurrent signup test) that issues concurrent signUp
requests for the same loginId and asserts exactly one succeeds and the others
receive 409 CONFLICT.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/LoginInterceptor.java`:
- Around line 15-21: The preHandle method currently only checks for null headers
(LOGIN_ID_HEADER, LOGIN_PW_HEADER) which blocks CORS preflight (OPTIONS) and
allows blank strings; update preHandle to immediately return true for
HttpMethod.OPTIONS requests, and validate headers using a non-empty check (e.g.,
StringUtils.hasText or equivalent) instead of != null; on failure throw the
existing CoreException(ErrorType.UNAUTHORIZED, ...) as before. Update/ add tests
covering an OPTIONS request (should pass through) and requests with blank or
whitespace LOGIN_ID_HEADER/LOGIN_PW_HEADER (should result in UNAUTHORIZED) to
prevent regressions.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java`:
- Line 20: 컨트롤러 경계에서 Bean Validation이 빠져 있으니 UserV1Controller의 요청 바인딩 메서드들(예:
signUp -> UserV1Dto.SignUpRequest 및 다른 사용자 입력을 받는 메서드)에 `@Valid를` 붙여 컨트롤러 레벨에서 검증을
강제하고, 각 DTO 필드(UserV1Dto.SignUpRequest의 필드들)에 `@NotBlank/`@Email/@Size 등 적절한 제약
어노테이션을 선언하여 필수/형식/길이 검사를 정의하세요; 또한 ApiControllerAdvice에
MethodArgumentNotValidException을 처리하는 핸들러를 추가해 일관된 400 응답과 필드별 에러 포맷을 반환하도록
구현하고, 필수값 누락 및 형식 오류 케이스에 대한 단위/통합 테스트를 추가해 400 응답 및 에러 포맷을 검증하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java`:
- Around line 9-15: SignUpRequest and UpdatePasswordRequest currently use
record's default toString() which will include plaintext passwords; override
their toString() implementations to mask or omit the password field (e.g.,
replace password with "****" or "[REDACTED]") inside each record's body, and add
unit tests for SignUpRequest.toString() and UpdatePasswordRequest.toString()
asserting the original plaintext password is not present and the masked
representation is present; reference the record names SignUpRequest and
UpdatePasswordRequest when locating and updating the code and tests.
In `@build.gradle.kts`:
- Around line 86-91: The test task currently forces DOCKER_HOST,
TESTCONTAINERS_RYUK_DISABLED and DOCKER_API_VERSION unconditionally, which
breaks cross‑platform CI; update the tasks.test configuration to stop setting
those values globally: remove the hardcoded environment("DOCKER_HOST", ...),
environment("TESTCONTAINERS_RYUK_DISABLED", ...),
environment("DOCKER_API_VERSION", ...) and their systemProperty(...)
counterparts from the tasks.test block, or make DOCKER_HOST conditional to
Windows only (detect OS at runtime and only inject the Windows npipe value when
on Windows), and move optional overrides like TESTCONTAINERS_RYUK_DISABLED and
DOCKER_API_VERSION into developer/local configuration
(~/.testcontainers.properties) or CI pipeline-specific variables instead;
additionally add CI matrix jobs (ubuntu-latest and windows-latest) to validate
Testcontainers lifecycle across platforms.
In `@gradle.properties`:
- Around line 19-21: Remove the Windows-specific Testcontainers entries
(systemProp.DOCKER_HOST and systemProp.TESTCONTAINERS_RYUK_DISABLED) from the
repository gradle.properties and stop duplicating them in the test task of
build.gradle.kts; instead either document that Windows developers must place
those keys in their user ~/.gradle/gradle.properties or add conditional logic in
build.gradle.kts to set these system properties only when running on Windows
(use an OS check around the test task configuration so non-Windows CI and
developer machines use the default Docker socket). Also ensure the duplicated
settings in the test task are removed from the project-level config and add a
short note in the README explaining how Windows developers should override these
properties locally.
In `@gradlew`:
- Line 117: The CLASSPATH initialization in gradlew is malformed: replace the
incorrect CLASSPATH="\\\"\\\"" with the standard empty initialization used by
the Gradle wrapper (i.e., set CLASSPATH to an empty string rather than escaped
quotes) by regenerating the wrapper scripts from the Gradle distribution (run
gradle wrapper to recreate gradlew and gradlew.bat) so the CLASSPATH handling
and subsequent cygpath conversion behave correctly across Linux/macOS and
Windows; after regeneration verify with ./gradlew --version and gradlew.bat
--version in Bash and CMD.
In
`@modules/jpa/src/testFixtures/java/com/loopers/testcontainers/MySqlTestContainersConfig.java`:
- Around line 41-47: The catch block in MySqlTestContainersConfig currently
always falls back to a local DB; change it to check a boolean flag (e.g.
System.getProperty("testcontainers.allowLocalFallback") or an injected config)
and only set the datasource properties
(LOCAL_JDBC_URL/LOCAL_USERNAME/LOCAL_PASSWORD) when that flag is true; otherwise
throw a new IllegalStateException with a clear user-facing message and pass the
caught exception as the cause to preserve diagnostics; also ensure logging (e.g.
System.out or a logger) is only for informational messages and does not swallow
the original exception. Add tests that assert initialization throws when
allowLocalFallback=false (propagates the cause) and that properties are set when
allowLocalFallback=true.
In
`@modules/redis/src/testFixtures/java/com/loopers/testcontainers/RedisTestContainersConfig.java`:
- Around line 21-25: In RedisTestContainersConfig (the try/catch that currently
swallows exceptions), stop silently swallowing the Exception: introduce a config
flag (e.g., allowLocalFallback) and if it's false rethrow the original exception
or wrap it while preserving the cause; if true, explicitly set the Redis
connection properties (host/port) used by your app (set the master/replica
host/port properties to localhost:6379 and localhost:6380) and log a clear
user-facing message separately from the detailed error log (include e as the
cause in the logger). Update the catch block in RedisTestContainersConfig to
implement this logic and add tests that assert failure propagation when
allowLocalFallback=false and correct property population when true.
- Around line 14-15: In RedisTestContainersConfig replace the unstable
DockerImageName.parse("redis:latest") used when creating redisContainer
(RedisContainer redisContainer; call) with a pinned version like "redis:7.2.5"
(or another explicit version you choose) so tests are reproducible, and update
the exception handling around redisContainer.start() (the try/catch block that
currently uses System.out.println) to use the project's logger (e.g., a
SLF4J/Log4j logger instance) and include the caught exception in the log call so
the Docker connection failure cause is recorded.
---
Nitpick comments:
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java`:
- Around line 142-147: Replace the loose 2xx status assertions in
UserV1ApiE2ETest (the assertAll blocks that currently use
response.getStatusCode().is2xxSuccessful()) with strict equality checks against
HttpStatus.OK (use
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK)) so the tests only
pass for a 200 response; apply the same change for the other occurrences
mentioned (the GET /me and PATCH /me/password assertions around the other
assertAll block) to ensure those endpoints accept only 200 OK.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cd63e88e-256b-4c63-bb7d-b3fe85b991d3
⛔ Files ignored due to path filters (1)
gradle/wrapper/gradle-wrapper.jaris excluded by!**/*.jarand included by**
📒 Files selected for processing (30)
.github/ISSUE_TEMPLATE/01-design-doc.yml.github/ISSUE_TEMPLATE/02-retrospective.yml.github/ISSUE_TEMPLATE/03-challenge-story.yml.github/ISSUE_TEMPLATE/04-benchmark-report.yml.github/ISSUE_TEMPLATE/README.md.github/ISSUE_TEMPLATE/config.yml.gitignoreapps/commerce-api/src/main/java/com/loopers/application/user/UserFacade.javaapps/commerce-api/src/main/java/com/loopers/application/user/UserInfo.javaapps/commerce-api/src/main/java/com/loopers/config/WebMvcConfig.javaapps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.javaapps/commerce-api/src/main/java/com/loopers/domain/user/UserRepository.javaapps/commerce-api/src/main/java/com/loopers/domain/user/UserService.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserJpaRepository.javaapps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/user/LoginInterceptor.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.javaapps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.javaapps/commerce-api/src/main/java/com/loopers/support/error/ErrorType.javaapps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.javaapps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.javaapps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.javabuild.gradle.ktsgradle.propertiesgradle/wrapper/gradle-wrapper.propertiesgradlewgradlew.batmodules/jpa/src/testFixtures/java/com/loopers/testcontainers/MySqlTestContainersConfig.javamodules/redis/src/testFixtures/java/com/loopers/testcontainers/RedisTestContainersConfig.javasettings.gradle.kts
Comment on lines
+33
to
+35
| ``` | ||
| [포맷] 키워드 (N주차 · K팀 · 이름) | ||
| ``` |
There was a problem hiding this comment.
펜스 코드블록 언어 미지정으로 문서 린트 경고가 발생한다
운영 관점에서 문서 린트 경고가 CI 품질 게이트를 불안정하게 만들 수 있어 유지보수 신뢰도를 낮춘다. 코드블록 시작 fence에 언어를 명시해 MD040 경고를 제거하는 것이 좋다. 수정 후 markdownlint-cli2 .github/ISSUE_TEMPLATE/README.md를 실행해 경고가 0건인지 확인하는 테스트를 추가로 수행해야 한다.
수정안 예시
-```
+```text
[포맷] 키워드 (N주차 · K팀 · 이름)</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 @.github/ISSUE_TEMPLATE/README.md around lines 33 - 35, The fenced code block
in README.md lacks a language specifier which triggers MD040; update the fence
in the example block (the triple-backtick block showing "[포맷] 키워드 (N주차 · K팀 ·
이름)") to include a language such as "text" (i.e., replace the opening ``` with
```text) and then run the validation command `markdownlint-cli2
.github/ISSUE_TEMPLATE/README.md` to ensure the warning count is zero; make this
change in the README.md example block and add/verify the CI/local check that
runs the markdownlint-cli2 command as described.
Comment on lines
+19
to
+24
| private String loginId; | ||
| private String password; | ||
| private String name; | ||
| private LocalDate birthDate; | ||
| private String email; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 비밀번호 해시 사용 여부와 평문 비교 지점을 점검한다.
rg -nP --type=java '\bPasswordEncoder\b|BCryptPasswordEncoder|Argon2PasswordEncoder|matchesPassword\s*\(|private\s+String\s+password\b'Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 991
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java | head -80Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 2966
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java | head -50Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 1872
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/domain/user/UserModelTest.java | grep -A 10 -B 5 "matchesPassword\|password"Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 189
🏁 Script executed:
find apps/commerce-api -name "*UserModelTest.java" -o -name "*UserServiceIntegrationTest.java"Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 245
🏁 Script executed:
rg -n "BCryptPasswordEncoder|PasswordEncoder|hashPassword|encodePassword" --type=java apps/commerce-api/Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 64
🏁 Script executed:
cat -n apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java | grep -A 20 "password\|Password" | head -100Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 4488
🏁 Script executed:
cat -n apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java | grep -A 20 "password\|Password" | head -100Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 4215
🏁 Script executed:
cat -n apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java | grep -A 30 "updatePassword\|비밀번호를 수정"Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 2474
🏁 Script executed:
cat -n apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java | grep -A 30 "updatePassword\|비밀번호를 수정"Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 5526
평문 비밀번호 저장/비교는 즉시 차단해야 한다.
비밀번호를 평문으로 저장·비교하면 DB 유출 시 계정 탈취로 직결되는 운영·보안 사고 위험이 매우 크다. Line 35 생성자와 Line 53 matchesPassword() 메서드에서 평문 저장 및 equals() 비교를 수행 중이므로, 저장은 단방향 해시로 처리하고 비교는 해시 검증으로만 수행하도록 변경해야 한다. 추가로 저장값이 원문과 다름을 검증하는 테스트, 올바른/잘못된 비밀번호 검증 테스트, 비밀번호 변경 후 이전 비밀번호 재사용 실패 테스트를 보강해야 한다.
🤖 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 `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java`
around lines 19 - 24, The UserModel currently stores and compares passwords in
plaintext (field password, constructor, and matchesPassword()), which must be
replaced with secure one-way hashing: change the constructor/any setter that
persists password to store a salted hash (e.g., BCrypt) instead of the raw
string and update matchesPassword() to verify the raw candidate against the
stored hash using the hash verifier (e.g., BCrypt.checkpw or a
PasswordEncoder.matches call) so no plaintext comparisons or storage occur; also
add/adjust tests to assert stored value differs from the plaintext, to verify
successful match for correct password and failure for incorrect password, and to
verify that after a password change the old password no longer matches.
Comment on lines
+16
to
+20
| public UserModel signUp(UserModel user) { | ||
| if (userRepository.findByLoginId(user.getLoginId()).isPresent()) { | ||
| throw new CoreException(ErrorType.CONFLICT, "[loginId = " + user.getLoginId() + "] 이미 존재하는 아이디입니다."); | ||
| } | ||
| return userRepository.save(user); |
There was a problem hiding this comment.
회원가입 중복 검사에 TOCTOU 경합이 있다.
Line 17 사전 조회 후 Line 20 저장 방식은 동시 요청에서 둘 다 통과해 중복 loginId가 저장될 수 있어 계정 식별 무결성을 깨뜨린다. users.login_id에 DB 유니크 제약을 두고, 저장 시 제약 위반을 CONFLICT로 매핑하는 방식으로 원자성을 보장해야 한다. 추가로 동일 loginId 동시 회원가입 시 1건만 성공하고 나머지가 409로 실패하는 통합 테스트를 추가해야 한다.
🤖 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 `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`
around lines 16 - 20, The signUp method has a TOCTOU race: it checks
userRepository.findByLoginId(...) then saves, allowing concurrent inserts;
enforce uniqueness at the DB (add a UNIQUE constraint on users.login_id) and
make signUp resilient by removing the pre-check or keeping it only for fast-fail
but catching the persistence-layer unique-violation exception thrown by
userRepository.save(...) (e.g. DataIntegrityViolationException /
DuplicateKeyException) and map it to throw new CoreException(ErrorType.CONFLICT,
"[loginId = " + user.getLoginId() + "] 이미 존재하는 아이디입니다."); and add an integration
test (e.g. UserService concurrent signup test) that issues concurrent signUp
requests for the same loginId and asserts exactly one succeeds and the others
receive 409 CONFLICT.
Comment on lines
+24
to
+29
| public UserModel getUser(String loginId, String password) { | ||
| UserModel user = findUserOrThrow(loginId); | ||
|
|
||
| if (!user.matchesPassword(password)) { | ||
| throw new CoreException(ErrorType.UNAUTHORIZED, "비밀번호가 일치하지 않습니다."); | ||
| } |
There was a problem hiding this comment.
인증 실패 응답이 계정 존재 여부와 식별자를 노출한다.
Line 42는 NOT_FOUND와 loginId 포함 메시지를, Line 28은 UNAUTHORIZED를 사용하여 호출자가 계정 존재 여부를 추정할 수 있는 열거 벡터를 만든다. 인증 실패는 모두 동일한 UNAUTHORIZED와 일반 메시지로 통일하고, 상세 원인은 내부 로그에서만 마스킹된 값으로 분리해야 한다. 추가로 잘못된 loginId/비밀번호 두 케이스가 동일 상태코드와 동일 메시지를 반환하고 응답 본문에 loginId가 노출되지 않음을 검증하는 E2E 테스트를 추가해야 한다. As per coding guidelines, "예외 처리 시 cause를 보존하고, 사용자 메시지와 로그 메시지를 분리하도록 제안한다. 로깅 시 민감정보 노출 가능성을 점검한다."
Also applies to: 40-42
🤖 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 `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`
around lines 24 - 29, Unify authentication failures in UserService.getUser so
both missing-user and wrong-password paths return the same HTTP status and
public message: throw a CoreException with ErrorType.UNAUTHORIZED and a generic
message (e.g., "인증에 실패했습니다.") for both findUserOrThrow failures and password
mismatches; when propagating the original error, preserve the cause in the
CoreException (use the exception constructor that accepts a cause) and log the
detailed reason (including loginId) only to internal logs (not the exception
message returned to clients). Update findUserOrThrow usage or implementation to
avoid throwing a NOT_FOUND that exposes loginId in its message (wrap/translate
it to UNAUTHORIZED in getUser). Add E2E tests that assert both invalid-loginId
and invalid-password requests return identical status and response body without
the loginId present.
Comment on lines
+15
to
+21
| public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) { | ||
| String loginId = request.getHeader(LOGIN_ID_HEADER); | ||
| String password = request.getHeader(LOGIN_PW_HEADER); | ||
|
|
||
| if (loginId == null || password == null) { | ||
| throw new CoreException(ErrorType.UNAUTHORIZED, "인증 헤더가 필요합니다."); | ||
| } |
There was a problem hiding this comment.
CORS preflight와 공백 헤더를 인터셉터에서 차단해야 한다
Line [19]~[21]은 null만 검사하므로 운영 시 브라우저 preflight(OPTIONS)가 401로 막혀 API 호출이 실패할 수 있다. 또한 공백 문자열이 통과되어 인증 실패 원인이 서비스 계층으로 전파되어 장애 분석이 어려워진다.
수정안으로 OPTIONS는 통과시키고, hasText 기준으로 헤더 유효성을 검증하는 것이 안전하다. 추가로 OPTIONS/공백 헤더 케이스 E2E 또는 MockMvc 테스트를 넣어 재발을 막아야 한다.
수정 예시
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
+import org.springframework.util.StringUtils;
import org.springframework.web.servlet.HandlerInterceptor;
`@Override`
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {
+ if ("OPTIONS".equalsIgnoreCase(request.getMethod())) {
+ return true;
+ }
+
String loginId = request.getHeader(LOGIN_ID_HEADER);
String password = request.getHeader(LOGIN_PW_HEADER);
- if (loginId == null || password == null) {
+ if (!StringUtils.hasText(loginId) || !StringUtils.hasText(password)) {
throw new CoreException(ErrorType.UNAUTHORIZED, "인증 헤더가 필요합니다.");
}📝 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
| public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) { | |
| String loginId = request.getHeader(LOGIN_ID_HEADER); | |
| String password = request.getHeader(LOGIN_PW_HEADER); | |
| if (loginId == null || password == null) { | |
| throw new CoreException(ErrorType.UNAUTHORIZED, "인증 헤더가 필요합니다."); | |
| } | |
| public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) { | |
| if ("OPTIONS".equalsIgnoreCase(request.getMethod())) { | |
| return true; | |
| } | |
| String loginId = request.getHeader(LOGIN_ID_HEADER); | |
| String password = request.getHeader(LOGIN_PW_HEADER); | |
| if (!StringUtils.hasText(loginId) || !StringUtils.hasText(password)) { | |
| throw new CoreException(ErrorType.UNAUTHORIZED, "인증 헤더가 필요합니다."); | |
| } |
🤖 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
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/LoginInterceptor.java`
around lines 15 - 21, The preHandle method currently only checks for null
headers (LOGIN_ID_HEADER, LOGIN_PW_HEADER) which blocks CORS preflight (OPTIONS)
and allows blank strings; update preHandle to immediately return true for
HttpMethod.OPTIONS requests, and validate headers using a non-empty check (e.g.,
StringUtils.hasText or equivalent) instead of != null; on failure throw the
existing CoreException(ErrorType.UNAUTHORIZED, ...) as before. Update/ add tests
covering an OPTIONS request (should pass through) and requests with blank or
whitespace LOGIN_ID_HEADER/LOGIN_PW_HEADER (should result in UNAUTHORIZED) to
prevent regressions.
Comment on lines
+19
to
+21
| ### Testcontainers (Windows Docker Desktop) ### | ||
| systemProp.DOCKER_HOST=npipe:////./pipe/dockerDesktopLinuxEngine | ||
| systemProp.TESTCONTAINERS_RYUK_DISABLED=true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# gradle.properties 파일 위치 및 내용 확인
fd gradle.propertiesRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 100
🏁 Script executed:
# gradle.properties의 전체 내용 확인
cat -n gradle.properties 2>/dev/null || echo "파일을 찾을 수 없습니다"Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 924
🏁 Script executed:
# 리포지토리 구조 확인
ls -la | head -20Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 1245
🏁 Script executed:
# Testcontainers 관련 설정 및 test 구성 확인
fd -e gradle -e properties | head -20Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 141
🏁 Script executed:
# CI 환경설정 확인 (GitHub Actions, GitLab CI 등)
fd -e yml -e yaml | grep -E '(\.github|\.gitlab|\.circleci)' | head -10Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 64
🏁 Script executed:
# GitHub Actions 워크플로우 확인
find .github -type f -name "*.yml" -o -name "*.yaml" | head -10Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 345
🏁 Script executed:
# CI 워크플로우 파일 내용 확인
cat .github/workflows/*.yml 2>/dev/null | head -100Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 637
🏁 Script executed:
# build.gradle.kts에서 Testcontainers 관련 설정 확인
grep -n -i "testcontainer\|docker" build.gradle.ktsRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 782
🏁 Script executed:
# 테스트 코드에서 Testcontainers 사용 확인
fd -e kt -e java | xargs grep -l "testcontainer\|`@Testcontainers`\|DockerComposeContainer" 2>/dev/null | head -10Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 268
Windows 전용 Docker 설정을 저장소 공통 gradle.properties에 두면 다른 플랫폼 환경에서 이식성이 깨진다.
Gradle의 systemProp은 모든 gradle 명령에 주입되므로, npipe:// URL은 Linux/macOS 환경에서 작동하지 않는다. 특히 CI 환경(ubuntu-latest)에서 이 값이 전역 적용되면 Docker 데몬 연결 실패로 테스트가 예측 불가능하게 실패할 수 있다.
추가로 build.gradle.kts에서도 test task에 동일한 설정이 중복으로 정의되어 있다. 이들 설정은 저장소 기본값에서 제거하고, Windows 로컬 환경에서만 ~/.gradle/gradle.properties로 오버라이드하거나 CI 환경에서는 조건부 로직으로 분리해야 한다. 비-Windows 환경에서 기본 Docker 소켓(/var/run/docker.sock)으로 정상 연결되는지, Windows 개발 환경에서만 npipe 설정 적용 시 정상 동작하는지 각각 검증이 필요하다.
🤖 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 `@gradle.properties` around lines 19 - 21, Remove the Windows-specific
Testcontainers entries (systemProp.DOCKER_HOST and
systemProp.TESTCONTAINERS_RYUK_DISABLED) from the repository gradle.properties
and stop duplicating them in the test task of build.gradle.kts; instead either
document that Windows developers must place those keys in their user
~/.gradle/gradle.properties or add conditional logic in build.gradle.kts to set
these system properties only when running on Windows (use an OS check around the
test task configuration so non-Windows CI and developer machines use the default
Docker socket). Also ensure the duplicated settings in the test task are removed
from the project-level config and add a short note in the README explaining how
Windows developers should override these properties locally.
| esac | ||
|
|
||
| CLASSPATH=$APP_HOME/gradle/wrapper/gradle-wrapper.jar | ||
| CLASSPATH="\\\"\\\"" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "[1] Wrapper launcher 패턴 확인"
rg -n 'CLASSPATH=|GradleWrapperMain|-jar .*/gradle-wrapper\.jar|-classpath' gradlew gradlew.bat
echo "[2] gradle-wrapper.jar Main-Class 확인"
if command -v unzip >/dev/null 2>&1; then
unzip -p gradle/wrapper/gradle-wrapper.jar META-INF/MANIFEST.MF | rg -n 'Main-Class|Implementation-Version' || true
else
echo "unzip 명령이 없어 MANIFEST 검증은 생략한다."
fiRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 670
🏁 Script executed:
#!/bin/bash
echo "[1] gradlew 100-130행 범위 확인 (117행 문맥)"
sed -n '100,130p' gradlew
echo ""
echo "[2] gradlew 210-220행 범위 확인 (실행 부분)"
sed -n '210,220p' gradlew
echo ""
echo "[3] gradlew.bat 70-80행 범위 확인 (Windows 버전)"
sed -n '70,80p' gradlew.batRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 1706
🏁 Script executed:
#!/bin/bash
echo "[1] gradlew의 CLASSPATH 모든 언급 부분"
rg -n 'CLASSPATH' gradlew
echo ""
echo "[2] gradlew의 cygpath 변환 구간 확인 (165-180행)"
sed -n '165,180p' gradlew
echo ""
echo "[3] 최근 변경 이력 (gradlew 파일)"
git log --oneline -10 -- gradlewRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 894
gradlew의 CLASSPATH 설정이 표준 wrapper 패턴을 위반하며 크로스플랫폼 일관성을 해친다.
117행의 CLASSPATH="\\\"\\\"" 초기화는 표준 Gradle wrapper 스크립트의 패턴에서 벗어난다. gradlew.bat는 73행에서 set CLASSPATH= (공백)로 정상 초기화하는 반면, gradlew의 해당 값은 이스케이프된 따옴표 문자열로 설정되어 있다. 이 값은 175행의 cygpath 변환 과정에서도 비정상 문자열로 처리되며, Linux/macOS와 Windows 간 호환성을 저해한다.
표준 gradle wrapper 패턴을 유지하기 위해 해당 버전의 Gradle 배포판에서 제공하는 gradlew/gradlew.bat로 재생성할 것을 권고한다. 변경 후 Linux/macOS 셸, Windows Git Bash, Windows CMD 환경에서 ./gradlew --version 또는 gradlew.bat --version 으로 정상 동작을 확인해야 한다.
🤖 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 `@gradlew` at line 117, The CLASSPATH initialization in gradlew is malformed:
replace the incorrect CLASSPATH="\\\"\\\"" with the standard empty
initialization used by the Gradle wrapper (i.e., set CLASSPATH to an empty
string rather than escaped quotes) by regenerating the wrapper scripts from the
Gradle distribution (run gradle wrapper to recreate gradlew and gradlew.bat) so
the CLASSPATH handling and subsequent cygpath conversion behave correctly across
Linux/macOS and Windows; after regeneration verify with ./gradlew --version and
gradlew.bat --version in Bash and CMD.
Comment on lines
+41
to
+47
| } catch (Exception e) { | ||
| // Docker를 사용할 수 없는 경우 로컬 MySQL로 폴백 | ||
| System.out.println("[TestContainers] Docker 연결 실패 - 로컬 MySQL(" + LOCAL_JDBC_URL + ")로 대체합니다."); | ||
| System.setProperty("datasource.mysql-jpa.main.jdbc-url", LOCAL_JDBC_URL); | ||
| System.setProperty("datasource.mysql-jpa.main.username", LOCAL_USERNAME); | ||
| System.setProperty("datasource.mysql-jpa.main.password", LOCAL_PASSWORD); | ||
| } |
There was a problem hiding this comment.
Docker 실패를 무조건 로컬 DB로 숨기는 처리로 테스트 신뢰성이 떨어진다.
운영 관점에서 Line 41~47은 인프라 장애를 숨겨 테스트가 공유 로컬 DB에 붙어도 통과할 수 있어 회귀를 놓칠 위험이 크다. testcontainers.allowLocalFallback 같은 명시적 플래그가 있을 때만 폴백하고, 기본값은 IllegalStateException(..., e)로 cause를 보존해 실패시키는 방식으로 수정하는 것이 안전하다. 추가 테스트로 allowLocalFallback=false일 때 초기화 예외 전파, true일 때 로컬 datasource 프로퍼티 설정을 각각 검증해야 한다.
수정 예시
+ private static final boolean ALLOW_LOCAL_FALLBACK =
+ Boolean.parseBoolean(System.getProperty("testcontainers.allowLocalFallback", "false"));
...
- } catch (Exception e) {
- // Docker를 사용할 수 없는 경우 로컬 MySQL로 폴백
- System.out.println("[TestContainers] Docker 연결 실패 - 로컬 MySQL(" + LOCAL_JDBC_URL + ")로 대체합니다.");
- System.setProperty("datasource.mysql-jpa.main.jdbc-url", LOCAL_JDBC_URL);
- System.setProperty("datasource.mysql-jpa.main.username", LOCAL_USERNAME);
- System.setProperty("datasource.mysql-jpa.main.password", LOCAL_PASSWORD);
- }
+ } catch (Exception e) {
+ if (!ALLOW_LOCAL_FALLBACK) {
+ throw new IllegalStateException("[TestContainers] MySQL 컨테이너 기동 실패", e);
+ }
+ System.err.println("[TestContainers] Docker 연결 실패 - 로컬 MySQL로 폴백합니다.");
+ System.setProperty("datasource.mysql-jpa.main.jdbc-url", LOCAL_JDBC_URL);
+ System.setProperty("datasource.mysql-jpa.main.username", LOCAL_USERNAME);
+ System.setProperty("datasource.mysql-jpa.main.password", LOCAL_PASSWORD);
+ }As per coding guidelines "예외 처리 시 cause를 보존하고, 사용자 메시지와 로그 메시지를 분리하도록 제안한다."
🤖 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
`@modules/jpa/src/testFixtures/java/com/loopers/testcontainers/MySqlTestContainersConfig.java`
around lines 41 - 47, The catch block in MySqlTestContainersConfig currently
always falls back to a local DB; change it to check a boolean flag (e.g.
System.getProperty("testcontainers.allowLocalFallback") or an injected config)
and only set the datasource properties
(LOCAL_JDBC_URL/LOCAL_USERNAME/LOCAL_PASSWORD) when that flag is true; otherwise
throw a new IllegalStateException with a clear user-facing message and pass the
caught exception as the cause to preserve diagnostics; also ensure logging (e.g.
System.out or a logger) is only for informational messages and does not swallow
the original exception. Add tests that assert initialization throws when
allowLocalFallback=false (propagates the cause) and that properties are set when
allowLocalFallback=true.
Comment on lines
+14
to
+15
| redisContainer = new RedisContainer(DockerImageName.parse("redis:latest")); | ||
| redisContainer.start(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# redis 이미지 태그 사용 현황 및 latest 사용 여부 확인
rg -n 'DockerImageName\.parse\("redis:[^"]+"\)' --type=java
rg -n 'DockerImageName\.parse\(".*:latest"\)' --type=javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 452
🏁 Script executed:
cat -n modules/redis/src/testFixtures/java/com/loopers/testcontainers/RedisTestContainersConfig.javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 1666
🏁 Script executed:
# 프로젝트 내 다른 testcontainer 패턴 확인
rg -B2 -A2 'Container\(DockerImageName\.parse' --type=java | head -50Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 769
🏁 Script executed:
find modules -name "*.java" -type f | xargs grep -l "testcontainers" | head -10Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 268
🏁 Script executed:
# 프로젝트 전체 testcontainer 버전 관리 패턴 확인
rg -B3 'new.*Container\(' --type=java modules/ | head -60Repository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 574
🏁 Script executed:
cat -n modules/jpa/src/testFixtures/java/com/loopers/testcontainers/MySqlTestContainersConfig.javaRepository: loopers-labs/loop-pack-be-l2-vol4-java
Length of output: 2552
redis:latest 사용은 테스트 재현성을 깨뜨린다.
Line 14의 latest 태그는 이미지 변경 시점마다 테스트 결과가 달라져 CI 플래키와 원인 미상 장애를 유발한다. 동일 프로젝트의 MySQL testcontainer가 mysql:8.0처럼 고정 버전을 사용하는 것처럼, Redis도 특정 버전(예: redis:7.2.5)으로 핀하고 주기적으로 업데이트하는 방식으로 수정해야 한다.
추가로 lines 21-25의 예외 처리는 발생한 예외를 기록하지 않아 Docker 연결 실패 원인 파악이 어렵다. System.out.println 대신 적절한 로거를 사용하고 예외를 함께 기록하도록 개선하면 장애 진단이 용이해진다.
수정 예시
- redisContainer = new RedisContainer(DockerImageName.parse("redis:latest"));
+ redisContainer = new RedisContainer(DockerImageName.parse("redis:7.2.5")); } catch (Exception e) {
// Docker를 사용할 수 없는 경우 로컬 Redis로 폴백 (redis.yml test profile 기본값 사용)
- System.out.println("[TestContainers] Docker 연결 실패 - 로컬 Redis(localhost:6379/6380)로 대체합니다.");
+ log.warn("[TestContainers] Docker 연결 실패 - 로컬 Redis(localhost:6379/6380)로 대체합니다.", e);📝 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
| redisContainer = new RedisContainer(DockerImageName.parse("redis:latest")); | |
| redisContainer.start(); | |
| redisContainer = new RedisContainer(DockerImageName.parse("redis:7.2.5")); | |
| redisContainer.start(); |
🤖 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
`@modules/redis/src/testFixtures/java/com/loopers/testcontainers/RedisTestContainersConfig.java`
around lines 14 - 15, In RedisTestContainersConfig replace the unstable
DockerImageName.parse("redis:latest") used when creating redisContainer
(RedisContainer redisContainer; call) with a pinned version like "redis:7.2.5"
(or another explicit version you choose) so tests are reproducible, and update
the exception handling around redisContainer.start() (the try/catch block that
currently uses System.out.println) to use the project's logger (e.g., a
SLF4J/Log4j logger instance) and include the caught exception in the log call so
the Docker connection failure cause is recorded.
Comment on lines
+21
to
+25
| } catch (Exception e) { | ||
| // Docker를 사용할 수 없는 경우 로컬 Redis로 폴백 (redis.yml test profile 기본값 사용) | ||
| System.out.println("[TestContainers] Docker 연결 실패 - 로컬 Redis(localhost:6379/6380)로 대체합니다."); | ||
| // redis.yml test profile에 localhost:6379(master), localhost:6380(replica)가 이미 설정되어 있음 | ||
| } |
There was a problem hiding this comment.
Docker 실패를 로그만 남기고 진행하면 장애가 숨겨진다.
운영 관점에서 Line 21~25는 예외를 삼키고 기본 설정에 암묵 의존해 실패 원인과 환경 불일치를 놓치기 쉽다. MySQL과 동일하게 명시적 폴백 플래그가 없으면 예외를 cause 포함해 재던지고, 폴백 시에는 적용된 host/port를 프로퍼티로 명시 설정하는 방식으로 수정해야 한다. 추가 테스트로 Docker 비가용 상황에서 allowLocalFallback=false 실패 전파, true에서 로컬 Redis 프로퍼티가 정확히 설정되는 경로를 검증해야 한다.
수정 예시
+ private static final boolean ALLOW_LOCAL_FALLBACK =
+ Boolean.parseBoolean(System.getProperty("testcontainers.allowLocalFallback", "false"));
...
- } catch (Exception e) {
- // Docker를 사용할 수 없는 경우 로컬 Redis로 폴백 (redis.yml test profile 기본값 사용)
- System.out.println("[TestContainers] Docker 연결 실패 - 로컬 Redis(localhost:6379/6380)로 대체합니다.");
- // redis.yml test profile에 localhost:6379(master), localhost:6380(replica)가 이미 설정되어 있음
- }
+ } catch (Exception e) {
+ if (!ALLOW_LOCAL_FALLBACK) {
+ throw new IllegalStateException("[TestContainers] Redis 컨테이너 기동 실패", e);
+ }
+ System.err.println("[TestContainers] Docker 연결 실패 - 로컬 Redis로 폴백합니다.");
+ System.setProperty("datasource.redis.database", "0");
+ System.setProperty("datasource.redis.master.host", "localhost");
+ System.setProperty("datasource.redis.master.port", "6379");
+ System.setProperty("datasource.redis.replicas[0].host", "localhost");
+ System.setProperty("datasource.redis.replicas[0].port", "6380");
+ }As per coding guidelines "예외 처리 시 cause를 보존하고, 사용자 메시지와 로그 메시지를 분리하도록 제안한다."
🤖 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
`@modules/redis/src/testFixtures/java/com/loopers/testcontainers/RedisTestContainersConfig.java`
around lines 21 - 25, In RedisTestContainersConfig (the try/catch that currently
swallows exceptions), stop silently swallowing the Exception: introduce a config
flag (e.g., allowLocalFallback) and if it's false rethrow the original exception
or wrap it while preserving the cause; if true, explicitly set the Redis
connection properties (host/port) used by your app (set the master/replica
host/port properties to localhost:6379 and localhost:6380) and log a clear
user-facing message separately from the detailed error log (include e as the
cause in the logger). Update the catch block in RedisTestContainersConfig to
implement this logic and add tests that assert failure propagation when
allowLocalFallback=false and correct property population when true.
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.
📌 Summary
🧭 Context & Decision
문제 정의
선택지와 결정
🤔 고민한 점 / 막혔던 부분
🙋 기타