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

sonypark · 2020-05-27T07:14:12Z

화투 안녕하세요!
레벨1에 이어 두 번째 만남이네요
이번 피드백도 잘 부탁드립니다:)

중점을 두고 개발한 부분

테스트 코드 꼼꼼히 짜기
- 기본 테스트와, 예외처리 등의 테스트를 꼼꼼히 짜려고 노력했습니다.
즐겨찾기 toggle 버튼으로 추가/취소 하는 기능

궁금한점

(실험해본 결과) 기본 생성자 필요한 경우
- DTO의 경우 필드의 갯수가 1개인 경우는 기본 생성자가 필요
  - 필드의 갯수가 2개인 경우는 기본 생성자 불필요
- Entity의 경우 기본 생성자 항상 필요
자문 자답
- 위의 경우는 Gradle로 빌드 할 경우만 해당
- Intellij IDEA로 빌드할 경우 DTO에 기본 생성자 생성해 주어야 함

DTO나 Entity에는 항상 기본 생성자를 생성해주어야 하는 것 같은데. .맞을까요? 🤔

추가 기능

즐겨찾기 목록 클릭했을 때 경로 검색하는 기능
- 즐겨찾기 목록에 링크를 달아서 해당 목록을 클릭하면 경로 검색이 이루어지도록 하는 기능을 구현했습니다.

- DTO에 기본 생성자가 없을 경우 intellij IDEA로 빌드 시 에러 발생

디버깅을 위해 주석 처리했던 코드 재활성화

- 로그인 했을 때만 나오도록 수정

phs1116 · 2020-05-31T13:07:02Z

(실험해본 결과) 기본 생성자 필요한 경우
- DTO의 경우 필드의 갯수가 1개인 경우는 기본 생성자가 필요
  - 필드의 갯수가 2개인 경우는 기본 생성자 불필요
- Entity의 경우 기본 생성자 항상 필요
자문 자답
- 위의 경우는 Gradle로 빌드 할 경우만 해당
- Intellij IDEA로 빌드할 경우 DTO에 기본 생성자 생성해 주어야 함

DTO나 Entity에는 항상 기본 생성자를 생성해주어야 하는 것 같은데. .맞을까요? 🤔

스프링에서 비직렬화, 직렬화시에 Jackson이라는 라이브러리르 사용하는데요.
비직렬화시(DTO 변환) 기본생성자와 리플렉션 기반으로 동작합니다.
반대로 Response의 경우엔 기본 생성자 없이 getter만으로도 동작할거에요. :)

spirng data jdbc의 경우 객체 생성을 어떻게 하는지 저도 한번 찾아봤는데요.
https://docs.spring.io/spring-data/jdbc/docs/current/reference/html/#mapping.object-creation
꼭 기본생성자가 아니어도 상관 없는 것 같네요. :)

phs1116

안녕하세요 소니! :)
구현 깔끔하게 잘해주셨어요! 👍
피드백 몇가지랑 질문 코멘트 추가했는데 참고 부탁드릴게요. :)

phs1116 · 2020-05-31T12:39:25Z

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

-            return new Member();
-        }
+        NativeWebRequest webRequest, WebDataBinderFactory binderFactory) {
+        String email = (String)webRequest.getAttribute("loginMemberEmail", SCOPE_REQUEST);


loginMemberEmail 등의 값은 상수로 관리해주는게 좋을 것 같아요. :)

phs1116 · 2020-05-31T12:39:54Z

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

-        request.setAttribute("loginMemberEmail", email);
-        return true;
+        HttpServletResponse response, Object handler) {
+        String accessToken = authExtractor.extract(request, "Bearer");


"Bearer" 상수로! :)

phs1116 · 2020-05-31T12:40:53Z

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

+    @ExceptionHandler(Exception.class)
+    public ResponseEntity<ErrorResponse> handleException() {
+        return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
+            .body(new ErrorResponse("서버에서 오류가 발생했습니다."));
+    }


