feat/ADFA-3674-template-custom-functions template custom functions#1175
Conversation
📝 Walkthrough
WalkthroughRecipe execution now carries an Android Context through RecipeExecutor -> TemplateRecipeExecutor; ZipRecipeExecutor uses that context to extract a template Changes
Sequence DiagramsequenceDiagram
participant Fragment as TemplateDetailsFragment
participant Exec as TemplateRecipeExecutor
participant ZipExec as ZipRecipeExecutor
participant Archive as Template Archive (zip)
participant Cache as CodeCacheDir
participant DexLoader as DexClassLoader
participant Service as ServiceLoader
participant Pebble as PebbleEngine
Fragment->>Exec: instantiate with applicationContext
Fragment->>Exec: exec.template.recipe.execute(exec)
Exec->>ZipExec: execute(recipe, executor)
ZipExec->>Archive: open archive, look for META_EXTENSION_JAR
Archive-->>ZipExec: extensions.jar (if present)
ZipExec->>Cache: write temp extensions.jar to codeCacheDir
ZipExec->>DexLoader: create DexClassLoader (jar, optimized dex dir)
ZipExec->>Service: ServiceLoader.load(Extension, DexClassLoader)
Service-->>ZipExec: Extension instances
loop register each Extension
ZipExec->>Pebble: builder.extension(ext)
end
Pebble-->>ZipExec: built engine with extensions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.kt`:
- Around line 108-109: The background lambda passed to executeAsyncProvideError
captures requireContext() directly which can throw IllegalStateException if the
fragment detaches and also improperly passes a fragment Context to background
work; fix it by capturing val appCtx = requireContext().applicationContext on
the UI thread before calling executeAsyncProvideError and then construct
TemplateRecipeExecutor(appCtx) inside the lambda so template.recipe.execute(...)
uses the application context instead of requireContext().
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt`:
- Around line 81-87: The code force-unwraps executor.context when calling
loadExtensionFromArchive; change this to guard against null: check if
executor.context is null before using it (e.g., if executor.context == null then
log or warn and skip processing extensionsEntry) and only call
loadExtensionFromArchive(extensionsEntry, executor.context) when context is
non-null, then iterate and call builder.extension(ext) as before; this avoids
the NullPointerException while preserving behavior (or alternatively throw a
descriptive IllegalStateException if you prefer failing fast).
- Around line 352-364: The loadExtensionFromArchive function currently calls
zipProvider() and reopens the archive (and uses a ZipEntry from a different
ZipFile), causing a resource leak and unsafe mixing of ZipEntry instances;
change loadExtensionFromArchive signature to accept the already-open ZipFile
(the instance created and used in execute) alongside the ZipEntry and Context,
then use that passed ZipFile to obtain the InputStream (e.g.,
zipFile.getInputStream(entry)) instead of calling zipProvider(), removing any
new ZipFile creation and ensuring the original execute-managed use block remains
responsible for closing the archive.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b37dde92-16e9-4ae4-97f9-5952bb62976c
📒 Files selected for processing (5)
app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.ktapp/src/main/java/com/itsaky/androidide/utils/TemplateRecipeExecutor.kttemplates-api/src/main/java/com/itsaky/androidide/templates/RecipeExecutor.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateConstants.kt
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt`:
- Around line 427-430: Fix the typo in the exception message inside
ZipRecipeExecutor.kt's catch block: update the Exception creation in the catch
(e: Throwable) handler where error("ServiceLoader iteration failed for
${entry.name}", Exception("FServiceLoader iteration failed", e)) is called so
the inner Exception message reads "ServiceLoader iteration failed" (replace
"FServiceLoader" with "ServiceLoader") to ensure consistent and correct logging.
- Around line 363-374: The temp JAR created in ZipRecipeExecutor.kt (tempJar) is
never deleted, causing accumulation in context.codeCacheDir; after you
construct/use the DexClassLoader (and after any loading/initialization that
depends on that JAR and its optimized dex in optimizedDir), explicitly delete
tempJar (e.g., tempJar.delete() and/or tempJar.deleteOnExit()) and log any
failure; ensure deletion happens in a finally block or after all uses so the
file is removed even when exceptions are thrown around zip.getInputStream(entry)
or when creating/using the DexClassLoader.
- Around line 314-317: The private method resolveBoolean in ZipRecipeExecutor
(ZipRecipeExecutor.kt) is unused dead code; delete the entire
resolveBoolean(Boolean?, Boolean): ResolvedParam<Boolean> method declaration
from the class to remove the dead method and then rebuild/compile to ensure no
callers remain and no unused-import warnings are introduced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b807ef13-1033-4c80-9be5-eda5943f6078
📒 Files selected for processing (2)
app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt
There was a problem hiding this comment.
🧹 Nitpick comments (1)
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt (1)
363-363: AdddeleteOnExit()to mitigate disk accumulation over time.Per the prior discussion about DexClassLoader's lazy JAR references, immediate deletion isn't viable. However, the acceptable mitigation of
tempJar.deleteOnExit()mentioned in that conversation hasn't been implemented. This will clean up temp JARs when the process terminates, preventing accumulation across sessions.Suggested fix
val tempJar = File.createTempFile("ext_", ".jar", context.codeCacheDir) + tempJar.deleteOnExit()Based on learnings: "Acceptable mitigations include
tempJar.deleteOnExit()or storing the JAR at a deterministic path (e.g., alongside the optimized DEX directory) for cleanup on the next run."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt` at line 363, The temporary JAR created via File.createTempFile("ext_", ".jar", context.codeCacheDir) in ZipRecipeExecutor (tempJar variable) needs to call tempJar.deleteOnExit() immediately after creation to avoid accumulating stale temp files across runs; update the code where tempJar is instantiated to invoke deleteOnExit() on that File instance (or alternatively store at a deterministic path) so the JVM will remove the file when the process exits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt`:
- Line 363: The temporary JAR created via File.createTempFile("ext_", ".jar",
context.codeCacheDir) in ZipRecipeExecutor (tempJar variable) needs to call
tempJar.deleteOnExit() immediately after creation to avoid accumulating stale
temp files across runs; update the code where tempJar is instantiated to invoke
deleteOnExit() on that File instance (or alternatively store at a deterministic
path) so the JVM will remove the file when the process exits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f3bc119-9ed9-4326-afa1-84acb7909d04
📒 Files selected for processing (1)
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt
Capability to dynamically load pebble template extension custom functions from the CGT