Conversation
- toFile() 호출 제거 - 더미 바디로 서버의 파일 수신 규격을 준수하도록 변경
📝 WalkthroughWalkthroughAdds a serializable Changes
sequenceDiagram
autonumber
participant Repo as ImageAnalysisRepository
participant DB as LocalDatabase
participant Web as WebService
participant AI as AIService
Repo->>DB: load gallery paths
DB-->>Repo: gallery paths
Repo->>DB: load analyzed paths
DB-->>Repo: existing paths
Repo->>Repo: compute missing / deletion candidates
alt deletion candidates exist
loop per image
Repo->>Web: POST /deleteImage (DeleteImageRequest)
Web-->>Repo: web success/fail
Repo->>AI: POST uploadDeleteImage (MultipartBody.Part)
AI-->>Repo: ai success/fail
alt web AND ai success
Repo->>DB: remove path entry
DB-->>Repo: deleted
else any failure
Repo-->>Repo: log combined failure, retain DB entry
end
end
else none
Repo-->>Repo: log no deletions
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt (3)
100-100: Consider defensive filename extraction.While Android gallery URIs should always contain '/', adding a fallback would make the code more robust against unexpected path formats in the database.
🔎 Proposed enhancement for defensive coding
- val fileName = pathString.substringAfterLast('/') + val fileName = pathString.substringAfterLast('/', "unknown_image.jpg").ifEmpty { "unknown_image.jpg" }This ensures a valid filename even if the path format is unexpected.
108-113: Consider adding retry limits for failed deletions in production.The current implementation will retry failed deletions on every
runFullAnalysiscall. While this ensures eventual consistency, it could lead to repeated failures for permanently inaccessible images (e.g., deleted from server but failed to delete from local DB due to network issues).For production resilience, consider:
- Adding a retry counter to
AnalyzedImageEntity- Skipping deletion attempts after N failures
- Or implementing exponential backoff for retries
This is beyond the scope of the current bottleneck fix but would improve long-term maintainability.
9-9: PR description inconsistency:toFile()not fully removed.The PR description states "toFile() 호출 제거" (Remove toFile() calls), but
toFile()is still used in:
- Line 133:
uploadSingleImagefunction- Line 211:
createMultipartBodyPartFromUrifunctionThe removal only applies to the
deleteMissingImagesfunction, which is correct for fixing the bottleneck when analyzing deleted images. However, the description could be more precise: "Remove toFile() calls from delete operations" to avoid confusion.Consider updating the PR description to clarify the scope of the change, or documenting why
toFile()is safe in upload operations (where files must exist) but problematic in delete operations (where files are already gone).Also applies to: 133-133, 211-211
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (1)
core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt (1)
100-103: Fix form field name mismatch in WebService delete call.The
fileNamePartis created with the form field name"deleteImage", which matchesAIService.uploadDeleteImagebut notWebService.uploadWebDeleteImage. The WebService endpoint expects the part field to be named"filename"(inferred from the unannotated@Part filenameparameter), not"deleteImage".This will cause the WebService delete to fail because the server receives a part with an unexpected field name.
Create two separate parts:
- For WebService: use field name
"filename"- For AIService: use field name
"deleteImage"Or verify with the backend team that WebService actually accepts
"deleteImage"as an alternative field name.Likely an incorrect or invalid review comment.
| Log.d(tag, "1. runFullAnalysis 함수 실행") | ||
|
|
||
| val currentGalleryUris = galleryRepository.getAllGalleryImages() | ||
| Log.d(tag, "2. 갤러리 이미지 로드 완료: ${currentGalleryUris.size}개") | ||
| val currentGalleryUrisString = currentGalleryUris.map { it.toString() } | ||
|
|
||
| val alreadyAnalyzedPaths = analyzedImageDao.getAllAnalyzedPaths() | ||
| Log.d(tag, "3. 기존 분석 경로 로드 완료: ${alreadyAnalyzedPaths.size}개") | ||
| val dbName = databaseNameRepository.getPersistentDeviceDatabaseName() | ||
|
|
||
| Log.d(tag, "5. 삭제 로직 시작") | ||
| deleteMissingImages(alreadyAnalyzedPaths, currentGalleryUrisString, dbName) | ||
| Log.d(tag, "6. 삭제 로직 완료") | ||
|
|
||
| val addUris = currentGalleryUris.filter { uri -> | ||
| uri.toString() !in alreadyAnalyzedPaths | ||
| } | ||
| Log.d(tag, "7. 추가할 이미지 수: ${addUris.size}") |
There was a problem hiding this comment.
Fix the inconsistent log numbering sequence.
The debug logs skip from step "3" to step "5", missing step "4". This inconsistency could confuse developers tracing execution flow.
🔎 Proposed fix to correct the numbering
- Log.d(tag, "5. 삭제 로직 시작")
+ Log.d(tag, "4. 삭제 로직 시작")
deleteMissingImages(alreadyAnalyzedPaths, currentGalleryUrisString, dbName)
- Log.d(tag, "6. 삭제 로직 완료")
+ Log.d(tag, "5. 삭제 로직 완료")
val addUris = currentGalleryUris.filter { uri ->
uri.toString() !in alreadyAnalyzedPaths
}
- Log.d(tag, "7. 추가할 이미지 수: ${addUris.size}")
+ Log.d(tag, "6. 추가할 이미지 수: ${addUris.size}")And update line 87:
} else {
- Log.d(tag, "8. 추가할 이미지가 없어 종료함")
+ Log.d(tag, "7. 추가할 이미지가 없어 종료함")
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt
around lines 54 to 71, the debug log numbering skips "4" (it goes 1,2,3,5...).
Rename the log strings so the sequence is consecutive: change "5. 삭제 로직 시작" →
"4. 삭제 로직 시작", "6. 삭제 로직 완료" → "5. 삭제 로직 완료", and "7. 추가할 이미지 수" → "6. 추가할 이미지
수"; also scan nearby logs (e.g., around line 87) and adjust any further step
numbers to maintain a consistent incremental sequence.
- 필요없는 멀티파트 제거 - 라우팅 경로 수정
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 Android CI Summary
Step Results:
- Debug Build: ❌ Failure (0s)
- Code Style Check: ✅ Success (44s)
Total Time: 44s
See the Actions Log for details.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
core/network/src/main/java/com/example/metasearch/core/network/request/DeleteImageRequest.kt (1)
6-9: Consider a more descriptive property name.The property
deleteImagecould be more explicit about what it contains. Based on the PR context indicating this replaces file-based multipart with a "dummy body," this likely holds a filename or path string rather than image data itself. Consider renaming todeleteImageNameordeleteImagePathfor clarity.🔎 Proposed refactor for clearer naming
@Serializable data class DeleteImageRequest( val dbName: String, - val deleteImage: String, + val deleteImageName: String, )
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/network/src/main/java/com/example/metasearch/core/network/request/DeleteImageRequest.ktcore/network/src/main/java/com/example/metasearch/core/network/service/AIService.ktcore/network/src/main/java/com/example/metasearch/core/network/service/WebService.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (2)
core/network/src/main/java/com/example/metasearch/core/network/service/WebService.kt (2)
5-5: LGTM: Import added for new request model.The import of
DeleteImageRequestcorrectly supports the refactored method signature.
69-72: Verify the endpoint path change with the backend.The method has been refactored to use
@Body DeleteImageRequestinstead of multipart. However, the endpoint changed from"android/deleteimg"to"deleteImage/"— the old path no longer exists in the codebase. Confirm with the backend team that the new endpoint path matches the API specification before merging.
| @Multipart | ||
| @POST("android/upload_delete") | ||
| suspend fun uploadDeleteImage( | ||
| @Part("deleteImage") filename: MultipartBody.Part, | ||
| @Part filename: MultipartBody.Part, | ||
| @Part("dbName") dbName: RequestBody, | ||
| ): CommonResponse |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for invocations of uploadDeleteImage
rg -nP --type=kotlin -C5 'uploadDeleteImage\s*\(' -g '!AIService.kt'Repository: komodgn/meta-android
Length of output: 1896
🏁 Script executed:
# Also check for DeleteImageRequest usage in the codebase
rg -nP --type=kotlin 'DeleteImageRequest' -C2Repository: komodgn/meta-android
Length of output: 1997
🏁 Script executed:
# Verify the WebService pattern mentioned in the review
rg -nP --type=kotlin -C5 'uploadWebDeleteImage'Repository: komodgn/meta-android
Length of output: 2945
🏁 Script executed:
# Check if there are other implementations or related methods
rg -nP --type=kotlin 'uploadDelete|deleteImage' -C2Repository: komodgn/meta-android
Length of output: 3080
🏁 Script executed:
# Check the exact WebService signature and any overloads
rg -nP --type=kotlin -A3 'fun uploadWebDeleteImage' core/network/src/main/java/com/example/metasearch/core/network/service/WebService.ktRepository: komodgn/meta-android
Length of output: 167
🏁 Script executed:
# Check if there might be an older version of uploadWebDeleteImage still present
rg -nP --type=kotlin 'uploadWebDeleteImage|uploadDeleteImage' core/network/src/main/java/com/example/metasearch/core/network/service/Repository: komodgn/meta-android
Length of output: 312
🏁 Script executed:
# Get more context from the repository to understand the invocation
cat -n core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt | sed -n '95,115p'Repository: komodgn/meta-android
Length of output: 1265
Critical: Incomplete refactoring across the entire call chain—WebService invocation is broken.
The AIService incompleteness is real, but the larger issue is that the repository layer is invoking webService.uploadWebDeleteImage with the old signature. Line 105 in ImageAnalysisRepositoryImpl.kt calls:
webService.uploadWebDeleteImage(fileNamePart, dbName)But WebService.uploadWebDeleteImage expects a single @Body DeleteImageRequest parameter (lines 70–72). This signature mismatch would cause a runtime/compilation failure.
To complete the refactoring, both services must be aligned and the repository must be updated:
- AIService: Remove
@Multipartand update to accept@Body request: DeleteImageRequest(matching WebService) - Repository: Create
DeleteImageRequestinstance and pass it correctly to both services
The inconsistency across AIService, WebService, and the repository layer breaks the entire delete flow.
🤖 Prompt for AI Agents
In
core/network/src/main/java/com/example/metasearch/core/network/service/AIService.kt
around lines 36 to 41, the uploadDeleteImage method remains multipart but the
WebService and repository were refactored to expect a single DeleteImageRequest
body; update AIService to remove @Multipart and change the signature to suspend
fun uploadDeleteImage(@Body request: DeleteImageRequest): CommonResponse so it
matches WebService, then update the repository call (ImageAnalysisRepositoryImpl
line ~105) to construct a DeleteImageRequest instance and pass that single
request object to both webService.uploadWebDeleteImage(...) and
aiService.uploadDeleteImage(...), ensuring parameter types and imports are
consistent.
TODO: - 저장 방식 수정
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt (1)
56-73: Fix the log numbering sequence (duplicate concern).The debug logs still skip from step "3" to step "5", missing step "4". This was already flagged in a previous review.
Also applies to: 88-90
🧹 Nitpick comments (1)
core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt (1)
120-120: Specify media type for dbName request body.While this will likely work, specifying an explicit media type improves clarity and ensures consistent behavior across different HTTP clients.
🔎 Proposed enhancement
- val aiSuccess = runCatching { aiService.uploadDeleteImage(aiPart, dbName.toRequestBody()) }.isSuccess + val aiSuccess = runCatching { + aiService.uploadDeleteImage(aiPart, dbName.toRequestBody("text/plain".toMediaTypeOrNull())) + }.isSuccess
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-build
🔇 Additional comments (1)
core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt (1)
17-17: LGTM!The import is correctly added to support the new deletion request payload structure.
| val fileName = pathString.substringAfterLast('/') | ||
| Log.d(tag, "AI 서버로 보낼 파일명: $fileName") | ||
|
|
||
| val finalFileName = if (originalFileName != null) { | ||
| "$originalFileName.jpg" | ||
| } else { | ||
| "$fileName.jpg" | ||
| } |
There was a problem hiding this comment.
Fix potential file extension duplication and incorrect extension assignment.
The current logic has critical flaws:
- Extension duplication: If
originalFileNameor the derivedfileNamealready contains an extension (e.g., "photo.jpg"), appending ".jpg" results in "photo.jpg.jpg". - Incorrect extension: Non-JPEG images (PNG, WEBP, etc.) will be incorrectly labeled with a ".jpg" extension, potentially causing server-side lookup failures.
🔎 Proposed fix to handle extensions properly
- val finalFileName = if (originalFileName != null) {
- "$originalFileName.jpg"
- } else {
- "$fileName.jpg"
- }
+ val baseFileName = originalFileName ?: fileName
+ val finalFileName = if (baseFileName.contains('.')) {
+ baseFileName // Already has extension
+ } else {
+ "$baseFileName.jpg" // Add extension only if missing
+ }Or, if you need to preserve the exact original extension:
- val finalFileName = if (originalFileName != null) {
- "$originalFileName.jpg"
- } else {
- "$fileName.jpg"
- }
+ val baseFileName = originalFileName ?: fileName
+ // Use the filename as-is if it has an extension, otherwise default to .jpg
+ val finalFileName = if (baseFileName.lastIndexOf('.') > 0) {
+ baseFileName
+ } else {
+ "$baseFileName.jpg"
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val fileName = pathString.substringAfterLast('/') | |
| Log.d(tag, "AI 서버로 보낼 파일명: $fileName") | |
| val finalFileName = if (originalFileName != null) { | |
| "$originalFileName.jpg" | |
| } else { | |
| "$fileName.jpg" | |
| } | |
| val fileName = pathString.substringAfterLast('/') | |
| Log.d(tag, "AI 서버로 보낼 파일명: $fileName") | |
| val baseFileName = originalFileName ?: fileName | |
| // Use the filename as-is if it has an extension, otherwise default to .jpg | |
| val finalFileName = if (baseFileName.lastIndexOf('.') > 0) { | |
| baseFileName | |
| } else { | |
| "$baseFileName.jpg" | |
| } |
🤖 Prompt for AI Agents
In
core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt
around lines 105–112, the code naively appends ".jpg" causing duplicate
extensions or incorrect labeling for non-JPEGs; instead, extract the filename
base and its existing extension (prefer originalFileName's extension, else
derive from pathString), normalize the extension (lowercase, ensure it starts
with a '.'), and if no extension is found fall back to ".jpg"; then build
finalFileName by combining the base name (without any extension) plus the chosen
extension so you never produce "photo.jpg.jpg" or mislabel PNG/WEBP files.
| val webResponse = runCatching { webService.uploadWebDeleteImage(DeleteImageRequest(dbName, finalFileName)) } | ||
| webResponse.onFailure { | ||
| Log.e(tag, "Web 삭제 실패 원인: ${it.message}") | ||
| } | ||
|
|
||
| if (webSuccess && aiSuccess) analyzedImageDao.deletePath(pathString) | ||
| val aiPart = MultipartBody.Part.createFormData("deleteImage", fileName, "".toRequestBody()) |
There was a problem hiding this comment.
Critical: Web and AI services receive different filenames.
Line 114 sends finalFileName (which includes the .jpg extension) to the web service, while Line 119 sends fileName (path-derived name without the added extension) to the AI service. This inconsistency will likely cause deletion to fail on one service while succeeding on the other, leading to data synchronization issues.
Both services should receive the same filename to ensure consistent deletion across web and AI backends.
🔎 Proposed fix to use consistent filename
val aiPart = MultipartBody.Part.createFormData("deleteImage", fileName, "".toRequestBody())
+ // Use finalFileName for consistency with web service
+ val aiPart = MultipartBody.Part.createFormData("deleteImage", finalFileName, "".toRequestBody())Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt
around lines 114 to 119, the web service call uses finalFileName (with the .jpg
extension) while the AI multipart part uses fileName (path-derived, without the
appended extension), causing inconsistent deletion; update the multipart
creation to use finalFileName for the form-data filename so both web and AI
services receive the identical filename (keep the rest of the
MultipartBody.Part.createFormData call and request body unchanged).
| val webResponse = runCatching { webService.uploadWebDeleteImage(DeleteImageRequest(dbName, finalFileName)) } | ||
| webResponse.onFailure { | ||
| Log.e(tag, "Web 삭제 실패 원인: ${it.message}") | ||
| } | ||
|
|
||
| if (webSuccess && aiSuccess) analyzedImageDao.deletePath(pathString) | ||
| val aiPart = MultipartBody.Part.createFormData("deleteImage", fileName, "".toRequestBody()) | ||
| val aiSuccess = runCatching { aiService.uploadDeleteImage(aiPart, dbName.toRequestBody()) }.isSuccess | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find WebService file
echo "=== Finding WebService file ==="
fd -t f "WebService\.kt"
# Find AIService file
echo -e "\n=== Finding AIService file ==="
fd -t f "AIService\.kt"
# Find DeleteImageRequest
echo -e "\n=== Finding DeleteImageRequest ==="
fd -t f "\.kt" | xargs rg -l "DeleteImageRequest" | head -5Repository: komodgn/meta-android
Length of output: 707
🏁 Script executed:
#!/bin/bash
echo "=== WebService.kt content ==="
cat core/network/src/main/java/com/example/metasearch/core/network/service/WebService.kt
echo -e "\n\n=== AIService.kt content ==="
cat core/network/src/main/java/com/example/metasearch/core/network/service/AIService.kt
echo -e "\n\n=== DeleteImageRequest.kt content ==="
cat core/network/src/main/java/com/example/metasearch/core/network/service/DeleteImageRequest.ktRepository: komodgn/meta-android
Length of output: 4898
🏁 Script executed:
#!/bin/bash
echo "=== Finding DeleteImageRequest ==="
fd -t f "DeleteImageRequest\.kt"
echo -e "\n=== Searching for DeleteImageRequest definition ==="
rg "data class DeleteImageRequest|class DeleteImageRequest" -A 3Repository: komodgn/meta-android
Length of output: 705
Fix API contract mismatch in AI service deletion call.
The AI service's uploadDeleteImage method expects a multipart part named "filename", but the code creates one named "deleteImage". Change:
val aiPart = MultipartBody.Part.createFormData("deleteImage", fileName, "".toRequestBody())to:
val aiPart = MultipartBody.Part.createFormData("filename", fileName, "".toRequestBody())The web service contract (DeleteImageRequest with dbName and deleteImage fields) correctly matches its usage.
🤖 Prompt for AI Agents
In
core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt
around lines 114-121, the multipart part name for the AI service delete call is
incorrect ("deleteImage"); change the multipart part name to "filename" so it
matches the AI service contract (create the MultipartBody.Part with name
"filename" using the existing fileName and empty request body) and keep the web
service call as-is.
1 participant
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.