Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: S3 파일 업로드 베이스 코드 구현 #24

Merged
merged 4 commits into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ public class GlobalExceptionHandler {

@ExceptionHandler(BusinessException.class)
public ResponseEntity<APIResponse<?>> handleBusinessException(BusinessException e) {
e.printStackTrace();
return ResponseEntity
.status(e.getErrorCode().getStatus())
.body(APIResponse.fail(e.getErrorCode()));
kdh10806 marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ public static <T> APIResponse<T> fail(ErrorCode code, String message) {
}

//생성
// public static <T> APIResponse<T> create(T data) {
// return new APIResponse<>(data);
// }
public static <T> APIResponse<T> create(T data) {
return new APIResponse<>(SuccessCode.CREATED.getStatusCode(), SuccessCode.CREATED.getMessage(), data);
}

public static <T> APIResponse<T> create(int code, String message, T data) {
return new APIResponse<>(code, message, data);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.seveneleven.devlens.global.util.file.Service;

import com.seveneleven.devlens.global.exception.BusinessException;
import com.seveneleven.devlens.global.response.APIResponse;
import com.seveneleven.devlens.global.response.ErrorCode;
import com.seveneleven.devlens.global.util.file.FileValidator;
import com.seveneleven.devlens.global.util.file.dto.FileMetadataDto;
Expand Down Expand Up @@ -28,7 +29,7 @@ public class FileService {
* @return FileMetadataDto 업로드한 파일 메타데이터
*/
@Transactional
public FileMetadataDto uploadFile(MultipartFile file, Long uploaderId, String fileCategory, Long referenceId) throws Exception {
public APIResponse uploadFile(MultipartFile file, Long uploaderId, String fileCategory, Long referenceId) throws Exception {
//1. 파일 검증
FileValidator.validateFile(file, fileCategory);

Expand Down Expand Up @@ -67,7 +68,7 @@ public FileMetadataDto uploadFile(MultipartFile file, Long uploaderId, String fi
FileMetadata savedMetadata = fileMetadataRepository.save(metadata);

//DTO로 변환 후 반환
return FileMetadataDto.toDto(savedMetadata);
return APIResponse.create(FileMetadataDto.toDto(savedMetadata));

} catch (Exception e){
//저장 실패시 S3에서 삭제
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,13 @@ public String generateUniqueFileName(String originalFileName) {
* 키 = S3 저장시 파일 경로
*/
public String generateS3Key(Long uploaderId, String category, String fileName) {
return String.format("%s/%s/%s", uploaderId, category, fileName);
return new StringBuilder()
.append(uploaderId)
.append("/")
.append(category)
.append("/")
.append(fileName)
.toString();
Comment on lines +39 to +45
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 코드를 잘 못 이해한 것 같습니다. 지금 상황이라면 StringJoiner를 사용하는게 더 효과적인것 같습니다.

지금 상황으로도 딱히 문제는 없으므로 참고만 해주시면 좋을 것 같아요.

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ public class FileController {
@ApiResponse(responseCode = "400", description = "Invalid file upload request")
}
)
public ResponseEntity<FileMetadataDto> uploadFile(@RequestParam("file") @Schema(type = "string", format = "binary", description = "File to upload") MultipartFile file) throws Exception {
FileMetadataDto fileMetadataDto = fileService.uploadFile(file, 1L, "POST_ATTACHMENT", 1L);
// return APIResponse.success(fileMetadataDto);
return ResponseEntity.status(HttpStatus.CREATED).body(fileMetadataDto);
public ResponseEntity<Object> uploadFile(@RequestParam("file") @Schema(type = "string", format = "binary", description = "File to upload") MultipartFile file) throws Exception {
APIResponse uploadResponse = fileService.uploadFile(file, 1L, "POST_ATTACHMENT", 1L);

return ResponseEntity.status(uploadResponse.getCode()).body(uploadResponse.getData());
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 리뷰를 진행하겠습니다.

  1. 예외 처리: 현재 uploadFile 메서드에서 throws Exception로 모든 예외를 포괄적으로 던지고 있습니다. 이는 코드의 읽기 쉬운 사용성을 떨어뜨릴 수 있고, 클라이언트에게 필요한 정보를 제공하지 못할 수 있습니다. 특정 예외를 잡아 처리하거나 커스텀 예외를 만들어 보다 명확한 에러 메시지를 전달하는 것이 좋습니다.

  2. 파일 크기 및 형식 검증: 업로드하는 파일에 대한 크기 및 형식 검증 로직이 없는 것처럼 보입니다. 이를 추가해서 사용자가 의도치 않게 잘못된 파일을 업로드하지 못하도록 하는 것이 좋습니다.

  3. 매직 넘버 사용: fileService.uploadFile(file, 1L, "POST_ATTACHMENT", 1L);에서 1L"POST_ATTACHMENT" 같은 매직 넘버와 문자열을 사용하고 있습니다. 이러한 값들은 의미를 명확히 하고 유지보수를 용이하게 하기 위해 상수로 선언하는 것이 바람직합니다.

  4. 로깅 추가: 파일 업로드 시 성공 및 실패에 대한 로깅이 없다면, 문제 발생 시 원인을 추적하기 어려울 수 있습니다. 따라서 성공적인 파일 업로드 시 및 예외 발생 시 로깅을 추가하는 것을 고려해보세요.

  5. 응답 객체의 메시지: 성공적인 파일 업로드 시의 응답에 메시지를 추가하면 클라이언트 측에서 더 나은 피드백을 받을 수 있습니다. APIResponse.success(fileMetadataDto)에 메시지를 포함하는 방법을 고려해보세요.

  6. API 문서화: Swagger에 대한 주석이 잘 작성되어 있으나, 너무 간단하게 작성된 것도 있습니다. 요청 및 응답의 예시를 추가하면 API 소비자가 이해하기 쉽게 만들 수 있습니다.

  7. 패키지 구조: com.seveneleven.devlens.global.util.file.Service.FileService와 같은 패키지 경로는 대문자로 시작하는 클래스명 관례에 맞지 않는데, 이는 정규화된 코딩 스타일을 따르지 않는 것입니다. 패키지 명식을 점검하고 필요하다면 경로를 조정하는 것이 좋습니다.

이러한 점들을 참고하여 개선하면 더욱 견고하고 유지보수가 쉬운 코드를 작성할 수 있을 것입니다.

Loading