-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[YS-141] feat: 실험자 본인이 작성한 공고 게시글 조회 API 구현 #34
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isLast
, page
, size
와 같은 응답 필드가 추가되면서 수정님과 이 부분을 어떻게 처리할지에 대해 협의를 하면 좋겠다고 생각이 들었습니다!
의문이 가는 점이나 더 좋은 방안이 생각나신다면 코멘트로 알려주세요~
fun getMyExperimentPosts(input: GetMyExperimentPostUseCase.Input): List<GetMyExperimentPostUseCase.Output> { | ||
validateSortOrder(input.pagination.order) | ||
return getMyExperimentPostUseCase.execute(input) | ||
} | ||
|
||
private fun validateSortOrder(sortOrder: String): String { | ||
return when (sortOrder) { | ||
"ASC", "DESC" -> sortOrder | ||
else -> throw InvalidParameterException("Invalid sort order. Please use 'ASC' or 'DESC'") | ||
} | ||
} | ||
|
||
fun getMyExperimentPostsCount(input: GetTotalMyExperimentPostCountUseCase.Input): GetTotalMyExperimentPostCountUseCase.Output { | ||
return getTotalMyExperimentPostCountUseCase.execute(GetTotalMyExperimentPostCountUseCase.Input(input.memberId)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
페이지네이션된 결과를 가져오는 유즈케이스와 isLast와 같은 필드 값을 계산하기 위해 자신이 작성한 게시글의 개수를 조회하는 유즈케이스를 둘로 나누어 진행한 이유는, 각 유즈케이스가 고유의 책임에 집중하도록 하기 위해서입니다.
페이지네이션된 결과 조회는 사용자에게 실험 공고 게시글 목록을 반환하는 데 집중하고, 게시글 개수 조회는 이후 isLast 필드 값을 계산하기 위해 전체 게시글 수만을 계산하는 역할을 하도록 했습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 저도 지수님 코드 보면서 해당 유즈케이스들이 SRP를 준수한 채 잘 짜여진 코드라고 생각했습니다!
- 목록 반환 : GetMyExperimentPostUseCase
isLast
필드를 통한 전체 게시글 개수 계산: GetTotalMyExperimentPostCountUseCase
다만, '여러 개의 포스트들을 조회한다' 라는 유즈케이스의 기능을 고려하여,
유즈케이스 명을 ~~Post
보다는 ~~Posts
로 리네이밍하면 어떨지 조심스레 의견 내봅니다 ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 저도 복수형으로 통일하면 더 일관성있을 거 같습니다. 👍
data class PostInfoOutput( | ||
val postId: Long, | ||
val experimentPostId: Long, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지금 배포된 API에서 experimentPostId
로 다 내보내고 있어서 통일성을 위해 수정했습니다~
val pagination = MemberMapper.toUseCasePagination(page, count, order) | ||
val input = MemberMapper.toGetMyExperimentPosts(pagination) | ||
val posts = memberService.getMyExperimentPosts(input) | ||
|
||
val totalCountInput = MemberMapper.toGetTotalMyExperimentPostCountUseCaseInput() | ||
val totalCount = memberService.getMyExperimentPostsCount(totalCountInput).totalPostCount | ||
return MemberMapper.toGetMyExperimentPostsResponse(posts, page, totalCount) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다른 유즈케이스처럼 input과 output만 사용하는 방식도 고려했으나, 고민 끝에 저희가 정한 아키텍처 원칙에 따르면 posts
와 totalCount
를 별도로 처리하여 넘겨주는 방식이 더 적합하다고 판단했습니다. 이렇게 작성한다고 해서 클린 아키텍처 원칙을 어기는 것도 아니라 생각하여, 이 방식으로 진행하게 되었습니다.
만약 더 나은 방법이 있으면 의견 부탁드립니다!
data class PaginatedResponse<T>( | ||
val content: List<T>, | ||
val page: Int, | ||
val size: Int, | ||
val isLast: Boolean | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 응답 DTO를 앞으로의 페이지네이션 기능에서 쓰면 좋을 거 같습니다!
}, | ||
page = page, | ||
size = output.size, | ||
isLast = isLastPage(totalCount, output.size, page), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isLast
여부를 판단하는 로직을 작성하는 위치가 애매했지만, 저희 로직 상으로는 Mapper
위치가 적합하다고 생각했습니다. 다만, Mapper
는 DTO 변환만 담당하는 것이 좋다는 원칙에 따라, isLastPage
계산 로직을 별도의 util 클래스로 분리하여 처리했습니다.
이렇게 하면 클린 아키텍처 원칙을 지키면서 SRP를 함께 유지할 수 있을 것으로 판단하여 이 방식으로 진행했습니다. 이 방식에 대해 수정님의 의견도 듣고 싶습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isLast 여부를 판단하는 로직이 Mapper 에 속하지 않아야한다는 점에 동의합니다!
지수님께서는 재사용성을 고려하시고 페이지네이션 결과를 보조적으로 평가하기 때문에, 부가로직이라는 판단 하에 Util 패키지에 위치하신 것 같아요.
다만, 현재 저희 프로젝트에서 커스텀 필터 로직에 따라 반환해야 하는 공고 데이터들이 달라지고, 해당 필터 로직의 복잡도가 1차 MVP 이후 단계에서 더 복잡해질 수도 있겠다는 생각이 들어요.
이에 따라, 마지막 페이지인지에 따라 추가 작업을 서버 단에서 트리거한다든가 하는 추가 요구사항 등이 늘어난다면, 더 이상 페이지네이션 결과를 보조적으로 평가하는 게 아닌 비즈니스의 핵심 로직이라고 생각합니다.
그래서 저는 해당 부분이 application 계층에 위치해야 한다고 생각해요!
지수님께서는 어떻게 생각하실까요? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isLast
여부를 판단하는 로직은 특정 비즈니스 유즈케이스의 실행 흐름 그 자체라기보다는, 비즈니스 도메인에서 공통적으로 활용되는 부가적인 로직에 더 가깝다고 생각했습니다. 따라서 이를 유즈케이스 내부에 포함시키는 것은 오히려 유즈케이스의 목적과 역할을 모호하게 만들 수 있어서 별도의 Util로 빼서 계산하도록 구현했습니다.
물론, 수정님의 의견을 듣고 isLast를 판단하는 여부 로직은 application 계층 내에서 계산을 하는 것이 좋을 거 같다는 의견에 공감하고 좋은 방향이라 생각합니다. 그래서 제 생각에는 utils에 있던 isLastPage 함수를 application 계층의 PaginationService
로 옮겨서 Controller 단에서 사용을 하면 좋지 않을까 생각이 듭니다. 아니면 기존 도메인 서비스에서 isLastPage 함수를 생성해서 쓰는 방법도 있는데 Pagination 기능은 여러 도메인에서 사용될 수 있기에 별도의 서비스로 빼는 것이 더 좋지 않을까 생각이 듭니다.
이 방식을 채택하면 이후 페이지네이션 관련 로직이 확장될 경우에도 유연하게 대응할 수 있다고 생각하는데 어떻게 생각하시는 지 궁금해요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
application 내에서의 비즈니스 중 부가 로직이라는 점에 적극 공감합니다! :D
PaginationService
로 옮겨서 별도로 모듈화를 시키면 비즈니스 로직에 좀 더 유연한 확장성을 확보할 수 있다는 방향인 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
여러 가지 고민하시고 구현하신 게 느껴져서 좋았어요.
관련해서 논의할 내용 남겨두었으니 확인 부탁드려요.
1. 공고 전체 조회 / 연구자 회원이 등록한 내 공고 조회 응답 방식 통일
-
두 개의 API 응답 방식을 굳이 통일하지 않아도 되지 않나 제안합니다.
디자인 시스템 보면 아시겠지만... 둘이 반환 시 필요한 필드값 종류 자체가 다릅니다!
그래서 동일한 응답 dto를 강제하는 것이 오히려 유지보수성 관점에서는 떨어질 수도 있지 않나 싶네요 🤔 -
- 저는 공고 전체 조회에서 한 블럭에 필요한 정보값들을 PostInfo 클래스로 묶었어요. 공고 전체 조회에서는 애초에 요청 필터링 기능 자체에 recruitDone의 여부를 체크할 수 있어서, 해당 값에 맞는 공고들만 조회하므로 반환값에서의 필수필드가 아니에요.
-
- 연구자 회원이 자신이 올린 공고를 조회하는 것에 있어서는, '공고 마감'을 처리할 수 있는 recruitDone이 필수 필드예요.
2. isLast 관련 페이지 계산하는 로직의 패키지 위치
- 지수님께서는 Util 패키지로 빼셨지만, 저는 application 계층에 위치해야 한다는 입장이에요. 관련해서 이유는 코멘트 남겨드렸으니 참고 부탁드려요!
fun getMyExperimentPosts(input: GetMyExperimentPostUseCase.Input): List<GetMyExperimentPostUseCase.Output> { | ||
validateSortOrder(input.pagination.order) | ||
return getMyExperimentPostUseCase.execute(input) | ||
} | ||
|
||
private fun validateSortOrder(sortOrder: String): String { | ||
return when (sortOrder) { | ||
"ASC", "DESC" -> sortOrder | ||
else -> throw InvalidParameterException("Invalid sort order. Please use 'ASC' or 'DESC'") | ||
} | ||
} | ||
|
||
fun getMyExperimentPostsCount(input: GetTotalMyExperimentPostCountUseCase.Input): GetTotalMyExperimentPostCountUseCase.Output { | ||
return getTotalMyExperimentPostCountUseCase.execute(GetTotalMyExperimentPostCountUseCase.Input(input.memberId)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 저도 지수님 코드 보면서 해당 유즈케이스들이 SRP를 준수한 채 잘 짜여진 코드라고 생각했습니다!
- 목록 반환 : GetMyExperimentPostUseCase
isLast
필드를 통한 전체 게시글 개수 계산: GetTotalMyExperimentPostCountUseCase
다만, '여러 개의 포스트들을 조회한다' 라는 유즈케이스의 기능을 고려하여,
유즈케이스 명을 ~~Post
보다는 ~~Posts
로 리네이밍하면 어떨지 조심스레 의견 내봅니다 ㅎㅎ
}, | ||
page = page, | ||
size = output.size, | ||
isLast = isLastPage(totalCount, output.size, page), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isLast 여부를 판단하는 로직이 Mapper 에 속하지 않아야한다는 점에 동의합니다!
지수님께서는 재사용성을 고려하시고 페이지네이션 결과를 보조적으로 평가하기 때문에, 부가로직이라는 판단 하에 Util 패키지에 위치하신 것 같아요.
다만, 현재 저희 프로젝트에서 커스텀 필터 로직에 따라 반환해야 하는 공고 데이터들이 달라지고, 해당 필터 로직의 복잡도가 1차 MVP 이후 단계에서 더 복잡해질 수도 있겠다는 생각이 들어요.
이에 따라, 마지막 페이지인지에 따라 추가 작업을 서버 단에서 트리거한다든가 하는 추가 요구사항 등이 늘어난다면, 더 이상 페이지네이션 결과를 보조적으로 평가하는 게 아닌 비즈니스의 핵심 로직이라고 생각합니다.
그래서 저는 해당 부분이 application 계층에 위치해야 한다고 생각해요!
지수님께서는 어떻게 생각하실까요? 🤔
제 의견은 페이지네이션 기능이 있는 API의 경우, 원래 응답해야 할 data를 담은 Response를 혹시 이 방식에 대해서도 공감이 가지 않으시다면, 이 부분은 함께 논의해보면 좋을 것 같습니다. 다만, 지금까지의 프로젝트에서는 페이지네이션 기능을 포함한 API의 응답 형식을 통일해왔고, 그렇게 해야 프론트엔드에서 작업하기 더 편하다는 피드백을 받아왔기 때문에 저는 이 방향으로 진행하는 것이 적절하다고 생각했습니다! |
아! 제가 전달 드리고자 하는 바에서 약간 오해가 있었던 것 같네요 😅 지수님께서 이해하신대로 진행하셔도 좋습니다! 해당 방식에 대해서 저도 동의합니다! 😊😊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고 많으셨습니다!
리뷰로 의견 나눈 부분이 잘 반영되어서 approve합니다.
conflict 난 부분만 확인하셔서 머지해주시면 감사하겠습니다 :D 💪
|
* feat: add Repository's method for GetMyExperimentPostUseCase * feat: add MemberMapper for GetMyExperimentPostUseCase * feat: add ParginatedResponse for pagination * refact: rename field or file name for consistency * feat: add MyExperimentPostResponse for get method * feat: add GetMyExperimentPosts's logic * refact: extract pagination logic into utility function in response mapper * feat: add sorting logic * refact: rename usecase name * refact: add PaginationService * test: add GetMyExperimentPostsUseCase Test * refact: rename usecase name * test: add GetMyExperimentPostTotalCountUseCase Test code * style: delete unused import * refact: delete unused utils class and rename controller tag name
💡 작업 내용
DESC 정렬
![image](https://private-user-images.githubusercontent.com/69844138/403295986-67536bae-6050-4c68-9e16-4555f95b251b.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NTMyMzEsIm5iZiI6MTczOTY1MjkzMSwicGF0aCI6Ii82OTg0NDEzOC80MDMyOTU5ODYtNjc1MzZiYWUtNjA1MC00YzY4LTllMTYtNDU1NWY5NWIyNTFiLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDIwNTUzMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWI1ZmIyOGExN2M3MGNmMWJmNTdlOGI4YjRmODM2YzNlMDdlYWYwNjI5MzRiZTI1NDcxZTk0ZTE2M2QwOTAxM2EmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.VdH91_qIAJ5mj3w7fkt5RrK5fORgKZICmQ3IcD_WChI)
ASC 정렬
![스크린샷 2025-01-15 오후 4 10 49](https://private-user-images.githubusercontent.com/69844138/403293900-90cd3ed7-3e48-4b74-9668-57aab5cb9951.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NTMyMzEsIm5iZiI6MTczOTY1MjkzMSwicGF0aCI6Ii82OTg0NDEzOC80MDMyOTM5MDAtOTBjZDNlZDctM2U0OC00Yjc0LTk2NjgtNTdhYWI1Y2I5OTUxLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDIwNTUzMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTdkMGNlOTYzZmQ3ZmJjOWU4ZTRjODJiOTI4MWEwYjNjYWUxYjhmMjExZjBlMTI5ZGJmNjI4MDhkMWE3ZmMwNDUmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.F4s1dbELmzMpMTV7KirmZ9-ISKuSroorDEp8MHHoccA)
잘못된 정렬 파라미터에 대한 예외
![스크린샷 2025-01-15 오후 4 11 00](https://private-user-images.githubusercontent.com/69844138/403293709-b22414df-ba64-4c7b-a42c-76d51b1f84c3.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NTMyMzEsIm5iZiI6MTczOTY1MjkzMSwicGF0aCI6Ii82OTg0NDEzOC80MDMyOTM3MDktYjIyNDE0ZGYtYmE2NC00YzdiLWE0MmMtNzZkNTFiMWY4NGMzLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDIwNTUzMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTc0N2ZmN2FlNjRhODZiNzliOTA3YjlmNTc1MmNjZDg5NWY3MDIzNjY2OGUzMGVmNjM4OWMxOWUzNzQ5NWU4YzgmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.Y0oyVL4yXf7_Wdr4L6ig3JoA5wWCHsfuieg4IgglOxY)
✅ 셀프 체크리스트
🙋🏻 확인해주세요
🔗 Jira 티켓
https://yappsocks.atlassian.net/browse/YS-141