Skip to content

Commit d90259c

Browse files
Daniel-ADFADaniel-ADFA
committed
fix: address CodeRabbit review findings for plugin build actions
1 parent e64ffea commit d90259c

File tree

6 files changed

+108
-59
lines changed

6 files changed

+108
-59
lines changed

‎app/src/main/java/com/itsaky/androidide/actions/build/PluginBuildActionItem.kt‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import com.itsaky.androidide.resources.R
2020
import com.itsaky.androidide.utils.resolveAttr
2121
import com.itsaky.androidide.viewmodel.BottomSheetViewModel
2222
import com.google.android.material.bottomsheet.BottomSheetBehavior
23+
import androidx.lifecycle.lifecycleScope
24+
import kotlinx.coroutines.CancellationException
2325
import kotlinx.coroutines.Dispatchers
2426
import kotlinx.coroutines.launch
2527
import kotlinx.coroutines.withContext
@@ -109,24 +111,32 @@ class PluginBuildActionItem(
109111
activity.appendBuildOutput("━━━ ${registered.action.name} ━━━")
110112
activity.invalidateOptionsMenu()
111113

112-
actionScope.launch {
113-
execution.output.collect { output ->
114-
val line = when (output) {
115-
is CommandOutput.StdOut -> output.line
116-
is CommandOutput.StdErr -> output.line
117-
is CommandOutput.ExitCode ->
118-
if (output.code != 0) "Process failed with code ${output.code}" else null
119-
}
120-
if (line != null) {
121-
withContext(Dispatchers.Main) {
122-
activity.appendBuildOutput(line)
114+
activity.lifecycleScope.launch(Dispatchers.Default) {
115+
runCatching {
116+
execution.output.collect { output ->
117+
val line = when (output) {
118+
is CommandOutput.StdOut -> output.line
119+
is CommandOutput.StdErr -> output.line
120+
is CommandOutput.ExitCode ->
121+
if (output.code != 0) "Process failed with code ${output.code}" else null
122+
}
123+
if (line != null) {
124+
withContext(Dispatchers.Main) {
125+
activity.appendBuildOutput(line)
126+
}
123127
}
124128
}
125-
}
126129

127-
val result = execution.await()
128-
manager.notifyActionCompleted(pluginId, actionId, result)
129-
withContext(Dispatchers.Main) { resetProgressIfIdle(activity) }
130+
val result = execution.await()
131+
manager.notifyActionCompleted(pluginId, actionId, result)
132+
withContext(Dispatchers.Main) { resetProgressIfIdle(activity) }
133+
}.onFailure { e ->
134+
if (e is CancellationException) {
135+
manager.cancelAction(pluginId, actionId)
136+
throw e
137+
}
138+
withContext(Dispatchers.Main) { resetProgressIfIdle(activity) }
139+
}
130140
}
131141

132142
return true

‎app/src/main/java/com/itsaky/androidide/utils/EditorActivityActions.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,8 @@ class EditorActivityActions {
189189
val action = PluginActionItem(context, menuItem, order++)
190190
registry.registerAction(action)
191191
}
192-
} catch (_: Exception) {
192+
} catch (e: Exception) {
193+
Log.w("plugin_debug", "Failed to register menu items for plugin: ${plugin.javaClass.simpleName}", e)
193194
}
194195
}
195196

‎plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/build/PluginBuildActionManager.kt‎

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import com.itsaky.androidide.plugins.manager.loaders.ManifestBuildAction
1010
import com.itsaky.androidide.plugins.manager.loaders.PluginManifest
1111
import com.itsaky.androidide.plugins.services.CommandExecution
1212
import com.itsaky.androidide.plugins.services.IdeCommandService
13+
import android.util.Log
1314
import java.util.concurrent.ConcurrentHashMap
1415

1516
class PluginBuildActionManager private constructor() {
@@ -20,6 +21,8 @@ class PluginBuildActionManager private constructor() {
2021
private val activeExecutions = ConcurrentHashMap<String, CommandExecution>()
2122

2223
companion object {
24+
private const val TAG = "PluginBuildActionManager"
25+
2326
@Volatile
2427
private var INSTANCE: PluginBuildActionManager? = null
2528

@@ -47,11 +50,13 @@ class PluginBuildActionManager private constructor() {
4750

4851
for ((pluginId, extension) in pluginExtensions) {
4952
val name = pluginNames[pluginId] ?: pluginId
50-
try {
53+
runCatching {
5154
extension.getBuildActions().forEach { action ->
5255
actions.add(RegisteredBuildAction(pluginId, name, action))
5356
}
54-
} catch (_: Throwable) {}
57+
}.onFailure { e ->
58+
Log.w(TAG, "Failed to get build actions from plugin $pluginId", e)
59+
}
5560
}
5661

5762
for ((pluginId, pluginActions) in manifestActions) {
@@ -69,10 +74,12 @@ class PluginBuildActionManager private constructor() {
6974
val hidden = mutableSetOf<String>()
7075

7176
for ((_, extension) in pluginExtensions) {
72-
try {
77+
runCatching {
7378
val requested = extension.toolbarActionsToHide()
7479
hidden.addAll(requested.intersect(ToolbarActionIds.ALL))
75-
} catch (_: Throwable) {}
80+
}.onFailure { e ->
81+
Log.w(TAG, "Failed to get hidden action ids from plugin", e)
82+
}
7683
}
7784

7885
return hidden
@@ -88,45 +95,55 @@ class PluginBuildActionManager private constructor() {
8895

8996
extension?.onActionStarted(actionId)
9097

91-
val execution = commandService.executeCommand(action.command, action.timeoutMs)
92-
val executionKey = "$pluginId:$actionId"
93-
activeExecutions[executionKey] = execution
94-
95-
return execution
98+
return runCatching {
99+
commandService.executeCommand(action.command, action.timeoutMs)
100+
}.onSuccess { execution ->
101+
activeExecutions[executionKey(pluginId, actionId)] = execution
102+
}.onFailure { e ->
103+
extension?.onActionCompleted(actionId, CommandResult.Failure(-1, "", "", e.message, 0))
104+
}.getOrThrow()
96105
}
97106

98107
fun notifyActionCompleted(pluginId: String, actionId: String, result: CommandResult) {
99-
activeExecutions.remove("$pluginId:$actionId")
108+
activeExecutions.remove(executionKey(pluginId, actionId))
100109
pluginExtensions[pluginId]?.onActionCompleted(actionId, result)
101110
}
102111

103112
fun isActionRunning(pluginId: String, actionId: String): Boolean {
104-
return activeExecutions.containsKey("$pluginId:$actionId")
113+
return activeExecutions.containsKey(executionKey(pluginId, actionId))
105114
}
106115

107116
fun cancelAction(pluginId: String, actionId: String): Boolean {
108-
val key = "$pluginId:$actionId"
117+
val key = executionKey(pluginId, actionId)
109118
return activeExecutions.remove(key)?.let {
110119
it.cancel()
111120
true
112121
} ?: false
113122
}
114123

115124
fun cleanupPlugin(pluginId: String) {
116-
activeExecutions.entries.removeAll { it.key.startsWith("$pluginId:") }
125+
activeExecutions.entries.removeAll { (key, execution) ->
126+
if (key.startsWith("$pluginId:")) {
127+
execution.cancel()
128+
true
129+
} else false
130+
}
117131
pluginExtensions.remove(pluginId)
118132
manifestActions.remove(pluginId)
119133
pluginNames.remove(pluginId)
120134
}
121135

136+
private fun executionKey(pluginId: String, actionId: String) = "$pluginId:$actionId"
137+
122138
private fun findAction(pluginId: String, actionId: String): PluginBuildAction? {
123-
pluginExtensions[pluginId]?.let { ext ->
124-
try {
125-
return ext.getBuildActions().find { it.id == actionId }
126-
} catch (_: Throwable) {}
139+
val fromExtension = pluginExtensions[pluginId]?.let { ext ->
140+
runCatching {
141+
ext.getBuildActions().find { it.id == actionId }
142+
}.onFailure { e ->
143+
Log.w(TAG, "Failed to find action $actionId in plugin $pluginId", e)
144+
}.getOrNull()
127145
}
128-
129-
return manifestActions[pluginId]?.find { it.id == actionId }
146+
return fromExtension ?: manifestActions[pluginId]?.find { it.id == actionId }
130147
}
131148
}
132149

‎plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/loaders/PluginManifest.kt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,16 +59,23 @@ data class ExtensionInfo(
5959
)
6060

6161
data class ManifestBuildAction(
62+
@SerializedName("id")
6263
val id: String,
64+
@SerializedName("name")
6365
val name: String,
66+
@SerializedName("description")
6467
val description: String = "",
68+
@SerializedName("category")
6569
val category: String = "CUSTOM",
70+
@SerializedName("command")
6671
val command: String? = null,
72+
@SerializedName("arguments")
6773
val arguments: List<String> = emptyList(),
6874
@SerializedName("gradle_task")
6975
val gradleTask: String? = null,
7076
@SerializedName("working_directory")
7177
val workingDirectory: String? = null,
78+
@SerializedName("environment")
7279
val environment: Map<String, String> = emptyMap(),
7380
@SerializedName("timeout_ms")
7481
val timeoutMs: Long = 600_000

‎plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeCommandServiceImpl.kt‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ class IdeCommandServiceImpl(
5050
is CommandSpec.GradleTask -> {
5151
val gradleWrapper = projectRoot?.let { File(it, "gradlew") }
5252
?: throw IllegalStateException("No project root available for Gradle task execution")
53+
if (!gradleWrapper.exists()) {
54+
throw IllegalStateException("Gradle wrapper not found at ${gradleWrapper.absolutePath}")
55+
}
56+
if (!gradleWrapper.canExecute()) {
57+
throw IllegalStateException("Gradle wrapper is not executable: ${gradleWrapper.absolutePath}")
58+
}
5359
ProcessBuilder(listOf(gradleWrapper.absolutePath, spec.taskPath) + spec.arguments).apply {
5460
directory(projectRoot)
5561
}
@@ -83,8 +89,10 @@ class IdeCommandServiceImpl(
8389
override fun getRunningCommandCount(): Int = runningCommands.size
8490

8591
fun cancelAllCommands() {
86-
runningCommands.values.forEach { it.cancel() }
87-
runningCommands.clear()
92+
runningCommands.entries.removeAll { (_, execution) ->
93+
execution.cancel()
94+
true
95+
}
8896
}
8997

9098
private fun requirePermission() {
@@ -160,7 +168,8 @@ private class CommandExecutionImpl(
160168
fun start(onComplete: () -> Unit) {
161169
scope.launch {
162170
val startTime = System.currentTimeMillis()
163-
try {
171+
172+
runCatching {
164173
withTimeout(timeoutMs) {
165174
process = processBuilder.start()
166175
val proc = process!!
@@ -176,32 +185,34 @@ private class CommandExecutionImpl(
176185
outputChannel.close()
177186

178187
val duration = System.currentTimeMillis() - startTime
179-
val result = if (exitCode == 0) {
188+
if (exitCode == 0) {
180189
CommandResult.Success(exitCode, stdoutBuilder.toString(), stderrBuilder.toString(), duration)
181190
} else {
182191
CommandResult.Failure(exitCode, stdoutBuilder.toString(), stderrBuilder.toString(), null, duration)
183192
}
184-
resultDeferred.complete(result)
185193
}
186-
} catch (e: kotlinx.coroutines.TimeoutCancellationException) {
187-
process?.destroyForcibly()
188-
outputChannel.close()
189-
val duration = System.currentTimeMillis() - startTime
190-
resultDeferred.complete(
191-
CommandResult.Failure(-1, stdoutBuilder.toString(), stderrBuilder.toString(), "Command timed out after ${timeoutMs}ms", duration)
192-
)
193-
} catch (e: Exception) {
194+
}.onSuccess { result ->
195+
resultDeferred.complete(result)
196+
}.onFailure { e ->
194197
process?.destroyForcibly()
195198
outputChannel.close()
199+
val stdout = stdoutBuilder.toString()
200+
val stderr = stderrBuilder.toString()
196201
val duration = System.currentTimeMillis() - startTime
202+
val failureResult = when (e) {
203+
is kotlinx.coroutines.TimeoutCancellationException ->
204+
CommandResult.Failure(-1, stdout, stderr, "Command timed out after ${timeoutMs}ms: ${e.message}", duration)
205+
is kotlinx.coroutines.CancellationException ->
206+
CommandResult.Cancelled(stdout, stderr)
207+
else ->
208+
CommandResult.Failure(-1, stdout, stderr, "Unexpected error: ${e.message}", duration)
209+
}
197210
if (resultDeferred.isActive) {
198-
resultDeferred.complete(
199-
CommandResult.Cancelled(stdoutBuilder.toString(), stderrBuilder.toString())
200-
)
211+
resultDeferred.complete(failureResult)
201212
}
202-
} finally {
203-
onComplete()
204213
}
214+
215+
onComplete()
205216
}
206217
}
207218

‎plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/ui/PluginDrawableResolver.kt‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package com.itsaky.androidide.plugins.manager.ui
33
import android.content.Context
44
import android.content.res.Resources
55
import android.graphics.drawable.Drawable
6+
import android.util.Log
67
import androidx.core.content.ContextCompat
78
import com.itsaky.androidide.plugins.base.PluginFragmentHelper
89

@@ -14,12 +15,14 @@ object PluginDrawableResolver {
1415
if (pluginContext == null) {
1516
return loadDrawable(fallbackContext, resId)
1617
}
17-
try {
18-
val drawable = ContextCompat.getDrawable(pluginContext, resId)
19-
return drawable
20-
} catch (_: Resources.NotFoundException) {
21-
} catch (_: Throwable) {
22-
}
18+
val drawable = runCatching {
19+
ContextCompat.getDrawable(pluginContext, resId)
20+
}.onFailure { e ->
21+
if (e !is Resources.NotFoundException && e !is IllegalArgumentException) {
22+
Log.w("PluginDrawableResolver", "Failed to resolve drawable $resId for plugin $pluginId", e)
23+
}
24+
}.getOrNull()
25+
if (drawable != null) return drawable
2326
}
2427
return loadDrawable(fallbackContext, resId)
2528
}

0 commit comments

Comments
 (0)