[터틀] API 테스트/문서자동화 미션 제출합니다. #11

begaonnuri · 2020-05-26T09:02:40Z

감사합니다 :)

- Spring REST Docs 의존성 추가 - MemberControllerTest 내용 복사/붙여넣기 - AuthAcceptanceTest 임시 비활성화 - LineControllerTest를 SpringBootTest로 변경 - MemberDocumentation 주석 해제

- Utf8EncodingFilter 작성 - Session / Basic / Bearer AuthInterceptor 작성 - MockMvc 사용하여 AuthAcceptanceTest 작성

- JWT 토큰 활용하여 회원정보 CRUD시 인증절차 거치도록 구현 - 인증이 필요한 컨트롤러들의 패턴을 BearerAuthInterceptor에 추가 - 인증정보(LoginMember)가 없을 경우 200 상태로 빈 멤버를 응답 - 인증정보가 틀릴 경우 401 응답 - 사용하지 않는 다른 인증 및 관련 테스트 삭제 - 서버 프로젝트 스코프 컨벤션 리팩토링 진행

- RestAssured 의존성 삭제

- Login.js에서 비밀번호 input keypress event에 onLogin 추가

…mber.Type으로 이동

jihan805

안녕하세요 깔끔하게 구현 잘하셨어요 💯
피드백 남겼으니 확인해주세요!

jihan805 · 2020-05-27T14:20:06Z

src/main/java/wooteco/subway/service/member/dto/MemberRequest.java

@@ -7,6 +7,12 @@
    private String name;
    private String password;

+    public MemberRequest(final String email, final String name, final String password) {


전체 테스트 실행시 MemberReq/Res 에 default constructor가 없어 테스트가 실패하네요 ㅜ

제 로컬환경에선 모두 통과하는데 이상하네요😱
MemberRequest와 MemberResponse에 기본생성자를 추가했습니다!

jihan805 · 2020-05-27T14:23:28Z

src/main/java/wooteco/subway/web/member/MemberController.java

        return ResponseEntity.ok().build();
    }

    @DeleteMapping("/members/{id}")
-    public ResponseEntity<MemberResponse> deleteMember(@PathVariable Long id) {
-        memberService.deleteMember(id);
+    public ResponseEntity<MemberResponse> deleteMember(@LoginMember(type = Type.ID) Member member) {


Entity를 같이 사용하는 대신 요청을 위한 Member 클래스를 만들어주세요!

jihan805 · 2020-05-27T14:25:00Z

src/main/java/wooteco/subway/service/member/MemberService.java

    }

    public String createToken(LoginRequest param) {
-        Member member = memberRepository.findByEmail(param.getEmail()).orElseThrow(RuntimeException::new);
+        Member member = memberRepository.findByEmail(param.getEmail())


아래 메서드와 중복이네요! findMemberByEmail

findByEmail을 사용하는 부분을 모두 findMemberByEmail을 사용하도록 변경했습니다!

jihan805 · 2020-05-27T14:31:49Z

src/main/java/wooteco/subway/service/member/MemberService.java

+            .orElseThrow(MemberNotFoundException::new);
+    }
+
+    public void addFavorite(final Member member, final FavoriteRequest request) {


favorite 관련 서비스를 추가하면 어떨까요?

그게 더 직관적이여서 처음엔 그렇게 하는 것을 고려했습니다.
하지만 다음과 같은 이유들로 만들지 않기로 했습니다.

FavoriteService가 생겨도 MemberService와 갖고 있는 필드가 같다.

Favorite에 대한 기능이 더 커질 경우 분리한다.

Favorite의 변경도 결국 Member의 변경으로 이어진다.

Member가 Favorites를 갖고 있기도 하고, Spring Data JDBC를 통해 Favorites의 CUD를 할 때 내부적으로 Member를 변경한다.

혹시 어떤 이유인지 자세히 알 수 있을까요? 🙂

Member 기반으로 이루어지는 모든 행위들이 Member 변경이라는 이유하나만으로 모두 MemberService에 있다면 말씀하신대로 MemberService의역할이 너무 커질 것 같아요!!!
단순히 가지고 있는 필드가 같기 때문의 이유는 좋은 이유는 아니라고 생각합니다.
즐겨찾기와 관련있는 기능들은 담당하기 위한 Service를 분리해도 좋을 것 같아 남겼으며 위의 이유로 MemberService에 두고 싶다면 두어도 무관합니다.

jihan805 · 2020-05-27T14:34:10Z

src/test/java/wooteco/subway/service/member/MemberServiceTest.java

        verify(memberRepository).save(any());
    }

+    @DisplayName("토큰 생성 테스트")


생성에 실패하는 경우에 대한 테스트도 추가하면 어떨까요?

토큰 생성에 실패하는 테스트를 추가했습니다 :)

jihan805 · 2020-05-27T14:37:17Z