시스템적으로 확인이 되야하는 예외에 대해서
익셉션 핸들러에서 처리하고 넘어가면 에러 확인이 어려울 수 있어요.
slf4j를 적용해서 에러 로그를 찍어주는게 좋을 것 같아요. :)

slf4j를 이용해서 에러 로그를 찍고, log 파일로 기록하도록 수정했습니다!

phs1116 · 2020-05-31T12:48:13Z

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

+            + "LEFT JOIN station as target "
+            + "ON favorite.target_station_id = target.id "
+            + "WHERE favorite.member = :id")
+    List<FavoriteDetail> findFavoritesById(@Param("id") Long id);


MemberRepository에 있어야하는 쿼리가 맞을까요?:)

phs1116 · 2020-05-31T12:57:14Z

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

+        "select favorite.source_station_id as source_id, favorite.target_station_id as target_id, "
+            + "source.name as source_name, target.name as target_name from favorite "
+            + "LEFT JOIN station as source "
+            + "ON favorite.source_station_id = source.id "
+            + "LEFT JOIN station as target "
+            + "ON favorite.target_station_id = target.id "
+            + "WHERE favorite.member = :id")


LEFT JOIN을 두번이나 해야할지 조금 고민이 드네요. RDB와 실제 데이터의 괴리의 영향도 크네요.
현재는 데이터가 적으니 괜찮지만 station에 대한 id를 in절로 요청하여 처리하는게 더 나을수도 있어요

"SELECT favorite.source_station_id as source_id, favorite.target_station_id as target_id, " + "station.name as source_name, station.name as target_name from favorite " + "LEFT JOIN station " + "ON station.id IN (favorite.source_station_id, favorite.target_station_id) " + "WHERE favorite.member = :id"

station에 대한 id를 in절로 요청하여 처리 하는 부분을 위와 같이 해보았는데요. 문제는 FavoriteDetail DTO를 사용하려면 즐겨찾기 지하철역의 sourceName과 targetName이 둘 다 필요한데, 위와 같은 SQL문에서는 targetName을 받아올 수 없는 것 같습니다.. 😢 제가 제대로 이해하고 사용하지 못 한 건지.. 좀 더 힌트를 주신다면 LEFT JOIN을 두 번 사용하지 않고 하는 방법으로 적용해보겠습니다!

테스트를 돌려보면 아래와 같인 sourceName과 targetName이 같아서 오류가 발생합니다.

제가 말씀드린 부분은 한번의 쿼리로 처리하는게 아닌

favorite에 대한 데이터를 한번 가져오고,
가져온 favorite에 있는 stationId를 기반으로 station 정보를 쿼리해서
두 데이터를 매핑할수 있지 않을까해서 말씀드렸어요. :)

네 JOIN 쿼리를 두 번 쓴 부분은 Favorite이 memberId를 가지고 있는 형식으로 변경함에 따라 삭제했습니다.
favorite에 대한 데이터를 한번 가져오고, 가져온 favorite에 있는 stationId를 기반으로 station 정보를 쿼리해서 매핑하도록 수정했습니다!

phs1116 · 2020-05-31T13:08:44Z

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

 public class Member {
    @Id
    private Long id;
    private String email;
    private String name;
    private String password;
+    private Set<Favorite> favorites;


Favorite을 Member에서 책임질 필요가 있을까는 조금 고민의 여지가 있는 부분으로 보여요.
도메인 상으로도 둘이 memberId를 공유한다는 것 외에는 공통 컨텍스트로 가져갈 필요도 없기도 하구요, Member의 불필요하게 늘어나는 경향도 있네요. :0

Favorite에 대해서든 별도 바운디드 컨텍스트로 쪼개서 Favorite이 memberId를 알고있고, 이로 인하여 비즈니스를 풀어나갈수도 있지 않을까 싶어요. :) 한번 고민해보세요.

네 저도 이 부분에 대해 고민이 됐었는데요. Member가 자신이 추가한 Favorite을 가지고 있는게 자연스럽다고 생각했습니다. 또한, Member가 탈퇴하면 멤버가 가지고 있던 Favorite도 삭제되는게 맞다고 생각해서 Member에 필드로 Favorite을 두도록 하였습니다.

