[BE] refactor: 세션을 가져오는 역할을 `ReviewGroupSessionResolver`에 위임 #843

donghoony · 2024-10-12T13:13:09Z

resolves [BE] 세션에서 객체를 생성하는 Resolver를 구현한다. #815

🚀 어떤 기능을 구현했나요 ?

세션에서 ReviewGroup을 꺼내는 Resolver를 만들었습니다.
중복으로 존재했던 repository.findByReviewRequestCode().orElseThrow()를 많이 없앴습니다. 테스트도 몇 개 제거했어요.

🔥 어떻게 해결했나요 ?

세션이 없거나, 세션에 reviewRequestCode가 존재하지 않으면 새로운 예외를 발생합니다.
기존에 존재하던 HeaderAttribute를 더이상 사용하지 않아 제거했습니다.

📝 어떤 부분에 집중해서 리뷰해야 할까요?

Layered Architecture 상 Controller에 위치하는데요, 패키지 위치가 적당한지 봐주시면 감사하겠슴다
Resolver가 Component로 등록돼 있는데요, 이게 적절할까요? 🤔 Resolver에서 Repository를 꺼내는 게 레이어를 너무 넘나들지 않은가요? 중간에 Service가 하는 게 적절할까 싶어서요. 자유롭게 토의해주세요!
CorsConfigTest 두 개를 Disable 처리했습니다. 이미 도는 것을 확인했는데 얘 때문에 컨텍스트가 두 개나 새로 뜨더라고요. 나중에 확인할 때 띄워보아도 좋겠습니다.

github-actions · 2024-10-12T13:14:36Z

Test Results

145 tests - 1 142 ✅ - 4 5s ⏱️ -1s
55 suites ±0 3 💤 +3
55 files ±0 0 ❌ ±0

Results for commit 980649c. ± Comparison against base commit d93a1a7.

This pull request removes 6 and adds 5 tests. Note that renamed tests count towards both.

reviewme.global.HeaderPropertyArgumentResolverTest ‑ 검증값이_헤더에_존재하면_값을_반환한다()
reviewme.global.HeaderPropertyArgumentResolverTest ‑ 검증값이_헤더에_존재하지_않으면_검증에_실패한다()
reviewme.review.service.ReviewDetailLookupServiceTest ‑ 잘못된_리뷰_요청_코드로_리뷰를_조회할_경우_예외가_발생한다()
reviewme.review.service.ReviewListLookupServiceTest ‑ 리뷰_요청_코드가_존재하지_않는_경우_예외가_발생한다()
reviewme.review.service.ReviewSummaryServiceTest ‑ 리뷰_요약_정보_조회시_리뷰_요청_코드가_존재하지_않는_경우_예외가_발생한다()
reviewme.template.service.SectionServiceTest ‑ 템플릿에_있는_섹션_이름_목록_조회시_리뷰_요청_코드가_존재하지_않는_경우_예외가_발생한다()

reviewme.reviewgroup.controller.ReviewGroupSessionResolverTest ‑ 세션에_코드가_없는_경우_예외를_발생한다()
reviewme.reviewgroup.controller.ReviewGroupSessionResolverTest ‑ 세션에서_코드를_가져와_리뷰그룹으로_변환한다()
reviewme.reviewgroup.controller.ReviewGroupSessionResolverTest ‑ 세션이_존재하지_않는_경우_예외를_발생한다()
reviewme.reviewgroup.service.ReviewGroupServiceTest ‑ 리뷰_요청_코드로_리뷰_그룹을_반환한다()
reviewme.reviewgroup.service.ReviewGroupServiceTest ‑ 리뷰_요청_코드로_리뷰_그룹을_찾을_수_없는_경우_예외가_발생한다()

♻️ This comment has been updated with latest results.

skylar1220

온갖 서비스에서 있던 리뷰 그룹 찾기 로직이 빠진 것, 관련해 테스트도 다 없어진 것 너무 좋네요!
스스로 그동안 저걸 보고도 분리해야겠다는 생각을 못했다니... 바뀐 코드보니 불편한점을 찾는 게 중요하다는 생각이 다시 드네요😂
질문한 부분에 대해 산초, 테드와 더 얘기해봅시당!

skylar1220 · 2024-10-14T01:56:48Z

backend/src/main/java/reviewme/reviewgroup/controller/ReviewGroupSessionResolver.java

+import reviewme.reviewgroup.domain.ReviewGroup;
+import reviewme.reviewgroup.repository.ReviewGroupRepository;
+
+@Component


의견)
WebConfig에서만 최초 생성되고 쓰이고 끝이니까 굳이 컴포넌트가 아니어도 된다고 생각해요~

제거했습니다 확인해주세요~

skylar1220 · 2024-10-14T02:01:25Z

backend/src/main/java/reviewme/reviewgroup/controller/ReviewGroupSessionResolver.java

+
+    private static final String SESSION_KEY = "reviewRequestCode";
+
+    private final ReviewGroupRepository reviewGroupRepository;


