-
Notifications
You must be signed in to change notification settings - Fork 1
[FEAT] 장소 업로드 API 구현 (#251) #253
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
Conversation
Merge branch 'develop' into feature/#251
BaseInquiryView에서 하고있음
사진 없으면 바로 postSpot. 있으면 presignedURL 처리 후 postSpot
cirtuare
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.
💗 최고 !! 고생했어용 리뷰 한 번만 확인해주세요 !!
| private func putPhotosToPresignedURL() { | ||
| // NOTE: `getPresignedURL`에서 handleNetworkError로 처리되기 때문에 guard문 걸릴 가능성 적음 | ||
| // 다만, 예외적인 상황을 위해 방어 로직으로 추가함 | ||
| guard !photos.isEmpty, photoPresignedURLInfos.count == photos.count else { |
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.
🐿️🐿️
이미 getPresignedURL의 guard문에서 호출하고 있는데, 여기서도 !photos.isEmpty조건이 필요한가요?
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.
필요 없어서 삭제했습니다!
| for (index, photo) in photos.enumerated() { | ||
| guard let imageData = photo.jpegData(compressionQuality: 1) else { | ||
| print("❌ 이미지 데이터 변환 실패 at: \(index)") | ||
| continue | ||
| } | ||
|
|
||
| dispatchGroup.enter() | ||
|
|
||
| let request = PutImageToPresignedURLRequest( | ||
| presignedURL: photoPresignedURLInfos[index].presignedURL, | ||
| imageData: imageData | ||
| ) | ||
|
|
||
| ACService.shared.imageService.putImageToPresignedURL(requestBody: request) { [weak self] response in | ||
| defer { dispatchGroup.leave() } | ||
| guard let self = self else { return } | ||
|
|
||
| switch response { | ||
| case .success(_): break | ||
| case .reIssueJWT: | ||
| self.handleReissue { | ||
| self.putPhotosToPresignedURL() | ||
| } | ||
| default: | ||
| self.handleNetworkError { | ||
| self.putPhotosToPresignedURL() | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| dispatchGroup.notify(queue: .main) { [weak self] in | ||
| self?.postSpot() | ||
| } |
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.
🐿️🐿️🐿️
현재 getPresignedURL -> putPhotosToPresignedURL 로 넘어갈 때 photoPresignedURLInfos.count == photos.count, 즉 사진을 한 장이라도 업로드하지 못하면 바로 onSuccessPostSpot을 false로 처리하고 있는데
putPhotosToPresignedURL -> postSpot으로 넘어갈 때는 presignedURL에 사진 일부를 업로드하지 못해도 postSpot을 아무 문제 없이 호출하고 있는 것 같아요!
물론 부가적인 방어 로직이지만, 이 두 로직의 일관성은 지켜져야 한다고 생각해서 말씀드려 봅니다
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.
case .success(_):
uploadResults[index] = true
case .reIssue, .networkError:
uploadResults[index] = false
...
dispatchGroup.notify(queue: .main) { [weak self] in
let allSuccess = uploadResults.allSatisfy { $0 }
allSuccess ? self?.postSpot() : self?.onSuccessPostSpot.value = false
}
이런 식으로 처리하면 어떨까요? 혹시 제가 잘못 이해했다면 말씀해주세용
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.
putPhotosToPresignedURL에서는 업로드를 실패할 경우
reIssueJWT, handleNetworkError로 핸들링되어 다시 시도하여 사진 일부를 업로드하지 못한 채로 postSpot이 호출되지 않을 거라 생각했었는데
지금 다시 보니 성공하든 실패하든 DispatchGroup.leave()돼서 postSpot 되네요 😅😅
제안 주신 코드 좋은 것 같아요!!! 꼼꼼히 봐주셔서 감사합니다~~~
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.
반영했습니다!
그리고 photoPresignedURLInfos.count == photos.count guard문을
getPresignedURLs 말미에서 .allSatisfy { $0.value }인지 확인하는 방식으로 수정했습니다-!
여기서 삼항연산자 사용은 안 돼서 if문으로 구현했습니다!
private func getPresignedURLs() {
...
dispatchGroup.notify(queue: .main) { [weak self] in
let allSuccess = self?.photoPresignedURLResults.allSatisfy { $0.value } ?? false
if allSuccess {
self?.putPhotosToPresignedURL()
} else {
self?.onSuccessPostSpot.value = false
}
}
}
private func putPhotosToPresignedURL() {
...
dispatchGroup.notify(queue: .main) { [weak self] in
let allSuccess = self?.photoPresignedURLResults.allSatisfy { $0.value } ?? false
if allSuccess {
self?.postSpot()
} else {
self?.onSuccessPostSpot.value = false
}
}
}
cirtuare
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.
최고~~
Merge branch 'develop' into feature/#251
🐿️ Pull Requests
🪵 작업 브랜치
🥔 작업 내용
🚨 참고 사항
putPhotosToPresignedURL() 방어 로직
SpotUploadViewModelputPhotosToPresignedURL() 내부에photos.count 와 presignedURLInfos.count를 비교하는 guard문이 있는데,
handleNetworkError 때문에 count가 미스매치될 일은 거의 없습니다.
다만, 예외적인 상황을 대비해 방어 로직으로서 추가했습니다
이 경우에 대응하는 코드로
SpotUploadViewController160번째 줄에서 장소 업로드에 실패할 경우 Alert을 띄우고 tabBar로 가게 했습니다📸 스크린샷
default.mov
💥 To be sure
🌰 Resolve issue