feat: 로드맵 키워드 구현 #1098

wishoon · 2022-10-24T17:20:57Z

구현 사항

키워드 생성
키워드 삭제
키워드 수정
키워드 단건 조회
세션에 속한 최상위 키워드 목록 조회
최상위 키워드에 속한 모든 자식 키워드 목록 조회

전달 사항

DB migration은 feat: 로드맵 퀴즈 구현 #1088 에서 수달이 진행해주셔서 따로 하지 않았습니다.
NewDocumentTest 기반으로 문서 테스트 작성했습니다.
a7918b1 에 대해서 개선 의견 받습니다 ㅎㅎ
- 현재는 키워드 계층의 갯수 + 1 만큼 조회 쿼리가 발생 하고 있습니다.. JPA 상에서 더 최적화를 할 수 있을지 고민인데 잘 방법이 떠오르지 않네용.. 개선방안 있으면 피드백 부탁드립니다 🙇‍♂️

2yujeong

안녕하세요 루키!
Keyword 엔티티가 계층 구조 때문에 구현하기 많이 힘들었을 것 같은데 정말 고생하셨습니다😭
말씀 드렸던 것처럼 조회 로직은 다 같이 코드보면서 고민해보는 게 좋을 것 같아요. 저도 지금은 더 좋은 방법이 생각나지 않네요😓 일단 흐름 파악만 했는데 요구 사항은 모두 잘 만족하고 있는 것 같아요👍🏼
코멘트는 CUD, 테스트 위주로 남겼어요. 개인적인 견해가 반영된 코멘트라서 읽어보시고 괜찮으면 반영해주세요!

2yujeong · 2022-10-26T06:25:20Z

backend/src/main/java/wooteco/prolog/roadmap/application/KeywordService.java

+            return null;
+        }
+        Optional<Keyword> findKeyword = keywordRepository.findById(keywordId);
+        return findKeyword.orElse(null);


Request에 지정된 parentKeywordId 값은 있는데 그 값에 해당하는 키워드는 DB에 없는 경우니까 이 경우는 예외를 던져주는 건 어떨까요?

엇 그렇네요 개발할 때 뭔가 잘못 생각하고 있었던 것 같네요 ㅋㅋ.. 😅

2yujeong · 2022-10-26T06:30:56Z

backend/src/main/java/wooteco/prolog/roadmap/application/KeywordService.java