의견)
만약 resolver가 repository에 의존하게 된다면, 컨트롤러의 패키지에 있는 객체가 repository에 의존하는, 너무 도메인 레벨을 의존한다는 느낌이 들어요.

따라서, 안에서 리뷰 그룹을 조회하느라 어떤 작업을 하든 서비스에서 처리하고 리졸버는 그 ReviewGroupService를 의존함으로써 변경 사항에 대해서도 유연할 수 있지 않을까 싶어요.

이건 저도 그렇게 생각해유~

skylar1220 · 2024-10-14T02:05:38Z

backend/src/main/java/reviewme/reviewgroup/controller/ReviewGroupSessionResolver.java

@@ -0,0 +1,46 @@
+package reviewme.reviewgroup.controller;


의견)
패키지 위치는 지금처럼 reviewGroup.controller가 적절하다고 생각해요.

요청이 컨트롤러로 들어오고 → 해당하는 부분 처리를 리졸버로 보냈다가 → 컨트롤러로 다시 받아서 다음 처리를 한다.

위처럼 컨트롤러의 처리 사이에서만 쓰이기때문이죠!

donghoony · 2024-10-14T05:59:13Z

backend/src/test/java/reviewme/reviewgroup/controller/ReviewGroupSessionResolverTest.java

+    private final ReviewGroupService reviewGroupService = mock(ReviewGroupService.class);
+
+    private ReviewGroupSessionResolver reviewGroupSessionResolver;


Service를 모킹합니다. 그 이유는:

ReviewGroup을 잘 찾아오는지에 대한 테스트는 ReviewGroupService에 이미 작성되었습니다.

Service가 어떻게 동작하는지가 궁금한 게 아니라, Resolver 내부의 if에 따른 분기가 잘 이루어지고 있는지를 테스트하고자 했습니다.

skylar1220

반확완🚀🚀

nayonsoso

기존에 존재하던 HeaderAttribute를 더이상 사용하지 않아 제거했습니다.

확인완✅

Layered Architecture 상 Controller에 위치하는데요, 패키지 위치가 적당한지 봐주시면 감사하겠슴다

argumentResolver 는 controller 패키지가 맞다고 생각합니다👍

Resolver가 Component로 등록돼 있는데요, 이게 적절할까요?

커비와 같은 의견입니다🙆🏻‍♀️

CorsConfigTest 두 개를 Disable 처리했습니다.

Disable 떼고 실행해봤는데, 아래 예외가 생기면서 테스트 실패하고 있어요.!
의도한 상황이 맞나요?😨

Caused by: org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'webConfig' defined in file [/Users/nayonsoso/Documents/IdeaProjects/2024-review-me/backend/build/classes/java/main/reviewme/config/WebConfig.class]: Unsatisfied dependency expressed through constructor parameter 0: No qualifying bean of type 'reviewme.reviewgroup.service.ReviewGroupService' available: expected at least 1 bean which qualifies as autowire candidate. Dependency annotations: {}

(마지막 질문 때문에 RC 합니다~ 요 부분만 확인해주세요!)

nayonsoso · 2024-10-14T11:19:38Z

backend/src/main/java/reviewme/reviewgroup/controller/ReviewGroupSessionNotFoundException.java

