-
Notifications
You must be signed in to change notification settings - Fork 0
[FEAT] 9주차 미션_오아시스 #42
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
Open
wnsgh7368
wants to merge
1
commit into
oasis
Choose a base branch
from
oasis-week9
base: oasis
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
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
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
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
55 changes: 55 additions & 0 deletions
55
oasis/week4/umc10th/src/main/java/com/example/umc10th/global/security/AuthMember.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| package com.example.umc10th.global.security; | ||
|
|
||
| import com.example.umc10th.domain.member.entity.Member; | ||
| import lombok.Getter; | ||
| import org.springframework.security.core.GrantedAuthority; | ||
| import org.springframework.security.core.authority.SimpleGrantedAuthority; | ||
| import org.springframework.security.core.userdetails.UserDetails; | ||
|
|
||
| import java.util.Collection; | ||
| import java.util.List; | ||
|
|
||
| @Getter | ||
| public class AuthMember implements UserDetails { | ||
|
|
||
| private final Member member; | ||
|
|
||
| public AuthMember(Member member) { | ||
| this.member = member; | ||
| } | ||
|
|
||
| @Override | ||
| public Collection<? extends GrantedAuthority> getAuthorities() { | ||
| return List.of(new SimpleGrantedAuthority("ROLE_USER")); | ||
| } | ||
|
|
||
| @Override | ||
| public String getPassword() { | ||
| return member.getPassword(); | ||
| } | ||
|
|
||
| @Override | ||
| public String getUsername() { | ||
| return member.getEmail(); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isAccountNonExpired() { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isAccountNonLocked() { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isCredentialsNonExpired() { | ||
| return true; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean isEnabled() { | ||
| return member.getIsEnabled(); | ||
| } | ||
|
Comment on lines
+36
to
+54
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오 현재 사용자 상태도 함께 표현해준 것 좋은 것 같다 ! 굿굿 |
||
| } | ||
23 changes: 23 additions & 0 deletions
23
...4/umc10th/src/main/java/com/example/umc10th/global/security/CustomUserDetailsService.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package com.example.umc10th.global.security; | ||
|
|
||
| import com.example.umc10th.domain.member.entity.Member; | ||
| import com.example.umc10th.domain.member.repository.MemberRepository; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.security.core.userdetails.UserDetails; | ||
| import org.springframework.security.core.userdetails.UserDetailsService; | ||
| import org.springframework.security.core.userdetails.UsernameNotFoundException; | ||
| import org.springframework.stereotype.Service; | ||
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class CustomUserDetailsService implements UserDetailsService { | ||
|
|
||
| private final MemberRepository memberRepository; | ||
|
|
||
| @Override | ||
| public UserDetails loadUserByUsername(String email) throws UsernameNotFoundException { | ||
| Member member = memberRepository.findByEmail(email) | ||
| .orElseThrow(() -> new UsernameNotFoundException("사용자를 찾을 수 없습니다: " + email)); | ||
| return new AuthMember(member); | ||
| } | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| package com.example.umc10th.global.security; | ||
|
|
||
| import com.example.umc10th.global.security.jwt.JwtAuthFilter; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
|
|
@@ -9,13 +10,15 @@ | |
| import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; | ||
| import org.springframework.security.crypto.password.PasswordEncoder; | ||
| import org.springframework.security.web.SecurityFilterChain; | ||
| import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; | ||
|
|
||
| @Configuration | ||
| @RequiredArgsConstructor | ||
| public class SecurityConfig { | ||
|
|
||
| private final CustomAuthenticationEntryPoint authenticationEntryPoint; | ||
| private final CustomAccessDeniedHandler accessDeniedHandler; | ||
| private final JwtAuthFilter jwtAuthFilter; | ||
|
|
||
| private static final String[] PUBLIC_PATHS = { | ||
| "/api/auth/**", | ||
|
|
@@ -45,7 +48,8 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti | |
| .anyRequest().authenticated()) | ||
| .exceptionHandling(handler -> handler | ||
| .authenticationEntryPoint(authenticationEntryPoint) | ||
| .accessDeniedHandler(accessDeniedHandler)); | ||
| .accessDeniedHandler(accessDeniedHandler)) | ||
| .addFilterBefore(jwtAuthFilter, UsernamePasswordAuthenticationFilter.class); | ||
|
Comment on lines
+51
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 굿굿 |
||
|
|
||
| return http.build(); | ||
| } | ||
|
|
||
Oops, something went wrong.
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.
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.
Home이랑Member도메인을 나눈 이유가 궁금해 !같은
member도메인이면 컨트롤러를 하나로 둬도 되지 않나 ? 오아시스의 생각이 궁금하넹 ~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.
이 부분은 스터디 초반 api 설계때 했던 고민이 이어지는건데, 먼저 홈 화면 데이터를 어떻게 내려줄지 고민했어
각 도메인의 API를 클라이언트가 여러 번 호출해서 조합하게 하거나, 서버에서 홈 화면에 필요한 데이터를 한 번에 조합해서 내려주거나 이 두 개의 방식중 선택을 해야하는데,
나는 이중 후자가 더 적절하다고 생각했어. 이유는 홈 화면은 화면 진입 시 필요한 데이터가 정해져 있고, 클라이언트가 여러 API를 호출해서 조합하는 것보다 서버에서 한 번에 내려주는 편이 클라이언트 구현도 단순하고 응답 구조도 관리하기 쉽다고 생각했거든.
그 다음에 고민한 게, 이 홈 화면 API를 어느 컨트롤러에 둬야하지?
home 이라는 리소스는 여러 도메인의 데이터가 조합된거라 하나의 도메인으로 보기에는 애매했고, 어쨌든 우리가 도메인형 아키텍처로 설계를 하고있기 때문에 굳이 특정 도메인에 속해야 한다면,
홈 화면은 특정 member를 기준으로 조회되는 데이터이기 때문에 member 도메인 안에 두는게 제일 자연스럽다고 생각했어.
다만 응답 안에는 mission과 같은 다른 도메인의 데이터도 포함되기 때문에, MemberController에 넣으면 단일책임원칙을 위배하는 설계라고 생각했어. 그래서 도메인은 member 안에 두되, 역할과 책임을 분리하기 위해 HomeController를 따로 둔 거야.
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.
홈 화면은 보통 여러 도메인의 데이터로 구성되잖아? 오아시스 말대로 각 도메인의 API를 클라이언트가 따로 호출해서 조합하게 하면, 클라이언트 쪽 복잡도가 올라갈 수 있다고 생각해. 로딩 상태도 각각 관리해야 하고, 일부 API가 실패했을 때 어떻게 보여줄지도 프론트에서 신경 써야 하니까.
지금 홈 화면에서 노출되는 정보는 간단해서 오아시스처럼 한 번에 내려줘도 괜찮을 것 같아! 다만 나중에 각 섹션이 무거워지면(뭐 위치기반 미션 추천 섹션, 이미지와 함께 미션을 조회 등등이 추가될 수도 있겠지?) 그때는 분리를 고려해볼 것 같다!
서비스 메서드는 특정 멤버의 홈 화면이니까 멤버서비스 안에 두고, 컨트롤러만 클래스 레벨로 분리해줬구나. 멤버 도메인 안에 둔 이유를 이렇게 설명할 수 있게 설계한다면 팀원들이 충분히 납득가능할듯!!
다만 한 가지 더 생각해볼 수 있는 부분은 어떤 기능이 특정 멤버를 기준으로 동작한다고 해서 항상 MemberService의 책임이라고 보기는 어렵다는 점인 것 같아..! 예를 들어 auth도 회원과 밀접하게 관련되어 있지만 인증이라는 별도 책임이 있어서 member와 분리해서 다루잖아? 홈도 auth만큼 독립적인 도메인은 아니지만, member, mission, point 같은 여러 데이터를 조합하는 화면 조회용도라면 HomeQueryService처럼 따로 분리하는것도 난 고려해볼거같아. 오아시스도 한번 고민해보면 좋을듯!!
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.
오아시스 이거 관련해서 요런 레퍼런스도 있어서 읽어보면 좋을거같아!
https://learn.microsoft.com/en-us/azure/architecture/patterns/backends-for-frontends