+        return Keyword.createKeyword(
+            request.getName(), request.getDescription(), request.getOrder(),
+            request.getImportance(), sessionId, keywordParent


Keyword에 똑같은 매개변수 타입을 갖는 생성자가 있는데 여기서 정적 팩토리 메소드 + 빌더 패턴을 사용하신 이유가 있을까요?? 현재 해당 생성자는 선언만 되어 있고 쓰이지 않고 있는 것 같아용..!

앗.. 그렇네요 뭔가 빌더를 쓰다가 정적 팩터리 메서드로 변경하면서 그대로 가져가버린 것 같네요 ㅋㅋ.. 빌더를 지우고 정적 팩터리 메서드만 사용해도 괜찮을 것 같아요..!!

2yujeong · 2022-10-26T10:16:31Z

backend/src/main/java/wooteco/prolog/roadmap/Keyword.java

+    public void update(final String name, final String description, final int seq,
+                       final int importance, final Keyword keywordParent) {
+        this.name = name;
+        this.description = description;
+        this.seq = seq;
+        this.importance = importance;
+        this.parent = keywordParent;
+        validateKeywordParent();
    }


update할 때도 validateSeq()로 순서값 검증까지 해주면 좋을 것 같아요

2yujeong · 2022-10-26T10:25:29Z

backend/src/main/java/wooteco/prolog/roadmap/application/KeywordService.java

+
+        Keyword keyword = createKeyword(sessionId, request, keywordParent);
+        keywordRepository.save(keyword);
+        keyword.validateKeywordParent();


update처럼 엔티티를 생성할 때 Keyword 생성자 내부에서 검증 메소드를 호출하게 하는 건 어떨까요?

이것도 개발할 때 뭔가 잘못 생각하고 있었던 것 같네요 ㅋㅋ.. 😅 지금 다시 생각해보니 create 할때는 이 검증이 필요가 없어도 괜찮을 것 같은데 혹시 어떻게 생각하시나요?.?

2yujeong · 2022-10-26T10:49:38Z

backend/src/main/java/wooteco/prolog/roadmap/ui/KeywordController.java

+    public ResponseEntity<KeywordResponse> findKeyword(@PathVariable Long sessionId,
+                                            @PathVariable Long keywordId) {


Suggested change

public ResponseEntity<KeywordResponse> findKeyword(@PathVariable Long sessionId,

@PathVariable Long keywordId) {

public ResponseEntity<KeywordResponse> findKeyword(@PathVariable Long sessionId,

@PathVariable Long keywordId) {

2yujeong · 2022-10-26T11:51:58Z

backend/src/test/java/wooteco/prolog/roadmap/application/KeywordServiceTest.java

+        // given
+        Session session = createAndSaveSession(new Session("2022 백엔드 레벨 1"));
+        Keyword parent = createKeywordParent(Keyword.createKeyword("자바", "자바에 대한 설명", 1, 1, 1L, null));
+        Keyword child = createKeywordChildren(Keyword.createKeyword("List", "List에 대한 설명", 1, 1, 1L, parent));


테스트에서 child는 사용되지 않고 있는데, 밑에 then에서 child 키워드가 같이 삭제되는 것까지 검증해도 좋을 것 같아요!

앗 그렇네요.. 검증 추가하도록 할게요!!

2yujeong · 2022-10-26T12:07:12Z

backend/src/acceptanceTest/java/wooteco/prolog/steps/KeywordStepDefinitions.java

+    @When("{int}번 세션에 {string}라는 키워드를 순서 {int}, 중요도 {int}, 부모 키워드 {long}로 작성하고")
+    public void 키워드를_부모_키워드와_함께_작성하고(int sessionId, String keywordName, int seq, int importance, long parentId) {


혹시 @Given을 의도하신 걸까요?

헉.. 매의 눈이네요 👀 @Given을 의도한게 맞습니다 🙇‍♂️

2yujeong · 2022-10-26T12:18:07Z

backend/src/documentation/java/wooteco/prolog/docu/KeywordDocumentation.java

+        given(keywordService.findKeywordWithAllChild(any(), any())).willReturn(KEYWORD_WITH_ALL_CHILD_MULTI_RESPONSE);
+
+        given
+            .when().get()


url이 비어있네요..!

😅😅 꼼꼼하게 봐주셔서 감사합니다 이브~

2yujeong · 2022-10-26T12:19:06Z

backend/src/documentation/java/wooteco/prolog/docu/KeywordDocumentation.java

+    private static final KeywordCreateRequest KEYWORD_CREATE_REQUEST = new KeywordCreateRequest(
+        "자바",
+        "자바에 대한 설명을 작성했습니다.",
+        1,
+        1,
+        null
+    );


개행이 조금 어색한 것 같은데 혹시 의도하신 걸까요?

#1041 (comment) 백엔드 테스트 개선에 대한 Discussion 내용을 바탕으로 작성했어요 😁

2yujeong · 2022-10-26T12:19:53Z

backend/src/main/java/wooteco/prolog/roadmap/ui/KeywordController.java

+    public ResponseEntity<Void> createKeyword(@PathVariable Long sessionId,
+                                              @RequestBody KeywordCreateRequest createRequest) {
+
+        Long keywordId = keywordService.createKeyword(sessionId, createRequest);


Suggested change

public ResponseEntity<Void> createKeyword(@PathVariable Long sessionId,

@RequestBody KeywordCreateRequest createRequest) {

Long keywordId = keywordService.createKeyword(sessionId, createRequest);

public ResponseEntity<Void> createKeyword(@PathVariable Long sessionId,

@RequestBody KeywordCreateRequest createRequest) {

Long keywordId = keywordService.createKeyword(sessionId, createRequest);

wishoon · 2022-10-27T10:37:19Z

a7918b1 의 내용에 대해서 6531162 으로 변경했습니다

[변경 전]

[변경 후]

her0807

위에 이브가 코멘트를 많이 달아두셔서 제가 첨언할 부분이 많지 않네요. 루키 고생하셨습니다

her0807 · 2022-10-30T02:16:09Z

backend/src/acceptanceTest/java/wooteco/prolog/steps/KeywordStepDefinitions.java

+    /**
+     * MissionStepDefinitions 에 세션을 만드는 내용이 있지만 세션 1, 세션 2로 만들고 있어서 새롭게 생성하였음
+     * 추후 논의 후 MissionDefinitions 의 세션을 제거
+     */


javadoc 로 히스토리 마킹해두어서 좋네요 👍🏻

her0807 · 2022-10-30T02:20:01Z

backend/src/documentation/java/wooteco/prolog/docu/KeywordDocumentation.java

+                    null
+                ))
+        )
+//        Arrays.asList(


불필요한 주석은 제거해볼까요?

Set으로 자료구조를 변경하기 전의 테스트 값이네요.. 제거해서 올리도록 할게요~

her0807 · 2022-10-30T02:29:51Z

backend/src/main/java/wooteco/prolog/roadmap/application/KeywordService.java

+
+    public Long createKeyword(final Long sessionId, final KeywordCreateRequest request) {
+        existSession(sessionId);
+        Keyword keywordParent = findKeywordParentOrNull(request.getParentKeywordId());


전체 로직, 엔티티 연관관계만 살펴볼 때는 이해가 가는데,
여기 로직만 보면 갑자기 키워드에 null 이 들어갈 수 있게 되는 점이 어색하네요.
javadoc 으로 표시해두면 후임자가 파악하기 수월할 것 같아요

좋은 의견이네요!! 반영하도록 할게요~!

her0807

루키 ~ 전체적인 코드 다시 확인했습니다 ☺️
머지해주시고, 공유해주시면 이어서 작업하겠습니다!

her0807 · 2022-11-17T03:46:22Z

backend/src/main/java/wooteco/prolog/roadmap/domain/Keyword.java

+    public Keyword(final Long id, final String name, final String description, final int seq, final int importance,
+                   final Long sessionId, final Keyword parent, final Set<Keyword> children) {
+        validateSeq(seq);
+        this.id = id;
+        this.name = name;
+        this.description = description;
+        this.seq = seq;
+        this.importance = importance;
+        this.sessionId = sessionId;
+        this.parent = parent;
+        this.children = children;
+    }


생성하는 시점에 validate vs validate 된 값을 생성자로 주입 두개 의 장단점과 루키가 선택한 방식에 이유를 듣고싶네요

(지난번에 토미와 관련한 얘기를 나눴다고 공유했던게 생각나는데 구체적인 내용이 생각이 안나서,...! ㅎㅎㅎ)

첫번째로는 검증의 시점이 빨라진다는 장점이 있을 것 같아요! 생성하는 시점에 내부에서 validate를 할 경우 그 만큼 검증의 시점이 느려져서 불필요한 객체가 생성되는 문제점이 있을 것 같네요!!

두번째로는 값 객체를 사용함으로서 얻는 이점이 있을 것 같네요!! 가격이라는 객체를 예시로 든다면 가격은 int 타입일 수도 있고, BigDecimal이 될 수도 있습니다. 이 처럼 가격의 타입이 변경되더라도 사용하는 클라이언트는 코드 변경에 대한 전파가 이루어지지 않는다는 장점이 있을 것 같네요!!

her0807 · 2022-11-17T03:47:56Z

backend/src/main/java/wooteco/prolog/roadmap/ui/KeywordController.java

+        return ResponseEntity.ok(response);
+    }
+
+    @GetMapping("/{sessionId}/keywords/{keywordId}/children")


/{sessionId}/keywords 까지가 기본 path 인 것 같은데

@RequestMapping("/sessions/{sessionId}/keywords") 로 기본 path 를 지정해주는건 어떨까요 ?

wishoon · 2022-11-17T05:59:01Z

수달, 이브 리뷰 감사합니다~~ 머지할게용 😋

@given

* feat: Keyword의 정적 팩터리 메서드 생성 및 order 예약어 문제로 인한 컬럼명 수정 * feat: Keyword의 생성 어플리케이션 비즈니스 로직 구현 * feat: Keyword의 부모와 자식 키워드에 대한 전체 조회 도메인 기능 구현 * fix: Keyword의 parent 값도 Fetch로 가져오도록 수정 * feat: Keyword와 자식 Keyword들을 조회하는 어플리케이션 비즈니스 로직 구현 * feat: Keyword의 단일 조회에 대한 어플리케이션 비즈니스 로직 구현 * feat: Keyword의 수정에 대한 어플리케이션 비즈니스 로직 구현 * feat: Keyword의 삭제에 대한 어플리케이션 비즈니스 로직 구현 * feat: Session에 속하는 Keyword 목록을 가져오는 도메인 기능 구현 * feat: 하나의 Session에 속하는 최상위 Keyword 목록을 조회하는 어플리케이션 비즈니스 로직 구현 * refactor: ordinal -> seq로 이름 변경 * feat: Keyword를 생성하는 API 구현 * refactor: Keyword 엔티티와 대응되도록 컬럼 위치 변경 * feat: Keyword를 단건 조회하는 API 구현 * feat: Keyword를 단건 수정하는 API 구현 * feat: Keyword를 삭제하는 API 구현 * feat: 세선별 최상위 Keyword 목록 조회 API 구현 * feat: 최상위 Keyword에 속한 모든 자식 Keyword 목록 조회 API 구현 * test: 키워드 생성에 대한 인수테스트 작성 * test: 키워드 단일 조회에 대한 인수테스트 작성 * test: 키워드 수정에 대한 인수테스트 작성 * fix: children에 대해서 Left Fetch Join으로 수정 * test: 키워드 삭제에 대한 인수테스트 작성 * test: 세션별 최상위 키워드 목록 조회 인수테스트 작성 * test: 최상위 키워드의 모든 자식 키워드 조회 인수테스트 작성 * refactor: Fixture 이름 변경 * fix: documentTest의 URI 작성 오류 수정 * refactor: 최상위 키워드의 모든 자식 키워드 조회 개선 * fix: 잘못 작성된 @when을 @given으로 수정 * fix: 누락된 URL 추가 * refactor: 키워드 ID로 검색했을 때, 키워드가 없는 경우 예외가 발생하도록 변경 * feat: 잘못 사용되고 있는 검증 제거 * refactor: Builder 의존 제거 * feat: update에도 순서에 대한 검증 추가 * refactor: 코드 컨벤션 적용 * refactor: 도메인 생성 책임을 DTO로 변경 * refactor: 불필요한 주석 제거 * refactor: 패키지 구조 변경 * docs: javadoc를 통한 설명 추가 * refactor: 메서드 호출에 대한 validate 검증을 호출 시점으로 변경 * feat: AfterEach 상황에 동작하는 DataCleaner 추가 * refactor: 계층 구조로 테스트 코드 리팩토링 및 NewDataCleaner로 변경 * feat: equals & hashCode 재정의 * refactor: 기본 path 변경

wishoon added 27 commits October 22, 2022 15:39

feat: Keyword의 정적 팩터리 메서드 생성 및 order 예약어 문제로 인한 컬럼명 수정

a8be578

feat: Keyword의 생성 어플리케이션 비즈니스 로직 구현

10fae5e

feat: Keyword의 부모와 자식 키워드에 대한 전체 조회 도메인 기능 구현

0f74fc9

fix: Keyword의 parent 값도 Fetch로 가져오도록 수정

8e5d884

feat: Keyword와 자식 Keyword들을 조회하는 어플리케이션 비즈니스 로직 구현

f456b29

feat: Keyword의 단일 조회에 대한 어플리케이션 비즈니스 로직 구현

170cf8f

feat: Keyword의 수정에 대한 어플리케이션 비즈니스 로직 구현

762bb8f

feat: Keyword의 삭제에 대한 어플리케이션 비즈니스 로직 구현

5056633

feat: Session에 속하는 Keyword 목록을 가져오는 도메인 기능 구현

fbae104

feat: 하나의 Session에 속하는 최상위 Keyword 목록을 조회하는 어플리케이션 비즈니스 로직 구현

b92e1f6

refactor: ordinal -> seq로 이름 변경

c189d2b

feat: Keyword를 생성하는 API 구현

6497480

refactor: Keyword 엔티티와 대응되도록 컬럼 위치 변경

e31f9ae

feat: Keyword를 단건 조회하는 API 구현

d0eabaf

feat: Keyword를 단건 수정하는 API 구현

508e104

feat: Keyword를 삭제하는 API 구현

0b9fa00

feat: 세선별 최상위 Keyword 목록 조회 API 구현

2487987

feat: 최상위 Keyword에 속한 모든 자식 Keyword 목록 조회 API 구현

156a4b1

test: 키워드 생성에 대한 인수테스트 작성

c78c689

test: 키워드 단일 조회에 대한 인수테스트 작성

d4f04e7

test: 키워드 수정에 대한 인수테스트 작성

5777fbc

fix: children에 대해서 Left Fetch Join으로 수정

a7918b1

test: 키워드 삭제에 대한 인수테스트 작성

1b7e28d

test: 세션별 최상위 키워드 목록 조회 인수테스트 작성

9735a72

test: 최상위 키워드의 모든 자식 키워드 조회 인수테스트 작성

6a5548a

refactor: Fixture 이름 변경

6f7da35

fix: documentTest의 URI 작성 오류 수정

f5bd53d

wishoon requested review from verus-j, her0807 and 2yujeong October 24, 2022 17:20

wishoon self-assigned this Oct 24, 2022

2yujeong reviewed Oct 26, 2022

View reviewed changes

wishoon added 9 commits October 27, 2022 18:23

refactor: 최상위 키워드의 모든 자식 키워드 조회 개선

6531162

fix: 잘못 작성된 @when을 @given으로 수정

de5726f

fix: 누락된 URL 추가

258ba76

refactor: 키워드 ID로 검색했을 때, 키워드가 없는 경우 예외가 발생하도록 변경

3d1d2ef

feat: 잘못 사용되고 있는 검증 제거

6c8ed57

refactor: Builder 의존 제거

ae99565

feat: update에도 순서에 대한 검증 추가

178816f

refactor: 코드 컨벤션 적용

c940252

refactor: 도메인 생성 책임을 DTO로 변경

a445c2a

her0807 suggested changes Oct 30, 2022

View reviewed changes

wishoon added 7 commits November 16, 2022 10:42

refactor: 불필요한 주석 제거

87dbd05

refactor: 패키지 구조 변경

27017a6

docs: javadoc를 통한 설명 추가

4a9730b

refactor: 메서드 호출에 대한 validate 검증을 호출 시점으로 변경

387b28f

feat: AfterEach 상황에 동작하는 DataCleaner 추가

1052188

refactor: 계층 구조로 테스트 코드 리팩토링 및 NewDataCleaner로 변경

af80afc

feat: equals & hashCode 재정의

655b404

her0807 approved these changes Nov 17, 2022

View reviewed changes

refactor: 기본 path 변경

cac7171

wishoon merged commit efca18c into feature/1056-roadmap Nov 17, 2022

seokhongkim deleted the feature/1087-keyword branch January 20, 2023 07:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 로드맵 키워드 구현 #1098

feat: 로드맵 키워드 구현 #1098

wishoon commented Oct 24, 2022 •

edited

Loading

2yujeong left a comment

2yujeong Oct 26, 2022

wishoon Oct 26, 2022

2yujeong Oct 26, 2022

wishoon Oct 26, 2022

2yujeong Oct 26, 2022

wishoon Oct 26, 2022

2yujeong Oct 26, 2022

wishoon Oct 26, 2022

2yujeong Oct 26, 2022

2yujeong Oct 26, 2022

wishoon Oct 26, 2022

2yujeong Oct 26, 2022

wishoon Oct 26, 2022

2yujeong Oct 26, 2022

wishoon Oct 26, 2022

2yujeong Oct 26, 2022

wishoon Oct 26, 2022

2yujeong Oct 26, 2022

wishoon commented Oct 27, 2022

her0807 left a comment

her0807 Oct 30, 2022

her0807 Oct 30, 2022

wishoon Oct 30, 2022

her0807 Oct 30, 2022

wishoon Oct 30, 2022

her0807 left a comment

her0807 Nov 17, 2022 •

edited

Loading

wishoon Nov 17, 2022

her0807 Nov 17, 2022

wishoon commented Nov 17, 2022

		public ResponseEntity<KeywordResponse> findKeyword(@PathVariable Long sessionId,
		@PathVariable Long keywordId) {

		@When("{int}번 세션에 {string}라는 키워드를 순서 {int}, 중요도 {int}, 부모 키워드 {long}로 작성하고")
		public void 키워드를_부모_키워드와_함께_작성하고(int sessionId, String keywordName, int seq, int importance, long parentId) {

feat: 로드맵 키워드 구현 #1098

feat: 로드맵 키워드 구현 #1098

Conversation

wishoon commented Oct 24, 2022 • edited Loading

구현 사항

전달 사항

2yujeong 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

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

wishoon commented Oct 27, 2022

her0807 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

her0807 left a comment

Choose a reason for hiding this comment

her0807 Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wishoon commented Nov 17, 2022

wishoon commented Oct 24, 2022 •

edited

Loading

her0807 Nov 17, 2022 •

edited

Loading