hyuk0309 commented 2 years ago

🛠️ 작업 내용

user 모듈 리팩토링

🗨️ 기타

user 모듈 패키지 구조를 변경했습니다.
- User 모듈은 외부 API도 사용하고, 추후 Redis도 도입할 예정이라 Application 영역의 필요성을 느꼈습니다. 그래서 Application 영역을 도입했습니다. 다른 모듈에서는 해당 모듈의 특성에 맞게 Application 영역 도입을 결정해도 좋을 것 같습니다.
기존 Domain 패키지 이름을 Internal로 바꿨습니다. Domain 레이어를 도입하니 이름에 충돌이 생겨 적절한 이름으로 변경했습니다.

아직 남아있는 고민

현재 User 모델이 Profile 모델에 대한 참조를 갖고 있습니다. 옛날에는 User가 Profile의 부모 개념이라고 강하게 느껴 그렇게 설계한 것 같습니다. 하지만 지금와서 생각해보니 User는 인증을 위한 개념이고 Profile은 비지니스 로직상 사용자와 관련된 개념이라고 생각합니다. 따라서 둘은 상-하 관계가 아니라 개념적으로 동등한 관계인 것 같습니다. 그리고 인증 과정에서는 Profile의 유무가 중요하지 않습니다. 반면에 Profile의 경우 인증 여부가 필요합니다. 따라서 Profile 모델이 UserId를 갖고 있는게 더 적절하다는 생각이 들었습니다.

😎 리뷰어

@ndy2

github-actions[bot] commented 2 years ago

Unit Test Results

    82 files     82 suites 1m 2s :stopwatch:   649 tests   647 :heavy_check_mark: 2 :zzz: 0 :x: 1 449 runs 1 447 :heavy_check_mark: 2 :zzz: 0 :x:

Results for commit 48243ebd.

:recycle: This comment has been updated with latest results.

codecov-commenter commented 2 years ago

Codecov Report

Merging #231 (48243eb) into develop (db48104) will not change coverage. The diff coverage is 93.10%.

@@            Coverage Diff             @@
##             develop     #231   +/-   ##
==========================================
  Coverage      89.24%   89.24%           
  Complexity       337      337           
==========================================
  Files             71       71           
  Lines           1199     1199           
  Branches          49       49           
==========================================
  Hits            1070     1070           
  Misses           111      111           
  Partials          18       18

Impacted Files	Coverage Δ
...guration/security/jwt/JwtAuthenticationFilter.java	`87.87% <ø> (ø)`
...kocean/configuration/security/jwt/JwtProvider.java	`64.28% <ø> (ø)`
...ocean/configuration/security/jwt/SecurityUser.java	`90.90% <ø> (ø)`
...ration/security/oauth/CustomOAuth2UserService.java	`46.66% <ø> (ø)`
...onfiguration/security/oauth/SecurityOAuthType.java	`100.00% <ø> (ø)`
...kocean/controller/bookmark/BookmarkController.java	`100.00% <ø> (ø)`
...kocean/controller/bookmark/CategoryController.java	`100.00% <ø> (ø)`
...kocean/controller/bookmark/ReactionController.java	`100.00% <ø> (ø)`
...ontroller/linkmetadata/LinkMetadataController.java	`100.00% <ø> (ø)`
...ontroller/notification/NotificationController.java	`100.00% <ø> (ø)`
... and 52 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

ndy2 commented 2 years ago

패키지 구조와 dependency test 에서의 변경만 있었군요

그런데 보통 domain 같은 패키지의 경우 중첩된 이름으로 사용하는 경우를 꽤 많이 본것 같아요

userId 의 위치에 대한 고민은

제가 지금 모듈? MSA 를 강의 기반으로 학습중이라고 어제 말씀드렸었는데 여기서 제안하는(정도는 아니고 그냥 강의 중에 나오는) 구조를 한번 소개해드리고 의견을 들어보고 싶네요.

그 방법은 간단하게 말씀드리면

user 모듈이 로그인에 따른 토큰의 발급을 책임지지만 다른 모듈이 인증 여부가 필요할때는 user 모듈에 요청하는 것이 아니라

api-gateway 에서 user 모듈과 공유한 secret 을 사용해 jwt 의 유효성을 검사하고 다른 모듈에 요청을 넘기는 방식입니다.

hyuk0309 commented 2 years ago

그렇군요! 어찌보면 api-gateway 역할이 저희 JwtAuthenticationFilter와 비슷한 것 같네요.

그리고 현재 로직이 jwtAuthenticationFiler에서 User 테이블에 접근해 profileId를 같이 가져오는고, 이걸로 AuthenticationToken을 만들어주고 있어요.

근데 Jwt를 발급할 때는 Claim에 userId 밖에 없죠. 이게 좀 어색한 것 같아요. 필터에서 DB에 접근하는것도 좋아보이지 않고요. 오히려 profile이 userId를 갖고, AuthenticationToken에는 userId만 갖고 있으면 모든 문제가 해결될 것 같아요.

prgrms-web-devcourse / Team-Meoguri-Linkocean-BE