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

9조 BE 코드리뷰 5회차 #96

Merged
merged 39 commits into from
Nov 10, 2024
Merged

9조 BE 코드리뷰 5회차 #96

merged 39 commits into from
Nov 10, 2024

Conversation

sim-mer
Copy link
Contributor

@sim-mer sim-mer commented Nov 9, 2024

PR

✨ 작업 내용

  • 리팩토링을 통해 Api response를 통합
  • 리뷰 관련 로직 작성
  • 작가 검색 기능에 팔로우 여부 확인 로직 추가
  • Redis를 사용한 인기 검색어 조회 기능 작성

✨ 궁금한 점

  • (심규민) 현재 스케줄링 작업으로 인기 검색어 순위를 업데이트 하고 있는데 스케줄링 작업의 테스트는 어떻게 이루어지는지 궁금합니다

yooonwodyd and others added 30 commits November 4, 2024 00:37
임시로 랜덤한 페이징을 반환하는 api 구현
[yooonwodyd-> weekly] 상품 임시 피드 추가 #79
로그 일부 추가, properties 추가, 비로그인 유저 접속 가능 url 추가
유저 이미지 url 추가
baseentity 추가 및 dto 변경
불필요한 의존성 정리
[jupyter471->weekly] ApiResponse
반환타입 변경
data.sql 추가
[yooonwodyd-> weekly] 유저 리팩토링 #84
data.sql 추가
아티스트 API 스펙 변경에 따른 세부사항 수정
data.sql 추가
@sim-mer sim-mer requested a review from leaf-upper November 9, 2024 02:14

@Bean
public RedisConnectionFactory redisConnectionFactory() {
return new LettuceConnectionFactory(host, port);

Choose a reason for hiding this comment

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

Lettuce도 좋지만, Redission을 사용하시면 분산락 기능도 사용하실수 있어요~
분산락 기능이 필요없으시다면 Lettuce로도 충분한것 같습니다. 다만 분산락은 애플리케이션에서 많이 활용되는 측면이 있어서
나중에 한번 공부해보시면 좋을것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 한번 공부해보겠습니다!

@@ -25,4 +24,11 @@ public class Like {
@ManyToOne
@JoinColumn(name = "product_id")
private Product product;

protected Like(){};

Choose a reason for hiding this comment

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

👍

private final ProductRepository productRepository;

@Transactional
public void productLike(Long userId, Long productId) {

Choose a reason for hiding this comment

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

like를 동시에 두번 요청하면 어떻게 될까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

아! 이미 좋아요 되었을 때 예외처리가 안되어 있네요..!! 감사합니다!! 관련 처리 반영하겠습니다

}

@Transactional
@GetMapping

Choose a reason for hiding this comment

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

요 GetMapping은 잘못들어간걸까요? 아니면 의도하고 넣으신걸까요?

private final double WEEK_WEIGHT = 0.04;
private final double TOTAL_WEIGHT = 0.12;

@Scheduled(cron = "0 0 * * * *")
Copy link

@leaf-upper leaf-upper Nov 10, 2024

Choose a reason for hiding this comment

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

스케줄링 기능을 넣으셨군요 고생하셨습니다 👍
혹시 서버 애플리케이션을 2대 이상 띄워야할때 어떻게 동작할까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

찾아보니 ShedLock 라이브러리로 Lock을 통해 한 서버만 스케줄링 작업을 수행하도록 하는 방법이 있는 것 같습니다. 이외에도 스케줄링만을 위한 서버를 분리하는 방법도 고려해 볼 수 있을 것 같습니다.

Copy link

@leaf-upper leaf-upper left a comment

Choose a reason for hiding this comment

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

👍 👍 마지막 주차까지 고생많으셨습니다~
몇가지 코멘트를 남겨드리긴했는데 나중에 한번씩 고민해봐주시면 좋을것 같습니다.
마지막까지 프로젝트 진행하시느라 고생많으셨고, 마무리까지 잘 진행되시길 바라겠습니다
👍 👍

@leaf-upper
Copy link

leaf-upper commented Nov 10, 2024

@sim-mer
(심규민) 현재 스케줄링 작업으로 인기 검색어 순위를 업데이트 하고 있는데 스케줄링 작업의 테스트는 어떻게 이루어지는지 궁금합니다

  • 결국 이런 기능의 테스트는 Unit Test 혹은 Integration Test, 코드 단위로 테스트하기 어렵습니다. 결국 코드만으로 하는 테스트로는 어느순간 한계가 있는것 같아요. 제가 생각할때 스케줄링 작업의 가장 좋은 테스트는, 실제환경과 똑같은 테스트환경을 구축하고 해당 테스트 환경에서 스케줄링 작업을 진행해서 제대로 데이터가 쌓이는지 눈으로 확인하는거예요. 이거는 E2E 테스트 스크립트를 짜시거나, 테스트 환경에서 직접 스케줄링을 돌리면서 확인해보실수가 있을것 같아요.

결국 테스트는 아래와 같이 3종류의 테스트가 필요하다고 개인적으로 생각하는데요,

  • 특정 한가지 기능을 테스트하기위한 단위테스트
  • 기능들을 묶어서 API가 잘동작하는지 테스트하기 위한 통합 테스트 혹은 E2E Test
  • 실 환경에서 복합적으로 동작하는 애플리케이션이 잘 동작하는지 테스트하기 위해 테스트 환경을 운영하고, 해당 환경에서 소프트웨어의 QA를 진행

스케줄링 테스트는 3번과 같은 테스트에서 조금더 쉽게 테스트할수있지않을까 생각이듭니다.

@leaf-upper leaf-upper merged commit ddf1418 into review Nov 10, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants