Feature/hodadako step2 #10
Conversation
- EmailRecordRequest 내부에 있던 Save DTO를 단독 레코드로 변경
- 동시에 모든 데이터를 메모리에 로딩하지 않도록 ``@Query``와 ``Stream``을 사용하였음
- DTO를 조회할 시 fetch join 사용 불가하므로 join을 사용하도록 변경
- 개발을 위해 ddl-auto를 update로 두어 스키마를 자동으로 업데이트 하게 두었음
- 날짜에 시간이 필요 없는걸 고려하여 LocalDate로 변경
- 통계 집계에 이전까지의 누적금액 합산을 위해 조회 기능 추가
- 새벽 4시 마다 통계를 집계할 수 있도록 스케쥴된 통계 집계 함수 추가 - 특정 날짜 강제로 집계할 수 있는 기능 추가 - 통계 집계 시 통계 생성 로직을 TransferStatistics 에서 하도록 구현 - DTO 내부에 유효성 검증 추가 - 빈 값이거나 현재 혹은 과거에 해당 하는 날짜인지 검증, 거래 특성상 미래에 대한 기록이 존재하지 않을 것이라 생각하여 추가
- 시작 날짜가 종료 날짜를 넘지 않도록 검증 추가 - 오늘 이전의 날짜만 입력 값으로 받을 수 있도록 검증 추가
- Get 대신 Aggregate이라는 이름으로 바꾸어 집계라는 의미를 추가
- 해당 통계를 서버 실행시에 저장할 것
- 맞지 않는 컬럼들 삭제 및 수정
- 맞지 않는 컬럼들 삭제 및 수정
| today.toLocalDate()); | ||
| var transferStatistics = transferStatisticsReader.findByUnitDate(yesterday.toLocalDate()); | ||
| Long dailyTotal = allTransfersByDate.map(TransferStatistics::getDailyTotalAmount).reduce(0L, Long::sum); | ||
| Long cumulativeTotal = transferStatistics.getCumulativeTotalAmount() + allTransfersByDate.map( |
There was a problem hiding this comment.
현재 로직은 어떻게든 이전 데이터가 존재해야 끼워넣을 수 있는 구조로 보이는데요.
- 처음 해당 통계 도입 시, 기본 데이터를 강제로 삽입해서 동작할 계획이실까요?
- 징검다리 연휴에 순간적인 네트워크 에러가 발생하여 데이터 삽입이 실패했다면, 그 기간동안 쭉 에러가 발생하지 않을까요?
| ORDER BY ts.unitDate DESC | ||
| """) | ||
| @QueryHints(value = { | ||
| @QueryHint(name = HINT_FETCH_SIZE, value = "" + Integer.MAX_VALUE), |
There was a problem hiding this comment.
String.valueOf(Integer.MAX_VALUE) 가 좀 더 자연스럽지 않을까요?
|
|
||
| public interface TransactionRepository extends JpaRepository<Transaction, Long> { | ||
| @Query( | ||
| """ |
There was a problem hiding this comment.
혹시 해당 쿼리의 실행계획을 확인하셨을까요?
(해당 테이블의 데이터가 많은 것 같지만, 실제로는 그렇게 많은 것도 아닙니다.)
| import jakarta.validation.constraints.PastOrPresent; | ||
|
|
||
| public record AggregateTransferStatisticsRequest( | ||
| @NotBlank |
There was a problem hiding this comment.
@NotNull 을 원하신건가요?
@NotBlank 는 문자열에서 사용되는 어노테이션입니다.
|
|
||
| @PostMapping("/v1/transfer-statistics") | ||
| public ResponseEntity<TransferStatistics> aggregateTransferStatistics( | ||
| @RequestBody AggregateTransferStatisticsRequest request |
There was a problem hiding this comment.
Request 객체에 Validation 어노테이션을 박았지만, 막상 여기서 @Valid를 안 씌우셔서 검증이 안 이뤄질 것 같네요.
| @@ -0,0 +1,40 @@ | |||
| package org.c4marathon.assignment.domain.transfer_statistics.presentation; | |||
There was a problem hiding this comment.
일반적으로 패키지명에 underscore를 삽입하는 것은 매우 권장되지 않습니다.
(ex1. https://google.github.io/styleguide/javaguide.html#s5.2.1-package-names)
(ex2. https://naver.github.io/hackday-conventions-java/#package-lowercase)
| private LocalDateTime transactionDate; | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { |
There was a problem hiding this comment.
전반적으로 equals 와 hashCode 를 정의하고 있는데요. 혹시 해당 데이터가 HashMap 에 들어갈 것을 상정하셔서 그런걸까요?
| @@ -15,6 +15,9 @@ spring: | |||
| mail.smtp.ssl.enable: true | |||
| mail.smtp.ssl.trust: ${SMTP_HOST} | |||
| mail.smtp.starttls.enable: true | |||
| jpa: | |||
| hibernate: | |||
| ddl-auto: update | |||
There was a problem hiding this comment.
데이터가 들어있는 테이블에선, 절대 create/create-drop/update 를 사용하지 말아주세요. (지금도 당장 수천만개의 데이터가 들어있는 transaction 테이블을 바라보고 있기 때문에, 다소 위험한 상황이 발생할 뻔 했네요.)
|
|
||
| @Transactional(readOnly = true) | ||
| public List<TransferStatistics> findAllByUnitDateBetween(GetAllTransferStatisticsRequest request) { | ||
| return transferStatisticsReader.findAllByUnitDateBetween(request.startDate(), request.endDate()) |
There was a problem hiding this comment.
처음부터 Stream 이 아닌 List를 반환하도록 하면 되지 않나요?
| @Column(name = "unit_date", columnDefinition = "datetime") | ||
| private LocalDateTime unitDate; | ||
|
|
||
| public TransferStatistics(Long dailyTotalAmount, Long cumulativeTotalAmount, LocalDateTime unitDate) { |
There was a problem hiding this comment.
@NoArgConstructor는 Lombok 으로 작성하셨는데, 정작 이건 Lombok을 안 쓰셨네요.
스타일은 통일해 주세요.
구현사항
구현내용
수정사항