Skip to content
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

[Feat]: 채팅방 메세지를 조회할 수 있다 #14

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

Hwanvely
Copy link
Contributor

@Hwanvely Hwanvely commented May 15, 2024

관련 이슈

변경 사항

TO-BE

  • cursor가 없으면 chat_user created_time기준으로 조회
  • cursor있으면 cursor기준으로 조회

@Hwanvely Hwanvely requested a review from jihwan2da May 15, 2024 04:10
@Hwanvely Hwanvely self-assigned this May 15, 2024
Copy link
Member

@jihwan2da jihwan2da left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 테스트 된건가요?? 테스트 안되었다면 테스트 코드 작성해주세요
테스트 코드 작성하기 힘들다면 로컬에서 테스트 부탁드립니다!~

Controller 로직도 빠진거 같네요

import gloddy.util.CursorRequest
import gloddy.util.PageCursor

class GroupChatGetMessageCommander(private val getGroupChatMessagePort: GetGroupChatMessagePort) : GetGroupChatMessageUseCase {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

조회에 대한 도메인 서비스 이름을 ~Commander라고 지은 이유가 뭔가요??
Commander는 CUD에 대한 도메인 서비스를 가리키는 의도로 지었는데 여기서는 GroupChatReader라는 클래스 명이 맞지 않을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CUD에 대한 서비스가 아닌 단순히 '요청(명령)을 한다'라는 의미로 이해하여 네이밍을 하게되었습니다. 말씀해주신대로 GroupChatReader로 변경하고 앞으로 맞춰서 작성하겠습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그럼 혹시 dto 패키지 하위의 command들도 CUD를 위한 dto인가요??

Comment on lines 7 to 10
interface GetGroupChatMessageUseCase {

fun getPosts(groupChatGetMessageCommand: GroupChatGetMessageCommand): PageCursor<GroupChatMessage>
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

도메인 서비스에 대한 추상화는 필요 없을 거라는 판단하에 GroupChatCommander 또한 추상화를 하지 않았기 때문에 해당 클래스는 삭제 부탁드립니다~

Comment on lines 6 to 10
interface GetGroupChatMessagePort {
fun findWithoutKey(groupChatGetMessageCommand: GroupChatGetMessageCommand): List<GroupChatMessage>

fun findWithKey(groupChatGetMessageCommand: GroupChatGetMessageCommand): List<GroupChatMessage>
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Domain 모듈의 영속성 추상클래스 <-(implement) Storage 모듈의 영속성 클래스의 이름 규칙을
~Repository <- ~AdapterRepository 로 Command 로직을 짤 때 정했기 때문에
GroupChatQueryRepository <- GroupChatQueryAdapterRepository로 수정부탁합니다~

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인했습니다 앞으로 반영하겠습니다🫡

import gloddy.persistence.repository.GroupChatMessageRepository
import gloddy.port.out.GetGroupChatMessagePort

class GroupChatMessageCommandAdapter(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

조회용 Adapter Repository니깐 GroupChatQueryAdapterRepository가 맞는 표현이겠죠??
수정부탁드립니다~

그리고 GroupChat 하위 도메인들(GroupChatMessage, GroupChatUser) 또한 그에 맞는 새로운 레포지토리를 파는게 아닌 GroupChatRepository 내부에서 처리하는게 관리가 용이할 것 같애서 위처럼 GroupChatQueryAdapterRepository로 수정 부탁드립니다~!

Copy link
Contributor Author

@Hwanvely Hwanvely May 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GroupChatRepository 내부에서 처리하는게 관리가 용이할 것 같다는 말씀이 GroupChat(Query, Command) Repository이런 방식을 말씀하시는거죠??
수정했습니다!

Comment on lines 10 to 12

class GroupChatMessageRepository(private val namedParameterJdbcTemplate: NamedParameterJdbcTemplate) {

Copy link
Member

@jihwan2da jihwan2da May 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QueryDsl이나 JPARepository가 아닌 NamedParameterJdbcTemlate를 사용한 이유가 뭔가요??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

일단 쿼리를 직접짜야겠다는 생각을 하였고 보다 익숙한 NamedParameterJdbcTemplate을 사용했습니다. 추후에 리팩토링을 해보겠습니다.

@Hwanvely
Copy link
Contributor Author

테스트 제외 리뷰 반영해서 올렸습니다. 확인한번 해주세요 🫡

@Hwanvely Hwanvely requested a review from jihwan2da May 29, 2024 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

채팅방 메세지를 조회할 수 있다
2 participants