FavoriteId와 MemberId를 갖고 있는 참조 테이블을 만들어서 구현할 수도 있지만 이때 Member가 탈퇴하면 추가적으로 참조 테이블에서 해당 MemberId에 해당하는 Favorite을 삭제해야하는 작업이 이루어져야 한다고 생각했기 때문에 전자로 구현해보았습니다.

어떤게 더 좋은 방법일까요?🤔

참조테이블을 굳이 가지고 있지 않고 Favorite이 memberId를 가지고 있어도 되지 않을까요?
분리를 할 경우에는 Member 탈퇴 시 Favorite 정보 삭제는 트랜잭션을 묶어서 비즈니스 단에서 처리해줄수 있는 부분이구요. :)
(그리고 사실 실제 비즈니스에서 회원이 탈퇴 해도 식별 데이터를 영구 삭제하지는 않아요. :)
soft delete를 통해 관리해요. 회원이 soft delete되면 굳이 트랜잭션 보장하지 않아도 Favorite은 무효한 데이터가 되겠죠.)

어떤게 더 좋은 방법이라기 보다는 분리함으로써 회원 도메인이 확장을 할 때 특정 도메인에 묶이지 않는다는 장점이 있을 것 같아요.
(회원 / 인증 도메인을 별도 서버로 분리할 경우, 서비스의 비즈니스 확장 등등, 회원 도메인이 특정 도메인에 묶이게 되므로)

물론 현재는 도메인이 그렇게 크지 않지만요. :)

그렇군요! 특정 도메인에 묶이지 않도록 로직을 분리하는 연습을 해보는게 좋겠네요!ㅎㅎ
Favorite이 memberId를 가지고 있는 형식으로 수정했습니다 😊

phs1116

안녕하세요, 소니!
로깅 처리 잘해주셨어요. :) 👍
질문주신 사항에 대한 의견 및
피드백 몇가지 추가했는데 한번 확인 부탁드릴게요.

phs1116 · 2020-06-06T12:09:39Z

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

 public class Member {
    @Id
    private Long id;
    private String email;
    private String name;
    private String password;
+    private Set<Favorite> favorites;


참조테이블을 굳이 가지고 있지 않고 Favorite이 memberId를 가지고 있어도 되지 않을까요?
분리를 할 경우에는 Member 탈퇴 시 Favorite 정보 삭제는 트랜잭션을 묶어서 비즈니스 단에서 처리해줄수 있는 부분이구요. :)
(그리고 사실 실제 비즈니스에서 회원이 탈퇴 해도 식별 데이터를 영구 삭제하지는 않아요. :)
soft delete를 통해 관리해요. 회원이 soft delete되면 굳이 트랜잭션 보장하지 않아도 Favorite은 무효한 데이터가 되겠죠.)

어떤게 더 좋은 방법이라기 보다는 분리함으로써 회원 도메인이 확장을 할 때 특정 도메인에 묶이지 않는다는 장점이 있을 것 같아요.
(회원 / 인증 도메인을 별도 서버로 분리할 경우, 서비스의 비즈니스 확장 등등, 회원 도메인이 특정 도메인에 묶이게 되므로)

물론 현재는 도메인이 그렇게 크지 않지만요. :)

phs1116 · 2020-06-06T12:12:05Z

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

+        "select favorite.source_station_id as source_id, favorite.target_station_id as target_id, "
+            + "source.name as source_name, target.name as target_name from favorite "
+            + "LEFT JOIN station as source "
+            + "ON favorite.source_station_id = source.id "
+            + "LEFT JOIN station as target "
+            + "ON favorite.target_station_id = target.id "
+            + "WHERE favorite.member = :id")


제가 말씀드린 부분은 한번의 쿼리로 처리하는게 아닌

favorite에 대한 데이터를 한번 가져오고,
가져온 favorite에 있는 stationId를 기반으로 station 정보를 쿼리해서
두 데이터를 매핑할수 있지 않을까해서 말씀드렸어요. :)

phs1116 · 2020-06-06T12:15:42Z

src/main/java/wooteco/subway/web/PageController.java

@@ -49,6 +50,11 @@ public String searchPage() {
        return "service/search";
    }

+    @GetMapping(value = "/service", produces = MediaType.TEXT_HTML_VALUE)


해당 컨트롤러가 책임지는 페이지가 이제 너무 많네요.
이제 슬슬 분리해도 되지 않을까 싶네요. :)

인증 페이지와 마이 페이지 부분을 따로 분리했습니다~

phs1116 · 2020-06-06T12:16:56Z

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

+        return ResponseEntity.ok().body(FavoriteResponse.of(memberService.getFavorites(member)));
+    }
+
+    @GetMapping("/me/favorites/from/{sourceId}/to/{targetId}")


pathVariable보다는 requestParam을 통한 쿼리가 더 적합하지 않을까 생각도 드네요. :)

앗 그런군요. 저도 이 부분이 좀 고민이 되었는데요.

/me/favorites?sourceId=1&targetId=2
쿼리를 통해 전달하게 되면 이런 형태가 될 것 같은데, 저는 쿼리만 보고는 위 url이 어떤 의미인지 파악하기 힘들것 같다고 생각했습니다. 그래서 /me/favorites/from/{sourceId}/to/{targetId}형태로 하는게 가독성과 의미전달을 하는데 적합하다고 판단했습니다.

혹시 requestParam을 통한 쿼리가 어떤점에서 더 적합한지 말씀해주실 수 있을까요~~?

Restful 관점에서의 URL 디자인은 리소스 중심으로 작성되고, 가져올 리소스, 그리고 다른 리소스를 가져올 경우 그 리소스와의 관계를 명시하고들 해요.
가져올 리소스에 대한 조건은 쿼리스트링으로 명시하구요.
Restful API Design에 대해서 한번 살펴보시면 될 것 같아요. :)
물론 요새는 Restful을 너무 빡빡하게 지키지 말자는 분위기도 많이 있기 때문에 작성하신 URL이 명시적이라면 사용할수도 있는 URL이기도 해요.

phs1116

안녕하세요, 소니!
피드백 반영 잘해주셨어요. 👍
이번 과제 머지할게요.
구현하시느라 정말 수고 많으셨어요. :)

phs1116 · 2020-06-12T13:56:31Z

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

+        return ResponseEntity.ok().body(FavoriteResponse.of(memberService.getFavorites(member)));
+    }
+
+    @GetMapping("/me/favorites/from/{sourceId}/to/{targetId}")


Restful 관점에서의 URL 디자인은 리소스 중심으로 작성되고, 가져올 리소스, 그리고 다른 리소스를 가져올 경우 그 리소스와의 관계를 명시하고들 해요.
가져올 리소스에 대한 조건은 쿼리스트링으로 명시하구요.
Restful API Design에 대해서 한번 살펴보시면 될 것 같아요. :)
물론 요새는 Restful을 너무 빡빡하게 지키지 말자는 분위기도 많이 있기 때문에 작성하신 URL이 명시적이라면 사용할수도 있는 URL이기도 해요.

sonypark added 25 commits May 20, 2020 10:34

feat: Bearer 인증 기능 구현

3c46cfc

test: 회원가입 컨트롤러 테스트 추가

59d7f44

feat: 회원가입 페이지 연동

edccd2d

test: 중복 유저 회원가입 불가 테스트

812ac38

test: 로그인 테스트

9b6d882

feat: 로그인 페이지 연동

6b0be0a

refactor: 로그인 테스트 수정

8cc3e51

refactor: 회원가입 테스트 수정

4a19d90

feat: 회원 정보 조회.수정.삭제 기능 추가

9c6cee8

feat: 회원 정보 조회,수정,삭제 페이지 연동

b3d962b

feat: 문서화를 위한 테스트 작성

33d537b

