[REFACTOR]: 가게 테이블 생성 시 이미지 업로드 로직 추가#93
Conversation
- 기존 DTO tableImageUrl -> tableImageKey - 서비스 로직에 이미지 처리 로직 추가 - Key -> Url로 변환하여 응답 DTO로 넘겨주도록 수정
📝 WalkthroughWalkthroughThis pull request adds a temporary image upload endpoint for store tables that defers S3 object movement to permanent storage until after transaction commit. It introduces new DTOs, converter methods, and transaction synchronization hooks to handle pre-upload workflows before table creation. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller
participant Service
participant S3
participant Database as Database<br/>(Transaction)
participant TxnSync as Transaction<br/>Synchronization
Note over Client,TxnSync: Temporary Image Upload Flow
Client->>Controller: POST /stores/{id}/tables/images<br/>(multipart image)
Controller->>Service: uploadTableImageTemp(storeId, file)
Service->>S3: Upload to temp/tables/UUID.jpg
S3-->>Service: Return imageKey & URL
Service-->>Controller: Return ImageUploadDto
Controller-->>Client: 200 OK (imageKey, imageUrl)
Note over Client,TxnSync: Table Creation with Pre-uploaded Image
Client->>Controller: POST /stores/{id}/tables<br/>(tableData + imageKey)
Controller->>Service: createTable(storeId, tableCreateDto)
Service->>Database: Begin Transaction
Service->>Database: Create table entity with imageKey
Service->>TxnSync: Register afterCommit hook<br/>(move temp → permanent)
Database-->>TxnSync: Transaction Committed
TxnSync->>S3: Move temp/tables/UUID.jpg<br/>→ stores/{id}/tables/UUID.jpg
S3->>S3: Delete temp file
Service-->>Controller: Return TableCreateDto
Controller-->>Client: 201 Created
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/storetable/controller/StoreTableControllerDocs.java`:
- Around line 66-72: The `@Parameter` for the multipart parameter in
StoreTableControllerDocs.java should explicitly set name="image" to match the
controller binding; update the `@Parameter`(...) on the MultipartFile file
declaration to include name = "image" so the OpenAPI spec reflects the request
part name used by `@RequestPart`("image") and avoids clients using the wrong form
field name.
In
`@src/main/java/com/eatsfine/eatsfine/domain/storetable/service/StoreTableCommandServiceImpl.java`:
- Around line 88-97: Validate the client-supplied temp image key before moving
it: in StoreTableCommandServiceImpl where tempImageKey = dto.tableImageKey() is
used, add a guard that ensures tempImageKey matches the expected temp
prefix/pattern (for example startsWith("temp/tables/") and a simple filename
extension check) before calling s3Service.extractExtension(...) and creating
permanentImageKey; if the check fails, either skip processing or throw a
validation exception (e.g., IllegalArgumentException) so only properly-prefixed
temp keys are accepted.
🧹 Nitpick comments (2)
src/main/java/com/eatsfine/eatsfine/domain/storetable/service/StoreTableCommandServiceImpl.java (2)
100-111: Add a fallback for failed post-commit moves.If the move fails after commit, the table record already points to the permanent key, which can produce broken image URLs. Consider a retry/compensation path (e.g., background retry and/or updating the table only after a successful move).
214-225: Log failures when deleting stored images.The deletion after commit currently fails silently. Logging (or metrics) here helps track orphaned images.
🧾 Suggested logging
if (imageKey != null && !imageKey.isBlank()) { TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { `@Override` public void afterCommit() { - s3Service.deleteByKey(imageKey); + try { + s3Service.deleteByKey(imageKey); + } catch (Exception e) { + log.error("테이블 이미지 삭제 실패. key={}", imageKey, e); + } } } ); }
| @Parameter( | ||
| description = "업로드할 이미지 파일", | ||
| required = true, | ||
| content = @Content(mediaType = MediaType.MULTIPART_FORM_DATA_VALUE) | ||
| ) | ||
| MultipartFile file | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.java" | grep -i "storetable" | head -20Repository: Eatsfine/BE
Length of output: 1601
🏁 Script executed:
cat -n src/main/java/com/eatsfine/eatsfine/domain/storetable/controller/StoreTableControllerDocs.java | head -100Repository: Eatsfine/BE
Length of output: 5603
🏁 Script executed:
# Search for the controller file
find . -type f -name "*StoreTableController.java" | grep -v DocsRepository: Eatsfine/BE
Length of output: 147
🏁 Script executed:
cat -n src/main/java/com/eatsfine/eatsfine/domain/storetable/controller/StoreTableController.java | grep -A 20 "uploadTableImageTemp"Repository: Eatsfine/BE
Length of output: 1632
🏁 Script executed:
# Look for the entire method implementation
ast-grep --pattern $'@$_
public $_ uploadTableImageTemp($$$) {
$$$
}'Repository: Eatsfine/BE
Length of output: 2130
🏁 Script executed:
# Search for other multipart parameter documentation patterns in the codebase
rg -A 5 "@RequestPart" --type java | head -50Repository: Eatsfine/BE
Length of output: 4068
🏁 Script executed:
# Check for any springdocs or OpenAPI configuration
find . -type f -name "*.java" | xargs rg -l "springdoc|OpenAPI" | head -10Repository: Eatsfine/BE
Length of output: 295
🏁 Script executed:
# Look for similar `@Parameter` examples with multipart in the codebase
rg -B 2 -A 5 "@Parameter.*MultipartFile" --type javaRepository: Eatsfine/BE
Length of output: 37
🏁 Script executed:
cat -n src/main/java/com/eatsfine/eatsfine/global/config/SwaggerConfig.javaRepository: Eatsfine/BE
Length of output: 2454
🏁 Script executed:
# Search for other `@Parameter` usages with MultipartFile by checking the docs files
rg "@Parameter" src/main/java/com/eatsfine/eatsfine/domain/ -A 3 | grep -B 3 -A 3 "MultipartFile"Repository: Eatsfine/BE
Length of output: 37
🏁 Script executed:
# Look for MenuControllerDocs to see if they have multipart parameter documentation
find . -name "*ControllerDocs.java" -type fRepository: Eatsfine/BE
Length of output: 348
🏁 Script executed:
# Check MenuControllerDocs.java for multipart parameter patterns
cat src/main/java/com/eatsfine/eatsfine/domain/menu/controller/MenuControllerDocs.java | grep -B 5 -A 5 "MultipartFile"Repository: Eatsfine/BE
Length of output: 169
🏁 Script executed:
# Check StoreControllerDocs for multipart parameter patterns
cat src/main/java/com/eatsfine/eatsfine/domain/store/controller/StoreControllerDocs.java | grep -B 5 -A 5 "MultipartFile"Repository: Eatsfine/BE
Length of output: 171
🏁 Script executed:
# Check if StoreTableControllerDocs has another multipart parameter documented
grep -B 5 -A 5 "@RequestPart" src/main/java/com/eatsfine/eatsfine/domain/storetable/controller/StoreTableControllerDocs.javaRepository: Eatsfine/BE
Length of output: 37
🏁 Script executed:
# Look at the actual controller to see if there are other RequestPart mappings
cat src/main/java/com/eatsfine/eatsfine/domain/storetable/controller/StoreTableController.java | grep -B 15 "@RequestPart" | head -60Repository: Eatsfine/BE
Length of output: 1619
🏁 Script executed:
# Let's verify by looking at how Springdocs handles this
# Check if there's any documentation or examples in the repo about `@Parameter` with multipart
rg -i "springdoc\|@Parameter" src/main/java/com/eatsfine/eatsfine/global/ --type javaRepository: Eatsfine/BE
Length of output: 37
🏁 Script executed:
# Check the pom.xml or build.gradle to see what version of springdocs is used
find . -name "pom.xml" -o -name "build.gradle" | head -1 | xargs cat | grep -i springdoc -A 2 -B 2Repository: Eatsfine/BE
Length of output: 170
🏁 Script executed:
# Search for web resources about springdocs behavior with multipart parameters
# Let's also check if there are other instances of `@Parameter` with name attribute in the docs
rg "name\s*=" src/main/java/com/eatsfine/eatsfine/domain/storetable/controller/StoreTableControllerDocs.javaRepository: Eatsfine/BE
Length of output: 37
🏁 Script executed:
# Let's check the actual second multipart endpoint in the docs to see if it has the same issue
cat -n src/main/java/com/eatsfine/eatsfine/domain/storetable/controller/StoreTableControllerDocs.java | sed -n '73,120p'Repository: Eatsfine/BE
Length of output: 2958
🏁 Script executed:
# Find the uploadTableImage endpoint in the docs interface
grep -n "uploadTableImage" src/main/java/com/eatsfine/eatsfine/domain/storetable/controller/StoreTableControllerDocs.javaRepository: Eatsfine/BE
Length of output: 204
🏁 Script executed:
# Let me search for where the uploadTableImage method is documented in the docs interface
cat src/main/java/com/eatsfine/eatsfine/domain/storetable/controller/StoreTableControllerDocs.java | tail -100 | head -50Repository: Eatsfine/BE
Length of output: 2277
🏁 Script executed:
# Get the uploadTableImage method documentation from lines around 240
sed -n '220,260p' src/main/java/com/eatsfine/eatsfine/domain/storetable/controller/StoreTableControllerDocs.javaRepository: Eatsfine/BE
Length of output: 1711
🏁 Script executed:
# Let's verify what version of springdoc is being used and check its behavior with multipart
# Check if there's any test or example showing the generated OpenAPI for this endpoint
find . -name "*.json" -o -name "*.yaml" | grep -i "openapi\|swagger"Repository: Eatsfine/BE
Length of output: 37
🏁 Script executed:
# Let's also verify the actual behavior by searching for springdocs documentation patterns
# Check if name attribute is used elsewhere in `@Parameter` annotations
rg "@Parameter" src/main/java/com/eatsfine/eatsfine/domain/ --type java -A 2 | grep -E "(name\s*=|@Parameter)" | head -30Repository: Eatsfine/BE
Length of output: 4313
Add explicit name = "image" to the @Parameter annotation.
The controller binds this multipart parameter to the request part named "image" via @RequestPart("image"), but the documentation parameter lacks an explicit name. Since the parameter variable name is file, there's a risk that the OpenAPI spec may not correctly reflect that the multipart field name should be image, leading to client confusion.
Suggested fix
`@Parameter`(
+ name = "image",
description = "업로드할 이미지 파일",
required = true,
content = `@Content`(mediaType = MediaType.MULTIPART_FORM_DATA_VALUE)
)
MultipartFile file🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/storetable/controller/StoreTableControllerDocs.java`
around lines 66 - 72, The `@Parameter` for the multipart parameter in
StoreTableControllerDocs.java should explicitly set name="image" to match the
controller binding; update the `@Parameter`(...) on the MultipartFile file
declaration to include name = "image" so the OpenAPI spec reflects the request
part name used by `@RequestPart`("image") and avoids clients using the wrong form
field name.
| // 이미지 처리 temp → permanent | ||
| String permanentImageKey = null; | ||
| String tempImageKey = dto.tableImageKey(); | ||
|
|
||
| if (tempImageKey != null && !tempImageKey.isBlank()) { | ||
| Long tableId = savedTable.getId(); | ||
| String extension = s3Service.extractExtension(tempImageKey); | ||
|
|
||
| permanentImageKey = "stores/" + storeId + "/tables/" + tableId + "/" + UUID.randomUUID() + extension; | ||
|
|
There was a problem hiding this comment.
Validate temp image keys before processing.
tableImageKey is client-supplied; without format checks, arbitrary keys can be passed through. Enforce the expected temp prefix (e.g., temp/tables/) before proceeding.
🛡️ Proposed guard
String tempImageKey = dto.tableImageKey();
if (tempImageKey != null && !tempImageKey.isBlank()) {
+ if (!tempImageKey.startsWith("temp/tables/")) {
+ throw new ImageException(ImageErrorStatus.INVALID_IMAGE_KEY);
+ }
Long tableId = savedTable.getId();
String extension = s3Service.extractExtension(tempImageKey);🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/storetable/service/StoreTableCommandServiceImpl.java`
around lines 88 - 97, Validate the client-supplied temp image key before moving
it: in StoreTableCommandServiceImpl where tempImageKey = dto.tableImageKey() is
used, add a guard that ensures tempImageKey matches the expected temp
prefix/pattern (for example startsWith("temp/tables/") and a simple filename
extension check) before calling s3Service.extractExtension(...) and creating
permanentImageKey; if the check fails, either skip processing or throw a
validation exception (e.g., IllegalArgumentException) so only properly-prefixed
temp keys are accepted.
💡 작업 개요
1. FEAT: 가게 테이블 이미지 선 업로드 (POST)
POST /api/v1/stores/{storeId}/tables/images2. REFACTOR: 가게 테이블 생성 (POST)
POST /api/v1/stores/{storeId}/tablesstores/{storeId}/tables/{tableId}/tableId를 경로에 포함하기 위해 생성한 테이블을 먼저 저장 후, 이미지를 이동하고 경로를 업데이트 하는 방식으로 수정.3. REFACTOR: 가게 테이블 삭제 (DELETE)
DELETE /api/v1/stores/{storeId}/tables/{tableId}✅ 작업 내용
🧪 테스트 내용
📝 기타 참고 사항
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.