-
Notifications
You must be signed in to change notification settings - Fork 1
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
S3 image upload #22
S3 image upload #22
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인하였습니다.
이미지 업로드 (혹은 파일어로드)에 대해서 검증 로직을 백엔드에 추가하고,
테스트코드로 문서화도 하면 좋을것같네요.
@@ -18,11 +20,12 @@ public class BoardApiController { | |||
private final BoardService boardService; | |||
|
|||
// 게시글 생성 | |||
@PostMapping("/boards") | |||
@PostMapping(value = "/boards", consumes = {MediaType.MULTIPART_FORM_DATA_VALUE, MediaType.APPLICATION_JSON_VALUE}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
멀티파트 form request에서 실제 http 메세지가 어떻게 전송되는지,
어떤 구조로 이루어져있어 여러개의 파일을 전송 할 수 있는지 찾아보면 좋을듯합니다.
private String region; | ||
|
||
@Bean | ||
@Primary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Primary 가 어떨때 쓰이는 애너테이션인지 확인이 필요합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
정리했습니다!
return boardMapper.findByIdWithNickName(boardId); | ||
BoardDetailResponse response = boardMapper.findByIdWithNickName(boardId); | ||
String imageFileName = imageMapper.findImageFileName(boardId); | ||
response.setImageURL(s3Service.loadImage(imageFileName)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vo, dto, 엔티티에 대한 차이를 공부해서 정리하고,
용도에 맞춰 클래스를 분리해주세요
지금 상황에서는 BoardDetailResponse의
일부 필드는 db에서 가져오고, 일부 필드는 s3 요청에 대한 응답을 가져와서 저장합니다.
나중에 BoardDetailResponse가 여러곳에서 쓰인다면, 코드를 수정 했을때 영향도를 예측하기 어려울듯합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인하였습니다. 고생하셨습니다.
머지 후 추가로 작업 진행해주세요
@@ -16,6 +17,13 @@ public class CreateBoardRequest { | |||
private String title; | |||
@NotEmpty(message = "내용을 입력해주세요.") | |||
private String content; | |||
private List<ImageInfoRequest> imageInfo; | |||
|
|||
public CreateBoardRequest changeS3ImageKey(String from, String to) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changeS3ImageKey()로 객체 생성 이후 2차로 필드를 초기화 하는 경우,
다른 개발자들이 해당 코드를 사용 할 때 실수로 해당 메서드를 놓칠 수도 있습니다.
(RequestBody로 넘어오지 않고, 직접 객체를 생성하는 경우 등등 포함)
이 부분에 대해서 객체 생성시점에 데이터 세팅이 완료되도록 설계를 변경하는게 좋을듯합니다.
|
||
@AllArgsConstructor | ||
@Getter | ||
@Setter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
필요한 필드만 setter를 열어주면 좋을것같네요,
생성자도 포함입니다.
|
||
public CreateBoardRequest changeS3ImageKey(String from, String to) { | ||
// regex 를 이용해 from 폴더 -> to 폴더로 src 속성값 변경 | ||
content = content.replace(".com/" + from + "/", ".com/" + to + "/"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이부분은 테스트 코드같은걸로 빼서만들어도 좋겠네요.
다른사람이 알아보기 쉬울것같아요
No description provided.