Feature/86 봉사자의 관심기관 조회 기능 #96

7zrv · 2024-11-30T18:35:23Z

[FEATURE]봉사자 관심기관 조회 기능 구현 #86

📌 과제 설명

개인 마이페이지에서 사용할 봉사자의 관심기관 리스트 기능 구현입니다.

👩‍💻 요구 사항과 구현 내용

봉사자 Id를 이용한 관심기관 조회 기능 구현
기관 정보를 가져올 기관의 조회 메서드(서비스, 영속성 레이어) 구현
관심기관 목록을 반환 하는 관심기관의 조회 메서드 구현
컨트롤러 구현
테스트시 이용할 가짜 인증유저 어노테이션 기능 구현

✅ PR 포인트 & 궁금한 점

페이징 처리는 추후에 리팩토링을 진행하도록 하겠습니다.

테스트용 유저 어노테이션은 spring security 자체에 있는 UsernamePasswordAuthenticationToken을 응용하여
제작했습니다 자세한 내용은 금일 직접 설명 드리도록 하겠습니다.

이번 PR도 queryDSL에서의 실수가 없는지 봐주시면 감사하겠습니다!

m-a-king

컨트롤러 테스트를 복잡하지 않게 잘 작성하신 것 같습니다.
제가 해결하지 못한 문제를 해결해 주셔서 감사합니다!

다만, 이와 같은 컨트롤러 테스트가 서비스 테스트처럼 TDD의 목적에 부합하는가? 고민되는 순간이 있었습니다. 특히, Mock을 사용하다 보니 “Mock을 위한 Mock”이라는 모순적인 상황이 생겨서 이런 고민이 더 크게 다가온 것 같습니다.

여기서 Mock은 Mock 객체뿐만 아니라, 테스트만을 위한 설정도 포함된다고 생각하시면 조금 더 이해가 잘 되실 것 같습니다.

비슷한 고민을 리포지토리 테스트 작성 중에도 느꼈습니다. 서비스 테스트에서 실제 리포지토리를 주입받고 테스트를 진행하고 있는데, 굳이 별도의 리포지토리 테스트를 작성하는 것이 적절한지 의문이 들었습니다.

혹시 이 부분에 대해 다른 의견이나 좋은 방법이 있다면 여쭤보고 싶습니다! 이 글을 보신 모두에게...

m-a-king · 2024-11-30T20:12:04Z

src/test/java/com/somemore/CustomSecurityContextFactory.java

