#9 Member 패키지 리팩터링 및 테스트 작성 #10

LeeJaeYun7 · 2023-07-25T11:46:21Z

Member 패키지를 리팩터링하고, 테스트를 작성했습니다

ErrorCode 필드 수정

f-lab-moony

고생많으셨어요 ~
코멘트가 좀 많은데, 한번 확인해보시고 다시 올려주시면 좋을 것 같아요 ~

f-lab-moony · 2023-07-27T13:59:06Z

src/main/java/com/example/interviewPrep/quiz/member/controller/MemberController.java

    private final MemberService memberService;
    private final OauthService oauthService;

+    @Autowired


요거는 붙이신 이유가 있을까요 ?

생성자가 하나일 경우, @Autowired를 생략할 수 있다는 내용을 깜박해서 그런 것 같습니다!
수정해보겠습니다

f-lab-moony · 2023-07-27T13:59:34Z

src/main/java/com/example/interviewPrep/quiz/member/controller/MemberController.java

+
    @PostMapping("signup")
-    public ResultResponse<?> signUp(@RequestBody SignUpRequestDTO memberDTO) throws Exception {
+    public ResultResponse<Member> signUp(@RequestBody SignUpRequestDTO memberDTO) throws Exception {


👍
근데 뒤에 throws Excpetion은 무슨 역할을 하고 있을까요 ?

아무런 역할을 하고 있지 않아서 제외시켰습니다

f-lab-moony · 2023-07-27T14:02:51Z

src/main/java/com/example/interviewPrep/quiz/member/controller/MemberController.java


    @GetMapping("userInfo")
-    public ResultResponse<?> getUserInfo(){
+    public ResultResponse<Member> getUserInfo(){


ResultResponse로 전부 감싸져있는 것 같은데, 이 구조로 얻을 수 있는 장점과 단점은 어떤게 있을까요 ?

장점
-개발자가 원하는대로 커스텀해서 사용할 수 있다
-통일된 양식으로 ResponseEntity를 만들 수 있다

단점
-추상 계층이 하나가 추가된다
-코드 오버헤드가 늘어난다
-스프링의 built-in 기능을 사용할 수 없다
(ex) 컨텐츠 negotiation, HTTP 헤더 조작, 자동 응답 코드 핸들링 등)

멘토링 시간에 말씀 드리긴 했지만, 이 구조는 api를 사용하는 클라이언트 입장에서 가장 불편을 크게 느끼는 구조에요, 공통된 레이어가 꼭 필요하거나 (페이징 응답이라던지) 합의 된 경우가 아니라면 응답객체를 그대로 리턴하는 걸 추천합니다!

응답 객체를 그대로 리턴

f-lab-moony · 2023-07-27T14:11:23Z

src/main/java/com/example/interviewPrep/quiz/member/controller/MemberController.java

+        this.oauthService = oauthService;
+    }
+
    @PostMapping("signup")


endpoint는 MethodType과 합쳐져서 의미를 명확하게 표현할 수 있는데 이 메서드같은 경우는

POST "api/v1/members/"

로만 표현해줘도 좋을 것 같아요

api는 굳이 안붙여도 되긴 하는데, 정적 리소스를 가져올때랑 구분해서 사용할 수 있어서 붙여주는게 좋다고 생각하는 편이구요
같은 역할을 하지만 비즈니스 로직이 바뀌거나 in/out이 변경되는 경우, 특히 서비스를 운영하고 있는 중이라면 이전 버전의 api를 유지해야 되는 경우가 있어요. 말하자면 하위호환을 해주는건데, 그럴 때 분기를 위해서 버전 정보도 필요해요

그 다음으로는 도메인의 복수형, 지금은 member 도메인을 다루니까 members 를 적어주고,그 뒤에를 보통 pathVariable로 id를 받게 돼요

지금같은경우는 등록이니까 아이디가 없으니 받을 필요는 없어보이구요

POST : Create
PUT : Update
DELETE : Delete
GET : Select

이렇게 무슨 일을 할지 나뉘기때문에

이 컨트롤러를 예로 들면

유저 등록 : POST api/v1/members
전체 유저 검색 : GET api/v1/members
특정 유저 검색 : GET api/v1/members/{id}
유저 정보 변경 : PUT api/v1/members/{id}
유저 삭제 : DELETE api/v1/members/{id}

이런식으로요! 기본적인 CRUD는 위처럼 만들고, 추가적인 정보가 필요할 때는 그에 맞는 네이밍을 고민해보면 좋을 것 같습니다 :)

그러면 이전 버전의 API를 유지하고 새로운 버전의 API를 추가할 때면,
기존의 것은 그대로 놔두고, /v2/members를 추가하는 방식으로 사용하는 것일까요?

그리고 저렇게 버전을 표시해줄려면, 앞에 @RequestMapping("/members")를 붙여주는 것보다,
각각의 메소드에 독립적으로 선언해주어야 할 것 같은데, 맞을까요?

반영 완료

f-lab-moony · 2023-07-27T14:12:30Z

src/main/java/com/example/interviewPrep/quiz/member/domain/Member.java

    @Enumerated(EnumType.STRING)
    private Role role;

+    public Member() {


요것도 상관은 없지만 기본생성자가 따로 필요할떄는 @NoArgConstructor 써주면 좋을 것 같아요!

넵 확인했습니다

f-lab-moony · 2023-07-27T15:06:30Z

src/main/java/com/example/interviewPrep/quiz/member/service/MemberService.java

+        return member;
+    }
+
+    public boolean isValidPassword(Member member, String memberPassword, String inputPassword, String newPassword){


이름은 isValid~~ 인데 내부에서는 암호도 바꾸고 다른 추가작업이 있는 것 같아요 ?

넵 밖에서 처리하도록 수정했습니다

f-lab-moony · 2023-07-27T15:07:05Z

src/main/java/com/example/interviewPrep/quiz/member/service/MemberService.java

+      if(memberPassword.equals(encryptedInputPassword)){
+        String newEncryptedPassword = SHA256Util.encryptSHA256(newPassword);
+        member.setPassword(newEncryptedPassword);
+        memberRepository.save(member);


이 부분은 필요가 없지 않을까요 ?

넵 이 부분 밖에서 처리하도록 수정했습니다

f-lab-moony · 2023-07-27T15:09:40Z

src/test/java/com/example/interviewPrep/quiz/Member/service/AuthenticationServiceTest.java

-        Member member = Member.builder()
-                        .password(password)
-                        .build();
+        assertNotNull(loginResponseDTO, "Expected non-null LoginResponseDTO");


검증은 assertThat()으로 시작하시는 걸 추천드려요 ~

넵 반영했습니다

f-lab-moony · 2023-07-27T15:09:53Z

src/test/java/com/example/interviewPrep/quiz/Member/service/AuthenticationServiceTest.java


    @Test
-    void loginWithWrongEmail(){
+    @DisplayName("login failure by wrong email")


f-lab-moony · 2023-07-27T15:10:55Z

src/test/java/com/example/interviewPrep/quiz/Member/service/AuthenticationServiceTest.java


    @Test
-    void loginWithWrongPassword(){
+    @DisplayName("login failure by wrong password")


DisplayName은 테스트 설명부분이라 왠만하면 한글로 설명이 되게 적어주시는게 좋아요 ~

MemberService 테스트 삭제 Role 클래스 필드 수정

f-lab-moony

혹시 피드백 반영 어떻게 진행하고 계신걸까요 ~?
이전 피드백에 코멘트가 달린 흔적이 있는데, 피드백 반영분에는 적용되지 않은 것 같아 여쭤봅니다 ~

f-lab-moony · 2023-07-30T12:32:10Z

src/main/java/com/example/interviewPrep/quiz/member/dto/MemberDTO.java

    private String type;

+    public MemberDTO(Long id, String email, String password, String nickName, String newPassword, String type) {
+        if (email == null || password == null || nickName == null || newPassword == null || type == null) {


생성자에서 잡아주실거면 위에 @NotNull 은 필요 없을 것 같아요 ?
그리고 이렇게 검사하면 어떤 값이 null인지 메세지로 파악이 안되지 않을까요 ?
저는 보통 Objects.requireNonNull 메서드를 이용하는 편입니다.

f-lab-moony · 2023-07-30T12:35:14Z

src/test/java/com/example/interviewPrep/quiz/Member/domain/MemberTest.java

-        assertThat(member.getId()).isEqualTo(11L);
-        assertThat(member.getEmail()).isEqualTo("hello@gmail.com");
-        assertThat(member.getPassword()).isEqualTo("1234");
+    @DisplayName("member create")


생성자가 두개라면, 테스트도 두개가 되어야 하지 않을까요 ?
그리고 저번에 코멘트 한 내용인데, 굳이 생성자가 두개여야 할 필요가 있을까요 ?

넵 빌더 패턴으로 생성하도록 수정했습니다

f-lab-moony · 2023-07-30T12:36:25Z

src/test/java/com/example/interviewPrep/quiz/Member/dto/MemberDTOTest.java

+
+
+    @Test
+    @DisplayName("memberDTO create with null value")


null체크가 여러개가 있을 것 같은데, @ParameterizedTest 를 이용해보는 건 어떨까요 ?

@ParameterizedTest 반영해서 완료했습니다

#10 Question 패키지 리팩토링 및 테스트 작성

LeeJaeYun7 added 2 commits July 25, 2023 20:44

Member 패키지 리팩터링

4069750

AuthService 클래스 테스트 리팩터링,

77219f7

ErrorCode 필드 수정

f-lab-moony requested changes Jul 27, 2023

View reviewed changes

LeeJaeYun7 changed the title ~~#9 Member 패키지 리팩터링~~ #9 Member 패키지 리팩터링 및 테스트 작성 Jul 29, 2023

Member 테스트, MemberDTO 테스트 추가

edfa19c

MemberService 테스트 삭제 Role 클래스 필드 수정

f-lab-moony requested changes Jul 30, 2023

View reviewed changes

LeeJaeYun7 added 2 commits August 1, 2023 18:33

리팩터링 피드백 반영

e945bee

MemberDTO 테스트, AuthenticationService 테스트 업데이트

e6766b6

f-lab-moony merged commit aa567bf into main Aug 4, 2023

LeeJaeYun7 deleted the fix/MemberRefactoring branch August 15, 2023 13:21

f-lab-moony added a commit that referenced this pull request Aug 24, 2023

Merge pull request #11 from f-lab-edu/feat/QuestionRefactoring

ea653cc

#10 Question 패키지 리팩토링 및 테스트 작성

#9 Member 패키지 리팩터링 및 테스트 작성 #10

#9 Member 패키지 리팩터링 및 테스트 작성 #10

Uh oh!

Conversation

LeeJaeYun7 commented Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

f-lab-moony left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LeeJaeYun7 Jul 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

f-lab-moony left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LeeJaeYun7 commented Jul 25, 2023 •

edited

Loading

LeeJaeYun7 Jul 31, 2023 •

edited

Loading