refactor: Jwt 토큰 관련 클래스를 상속이 아니라 조합하도록 #324
Conversation
변경 사항 워크스루
Suggested reviewers
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 30th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (3)
src/test/java/com/example/solidconnection/security/filter/AuthorizationHeaderParserTest.java (1)
1-55:⚠️ Potential issue잘 작성된 테스트 클래스입니다!
다음은 몇 가지 개선점을 제안드립니다:
테스트 구조가 잘 정리되어 있습니다
- 중첩 클래스를 사용하여 테스트 시나리오를 명확하게 그룹화했습니다
- given-when-then 패턴을 일관되게 적용했습니다
버그 수정 필요 - 44-45줄의 오류
MockHttpServletRequest emptyToken = new MockHttpServletRequest(); -wrongPrefix.addHeader("Authorization", "Bearer "); +emptyToken.addHeader("Authorization", "Bearer ");
- 현재 코드에서는
emptyToken요청에 헤더를 추가하지 않고wrongPrefix에 잘못 추가하고 있습니다- 이로 인해 50번 줄의
emptyToken테스트 케이스가 "Bearer " 토큰을 테스트하지 않고 헤더가 없는 경우를 중복 테스트하게 됩니다테스트 케이스 추가 고려사항
- 다양한 형식의 잘못된 토큰(예: 대소문자가 다른 "bearer")에 대한 테스트도 추가하면 좋을 것 같습니다
src/main/java/com/example/solidconnection/auth/service/EmailSignUpTokenProvider.java (1)
40-46:⚠️ Potential issue② 비밀번호 해시를 JWT Claim에 담는 것에 대한 보안 우려
설령 해시된 값이라도 비밀번호 파생물이 토큰에 그대로 노출됩니다.
① 네트워크·로그 등에 토큰이 유출될 경우 크래킹 대상이 늘어나고,
② 장기적으로 해시 알고리즘이 약화될 때 위험이 커집니다.
해시 대신 랜덤 토큰을 Redis 에 저장한 뒤 이메일로 전달하는 방식(일회용 코드)으로 변경을 권고드립니다.src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java (1)
34-39: 🛠️ Refactor suggestion① 유효성 검사 시 파싱 예외에 대한 graceful fallback 필요
isValidRefreshToken내부에서tokenProvider.parseSubject가 실패하면CustomException이 바로 전파되어 500/4xx 로 직결됩니다.
토큰이 잘못되었을 때는false를 반환하도록 try-catch 로 감싸면 클라이언트에게 일관된 401 응답을 줄 수 있어 UX 가 좋아집니다.
🧹 Nitpick comments (5)
src/main/java/com/example/solidconnection/auth/service/TokenBlackListService.java (1)
1-33: 잘 설계된 토큰 블랙리스트 서비스입니다!다음은 몇 가지 관찰과 개선점을 제안드립니다:
구조적 장점
BlacklistChecker인터페이스 구현을 통해 느슨한 결합을 달성했습니다- Redis를 사용하여 블랙리스트를 효율적으로 관리하고 있습니다
- 주석을 통해 코드의 의도가 명확히 전달됩니다
개선 고려사항
- 블랙리스트에 추가되는 토큰의 만료 시간 설정이 없습니다. 토큰 만료 시간과 동일하게 Redis 키의 만료 시간을 설정하는 것이 좋습니다:
public void addToBlacklist(AccessToken accessToken) { String blackListKey = BLACKLIST.addPrefix(accessToken.token()); - redisTemplate.opsForValue().set(blackListKey, SIGN_OUT_VALUE); + redisTemplate.opsForValue().set(blackListKey, SIGN_OUT_VALUE, accessToken.getExpiryDuration(), TimeUnit.MILLISECONDS); }확장성 고려사항
- 향후 블랙리스트에 추가된 시간, 사유 등의 메타데이터가 필요할 수 있습니다. 로깅이나 관리 목적으로 이런 정보를 저장하는 방안도 고려해보세요.
src/main/java/com/example/solidconnection/auth/service/EmailSignUpTokenProvider.java (2)
48-55: ① 토큰 생성 로직 중복 제거 제안한 파일 안에서 직접
Jwts.builder()를 사용하여 토큰을 생성하고 있습니다.
이미TokenProvider.generateToken(...)가 동일한 역할을 담당하므로, Claim 을 추가할 수 있는 오버로드 메서드를TokenProvider에 신설하여 여기서도 재사용하면 알고리즘·시크릿 관리의 단일화와 중복 제거를 동시에 달성할 수 있습니다.
63-71: ③ 예외 잡을 때 원인 정보가 소실됩니다모든 예외를 catch 후
CustomException으로 래핑하지만 원래 stack trace 가 사라져 디버깅이 어려워집니다.
throw new CustomException(SIGN_UP_TOKEN_INVALID, e);처럼 cause 를 전달해 두면 추후 문제 분석이 한결 수월합니다.src/main/java/com/example/solidconnection/auth/service/TokenProvider.java (2)
53-60: ① 원인 전달 누락 – 디버깅 정보 보존
parseClaims에서 모든 예외를 삼키고CustomException만 던지면 실제 실패 원인을 확인하기 어렵습니다.
throw new CustomException(INVALID_TOKEN, e);형태로 cause 를 포함해 두면 운영 중 로그 분석이 훨씬 수월합니다.
25-35: ② 메서드 시그니처 개선으로 Claim 확장성 확보현재
generateToken(String subject, TokenType)만 제공되어 추가 Claim 을 넣으려면 별도 Builder 코드가 필요한 상황입니다.
Map<String, Object> claims파라미터를 받는 오버로드 버전을 추가하면 모든 Provider 가 코드 중복 없이 Claim 을 확장할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (21)
src/main/java/com/example/solidconnection/auth/service/AuthService.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/CommonSignUpTokenProvider.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/EmailSignUpTokenProvider.java(5 hunks)src/main/java/com/example/solidconnection/auth/service/TokenBlackListService.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/TokenProvider.java(3 hunks)src/main/java/com/example/solidconnection/auth/service/oauth/OAuthSignUpTokenProvider.java(5 hunks)src/main/java/com/example/solidconnection/security/filter/AuthorizationHeaderParser.java(1 hunks)src/main/java/com/example/solidconnection/security/filter/BlacklistChecker.java(1 hunks)src/main/java/com/example/solidconnection/security/filter/JwtAuthenticationFilter.java(1 hunks)src/main/java/com/example/solidconnection/security/filter/SignOutCheckFilter.java(1 hunks)src/main/java/com/example/solidconnection/security/provider/SiteUserAuthenticationProvider.java(2 hunks)src/main/java/com/example/solidconnection/util/JwtUtils.java(0 hunks)src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java(3 hunks)src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java(1 hunks)src/test/java/com/example/solidconnection/auth/service/SignInServiceTest.java(2 hunks)src/test/java/com/example/solidconnection/auth/service/TokenBlackListServiceTest.java(1 hunks)src/test/java/com/example/solidconnection/auth/service/TokenProviderTest.java(1 hunks)src/test/java/com/example/solidconnection/auth/service/oauth/OAuthSignUpTokenProviderTest.java(3 hunks)src/test/java/com/example/solidconnection/security/filter/AuthorizationHeaderParserTest.java(1 hunks)src/test/java/com/example/solidconnection/util/JwtUtilsTest.java(0 hunks)
💤 Files with no reviewable changes (2)
- src/test/java/com/example/solidconnection/util/JwtUtilsTest.java
- src/main/java/com/example/solidconnection/util/JwtUtils.java
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/com/example/solidconnection/auth/service/TokenBlackListService.java (3)
src/main/java/com/example/solidconnection/auth/service/AuthService.java (1)
RequiredArgsConstructor(15-62)src/main/java/com/example/solidconnection/security/filter/SignOutCheckFilter.java (1)
Component(18-39)src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java (1)
Component(11-59)
src/main/java/com/example/solidconnection/security/filter/AuthorizationHeaderParser.java (2)
src/main/java/com/example/solidconnection/security/filter/SignOutCheckFilter.java (1)
Component(18-39)src/main/java/com/example/solidconnection/security/filter/JwtAuthenticationFilter.java (1)
Component(21-48)
src/main/java/com/example/solidconnection/auth/service/oauth/OAuthSignUpTokenProvider.java (1)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java (1)
RequiredArgsConstructor(35-129)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (39)
src/main/java/com/example/solidconnection/security/filter/AuthorizationHeaderParser.java (2)
8-13: 잘 구성된 컴포넌트와 상수 정의! 👏다음과 같은 훌륭한 설계 선택을 확인했습니다:
- Spring
@Component로 적절하게 정의되어 의존성 주입이 가능합니다- 토큰 관련 값들을 상수로 명확하게 정의하여 코드의 가독성을 높였습니다
- 각 상수의 목적이 명확하고 이름이 직관적입니다
이는 JWT 토큰 처리를 상속이 아닌 조합(컴포지션) 패턴으로 변경하는 리팩토링 방향에 완벽하게 부합합니다.
15-21: 옵셔널을 활용한 안전한 토큰 파싱 방식! 👍메서드 구현에서 다음과 같은 좋은 점들이 있습니다:
Optional<String>을 반환하여 null 처리를 명시적이고 안전하게 합니다- 유효하지 않은 토큰에 대한 검증 조건이 명확합니다:
- null 체크
- 빈 문자열 체크
- 올바른 접두사 확인
- 코드가 간결하면서도 필요한 모든 검증을 수행합니다
이 구현은 기존
JwtUtils.parseTokenFromRequest정적 메서드를 대체하여 테스트 가능성과 유지보수성을 향상시켰습니다.src/main/java/com/example/solidconnection/security/filter/BlacklistChecker.java (1)
1-1: 패키지 위치 변경이 적절합니다인터페이스의 패키지를
com.example.solidconnection.auth.service에서com.example.solidconnection.security.filter로 변경한 것은 다음과 같은 이유로 적절합니다:
- 이 인터페이스를 주로 사용하는
SignOutCheckFilter클래스와 같은 패키지에 위치하게 되어 관련 코드의 응집도가 높아졌습니다- 보안 필터 로직과 관련된 코드들을 한 패키지에 모아 코드 구조가 더 명확해졌습니다
- 인터페이스와 구현체의 관심사가 적절히 분리되었습니다
이 변경은 JWT 토큰 관련 클래스들을 상속이 아닌 조합 패턴으로 리팩토링하는 전체적인 방향성과 일치합니다.
src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java (3)
33-35: 의존성 주입 방식 변경이 잘 되었습니다
TokenBlackListService를 새로운 의존성으로 추가한 것은 다음과 같은 측면에서 좋은 변경입니다:
- 블랙리스트 관리 책임을 전용 서비스로 분리하여 단일 책임 원칙(SRP)을 준수합니다
- 인터페이스를 통한 의존성 주입으로 결합도를 낮추고 테스트 용이성을 높였습니다
- 관련 기능이 명확히 구분되어 코드 이해도가 향상되었습니다
이 변경은 JWT 토큰 처리를 상속 대신 조합 방식으로 리팩토링하는 방향과 일치합니다.
55-55: 토큰 블랙리스트 검증 방식 변경이 적절합니다검증 코드를
authTokenProvider.isTokenBlacklisted()에서tokenBlackListService.isTokenBlacklisted()로 변경한 것은 다음과 같은 이유로 좋습니다:
- 책임 분리가 명확해져 코드의 단일 책임 원칙(SRP)을 더 잘 준수합니다
- 토큰 생성과 블랙리스트 관리라는 서로 다른 책임이 분리되었습니다
- 테스트 코드가 실제 구현 변경을 정확히 반영하고 있습니다
이 변경으로 코드의 유지보수성과 테스트 용이성이 향상되었습니다.
75-75: 블랙리스트 검증 방식이 일관되게 적용되었습니다탈퇴 테스트에서도 블랙리스트 검증 방식을
tokenBlackListService.isTokenBlacklisted()로 일관되게 변경한 점이 좋습니다:
- 모든 테스트 케이스에서 동일한 검증 방식을 사용하여 일관성을 유지했습니다
- 리팩토링 변경사항이 전체 코드베이스에 일관되게 적용되었습니다
- 이전 코드와 기능적으로 동일하면서도 구조적으로 개선되었습니다
이러한 일관된 변경은 전체 리팩토링의 품질을 높이는 데 기여합니다.
src/test/java/com/example/solidconnection/auth/service/oauth/OAuthSignUpTokenProviderTest.java (3)
4-4: TokenProvider 임포트 추가가 적절합니다
TokenProvider를 임포트한 것은 다음과 같은 이유로 적절합니다:
- JWT 토큰 처리 책임을 전용 컴포넌트로 분리하는 리팩토링 방향과 일치합니다
- 정적 유틸리티 메서드 대신 주입 가능한 서비스를 사용하여 테스트 용이성을 높입니다
- 코드의 결합도를 낮추고 의존성을 명시적으로 만듭니다
이 변경은 전체적인 리팩토링 목표에 부합합니다.
37-39: TokenProvider 의존성 주입이 잘 구현되었습니다
TokenProvider를 의존성으로 주입받도록 변경한 점이 다음과 같은 이유로 좋습니다:
- 컴포지션 패턴을 통해 기능을 재사용하여 코드 중복을 방지합니다
- 스프링의 의존성 주입을 활용하여 결합도를 낮추고 테스트 용이성을 높입니다
- 정적 유틸리티 클래스 대신 인터페이스 기반 의존성을 사용하여 모킹이 용이해집니다
이 변경은 JWT 토큰 관련 클래스들을 상속이 아닌 조합 방식으로 리팩토링하는 방향과 일치합니다.
56-56: TokenProvider를 사용한 클레임 파싱이 적절합니다
JwtUtils.parseClaims(signUpToken, jwtProperties.secret())에서tokenProvider.parseClaims(signUpToken)로 변경한 점이 다음과 같은 이유로 좋습니다:
- 정적 유틸리티 메서드 대신 주입된 서비스를 사용하여 테스트 용이성이 향상되었습니다
- 토큰 검증 로직이 중앙화되어 코드 중복이 줄어들었습니다
- secret 값과 같은 민감한 파라미터가 캡슐화되어 보안이 향상되었습니다
이 변경으로 JWT 토큰 관련 로직의 응집도가 높아지고 재사용성이 향상되었습니다.
src/main/java/com/example/solidconnection/auth/service/AuthService.java (2)
20-20: 토큰 블랙리스트 관리 기능이 분리되었습니다 👍
단일 책임 원칙(SRP)을 따르는 좋은 리팩토링입니다:
- 블랙리스트 관리 로직이
AuthTokenProvider에서 전용 서비스로 추출됨- 각 클래스의 책임이 명확해짐
관심사 분리로 코드의 가독성과 유지보수성이 향상되었습니다:
- 토큰 생성/검증과 블랙리스트 관리가 분리됨
- 상속 대신 조합 패턴을 사용하여 더 유연한 구조 구현
30-30: 블랙리스트 관리 메서드 호출이 적절히 변경되었습니다 👍
책임 이동이 일관되게 적용되었습니다:
authTokenProvider.addToBlacklist→tokenBlackListService.addToBlacklist로 변경- 새로운 서비스의 API를 올바르게 사용함
로그아웃 로직의 흐름이 명확하게 유지됩니다:
- 토큰 변환 → 리프레시 토큰 삭제 → 액세스 토큰 블랙리스트 추가
src/main/java/com/example/solidconnection/auth/service/CommonSignUpTokenProvider.java (2)
15-15: 정적 유틸리티 대신 TokenProvider 의존성 주입을 사용합니다 👍
개선된 설계 패턴 적용:
- 직접
JwtProperties사용 대신TokenProvider의존성 주입- 상속보다 조합을 사용하는 객체지향 설계 원칙 준수
테스트 용이성 증가:
- 의존성 주입으로 테스트 시 쉽게 모킹(mocking) 가능
- 결합도 감소로 유닛 테스트 작성이 간소화됨
19-19: 토큰 파싱 로직이 TokenProvider로 위임되었습니다 👍
JWT 처리 로직 중앙화:
- 정적
JwtUtils대신tokenProvider.parseClaims()호출- 중복 코드 제거 및 JWT 처리 로직의 일관성 확보
관심사 분리 개선:
- 이 클래스는 이제 비즈니스 로직(AuthType 파싱)에만 집중
- JWT 관련 기술적 복잡성은
TokenProvider에 위임src/test/java/com/example/solidconnection/auth/service/AuthTokenProviderTest.java (2)
74-74: 메서드 이름의 오타가 수정되었습니다 👍
명확성 개선:
액세서_토큰에_해당하는_리프레시_토큰을_삭제한다→액세스_토큰에_해당하는_리프레시_토큰을_삭제한다- 정확한 용어 사용으로 가독성 향상
테스트 목적 명확화:
- 메서드 이름이 실제 기능을 더 정확하게 반영
- 문서로서의 테스트 코드 가치 상승
17-98: 블랙리스트 관련 테스트 제거 확인 👍
리팩토링과 일관된 테스트 구조 변경:
블랙리스트를_관리한다중첩 클래스가 제거됨- 블랙리스트 기능이
TokenBlackListService로 이동된 변경사항과 일치단일 책임 원칙이 테스트에도 적용됨:
- 각 클래스의 책임에 맞는 테스트 구조
TokenBlackListServiceTest에서 블랙리스트 기능 테스트가 이루어질 것으로 예상src/test/java/com/example/solidconnection/auth/service/SignInServiceTest.java (2)
27-27: JwtProperties 대신 TokenProvider 주입으로 변경되었습니다 👍
리팩토링 일관성 유지:
JwtProperties→TokenProvider변경- 다른 클래스들의 변경 패턴과 일치
의존성 개선:
- 저수준 설정(properties) 대신 추상화된 서비스에 의존
- 관심사 분리 원칙 준수
50-51: 토큰 파싱 로직이 TokenProvider로 위임되었습니다 👍
정적 유틸리티 사용 제거:
JwtUtils정적 메서드 →tokenProvider.parseSubject()호출- JWT secret 직접 전달 불필요
테스트 가독성 개선:
- 테스트 코드가 비즈니스 로직에 더 집중
- JWT 파싱 세부사항 숨김으로써 의도가 명확해짐
src/main/java/com/example/solidconnection/security/filter/SignOutCheckFilter.java (4)
13-14: Optional 임포트 추가 확인!JWT 토큰 관련 리팩토링으로 인해
Optional클래스를 사용하게 되어 필요한 임포트가 추가되었습니다.
22-23: 의존성 주입 변경이 잘 되었습니다!
변경 사항
- 정적 유틸리티 메서드 대신
AuthorizationHeaderParser를 주입받아 사용합니다BlacklistChecker인터페이스를 통해 토큰 블랙리스트 확인 책임을 위임합니다이점
- 단일 책임 원칙(SRP)을 더 잘 준수하게 되었습니다
- 코드의 테스트 용이성이 향상되었습니다
- 컴포넌트 간의 결합도가 낮아졌습니다
29-30: 토큰 파싱 로직이 개선되었습니다!
변경 사항
authorizationHeaderParser.parseToken(request)를 사용하여 토큰 파싱 로직을 위임합니다Optional<String>을 활용하여 토큰 존재 여부를 명시적으로 처리합니다이점
- null 체크 대신
Optional을 사용하여 더 안전하고 읽기 쉬운 코드가 되었습니다- 토큰 추출 로직이 별도 컴포넌트로 분리되어 재사용성이 향상되었습니다
36-38: 블랙리스트 확인 로직이 잘 리팩토링되었습니다!
blacklistChecker.isTokenBlacklisted(accessToken)호출을 통해 토큰 블랙리스트 확인 책임을 인터페이스에 위임함으로써 유연성이 향상되었습니다.src/main/java/com/example/solidconnection/security/filter/JwtAuthenticationFilter.java (4)
17-18: Optional 임포트 추가 확인!JWT 토큰 관련 리팩토링으로 인해
Optional클래스를 사용하게 되어 필요한 임포트가 추가되었습니다.
25-26: AuthorizationHeaderParser 의존성 주입이 잘 추가되었습니다!정적 유틸리티 메서드 대신
AuthorizationHeaderParser를 주입받아 사용함으로써 단일 책임 원칙을 더 잘 준수하고 테스트 용이성이 향상되었습니다.
32-36: 토큰 파싱 및 검증 로직이 개선되었습니다!
변경 사항
authorizationHeaderParser.parseToken(request)를 사용하여 토큰 파싱 로직을 위임합니다Optional<String>을 활용하여 토큰 존재 여부를 명시적으로 처리합니다이점
- null 체크 대신
Optional.isEmpty()를 사용하여 더 명확하고 안전한 코드가 되었습니다- 토큰 추출 로직이 별도 컴포넌트로 분리되어 재사용성이 향상되었습니다
- 코드 가독성이 향상되었습니다
38-43: Optional에서 토큰을 안전하게 추출합니다!
token.get()을 사용하여 Optional에서 토큰을 안전하게 추출하고 있습니다. 이전 조건문에서isEmpty()를 확인했기 때문에 안전합니다.src/test/java/com/example/solidconnection/auth/service/TokenBlackListServiceTest.java (1)
1-57: 잘 작성된 테스트 클래스입니다!이 테스트 클래스는 TokenBlackListService의 기능을 검증하기 위한 좋은 예시입니다. 다음과 같은 장점들이 있습니다:
테스트 구조
- 명확한 given/when/then 구조로 테스트가 잘 구성되어 있습니다.
- 중첩 클래스(Nested)를 활용하여 관련 테스트들을 그룹화했습니다.
검증 범위
- 블랙리스트 추가 기능 검증
- 블랙리스트 토큰 확인 기능의 긍정/부정 케이스 모두 검증
테스트 가독성
- 한글 메서드명을 사용하여 테스트 의도가 명확합니다.
- RedisTemplate을 직접 사용하여 저장된 값을 검증하는 방식이 적절합니다.
모든 테스트 시나리오가 잘 구현되어 있으며, 새로운 TokenBlackListService의 기능을 충분히 검증하고 있습니다.
src/main/java/com/example/solidconnection/auth/service/oauth/OAuthSignUpTokenProvider.java (5)
11-31: 상속에서 조합으로의 성공적인 전환!이 변경은 PR의 목표인 "JWT 토큰 관련 클래스를 상속이 아니라 조합하도록" 리팩토링하는 것을 잘 구현했습니다:
클래스 구조 개선
@RequiredArgsConstructor를 추가하여 생성자 주입 패턴 적용- 기존 상속 대신 TokenProvider를 의존성으로 주입받음
관심사 분리
- 토큰 관련 공통 로직을 TokenProvider에 위임하여 코드 중복 감소
- 이 클래스는 OAuthSignUp 관련 특화된 비즈니스 로직에만 집중할 수 있음
이러한 변경은 SOLID 원칙 중 단일 책임 원칙(SRP)과 인터페이스 분리 원칙(ISP)을 더 잘 준수하게 되었습니다.
45-45: 토큰 저장 로직 위임 잘 적용했습니다TokenProvider 컴포넌트에 토큰 저장 로직을 위임하여 코드 일관성이 향상되었습니다. 이전에는 상속받은 메서드를 호출했거나 직접 구현했을 것으로 추정되는데, 이제 주입된 tokenProvider 인스턴스를 통해 동일한 기능을 사용하고 있습니다.
56-56: 토큰 검증 로직 위임 적절히 적용됨토큰의 클레임을 파싱하는 작업을 tokenProvider에 위임하여 JWT 파싱 로직의 중앙화가 잘 이루어졌습니다. 이로 인해 JWT 관련 예외 처리나 로직 변경 시 한 곳만 수정하면 되므로 유지보수성이 크게 향상됩니다.
72-74: 주제 파싱 로직 위임 잘 적용됨이메일 파싱 메서드에서도 tokenProvider를 활용하여 일관된 구현 패턴을 유지하고 있습니다. 이는 코드의 일관성과 가독성을 높이는 좋은 접근 방식입니다.
76-80: AuthType 파싱 로직 개선됨AuthType 파싱 메서드에서도 tokenProvider를 활용하여 클레임 파싱 작업을 위임하고 있습니다. 이 클래스는 이제 비즈니스 로직(AuthType 변환)에만 집중하고, JWT 토큰 파싱 같은 기술적 세부사항은 TokenProvider가 담당하게 되었습니다.
src/test/java/com/example/solidconnection/auth/service/TokenProviderTest.java (5)
25-54: 종합적인 토큰 생성 테스트 잘 구현됨TokenProvider의 토큰 생성 기능을 검증하는 테스트가 잘 구현되어 있습니다:
테스트 검증 포인트
- 생성된 토큰에서 subject가 정확히 추출되는지 확인
- 토큰의 만료 시간이 지정된 TokenType의 만료 시간과 일치하는지 검증
테스트 구조
- given/when/then 구조가 명확함
- assertAll을 사용하여 여러 검증을 한번에 수행
이 테스트는 TokenProvider의 핵심 기능인 토큰 생성 로직을 효과적으로 검증합니다.
56-73: 토큰 저장 기능 테스트 완벽하게 구현됨토큰 저장 기능 테스트가 매우 철저하게 구현되어 있습니다:
검증 내용
- Redis에 저장된 키 형식 검증 (TokenType 접두사 + subject)
- 저장된 값이 원본 토큰과 일치하는지 확인
- saveToken 메서드의 반환값이 저장된 토큰과 일치하는지 확인
구현 방식
- RedisTemplate을 직접 사용하여 저장된 값 검증
- assertAll을 통한 다중 검증
이런 종합적인 테스트는 TokenProvider 리팩토링의 안정성을 높이는데 큰 도움이 됩니다.
75-102: subject 추출 테스트 케이스 잘 구성됨토큰으로부터 subject를 추출하는 기능에 대한 테스트가 잘 구성되어 있습니다:
테스트 범위
- 유효한 토큰에서 subject 추출 검증
- 유효하지 않은 토큰(만료된 토큰)에서 예외 발생 검증
예외 검증
- CustomException 타입 검증
- 예외 메시지가 INVALID_TOKEN 에러 코드와 일치하는지 확인
중첩 클래스를 사용하여 관련 테스트를 그룹화한 것도 코드 가독성을 높이는 좋은 방법입니다.
104-138: claim 추출 테스트 케이스 완벽하게 구성됨토큰으로부터 claim을 추출하는 기능에 대한 테스트가 철저하게 구성되어 있습니다:
테스트 범위
- 유효한 토큰에서 여러 claim 값 추출 검증
- 유효하지 않은 토큰에서 예외 발생 검증
구체적인 검증
- subject와 custom claim 값 모두 검증
- 예외 타입과 메시지 검증
이 테스트는 TokenProvider가 다양한 claim을 올바르게 처리하는지 확인하는 중요한 역할을 합니다.
140-174: 테스트 헬퍼 메서드 잘 구현됨테스트에 필요한 유효한/만료된 토큰을 생성하는 헬퍼 메서드들이 잘 구현되어 있습니다:
메서드 구성
- subject만 사용하는 버전과 claims를 직접 설정하는 버전 모두 제공
- 유효한 토큰과 만료된 토큰을 생성하는 메서드 분리
구현 품질
- 메서드명이 명확하여 사용 목적을 쉽게 이해할 수 있음
- 실제 토큰 생성 로직과 유사하게 구현되어 현실적인 테스트 가능
이런 헬퍼 메서드는 테스트 코드의 중복을 줄이고 가독성을 높이는 좋은 예시입니다.
src/main/java/com/example/solidconnection/security/provider/SiteUserAuthenticationProvider.java (3)
3-3: TokenProvider 임포트 추가 적절함TokenProvider 컴포넌트를 임포트하여 JWT 관련 책임을 위임하기 위한 준비가 잘 되어 있습니다. 이는 전체적인 리팩토링 방향성에 부합합니다.
20-20: TokenProvider 의존성 주입 잘 적용됨TokenProvider를 의존성으로 주입받도록 변경하여 JWT 토큰 파싱 책임을 위임하는 구조로 잘 개선되었습니다. 이 변경은 코드의 결합도를 낮추고 테스트 용이성을 높이는 효과가 있습니다.
27-27: 토큰 파싱 로직 위임 적절히 구현됨기존에 static 유틸리티 메서드였을 것으로 추정되는 JWT 파싱 로직을 주입된 tokenProvider 컴포넌트에 위임하여 코드 결합도가 낮아졌습니다. 이는 다음과 같은 장점이 있습니다:
테스트 용이성 향상
- static 메서드 대신 모킹 가능한 인터페이스 호출로 변경되어 단위 테스트가 용이해짐
코드 유지보수성 향상
- JWT 파싱 로직 변경 시 TokenProvider만 수정하면 됨
- 의존성이 명시적으로 드러나 코드 가독성 향상
관심사 분리
- 인증 제공자는 인증 로직에만 집중하고, 토큰 파싱은 TokenProvider에 위임
이러한 변경은 PR 목표인 "JWT 토큰 관련 클래스를 상속이 아니라 조합하도록" 리팩토링하는 방향에 완벽하게 부합합니다.
- 추가로, null 이 아니라 Optional 을 반환하여 빈 값 처리를 강제하도록 수정
- JwtUtils를 사용하던 곳에서 TokenProvider를 주입받아 사용하게 하기 위한 사전 작업
- AS-IS: JwtUtils에 정의된 xxxParse 함수를 사용하기 위해서는, 함수 자체가 static이라서 호출하는 곳으로부터 jwt 시크릿 키를 건네받아야했다. 이를 위해 여러곳에서 JwtProperties를 주입받아 jwtProperties.secret()을 호출하며 xxxParse()함수에 값을 넘겨줬다. - TO-BE: TokenProvider를 컴포넌트로 만듦으로서, TokenProvider의 내부에서 jwt 시크릿 키를 주입받을 수 있게 되었다. 함수의 인자로 시크릿키를 받을 필요가 없게 되었으므로 함수 인자를 수정한다.
- AS-IS : TokenProvider가 추상 클래스일 때는 이를 상속하는 클래스들로 TokenProvider의 기능을 테스트했다. - TO-BE : TokenProvider도 컴포넌트가 되었으므로, 테스트 코드를 작성한다.
- parseSubject가 parseClaims를 호출하는데, 이들이 발생시키는 예외의 종류가 다르면 안된다.
- security가 auth를 의존하지 않게 하기 위해, BlackListChecker 인터페이스를 security패키지 내부로 옮긴다.
nayonsoso
force-pushed
the
refactor/296-use-composition-for-token
branch
from
31ba489 to
a10c991
Compare
|
고생하셨습니다! 저도 아직 제 생각이 완전히 정리되지 않아서 코드를 좀 더 읽고 고민해야할 거 같은데 너무 늦어질까봐 느낀 점들을 우선 남겨보겠습니다.. 🥲
이걸 TokenProvider 쪽으로 옮기는 거 어떤가요? 그러면 JwtProperties를 주입받을 필요도 없어질 거 같습니다!
as-is to-be
전반적으로 변경 전파 감소, 테스트 용이성, 느슨한 결합도 측면에서 정말 고민하신 게 많이 느껴지고 잘 반영된 거 같습니다!! |
There was a problem hiding this comment.
열심히 고민한 PR을 정성껏 봐주시는 것만큼 감사한게 없습니다~~ 감사합니다
1/
사실 JWT 외에 다른 것으로 바뀔일은 없다고 봐도 되니 굳이 추상화를 할 필요까진 없을 거 같긴 하지만...
이 말씀도 맞습니다😅
이번 변경은 ‘실제 일어날 변경을 최소화’하기보다, ‘프로덕션 코드는 인프라를 몰라야한다’라는 원론적인 관점에서 진행한거긴해요.
그렇다면 그냥 JwtTokenProvider이름이 더 적절한 거 아닌가란 생각도 드네요!
저도 해당 클래스 자체의 이름은 JwtTokenProvider가 좋다 생각하는데요,
그렇게 되면 이를 사용하는 서비스들이 JwtTokenProvider를 의존하게되는 것이니
이름으로 인프라를 노출하는게 아닌가~ 하는 생각도 듭니다.
그래서 해당 클래스 이름은 JwtTokenProvider로 바꾸고,
TokenProvider 인터페이스를 auth.service 하위에 두는 것을 생각해봤는데 이건 어떤가요?
조금 과하려나요 ㅎㅎ;
2/
EmailSignUpTokenProvider, OAuthSignUpTokenProvider에서 여전히 JWT를 직접 사용하고 있는데 이걸 TokenProvider 쪽으로 옮기는 거 어떤가요?
이번 pr과는 조금 떨어진 이야기지만 AuthController에서 "/email/sign-up", "/sign-up"들만 관련된 tokenprovider를 직접 호출하는 게 조금 어색하다는 생각이 들었네요. 얘네도 똑같이 Service에서 호출하는 게 더 일관성이 있을 거 같다는 생각이 들었습니다!
네 저도 리팩터링을 진행하며,signUpTokenProvider 를 사용하는 쪽에서 아쉬움을 느꼈습니다🥲
이후의 PR에서 리팩터링 진행해보겠습니다!
3/
여기 패키지 구조좀 나누면 더 이해하기 편할 거 같다는 생각이 듭니다! 토큰관련을 따로 뺀다던가 해도 좋겠네요!
동의합니다~ client도 인프라라 따로 둔 것처럼, token도 같은 위치(auth 바로 밑에) 패키지를 만들면 좋을 것 같은데 어떤가요?
4/
모든 예외를 INVALID_TOKEN로 해도 괜찮을까란 생각이 들긴 했는데 조금 과한 거 같기도 하네요!
사용자에게 구체적인 예외를 알려주는 것도 좋을 수 있는데요, 그건 ‘사용자가 복구할 수 있는 경우’에 해당한다 생각해요.
e.g. 닉네임은 빈칸이면 안된다 등..
그런데 토큰에 대해서 너무 구체적으로 예외 메세지를 주면,
저희가 주는 정보를 힌트삼아 접근할 수 있으니 보안상 위험할 수 있다 생각합니다. 🫨
1/저도 이게 가장 좋다고 생각하긴 했습니다! ㅎㅎ 좋습니다!! 2/역시 해당 고민을 하고 계셨군요 👍 3/좋은 거 같습니다~~ 4/으앗 그말도 맞네요! 제가 너무 사용자관점만 생각했던 거 같네요 🥲 다음 pr에서도 추가작업을 하시는거라면 그 pr에서 고민을 더 이어가도 좋을 거 같습니다!! |
- TokenProvider, TokenBlackListService를 auth.token 으로 - JwtProperties를 auth.token.config 로
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java (1)
27-37: ① 매개변수 이름string은 의미가 모호합니다
토큰 subject 역할이므로subject(또는memberId등 도메인 용어)로 변경하면 가독성이 향상됩니다.-public final String generateToken(String string, TokenType tokenType) { - Claims claims = Jwts.claims().setSubject(string); +public final String generateToken(String subject, TokenType tokenType) { + Claims claims = Jwts.claims().setSubject(subject);src/main/java/com/example/solidconnection/auth/service/TokenProvider.java (1)
6-14: ① 인터페이스 시그니처도 의미 있는 파라미터명을 사용하세요
구현체(PR 내JwtTokenProvider)와 달리 인터페이스는 그대로string으로 남아 혼란을 줄 수 있습니다.subject로 통일하면 좋겠습니다.-String generateToken(String string, TokenType tokenType); +String generateToken(String subject, TokenType tokenType);src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java (5)
1-38: JwtTokenProvider 테스트 구조 설계에 대한 피드백테스트 클래스의 기본 구조와 의존성 주입이 잘 설계되었습니다. 몇 가지 개선할 수 있는 부분을 제안합니다:
패키지 구조 일관성
- 테스트 클래스는
auth.service패키지에 있지만 JwtTokenProvider는auth.token에 있습니다. 테스트 클래스도 동일한 패키지 구조를 따르는 것이 더 일관성 있을 것 같습니다.테스트 이름과 구조
- 한글 테스트 메소드 이름이 명확하고 좋습니다만, DisplayName 어노테이션도 각 테스트 메소드에 추가하면 테스트 리포트에서 더 읽기 쉬울 것입니다.
의존성 주입
- JwtProperties와 RedisTemplate 주입은 테스트 목적에 적합하지만, 실제 토큰 생성 로직을 테스트하는 경우에는 Mock으로 대체하는 방식도 고려해볼 수 있습니다.
39-55: 토큰 생성 테스트 - 좋은 접근 방식입니다토큰 생성 테스트가 잘 작성되었습니다. 토큰 생성 후 실제로 해당 토큰에서 claims를 파싱하여 검증하는 방식이 적절합니다.
검증 강화 포인트
- 현재 검증은 subject와 만료 시간에 초점을 맞추고 있는데, issuedAt 같은 다른 메타데이터도 확인하면 더 완전한 테스트가 될 것 같습니다.
- TokenType에 따라 다른 만료 시간이 설정되는지 여러 TokenType 케이스를 테스트하면 좋을 것 같습니다.
코드 개선 제안
@DisplayName("토큰 타입에 따라 적절한 만료 시간으로 토큰을 생성한다") @ParameterizedTest @EnumSource(TokenType.class) void 토큰을_타입별로_생성한다(TokenType tokenType) { // given String subject = "subject123"; // when String token = tokenProvider.generateToken(subject, tokenType); // then Claims claims = tokenProvider.parseClaims(token); long actualExpireTime = claims.getExpiration().getTime() - claims.getIssuedAt().getTime(); assertAll( () -> assertThat(claims.getSubject()).isEqualTo(subject), () -> assertThat(actualExpireTime).isEqualTo(tokenType.getExpireTime()) ); }
57-74: 토큰 저장 테스트 - 완성도 높은 테스트입니다Redis에 토큰 저장하는 기능 테스트가 명확하게 작성되었습니다. 키와 값이 예상대로 저장되는지 검증하는 부분이 잘 되어 있습니다.
만료 시간 검증 추가
- Redis에 저장된 토큰의 TTL(Time To Live)도 검증하면 더 완전한 테스트가 될 것 같습니다.
// then 부분에 추가 Long ttl = redisTemplate.getExpire(key); assertThat(ttl).isGreaterThan(0L); assertThat(ttl).isLessThanOrEqualTo(tokenType.getExpireTime() / 1000);다양한 TokenType 테스트
- 다양한 TokenType에 대해 저장이 잘 되는지 확인하기 위해 ParameterizedTest를 활용하면 좋을 것 같습니다.
105-139: Claims 추출 테스트 - 에러 처리 세분화 필요Claims 추출 테스트가 잘 작성되어 있습니다. 유효한 토큰과 유효하지 않은 토큰에 대한 테스트가 모두 포함되어 있습니다.
예외 응답 세분화
- Subject 추출 테스트와 마찬가지로, 단순히 INVALID_TOKEN으로 모든 예외를 처리하기보다 TOKEN_EXPIRED, TOKEN_SIGNATURE_INVALID, TOKEN_MALFORMED 등으로 세분화하는 것이 좋을 것 같습니다.
다양한 Claim 유형 테스트
- 현재는 단일 String 타입의 Claim만 테스트하고 있는데, List, Map, Boolean, Number 등 다양한 타입의 Claim도 테스트하면 더 견고한 테스트가 될 것 같습니다.
@Test void 다양한_타입의_Claim을_추출한다() { // given String subject = "subject"; Map<String, Object> claims = new HashMap<>(); claims.put("stringKey", "stringValue"); claims.put("numberKey", 123); claims.put("booleanKey", true); claims.put("listKey", List.of("item1", "item2")); Claims expectedClaims = Jwts.claims(claims).setSubject(subject); String token = createValidToken(expectedClaims); // when Claims actualClaims = tokenProvider.parseClaims(token); // then assertAll( () -> assertThat(actualClaims.getSubject()).isEqualTo(subject), () -> assertThat(actualClaims.get("stringKey")).isEqualTo("stringValue"), () -> assertThat(actualClaims.get("numberKey")).isEqualTo(123), () -> assertThat(actualClaims.get("booleanKey")).isEqualTo(true), () -> assertThat(actualClaims.get("listKey", List.class)).containsExactly("item1", "item2") ); }
141-175: 헬퍼 메소드 - 코드 중복 제거 방안토큰 생성 헬퍼 메소드들이 테스트 코드를 간결하게 만들어주고 있습니다. 그러나 코드 중복을 줄일 수 있는 개선 포인트가 있습니다:
코드 중복 제거
createValidToken과createExpiredToken메소드들 사이에 중복된 코드가 많습니다. 공통 로직을 추출하여 중복을 제거하면 좋을 것 같습니다.private String createToken(String subject, long expirationOffsetMillis) { return Jwts.builder() .setSubject(subject) .setIssuedAt(new Date()) .setExpiration(new Date(System.currentTimeMillis() + expirationOffsetMillis)) .signWith(SignatureAlgorithm.HS256, jwtProperties.secret()) .compact(); } private String createToken(Claims claims, long expirationOffsetMillis) { return Jwts.builder() .setClaims(claims) .setIssuedAt(new Date()) .setExpiration(new Date(System.currentTimeMillis() + expirationOffsetMillis)) .signWith(SignatureAlgorithm.HS256, jwtProperties.secret()) .compact(); } private String createValidToken(String subject) { return createToken(subject, 1000); } private String createValidToken(Claims claims) { return createToken(claims, 1000); } private String createExpiredToken(String subject) { return createToken(subject, -1000); } private String createExpiredToken(Claims claims) { return createToken(claims, -1000); }추가 헬퍼 메소드
- 서명이 유효하지 않은 토큰, 잘못된 형식의 토큰 등을 생성하는 헬퍼 메소드도 추가하면 더 다양한 예외 케이스를 테스트하는 데 유용할 것 같습니다.
private String createTokenWithInvalidSignature(String subject) { String token = createValidToken(subject); // 마지막 문자를 변경하여 서명을 손상시킴 return token.substring(0, token.length() - 1) + (token.charAt(token.length() - 1) == '0' ? '1' : '0'); } private String createMalformedToken() { return "malformed.token.string"; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (15)
src/main/java/com/example/solidconnection/auth/service/AuthService.java(3 hunks)src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java(2 hunks)src/main/java/com/example/solidconnection/auth/service/EmailSignUpTokenProvider.java(5 hunks)src/main/java/com/example/solidconnection/auth/service/TokenProvider.java(1 hunks)src/main/java/com/example/solidconnection/auth/service/oauth/OAuthSignUpTokenProvider.java(5 hunks)src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java(1 hunks)src/main/java/com/example/solidconnection/auth/token/TokenBlackListService.java(1 hunks)src/main/java/com/example/solidconnection/auth/token/config/JwtProperties.java(1 hunks)src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java(6 hunks)src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java(1 hunks)src/test/java/com/example/solidconnection/auth/service/TokenBlackListServiceTest.java(1 hunks)src/test/java/com/example/solidconnection/auth/service/oauth/OAuthSignUpTokenProviderTest.java(3 hunks)src/test/java/com/example/solidconnection/security/filter/JwtAuthenticationFilterTest.java(1 hunks)src/test/java/com/example/solidconnection/security/filter/SignOutCheckFilterTest.java(1 hunks)src/test/java/com/example/solidconnection/security/provider/SiteUserAuthenticationProviderTest.java(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/main/java/com/example/solidconnection/auth/token/config/JwtProperties.java
- src/test/java/com/example/solidconnection/security/filter/JwtAuthenticationFilterTest.java
- src/test/java/com/example/solidconnection/security/filter/SignOutCheckFilterTest.java
- src/test/java/com/example/solidconnection/security/provider/SiteUserAuthenticationProviderTest.java
🚧 Files skipped from review as they are similar to previous changes (7)
- src/main/java/com/example/solidconnection/auth/service/AuthService.java
- src/test/java/com/example/solidconnection/auth/service/AuthServiceTest.java
- src/test/java/com/example/solidconnection/auth/service/oauth/OAuthSignUpTokenProviderTest.java
- src/main/java/com/example/solidconnection/auth/service/oauth/OAuthSignUpTokenProvider.java
- src/main/java/com/example/solidconnection/auth/service/EmailSignUpTokenProvider.java
- src/test/java/com/example/solidconnection/auth/service/TokenBlackListServiceTest.java
- src/main/java/com/example/solidconnection/auth/service/AuthTokenProvider.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
src/main/java/com/example/solidconnection/auth/token/TokenBlackListService.java (1)
15-16: ② 상수명·클래스명 CamelCase 통일 제안
TokenBlackListService→TokenBlacklistService처럼Blacklist를 한 단어로 쓰면 CamelCase 일관성이 높아집니다. 마찬가지로SIGN_OUT_VALUE는 의미상"signedOut"정도가 더 자연스러울 수 있습니다.
[ suggest_optional_refactor ]
src/main/java/com/example/solidconnection/auth/token/TokenBlackListService.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/auth/token/JwtTokenProvider.java
Show resolved
Hide resolved
src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java
Show resolved
Hide resolved
src/test/java/com/example/solidconnection/auth/service/JwtTokenProviderTest.java
Show resolved
Hide resolved
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
확인했습니다! 사소한 리뷰 하나만 남겼습니다 ㅎㅎ
|
|
||
| import static com.example.solidconnection.util.JwtUtils.parseSubject; | ||
|
|
2 participants
관련 이슈
작업 내용
작업 항목은 커밋 단위로 따라가시면 이해가 쉬울 것입니다.
이 PR 설명에서는 ‘왜 이렇게 구현했는지’, 그 과정에서의 고민을 공유합니다. 😊
📌 JwtUtils 삭제
초기에는 JWT 전반에 필요한 기능을 담기 위해 JwtUtils 유틸 클래스(public static 메서드 집합)로 구성했습니다.
하지만 이는 다음과 같은 문제로 이어졌습니다. 😓
이에 JwtUtils를 삭제하고 관련 코드를 TokenProvider 내부로 옮겼습니다.
📌 TokenProvider를 상속 → 조합 방식으로 변경
처음 296 이슈를 만든 이유는, JWT와 무관한 테스트 코드에서 JwtException이 발생하는 상황이 싫었기 때문입니다.
JWT와 전혀 무관한 테스트에서도 내부적으로 JWT 파싱이 일어나면서 테스트의 목적이 흐려졌습니다.
이를 해결하기 위해TokenProvider를 빈으로 주입받는다면,
JWT 관련 부분을 mock 처리할 수 있기 때문에 JwtException 이 발생하지 않고 테스트가 단순해질 것이라 생각했습니다.
하지만 위처럼 구성하려면
결국 테스트 코드를 작성하는 개발자는 어느곳에서 JWT 파싱이 일어나는지 알고 있어야하므로,
처음 문제시했던 것이 반복됩니다.
따라서, 처음의 목적을 해결하기 위해서 'TokenProvider를 상속 → 조합 방식으로 변경'할 필요는 없다고 생각했습니다.
📌 처음의 목적을 달성할 필요가 없다 생각했음에도 조합 방식으로 바꾼 이유
1/ 프로덕션 코드에서 JwtProperties 노출 감소
ed4f4ce 커밋에서도 볼 수 있듯,
TokenProvider를 조합 방식으로 바꿈으로써 서비스 코드에서 JwtProperties가 직접 사용되는 경우를 줄일 수 있었습니다.
서비스 레이어는 JWT라는 인프라에 대해서 최대한 알지 않는 것이 바람직합니다.
2/ 변경 전파 문제
처음에는 OAuthSignUpTokenProvider와 TokenProvider가 is-a 관계라 생각해서 상속으로 구현하게 했습니다.
순수한 자바 객체라면 문제가 되지 않을 수 있지만,
OAuthSignUpTokenProvider 가 스프링 빈으로 등록되어야하기에 문제가 발생했습니다.
빈이 다른 빈을 상속하는 것은 문제가 될 수 있습니다.
따라서 TokenProvider를 순수 자바 객체로 만들고, 자식 클래스에서 TokenProvider의 생성자를 호출하여 필드를 초기화하게 했습니다.
하지만 이런 구조에서는 TokenProvider가 내부 의존성을 변경하면
하위 클래스인 OAuthSignUpTokenProvider도 변경을 강요받게 됩니다.
→ 조합으로 변경하면 이러한 변경 전파를 줄였습니다.