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/user redis test #20

Merged
merged 6 commits into from
Jun 4, 2024
Merged

Feat/user redis test #20

merged 6 commits into from
Jun 4, 2024

Conversation

david-parkk
Copy link
Member

@david-parkk david-parkk commented May 31, 2024

✏️ 작업 개요

redis crud 관련 통합 테스트와 dto를 추가

⛳ 작업 분류

  • 통합테스트
  • dto 추가
  • 일부 함수 주석 추가

🔨 작업 상세 내용

  1. 통합데스트를 통해 GET ,POST 를 검증했습니다.(POST에 문제가 있습니다.) 💣 webflux test #3
  2. dto를 추가하였습니다.
  3. javadoc 주석 추가(생각보다 다른 사람 코드를 보기 쉽지 않다는 것을 느꼈음)
  4. test 환경관련해서 yml 파일로 local(로컬)와 prod(배포) 를 분리하였습니다.
  5. CI 환경은 local입니다.
  6. 배포관련 build 설정을 안해놔서 별도의 처리 필요

💡 생각해볼 문제

  • 주석은 생각 날때마다 달 예정입니다.
  • dto는 service에서 변환합니다.
  • dto를 사용하게 되면서 인터페이스와 dto과 종속성을 지니게 되는데 이야기해봐야할 것 같습니다.

Copy link

github-actions bot commented May 31, 2024

Test Results

12 tests   12 ✅  1s ⏱️
 4 suites   0 💤
 4 files     0 ❌

Results for commit 1e8a011.

♻️ This comment has been updated with latest results.

@david-parkk
Copy link
Member Author

david-parkk commented May 31, 2024

  • 통합테스트 관련 redis connect 필요 😨
  • 해결되었습니다(local redis에 연결)

@kamothi
Copy link
Member

kamothi commented Jun 4, 2024

잘 작성해주신거 같습니다. 이번 작업은 기존 엔티티 반환들을 dto로 반환하는 작업과 테스트 코드 작성인 것으로 확인하였습니다.
생각해볼 문제에 dto를 어느 레이어에서 변환할지에 대한 것인데... 저는 service에서 변환 작업을 수행한다면 그것은 service 레이어의 비즈니스 로직 수행뿐만 아닌 변환이라는 작업에 대한 추가적인 역할이 부여되는 것이 아닌가라는 생각이드네요. 제 생각은 service 레이어는 비즈니스 로직을 수행하는데만 집중을 해야한다고 생각하는데 혹시 어떻게 생각하시나요?

@david-parkk
Copy link
Member Author

david-parkk commented Jun 4, 2024

Member

service layer에서 dto 변환이 일어날 경우 문제가 service에서 너무 많은 처리를 한다는 것 입니다. 수많은 dto가 있다고 생각하면 service 클래스가 더러워질 수 있습니다.
controller layer에서 dto 변환이 일어날 경우 controller의 역활을 벗어난 행위를 하게됩니다.
제가 개발하면서 느낀 것은 dto 변환이 비즈니스 로직의 일부가 된다는 것 입니다.(데이터를 가공할 경우)
그리고 service에서 dto를 변환하게 되면 service 인터페이스도 모두 반환 type이 dto가 되야하기 때문에 dto에 의존적이라고 생각이 들었습니다.(실제로 변경함) 코드의 재사용성이 줄어 들기도 합니다.
그러나 비즈니스 로직을 실행하는 것을 목표로 한다면 service 계층에서 반환하는 것이 옳바르다고 생각합니다.(재사용성이 높지만 강한 결합력)
실제로 service에서 repository를 호출하는 경우만 있는 경우도 많아서 service에서 dto를 변환한다고 생각하고 진행했습니다...

@david-parkk
Copy link
Member Author

이건 정답이 없는 문제라 정해야 할 것 같네요

@kamothi
Copy link
Member

kamothi commented Jun 4, 2024

확실히 정답이 없는 문제인거 같습니다. 저는 재사용성 측면에서 바라봤을 때 결국 dto가 비즈니스 로직에 묶여 있다면 장기적으로 봤을 때 프로젝트가 커졌을 경우 재사용성으로 인한 문제가 발생하지 않을까 생각이드네요. 당장의 구현에 있어서는 이런 부분이 발생하지 않을 것으로 보입니다.

public Mono<User> addUser(@RequestBody User user) {
return userService.create(user);
}
@PostMapping(produces= MediaType.APPLICATION_JSON_VALUE)
Copy link
Member

Choose a reason for hiding this comment

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

이렇게 value 값을 지정하면 어떤 이점이 있나요? 저 형태가 아니면 예외를 발생시키나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

불필요한 부분 같습니다 제거 했습니다.

Copy link
Member

@kamothi kamothi left a comment

Choose a reason for hiding this comment

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

머지하겠습니다.

@kamothi kamothi merged commit 422fc2c into main Jun 4, 2024
3 checks passed
@kamothi kamothi deleted the feat/user-redis-test branch June 4, 2024 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants