-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/#28 #31
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
base: staging
Are you sure you want to change the base?
Conversation
Code Format Check ✅
|
Code Format Check ✅
|
Code Format Check ✅
|
Code Format Check ✅
|
Code Format Check ✅
|
Code Format Check ✅
|
Code Format Check ✅
|
Darren4641
left a comment
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.
코멘트 남겼습니다~! @koosco
| // 오브젝트 스토리지에 해당 키가 존재하는지 확인 | ||
| val exists = mediaStorage.exists(media.storageKey) | ||
|
|
||
| return transactionRunner.runNew { |
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.
execute내에서는 트랜잭션이 없는데 새로운 트랜잭션으로 작업하는 이유가 궁금합니다!
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.
Media가 다른 도메인에서 호출되는 경우가 많고, 모놀리식 아키텍처 특성상 실수로 다른 곳에서 트랜잭션이 시작된 후 호출될 수 있다고 생각했습니다. 기존 트랜잭션과 별개로 Media 도메인에서 트랜잭션이 독립적으로 실행되는 것을 보장하기 위해 REQUIRES_NEW로 두었습니다. 기존 트랜잭션이 없다면 동일하게 동작하기 때문에 runNew를 사용했습니다.
| @field:Nullable | ||
| val folderId: Long?, | ||
|
|
||
| val memo: String?, |
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.
memo Length PM과 논의
| val photoIds: List<Long>, | ||
| ) | ||
|
|
||
| data class UpdatePhotoRequest(val memo: String?) |
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.
기획이슈? 인건지 궁금합니다! memo에대해 NotNull 어노테이션이 더 적절해보이긴합니다!
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.
NotNull인 경우 기존에 사진에 있던 메모를 지울 수 없다 생각하여 Null 허용을 두었습니다. 개인이 보기 위한 메모이고, 메모를 없앨 수 있도록 하는 것이 UX상 좋을 것 같다고 판단했습니다. 기획자분과 다시 한 번 얘기 나눠보겠습니다.
| // TODO : 실패한 경우 처리, 재시도 or 실패 간주 | ||
| // 현재는 전체 usecase 실패로 간주 | ||
| if (availability != AVAILABLE) { | ||
| throw BusinessException(ResultCode.UPLOAD_FAILED) |
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.
프론트와 이야기해서 프론트에서 재처리를 진행해야할 것 같은데, (서버측에서 재시도를 못할 것 같다는 생각이,,,) 프론트에서 응답코드로 분기태워야할 것 같긴하네요
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.
아마 클라이언트가 S3 이미지 업로드에 성공한 후에 동기적으로 현재 API를 호출할 것 같습니다. 여기서 실패는 네트워크 장애 등 일시적인 장애를 고려한 내용인데 이거는 회의하며 다시 얘기해보면 좋을 것 같습니다.
src/main/kotlin/com/yapp2app/photo/infra/persist/jpa/JpaFolderRepository.kt
Outdated
Show resolved
Hide resolved
Code Format Check ✅
|
|
|
||
| fun execute(command: ConfirmMediaUploadedCommand): ConfirmMediaUploadedResult { | ||
| val media = mediaRepository.getActiveMedia(command.ownerId, command.mediaId) | ||
| ?: throw BusinessException(ResultCode.NOT_FOUND) |
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.
presignedUrl 발급받은 후 [POST] /api/photos 사진등록 API를 호출하는데, 이부분에서 계속 예외가 발생합니다!
.markAsUploaded() 처리하는 부분이 아래 return 부분밖에없는데, mediaRepository.getActiveMedia() 의 조건에 MediaStatus.UPLOADED이 있어서 그런 것 같은데 한번 확인 해주세요!
추가로 제가 스테이징 환경으로 한번 로킬에서 실행해보고 presignedUrl 로 업로드를 해본 결과 사진은 잘 저장됐습니다!
근데 사진 파일명에 확장자가 빠져서 들어가더라구요
GenerateUploadTicketUseCase 클래스에서 command.contentType이 image/png로 들어가는거 확인했는데도 실제 S3에서는 확장자가 빠져있습니다! 이것도 한번 확인해주세요!
Code Format Check ✅
|
Code Format Check ✅
|
Code Format Check ✅
|
summary
MediaUseCase 추가
FakeMediaStorage 추가
Photo API 추가
PhotoImageE2ETest 추가
details
MediaUseCase 추가
GenerateUploadTicketUseCase
GetMediasUseCase
ConfirmMediaUseCase
DeleteMediaUseCase
FakeMediaStorage 추가
Photo API 추가
이미지 업로드 API
이미지 목록 API
이미지 삭제 API / 벌크 삭제 API
이미지 수정 API
PhotoImageE2ETest 추가
c.c.