[FEAT] 피드 이미지 기능 추가 #347
[FEAT] 피드 이미지 기능 추가 #347
Conversation
해당 Config는 다음 설정 값을 필요로 합니다. application.yml - s3.access-key - s3.secret-key - s3.bucket
S3 버킷의 feed 폴더에 이미지를 업로드합니다.
이미지 파일의 이름은 UUID.{기존 이미지 확장자} 입니다.
성공적으로 업로드시 이미지가 업로드된 URL을 제공하며, 실패시 예외가 발생합니다.
또한 다음과 같은 유효성을 검증합니다.
- 빈 파일 여부
- 파일 확장자 여부
FeedImage는 썸네일과 일반 이미지로 구분됩니다. 이미지의 순서를 보장하며, 썸네일의 경우는 0번째의 순서를 지닙니다.
Image객체에서 Feed를 바라볼 일이 없다고 판단하여, 단방향 맵핑으로 구성하였습니다. Controller의 Validate 메서드는 DTO로 랩핑후 제거될 예정입니다.
기존 사용중인 이미지는 eventListener를 사용하여 피드 수정 커밋이 완료된 후에 삭제하도록 하였습니다. 이미지 삭제 과정에서 예외가 발생하면, log만 남기고 계속해서 삭제 과정을 진행하도록 하였습니다.
- Feed 목록 조회시에는 각 Feed의 썸네일 이미지, 이미지의 개수를 반환하도록 하였습니다. - 이미지의 개수는 List의 size 메서드로 반환하고 있기 때문에 추후, 쿼리로 개수만 불러올 수 있도록 수정할 예정입니다. - Feed 상세 조회시에는 Feed의 모든 이미지를 List로 반환하도록 하였습니다. - 썸네일의 경우는 가장 첫 번째 이미지이며, 이후 이미지들은 저장할 때의 순서를 보장해서 불러오록 하였습니다. - 해당 과정에서 @orderby 애너테이션을 사용하였습니다.
@RequestPart List<MultipartFile> 매개변수로 이미지를 받고 있었으며, 이로 인해 private void validateImage라는 메서드를 통해서 이미지 개수 검증을 하였습니다. 기존 코드의 통일성을 위해 @Valid @ModelAttribute FeedImageUpdateRequest 로 변경하였으며, 해당 DTO에서 List 최댓값을 검증하였습니다.
라이브러리에서 예외처리 로직이 전부 구현되지 않았기 때문에 라이브러리에서 발생하는 예외 핸들링은 추후 추가될 예정입니다. 작성된 예외 처리는 다음과 같습니다. - 빈 이미지 파일 업로드 - 이미지 파일의 이름 및 확장자 누락 - 이미지 파일이 손상되었거나 존재하지 않음 - 이미지 업로드 실패
피드 목록 조회시, 피드의 모든 이미지를 불러온 후, 애플리케이션 로직에서 썸네일과 이미지 개수를 구하는 문제가 있었음 이를 가볍게 불러오기 위해, 썸네일과 이미지 개수를 불러오는 쿼리를 작성함
GiJungPark
requested review from
EunjeongHeo,
Kim-TaeUk,
rinarina0429 and
sansan20535
EunjeongHeo
left a comment
EunjeongHeo
left a comment
There was a problem hiding this comment.
수고하셨습니다!!
이미지 수정 시 트랜잭션 커밋 이후에 삭제 처리하도록 구성하신 부분 좋은 것 같아요!
리뷰 두 개 달아두었습니다 ~~
| log.info("AccessKey = " + accessKey); | ||
| log.info("SecretKey = " + secretKey); | ||
| log.info("Bucket = " + bucket); |
There was a problem hiding this comment.
P3; 키를 직접 로그로 찍는 이유가 있을까요? 아니면 임시로 쓰는 디버깅용일까요?
There was a problem hiding this comment.
서버 실행 시점에 어떤 자격 증명과 버킷을 사용했는지 확인하기 위해 info log 찍어놨어요. 큰 의미가 있는 로그는 아닙니다!
There was a problem hiding this comment.
p4; 이러한 로그가 운영기에 올라가도 되는 정보인지가 궁금합니다!
이러한 정보는 yml도 둘 만큼 민감한 정보라고 알고 있는데 로그를 찍게 되면 보안상 위험하지 않을까..? 라는 생각을 했습니다! 현재 로그 레벨을 분리하는 별도 작업이 없는 것 같은데 이러한 정보를 찍어도 괜찮은지가 궁금합니다!
There was a problem hiding this comment.
보안적인 측면은 고려하지 못했네요.
현재 프로젝트에서 모든 인원이 서버의 민감 정보를 전부 알 수 있기 때문에, 큰 문제는 없을 것 같습니다.
하지만 혹시 모르는 노출이 있을수도 있으니, 제거하는게 좋을것 같습니다 😄
There was a problem hiding this comment.
넵 저도 보안을 생각해서 달았던 코멘트였습니다.
제거하신 커밋 확인했습니다! 감사합니다~!!
rinarina0429
left a comment
rinarina0429
left a comment
There was a problem hiding this comment.
수고하셨습니다~! 컨벤션 한번씩 확인해주시면 더 좋을 것 같아요👍
src/main/java/org/websoso/WSSServer/dto/feed/FeedGetResponse.java
Outdated
Show resolved
Hide resolved
src/main/java/org/websoso/WSSServer/repository/FeedCustomRepositoryImpl.java
Outdated
Show resolved
Hide resolved
| log.info("AccessKey = " + accessKey); | ||
| log.info("SecretKey = " + secretKey); | ||
| log.info("Bucket = " + bucket); |
There was a problem hiding this comment.
p4; 이러한 로그가 운영기에 올라가도 되는 정보인지가 궁금합니다!
이러한 정보는 yml도 둘 만큼 민감한 정보라고 알고 있는데 로그를 찍게 되면 보안상 위험하지 않을까..? 라는 생각을 했습니다! 현재 로그 레벨을 분리하는 별도 작업이 없는 것 같은데 이러한 정보를 찍어도 괜찮은지가 궁금합니다!
src/main/java/org/websoso/WSSServer/repository/FeedCustomRepositoryImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/org/websoso/WSSServer/repository/FeedCustomRepositoryImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/org/websoso/WSSServer/repository/FeedCustomRepositoryImpl.java
Outdated
Show resolved
Hide resolved
Related Issue
Key Changes
피드 이미지 테이블 설계
FeedImage는Feed를 통해서만 CRUD가 이뤄지므로 단방향 일대다 관계로 설정하였습니다.@OrderColumn대신 애플리케이션 레벨에서 이미지 순서를 직접 관리하여, 확장성과 요구사항 변경에 유연하게 대응하도록 구성하였습니다.피드 이미지 수정
error로그로 기록하여 추적할 수 있도록 하였습니다.Feed 조회시 이미지도 같이 반환
To Reviewers
이미지 유효성 검사
라이브러리
WSS-Server-S3의S3ImageService에서 실제 이미지 파일 여부를 검사할 예정입니다.그리고, 애플리케이션 로직에서는 확장자 기반 간단 검사만 수행하고자 합니다.
이에 대해 어떻게 생각하시나요?
(
S3ImageService는 대부분의 이미지 타입을 지원 예정이며, 애플리케이션에서의 확장자 필터는 어떤 이미지들만 수용할 것인지 확정되면 적용 예정입니다.)잉여 이미지 정리 관련
이미지 업로드/수정 중 문제가 발생하면 S3 용량을 차지하는 고아 이미지가 생길 수 있습니다.
이미지 테이블을 따로 두고 맵핑 테이블을 구성하여 맵핑되지 않은 이미지들은 사용되지 않는 이미지로 식별하여 배치로 삭제하는 구조를 고려했습니다.
하지만 MAU가 많지 않은 상황에서 배치를 도는 것이 오버엔지니어링일 수 있다는 생각이 들어서 단순 일대다 관계로 구성했는데, 어떻게 생각하는지 의견을 묻고 싶습니다.
References