fix - #107
Conversation
There was a problem hiding this comment.
Pull request overview
This PR modernizes background archive/extraction/copy/move services and UI search by replacing LocalBroadcastManager progress/result signaling with a coroutine-based EventBus, adding wakelocks + throttled notification updates for long-running operations, and introducing chip-based search filtering in the app’s SearchView. It also includes substantial localization updates, a donation/share configuration tweak, and manifest/build adjustments for new behaviors and release packaging.
Changes:
- Replaced LocalBroadcastManager broadcasts with a
SharedFlow-basedEventBusand updated multiple services/fragments/activities accordingly. - Added
SearchViewfilter chips (Archive/Audio/Video/Document) and expanded MIME/extension classification helpers used by fast search. - Updated many string resources/translations, Open-With intent filters, app version, and NDK ABI filters.
Reviewed changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/ps/res/values/strings.xml | Adds PS source-set override for donation flag. |
| app/src/main/res/values/strings.xml | Adds Play Store share URL + donation flag config. |
| app/src/main/res/values-zh-rCN/strings.xml | Updates Simplified Chinese translations + adds plurals/rename. |
| app/src/main/res/values-vi/strings.xml | Adds Vietnamese translations file. |
| app/src/main/res/values-uk/strings.xml | Updates Ukrainian strings + adds plurals/rename. |
| app/src/main/res/values-tr/strings.xml | Updates Turkish strings + adds plurals/rename. |
| app/src/main/res/values-ta/strings.xml | Adds Tamil translations file. |
| app/src/main/res/values-ru/strings.xml | Updates Russian strings + adds plurals/rename. |
| app/src/main/res/values-pt/strings.xml | Updates Portuguese strings + adds plurals/rename. |
| app/src/main/res/values-pt-rBR/strings.xml | Updates Brazilian Portuguese strings + adds plurals/rename. |
| app/src/main/res/values-ko/strings.xml | Updates Korean strings + adds plurals/rename. |
| app/src/main/res/values-it/strings.xml | Updates Italian strings + adds plurals/rename. |
| app/src/main/res/values-fr/strings.xml | Updates French strings + adds missing items/plurals/rename. |
| app/src/main/res/values-es/strings.xml | Adds Spanish translations file. |
| app/src/main/res/values-de/strings.xml | Updates German strings + adds plurals/rename. |
| app/src/main/res/values-bg/strings.xml | Adds Bulgarian plurals/rename. |
| app/src/main/res/values-ar/strings.xml | Updates Arabic strings + adds plurals/rename. |
| app/src/main/res/layout/fragment_archive.xml | Removes archive filter chip row and tweaks margins. |
| app/src/main/res/layout/activity_main.xml | Adds search filter chip row above search history list. |
| app/src/main/java/com/wirelessalien/zipxtract/service/Update7zService.kt | Adds service coroutine scope, wakelock, EventBus emissions, throttled notif updates. |
| app/src/main/java/com/wirelessalien/zipxtract/service/ExtractRarService.kt | Adds service coroutine scope, wakelock, Zip Slip check, EventBus, throttling, scan helper. |
| app/src/main/java/com/wirelessalien/zipxtract/service/ExtractMultipartZipService.kt | Adds service coroutine scope, wakelock, Zip Slip hardening, EventBus, scan helper. |
| app/src/main/java/com/wirelessalien/zipxtract/service/ExtractMultipart7zService.kt | Adds service coroutine scope, wakelock, Zip Slip hardening, EventBus, scan helper. |
| app/src/main/java/com/wirelessalien/zipxtract/service/ExtractCsArchiveService.kt | Adds service coroutine scope, wakelock, Zip Slip hardening, EventBus, scan helper, bigger buffer. |
| app/src/main/java/com/wirelessalien/zipxtract/service/DeleteFilesService.kt | Adds service coroutine scope, wakelock, scan helper, throttled notif updates, fixes cancel ID. |
| app/src/main/java/com/wirelessalien/zipxtract/service/CopyMoveService.kt | Adds service coroutine scope, wakelock, scan helper, throttled notif updates, fixes cancel ID. |
| app/src/main/java/com/wirelessalien/zipxtract/service/CompressCsArchiveService.kt | Adds service coroutine scope, wakelock, EventBus, scan helper, bigger buffers, throttling. |
| app/src/main/java/com/wirelessalien/zipxtract/service/ArchiveZipService.kt | Adds service coroutine scope, wakelock, EventBus, scan helper, coroutine delay, throttling. |
| app/src/main/java/com/wirelessalien/zipxtract/service/ArchiveTarService.kt | Adds service coroutine scope, wakelock, EventBus, scan helper, bigger buffer, throttling. |
| app/src/main/java/com/wirelessalien/zipxtract/service/ArchiveSplitZipService.kt | Adds service coroutine scope, wakelock, EventBus, scan helper, coroutine delay, throttling. |
| app/src/main/java/com/wirelessalien/zipxtract/service/Archive7zService.kt | Adds service coroutine scope, wakelock, EventBus, scan helper, throttling. |
| app/src/main/java/com/wirelessalien/zipxtract/helper/Searchable.kt | Extends search callback to include filter type + exit flag. |
| app/src/main/java/com/wirelessalien/zipxtract/helper/MimeTypeHelper.kt | Adds file-type extension sets + helper predicates for filter chips. |
| app/src/main/java/com/wirelessalien/zipxtract/helper/FileUtils.kt | Adds chunked MediaScanner scan helper for large batches. |
| app/src/main/java/com/wirelessalien/zipxtract/helper/EventBus.kt | Introduces SharedFlow-based event bus and AppEvent types. |
| app/src/main/java/com/wirelessalien/zipxtract/fragment/SevenZipFragment.kt | Switches to EventBus for update progress + adds extraction bottom sheet/options. |
| app/src/main/java/com/wirelessalien/zipxtract/fragment/PathPickerFragment.kt | Auto-scrolls breadcrumb path chips to the right. |
| app/src/main/java/com/wirelessalien/zipxtract/fragment/FilePickerFragment.kt | Auto-scrolls breadcrumb path chips to the right. |
| app/src/main/java/com/wirelessalien/zipxtract/fragment/ArchiveFragment.kt | Moves extraction progress to EventBus + updates search signature and behavior. |
| app/src/main/java/com/wirelessalien/zipxtract/fragment/AboutFragment.kt | Gates donation visibility via resource flag + uses share_url resource. |
| app/src/main/java/com/wirelessalien/zipxtract/constant/ServiceConstants.kt | Adds EXTRA_ITEMS_TO_EXTRACT constant. |
| app/src/main/java/com/wirelessalien/zipxtract/adapter/FileAdapter.kt | Fixes folder-selection toggling logic against filtered list. |
| app/src/main/java/com/wirelessalien/zipxtract/activity/OpenWithActivity.kt | Improves extraction flow error handling and avoids duplicated dismiss logic. |
| app/src/main/java/com/wirelessalien/zipxtract/activity/MainActivity.kt | Adds global menu handling, filter chips, and routes filterType into fragment search. |
| app/src/main/AndroidManifest.xml | Adds WAKE_LOCK permission + expands Open-With MIME/extension handling. |
| app/build.gradle | Bumps version and restricts NDK ABIs to ARM variants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fun isArchive(file: File): Boolean { | ||
| val extension = file.extension.lowercase(Locale.getDefault()) | ||
| return archiveExtensions.contains(extension) | ||
| } |
| private fun searchAllFiles(query: String?, filterType: String?): Flow<List<FileItem>> = flow { | ||
| val results = getArchiveFiles(query, null) | ||
| emit(results) | ||
| } |
| <!-- General strings --> | ||
| <string name="folder">Folder</string> | ||
| <string name="share">Share App Link</string> | ||
| <string name="share_url">https://play.google.com/store/apps/details?id=com.wirelessalien.zipxtract.ps</string> |
| ~ along with this program. If not, see <https://www.gnu.org/licenses/>. | ||
| --> | ||
| <resources> | ||
| <string name="show_donation">false</string> |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the application's event broadcasting from LocalBroadcastManager to a coroutine-based EventBus, introduces category-based search filtering, adds wake locks to background services, and enhances Zip Slip security checks during extraction. Feedback focuses on several critical issues: potential event loss in the EventBus when the app is in the background, extraction failures for TAR archives containing root entries due to strict path checks, blocking database operations on the main thread in SevenZipFragment, potential UI state mismatches in the search filter, redundant directory traversals in DeleteFilesService, and unreliable file comparisons in FileAdapter.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| sealed class AppEvent { | ||
| data class ExtractionProgress(val progress: Int) : AppEvent() | ||
| data class ExtractionComplete(val dirPath: String) : AppEvent() | ||
| data class ExtractionError(val errorMessage: String) : AppEvent() | ||
| data class ArchiveProgress(val progress: Int) : AppEvent() | ||
| data class ArchiveComplete(val dirPath: String?) : AppEvent() | ||
| data class ArchiveError(val errorMessage: String?) : AppEvent() | ||
| } | ||
|
|
||
| object EventBus { | ||
| private val _events = MutableSharedFlow<AppEvent>( | ||
| extraBufferCapacity = 64, | ||
| onBufferOverflow = BufferOverflow.DROP_OLDEST | ||
| ) | ||
| val events: Flow<AppEvent> = _events.asSharedFlow() | ||
|
|
||
| fun emit(event: AppEvent) { | ||
| _events.tryEmit(event) | ||
| } | ||
| } |
There was a problem hiding this comment.
Architectural Issues with the Global EventBus
There are two major issues with the current EventBus implementation:
1. Potential Event Loss for One-Time Events
The EventBus is implemented using a MutableSharedFlow with replay = 0:
private val _events = MutableSharedFlow<AppEvent>(
extraBufferCapacity = 64,
onBufferOverflow = BufferOverflow.DROP_OLDEST
)When fragments collect these events using flowWithLifecycle(..., Lifecycle.State.STARTED), the collection is actively paused/cancelled when the fragment goes into the background (e.g., user minimizes the app or navigates to another screen), resulting in 0 active collectors.
Because replay = 0, any event emitted by a background service (such as ExtractionComplete or ArchiveComplete) while the app is in the background or during a configuration change (like screen rotation) will be immediately discarded and lost. When the user returns to the app, the progress dialog will hang indefinitely because the completion event was never received.
2. Global Scoping and Unintended Event Consumption
The global EventBus broadcasts events (AppEvent) to all active collectors without any scoping or filtering (e.g., by jobId or archive path).
If multiple fragments are active at the same time (for example, MainFragment and ArchiveFragment inside a ViewPager, or SevenZipFragment on top of the backstack), they will all receive and process the same events. This leads to duplicate Toasts/Snackbars being shown.
More seriously, in SevenZipFragment, receiving an unrelated ArchiveComplete event will trigger a reload of the current archive and show a success Toast, even if the completed archive task was initiated by MainFragment for a completely different file.
Recommendation:
- Add a unique identifier (like
jobIdorarchivePath) to eachAppEventclass, and filter the incoming events in each fragment to only process those meant for that specific fragment/job. - For one-time UI events, consider using a
Channelexposed as aFlow(which buffers events even when there are no active collectors), or use aSharedFlowwithreplay = 1combined with an event wrapper (e.g.,Event<T>) that prevents re-delivery of already handled events.
| val canonicalDstPath = destinationDir.canonicalPath | ||
| val safeDstPath = if (canonicalDstPath.endsWith(File.separator)) canonicalDstPath else canonicalDstPath + File.separator | ||
|
|
||
| TarArchiveInputStream(FileInputStream(file)).use { tarInput -> | ||
| var entry: TarArchiveEntry? = tarInput.nextEntry | ||
| while (entry != null) { | ||
| val outputFile = File(destinationDir, entry.name) | ||
|
|
||
| if (!outputFile.canonicalPath.startsWith(safeDstPath)) { | ||
| throw IOException("Zip Slip detected: ${entry.name}") | ||
| } |
There was a problem hiding this comment.
Issue: Extraction Failure for TAR Archives containing . or ./
In the TAR extraction logic, safeDstPath is defined with a trailing separator:
val canonicalDstPath = destinationDir.canonicalPath
val safeDstPath = if (canonicalDstPath.endsWith(File.separator)) canonicalDstPath else canonicalDstPath + File.separatorAnd the Zip Slip check is:
if (!outputFile.canonicalPath.startsWith(safeDstPath)) {
throw IOException("Zip Slip detected: ${entry.name}")
}Many standard TAR archives contain a . or ./ entry at the root. For these entries, outputFile.canonicalPath will resolve exactly to canonicalDstPath (without a trailing separator). Since canonicalDstPath does not start with safeDstPath (which has a trailing separator), this check will fail and throw a Zip Slip detected exception, completely breaking extraction for these archives.
Recommendation:
Align this check with the correct and robust check used in other parts of this file (e.g., line 474):
val canonicalDstPath = destinationDir.canonicalPath
TarArchiveInputStream(FileInputStream(file)).use { tarInput ->
var entry: TarArchiveEntry? = tarInput.nextEntry
while (entry != null) {
val outputFile = File(destinationDir, entry.name)
if (!outputFile.canonicalPath.startsWith(canonicalDstPath + File.separator) && outputFile.canonicalPath != canonicalDstPath) {
throw IOException("Zip Slip detected: ${entry.name}")
}| private fun startExtractionService(item: ArchiveItem, password: String?, destinationPath: String) { | ||
| val path = archivePath ?: return | ||
| val jobId = fileOperationsDao.addFilesForJob(listOf(path)) | ||
| val itemsToExtract = ArrayList<String>() | ||
| itemsToExtract.add(item.path) | ||
|
|
||
| val intent = Intent(requireContext(), ExtractArchiveService::class.java).apply { | ||
| putExtra(ServiceConstants.EXTRA_JOB_ID, jobId) | ||
| putExtra(ServiceConstants.EXTRA_PASSWORD, password) | ||
| putExtra(ServiceConstants.EXTRA_DESTINATION_PATH, destinationPath) | ||
| putStringArrayListExtra(ServiceConstants.EXTRA_ITEMS_TO_EXTRACT, itemsToExtract) | ||
| } | ||
| ContextCompat.startForegroundService(requireContext(), intent) | ||
| } |
There was a problem hiding this comment.
Issue: Database Operation on the Main Thread
In startExtractionService, fileOperationsDao.addFilesForJob is called directly on the main thread:
val jobId = fileOperationsDao.addFilesForJob(listOf(path))Performing blocking database operations on the main thread can cause UI stutters, freezes, or even crashes (if Room is configured to disallow main thread queries).
Recommendation:
Wrap the database call in withContext(Dispatchers.IO) inside a coroutine launched via lifecycleScope:
private fun startExtractionService(item: ArchiveItem, password: String?, destinationPath: String) {
val path = archivePath ?: return
lifecycleScope.launch {
val jobId = withContext(Dispatchers.IO) {
fileOperationsDao.addFilesForJob(listOf(path))
}
val itemsToExtract = ArrayList<String>()
itemsToExtract.add(item.path)
val intent = Intent(requireContext(), ExtractArchiveService::class.java).apply {
putExtra(ServiceConstants.EXTRA_JOB_ID, jobId)
putExtra(ServiceConstants.EXTRA_PASSWORD, password)
putExtra(ServiceConstants.EXTRA_DESTINATION_PATH, destinationPath)
putStringArrayListExtra(ServiceConstants.EXTRA_ITEMS_TO_EXTRACT, itemsToExtract)
}
ContextCompat.startForegroundService(requireContext(), intent)
}
}| val chipGroup = binding.searchView.findViewById<com.google.android.material.chip.ChipGroup>(R.id.chip_group_search_filter) | ||
| if (chipGroup != null) { | ||
| chipGroup.isSingleSelection = true |
There was a problem hiding this comment.
Issue: Potential UI State Mismatch on Chip Unselection
By default, a ChipGroup with isSingleSelection = true allows the user to click the currently selected chip to uncheck it, leaving 0 chips checked. If the user does this, checkedIds in setOnCheckedStateChangeListener will be empty, and the filter won't be updated, leading to a mismatch between the UI (no chips selected) and the actual search filter state.
Recommendation:
Set chipGroup.isSelectionRequired = true to prevent the user from unselecting the active chip:
val chipGroup = binding.searchView.findViewById<com.google.android.material.chip.ChipGroup>(R.id.chip_group_search_filter)
if (chipGroup != null) {
chipGroup.isSingleSelection = true
chipGroup.isSelectionRequired = true| private fun deleteFiles(files: List<File>) { | ||
| val totalFilesCount = countTotalFiles(files) | ||
| var deletedFilesCount = 0 | ||
| val pathsToScan = mutableListOf<String>() | ||
|
|
||
| fun deleteFile(file: File) { | ||
| pathsToScan.add(file.absolutePath) | ||
| if (file.isDirectory) { | ||
| file.listFiles()?.forEach { deleteFile(it) } | ||
| var totalFilesCount = 0 | ||
| files.forEach { file -> | ||
| file.walkTopDown().forEach { | ||
| totalFilesCount++ | ||
| updateNotification(0, totalFilesCount) | ||
| } | ||
| file.delete() | ||
| deletedFilesCount++ | ||
| updateNotification(deletedFilesCount, totalFilesCount) | ||
| } | ||
| updateNotification(0, totalFilesCount, true) | ||
|
|
||
| var deletedFilesCount = 0 | ||
| val pathsToScan = mutableSetOf<String>() | ||
|
|
||
| for (file in files) { | ||
| deleteFile(file) | ||
| file.walkBottomUp().forEach { currentFile -> | ||
| pathsToScan.add(currentFile.absolutePath) | ||
| if (currentFile.delete()) { | ||
| deletedFilesCount++ | ||
| updateNotification(deletedFilesCount, totalFilesCount) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (pathsToScan.isNotEmpty()) { | ||
| MediaScannerConnection.scanFile(this, pathsToScan.toTypedArray(), null, null) | ||
| } | ||
| FileUtils.scanFiles(this, pathsToScan) | ||
|
|
||
| stopForegroundService() | ||
| stopSelf() | ||
| } | ||
|
|
||
| private fun countTotalFiles(files: List<File>): Int { | ||
| var count = 0 | ||
| for (file in files) { | ||
| if (file.isDirectory) { | ||
| count += countTotalFiles(file.listFiles()?.toList() ?: emptyList()) | ||
| } else { | ||
| count++ | ||
| } | ||
| } | ||
| return count | ||
| } |
There was a problem hiding this comment.
Optimization: Double Directory Traversal
Currently, the service traverses the entire directory tree twice: once top-down to count the files, and once bottom-up to delete them. This doubles the disk I/O, which can be extremely slow on Android devices with large directory structures.
Recommendation:
Traverse the directory tree only once to collect all files into a list, then delete them in reverse order (which naturally deletes children before parents):
private fun deleteFiles(files: List<File>) {
val filesToDelete = files.flatMap { it.walkTopDown().toList() }
val totalFilesCount = filesToDelete.size
updateNotification(0, totalFilesCount, true)
var deletedFilesCount = 0
val pathsToScan = mutableSetOf<String>()
for (i in filesToDelete.indices.reversed()) {
val currentFile = filesToDelete[i]
pathsToScan.add(currentFile.absolutePath)
if (currentFile.delete()) {
deletedFilesCount++
updateNotification(deletedFilesCount, totalFilesCount)
}
}
FileUtils.scanFiles(this, pathsToScan)
stopForegroundService()
}| for ((index, item) in filteredFiles.withIndex()) { | ||
| if (item.file.parentFile == directory) { | ||
| toggleSelection(index) | ||
| } | ||
| } |
There was a problem hiding this comment.
Issue: Unreliable File Comparison
Comparing File objects directly using == compares their path strings. This can fail if one path is absolute and the other is relative, or if they use different root representations (e.g., /sdcard/ vs /storage/emulated/0/).
Recommendation:
Compare their absolute paths instead:
| for ((index, item) in filteredFiles.withIndex()) { | |
| if (item.file.parentFile == directory) { | |
| toggleSelection(index) | |
| } | |
| } | |
| for ((index, item) in filteredFiles.withIndex()) { | |
| if (item.file.parentFile?.absolutePath == directory.absolutePath) { | |
| toggleSelection(index) | |
| } | |
| } |
No description provided.