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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FEAT/#18] 소셜 로그인 유즈케이스 구현 #19
base: dev
Are you sure you want to change the base?
[FEAT/#18] 소셜 로그인 유즈케이스 구현 #19
Changes from 14 commits
5eb21f9
3c0aa67
3549802
5b3f295
54d0cb3
c76c61d
f2a1387
1c8386e
5623edd
14bb8f6
a88880f
875b1d7
b6b40b5
a29c24e
602403f
105348b
ac028b4
21186a8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
p3
사소한 것이긴 한데
web/login
/app/login
으로 지정하신 이유가 따로 있는지 궁금합니다!!저는 보통 path를 정할 때, 패키징을 많이 생각하는데요!
login
까지는 공통 관심사이고 플랫폼(web/app)에서 관심사가 나뉘어지는 논리적인 상황에서플랫폼에 대한 path가 먼저 나오게 된다면 플랫폼이 먼저 구분지어진다고 느껴져요!!
또한 Controller의 정의 규칙도 보통 path 기반으로 관심사를 분리하고 정의하게 되는데
위와 같은 방식으로 진행한다면 앞으로의 플랫폼 구분이 필요하 기능들은 플랫폼 path가 앞에 오게될거에요!!
그로 인해 Controller 분리가 필요해진다면 login 기능만 분리한 AuthLoginController 로 분리되기 보다는 AppController, WebController로 분리될 가능성이 커보이구요!!
물론 정답은 없는 요소이지만 정책을 정하면 좋을 것 같아 얘기해봅니다!!
(cc. @hyunw9 씨의 의견도 궁금해요!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이전에 전달받은 토큰 운용 관련 문서(cc. https://www.notion.so/sopt-makers/API-Token-045310ea8c5749e3b27846e2384b1c5b?pvs=4) 대로 구현을 하긴 했는데요!
다시 여쭤보지 않았던 이유는 이 문서를 전달해주실 때 제가 없는 자리에서 관련된 이야기가 나왔는데 그 내용을 제가 인지하지 못하고 있어서 오빠가 관련 내용을 작성해서 보내주신다고 하셨던거로 기억해요. 그래서 이미 논의가 끝난 사안이라고 생각하여 문서에 적힌 경로 및 Response DTO 및 토큰을 그대로 구현했습니다. 만약 클라이언트와 협의된 내용이 아니고 오빠가 작성해주신 내용이 클라이언트 분들과 경로에 대해 협의가 된 사안이 아니라 변경해도 된다면 저도 변경하는 것이 더 좋다고 생각합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3
해당 어노테이션은 UserJpaRepository <>에 붙이는게 더 적절하다고 생각하는데, 성은님 의견이 궁금합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
옹..!! 저는 성은이 방식이 적합하다고 생각했는데!!
본래
@Controller
,@Service
,@Repository
어노테이션들은 물론 각자마다의 기능이 조금씩 포함되어 있지만"역할"을 나타내는게 강한 친구라고 생각해요!
(만약 빈 등록만이 목적이라면
@Component
를 사용해도 무관하니까요!!)본 객체는 Usercase In 포트를 구현하는 Service 객체에 주입이 필요하니 빈 등록이 필요함과 동시에 "저장소"라는 의미를 전달할 수 있어야 하기 때문에
@Component
보다는@Repository
가 더 적합해 보이구요!!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JpaRepository를 상속받을 경우 스프링부트에서는 자동으로 빈 등록을 해준다고 하더라고요
그래서 중복해서 어노테이션을 붙일 필요는 없다는 입장이었습니다!
다만 in 포트를 구현해야하는 객체인 UserRepositoryImpl은 빈 등록을 해야하기에 고민하다가 Repository를 선택하게 되었습니다! 동규오빠와 같은 의견이었던 것 같아요 ㅎㅎ