[FEAT/#76] PreSigned URL 획득 API 구현 #81
Conversation
| @@ -58,11 +62,14 @@ public class MemberService { | |||
|
|
|||
| private final JwtTokenProvider jwtTokenProvider; | |||
| private final PrincipalHandler principalHandler; | |||
| // TODO: 네이밍 변경 | |||
There was a problem hiding this comment.
Q2: 네이밍 변경이 필요하다고 느끼신 이유가 어떤걸까요?
There was a problem hiding this comment.
외부 API에 연결하는 클래스들은 Service 대신 Adapter로 컨벤션 통일하는 게 좋을 것 같아서요 !
There was a problem hiding this comment.
오 좋네요! 그러면 캐시에 리프레시
토큰 저장하는 로직들도 Adatpter로 분리하는 거 어떠신가요?
| @GetMapping(path = "/images/presigned-url", produces = MediaType.APPLICATION_JSON_VALUE) | ||
| public ResponseEntity<PreSignedUrlResponse> getPreSignedUrl( |
There was a problem hiding this comment.
Q1: Presigned -> PreSigned 로 변경하신 이유가 있나요?!
단어를 분리해서 생각하셔서 변경하신 것 같아요! 그렇다면 엔드포인트도 pre-signed-url이 맞지 않을까요?
저는 개인적으로 Presigned가 더 적합한 것 같아용
There was a problem hiding this comment.
저도 그렇게 생각해서 Presigned로 사용하고 있었는데 인텔리제이에서 맞춤법 수정 띄우길래 찾아보니까 일단은 정식 명칭이 Pre-signed Url인 것 같더라구요 ?! 그래서 바꿔봤는데 별로인가요 . . . 🥹
There was a problem hiding this comment.
엇 아니요!수정의 의미로 말씀드린 것보다 PreSigned처럼 pre와 signed 두 단어를 분리한 부분도 있고 /presigned-url처럼 두 단어를 분리하지 않은 부분이 있어서 따로 이유가 있는지 여쭤보고 싶었습니다~!
헷갈릴 수도 있지 않을까 싶은데 큰 문제는 아닐 것 같아용
| if (!path.isEmpty()) { | ||
| fileName = path + "/" + fileName; | ||
| } |
There was a problem hiding this comment.
Q1: path가 Empty일 땐 따로 처리를 할 필요가 없나요 !? 에러를 뱉거나, 이후 로직들을 실행하지 않게 하거나 ...
There was a problem hiding this comment.
Empty일 때는 버킷 루트 경로에 파일이 저장되는 걸 생각하고 이렇게 로직을 짰는데요 ~!
어차피 getPreSignedUrl 메서드는 private으로 선언하고 getPreSignedUrlForProfileImage 형태로 외부에서 접근하도록 막아놔서 따로 처리할 필요가 없을 것 같긴 합니다 !
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| // TODO: 추후 AWS SDK v2를 사용하는 방식으로 변경 |
There was a problem hiding this comment.
Q1: 현재는 어떤 방식으로 진행하는데 어떤 문제 때문에 변경을 생각하시는건가용?
There was a problem hiding this comment.
현재는 AWS SDK v1을 사용하는 방식이고, 큰 문제는 없지만 v2를 사용하는 방식으로 마이그레이션하게 되면 비동기 처리 등 성능적으로 더 우수한 면이 있어 추후 리팩토링하면 좋을 것 같아 TODO 처리했습니다 !
💡 Issue
📸 Screenshot
📄 Description
💬 To Reviewers
🔗 Reference