[#33] 경매 활성화 및 종료 상태 변경 코드 추가 #34
Conversation
| @@ -44,7 +45,9 @@ public ProductResponse enroll( | |||
|
|
|||
| @GetMapping("/{id}") | |||
| public ProductFindResponse findById(@PathVariable Long id) { | |||
| return productFacade.findById(id); | |||
| LocalDateTime request = LocalDateTime.now(); | |||
There was a problem hiding this comment.
request라는 이름이 좀 어색한 것 같아요 ~ requestTime 이나 좀 더 명시적으로 쓰일 이름으로 바꿔주면 좋을 것 같아요
| @@ -44,7 +45,9 @@ public ProductResponse enroll( | |||
|
|
|||
| @GetMapping("/{id}") | |||
| public ProductFindResponse findById(@PathVariable Long id) { | |||
| return productFacade.findById(id); | |||
| LocalDateTime request = LocalDateTime.now(); | |||
| ProductFindResponse productFindResponse = productFacade.findById(id, request); | |||
There was a problem hiding this comment.
이런건 인라인으로 리턴해도 되지 않을까요 ?
| @@ -69,4 +69,8 @@ public void updateProductInfo(String productName, String description, LocalDateT | |||
| this.startPrice = startPrice; | |||
| } | |||
|
|
|||
| public void updateProductCondition(ProductCondition productCondition) { | |||
There was a problem hiding this comment.
엔티티 변경시에는 누가 변경했는지도 같이 받아서 바꿔주면 좋을 것 같아요!
| Product product = productRepository.findById(id) | ||
| .orElseThrow(() -> new NotFoundProductException(id)); | ||
|
|
||
| if (requestTime.isAfter(product.getStartDate()) && requestTime.isBefore(product.getCloseDate())) { | ||
| product.updateProductCondition(ProductCondition.ACTIVE); |
There was a problem hiding this comment.
아 요렇게 쓰이는거군요 ? 이거 상태를 엔티티가 직접 가지고 있을 이유가 있을까요 ? 이러면 조회 요청이 트리거가 돼서 상태가 변경되는건데 상태를 가지는게 아니라 응답에만 포함되어도 될 것 같아요 ~ 추가로, 상태값 관리하는건 별도의 배치나 스케줄러가 있어야 하지 않을까 싶은데 어떻게 생각하실까요 ?
There was a problem hiding this comment.
음 저도 상태쪽을 솔직히 requestTime을 받아서 필터링하는건 별로라고 생각하고있었는데 피드백 감사합니다!
저도 상태값 관리는 배치나 스케줄러를 사용해도 좋을 것 같습니다!
| public class ProductConditionUpdate implements ItemProcessor<Product, Product> { | ||
| // 상품의 상태를 갱신하는 로직 | ||
| @Override | ||
| public Product process(Product product) throws Exception { |
There was a problem hiding this comment.
요거 그냥 Writer에서 하고 Processor 빼도 될거같아요 ?
|
|
||
| @Component | ||
| public class ProductReader implements ItemReader<List<Product>> { | ||
| private static final int PAGE_SIZE = 500; // 한번에 읽을 데이터 개수 (청크 사이즈) |
There was a problem hiding this comment.
요거 CHUNK_SIZE를 같이 사용하는게 어떨까요 ?
|
|
||
| @Override | ||
| public List<Product> read() throws | ||
| Exception, |
There was a problem hiding this comment.
예외 목록이 약간 읽기 어렵게 줄바꿈이 된 것 같아요
There was a problem hiding this comment.
요게 저두 불편한데,, 줄바꿈을 제가 바꿔두 save 하면 저런식으로 줄바꿈이 되더라구용,,,🥲
| List<Product> products = productRepository.findAll(pageable).getContent(); | ||
|
|
||
| if (products.isEmpty()) { | ||
| currentPage = 0; |
There was a problem hiding this comment.
엇 이게 DB에서 더이상 읽어올게없으면 다시 처음부터 읽을 수 있도록 pageNo를 0으로 하는 코드였습니다!
| UnexpectedInputException, | ||
| ParseException, | ||
| NonTransientResourceException { | ||
| Pageable pageable = PageRequest.of(currentPage, PAGE_SIZE); |
There was a problem hiding this comment.
전체를 다 읽는거라 데이터가 많아지면 점점 느려질 것 같아요, more 방식으로 읽도록 변경하는게 어떨까요 ?
There was a problem hiding this comment.
아 그렇네요!! more 방식으로 수정하도록 하겠습니다!
|
| @@ -17,26 +17,24 @@ | |||
| @Configuration | |||
| @EnableBatchProcessing | |||
| public class BatchConfig { | |||
|
|
|||
| public static final int CHUNK_SIZE = 500; | |||
| @Value("${ready.auction.spring.batch.chunkSize}") | |||
There was a problem hiding this comment.
요거 근데 아마 잡마다 청크사이즈가 달라서 이렇게 퉁치긴 좀 어렵긴 할텐데 나중에 기회되면 바꿔보시죠 ~
저는 JobConfig가 최상단이라 거기 있는 애들 public static으로 같이 쓰고 있어요
There was a problem hiding this comment.
아하 넵 ! 저도 해보다가 애매하면 JobConfig에 사이즈를 두고 사용하도록 하겠습니다
| @@ -21,6 +23,17 @@ public ProductConditionWriter(ProductRepository productRepository) { | |||
| @Override | |||
| public void write(Chunk<? extends Product> chunk) throws Exception { | |||
| List<? extends Product> products = chunk.getItems(); | |||
| LocalDateTime now = LocalDateTime.now(); | |||
| // 경매 상태 업데이트 | |||
There was a problem hiding this comment.
요 주석들 굳이 없어도 읽는데 문제 없을 것 같아요 ~
주석은 최대한 일반적이지 않은 코드를 위한 히스토리 혹은 참고할 문서가 있는 경우에만 남겨주세요
경매 활성화 및 종료 상태 변경 코드 추가