[Feature] 베스트 셀러 Redis 조회 기능 및 스케쥴링 구현 #55
Conversation
There was a problem hiding this comment.
정우님 코드 잘봤습니다!
제가 남긴 리뷰 한 번 보시고 답 주시면 감사하겠습니다!
그리고 정우님이 남기신 질문
TestContainers를 사용하면서 까지 Redis를 테스트하는 것이 맞을지 아니면 @MockBean으로 처리하여 테스트하는 것이 좋을지 고민하고 있습니다.
이 부분 같은 경우에는 Redis 테스트 시 파이프라인에서 통과하지 못할까봐 @MockBean 처리를 하는것이 좋다고 생각이 들었었는데, 정우님이 잘 해결하신거같아, 굳이 @MockBean 테스트로 진행하지 않으셔도 될거같습니다!
| BestSellerResponse.of(key, book.getIsbn(), book.getTitle(), book.getPublisher(), | ||
| book.getThumbnail(), book.getAuthors(), book.getDateTime())); | ||
| } catch (JsonProcessingException e) { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
런타임 Exception은 보다는 무슨 Exception인지 명시 해주는게 좋을거 같습니다!! 정우님!
There was a problem hiding this comment.
앗! 까먹고 수정하지 못했네요..
수정하겠습니다!!
| @SpringBootTest | ||
| @RecordApplicationEvents | ||
| @Slf4j |
There was a problem hiding this comment.
여기에다가 @SpringBootTest를 다시는거보다 RedisTestContainer 쪽에다 다시는게 좋지않을까요 ? 나중에 TalkRoom에도 Redis를 적용시켜야 할 수도 있는데 서버가 두개 켜지는거보다 한 번 켜지는 편이 더 좋을거같아서 얘기드립니다!
There was a problem hiding this comment.
해당 컨테이너는 Redis 전용 테스트 컨테이너 입니다. 추후에 Kafka나 PostgreSql이 추가될 수도 있습니다. 이런 경우 나머지 테스트 컨테이너에도 @SpringBootTest를 붙여야 합니다.
나중에 상속으로 하지 않는 방식으로 구현하여 ServiceTestSupport에서 해결할 수 있도록 해보겠습니다!
| @SpringBootApplication | ||
| @EnableCaching | ||
| @EnableScheduling | ||
| @EnableRetry | ||
| @ConfigurationPropertiesScan | ||
| public class JisunginApplication { |
There was a problem hiding this comment.
애플리케이션 실행 시 캐싱, 스케줄링, 리트라이가 적용되는 건가요 ?
There was a problem hiding this comment.
네 @Enablexxx 어노테이션들은 관련된 라이브러리의 기능을 활성화하는 역할 입니다!
| redisTemplate.setKeySerializer(new StringRedisSerializer()); | ||
| redisTemplate.setValueSerializer(new StringRedisSerializer()); | ||
|
|
||
| redisTemplate.setHashKeySerializer(new StringRedisSerializer()); | ||
| redisTemplate.setHashValueSerializer(new StringRedisSerializer()); |
There was a problem hiding this comment.
Key, Value가 적용됐는데 HashKey, HashValue도 적용한 이유가 궁금합니다 !
There was a problem hiding this comment.
setKeySerializer()과 setValueSerializer()는 일반적인 키와 값을 저장하는 RedisTemplate의 직렬화 방식을 지정하는 데 사용됩니다.
redis에서의 get과 set의 명령어와 같은 기능을 사용할 때 String 타입으로 직렬화 한다는 의미입니다.
setHashKeySerializer()과 setValueSerializer()는 Redis 해시에 대해 사용됩니다. 저희는 베스트 셀러에 대한 값을 Hash로 저장하기 때문에 해당 설정도 추가적으로 적용하였습니다!
| @@ -24,7 +24,7 @@ public CrawlingBook crawlBook(String isbn) { | |||
| @Override | |||
| public Map<Long, CrawlingBook> crawlBestSellerBook() { | |||
| Map<Long, String> bestSellerBookIds = parser.parseBestSellerBookId(fetcher.fetchBestSellerBookId()); | |||
| Map<Long, CrawlingBook> bestSellerBooks = new HashMap<>(); | |||
| Map<Long, CrawlingBook> bestSellerBooks = new ConcurrentHashMap<>(); | |||
There was a problem hiding this comment.
thread-safe를 보장하기 위해 ConcurrentHashMap을 쓰신 건가요 ?
There was a problem hiding this comment.
네 HashMap으로 구현하였더니 테스트 단계에서 문제가 발생하여 Thread-safe한 ConcurrentHashMap으로 변경하였습니다!
| @Override | ||
| @Retryable(retryFor = BusinessException.class, maxAttempts = 5, backoff = @Backoff(delay = 1000)) |
There was a problem hiding this comment.
재시도 횟수가 최대 5회, 각 시도에서 1초만 동작하는 건가요 ?
만약 5회 실패하면 BusinessException을 날리나요 ?
There was a problem hiding this comment.
해당 메서드에서 BusinessException이 발생하면 5번 재시도 하며 재시도 간의 딜레이는 1초로 설정한 코드입니다!
jwooo
force-pushed
the
feature/45-best-seller-redis-and-scheduler
branch
from
563c6a9
to
2c6f463
Compare
💡 연관된 이슈
close #45
📝 작업 내용
💬 리뷰 요구 사항
Redis를 추가하면서 테스트에서Redis를 사용할 수 없는 문제가 발생하여TestContainers를 사용하였습니다.TestContainers를 사용하면서 까지Redis를 테스트하는 것이 맞을지 아니면@MockBean으로 처리하여 테스트하는 것이 좋을지 고민하고 있습니다.어떻게 하는게 좋은 방법인지 몰라 좋은 생각 있으시면 알려주시면 감사하겠습니다 !