Feat: Add Contest Thumbnail Upload API (#59)#60
Conversation
Add POST /api/contests/:id/thumbnail endpoint that uploads a thumbnail image to R2 storage and updates the contest's thumbnail URL and banner_key fields in a single operation. Only contest leaders can upload. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Summary of ChangesHello @Sunja-An, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 콘테스트 리더가 자신의 콘테스트에 썸네일 이미지를 업로드할 수 있도록 새로운 기능을 도입합니다. 이 기능은 R2 스토리지에 이미지를 안전하게 업로드하고, 관련 콘테스트 데이터베이스 레코드를 자동으로 업데이트하며, 적절한 사용자 권한 검사를 보장하여 이미지 관리 프로세스를 간소화합니다. 이를 통해 사용자 경험을 향상시키고 콘테스트 콘텐츠의 시각적 매력을 높이는 것을 목표로 합니다. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This PR introduces a new API endpoint for uploading contest thumbnails. While a security audit identified no critical vulnerabilities, a minor functional discrepancy was noted regarding the reuse of the BannerKey field for thumbnails. The implementation could be improved by addressing potential race conditions and error handling issues in contest_service.go to prevent data inconsistency or orphaned files. Additionally, there is code duplication in the controller and naming issues that impact maintainability.
| contest, err := c.repository.GetContestById(contestId) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| // Delete old thumbnail from storage if exists | ||
| if contest.BannerKey != nil && *contest.BannerKey != "" { | ||
| go func() { | ||
| if delErr := c.storagePort.Delete(context.Background(), *contest.BannerKey); delErr != nil { | ||
| log.Printf("[UploadThumbnail] Failed to delete old thumbnail %s: %v", *contest.BannerKey, delErr) | ||
| } | ||
| }() | ||
| } | ||
|
|
||
| contest.Thumbnail = &url | ||
| contest.BannerKey = &key | ||
|
|
||
| if err := c.repository.UpdateContest(contest); err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
여기에 잠재적인 경쟁 조건(race condition)이 있습니다. 동일한 대회에 대한 썸네일 업로드 요청 두 개가 동시에 들어오면, 두 요청 모두 동일한 이전 BannerKey를 읽을 수 있으며, 마지막 DB 업데이트가 이기게 되어 스토리지에 고아 파일(orphaned file)이 남을 수 있습니다. contest 객체에 대한 읽기-수정-쓰기 작업(GetContestById부터 UpdateContest까지)은 이를 방지하기 위해 데이터베이스 트랜잭션 내에서 원자적으로 수행되어야 합니다. 일반적으로 이는 행을 잠그기 위해 SELECT ... FOR UPDATE를 사용하는 것을 포함합니다.
| if err := c.repository.UpdateContest(contest); err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
UpdateContest가 실패하면 새 썸네일은 이미 스토리지에 업로드되었지만 데이터베이스 레코드는 업데이트되지 않습니다. 이로 인해 스토리지에 고아 파일이 남게 됩니다. 스토리지 작업을 롤백하기 위해 새로 업로드된 파일을 삭제하여 이 경우를 처리해야 합니다.
if err := c.repository.UpdateContest(contest); err != nil {
// Attempt to delete the newly uploaded file on DB update failure
go func() {
if delErr := c.storagePort.Delete(context.Background(), key); delErr != nil {
log.Printf("[UploadThumbnail] Failed to delete orphaned thumbnail %s after DB error: %v", key, delErr)
}
}()
return nil, err
}| } | ||
|
|
||
| contest.Thumbnail = &url | ||
| contest.BannerKey = &key |
| func (c *ContestController) UploadThumbnail(ctx *gin.Context) { | ||
| id, err := strconv.ParseInt(ctx.Param("id"), 10, 64) | ||
| if err != nil { | ||
| response.JSON(ctx, response.BadRequest("invalid contest id")) | ||
| return | ||
| } | ||
|
|
||
| userId, ok := middleware.GetUserIdFromContext(ctx) | ||
| if !ok { | ||
| response.JSON(ctx, response.Error(401, "user not authenticated")) | ||
| return | ||
| } | ||
|
|
||
| file, err := ctx.FormFile("file") | ||
| if err != nil { | ||
| response.JSON(ctx, response.BadRequest("file is required")) | ||
| return | ||
| } | ||
|
|
||
| result, err := c.service.UploadThumbnail(ctx.Request.Context(), id, userId, file) | ||
| c.helper.RespondCreated(ctx, result, err, "thumbnail uploaded successfully") | ||
| } |
There was a problem hiding this comment.
이 UploadThumbnail 메소드의 로직은 파일 끝에 있는 HandleUploadThumbnail 함수와 중복됩니다. 이는 DRY(Don't Repeat Yourself) 원칙을 위반하며 코드를 유지보수하기 어렵게 만듭니다. 이 파일의 기존 패턴은 로직을 포함하는 '테스트 가능한' 핸들러 함수를 두고 컨트롤러 메소드가 이를 호출하는 것으로 보입니다. 중복을 제거하기 위해 이 패턴을 따르는 것이 좋습니다.
이 메소드를 HandleUploadThumbnail을 호출하도록 리팩토링하세요.
func (c *ContestController) UploadThumbnail(ctx *gin.Context) {
HandleUploadThumbnail(ctx, c.service, c.helper)
}
Summary
POST /api/contests/:id/thumbnail— handles R2 upload + contest DB record update in a single operationChanges
internal/contest/application/port/contest_storage_port.gointernal/contest/application/contest_service.goUploadThumbnailmethod +SetStoragePortsetterinternal/contest/application/dto/contest_dto.goThumbnailUploadResponseDTOinternal/contest/presentation/contest_controller.goUploadThumbnailhandler + route registrationinternal/storage/domain/storage.goUploadTypeContestThumbnailtype and size constantinternal/storage/provider.goStoragePortfield in Dependenciescmd/server.goAPI Spec
POST /api/contests/:id/thumbnailmultipart/form-datafile(image, max 5MB, jpeg/png/webp){ key, url, size, mime_type, uploaded_at }Test plan
POST /api/contests/:id/thumbnailwith a leader accountCloses #59
🤖 Generated with Claude Code