refactor: data transfer format 변경 - #101
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the daily dispatch export functionality by replacing the DispatchExportDto with direct streaming of DispatchEntity and adding support for exporting daily dispatch requests. It introduces separate API endpoints for exporting dispatches and requests, streaming the compressed CSV data directly to the client. The review feedback highlights a potential file leak in the controller if response.outputStream.flush() throws an exception in the finally block, and suggests implementing proper CSV escaping for fields to prevent formatting corruption.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| try { | ||
| tempFile.inputStream().use { inputStream -> | ||
| inputStream.copyTo(response.outputStream) | ||
| } | ||
| } finally { | ||
| response.outputStream.flush() | ||
| tempFile.delete() | ||
| } |
There was a problem hiding this comment.
위험성 (High): finally 블록 내에서 response.outputStream.flush()를 호출할 때, 클라이언트가 다운로드 도중 연결을 끊는 등의 이유로 네트워크 오류가 발생하면 IOException이 발생할 수 있습니다. 이 경우 finally 블록의 실행이 중단되어 아래에 있는 tempFile.delete()가 호출되지 않고 서버에 임시 파일이 계속 누적되는 리소스 누수(File Leak)가 발생할 수 있습니다.
해결 방안: response.outputStream.flush()를 try 블록 내부로 이동시키고, finally 블록에서는 오직 tempFile.delete()만 안전하게 호출되도록 수정하는 것이 좋습니다. 이렇게 하면 flush() 도중 예외가 발생하더라도 임시 파일은 항상 안전하게 삭제됩니다.
| try { | |
| tempFile.inputStream().use { inputStream -> | |
| inputStream.copyTo(response.outputStream) | |
| } | |
| } finally { | |
| response.outputStream.flush() | |
| tempFile.delete() | |
| } | |
| try { | |
| tempFile.inputStream().use { inputStream -> | |
| inputStream.copyTo(response.outputStream) | |
| } | |
| response.outputStream.flush() | |
| } finally { | |
| tempFile.delete() | |
| } |
| dispatchJpaRepository.streamAllByCreatedAtBetween(startOfPeriod, endOfPeriod).use { stream -> | ||
| stream.forEach { dto -> | ||
| stream.forEach { entity -> | ||
| val pickupJson = "\"${entity.pickupRoutePath.toString().replace("\"", "\"\"")}\"" | ||
| val dropoffJson = "\"${entity.dropoffRoutePath.toString().replace("\"", "\"\"")}\"" | ||
| val row = buildString { | ||
| append(dto.createdAt).append(",") | ||
| append(dto.requestId).append(",") | ||
| append(dto.userId).append(",") | ||
| append(dto.startLatitude).append(",") | ||
| append(dto.startLongitude).append(",") | ||
| append(dto.endLatitude).append(",") | ||
| append(dto.endLongitude).append(",") | ||
| append(dto.status.name) | ||
| append("\n") | ||
| append(entity.dispatchId).append(",") | ||
| append(entity.requestId).append(",") | ||
| append(entity.userId).append(",") | ||
| append(entity.carId).append(",") | ||
| append(entity.carNumber ?: "").append(",") | ||
| append(entity.standId).append(",") | ||
| append(entity.createdAt).append(",") | ||
| append(entity.estimatedPickupTime).append(",") | ||
| append(entity.estimatedRideTime).append(",") | ||
| append(entity.fare).append(",") | ||
| append(entity.status.name).append(",") | ||
| append(pickupJson).append(",") | ||
| append(dropoffJson).append("\n") | ||
| } | ||
| writer.write(row) | ||
| } | ||
| } |
There was a problem hiding this comment.
개선 제안 (Medium): carNumber와 같이 쉼표(,)나 쌍따옴표(")가 포함될 수 있는 문자열 필드를 CSV에 직접 기록할 때, 적절한 이스케이프 처리가 되지 않으면 CSV 형식이 깨질 수 있습니다. 또한, JSON 문자열을 직접 replace하는 로직을 공통 헬퍼 함수로 분리하면 코드의 가독성과 안전성을 높일 수 있습니다.
해결 방안: CSV 필드를 안전하게 이스케이프할 수 있는 escapeCsv() 확장 함수를 정의하고, 이를 활용하여 데이터를 기록하도록 수정하는 것을 권장합니다.
dispatchJpaRepository.streamAllByCreatedAtBetween(startOfPeriod, endOfPeriod).use { stream ->
stream.forEach { entity ->
val pickupJson = entity.pickupRoutePath.toString().escapeCsv()
val dropoffJson = entity.dropoffRoutePath.toString().escapeCsv()
val row = buildString {
append(entity.dispatchId).append(",")
append(entity.requestId).append(",")
append(entity.userId).append(",")
append(entity.carId).append(",")
append(entity.carNumber.escapeCsv()).append(",")
append(entity.standId).append(",")
append(entity.createdAt).append(",")
append(entity.estimatedPickupTime).append(",")
append(entity.estimatedRideTime).append(",")
append(entity.fare).append(",")
append(entity.status.name).append(",")
append(pickupJson).append(",")
append(dropoffJson).append("\n")
}
writer.write(row)
}
}
Summary
dispatch-service에서 데이터를 미리 가공하여 내보내던 로직을 제거하고, 원본 테이블 데이터 그대로 추출하도록 변경