Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[#12] 사용자와 아이템을 조회하기 위한 Feign 클라이언트 및 에러 디코더 추가 #19

Merged
merged 9 commits into from
Nov 23, 2024

Conversation

jdalma
Copy link
Member

@jdalma jdalma commented Nov 16, 2024

목적

체크아웃 기능에서 payment 서비스가 user와 cart를 조회하기 위한 feign 클라이언트 및 에러 디코더 추가


[#16 바운디드 컨텍스트간 멀티모듈 적용] 이슈 포함

AS-IS

  • user
    • user
    • item
  • payment
    • payment
    • wallet
    • ledger

TO-BE

  • user-service
  • item-service
  • payment-service
  • wallet-service
  • common

GAP

기존에는 user와 payment 모듈이 여러 개의 도메인들을 일반 패키지로 보유하고 있어 의존성에 대한 격리가 완벽히 이루어지지 않았고, user와 payment 모듈의 책임들이 한 눈에 안보이는 문제가 있었습니다.
그래서 core 모듈 내부에 각 도메인 별로 모듈을 설정하여 의존성을 확실하게 격리시키고 모듈의 책임을 더 노출할 수 있도록 개선하였습니다.

- 체크아웃을 생성하는 과정에 필요한 사용자와 카트 정보 조회 구현
@jdalma jdalma requested a review from f-lab-lyan November 16, 2024 10:24
@jdalma jdalma self-assigned this Nov 16, 2024
import org.springframework.http.HttpStatus
import org.springframework.web.server.ResponseStatusException

class MockUserServiceClient: UserServiceClient {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 클래스는 모킹으로 해결할 수 있지 않을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

넵 모키토를 적극적으로 활용하겠습니다 ㅎㅎ 모킹의 복잡한 과정을 MockContainer로 옮겨놓았습니다

predicates:
- Path=/*
- Path=/*
logging:
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 여전히 logback.xml을 더 많이 쓰는 것 같네요. :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

추후 [#10 공통 모듈 추가] 이슈 처리할 때 공통 모듈에 로깅 기능을 추가하는 것으로 해결해보겠습니다

private val toy = listOf(Category("220", "Toys & Hobbies"),)

private val items = listOf(
Item("A", "camera", camera, 100000, 1, LocalDateTime.now()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

요렇게 쓴단 얘기는 나중에 join 같은 걸로 데이터를 가져오겠다는 얘기인거죠?

Copy link
Member Author

Choose a reason for hiding this comment

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

네 맞습니다. JPA 엔티티로 관계를 맺어서 해결해볼까 합니다

import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content
import org.springframework.web.context.WebApplicationContext

@SpringBootTest
Copy link
Collaborator

Choose a reason for hiding this comment

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

SpringBootTest와 mockMvc조합은 괜찮은 건가요? mockMvc를 쓸 건데 굳이 SpringBootTest를 쓸 필요가 있을까 싶어서요 ... :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

네 맞습니다 ㅎㅎ 테스트에 대해 조사하기 전에 작성해놓았던 테스트 코드인데 수정하는 것을 깜빡했네요 ㅎㅎ 수정해놓겠습니당

.andDo(document("find-item-by-id-success"))
.andReturn()

val actual = objectMapper.readValue(actualResult.response.contentAsString, ItemResponse::class.java)
Copy link
Collaborator

Choose a reason for hiding this comment

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

저는 개인적으로 이게 jsonPath를 쓰는 것보다 훨씬 좋은 것 같아요. 👍

val springCloudDependenciesVersion: String by project
val kotestExtensionsSpring: String by project
dependencies {
implementation("org.springframework.boot:spring-boot-starter-data-jpa")
Copy link
Collaborator

@f-lab-lyan f-lab-lyan Nov 18, 2024

Choose a reason for hiding this comment

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

이건 지금 필요한건가요? -> 어디서 쓰이는지 찾았습니다.

import org.jetbrains.kotlin.gradle.tasks.KotlinCompile

plugins {
application
Copy link
Collaborator

Choose a reason for hiding this comment

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

제가 아마도 안써봐서 모르는 걸텐데 ... 이 application플러그인은 스프링을 써도 필요한건가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

확인해보니 application 플러그인은 자바 애플리케이션 배포를 위한 추가 태스크, lib 경로 설정 등이 추가적으로 제공되네요 ㅎㅎ 자바 플러그인만 사용하여도 무방한 것 같습니다 수정하겠습니다

annotation("jakarta.persistence.Embeddable")
}

tasks.getByName("bootJar") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 기본값이 true인데 굳이 써줘야 하나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

굳이 작성해놓을 이유는 없을 것 같네요 ㅎㅎ 제거하겠습니다

import org.springframework.stereotype.Repository

@Repository
class CartRepository (
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 필요한 서비스에 일일이 다 만드실 생각이신건가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

네 만약 user 서비스에서 payment order 정보가 필요한 경우에도 user 서비스에 PaymentOrderRepository를 작성할 것 같습니다.
user 서비스가 실제로 payment db를 직접 뒤지진 않겠지만 FeignClient 같은 웹 클라이언트나 메시지 큐 클라이언트를 Repository가 주입받는 구조로 해결하면 일관성을 지킬 수 있을 것 같습니다

혹시 다른 방법이나 구조를 생각하고 계신것일까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yaml 파일은 분리해보는게 어떨까 싶은데요.. :-)

Copy link
Collaborator

Choose a reason for hiding this comment

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

아항 ~ 이게 그 목 객체군요. ㅎㅎㅎ

@f-lab-lyan
Copy link
Collaborator

수고하셨습니다. 이 PR 머지하고, 나머지 PR들은 일단 리베이스를 해볼까요? 그리고 봐야하는 순서를 알려주시면, 좋을 것 같아요. 사실 PR을 독립적으로 작성하는 것이 좋은데, 작업에 따라서 그럴 수 없는 상황이 있긴하죠. 그럴 땐, 순서를 알려주시면 좋을 것 같습니다. :-) 다른 PR열었는데, 변경 사항이 133개 파일이라 .... 깜짝 놀랐습니다. ㅎㅎ ㅜㅠ 그리고 제가 Approve했으면, 추가로 고치셨더라도 그냥 머지 하셔도 됩니다. :-) 아니면, 일단 이건 머지하고, 수정 사항은 다른 PR로 올려도 좋을 것 같구요. 그럼 다음 작업들을 독립적으로 하는데 도움이 되지 않을까요?

@jdalma jdalma merged commit 69f0009 into main Nov 23, 2024
1 check passed
@jdalma jdalma deleted the feature/12 branch November 23, 2024 08:48
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants