Feature/soeunuhm step1 #2
Conversation
| @Getter | ||
| @Setter | ||
| @Entity | ||
| public class SiteUser { |
There was a problem hiding this comment.
ddl 문을 따로 정의하지 않고 auto-ddl 옵션을 사용하시는 것 같은데 index 는 필요 없을까요?
| private String password; | ||
|
|
||
| @Column | ||
| private String userName; |
There was a problem hiding this comment.
[nit]
아마도 사용자의 이름을 의미하는 것 같은데 보통 userId 와 username 모두 우리가 "아이디"라고 부르는 것을 지칭할 때 사용되는 것 같고(로그인용 식별자) 이름은 그냥 name 이라고 저장해도 될 것 같아요
| } | ||
| SiteUser siteUser = _siteUser.get(); | ||
| List<GrantedAuthority> authorities = new ArrayList<>(); | ||
| if ("admin".equals(userId)){ |
There was a problem hiding this comment.
우리의 프로젝트에서는 큰 상관이 없겠지만 관리자의 계정의 id 를 "admin", "administrator", "root" 등과 같이 뻔한 것으로 짓는건 보안적으로 금기긴 합니다. 더 자세한 이유는 한번 찾아보셔도 좋을 것 같아요~
| authorities.add(new SimpleGrantedAuthority(UserRole.ADMIN.getValue())); | ||
| } | ||
| else { | ||
| authorities.add(new SimpleGrantedAuthority(UserRole.USER.getValue())); | ||
| } |
There was a problem hiding this comment.
사용자의 ROLE 을 id 를 보고 판단하면 새로운 관리자가 추가될 때마다 코드를 고쳐줘야 할 것 같은데 비효율적이지 않을까요? 지금은 어드민 아니면 모두 일반 유저지만 만약에 중간에 어드민보다는 조금 낮은 권한의 ROLE 을 만들어야 한다면 어떻게 처리하는게 좋을까요?
| SiteUser user = new SiteUser(); | ||
| user.setUserId(userId); | ||
| user.setUserName(userName); | ||
| user.setEmail(email); | ||
| user.setPassword(passwordEncoder.encode(password)); | ||
| this.userRepository.save(user); |
There was a problem hiding this comment.
이것 역시 크게 문제되는 코드는 아니지만 요즘 자바에서는 setter 대신 builder pattern 이나 constructor 를 사용하는 추세로 가고 있습니다. 많은 차이점이 있지만 가장 큰 이유는 immutable object 를 생성하기 위함인데 이것도 한번 찾아보시는 것 추천드려요!
간단한 답변이 있는 링크
| @PostMapping("/signup") | ||
| public String signup(@Valid UserCreateForm userCreateForm, BindingResult bindingResult){ |
There was a problem hiding this comment.
프론트까지 만들어주셔도 괜찮긴 하지만 이후 step 들까지 프론트를 만들긴 어려우니까 그냥 api-server 의 형태로만 구현해도 될 것 같아요. 따로 view controller 를 사용하신 이유가 있으신가요? 👀
| } | ||
| catch (DataIntegrityViolationException e) { | ||
| e.printStackTrace(); | ||
| bindingResult.reject("signupFailed", "Already Exsisting User"); |
| SecurityFilterChain filterChain(HttpSecurity http) throws Exception { | ||
| http | ||
| .authorizeHttpRequests((authorizeHttpRequests) -> authorizeHttpRequests | ||
| .requestMatchers(new AntPathRequestMatcher("/**")).permitAll()) |
| AuthenticationManager authenticationManager(AuthenticationConfiguration authenticationConfiguration) throws Exception { | ||
| return authenticationConfiguration.getAuthenticationManager(); | ||
| } |
There was a problem hiding this comment.
spring security 의 default authentication 방식을 채택하셨군요..! 그럼 얘는 어떤 인증 방식인가요? token 기반인가요? session 기반인가요?
프로덕션에서 이걸 그대로 사용하는 회사는 없고 솔루션 챌린지를 할 때도 사용자 인증을 customizing 해서 사용할거라(프론트가 따로 있고 redirecting 을 할 수는 없으니) 로그인을 AuthenticationProvider 를 implement 해서 직접 구현하는걸 강력하게 추천드리긴 합니다.
| * id : testid , username : testuser, email : [email protected] , password : 1234 | ||
| */ | ||
| void testJpa() { | ||
| List<SiteUser> all = this.userRepository.findAll(); | ||
| Optional<SiteUser> userOptional = this.userRepository.findByUserId("testid"); | ||
| SiteUser firstUser = userOptional.get(); | ||
| Assertions.assertThat("testuser").isEqualTo(firstUser.getUserName()); | ||
| Assertions.assertThat("[email protected]").isEqualTo(firstUser.getEmail()); |
There was a problem hiding this comment.
이 정보는 이미 database 에 저장되어 있는 정보인가요? 그러면 다른 컴퓨터에서 이 테스트를 돌리면 실패할 수도 있나요? 테스트는 어느 환경에서 실행해도 동일한 결과를 보장해야 합니다.
이를 위해서 보통 별도의 1회용 데이터베이스를 띄우고(빠른 속도를 원한다면 인메모리, 실제 서비스와 동일한 DB 환경의 테스트를 원한다면 도커 컨테이너를 활용할 수 있겠죠) 필요한 값들을 생성하고 지우면서 테스트를 진행합니다.
unit test 를 활용하셔도 되고, MockMVC 를 사용하면 테스트 안에서 HTTP 요청을 날릴 수 있어서 e2e 테스트를 하셔도 됩니다.
There was a problem hiding this comment.
저는 엔티티에 @Setter 어노테이션을 지양하는 편이에요! public으로 객체를 마음대로 조작하는 것보다는 필요한 경우에 public update 메서드를 쓰면 어떨까요?
There was a problem hiding this comment.
필수적인건 아니지만 UserResponse 같은 dto를 만들어서 리턴해주면 어떨까싶어요! 멤버를 조회하는 api 만들 때 활용도 높을 것 같습니당 +_+
There was a problem hiding this comment.
public 수준의 @Setter를 쓸거면 @AllArgsConstructor 사용하는 것도 추천해요!!
Step 1