Fix/adfa 3803 -- ADFA-3803 -- Add Pebble support to the documentation system#1318
Merged
Conversation
…y web server features and debug points are gutted in this version and need to be reintegrated.
Contributor
|
Caution Review failedFailed to post review comments 📝 Walkthrough
WalkthroughAdds Pebble and Jackson dependencies and integrates Pebble template rendering into the local WebServer: compiles/caches templates, parses DB-stored JSON content as template context, conditionally decompresses Brotli for templating, and refactors the GET handler to fetch templateId/compression, assemble fragments, render templates, and send responses. ChangesPebble Template Rendering for Database-Backed Web Content
Sequence DiagramsequenceDiagram
participant Client
participant RequestHandler
participant Database
participant instantiatePebbleTemplate
participant PebbleEngine
participant Jackson
Client->>RequestHandler: GET /db path (Accept-Encoding header)
RequestHandler->>Database: query Content JOIN ContentTypes (fetch content, compression, templateId)
Database-->>RequestHandler: content bytes, compression, templateId
RequestHandler->>instantiatePebbleTemplate: call with content bytes, templateId (if templateId>0)
instantiatePebbleTemplate->>Database: query Templates table for template source (templateId)
Database-->>instantiatePebbleTemplate: template source
instantiatePebbleTemplate->>PebbleEngine: compile (if missing) and cache template
instantiatePebbleTemplate->>Jackson: parse content bytes (UTF-8 JSON) -> Map<String,Any>
Jackson-->>instantiatePebbleTemplate: context Map
instantiatePebbleTemplate->>PebbleEngine: evaluate template with context -> rendered String
PebbleEngine-->>instantiatePebbleTemplate: rendered String
instantiatePebbleTemplate-->>RequestHandler: rendered UTF-8 bytes
RequestHandler-->>Client: HTTP/1.1 200 + headers (maybe Content-Encoding) then body
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt (1)
358-374: ⚡ Quick winAvoid quadratic copying when reassembling fragmented content.
dbContent += dbContent2reallocates and copies the full accumulated payload on every fragment. Multi-fragment documents will do O(n²) copying on the request path.Suggested fix
if (dbContent.size == 1024 * 1024) { val query2 = "SELECT content FROM Content WHERE path = ? AND languageId = 1" var fragmentNumber = 1 + val combined = ByteArrayOutputStream().apply { write(dbContent) } var dbContent2 = dbContent while (dbContent2.size == 1024 * 1024) { val path2 = "$path-$fragmentNumber" val cursor2 = database.rawQuery(query2, arrayOf(path2)) try { if (cursor2.moveToFirst()) { dbContent2 = cursor2.getBlob(0) - dbContent += dbContent2 + combined.write(dbContent2) fragmentNumber++ } else break } finally { cursor2.close() } } + dbContent = combined.toByteArray() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt` around lines 358 - 374, The current fragment reassembly in WebServer.kt uses repeated concatenation (dbContent += dbContent2) which causes quadratic copying; change the logic to stream-append fragments instead: after reading the initial dbContent blob, create a ByteArrayOutputStream (or a MutableList<ByteArray>) and write the initial dbContent and each dbContent2 into it inside the while loop (using fragmentNumber, query2, cursor2 as-is), then after the loop call toByteArray() once to produce the final byte[]; ensure cursor2 is still closed in the finally and replace in-place concatenation with the single-buffer assembly approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt`:
- Around line 415-460: The cached templates (templateCache keyed by templateId
in the templateCache.getOrPut block) are not invalidated when the backing
database is hot-swapped by handleClient(), so cached entries serve stale
template sources; fix by making cache entries sensitive to the current database
(either clear templateCache whenever handleClient() swaps the database or
include a unique database identifier (e.g., DB file path, timestamp, or a
databaseVersion token exposed where the swap happens) in the cache key used by
templateCache.getOrPut), and update the swap code path in handleClient() to call
templateCache.clear() or bump the DB identifier so pebbleEngine.getTemplate(...)
always uses templates from the active database.
- Around line 424-458: The tCursor returned by database.rawQuery(...) is only
closed in the success branch; wrap the rawQuery call in a Kotlin use block so
the cursor is closed on every path (including the error branches). Replace the
current val tCursor = database.rawQuery(...) and later tCursor.use { ... } with
a single database.rawQuery(tQuery, arrayOf(templateId.toString())).use { tCursor
-> ... } and move the count checks and throw calls inside that use block so
tCursor is always closed; update references to tCursor inside the block
accordingly.
---
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt`:
- Around line 358-374: The current fragment reassembly in WebServer.kt uses
repeated concatenation (dbContent += dbContent2) which causes quadratic copying;
change the logic to stream-append fragments instead: after reading the initial
dbContent blob, create a ByteArrayOutputStream (or a MutableList<ByteArray>) and
write the initial dbContent and each dbContent2 into it inside the while loop
(using fragmentNumber, query2, cursor2 as-is), then after the loop call
toByteArray() once to produce the final byte[]; ensure cursor2 is still closed
in the finally and replace in-place concatenation with the single-buffer
assembly approach.
🪄 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: b51ecb60-5cbd-4606-9e16-c72e98d64ced
📒 Files selected for processing (2)
app/build.gradle.ktsapp/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt
jomen-adfa
reviewed
May 19, 2026
Daniel-ADFA
reviewed
May 19, 2026
Daniel-ADFA
reviewed
May 19, 2026
jatezzz
previously requested changes
May 19, 2026
…me case logic in database response handler for standard GET requests
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt (1)
469-470: ⚡ Quick winReuse
ObjectMapperinstance instead of creating one per call.
ObjectMapperis thread-safe and expensive to instantiate. Creating a new instance on every template render adds unnecessary overhead.Proposed fix
Add a class-level field:
private val objectMapper = ObjectMapper()Then update the usage:
- val mapper = ObjectMapper() - val context: Map<String, Any> = mapper.readValue(dbContent.toString(Charsets.UTF_8), object : TypeReference<Map<String, Any>>() {}) + val context: Map<String, Any> = objectMapper.readValue(dbContent.toString(Charsets.UTF_8), object : TypeReference<Map<String, Any>>() {})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt` around lines 469 - 470, The code creates a new ObjectMapper inside the render path (the local variable mapper in WebServer.kt), which is expensive; make a single reusable instance by adding a class-level property (e.g., private val objectMapper = ObjectMapper()) and replace the local mapper usage with that objectMapper when calling readValue to build context (the readValue call that currently uses mapper). Ensure the new field is used across methods instead of instantiating ObjectMapper per call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt`:
- Line 67: Rename the misspelled constant comtentChunkSize to contentChunkSize
in WebServer.kt (keep the same value 1024 * 1024) and update all usages to the
new name (replace references to comtentChunkSize with contentChunkSize where the
chunk size is read/used). Ensure the identifier is updated consistently
(declaration and every reference) so compilation and behavior remain unchanged.
---
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt`:
- Around line 469-470: The code creates a new ObjectMapper inside the render
path (the local variable mapper in WebServer.kt), which is expensive; make a
single reusable instance by adding a class-level property (e.g., private val
objectMapper = ObjectMapper()) and replace the local mapper usage with that
objectMapper when calling readValue to build context (the readValue call that
currently uses mapper). Ensure the new field is used across methods instead of
instantiating ObjectMapper per call.
🪄 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: 6644b417-d3af-4ea0-8158-b2c1b5fa8c4a
📒 Files selected for processing (2)
app/build.gradle.ktsapp/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/build.gradle.kts
davidschachterADFA
approved these changes
May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This branch has an updated local web server that can instantiate templates with data stored on disk in JSON format.
https://appdevforall.atlassian.net/browse/ADFA-3803