[✨feat] AuthType 구현 #39

leeeyubin · 2025-05-17T17:54:42Z

📄 Work Description

Auth 도메인에서 사용되는 AuthType을 구현했습니다.
AuthErrorCode 구현도 해 주었습니다.

💭 Thoughts

현재는 유효하지 않은 메세지 값에 대해서만 테스트 코드를 작성했는데 혹시 더 테스트 필요하다고 생각하시는 케이스가 있다면 말씀해주세요!
추가로 소소한 지식 공유하자면 기존 Java 프로젝트에 있던 getMessage() 함수 네이밍을 그대로 사용했더니 테스트 코드에서 에러가 발생하더라구요! 이는 Kotlin 자동으로 생성해 주는 getter와 충돌해서 발생한 내용이라네요🫢 따라서 getPrefixMessage()로 네이밍 변경해 주었어요!

✅ Testing Result

수정 전 테스트코드

🗂 Related Issue

closed [✨feat] AuthType 구현 #36

jsoonworld

Auth 도메인 구조부터 예외 처리 흐름까지 전체적으로 정말 깔끔하게 정리해주셔서 인상 깊었어요!
특히 책임 분리가 잘 되어 있어서 읽는 내내 이해가 쉬웠고, 덕분에 많이 배워가는 PR이었습니다 🙌

그리고 말씀해주신 내용 덕분에 저도 하나 새롭게 알게 되었어요 🙂
기존 Java 프로젝트에서 사용하던 getMessage() 네이밍을 그대로 가져왔을 때 테스트 코드에서 에러가 발생한 이유가
Kotlin이 자동으로 생성하는 getter와 충돌해서 그렇다는 건 처음 알았어요 🫢
그래서 getPrefixMessage()로 네이밍을 바꿨다는 말씀 듣고, "아 그렇구나!" 하고 바로 납득이 됐습니다.
공유해주셔서 감사합니다!

구현하시느라 고생 많으셨고, 함께 이런 고민 나눌 수 있어 즐거운 리뷰였습니다 😄

jsoonworld · 2025-05-18T04:02:41Z

src/main/kotlin/com/terning/server/kotlin/domain/auth/AuthErrorCode.kt