+public class CustomSecurityContextFactory implements WithSecurityContextFactory<WithMockCustomUser> {
+    @Override
+    public SecurityContext createSecurityContext(WithMockCustomUser annotation) {
+        SecurityContext context = SecurityContextHolder.createEmptyContext();


아 저는 이게 문제였을까요...

m-a-king · 2024-11-30T20:12:36Z

src/test/java/com/somemore/WithMockCustomUser.java

+@Retention(RetentionPolicy.RUNTIME)
+@WithSecurityContext(factory = CustomSecurityContextFactory.class)
+public @interface WithMockCustomUser {
+    String username() default "123e4567-e89b-12d3-a456-426614174000";
+}


어노테이션에 능숙하시네요. 저는 처음보는 문법이 많은 것 같습니다.

7zrv · 2024-11-30T21:31:00Z

컨트롤러 테스트를 복잡하지 않게 잘 작성하신 것 같습니다. 제가 해결하지 못한 문제를 해결해 주셔서 감사합니다!

다만, 이와 같은 컨트롤러 테스트가 서비스 테스트처럼 TDD의 목적에 부합하는가? 고민되는 순간이 있었습니다. 특히, Mock을 사용하다 보니 “Mock을 위한 Mock”이라는 모순적인 상황이 생겨서 이런 고민이 더 크게 다가온 것 같습니다.

여기서 Mock은 Mock 객체뿐만 아니라, 테스트만을 위한 설정도 포함된다고 생각하시면 조금 더 이해가 잘 되실 것 같습니다.

비슷한 고민을 리포지토리 테스트 작성 중에도 느꼈습니다. 서비스 테스트에서 실제 리포지토리를 주입받고 테스트를 진행하고 있는데, 굳이 별도의 리포지토리 테스트를 작성하는 것이 적절한지 의문이 들었습니다.

혹시 이 부분에 대해 다른 의견이나 좋은 방법이 있다면 여쭤보고 싶습니다! 이 글을 보신 모두에게...

무조건 하게 되는 고민 같습니다 이래서 테스트가 필요하구나! 하는 상황이 오지 않으면 테스트 작성에 공감하기
어려운 부분들이 있는것 같아요 저도 같은 고민을 했습니다.

우선 저는, TDD의 진짜 목적은 현재의 검증보다는 미래의 분기에 더 무게를 둔다고 생각을 하고 있는데요
해피케이스, 예외 케이스를 같이 작성해보면 위 목적에 공감이 가는 상황이 발생하는것 같아요
예를 들어 저는 컨트롤러에서 응답 Dto의 값을 변화시킬때, 새로운 응답 Dto를 적용했을때, Dto에 스네이크 케이스 미적용등 과 같은 이유로
테스트가 깨지는 상황이 발생했었습니다. 아마 테스트가 없었다면, 혹시나 코드 리뷰에서도 발견하지 못한다면
프론트 분들과 작업중 이슈가 발생했을 것 이라는 생각이 들었습니다.

그리고 리포지토리 테스트는 단위 테스트의 느낌이 더 강해서 고민을 하셨을 것 같습니다
마찬가지로 저도 고민이 들었던 부분인데요. 리포지토리의 경우 생각보다 리팩토링이 자주 일어나는것 같아요
예를 들어 Jpa 와 DSL 사이의 변환, 저번에 말씀해주신 where 절에서 id와 deleted의 순서변경과 같은 최적화, In연산을 적용하는 최적화등등
리팩토링이 자주 일어나지만 눈에 잘 보이지 않는 부분이라는 생각이 들었습니다.
그래서 리팩토링이 발생했을때 문제가 생기면 테스트 코드가 없을시 이를 서비스 레이어에서 잡아줘야 하는데
도메인이 많이 엮인 유스케이스 일수록 문제 해결이 늦어질 것이라고 생각합니다
리팩토링 결과를 테스트에서 미리 파악 한다면 유스케이스에 생길 사이드 이펙트를 예방할 수 있을것 같아요
물론 지금은 Q도메인을 커버리지에서 제외시키긴했지만...

요약하면 해피케이스, 예외 케이스에 대해 테스트 코드를 작성해두면 미래의 리팩토링시에 사이드 이펙트 파악이
쉬워지고 그만큼 사이드 이펙트 관리도 쉬워지는것 같아요
그래서 저는 작성을 해주는 편입니다.
하지만 프로젝트 완성이 1목표이기 때문에 테스트코드는 일정에 소화 가능한 만큼만 작성해줘도 될 것 같다는게 제 의견입니다.

leebs0521

페이징처리는 리팩토링으로 한다하셨으니 아래 페이징관련 코멘트는 무시하셔도 될 것 같습니다.
고생하셨습니다.

leebs0521 · 2024-12-01T03:22:50Z

src/main/java/com/somemore/center/dto/response/CenterOverviewInfoResponseDto.java

+        String centerName,
+        String imgUrl
+) {
+


통일성을 위해
center.repository.mapper 디렉토리에 있어야 할 것 같아요
네이밍도 responsedto가 아닌 CenterOverviewInfo 가 적당해보입니다.

감사합니다 반영하겠습니다

leebs0521 · 2024-12-01T03:23:50Z

src/main/java/com/somemore/interestcenter/dto/response/GetInterestCentersResponseDto.java

+@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class)
+@Builder
+public record GetInterestCentersResponseDto(
+        UUID centerId,


Get 빠져도 될 것 같아요.

스웨거도 적용하면 좋을 것 같습니다.

스웨거를 안적었군요 감사합니다

leebs0521 · 2024-12-01T03:26:43Z

src/main/java/com/somemore/interestcenter/controller/InterestCenterQueryApiController.java

+    @Operation(summary = "관심기관 목록 조회 API")
+    @GetMapping("/api/interest-centers")
+    public ApiResponse<List<GetInterestCentersResponseDto>> getInterestCenters(@AuthenticationPrincipal String volunteerId) {
+


명세상으로 페이징 처리 해야할 것 같아요

https://www.notion.so/prgrms/getInterestCenters-977bdc0b0c164da3bf7261df7991097d?pvs=4

leebs0521

GOOD

ayoung-dev

고생하셨습니다!
저도 controller 테스트 작성할 때 참고하겠습니다

- 기관Id 리스트로 기관 Id, 이름, 프로필 이미지 url을 조회하는 query 메서드를 영속성 레이어에 구현 - 서비스 레이어 구현 - 테스트 코드 작성및 검증 완

- 봉사자 Id로 관심기관 데이터를 조회하는 쿼리 메서드 구현 - 서비스 레이어 구현 - 관심 기관의 Id, 이름, 프로필 이미지 링크를 담아줄 응답 객체 생성 - 테스트 코드 작성및 검증 완

- 컨트롤러 테스트에서 이용할 인증유저 어노테이션을 구현했습니다.

- 관심기관 조회 컨트롤러 구현 - swagger 어노테이션 작성 - 테스트 코드 작성및 검증 완료

- 응답 객체의 불필요한 import문 제거

- 테스트용 contextFactory의 테스트 커버리지 예외 설정

- 일부 mapper 클래스의 네이밍을 수정 - mapper 클래스의 디렉토리 위치 수정 - 응답 클래스에 대한 스웨거 작

sonarqubecloud · 2024-12-01T06:55:24Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

m-a-king · 2024-12-01T08:00:25Z

컨트롤러 테스트를 복잡하지 않게 잘 작성하신 것 같습니다. 제가 해결하지 못한 문제를 해결해 주셔서 감사합니다!
다만, 이와 같은 컨트롤러 테스트가 서비스 테스트처럼 TDD의 목적에 부합하는가? 고민되는 순간이 있었습니다. 특히, Mock을 사용하다 보니 “Mock을 위한 Mock”이라는 모순적인 상황이 생겨서 이런 고민이 더 크게 다가온 것 같습니다.

여기서 Mock은 Mock 객체뿐만 아니라, 테스트만을 위한 설정도 포함된다고 생각하시면 조금 더 이해가 잘 되실 것 같습니다.

비슷한 고민을 리포지토리 테스트 작성 중에도 느꼈습니다. 서비스 테스트에서 실제 리포지토리를 주입받고 테스트를 진행하고 있는데, 굳이 별도의 리포지토리 테스트를 작성하는 것이 적절한지 의문이 들었습니다.
혹시 이 부분에 대해 다른 의견이나 좋은 방법이 있다면 여쭤보고 싶습니다! 이 글을 보신 모두에게...

무조건 하게 되는 고민 같습니다 이래서 테스트가 필요하구나! 하는 상황이 오지 않으면 테스트 작성에 공감하기 어려운 부분들이 있는것 같아요 저도 같은 고민을 했습니다.

우선 저는, TDD의 진짜 목적은 현재의 검증보다는 미래의 분기에 더 무게를 둔다고 생각을 하고 있는데요 해피케이스, 예외 케이스를 같이 작성해보면 위 목적에 공감이 가는 상황이 발생하는것 같아요 예를 들어 저는 컨트롤러에서 응답 Dto의 값을 변화시킬때, 새로운 응답 Dto를 적용했을때, Dto에 스네이크 케이스 미적용등 과 같은 이유로 테스트가 깨지는 상황이 발생했었습니다. 아마 테스트가 없었다면, 혹시나 코드 리뷰에서도 발견하지 못한다면 프론트 분들과 작업중 이슈가 발생했을 것 이라는 생각이 들었습니다.

그리고 리포지토리 테스트는 단위 테스트의 느낌이 더 강해서 고민을 하셨을 것 같습니다 마찬가지로 저도 고민이 들었던 부분인데요. 리포지토리의 경우 생각보다 리팩토링이 자주 일어나는것 같아요 예를 들어 Jpa 와 DSL 사이의 변환, 저번에 말씀해주신 where 절에서 id와 deleted의 순서변경과 같은 최적화, In연산을 적용하는 최적화등등 리팩토링이 자주 일어나지만 눈에 잘 보이지 않는 부분이라는 생각이 들었습니다. 그래서 리팩토링이 발생했을때 문제가 생기면 테스트 코드가 없을시 이를 서비스 레이어에서 잡아줘야 하는데 도메인이 많이 엮인 유스케이스 일수록 문제 해결이 늦어질 것이라고 생각합니다 리팩토링 결과를 테스트에서 미리 파악 한다면 유스케이스에 생길 사이드 이펙트를 예방할 수 있을것 같아요 물론 지금은 Q도메인을 커버리지에서 제외시키긴했지만...

요약하면 해피케이스, 예외 케이스에 대해 테스트 코드를 작성해두면 미래의 리팩토링시에 사이드 이펙트 파악이 쉬워지고 그만큼 사이드 이펙트 관리도 쉬워지는것 같아요 그래서 저는 작성을 해주는 편입니다. 하지만 프로젝트 완성이 1목표이기 때문에 테스트코드는 일정에 소화 가능한 만큼만 작성해줘도 될 것 같다는게 제 의견입니다.

감사합니다 ㅎㅎ
저도 테스트 적용이 올바르다고 판단되는 곳은 시간을 고려해서 테스트 먼저 작성해보겠습니다!

7zrv added Feature 서진 labels Nov 30, 2024

7zrv requested review from ayoung-dev, leebs0521 and m-a-king November 30, 2024 18:35

7zrv self-assigned this Nov 30, 2024

7zrv linked an issue Nov 30, 2024 that may be closed by this pull request

[FEATURE]봉사자 관심기관 조회 기능 구현 #86

Closed

3 tasks

m-a-king approved these changes Nov 30, 2024

View reviewed changes

leebs0521 requested changes Dec 1, 2024

View reviewed changes

leebs0521 approved these changes Dec 1, 2024

View reviewed changes

ayoung-dev approved these changes Dec 1, 2024

View reviewed changes

7zrv added 7 commits December 1, 2024 15:53

feat: 기관 아이디 리스트를 통한 오버뷰 데이터 조회 구현

f0efddd

- 기관Id 리스트로 기관 Id, 이름, 프로필 이미지 url을 조회하는 query 메서드를 영속성 레이어에 구현 - 서비스 레이어 구현 - 테스트 코드 작성및 검증 완

feat: 테스트용 인증 유저 어노테이션 구현

3459705

- 컨트롤러 테스트에서 이용할 인증유저 어노테이션을 구현했습니다.

feat: 관심기관 조회 컨트롤러 구현

046a08f

- 관심기관 조회 컨트롤러 구현 - swagger 어노테이션 작성 - 테스트 코드 작성및 검증 완료

chore: 불필요한 import문 제거

c417022

- 응답 객체의 불필요한 import문 제거

chore: 테스트 커버리지 제외 클래스 추가

5fc83b4

- 테스트용 contextFactory의 테스트 커버리지 예외 설정

chore: 코드 리뷰 사항 반영

43790f2

- 일부 mapper 클래스의 네이밍을 수정 - mapper 클래스의 디렉토리 위치 수정 - 응답 클래스에 대한 스웨거 작

7zrv force-pushed the feature/86-add-interest-center-get-volunteer-id branch from 59eef43 to 43790f2 Compare December 1, 2024 06:53

7zrv merged commit 8ed8bd0 into main Dec 1, 2024
2 checks passed

7zrv deleted the feature/86-add-interest-center-get-volunteer-id branch December 1, 2024 06:55

Feature/86 봉사자의 관심기관 조회 기능 #96

Feature/86 봉사자의 관심기관 조회 기능 #96

Uh oh!

Conversation

7zrv commented Nov 30, 2024

📌 과제 설명

👩‍💻 요구 사항과 구현 내용

✅ PR 포인트 & 궁금한 점

Uh oh!

m-a-king left a comment • 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!

7zrv commented Nov 30, 2024

Uh oh!

leebs0521 left a comment • 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!

leebs0521 left a comment

Choose a reason for hiding this comment

Uh oh!

ayoung-dev left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Dec 1, 2024

Quality Gate passed

Uh oh!

Uh oh!

m-a-king commented Dec 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

m-a-king left a comment •

edited

Loading

leebs0521 left a comment •

edited

Loading

m-a-king commented Dec 1, 2024 •

edited

Loading