feat: 즐겨찾기 추가, 조회, 제거 및 인수 테스트 추가

803eaa1

feat: 즐겨찾기 여부 확인 기능 추가

992858b

test: Member 테스트 추가

8eb5128

refactor: 즐겨찾기 리팩토링

d04becb

feat: MemberService 즐겨찾기 테스트 추가

0261a82

feat: LoginMemberController 즐겨찾기 테스트 추가 및 문서화

09232e3

feat: 즐겨찾기 페이지 연동

38c9edd

feat: 즐겨찾기 여부 표시 추가

4b4ccab

feat: 로그아웃 기능 추가

a0885a7

feat: 즐겨찾기 목록 클릭 시 해당 경로 검색

53132f3

feat: 유효성 검사 추가

cd6f837

fix: DTO에 기본 생성자 추가

0509fe5

- DTO에 기본 생성자가 없을 경우 intellij IDEA로 빌드 시 에러 발생

style: 인수 테스트 코드 포맷 비활성화

ecb2a8f

chore: 에러 핸들러에 커스텀 에러 추가

ab80f72

디버깅을 위해 주석 처리했던 코드 재활성화

sonypark force-pushed the feat/favorite branch from 19b2746 to ab80f72 Compare May 27, 2020 07:40

sonypark added 2 commits May 29, 2020 13:00

fix: 비 로그인 상태에서 경로 검색 시 즐겨찾기 로딩 에러가 나오는 현상

06a42e9

- 로그인 했을 때만 나오도록 수정

refactor: DB에 데이터를 등록/수정/삭제 메서드에 @transactional 추가

f1a3218

phs1116 suggested changes May 31, 2020

View reviewed changes

sonypark added 2 commits June 2, 2020 16:35

refactor: 토큰 타입과 loginMemberEmail 상수 처리

fe5fcdf

refactor: logger로 error 내용 기록

bebd4ea

phs1116 suggested changes Jun 6, 2020

View reviewed changes

sonypark added 2 commits June 9, 2020 00:11

refactor: Member가 Favorite을 갖고 있지 않도록 변경

9aaceeb

refactor: PageController 내 메서드 역할별로 분리

29cad94

phs1116 approved these changes Jun 12, 2020

View reviewed changes

phs1116 merged commit 18a7335 into woowacourse:sonypark Jun 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

sonypark commented May 27, 2020 •

edited

Loading

phs1116 commented May 31, 2020

phs1116 left a comment

phs1116 May 31, 2020

phs1116 May 31, 2020

phs1116 May 31, 2020

sonypark Jun 2, 2020

phs1116 May 31, 2020

phs1116 May 31, 2020

sonypark Jun 3, 2020 •

edited

Loading

phs1116 Jun 6, 2020

sonypark Jun 8, 2020

phs1116 May 31, 2020

sonypark Jun 3, 2020 •

edited

Loading

phs1116 Jun 6, 2020

sonypark Jun 8, 2020 •

edited

Loading

phs1116 left a comment

phs1116 Jun 6, 2020

phs1116 Jun 6, 2020

phs1116 Jun 6, 2020

sonypark Jun 8, 2020

phs1116 Jun 6, 2020

sonypark Jun 9, 2020 •

edited

Loading

phs1116 Jun 12, 2020

phs1116 left a comment

phs1116 Jun 12, 2020

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

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

Conversation

sonypark commented May 27, 2020 • edited Loading

중점을 두고 개발한 부분

궁금한점

추가 기능

phs1116 commented May 31, 2020

phs1116 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

sonypark Jun 3, 2020 • edited Loading

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

sonypark Jun 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonypark Jun 8, 2020 • edited Loading

Choose a reason for hiding this comment

phs1116 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

sonypark Jun 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phs1116 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonypark commented May 27, 2020 •

edited

Loading

sonypark Jun 3, 2020 •

edited

Loading

sonypark Jun 3, 2020 •

edited

Loading

sonypark Jun 8, 2020 •

edited

Loading

sonypark Jun 9, 2020 •

edited

Loading