feat(configuration): 파일 API 공통 응답 규약 및 예외 처리 #26 - #58
Open
tlgms wants to merge 2 commits into
Hidden character warning
The head ref may contain hidden characters: "feat(document)-\ud30c\uc77c-\uc751\ub2f5-\uaddc\uc57d-#26"
Open
Conversation
명세의 성공/실패 응답 형식에 맞춘 ApiResponse envelope과 파일 관련 ErrorCode를 정의한다. 성공 응답에도 error 키를 항상 포함하고 timestamp는 실패 시에만 직렬화한다. 도메인 예외와 MaxUploadSizeExceededException을 명세의 HTTP 상태로 변환한다. 후자를 잡지 않으면 413이 아니라 500이 나간다. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
명세의 응답 스키마에 대응하는 DTO와, 참조 ID(attachment_1 형태) 변환, MultipartFile→커맨드 변환 헬퍼를 추가한다. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✨ 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 |
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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
`@systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/DocumentExceptionHandler.kt`:
- Around line 22-61: Add deterministic tests in the configuration adapter-in
subsystem for DocumentExceptionHandler.kt lines 22-61, covering each mapped
domain exception, MaxUploadSizeExceededException, and unexpected exceptions with
HTTP status and ApiResponse.failure body assertions; for ApiResponse.kt lines
13-24, verify success/failure success, data, error, and timestamp JSON
serialization; and for ErrorCode.kt lines 5-13, verify every code’s HTTP status
and message.
- Around line 34-41: Update handleInvalidRequestParam to remove the broad
IllegalArgumentException mapping, limiting INVALID_REQUEST_PARAM handling to the
declared request-binding exceptions. Add adapter-in tests verifying the
supported binding exceptions map correctly and unrelated
IllegalArgumentException instances are not classified as INVALID_REQUEST_PARAM.
In
`@systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/FileReferenceId.kt`:
- Around line 5-14: 동일 서브시스템의 테스트를 추가하십시오. FileReferenceId.kt:5-14에서는
FileReferenceId.of 및 parse의 정상 생성·파싱, 잘못된 접두사와 숫자 입력의 실패를 검증하고,
MultipartFileExtensions.kt:9-20에서는 유효·무효 확장자와 UploadFileCommand 필드 변환을 테스트하십시오.
FileResponses.kt:6-49에서는 UploadFileResponse와 DownloadUrlResponse의 도메인 변환 필드를
검증하십시오.
- Around line 9-11: Update FileReferenceId.parse to validate that value starts
with the prefix returned by prefixOf(category) before removing it; reject values
with a missing or incorrect category prefix, while preserving the existing
numeric parsing and IllegalArgumentException behavior for invalid IDs.
🪄 Autofix
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: ASSERTIVE
Plan: Pro Plus
Run ID: 5d3042ec-95a7-4c0c-9253-c6e415175ddd
📒 Files selected for processing (7)
systems/configuration/configuration-adapter-in/deps.bzlsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/ApiResponse.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/DocumentExceptionHandler.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/ErrorCode.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/FileReferenceId.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/MultipartFileExtensions.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/dto/FileResponses.kt
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/{BUILD.bazel,*.bzl}
📄 CodeRabbit inference engine (Custom checks)
In BUILD.bazel and .bzl files, require buildifier-compatible formatting and stable target naming
Files:
systems/configuration/configuration-adapter-in/deps.bzl
**/*.bzl
⚙️ CodeRabbit configuration file
**/*.bzl: Apply Bazel Starlark (.bzl) style guidance.Readability and docs:
- Keep file/module docstrings and docstrings for public functions/macros.
- Use descriptive parameter names and document attribute intent.
API design:
- Macros should take a
nameargument and derive generated target names from it.- Prefer keyword arguments when calling macros for clarity and stability.
- Keep macro side effects predictable and visible.
Encapsulation:
- Use private visibility for helper targets created by macros unless explicitly public.
- Avoid exposing internal implementation targets unintentionally.
Tooling:
- Enforce buildifier formatting and lint compliance.
Files:
systems/configuration/configuration-adapter-in/deps.bzl
**/*.{kt,go}
📄 CodeRabbit inference engine (Custom checks)
If production logic is changed in Kotlin or Go files, require corresponding test updates in the same subsystem unless the PR description explicitly justifies why tests are unnecessary
Files:
systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/ErrorCode.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/FileReferenceId.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/MultipartFileExtensions.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/ApiResponse.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/dto/FileResponses.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/DocumentExceptionHandler.kt
**/*.{java,kt,scala,groovy,go,js,ts,tsx,jsx,py,rb,rs,cpp,c,h,hpp,cs}
📄 CodeRabbit inference engine (Custom checks)
Flag TODO/FIXME comments introduced by this PR that do not include an issue reference in the form
#123or a full tracker key like PROJ-123
Files:
systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/ErrorCode.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/FileReferenceId.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/MultipartFileExtensions.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/ApiResponse.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/dto/FileResponses.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/DocumentExceptionHandler.kt
**/*.kt
⚙️ CodeRabbit configuration file
**/*.kt: Apply Kotlin Official Coding Conventions.Formatting and structure:
- Use 4 spaces for indentation; no tabs.
- Keep files focused and readable; avoid horizontal alignment for spacing.
- Place related declarations together and keep overloads adjacent.
- Keep implementation member order stable and logical for readability.
Naming:
- Package names are lowercase and do not use underscores.
- Class/object names use UpperCamelCase.
- Functions/properties/local variables use lowerCamelCase.
- Constants use UPPER_SNAKE_CASE only for true constants.
API and null-safety:
- Avoid platform type leakage in public APIs.
- Use explicit types in public APIs when inference obscures meaning.
- Prefer immutable values (
val) over mutable values (var) unless mutation is required.- Flag nullable flows that can be replaced with safer modeling.
Imports and idioms:
- Avoid wildcard imports unless justified by language/tooling conventions.
- Prefer expression bodies for short, clear functions.
- Prefer standard library idioms over custom utility wrappers when equivalent.
Architecture and tests:
- Respect module boundaries (domain/application/adapter/bootstrap layering).
- Highlight behavior-changing code that lacks corresponding unit/integration tests.
- Ask for deterministic tests and meaningful assertions, not only happy-path checks.
Files:
systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/ErrorCode.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/FileReferenceId.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/MultipartFileExtensions.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/ApiResponse.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/dto/FileResponses.ktsystems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/DocumentExceptionHandler.kt
🧠 Learnings (1)
📚 Learning: 2026-06-11T06:03:49.247Z
Learnt from: CR
Repo: EntryDSM/entrydsm-platform PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-06-11T06:03:49.247Z
Learning: Applies to **/*-application/**/*.{java,kt,scala,groovy} : For files under *-application modules, flag direct dependency on infrastructure-specific framework classes unless justified
Applied to files:
systems/configuration/configuration-adapter-in/deps.bzl
🔇 Additional comments (3)
systems/configuration/configuration-adapter-in/deps.bzl (1)
10-12: LGTM!systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/ErrorCode.kt (1)
9-9: 🎯 Functional Correctness수정 불필요
kotlin.MODULE.bazel은 Spring Boot4.0.1을 사용합니다. 따라서HttpStatus.CONTENT_TOO_LARGE호환성 문제는 없습니다.> Likely an incorrect or invalid review comment.systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/MultipartFileExtensions.kt (1)
13-18: 🎯 Functional Correctness
toUploadCommand내부 확장자 검증은 필요하지 않습니다.
FileDocumentService.upload가 저장 전에 확장자와FileCategory지원 여부를 검증하므로 검증을 우회할 수 없습니다.> Likely an incorrect or invalid review comment.
Comment on lines
+22
to
+61
| @ExceptionHandler(InvalidFileFormatException::class) | ||
| fun handleInvalidFileFormat(e: InvalidFileFormatException) = | ||
| respond(ErrorCode.INVALID_FILE_FORMAT, e) | ||
|
|
||
| @ExceptionHandler(FileTooLargeException::class, MaxUploadSizeExceededException::class) | ||
| fun handleFileTooLarge(e: Exception) = | ||
| respond(ErrorCode.FILE_TOO_LARGE, e) | ||
|
|
||
| @ExceptionHandler(FileDocumentNotFoundException::class) | ||
| fun handleFileNotFound(e: FileDocumentNotFoundException) = | ||
| respond(ErrorCode.FILE_NOT_FOUND, e) | ||
|
|
||
| @ExceptionHandler( | ||
| InvalidFileNameException::class, | ||
| MissingServletRequestParameterException::class, | ||
| MethodArgumentNotValidException::class, | ||
| IllegalArgumentException::class, | ||
| ) | ||
| fun handleInvalidRequestParam(e: Exception) = | ||
| respond(ErrorCode.INVALID_REQUEST_PARAM, e) | ||
|
|
||
| @ExceptionHandler(StorageUploadFailedException::class) | ||
| fun handleStorageUploadFailed(e: StorageUploadFailedException) = | ||
| respond(ErrorCode.STORAGE_UPLOAD_FAILED, e) | ||
|
|
||
| @ExceptionHandler(PresignFailedException::class) | ||
| fun handlePresignFailed(e: PresignFailedException) = | ||
| respond(ErrorCode.PRESIGN_FAILED, e) | ||
|
|
||
| @ExceptionHandler(Exception::class) | ||
| fun handleUnexpected(e: Exception): ResponseEntity<ApiResponse<Nothing>> { | ||
| log.error("Unhandled exception", e) | ||
| return ResponseEntity | ||
| .status(ErrorCode.INTERNAL_SERVER_ERROR.status) | ||
| .body(ApiResponse.failure(ErrorCode.INTERNAL_SERVER_ERROR)) | ||
| } | ||
|
|
||
| private fun respond(errorCode: ErrorCode, e: Exception): ResponseEntity<ApiResponse<Nothing>> { | ||
| log.warn("{}: {}", errorCode.name, e.message) | ||
| return ResponseEntity.status(errorCode.status).body(ApiResponse.failure(errorCode)) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
공통 응답 및 예외 변환 계약에 대한 테스트를 추가하세요.
PR 목표에 테스트가 아직 완료되지 않았다고 명시되어 있습니다. 이 변경은 HTTP 상태, 오류 코드, JSON 응답 구조를 외부 계약으로 추가합니다. 병합 전에 결정적 테스트로 계약을 고정하세요.
systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/DocumentExceptionHandler.kt#L22-L61: 각 도메인 예외,MaxUploadSizeExceededException, 예상치 못한 예외의 HTTP 상태와ApiResponse.failure본문을 검증하세요.systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/ApiResponse.kt#L13-L24: 성공 및 실패 응답의success,data,error,timestamp직렬화 계약을 검증하세요.systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/ErrorCode.kt#L5-L13: 각ErrorCode의 HTTP 상태와 메시지를 검증하세요.
As per coding guidelines, "If production logic is changed in Kotlin or Go files, require corresponding test updates in the same subsystem unless the PR description explicitly justifies why tests are unnecessary" 규칙을 적용했습니다.
📍 Affects 3 files
systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/DocumentExceptionHandler.kt#L22-L61(this comment)systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/ApiResponse.kt#L13-L24systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/ErrorCode.kt#L5-L13
🤖 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
`@systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/DocumentExceptionHandler.kt`
around lines 22 - 61, Add deterministic tests in the configuration adapter-in
subsystem for DocumentExceptionHandler.kt lines 22-61, covering each mapped
domain exception, MaxUploadSizeExceededException, and unexpected exceptions with
HTTP status and ApiResponse.failure body assertions; for ApiResponse.kt lines
13-24, verify success/failure success, data, error, and timestamp JSON
serialization; and for ErrorCode.kt lines 5-13, verify every code’s HTTP status
and message.
Source: Coding guidelines
Comment on lines
+34
to
+41
| @ExceptionHandler( | ||
| InvalidFileNameException::class, | ||
| MissingServletRequestParameterException::class, | ||
| MethodArgumentNotValidException::class, | ||
| IllegalArgumentException::class, | ||
| ) | ||
| fun handleInvalidRequestParam(e: Exception) = | ||
| respond(ErrorCode.INVALID_REQUEST_PARAM, e) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# configuration 모듈에서 IllegalArgumentException을 생성하는 위치를 확인합니다.
rg -n -C 3 --glob '*.kt' \
'throw\s+IllegalArgumentException|require\(|check\(' \
systems/configurationRepository: EntryDSM/entrydsm-platform
Length of output: 1400
🏁 Script executed:
#!/bin/bash
set -euo pipefail
handler='systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapter-in/common/DocumentExceptionHandler.kt'
# 실제 경로를 먼저 찾고, advice 구현과 예외 사용처 및 테스트를 함께 확인합니다.
fd -i 'DocumentExceptionHandler.kt|FileReferenceId.kt' systems/configuration
rg -n -C 5 --glob '*.kt' \
'`@RestControllerAdvice`|`@ExceptionHandler`|handleInvalidRequestParam|handleUnexpected|FileReferenceId|IllegalArgumentException' \
systems/configurationRepository: EntryDSM/entrydsm-platform
Length of output: 11160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -i 'DocumentExceptionHandler.kt|FileReferenceId.kt' systems/configuration
rg -n -C 5 --glob '*.kt' \
'`@RestControllerAdvice`|`@ExceptionHandler`|handleInvalidRequestParam|handleUnexpected|FileReferenceId|IllegalArgumentException' \
systems/configurationRepository: EntryDSM/entrydsm-platform
Length of output: 11160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- FileReferenceId 호출부 ---'
rg -n -C 6 --glob '*.kt' 'FileReferenceId\.(parse|of)\(' systems/configuration
echo '--- IllegalArgumentException 및 require/check 전체 사용처 ---'
rg -n -C 3 --glob '*.kt' \
'IllegalArgumentException|require\(|check\(' systems/configuration
echo '--- 관련 테스트 파일 ---'
fd -i -t f 'test|Test|Spec' systems/configuration | head -200Repository: EntryDSM/entrydsm-platform
Length of output: 192
🏁 Script executed:
#!/bin/bash
set -u
echo '--- FileReferenceId 모든 참조 ---'
rg -n -C 5 --glob '*.kt' 'FileReferenceId|parse\(' systems/configuration || true
echo '--- IllegalArgumentException 및 require/check 전체 사용처 ---'
rg -n -C 3 --glob '*.kt' \
'IllegalArgumentException|require\(|check\(' systems/configuration || true
echo '--- configuration 테스트 파일 ---'
fd -i -t f 'test|Test|Spec' systems/configuration | head -200 || true
echo '--- 컨트롤러 및 advice 선언 ---'
rg -n -C 4 --glob '*.kt' \
'`@RestController`|`@Controller`|`@RestControllerAdvice`|`@ControllerAdvice`' systems/configuration || trueRepository: EntryDSM/entrydsm-platform
Length of output: 7360
🏁 Script executed:
#!/bin/bash
set -u
echo '--- configuration Kotlin 파일 목록 ---'
git ls-files 'systems/configuration/**/*.kt' | head -300
echo '--- HTTP 엔드포인트 관련 선언 ---'
rg -n -C 4 --glob '*.kt' \
'RequestMapping|GetMapping|PostMapping|PutMapping|DeleteMapping|PatchMapping|Controller' \
systems/configuration || true
echo '--- 예외 클래스 선언 및 상속 관계 ---'
rg -n -C 4 --glob '*.kt' \
'class (InvalidFileNameException|InvalidFileFormatException|FileTooLargeException|FileDocumentNotFoundException|StorageUploadFailedException|PresignFailedException)|: .*Exception|: .*RuntimeException' \
systems/configuration || trueRepository: EntryDSM/entrydsm-platform
Length of output: 38641
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
import re
root = Path("systems/configuration")
kotlin_files = list(root.rglob("*.kt"))
direct_throws = []
controller_annotations = []
file_reference_refs = []
for path in kotlin_files:
text = path.read_text()
for match in re.finditer(r"throw\s+IllegalArgumentException", text):
line = text.count("\n", 0, match.start()) + 1
direct_throws.append((str(path), line))
for match in re.finditer(r"^\s*@(?:RestController|Controller)\b", text, re.MULTILINE):
line = text.count("\n", 0, match.start()) + 1
controller_annotations.append((str(path), line))
if "FileReferenceId" in text:
file_reference_refs.append(str(path))
print("direct IllegalArgumentException throws:")
for item in direct_throws:
print(item)
print("HTTP controller annotations:")
for item in controller_annotations:
print(item)
print("files referencing FileReferenceId:")
for item in file_reference_refs:
print(item)
print("DocumentExceptionHandler has global IllegalArgumentException mapping:",
any(
"DocumentExceptionHandler.kt" in path and "IllegalArgumentException::class" in Path(path).read_text()
for path, _ in direct_throws
) if direct_throws else
"systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/DocumentExceptionHandler.kt"
in [str(p) for p in kotlin_files]
and "IllegalArgumentException::class" in Path(
"systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/DocumentExceptionHandler.kt"
).read_text())
PYRepository: EntryDSM/entrydsm-platform
Length of output: 612
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
import re
root = Path("systems/configuration")
kotlin_files = list(root.rglob("*.kt"))
direct_throws = []
controller_annotations = []
file_reference_refs = []
for path in kotlin_files:
text = path.read_text()
for match in re.finditer(r"throw\s+IllegalArgumentException", text):
line = text.count("\n", 0, match.start()) + 1
direct_throws.append((str(path), line))
for match in re.finditer(r"^\s*@(?:RestController|Controller)\b", text, re.MULTILINE):
line = text.count("\n", 0, match.start()) + 1
controller_annotations.append((str(path), line))
if "FileReferenceId" in text:
file_reference_refs.append(str(path))
print("direct IllegalArgumentException throws:")
for item in direct_throws:
print(item)
print("HTTP controller annotations:")
for item in controller_annotations:
print(item)
print("files referencing FileReferenceId:")
for item in file_reference_refs:
print(item)
handler = next(
(
p for p in kotlin_files
if p.name == "DocumentExceptionHandler.kt"
),
None,
)
print(
"DocumentExceptionHandler has global IllegalArgumentException mapping:",
bool(handler and "IllegalArgumentException::class" in handler.read_text()),
)
PYRepository: EntryDSM/entrydsm-platform
Length of output: 611
IllegalArgumentException 매핑 범위를 요청 오류로 제한하세요.
현재 configuration 소스에는 HTTP controller와 도메인·인프라의 직접적인 IllegalArgumentException 발생 지점이 없습니다. 다만 이 매핑은 향후 모든 MVC controller의 IllegalArgumentException을 INVALID_REQUEST_PARAM으로 분류합니다. 요청 바인딩 예외만 매핑하고, 해당 동작을 검증하는 adapter-in 테스트를 추가하세요.
🤖 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
`@systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/common/DocumentExceptionHandler.kt`
around lines 34 - 41, Update handleInvalidRequestParam to remove the broad
IllegalArgumentException mapping, limiting INVALID_REQUEST_PARAM handling to the
declared request-binding exceptions. Add adapter-in tests verifying the
supported binding exceptions map correctly and unrelated
IllegalArgumentException instances are not classified as INVALID_REQUEST_PARAM.
Comment on lines
+5
to
+14
| object FileReferenceId { | ||
|
|
||
| fun of(category: FileCategory, id: Long): String = "${prefixOf(category)}$id" | ||
|
|
||
| fun parse(category: FileCategory, value: String): Long = | ||
| value.removePrefix(prefixOf(category)).toLongOrNull() | ||
| ?: throw IllegalArgumentException("Invalid ${category.name.lowercase()} id: $value") | ||
|
|
||
| private fun prefixOf(category: FileCategory) = "${category.name.lowercase()}_" | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
동일 서브시스템에 대응 테스트를 추가하십시오.
새 Kotlin 프로덕션 로직에 대한 테스트 갱신이 없습니다. FileReferenceId의 정상·잘못된 접두사·잘못된 숫자 입력을 테스트하십시오. MultipartFileExtensions의 유효·무효 확장자와 UploadFileCommand 필드 변환을 테스트하십시오. UploadFileResponse와 DownloadUrlResponse의 도메인 변환 필드도 테스트하십시오.
systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/FileReferenceId.kt#L5-L14: 참조 ID 생성과 파싱 실패 조건을 검증하는 단위 테스트를 추가하십시오.systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/MultipartFileExtensions.kt#L9-L20: 확장자 검증과 업로드 명령 변환을 검증하는 단위 테스트를 추가하십시오.systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/dto/FileResponses.kt#L6-L49: 응답 DTO 변환 필드를 검증하는 단위 테스트를 추가하십시오.
코딩 가이드라인에 따라 Kotlin 프로덕션 로직 변경 시 동일 서브시스템의 테스트 갱신이 필요합니다.
📍 Affects 3 files
systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/FileReferenceId.kt#L5-L14(this comment)systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/MultipartFileExtensions.kt#L9-L20systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/dto/FileResponses.kt#L6-L49
🤖 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
`@systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/FileReferenceId.kt`
around lines 5 - 14, 동일 서브시스템의 테스트를 추가하십시오. FileReferenceId.kt:5-14에서는
FileReferenceId.of 및 parse의 정상 생성·파싱, 잘못된 접두사와 숫자 입력의 실패를 검증하고,
MultipartFileExtensions.kt:9-20에서는 유효·무효 확장자와 UploadFileCommand 필드 변환을 테스트하십시오.
FileResponses.kt:6-49에서는 UploadFileResponse와 DownloadUrlResponse의 도메인 변환 필드를
검증하십시오.
Source: Coding guidelines
Comment on lines
+9
to
+11
| fun parse(category: FileCategory, value: String): Long = | ||
| value.removePrefix(prefixOf(category)).toLongOrNull() | ||
| ?: throw IllegalArgumentException("Invalid ${category.name.lowercase()} id: $value") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
카테고리 접두사를 먼저 검증하십시오.
Line 10의 removePrefix는 접두사가 없을 때 원본 값을 그대로 반환합니다. 따라서 parse(category, "123")는 카테고리 접두사가 없어도 성공합니다. 참조 ID 형식이 카테고리 기반이라는 계약이 깨집니다.
수정 예시
-fun parse(category: FileCategory, value: String): Long =
- value.removePrefix(prefixOf(category)).toLongOrNull()
- ?: throw IllegalArgumentException("Invalid ${category.name.lowercase()} id: $value")
+fun parse(category: FileCategory, value: String): Long {
+ val prefix = prefixOf(category)
+ if (!value.startsWith(prefix)) {
+ throw IllegalArgumentException("Invalid ${category.name.lowercase()} id: $value")
+ }
+
+ return value.removePrefix(prefix).toLongOrNull()
+ ?: throw IllegalArgumentException("Invalid ${category.name.lowercase()} id: $value")
+}📝 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
| fun parse(category: FileCategory, value: String): Long = | |
| value.removePrefix(prefixOf(category)).toLongOrNull() | |
| ?: throw IllegalArgumentException("Invalid ${category.name.lowercase()} id: $value") | |
| fun parse(category: FileCategory, value: String): Long { | |
| val prefix = prefixOf(category) | |
| if (!value.startsWith(prefix)) { | |
| throw IllegalArgumentException("Invalid ${category.name.lowercase()} id: $value") | |
| } | |
| return value.removePrefix(prefix).toLongOrNull() | |
| ?: throw IllegalArgumentException("Invalid ${category.name.lowercase()} id: $value") | |
| } |
🤖 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
`@systems/configuration/configuration-adapter-in/src/main/kotlin/hs/kr/entrydsm/configuration/adapterin/document/FileReferenceId.kt`
around lines 9 - 11, Update FileReferenceId.parse to validate that value starts
with the prefix returned by prefixOf(category) before removing it; reject values
with a missing or incorrect category prefix, while preserving the existing
numeric parsing and IllegalArgumentException behavior for invalid IDs.
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
1 participant
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
ApiResponseenvelope과ErrorCode를 정의합니다.@RestControllerAdvice를 추가합니다.Related Issue
Scope
configuration-adapter-in의common/document.dto패키지Implementation
API 공통 규약 §12가 "서비스마다 다른 응답 형식 사용"과 "에러 메시지만 반환하고 에러 코드 생략"을 금지합니다.
ApiResponse<T>error키를 항상 포함합니다. 개별 API 명세가 전부"error": null을 보여주고 있고, 클라이언트 파싱도 단순해집니다timestamp는 실패 시에만 직렬화합니다 (@get:JsonInclude(NON_NULL))DocumentExceptionHandlerInvalidFileFormatExceptionINVALID_FILE_FORMATInvalidFileNameException, 파라미터 누락INVALID_REQUEST_PARAMFileDocumentNotFoundExceptionFILE_NOT_FOUNDFileTooLargeException,MaxUploadSizeExceededExceptionFILE_TOO_LARGEStorageUploadFailedExceptionSTORAGE_UPLOAD_FAILEDPresignFailedExceptionPRESIGN_FAILEDMaxUploadSizeExceededException을 잡지 않으면 용량 초과 시 명세의 413이 아니라 500이 나갑니다. 이것 때문에 핸들러에 포함했습니다.헬퍼
FileReferenceId—attachment_1형태의 참조 ID ↔filesPK 변환MultipartFileExtensions— 확장자 검증, 커맨드 변환Testing
Deployment Notes
@RestControllerAdvice가 configuration 서비스 전역에 적용됩니다. 기존 gRPC 어댑터에는 영향이 없지만, 향후 다른 REST 컨트롤러가 추가되면 이 핸들러를 공유하게 됩니다Checklist
리뷰 시 봐주셨으면 하는 것
ApiResponse를packages/common으로 뺄지 — 다른 서비스도 같은 envelope이 필요합니다. 다만 공유 패키지 신설은 이슈 범위를 넘어 지금은configuration-adapter-in에 두었습니다. 두 번째 서비스가 필요해지는 시점에 추출을 권합니다.HttpStatus.CONTENT_TOO_LARGE—PAYLOAD_TOO_LARGE가 deprecated라 교체했습니다.itemsvsapplicants, page 1-base vs 0-base). Document에는 목록 API가 없어 이 PR에는 영향이 없지만, 명세 정정이 필요합니다.