+public class ReviewGroupSessionNotFoundException extends BadRequestException {
+
+    public ReviewGroupSessionNotFoundException() {
+        super("리뷰 그룹 세션이 존재하지 않습니다.");


[짱사소]
"~요"로 끝도록 바꿔봐요😊

nayonsoso

어푸루브 합니다~

Kimprodp

불필요한 부분이 제거되니 깔끔해져서 좋네요 😊👍

이번 구현 내용은 아니지만 보다가 발견한 아주 사소한 부분 하나 반영 부탁할게요~

    @PostMapping("/v2/groups/check")
    public ResponseEntity<Void> checkGroupAccessCode(
            @RequestBody @Valid CheckValidAccessRequest request,
            HttpServletRequest httpRequest
    ) {

요기 @RequestBody @Valid 어노테이션 순서가 다른 곳이랑 달라서 머지 전에 같이 반영 부탁합니다~

donghoony · 2024-10-15T05:13:59Z

#845 에서 반영되었어요 😁

내부에 Repository가 존재하여 자체를 Mocking합니다. 이에 대한 테스트는 따로 진행했어요.

두 테스트 때문에 컨텍스트가 두 개나 뜹니다..

* refactor: 사용하지 않는 클래스 제거 * feat: 세션에서 리뷰 그룹 resolve * refactor: `Resolver` `@MockBean` 등록 내부에 Repository가 존재하여 자체를 Mocking합니다. 이에 대한 테스트는 따로 진행했어요. * refactor: `CorsConfig` Disable 처리 두 테스트 때문에 컨텍스트가 두 개나 뜹니다.. * feat: `Resolver`를 사용해 리뷰 그룹을 가져오도록 구현 * chore: 사용하지 않는 `ReviewGroupRepository` 필드 제거 * refactor: 서비스 메서드 추가 및 `ServiceTest` 제거 * fix: `ReviewGroupService` mockbean 처리 * chore: -요 체로 변경

donghoony added ✨ BE 🛠️ 리팩터링 labels Oct 12, 2024

donghoony added this to the 6차 스프린트: 최종장 milestone Oct 12, 2024

donghoony self-assigned this Oct 12, 2024

donghoony linked an issue Oct 12, 2024 that may be closed by this pull request

[BE] 세션에서 객체를 생성하는 Resolver를 구현한다. #815

Closed

donghoony requested review from skylar1220, Kimprodp and nayonsoso and removed request for skylar1220 and Kimprodp October 12, 2024 13:14

skylar1220 requested changes Oct 14, 2024

View reviewed changes

donghoony commented Oct 14, 2024

View reviewed changes

skylar1220 approved these changes Oct 14, 2024

View reviewed changes

nayonsoso requested changes Oct 14, 2024

View reviewed changes

nayonsoso approved these changes Oct 15, 2024

View reviewed changes

Kimprodp approved these changes Oct 15, 2024

View reviewed changes

donghoony added 9 commits October 15, 2024 14:20

refactor: 사용하지 않는 클래스 제거

c3adcad

feat: 세션에서 리뷰 그룹 resolve

ae1edbf

refactor: Resolver @MockBean 등록

ee36397

내부에 Repository가 존재하여 자체를 Mocking합니다. 이에 대한 테스트는 따로 진행했어요.

refactor: CorsConfig Disable 처리

f06edf6

두 테스트 때문에 컨텍스트가 두 개나 뜹니다..

feat: Resolver를 사용해 리뷰 그룹을 가져오도록 구현

04a459b

chore: 사용하지 않는 ReviewGroupRepository 필드 제거

b23d810

refactor: 서비스 메서드 추가 및 ServiceTest 제거

dc9797b

fix: ReviewGroupService mockbean 처리

3ba4822

chore: -요 체로 변경

980649c

donghoony force-pushed the be/refactor/815-session-resolver branch from f71330d to 980649c Compare October 15, 2024 05:20

donghoony merged commit 8c3c5ae into develop Oct 15, 2024
2 checks passed

donghoony deleted the be/refactor/815-session-resolver branch November 17, 2024 08:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] refactor: 세션을 가져오는 역할을 `ReviewGroupSessionResolver`에 위임 #843

[BE] refactor: 세션을 가져오는 역할을 `ReviewGroupSessionResolver`에 위임 #843

donghoony commented Oct 12, 2024 •

edited

Loading

github-actions bot commented Oct 12, 2024 •

edited

Loading

skylar1220 left a comment

skylar1220 Oct 14, 2024

donghoony Oct 14, 2024

skylar1220 Oct 14, 2024 •

edited

Loading

donghoony Oct 14, 2024

skylar1220 Oct 14, 2024 •

edited

Loading

donghoony Oct 14, 2024

skylar1220 left a comment

nayonsoso left a comment

nayonsoso Oct 14, 2024

nayonsoso left a comment

Kimprodp left a comment •

edited by donghoony

Loading

donghoony commented Oct 15, 2024


		private static final String SESSION_KEY = "reviewRequestCode";

		private final ReviewGroupRepository reviewGroupRepository;

		private final ReviewGroupService reviewGroupService = mock(ReviewGroupService.class);

		private ReviewGroupSessionResolver reviewGroupSessionResolver;

[BE] refactor: 세션을 가져오는 역할을 ReviewGroupSessionResolver에 위임 #843

[BE] refactor: 세션을 가져오는 역할을 ReviewGroupSessionResolver에 위임 #843

Conversation

donghoony commented Oct 12, 2024 • edited Loading

🚀 어떤 기능을 구현했나요 ?

🔥 어떻게 해결했나요 ?

📝 어떤 부분에 집중해서 리뷰해야 할까요?

github-actions bot commented Oct 12, 2024 • edited Loading

Test Results

skylar1220 left a comment

Choose a reason for hiding this comment

skylar1220 Oct 14, 2024

Choose a reason for hiding this comment

donghoony Oct 14, 2024

Choose a reason for hiding this comment

skylar1220 Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

donghoony Oct 14, 2024

Choose a reason for hiding this comment

skylar1220 Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

donghoony Oct 14, 2024

Choose a reason for hiding this comment

skylar1220 left a comment

Choose a reason for hiding this comment

nayonsoso left a comment

Choose a reason for hiding this comment

nayonsoso Oct 14, 2024

Choose a reason for hiding this comment

nayonsoso left a comment

Choose a reason for hiding this comment

Kimprodp left a comment • edited by donghoony Loading

Choose a reason for hiding this comment

donghoony commented Oct 15, 2024

[BE] refactor: 세션을 가져오는 역할을 `ReviewGroupSessionResolver`에 위임 #843

[BE] refactor: 세션을 가져오는 역할을 `ReviewGroupSessionResolver`에 위임 #843

donghoony commented Oct 12, 2024 •

edited

Loading

github-actions bot commented Oct 12, 2024 •

edited

Loading

skylar1220 Oct 14, 2024 •

edited

Loading

skylar1220 Oct 14, 2024 •

edited

Loading

Kimprodp left a comment •

edited by donghoony

Loading