src/main/java/wooteco/subway/web/member/LoginMemberController.java

 @RestController
 public class LoginMemberController {
-    private MemberService memberService;
+    private final MemberService memberService;


해당 클래스에 대한 테스트는 작성하지 않아도 괜찮은걸까요?

LoginMemberController에 대한 테스트도 추가했습니다 :)

- 클래스 수준의 RequestMapping 추가 - 메소드 이름 변경 - dependency-management를 통해 버전관리하도록 변경

jihan805

안녕하세요 피드백 반영 & 구현 모두 잘하셨어요 👍
간단한 피드백 추가로 남겼으니 확인해주세요 :)

jihan805 · 2020-06-02T11:47:32Z

src/main/java/wooteco/subway/service/member/MemberService.java

+            .orElseThrow(MemberNotFoundException::new);
+    }
+
+    public void addFavorite(final Member member, final FavoriteRequest request) {


Member 기반으로 이루어지는 모든 행위들이 Member 변경이라는 이유하나만으로 모두 MemberService에 있다면 말씀하신대로 MemberService의역할이 너무 커질 것 같아요!!!
단순히 가지고 있는 필드가 같기 때문의 이유는 좋은 이유는 아니라고 생각합니다.
즐겨찾기와 관련있는 기능들은 담당하기 위한 Service를 분리해도 좋을 것 같아 남겼으며 위의 이유로 MemberService에 두고 싶다면 두어도 무관합니다.

jihan805 · 2020-06-02T11:51:32Z

src/main/java/wooteco/subway/domain/member/Member.java

+        return this.password.equals(password);
+    }
+
+    public boolean isAuthenticated(Long id) {


사용하지 않는 메서드인것 같은데 확인하고 삭제 하면 어떨까요?

앗 불필요한 메소드를 제거하겠습니다😅

jihan805 · 2020-06-02T11:53:33Z

src/main/java/wooteco/subway/service/member/MemberService.java

+            .orElseThrow(NoSuchElementException::new);
+        Station target = stationRepository.findById(request.getTarget())
+            .orElseThrow(NoSuchElementException::new);
+        member.addFavorite(new Favorite(source.getId(), target.getId()));


반영하지 않아도 됩니다!
다른 방법으로는 쿼리를 두번 날리는 대신, ids로 조회하고 그 결과를 source와 target에 mapping할수도 있을 것 같아요~

ids로 조회해서 데이터베이스에 접근하는 횟수를 1회로 줄였습니다.
ids로 받아온 List<Stations>의 역할이 생겨서 Stations로 만들었습니다.

Stations가 Favorite을 만들어서 리턴하는것은 범용적인 List<Station>이 하는 일이 아닌, 요소가 2개 있는 List<Station> 의 역할이라고 생각해서 Favorite을 만드는 일은 꺼내서 하기로 했습니다.

다음과 같이 Station들의 아이디 리스트인 List<Long>만 반환하는 역할만 주고 Favorite은 꺼내서 만드는데 get(0), get(1) 이 부분이 조금 불편합니다.

List<Long> ids = stations.getStationIds(); favorites.addFavorite(new Favorite(ids.get(0), ids.get(1)));

어떤식으로 처리하는 것이 좋을까요?🧐

jihan805 · 2020-06-02T11:56:47Z

src/main/java/wooteco/subway/web/member/interceptor/BearerAuthInterceptor.java

+    public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {
+        String authorizationHeaderValue = authExtractor.extract(request, BEARER);
+        if (jwtTokenProvider.validateToken(authorizationHeaderValue)) {
+            request.setAttribute("loginMemberEmail", jwtTokenProvider.getSubject(authorizationHeaderValue));


"loginMemberEmail"는 상수를 사용하면 어떨까요?

loginMemeberEmail을 상수로 변경하고 ArgumentResolver에서도 해당 상수를 import해서 사용하도록 변경했습니다 :)

jihan805 · 2020-06-02T12:00:39Z

src/main/java/wooteco/subway/web/exceptions/GlobalExceptionHandler.java

+import org.springframework.web.bind.annotation.RestControllerAdvice;
+
+@RestControllerAdvice
+public class GlobalExceptionHandler {


ExceptionHandler에 정의되지 않은 에러가 발생하는 경우에 대한 처리도 추가하면 어떨까요?
(default)

Exception.class를 처리하는 메소드를 추가했습니다! 500에러를 보내주는 것이 적절하다고 생각했는데 어떻게 생각하시나요?

jihan805

안녕하세요, 구현&피드백 반영 모두 잘하셨어요 👍
간단한 코멘트 남겼으니 확인해주세요,
머지할게요 :)

jihan805 · 2020-06-11T12:11:28Z

src/main/java/wooteco/subway/service/member/FavoriteService.java

+
+    public void addFavorite(final Member member, final FavoriteRequest request) {
+        List<Station> stations = stationRepository.findAllById(
+                Arrays.asList(request.getSource(), request.getTarget()));


findAllById 내부 설명을 보면
Note that the order of elements in the result is not guaranteed. 와 같이 적혀있어요. 항상 순서가 source, target순으로 반환되지 않을 수 있을 것 같아요.
get(0), get(1) 대신, stations로 부터 source와 target 각각에 해당하는 Station을 먼저 찾아 mapping 하는 부분이 필요할것 같아요!

kimhodol added 16 commits May 19, 2020 12:06

feat : 문서 자동화 실습 진행

440df6a

- Spring REST Docs 의존성 추가 - MemberControllerTest 내용 복사/붙여넣기 - AuthAcceptanceTest 임시 비활성화 - LineControllerTest를 SpringBootTest로 변경 - MemberDocumentation 주석 해제

feat : 사용자 인증 실습 진행

6e6fa42

- Utf8EncodingFilter 작성 - Session / Basic / Bearer AuthInterceptor 작성 - MockMvc 사용하여 AuthAcceptanceTest 작성

docs : 요구사항 작성

4faf30a

feat : 회원 CRUD 단위 테스트 작성

c8b0702

refactor : RestAssured 테스트 픽스쳐들을 MockMvc 픽스처들로 대체

763cce5

- RestAssured 의존성 삭제

feat : 회원관리 컨트롤러 테스트 작성 및 문서 자동화

0f70c36

refactor : MockMvc 테스트 픽스쳐 리팩토링

9a835e1

etc : 프론트엔드 샘플 코드 추가 및 1단계 요구사항 반영

ab2e5bf

feat : Favorite CRD 인수, 단위 테스트 및 구현

cb060f4

feat : Favorite 컨트롤러 테스트 작성 및 문서화

58408b0

feat : Favorite 페이지 연동

0fb282c

- Login.js에서 비밀번호 input keypress event에 onLogin 추가

refactor : 컨벤션에 맞게 프로젝트 스코프 리팩토링

f68fe7c

refactor : 컨트롤러에서의 사용자 검증을 LoginMemberMethodArgumentResolver, LoginMe…

8ad5302

…mber.Type으로 이동

refactor : GlobalExceptionHandler 및 커스텀 예외처리 추가

fbadee8

test : 일부 AcceptanceTest 다이나믹 테스트 적용 및 Fixture 리팩토링

0744b13

jihan805 suggested changes May 27, 2020

View reviewed changes

begaonnuri added 4 commits May 28, 2020 13:46

refactor: MemberDto에 기본 생성자 추가

8b40959

test : 토큰 생성 실패 테스트 추가

2e8a869

test : LoginMemberControllerTest 추가

2364bf1

refactor : 전반적인 리팩토링

22182de

- 클래스 수준의 RequestMapping 추가 - 메소드 이름 변경 - dependency-management를 통해 버전관리하도록 변경

jihan805 suggested changes Jun 2, 2020

View reviewed changes

begaonnuri added 4 commits June 9, 2020 14:18

refactor: MemberService에서 FavoriteService를 분리

8e09e33

refactor: Favorite을 생성할 때 데이터베이스 접근을 1회로 변경

1c53087

refactor: 하드코딩 값을 상수로 변경

6abb5e6

refactor: default Exception Handler 추가

15b2556

jihan805 approved these changes Jun 11, 2020

View reviewed changes

jihan805 merged commit c23ec1b into woowacourse:begaonnuri Jun 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[터틀] API 테스트/문서자동화 미션 제출합니다. #11

[터틀] API 테스트/문서자동화 미션 제출합니다. #11

begaonnuri commented May 26, 2020

jihan805 left a comment

jihan805 May 27, 2020

begaonnuri May 28, 2020

jihan805 May 27, 2020

jihan805 May 27, 2020

begaonnuri May 28, 2020

jihan805 May 27, 2020

begaonnuri May 28, 2020 •

edited

Loading

jihan805 Jun 2, 2020

jihan805 May 27, 2020

begaonnuri May 28, 2020 •

edited

Loading

jihan805 May 27, 2020

begaonnuri May 28, 2020

jihan805 left a comment

jihan805 Jun 2, 2020

jihan805 Jun 2, 2020

begaonnuri Jun 9, 2020

jihan805 Jun 2, 2020

begaonnuri Jun 9, 2020

jihan805 Jun 2, 2020

begaonnuri Jun 9, 2020

jihan805 Jun 2, 2020

begaonnuri Jun 9, 2020

jihan805 left a comment

jihan805 Jun 11, 2020

[터틀] API 테스트/문서자동화 미션 제출합니다. #11

[터틀] API 테스트/문서자동화 미션 제출합니다. #11

Conversation

begaonnuri commented May 26, 2020

jihan805 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

begaonnuri May 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

begaonnuri May 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihan805 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihan805 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

begaonnuri May 28, 2020 •

edited

Loading

begaonnuri May 28, 2020 •

edited

Loading