Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import android.content.Context
import android.net.Uri
import android.util.Base64.decode
import android.util.Log
import androidx.core.net.toUri
import androidx.work.WorkInfo
import androidx.work.WorkManager
import com.example.metasearch.core.common.utils.toFile
Expand All @@ -21,8 +20,11 @@ import com.example.metasearch.core.room.api.dao.AnalyzedImageDao
import com.example.metasearch.core.room.api.entity.AnalyzedImageEntity
import dagger.hilt.android.qualifiers.ApplicationContext
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.async
import kotlinx.coroutines.awaitAll
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.supervisorScope
import kotlinx.coroutines.withContext
import okhttp3.MediaType.Companion.toMediaTypeOrNull
import okhttp3.MultipartBody
Expand Down Expand Up @@ -73,17 +75,15 @@ class ImageAnalysisRepositoryImpl @Inject constructor(
Log.d(tag, "7. 추가할 이미지 수: ${addUris.size}")

if (addUris.isNotEmpty()) {
val allSuccessfulPaths = mutableListOf<String>()
val allSuccessfulData = mutableListOf<Pair<String, String>>()

addUris.chunked(chunkSize).forEachIndexed { index, chunk ->
val successfulInChunk = uploadOnlyImageChunk(context, index, chunk, dbName)
allSuccessfulPaths.addAll(successfulInChunk)

if (successfulInChunk.size != chunk.size) return@forEachIndexed
addUris.chunked(chunkSize).forEach { chunk ->
val successfulInChunk = uploadOnlyImageChunk(context, chunk, dbName)
allSuccessfulData.addAll(successfulInChunk)
}

if (allSuccessfulPaths.isNotEmpty()) {
processAnalysisFinish(allSuccessfulPaths, dbName)
if (allSuccessfulData.isNotEmpty()) {
processAnalysisFinish(allSuccessfulData, dbName)
}
} else {
Log.d(tag, "8. 추가할 이미지가 없어 종료함")
Expand All @@ -98,65 +98,52 @@ class ImageAnalysisRepositoryImpl @Inject constructor(
deletePaths.forEachIndexed { index, pathString ->
Log.d(tag, "이미지 삭제 중 (${index + 1}/${deletePaths.size}): $pathString")

val uri = pathString.toUri()
val originalFileName = galleryRepository.getFileName(uri)
Log.d(tag, "Web 서버로 보낼 파일명: $originalFileName")

val fileName = pathString.substringAfterLast('/')
Log.d(tag, "AI 서버로 보낼 파일명: $fileName")
val savedFileName = analyzedImageDao.getFileNameByPath(pathString)
Log.d(tag, "서버로 보낼 파일명: $savedFileName")

val finalFileName = if (originalFileName != null) {
"$originalFileName.jpg"
} else {
"$fileName.jpg"
val webResult = runCatching {
webService.uploadWebDeleteImage(DeleteImageRequest(dbName, savedFileName ?: "unknown.jpg"))
}

val webResponse = runCatching { webService.uploadWebDeleteImage(DeleteImageRequest(dbName, finalFileName)) }
webResponse.onFailure {
Log.e(tag, "Web 삭제 실패 원인: ${it.message}")
val aiPart = MultipartBody.Part.createFormData("deleteImage", savedFileName, "".toRequestBody())
val aiResult = runCatching {
aiService.uploadDeleteImage(aiPart, dbName.toRequestBody())
}

val aiPart = MultipartBody.Part.createFormData("deleteImage", fileName, "".toRequestBody())
val aiSuccess = runCatching { aiService.uploadDeleteImage(aiPart, dbName.toRequestBody()) }.isSuccess

if (webResponse.isSuccess && aiSuccess) {
if (webResult.isSuccess && aiResult.isSuccess) {
analyzedImageDao.deletePath(pathString)
Log.d(tag, "삭제 성공: $pathString")
} else {
Log.e(tag, "삭제 실패 (Web: $webResponse, AI: $aiSuccess): $pathString")
Log.e(tag, "삭제 실패 (Web: $webResult, AI: $aiResult): $pathString")
}
}
}

private suspend fun uploadOnlyImageChunk(context: Context, index: Int, chunk: List<Uri>, dbName: String): List<String> {
Log.d(tag, "청크 전송 중: ${index + 1}번째 (${chunk.size}개)")
val successfulPaths = mutableListOf<String>()

for (uri in chunk) {
if (uploadSingleImage(context, uri, dbName)) {
successfulPaths.add(uri.toString())
} else {
Log.e(tag, "업로드 실패: $uri")
break
private suspend fun uploadOnlyImageChunk(
context: Context,
chunk: List<Uri>,
dbName: String,
): List<Pair<String, String>> = supervisorScope {
chunk.map { uri ->
async {
val fileName = galleryRepository.getFileName(uri) ?: "unknown.jpg"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find the implementation of getFileName
ast-grep --pattern $'fun getFileName($$$): String? {
  $$$
}'

Repository: komodgn/meta-android

Length of output: 46


🏁 Script executed:

# Also search for getFileName method definitions more broadly
rg -t kt 'fun getFileName' -A 10

Repository: komodgn/meta-android

Length of output: 90


🏁 Script executed:

# Check if there's a GalleryRepository interface definition
rg -t kt 'interface GalleryRepository|class.*GalleryRepository' -A 20

Repository: komodgn/meta-android

Length of output: 90


🏁 Script executed:

# Search for getFileName without type constraints
rg 'fun getFileName' -A 10

Repository: komodgn/meta-android

Length of output: 3032


🏁 Script executed:

# Find all Kotlin files and their locations
fd -e kt -e java . core/

Repository: komodgn/meta-android

Length of output: 7521


🏁 Script executed:

# Look at the ImageAnalysisRepositoryImpl file directly
cat -n core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt | head -150 | tail -50

Repository: komodgn/meta-android

Length of output: 2537


getFileName can legitimately return null—ensure null uploads don't accumulate.

The implementation queries MediaStore.Images.Media.DISPLAY_NAME, which can return null if the query fails or the URI yields no results (e.g., invalid URI, deleted file, missing display name, or permission issues). The fallback to "unknown.jpg" is a reasonable defensive mechanism, and this pattern is consistently applied elsewhere in the code (e.g., line 105 in the delete operation). However, if null returns occur frequently in normal operation with valid URIs, files would be uploaded without proper names, defeating the purpose of the fix. Monitor logs to ensure this fallback is rare and investigate if it occurs unexpectedly.

🤖 Prompt for AI Agents
In
@core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt
around line 129, The fallback to "unknown.jpg" when
galleryRepository.getFileName(uri) returns null can mask frequent failures;
update ImageAnalysisRepositoryImpl where fileName is assigned to detect the null
case from getFileName(uri) and (1) log a warning including the uri and context
using the existing logger, (2) attempt a better deterministic fallback (e.g.,
derive a name from the uri path or timestamp) before using "unknown.jpg", and
(3) ensure any upload/delete code that relies on this name treats "unknown.jpg"
as exceptional; reference getFileName, galleryRepository, and the fileName usage
so you can add logging and the alternative fallback in the same assignment spot.

if (uploadSingleImage(context, uri, dbName, fileName)) uri.toString() to fileName else null
}
}
return successfulPaths
}.awaitAll().filterNotNull()
}

private suspend fun uploadSingleImage(context: Context, uri: Uri, dbName: String): Boolean {
private suspend fun uploadSingleImage(context: Context, uri: Uri, dbName: String, fileName: String): Boolean {
val tempFile = uri.toFile(context)

return try {
val originalFileName = galleryRepository.getFileName(uri) ?: "unknown.jpg"

val webImagePart = createMultipartBodyPartFromUri(context, "image", uri, originalFileName)
val aiImagePart = createMultipartBodyPartFromUri(context, "addImage", uri, originalFileName)
val webImagePart = createMultipartBodyPartFromUri(context, "image", uri, fileName)
val aiImagePart = createMultipartBodyPartFromUri(context, "addImage", uri, fileName)
val dbNameBody = dbName.toRequestBody("text/plain".toMediaTypeOrNull())

webService.uploadWebAddImage(webImagePart, dbName)
aiService.uploadAddImage(aiImagePart, dbNameBody)

Log.d(tag, "서버 전송 성공 (파일명: $originalFileName): $uri")
Log.d(tag, "서버 전송 성공 (파일명: $fileName): $uri")
true
} catch (e: Exception) {
Log.e(tag, "서버 전송 실패 원인: ${e.message}", e)
Expand All @@ -166,7 +153,7 @@ class ImageAnalysisRepositoryImpl @Inject constructor(
}
}

private suspend fun processAnalysisFinish(successfulPaths: List<String>, dbName: String) {
private suspend fun processAnalysisFinish(successfulPaths: List<Pair<String, String>>, dbName: String) {
val lastIndex = personIndexDataSource.getLastPersonIndex()

Log.d(tag, lastIndex.toString())
Expand All @@ -182,31 +169,38 @@ class ImageAnalysisRepositoryImpl @Inject constructor(
.mapNotNull { it.imageName?.filter { c -> c.isDigit() }?.toIntOrNull() }
.maxOrNull() ?: lastIndex

if (newMax > lastIndex) {
personIndexDataSource.setLastPersonIndex(newMax)
}

response.images.forEach { person ->
runCatching {
if (person.isFaceExit && person.imageName != null && person.imageBytes != null) {
val decodedBytes = decode(person.imageBytes, android.util.Base64.DEFAULT)

val existingPersonId = personRepository.getPersonIdByImageName(person.imageName!!)

if (existingPersonId != null) {
Log.d(tag, "기존 인물 매핑 성공: ${person.imageName} -> ID:$existingPersonId")
personRepository.addFaceToExistingPerson(existingPersonId, person.imageName!!, decodedBytes)
analyzedImageDao.runInTransaction {
if (newMax > lastIndex) {
personIndexDataSource.setLastPersonIndex(newMax)
}

response.images.forEach { person ->
runCatching {
if (person.isFaceExit && person.imageName != null && person.imageBytes != null) {
val decodedBytes = decode(person.imageBytes, android.util.Base64.DEFAULT)

val existingPersonId = personRepository.getPersonIdByImageName(person.imageName!!)

if (existingPersonId != null) {
Log.d(tag, "기존 인물 매핑 성공: ${person.imageName} -> ID:$existingPersonId")
personRepository.addFaceToExistingPerson(existingPersonId, person.imageName!!, decodedBytes)
} else {
Log.d(tag, "새로운 인물 생성: ${person.imageName}")
personRepository.addAnalyzedPerson(person.imageName!!, decodedBytes)
}
} else {
Log.d(tag, "새로운 인물 생성: ${person.imageName}")
personRepository.addAnalyzedPerson(person.imageName!!, decodedBytes)
Log.w(tag, "저장 스킵됨: 얼굴없음(${!person.isFaceExit}) 또는 데이터가 null임")
}
} else {
Log.w(tag, "저장 스킵됨: 얼굴없음(${!person.isFaceExit}) 또는 데이터가 null임")
}
}.onFailure { e -> Log.e(tag, "인물 개별 저장 실패: ${person.imageName}", e) }
}.onFailure { e -> Log.e(tag, "인물 개별 저장 실패: ${person.imageName}", e) }
}

val entries = successfulPaths.map { (path, fileName) ->
AnalyzedImageEntity(imagePath = path, fileName = fileName)
}
analyzedImageDao.insertAllPaths(entries)
}
Comment on lines +172 to 201
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Transaction will not work as implemented.

The analyzedImageDao.runInTransaction call does not provide actual transactional guarantees (see the DAO review comment). This means that if any operation within this block fails, the other operations may still commit, potentially causing data inconsistency between:

  • Person index updates
  • Person/face additions
  • Analyzed image path insertions

This issue is addressed in the review comment for AnalyzedImageDao.kt. Please apply the suggested fix using database.withTransaction { }.

🤖 Prompt for AI Agents
In
@core/data/impl/src/main/java/com/example/metasearch/core/data/impl/repository/ImageAnalysisRepositoryImpl.kt
around lines 172-201, The current use of analyzedImageDao.runInTransaction does
not provide real transactional guarantees; replace that block with a call to the
Room database's withTransaction (e.g., database.withTransaction { ... }) so the
personIndexDataSource.setLastPersonIndex, personRepository add/get methods and
analyzedImageDao.insertAllPaths run inside a single transaction; move the entire
block currently inside analyzedImageDao.runInTransaction into
database.withTransaction { } and ensure you call DAO methods (analyzedImageDao)
and repository operations from within that transaction context so failures roll
back atomically.

successfulPaths.forEach { analyzedImageDao.insertPath(AnalyzedImageEntity(imagePath = it)) }
Log.d(tag, "전체 분석 및 DB 반영 완료")

Log.d(tag, "트랜잭션 완료: 인물 및 경로(${successfulPaths.size})개 저장 성공")
}
.onFailure { Log.e(tag, "최종 finish 실패: ${it.message}") }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,28 @@ import androidx.room.Dao
import androidx.room.Insert
import androidx.room.OnConflictStrategy
import androidx.room.Query
import androidx.room.Transaction
import com.example.metasearch.core.room.api.entity.AnalyzedImageEntity

@Dao
interface AnalyzedImageDao {
@Insert(onConflict = OnConflictStrategy.REPLACE)
suspend fun insertPath(path: AnalyzedImageEntity): Long

@Insert(onConflict = OnConflictStrategy.REPLACE)
suspend fun insertAllPaths(entities: List<AnalyzedImageEntity>)

@Query("DELETE FROM analyzed_images WHERE image_path = :path")
suspend fun deletePath(path: String): Int

@Query("SELECT image_path FROM analyzed_images")
suspend fun getAllAnalyzedPaths(): List<String>

@Query("SELECT file_name FROM analyzed_images WHERE image_path = :path LIMIT 1")
suspend fun getFileNameByPath(path: String): String?

@Transaction
suspend fun runInTransaction(action: suspend () -> Unit) {
action()
}
Comment on lines +27 to +30
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Room @transaction does not wrap arbitrary lambda blocks.

The @Transaction annotation on runInTransaction will not provide transactional semantics for the action lambda. Room's @Transaction only works for database operations directly invoked within the annotated DAO method—it does not wrap arbitrary suspend lambdas passed as parameters.

To execute a transaction with multiple operations, use database.withTransaction { } from the repository layer or define a concrete @Transaction DAO method that calls multiple DAO operations directly.

🔎 Recommended fix using RoomDatabase.withTransaction

Remove this method from the DAO:

-    @Transaction
-    suspend fun runInTransaction(action: suspend () -> Unit) {
-        action()
-    }

In the repository (ImageAnalysisRepositoryImpl.kt), inject the AppDatabase instance and use:

// Add to constructor
private val database: AppDatabase

// In processAnalysisFinish
database.withTransaction {
    // existing transaction logic
}

Or define a proper @Transaction DAO method that directly invokes other DAO operations.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
@core/room/api/src/main/java/com/example/metasearch/core/room/api/dao/AnalyzedImageDao.kt
around lines 27-30, The runInTransaction(suspend () -> Unit) DAO method is
ineffective because Room’s @Transaction does not wrap lambdas; remove the
runInTransaction method from AnalyzedImageDao and instead perform transactional
work with AppDatabase.withTransaction from the repository layer (inject
AppDatabase into ImageAnalysisRepositoryImpl and wrap the existing transaction
logic in database.withTransaction { ... }) or alternatively replace the DAO
helper with a concrete @Transaction DAO method that directly calls the required
DAO operations (e.g., a new @Transaction method that orchestrates the multiple
DAO calls).

}
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ data class AnalyzedImageEntity(

@ColumnInfo(name = "image_path")
val imagePath: String,

@ColumnInfo(name = "file_name")
val fileName: String,
)
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import com.example.metasearch.core.room.api.entity.PersonEntity
FaceEntity::class,
AnalyzedImageEntity::class,
],
version = 3,
version = 4,
exportSchema = false,
)
abstract class AppDatabase : RoomDatabase() {
Expand All @@ -26,6 +26,12 @@ abstract class AppDatabase : RoomDatabase() {
db.execSQL("ALTER TABLE persons ADD COLUMN representative_face INTEGER DEFAULT NULL")
}
}

val MIGRATION_3_4 = object : Migration(3, 4) {
override fun migrate(db: SupportSQLiteDatabase) {
db.execSQL("ALTER TABLE analyzed_images ADD COLUMN file_name TEXT NOT NULL DEFAULT 'unknown.jpg'")
}
}
}

abstract fun personDao(): PersonDao
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ object DatabaseModule {
context,
AppDatabase::class.java,
DATABASE_NAME,
).build()
)
.addMigrations(AppDatabase.MIGRATION_1_2, AppDatabase.MIGRATION_3_4)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Search for MIGRATION_2_3 and migration definitions
rg -n "MIGRATION_2_3|MIGRATION_[0-9]_[0-9]" --type kotlin -A 2

Repository: komodgn/meta-android

Length of output: 1473


🏁 Script executed:

# Find AppDatabase file to check all migration definitions
fd -e "kt" "AppDatabase" --exec grep -l "MIGRATION" {} \;

Repository: komodgn/meta-android

Length of output: 156


🏁 Script executed:

# Get the DatabaseModule.kt file context around line 29
fd -e "kt" "DatabaseModule" --exec cat -n {} \;

Repository: komodgn/meta-android

Length of output: 1722


🏁 Script executed:

# Get AppDatabase class definition to see version annotation
cat -n core/room/impl/src/main/java/com/example/metasearch/core/room/impl/database/AppDatabase.kt | head -40

Repository: komodgn/meta-android

Length of output: 1748


🏁 Script executed:

# Search for any reference to database version or migration fallback strategies
rg -n "version|Schema|fallbackToDestructiveMigration" --type kotlin core/room/impl/src/main/java/com/example/metasearch/core/room/impl/database/

Repository: komodgn/meta-android

Length of output: 295


Add missing MIGRATION_2_3 to complete the migration chain.

The database schema version is 4, but the migration path is broken: MIGRATION_1_2 (1→2) and MIGRATION_3_4 (3→4) are defined, but MIGRATION_2_3 is missing. Any production database at version 2 will fail to migrate to version 4. Define MIGRATION_2_3 or document why the gap exists and that no databases are at version 2.

🤖 Prompt for AI Agents
In
@core/room/impl/src/main/java/com/example/metasearch/core/room/impl/di/DatabaseModule.kt
around line 29, The migration chain is incomplete: update the call to
addMigrations on the Room database builder to include AppDatabase.MIGRATION_2_3
between MIGRATION_1_2 and MIGRATION_3_4 (i.e.,
.addMigrations(AppDatabase.MIGRATION_1_2, AppDatabase.MIGRATION_2_3,
AppDatabase.MIGRATION_3_4)); if MIGRATION_2_3 intentionally doesn’t exist,
either implement AppDatabase.MIGRATION_2_3 with the schema changes from v2→v3 or
add a clear comment/validation that no production DBs are at version 2 and
handle/throw a descriptive error during initialization.

.build()
}

@Provides
Expand Down