+    val status: HttpStatus,
+    val message: String,
+) {
+    INVALID_TOKEN(status = HttpStatus.UNAUTHORIZED, message = "유효하지 않은 토큰입니다."),


이전에 남겨주셨던 피드백을 기반으로, 다시 한 번 제 생각을 정리해서 공유드려봅니다! 😊

현재 AuthErrorCode에서는 아래처럼 프로퍼티 이름을 명시적으로 붙여서 작성하고 있는데요,

INVALID_TOKEN(status = HttpStatus.UNAUTHORIZED, message = "유효하지 않은 토큰입니다.")

저는 이 방식이 조금 아쉽게 느껴졌어요

프로퍼티 자체에 이미 status, message라는 이름이 선언되어 있어서 컴파일 타임에서 검증이 충분히 이루어지고 있다고 생각했고,

현재 enum 값은 2개의 파라미터를 가지고 있지만, 다른 enum에서는 1개 또는 3개처럼 다양하게 구성될 수 있기 때문에
이 방식이 오히려 통일성을 해칠 수 있고, 코드 전체적으로 가독성도 떨어질 수 있다는 생각이 들었어요.

인자가 많아질수록 매번 key = value 형식으로 작성하는 것이 중복처럼 보이기도 해서,
저는 오히려 순서 기반으로 간결하게 작성하는 방식이 더 깔끔하고 읽기 쉽다고 느꼈습니다.

그래서 현재는 status =, message =를 명시하지 않고 순서대로만 작성하는 방향을 더 선호하고 있어요!
혹시 이 부분에 대해 다른 생각 있으시면 공유주셔도 좋을 것 같아요 😄

말씀해주신 부분을 보고 생각해보았는데 저는 함수를 사용하는 케이스들까지 고려해서 말씀을 드린 거였어요!

현재 코드처럼 enum class의 경우 별도의 argument를 넣어주지 않아도 간결하게 표현할 수 있다는 점에는 동의해요!

다만, 장기적으로 보았을 때 같은 타입의 인자가 많아질 경우나, 함수를 사용하는 곳까지 고려해보았을 때 Named arguments를 통해 코드의 정확도와 통일성을 챙길 수 있다고 생각했어요!

코틀린 공식 문서에서는 같은 타입의 파라미터가 여러 개일 경우, 혹은 불리언 타입의 파라미터일 경우 코딩 컨벤션으로 Named arguments를 사용하는 것을 권장하고 있는데 이런 경우에만 사용해 주는 것으로 할까요?

좋은 의견 감사합니다! 🙌
말씀해주신 것처럼 함수에서 같은 타입의 인자가 많아질 경우나, 불리언 타입처럼 의미가 모호해질 수 있는 경우에는 named arguments를 사용하는 것이 더 명확하고 안전하다는 점에 공감했습니다.

그래서 이 부분은 회의에서 논의한 대로 named arguments를 사용하는 방향으로 통일해서 작업해보면 좋을 것 같습니다.
작성 시점에서도 의도가 더 드러나고, 유지보수 측면에서도 안정적일 것 같아요!
앞으로의 개발에서도 이런 컨벤션을 함께 맞춰가면 더 좋을 것 같습니다 😊

의견 나눠주셔서 감사합니다!

jsoonworld · 2025-05-18T04:05:57Z

src/main/kotlin/com/terning/server/kotlin/domain/auth/AuthErrorCode.kt

+    fun getPrefixMessage(): String = "$PREFIX $message"
+
+    companion object {
+        private const val PREFIX = "[AUTH ERROR]"
+    }


예외 메시지 prefix 처리 방식에 대한 고민 공유

예외 메시지에 prefix를 붙이는 방식에 대해 다시 한 번 생각해보게 되었어요 😊

우선, "요란한 예외처리"의 중요성은 저도 공감하고 있습니다.
에러의 성격을 드러내기 위한 의도 자체는 충분히 의미 있는 방향이라고 생각해요.

그런데 이번 경우는 메시지가 클라이언트에도 그대로 전달되고 있어, UI에 직접 영향을 주는 부분이라 몇 가지 고민이 들었습니다.

제가 가져본 생각은요...

클라이언트 화면에서 에러 메시지를 그대로 보여주는 구조라면, prefix가 포함된 메시지는 사용자 입장에서 과도하거나 불필요하게 기술적인 메시지로 보일 수 있겠다는 생각이 들었습니다.

또한 상태 코드와 함께 메시지를 전달하는 구조라면, prefix 없이도 의도를 충분히 명확하게 전달할 수 있지 않을까? 하는 고민도 해보게 되었어요.

마지막으로, prefix를 별도로 관리하게 되면 메시지 수정 시 중복 관리가 발생하거나, 유지보수 비용이 커질 가능성도 고려하게 되었습니다.

그래서 저는 이번 케이스에서는 메시지 자체를 명확하게 표현하는 쪽으로 가고, prefix는 사용자 출력이 아닌 내부 로깅에만 활용하는 방식도 좋지 않을까 하는 생각이 들었어요!

혹시 이 부분에 대해서 다른 생각 있으시거나, prefix를 유지해야 하는 명확한 근거 혹은 생각이 있다면 공유 부탁드릴게요 😄

사실 이 부분은 기존 프로젝트의 방식을 사용했던 것인데요..!
말씀해 주신 내용을 토대로 생각해보니 prefix를 사용하지 않아도 될 것 같네요!
단순 메시지만 전달하는 구조로 바꿔도 충분할 것 같아요~!

jsoonworld · 2025-05-18T04:08:23Z

src/test/kotlin/com/terning/server/kotlin/domain/auth/AuthTypeTest.kt

+class AuthTypeTest {
+    @Test
+    @DisplayName("prefix와 message 결합하여 반환한다")
+    fun getPrefixMessage() {
+        val errorCode = AuthErrorCode.INVALID_TOKEN
+        val expectedMessage = "[AUTH ERROR] 유효하지 않은 토큰입니다."
+
+        assertThat(errorCode.getPrefixMessage()).isEqualTo(expectedMessage)
+        assertThat(errorCode.status).isEqualTo(HttpStatus.UNAUTHORIZED)
+        assertThat(errorCode.message).isEqualTo("유효하지 않은 토큰입니다.")
+    }
+}


getPrefixMessage()에 대한 테스트를 보면서 조금 고민이 들었습니다.

현재는 메시지를 직접 조합해서 반환하는 메서드의 결과를 검증하고 있는데요, 저는 이보다는 예외가 실제로 발생했을 때 해당 메시지가 어떻게 전달되는지를 검증하는 방식이 더 자연스럽지 않을까 생각했어요.

특히 이 메시지가 클라이언트에 그대로 전달되거나 사용자에게 보여지는 경우라면, 실제 예외 발생 시 메시지를 검증하는 테스트가 훨씬 의미 있는 피드백을 줄 수 있을 것 같아요. 지금처럼 조합된 문자열만 따로 떼어내서 검증하는 방식은 약간 단편적인 확인처럼 느껴졌습니다.

또한 메시지 포맷이 바뀌더라도 테스트가 쉽게 깨질 수 있다는 점에서 유지보수 비용도 고려해야 하지 않을까 싶었고요.

그래서 개인적으로는 메시지 자체보다는 예외 발생 흐름 안에서의 메시지 검증이 더 본질적인 테스트가 아닐까 하는 생각이 들었습니다. 혹시 이 부분에 대해서는 어떻게 생각하시나요? 😊

예리한 조언 감사합니다!!👍
아직 테스트 코드가 미숙해서 생각이 짧았던 것 같네요..!
예외 발생 흐름을 중점으로 테스트 코드를 작성하는 방향으로 수정해보겠습니다!

Copilot

Pull Request Overview

Implements the authentication domain types and error handling flow, including new enums, exception classes, and a handler.

Introduce AuthType enum with supported providers
Define AuthErrorCode and AuthException, plus update global exception handler
Add a basic test for invalid token scenarios

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/main/kotlin/.../domain/auth/AuthType.kt	Added `AuthType` enum listings
src/main/kotlin/.../domain/auth/AuthErrorCode.kt	Defined `AuthErrorCode` enum and message accessors
src/main/kotlin/.../domain/auth/AuthException.kt	New `AuthException` wrapping `AuthErrorCode`
src/main/kotlin/.../ui/api/ExceptionHandler.kt	Added handler for `AuthException`
src/test/kotlin/.../domain/auth/AuthTypeTest.kt	Test for invalid-token exception messaging/status

Comments suppressed due to low confidence (3)

src/test/kotlin/com/terning/server/kotlin/domain/auth/AuthTypeTest.kt:9

[nitpick] The test class is named AuthTypeTest but only covers AuthException. Consider renaming to AuthExceptionTest or adding tests for AuthType behavior to align naming with its contents.

class AuthTypeTest {

src/test/kotlin/com/terning/server/kotlin/domain/auth/AuthTypeTest.kt:12

Only invalid-token behavior is tested. It would be valuable to add tests for valid AuthType values, parsing from strings, or other enum-specific logic to improve coverage.

fun throwAuthExceptionWhenTokenIsInvalid() {

src/main/kotlin/com/terning/server/kotlin/domain/auth/AuthErrorCode.kt:12

[nitpick] getErrorMessage() is redundant since message is already exposed. Consider removing this method or standardizing on one accessor to simplify the API.

fun getErrorMessage(): String = message

Copilot · 2025-05-23T13:23:51Z

src/main/kotlin/com/terning/server/kotlin/ui/api/ExceptionHandler.kt

@@ -80,6 +81,14 @@ class ExceptionHandler : ResponseEntityExceptionHandler() {
            .body(ApiResponse.error(exception.errorCode.status, exception.errorCode.message))
    }

+    @ExceptionHandler(AuthException::class)
+    fun handleAuthException(exception: AuthException): ResponseEntity<ApiResponse<Unit>> {
+        logger.error("message", exception)


[nitpick] Logging the literal string "message" is not descriptive. Include context such as "AuthException occurred:" or the actual exception message for clearer logs.

Suggested change

logger.error("message", exception)

logger.error("AuthException occurred: ${exception.message}", exception)

leeeyubin self-assigned this May 17, 2025

leeeyubin added ✨ feat 🐬 유빈🐬 labels May 17, 2025

pull-request-size bot added the size/M label May 17, 2025

leeeyubin requested a review from jsoonworld May 17, 2025 18:10

jsoonworld approved these changes May 18, 2025

View reviewed changes

jsoonworld requested a review from Copilot May 23, 2025 13:22

Copilot AI reviewed May 23, 2025

View reviewed changes

leeeyubin added 13 commits May 25, 2025 13:24

feat(AuthType) : AuthType 구현

208be6c

feat(AuthErrorCode) : AuthErrorCode 구현

a826d97

feat(AuthErrorCode) : getPrefixMessage 함수 네이밍 변경

9e368a1

feat(AuthTypeTest) : AuthType 테스트 코드 작성

4ea96be

feat(AuthTypeTest) : ktlint 적용

c8070ef

feat(AuthErrorCode) : 빈 줄 추가

02bf206

feat(AuthErrorCode) : 빈 줄 삭제

9d79409

feat(AuthErrorCode) : 빈 줄 삭제

927e778

feat(AuthErrorCode) : EOF 수정

f2a9820

chore(AuthErrorCode) : 에러 문자 방식 수정

ee7aeb8

chore(AuthTypeTest) : 테스트 코드 흐름 수정

40654b6

chore(AuthTypeTest) : ktlint 규칙 맞추기

212617a

chore(ExceptionHandler) : 로그명 수정

45a9a4f

leeeyubin force-pushed the feat/#36 branch from a8eccad to 45a9a4f Compare May 25, 2025 04:25

leeeyubin added 5 commits May 25, 2025 13:27

chore(ExceptionHandler) : 오류 수정

2a080f8

chore(ExceptionHandler) : 불필요한 임포트 제거

d051edc

chore(AuthErrorCode) : BaseErrorCode 상속

865fb43

chore(AuthException) : BaseException 상속

1291ff8

chore(ExceptionHandler) : 에러 로그 수정

5e3b098

leeeyubin merged commit 6ae59e2 into develop May 25, 2025
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[✨feat] AuthType 구현 #39

[✨feat] AuthType 구현 #39

leeeyubin commented May 17, 2025 •

edited

Loading

Uh oh!

jsoonworld left a comment

Uh oh!

jsoonworld May 18, 2025

Uh oh!

leeeyubin May 20, 2025

Uh oh!

jsoonworld May 23, 2025

Uh oh!

jsoonworld May 18, 2025

Uh oh!

leeeyubin May 21, 2025

Uh oh!

jsoonworld May 18, 2025

Uh oh!

leeeyubin May 21, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI May 23, 2025

Uh oh!

Uh oh!

Uh oh!

	logger.error("message", exception)
	logger.error("AuthException occurred: ${exception.message}", exception)

[✨feat] AuthType 구현 #39

[✨feat] AuthType 구현 #39

Conversation

leeeyubin commented May 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📄 Work Description

💭 Thoughts

✅ Testing Result

🗂 Related Issue

Uh oh!

jsoonworld left a comment

Choose a reason for hiding this comment

Uh oh!

jsoonworld May 18, 2025

Choose a reason for hiding this comment

Uh oh!

leeeyubin May 20, 2025

Choose a reason for hiding this comment

Uh oh!

jsoonworld May 23, 2025

Choose a reason for hiding this comment

Uh oh!

jsoonworld May 18, 2025

Choose a reason for hiding this comment

예외 메시지 prefix 처리 방식에 대한 고민 공유

제가 가져본 생각은요...

Uh oh!

leeeyubin May 21, 2025

Choose a reason for hiding this comment

Uh oh!

jsoonworld May 18, 2025

Choose a reason for hiding this comment

Uh oh!

leeeyubin May 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

leeeyubin commented May 17, 2025 •

edited

Loading