Feature/#253 학습자료 파일명 및 다운로드 기능 구현 #259
Conversation
yessjun
changed the title
There was a problem hiding this comment.
수고하셨습니다~!! 간단한 리뷰 남겼으니 확인 부탁드릴게요!
우선 전체적으로 final이 사용안된 것 같습니다!
final 사용 이유
final 사용 refactoring pr (그런데 test는 실제 성능과는 관련이 없어서 필수 체크처럼은 하고 있지 않습니당..)
이 규칙에 따라 사용하고 있으니 수정 부탁드립니다!
그리고 포맷팅이 안되어 있는 것 같습니다. 저희가 사용하는 포매터는 프로젝트(intelliJ-java-~~.xml)에 넣어두었으니 적용해서 코드에 적용해주세용!
| @Validated | ||
| @RestController | ||
| @RequiredArgsConstructor | ||
| @RequestMapping |
There was a problem hiding this comment.
Validated annotation은 현재 코드 상 사용되고 있지 않은 것 같아 빼도 될 것 같습니다!(혹시 아니라면 알려주세용!)
그리고 RequestMapping을 사용하셨으니 @RequestMapping("/files") 를 사용해서 중복되는 url 코드를 줄일 수 있을 것 같습니다!
| @GetMapping("/files/images/{uuid}") | ||
| public ResponseEntity<Void> imageFile(@PathVariable String uuid) { |
There was a problem hiding this comment.
추가로 controller에도 수행하는 행위(동사) + 명사 이렇게 네이밍하면 어떤 기능을 수행하는 지 파악하기 쉬울 것 같아요!
ex) createTeam, updateTeam, deleteTeam, getTeam 등등
| return ResponseEntity.status(HttpStatus.MOVED_PERMANENTLY) | ||
| .header("Location", imageUrl) | ||
| .build(); |
There was a problem hiding this comment.
이런 HttpStatus도 있군요! 배워갑니다!!👍
| return ResponseEntity.status(HttpStatus.FOUND) | ||
| .header("Location", presignedUrl) | ||
| .build(); |
There was a problem hiding this comment.
앞의 HttpStatus.MOVED_PERMANENTLY 와 다른 점이 무엇인가요??
| @DisplayName("[성공] 이미지 파일의 URL을 조회한다.") | ||
| public void 이미지_파일_URL_조회() throws Exception { |
There was a problem hiding this comment.
추가로 DisplayName과 MethodName은 동일하면 좋을 것 같아요!
ex) 이미지_파일의_URL을_조회한다_성공()
#️⃣ 연관된 이슈
📝 작업 내용
💬 리뷰 요구사항(선택)