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

사용자가 식품에 대한 리뷰를 작성한다. #74

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

Conversation

koo995
Copy link
Collaborator

@koo995 koo995 commented Oct 10, 2024

About

  • 식품에 대한 리뷰를 작성합니다.
  • 리뷰를 작성하며 식품이 포함된 Store 정보와 DietTag 정보를 기록합니다.
  • aws S3 을 이용하여 ncp object storage 에 파일을 업로드합니다.

Problem

  • AWS관련 환경변수를 현재 IntelliJ 에서 설정하는 방법을 택했습니다. 이 방법으로 애플리케이션 실행 시 제대로 동작하나 테스트 코드에서 특정 객체가 환경변수의 값을 이용한 초기화 과정에서 빈이 제대로 생성되지 않는 문제가 있습니다.

close #72

@koo995 koo995 added the dev label Oct 10, 2024
@koo995 koo995 self-assigned this Oct 10, 2024
Comment on lines 15 to 25
@Value("${cloud.aws.s3.endpoint}")
private String endPoint;

@Value("${cloud.aws.s3.regionName}")
private String regionName;

@Value("${cloud.aws.credentials.accessKey}")
private String accessKey;

@Value("${cloud.aws.credentials.secretKey}")
private String secretKey;
Copy link
Collaborator Author

@koo995 koo995 Oct 10, 2024

Choose a reason for hiding this comment

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

여기서 @value의 값을 yml 파일에 설정한 값을 읽어오는데, 테스트 코드를 실행할 때 IntelliJ 에서 환경변수로 주입해 주지않아 S3Config 빈을 초기화하는데 에러가 발생합니다.

jenkins 서버에서 빌드할때도 같은 문제가 발생할 것 같은데 이러한 환경변수는 어떻게 관리하는게 좋을까요?
제일 간단하게 드는 방법은 젠킨스 서버에 접속해서 각각 환경변수를 설정해두고 AutoScaling 템플릿에도 설정하면 되는데, 이런 경우 새로운 환경변수가 필요할때마다 각 서버에 접속해서 변수를 설정해줘야 하는 문제가 있는것 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

건홍님 accessKey와 secretKey는 사용하지 마세요. role 기반으로 설정하시고, 테스트 환경에서는 s3를 직접 다루기 보단 다른 테스트 구현체를 띄우거나 docker를 활용하는 방법이 있습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

도커를 활용한 테스트 컨테이너로 적용했습니다!

Comment on lines 15 to 25
@Value("${cloud.aws.s3.endpoint}")
private String endPoint;

@Value("${cloud.aws.s3.regionName}")
private String regionName;

@Value("${cloud.aws.credentials.accessKey}")
private String accessKey;

@Value("${cloud.aws.credentials.secretKey}")
private String secretKey;
Copy link
Collaborator

Choose a reason for hiding this comment

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

건홍님 accessKey와 secretKey는 사용하지 마세요. role 기반으로 설정하시고, 테스트 환경에서는 s3를 직접 다루기 보단 다른 테스트 구현체를 띄우거나 docker를 활용하는 방법이 있습니다.


@RequiredArgsConstructor
@Service
public class FileStoreService {
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
Collaborator Author

Choose a reason for hiding this comment

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

인터페이스로 추상화할까 하다 코드에 Profile 설정이 많이 붙어있는게 보기 안좋아서 테스트할 때 이 부분을 Mocking 처리했습니다. 이런 방법도 괜찮나요?

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
Collaborator Author

@koo995 koo995 Oct 13, 2024

Choose a reason for hiding this comment

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

그렇다면 S3Config, FileStoreServiceImpl 클래스에 @Profile({“prod”, “dev”}) 로 하고 테스트는 mock객체로 처리했습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

mock을 사용할때 많은 단점들이 있는데요. 단점들을 한번 찾아보시죠.
mock을 사용하기 보단 Fake 객체를 구현해서 대체해보세요.
(테스트 코드에서 사용되는 mock,stub,fake,spy에 대해 찾아보고 정리해보세요)

return CreateReviewResponse.of(savedReview.getId());
}

private ProductStore saveProductStore(CreateReviewRequest request) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

간단한 변환로직은 request 객체 안에 toStore로 표현해보는건 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

비즈니스 로직이 아니니 그 방법이 더 간단하고 좋아보이네요! 수정했습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants