From 28fc5a2a0bd604b640248568a5cb05487d07d651 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 21 Aug 2026 20:16:54 -0700 Subject: [PATCH 1/8] ADFA-5176: Serve documentation to the app's WebViews in-process One pipeline for reading documentation.db, in common, with two transports over it. DocumentationContentSource owns row lookup, chunked-row reassembly, the dictionary-aware Brotli decode and the sdcard debug-database swap, under a read/write lock so readers do not have the handle closed under them. It also renders the Pebble template rows, so both transports serve finished pages and neither carries the template engine. DocumentationRequestInterceptor answers the app's own WebViews through WebViewClient.shouldInterceptRequest, so a page's assets cost a database read instead of a TCP connection each. It matches the same http://localhost:6174/... URL space, so strings.xml, ToolTipManager's link builder and the DocumentationExtension contract need no changes, and anything it declines -- a /pr/ endpoint, an unknown path, a failed read -- falls through to WebServer. The CodeOnTheGo.nointercept sentinel forces everything back onto the socket. WebServer keeps serving port 6174 for WebViews that are not wired to the interceptor and for the /pr/ developer endpoints, but it now reads through the shared source: no database handle, no Pebble engine, no gson, no decode path of its own. What is left is HTTP. This is a port rather than the original branch. ADFA-5172 was an investigation whose instrumentation is abandoned, and ADFA-5175's worker pool is declined, so the accept loop here is stage's single-threaded one and the config fields those tickets added are gone. The two duplicated rules the original carried are now shared instead: * The dictionary is gated on the version the database declares (DatabaseVersionResolver.resolveMajorVersion), as WebServer already does, rather than on whether a CompressionDictionary table happens to exist. * The charset comes from ContentTypeHeaders (ADFA-5241), so a row does not describe itself differently depending on which transport served it. The interceptor previously said utf-8 for text/ only, which left an SVG served in-process declaring no encoding while the same row over the socket declared one. 356 tests across app and common pass, including 14 for the content source and 9 for the interceptor. The two WebServer tests that assert the dictionary loads once per database now declare a version, without which the gate would leave them passing while testing nothing. Co-Authored-By: Claude Opus 5 (1M context) --- app/build.gradle.kts | 3 - .../activities/editor/FAQActivity.kt | 105 +-- .../fragments/IDETooltipWebViewFragment.kt | 182 ++--- .../androidide/localWebServer/WebServer.kt | 722 +++++------------- .../BrotliDictionaryDecodeTest.kt | 5 + .../localWebServer/WebServerTest.kt | 132 +--- common/build.gradle.kts | 6 + .../activities/editor/HelpActivity.kt | 397 ++++++---- .../DocumentationContentSource.kt | 578 ++++++++++++++ .../DocumentationRequestInterceptor.kt | 147 ++++ .../DocumentationContentSourceTest.kt | 287 +++++++ .../DocumentationRequestInterceptorTest.kt | 134 ++++ docs/documentation-database.md | 13 +- 13 files changed, 1767 insertions(+), 944 deletions(-) create mode 100644 common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt create mode 100644 common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt create mode 100644 common/src/test/java/com/itsaky/androidide/documentation/DocumentationContentSourceTest.kt create mode 100644 common/src/test/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptorTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index acff8f5ee7..9b4c9f51bc 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -431,9 +431,6 @@ dependencies { implementation(libs.androidx.lifecycle.process) implementation(libs.androidx.lifecycle.runtime.ktx) coreLibraryDesugaring(libs.desugar.jdk.libs.v215) - - // Pebble template engine - implementation("io.pebbletemplates:pebble:4.1.1") } tasks.register("downloadDocDb") { diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/FAQActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/FAQActivity.kt index b4505560ec..d1251917f3 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/FAQActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/FAQActivity.kt @@ -17,69 +17,84 @@ package com.itsaky.androidide.activities.editor -import androidx.core.graphics.Insets import android.os.Bundle import android.view.View +import android.webkit.WebResourceRequest +import android.webkit.WebResourceResponse +import android.webkit.WebView import android.webkit.WebViewClient -import org.adfa.constants.CONTENT_KEY +import androidx.core.graphics.Insets import com.itsaky.androidide.R import com.itsaky.androidide.app.EdgeToEdgeIDEActivity import com.itsaky.androidide.databinding.ActivityFaqBinding +import com.itsaky.androidide.documentation.DocumentationRequestInterceptor +import org.adfa.constants.CONTENT_KEY class FAQActivity : EdgeToEdgeIDEActivity() { + @Suppress("ktlint:standard:backing-property-naming") + private var _binding: ActivityFaqBinding? = null + private val binding: ActivityFaqBinding + get() = + checkNotNull(_binding) { + "FAQActivity has been destroyed" + } - private var _binding: ActivityFaqBinding? = null - private val binding: ActivityFaqBinding - get() = checkNotNull(_binding) { - "FAQActivity has been destroyed" - } - - override fun bindLayout(): View { - _binding = ActivityFaqBinding.inflate(layoutInflater) - return binding.root - } + override fun bindLayout(): View { + _binding = ActivityFaqBinding.inflate(layoutInflater) + return binding.root + } - override fun onCreate(savedInstanceState: Bundle?) { - super.onCreate(savedInstanceState) + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) - with(binding) { - setSupportActionBar(toolbar) - supportActionBar!!.setTitle(R.string.faq_activity_title) - supportActionBar!!.setDisplayHomeAsUpEnabled(true) - toolbar.setNavigationOnClickListener { onBackPressedDispatcher.onBackPressed() } + with(binding) { + setSupportActionBar(toolbar) + supportActionBar!!.setTitle(R.string.faq_activity_title) + supportActionBar!!.setDisplayHomeAsUpEnabled(true) + toolbar.setNavigationOnClickListener { onBackPressedDispatcher.onBackPressed() } - val htmlContent = intent.getStringExtra(CONTENT_KEY) + val htmlContent = intent.getStringExtra(CONTENT_KEY) // htmlContent?.let { // webView.clearCache(true) // webView.loadDataWithBaseURL(null, it, "text/html", "UTF-8", null) // } - // Enable JavaScript if required - webView.settings.javaScriptEnabled = true + // Enable JavaScript if required + webView.settings.javaScriptEnabled = true - // Set WebViewClient to handle page navigation within the WebView - webView.webViewClient = WebViewClient() + // Set WebViewClient to handle page navigation within the WebView. ADFA-5176: it answers + // documentation from the database in-process, falling through to the local web server + // for anything it declines. + webView.webViewClient = + object : WebViewClient() { + override fun shouldInterceptRequest( + view: WebView, + request: WebResourceRequest, + ): WebResourceResponse? = + DocumentationRequestInterceptor.shared.intercept(request) + ?: super.shouldInterceptRequest(view, request) + } - // Load the HTML file from the assets folder - htmlContent?.let { webView.loadUrl(it) } - } - } + // Load the HTML file from the assets folder + htmlContent?.let { webView.loadUrl(it) } + } + } - override fun onApplySystemBarInsets(insets: Insets) { - val toolbar: View = binding.toolbar - toolbar.setPadding( - toolbar.paddingLeft + insets.left, - toolbar.paddingTop, - toolbar.paddingRight + insets.right, - toolbar.paddingBottom - ) + override fun onApplySystemBarInsets(insets: Insets) { + val toolbar: View = binding.toolbar + toolbar.setPadding( + toolbar.paddingLeft + insets.left, + toolbar.paddingTop, + toolbar.paddingRight + insets.right, + toolbar.paddingBottom, + ) - val webview: View = binding.webView - webview.setPadding( - webview.paddingLeft + insets.left, - webview.paddingTop, - webview.paddingRight + insets.right, - webview.paddingBottom - ) - } -} \ No newline at end of file + val webview: View = binding.webView + webview.setPadding( + webview.paddingLeft + insets.left, + webview.paddingTop, + webview.paddingRight + insets.right, + webview.paddingBottom, + ) + } +} diff --git a/app/src/main/java/com/itsaky/androidide/fragments/IDETooltipWebViewFragment.kt b/app/src/main/java/com/itsaky/androidide/fragments/IDETooltipWebViewFragment.kt index b730b96367..f9874372ae 100644 --- a/app/src/main/java/com/itsaky/androidide/fragments/IDETooltipWebViewFragment.kt +++ b/app/src/main/java/com/itsaky/androidide/fragments/IDETooltipWebViewFragment.kt @@ -24,6 +24,7 @@ import android.view.LayoutInflater import android.view.View import android.view.ViewGroup import android.webkit.WebResourceRequest +import android.webkit.WebResourceResponse import android.webkit.WebView import android.webkit.WebViewClient import androidx.activity.OnBackPressedCallback @@ -31,96 +32,105 @@ import androidx.appcompat.view.ContextThemeWrapper import androidx.core.view.isVisible import androidx.fragment.app.Fragment import com.itsaky.androidide.R +import com.itsaky.androidide.documentation.DocumentationRequestInterceptor class IDETooltipWebviewFragment : Fragment() { - private lateinit var webView: WebView - private lateinit var website : String - - //This warning is unnecessary because we control the content - @SuppressLint("SetJavaScriptEnabled") - override fun onCreateView( - inflater: LayoutInflater, - container: ViewGroup?, - savedInstanceState: Bundle? - ): View? { - super.onCreateView(inflater, container, savedInstanceState) - Log.d(Companion.TAG, "IDETooltipWebviewFragment\\\\onCreateView called") - // Handle back press using OnBackPressedCallback - requireActivity().onBackPressedDispatcher.addCallback( - viewLifecycleOwner, - object : OnBackPressedCallback(true) { - override fun handleOnBackPressed() { - if (webView.canGoBack()) { - webView.goBack() - } else { - activity?.runOnUiThread { - webView.clearHistory() - webView.loadUrl("about:blank") - webView.destroy() - } - parentFragmentManager.popBackStack() - isEnabled = - false // Disable this callback to let the default back press behavior occur - } - } - }) - - website = arguments?.getString(MainFragment.KEY_TOOLTIP_URL).orEmpty() - - val safeContext = ContextThemeWrapper(requireContext().applicationContext, requireContext().theme) - val view = LayoutInflater.from(safeContext).inflate(R.layout.fragment_idetooltipwebview, container, false) - - webView = view.findViewById(R.id.IDETooltipWebView) - - // Set a WebViewClient to handle loading pages - webView.webViewClient = object : WebViewClient() { - override fun shouldOverrideUrlLoading(view: WebView, request: WebResourceRequest): Boolean { - // Allow loading of local assets files - if (request.url.toString().startsWith("file:///android_asset/")) { - view.loadUrl(request.url.toString()) - return true - } - return super.shouldOverrideUrlLoading(view, request) - } - } - - // Set up WebChromeClient to support JavaScript + private lateinit var webView: WebView + private lateinit var website : String + private val documentation = DocumentationRequestInterceptor.shared + + //This warning is unnecessary because we control the content + @SuppressLint("SetJavaScriptEnabled") + override fun onCreateView( + inflater: LayoutInflater, + container: ViewGroup?, + savedInstanceState: Bundle? + ): View? { + super.onCreateView(inflater, container, savedInstanceState) + Log.d(Companion.TAG, "IDETooltipWebviewFragment\\\\onCreateView called") + // Handle back press using OnBackPressedCallback + requireActivity().onBackPressedDispatcher.addCallback( + viewLifecycleOwner, + object : OnBackPressedCallback(true) { + override fun handleOnBackPressed() { + if (webView.canGoBack()) { + webView.goBack() + } else { + activity?.runOnUiThread { + webView.clearHistory() + webView.loadUrl("about:blank") + webView.destroy() + } + parentFragmentManager.popBackStack() + isEnabled = + false // Disable this callback to let the default back press behavior occur + } + } + }) + + website = arguments?.getString(MainFragment.KEY_TOOLTIP_URL).orEmpty() + + val safeContext = ContextThemeWrapper(requireContext().applicationContext, requireContext().theme) + val view = LayoutInflater.from(safeContext).inflate(R.layout.fragment_idetooltipwebview, container, false) + + webView = view.findViewById(R.id.IDETooltipWebView) + + // Set a WebViewClient to handle loading pages + webView.webViewClient = object : WebViewClient() { + // ADFA-5176: documentation comes from the database in-process; anything this declines + // still goes to the local web server. + override fun shouldInterceptRequest( + view: WebView, + request: WebResourceRequest, + ): WebResourceResponse? = documentation.intercept(request) ?: super.shouldInterceptRequest(view, request) + + override fun shouldOverrideUrlLoading(view: WebView, request: WebResourceRequest): Boolean { + // Allow loading of local assets files + if (request.url.toString().startsWith("file:///android_asset/")) { + view.loadUrl(request.url.toString()) + return true + } + return super.shouldOverrideUrlLoading(view, request) + } + } + + // Set up WebChromeClient to support JavaScript // webView.webChromeClient = WebChromeClient() - webView.settings.allowFileAccessFromFileURLs - webView.settings.allowFileAccess - webView.settings.allowUniversalAccessFromFileURLs - webView.scrollBarStyle = WebView.SCROLLBARS_OUTSIDE_OVERLAY - webView.scrollBarDefaultDelayBeforeFade = 1000 - - - // Enable JavaScript if needed - webView.settings.javaScriptEnabled = true - - // Load the HTML file from the assets folder - webView.loadUrl(website) - return view - } - - override fun onViewCreated(view: View, savedInstanceState: Bundle?) { - super.onViewCreated(view, savedInstanceState) - Log.d(Companion.TAG, "IDETooltipWebViewFragment\\\\onViewCreated called") - } - - override fun onDestroyView() { - super.onDestroyView() - // Clean up the WebView in Fragment - if(webView.isVisible) { - webView.clearHistory() - webView.loadUrl("about:blank") - webView.destroy() - } - - } - - companion object { - private const val TAG = "IDETooltipWebViewFragment" - } + webView.settings.allowFileAccessFromFileURLs + webView.settings.allowFileAccess + webView.settings.allowUniversalAccessFromFileURLs + webView.scrollBarStyle = WebView.SCROLLBARS_OUTSIDE_OVERLAY + webView.scrollBarDefaultDelayBeforeFade = 1000 + + + // Enable JavaScript if needed + webView.settings.javaScriptEnabled = true + + // Load the HTML file from the assets folder + webView.loadUrl(website) + return view + } + + override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + super.onViewCreated(view, savedInstanceState) + Log.d(Companion.TAG, "IDETooltipWebViewFragment\\\\onViewCreated called") + } + + override fun onDestroyView() { + super.onDestroyView() + // Clean up the WebView in Fragment + if(webView.isVisible) { + webView.clearHistory() + webView.loadUrl("about:blank") + webView.destroy() + } + + } + + companion object { + private const val TAG = "IDETooltipWebViewFragment" + } } diff --git a/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt b/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt index d9c6c03537..2a062394f5 100644 --- a/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt +++ b/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt @@ -4,38 +4,28 @@ import android.database.Cursor import android.database.sqlite.SQLiteDatabase import android.net.TrafficStats import android.os.Environment.getExternalStorageDirectory -import com.aayushatharva.brotli4j.Brotli4jLoader -import com.aayushatharva.brotli4j.decoder.BrotliInputStream -import com.google.gson.Gson -import com.google.gson.GsonBuilder -import com.google.gson.ToNumberPolicy -import com.google.gson.reflect.TypeToken +import com.itsaky.androidide.documentation.DocumentationContent +import com.itsaky.androidide.documentation.DocumentationContentSource +import com.itsaky.androidide.documentation.DocumentationLookup import com.itsaky.androidide.utils.ContentTypeHeaders import com.itsaky.androidide.utils.DatabaseVersionResolver -import io.pebbletemplates.pebble.PebbleEngine -import io.pebbletemplates.pebble.loader.StringLoader -import io.pebbletemplates.pebble.template.PebbleTemplate import okio.ByteString.Companion.toByteString import org.slf4j.LoggerFactory import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream import java.io.File -import java.io.IOException import java.io.InputStream import java.io.PrintWriter -import java.io.SequenceInputStream -import java.io.StringWriter import java.net.InetSocketAddress import java.net.ServerSocket import java.net.Socket import java.net.URLDecoder -import java.nio.ByteBuffer -import java.sql.Date -import java.text.SimpleDateFormat -import java.util.Collections -import java.util.Locale import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.RejectedExecutionException +import java.util.concurrent.SynchronousQueue +import java.util.concurrent.ThreadPoolExecutor import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicLong import java.util.concurrent.atomic.AtomicReference data class ServerConfig( @@ -58,6 +48,9 @@ data class ServerConfig( "/Download/CodeOnTheGo.webserver.cs0", // Yes, this is hack code. val projectDatabasePath: String = "/data/data/com.itsaky.androidide/databases/RecentProject_database", + // ADFA-5175: how often the sdcard debug database may be stat'ed. It lives on FUSE-backed + // emulated storage, and it is a developer-only override, so once a second is plenty. + val debugDatabaseCheckIntervalMs: Long = 1000, ) data class JavaExecutionResult( @@ -68,45 +61,6 @@ data class JavaExecutionResult( val timeoutLimit: Long, ) -/** - * Copies [bytes] into a direct [ByteBuffer] -- brotli4j's `attachDictionary` requires a direct - * buffer, a heap-backed one throws `IllegalArgumentException`. - * - * The capacity must be exactly [bytes]`.size`: `attachDictionary` reads the whole capacity and - * ignores position/limit, so trailing slack from an over-allocated buffer is treated as dictionary - * content and every decode then fails with `IOException: corrupted input`. - */ -internal fun toDirectByteBuffer(bytes: ByteArray): ByteBuffer = - ByteBuffer.allocateDirect(bytes.size).apply { - put(bytes) - flip() - } - -/** - * Reads [chunks] back to back as one stream, without concatenating them into a new array. - * Cheap to build twice, which the no-dictionary retry in `decompressBrotli` relies on. - */ -internal fun chunksAsStream(chunks: List): InputStream = - SequenceInputStream(Collections.enumeration(chunks.map { ByteArrayInputStream(it) })) - -/** - * Joins [chunks] into one exactly-sized array. A ByteArrayOutputStream would repeatedly double its - * buffer and then hand back a second full copy -- avoidable here since the total is known up front. - * Returns the sole element as-is when there is nothing to join. - */ -internal fun joinChunks(chunks: List): ByteArray { - if (chunks.size == 1) { - return chunks[0] - } - val joined = ByteArray(chunks.sumOf { it.size }) - var offset = 0 - for (chunk in chunks) { - chunk.copyInto(joined, offset) - offset += chunk.size - } - return joined -} - class WebServer( private val config: ServerConfig, ) { @@ -117,31 +71,18 @@ class WebServer( // socket then binds anyway a moment later, orphaned, and holds the port until the process // dies. The next start() attempt on that port then fails with "Address already in use." private val lifecycleLock = Any() + + // The one pipeline that reads documentation.db (ADFA-5176): row lookup, chunk reassembly, + // dictionary-aware Brotli decode, and the sdcard debug-database swap. A WebView answers the + // same paths through its own instance in DocumentationRequestInterceptor. + private val contentSource = + DocumentationContentSource( + File(config.databasePath), + File(config.debugDatabasePath), + config.debugDatabaseCheckIntervalMs, + ) private var stopRequested = false private lateinit var serverSocket: ServerSocket - private lateinit var database: SQLiteDatabase - private var databaseTimestamp: Long = -1 - - // Timestamp of a debug database whose swap already failed, so a corrupt or unreadable one - // isn't reopened on every single request (it is checked per request). A newer copy has a - // different timestamp and is retried, which is the case that matters -- the developer - // replacing the file is exactly how they'd fix it. - private var failedDebugSwapTimestamp: Long = -1 - - // The shared dictionary Content's brotli-compressed rows are compressed against (see - // ADFA-5153). Lazily (re)loaded on demand, right before the first content fetch that needs - // it after `database` changes -- see compressionDictionaryStale -- rather than eagerly at - // database-open/swap time, but still cached (not reloaded per-request) once loaded for the - // currently active database. Null (no dictionary attached, plain-brotli decode) unless the - // active database declares MAJOR >= MAJOR_VERSION_WITH_COMPRESSION_DICTIONARY in ADFA-5220's - // version table. - private var compressionDictionary: ByteBuffer? = null - - // Set whenever `database` changes (see switchToDatabase); cleared once compressionDictionary - // has been (re)loaded for that database. Lets the dictionary stay lazily loaded -- only right - // before the first content fetch that actually needs it -- while still loading at most once - // per database change rather than once per request. - private var compressionDictionaryStale = true private val log = LoggerFactory.getLogger(WebServer::class.java) private val debugEnabled: Boolean = File(config.debugEnablePath).exists() @@ -151,195 +92,34 @@ class WebServer( // Frozen at startup; restart the server to pick up a change. private val clearCacheEnabled: Boolean = File(config.clearCacheEnablePath).exists() - private val pebbleEngine = PebbleEngine.Builder().loader(StringLoader()).build() - private val templateCache = ConcurrentHashMap() - private val gson: Gson = - GsonBuilder() - .setObjectToNumberStrategy(ToNumberPolicy.LONG_OR_DOUBLE) - .create() - private val dbContextType = object : TypeToken>() {}.type - private var bookshelfTemplateId: Int = -1 - private val httpInternalServerError = 500 - private val httpNotFound = 404 - private val contentChunkSize = 1024 * 1024 - - // function to obtain the last modified date of a documentation.db database - // this is used to see if there is a newer version of the database on the sdcard - fun getDatabaseTimestamp( - pathname: String, - silent: Boolean = false, - ): Long { - val dbFile = File(pathname) - var timestamp: Long = -1 - - if (dbFile.exists()) { - timestamp = dbFile.lastModified() + // Read and written by any worker; -1 means "not fetched yet". Two workers racing to fetch it + // both write the same id, so a plain volatile is enough. + @Volatile + private var bookshelfTemplateId: Int = -1 - if (!silent) { - val dateFormat = SimpleDateFormat("yyyy-MM-dd HH:mm:ss", Locale.getDefault()) + private val cacheLock = Any() - if (debugEnabled) log.debug("{} was last modified at {}.", pathname, dateFormat.format(Date(timestamp))) - } - } + // Which of the source's databases templateCache and bookshelfTemplateId were filled from. + @Volatile + private var cachedDatabaseGeneration = 0L + private val httpInternalServerError = 500 + private val httpNotFound = 404 - return timestamp - } + // Hal Eisen: required to fix StrictMode.VmPolicy.Builder.detectUntaggedSockets(). + private val socketStatsTag = 0xC0DE fun logDatabaseLastChanged() { try { - log.debug("Database last change: {}.", DatabaseVersionResolver.resolveDatabaseVersion(database)) + log.debug( + "Database last change: {}.", + contentSource.withDatabase { DatabaseVersionResolver.resolveDatabaseVersion(it) }, + ) } catch (e: Exception) { log.error("Could not retrieve database last change info: {}", e.message) } } - /** - * Loads the shared Brotli dictionary most Content rows are compressed against (see ADFA-5153). - * Returns null (logged) when the database *definitively* has no dictionary -- so callers fall - * back to plain, dictionary-free brotli decode (see [decompressBrotli]). - * - * The gate is the MAJOR version the database declares in ADFA-5220's version table, not the - * presence of a `CompressionDictionary` table: table sniffing infers a whole content format - * from one table's existence, and gets it wrong in both directions -- a database carrying the - * table but *unmigrated* content makes every plain row pay a failed dictionary decode before - * its plain one, on every request. Below - * [DatabaseVersionResolver.MAJOR_VERSION_WITH_COMPRESSION_DICTIONARY] the dictionary is neither - * read nor attached. - * - * The `CompressionDictionary` checks below still run, for a database that declares a new-enough - * version but has no usable dictionary row: without them the data query would raise "no such - * table", which the caller correctly reads as transient and would then retry on every request. - * - * Deliberately does *not* catch exceptions itself: an unexpected `SQLiteException`/IO failure is - * likely transient, and the caller (see [handleClient]) must not cache that as "no dictionary" - * the way it does a definitive absence, or a transient failure would permanently disable - * dictionary decoding for the rest of this database's lifetime. - */ - private fun loadCompressionDictionary(db: SQLiteDatabase): ByteBuffer? { - val majorVersion = DatabaseVersionResolver.resolveMajorVersion(db) - if (majorVersion == null || majorVersion < DatabaseVersionResolver.MAJOR_VERSION_WITH_COMPRESSION_DICTIONARY) { - log.warn( - "Database declares documentation version {}, below {}; decoding brotli content without a dictionary.", - majorVersion ?: "none", - DatabaseVersionResolver.MAJOR_VERSION_WITH_COMPRESSION_DICTIONARY, - ) - return null - } - - val tableExists = - db - .rawQuery( - "SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'CompressionDictionary'", - null, - ).use { it.moveToFirst() } - if (!tableExists) { - log.warn("CompressionDictionary table not found; decoding brotli content without a dictionary.") - return null - } - - return db.rawQuery("SELECT data FROM CompressionDictionary WHERE id = 1", null).use { cursor -> - if (!cursor.moveToFirst()) { - log.warn("CompressionDictionary table is empty; decoding brotli content without a dictionary.") - return null - } - val bytes = cursor.getBlob(0) - if (bytes == null) { - log.warn("CompressionDictionary row has a NULL data column; decoding brotli content without a dictionary.") - return null - } - // An empty blob would yield a 0-capacity buffer, which attachDictionary rejects -- - // every row's dictionary decode would then fail with nothing above DEBUG to say why. - if (bytes.isEmpty()) { - log.warn("CompressionDictionary row has an empty data column; decoding brotli content without a dictionary.") - return null - } - toDirectByteBuffer(bytes) - } - } - - /** - * Opens [path] as the active database, refreshing every piece of state that depends on which - * database file is active -- [databaseTimestamp] and the per-database caches - * [bookshelfTemplateId]/[templateCache] -- as one atomic operation. Does *not* load - * [compressionDictionary] itself -- a different database can have a different dictionary (or - * none) -- it only marks [compressionDictionaryStale] so the next content fetch that needs it - * loads it lazily then (see [handleClient]), at most once per database change rather than - * once per request. Only closes the previous database once the new one has opened - * successfully, so a failed swap (this throws) leaves the previous, still-open database - * serving requests rather than leaving [database] referencing an already-closed handle. - */ - private fun switchToDatabase( - path: String, - timestamp: Long, - ) { - val newDatabase = SQLiteDatabase.openDatabase(path, null, SQLiteDatabase.OPEN_READONLY) - if (::database.isInitialized) { - try { - database.close() - } catch (e: Exception) { - log.error("Cannot close previous database: {}", e.message) - } - } - database = newDatabase - databaseTimestamp = timestamp - compressionDictionaryStale = true - bookshelfTemplateId = -1 - templateCache.clear() - } - - /** - * Loads brotli4j's native library if nothing else has yet, and turns its absence into a failed - * request rather than a dead app. - * - * Nothing here owns that load: it happens as a side effect of `AssetsInstallationHelper`'s - * install or `ToolsManager`'s tooling-jar update, neither of which runs on an ordinary cold - * start. A process that skips both -- Android restarting the app straight into the editor, say -- - * reaches the first brotli row with the natives unregistered, and `DecoderJNI.nativeCreate` - * raises `UnsatisfiedLinkError`. Being an Error rather than an Exception, that escapes - * [handleClient]'s catch and kills the app from a coroutine worker instead of failing one - * request (observed on-device, 20-Aug). - * - * Referencing [Brotli4jLoader] triggers the static init that performs the load, so this call is - * the warm-up; afterwards `ensureAvailability` is a single static null-check, cheap enough to - * leave on the per-decode path rather than tracking "warmed" state of our own. - */ - private fun ensureBrotliAvailable() { - try { - Brotli4jLoader.ensureAvailability() - } catch (e: UnsatisfiedLinkError) { - throw IOException("brotli4j's native library is unavailable, so brotli content cannot be decoded", e) - } - } - - /** - * Decompresses one Brotli-compressed Content row. Tries the shared dictionary first, since every - * ADFA-5153-migrated row requires it, then falls back to a plain decode for rows that were never - * dictionary-compressed: plugin-contributed Tier 3 docs (PluginDocumentationManager/BrotliCompressor - * compress with no dictionary) or any row served from a pre-migration database. Attaching a - * dictionary to a stream that wasn't compressed against one reliably fails to decode rather than - * silently producing wrong bytes (verified empirically -- see docs/documentation-database.md), so - * this ordering never lets a dictionary-compressed row fall through to the plain path by accident. - */ - private fun decompressBrotli(chunks: List): ByteArray { - ensureBrotliAvailable() - val dictionary = compressionDictionary - if (dictionary != null) { - try { - return BrotliInputStream(chunksAsStream(chunks)).use { stream -> - stream.attachDictionary(dictionary) - stream.readBytes() - } - } catch (e: IOException) { - log.debug( - "Dictionary decode failed for a brotli row (likely dictionary-free plugin content); retrying without a dictionary: {}", - e.message, - ) - } - } - return BrotliInputStream(chunksAsStream(chunks)).use { it.readBytes() } - } - /** * Stops the server by closing the listening socket. Safe to call from any thread. * Causes [start]'s accept loop to exit. If [start] hasn't bound the socket yet -- @@ -360,8 +140,7 @@ class WebServer( } fun start() { - // Hal Eisen: Required to fix StrictMode.VmPolicy.Builder.detectUntaggedSockets() - TrafficStats.setThreadStatsTag(0xC0DE) + TrafficStats.setThreadStatsTag(socketStatsTag) try { log.info( "Starting WebServer on {}, port {}, debugEnabled={}, debugEnablePath='{}', " + @@ -376,7 +155,7 @@ class WebServer( ) try { - switchToDatabase(config.databasePath, getDatabaseTimestamp(config.databasePath)) + contentSource.open() } catch (e: Exception) { log.error("Cannot open database: {}", e.message) return @@ -407,7 +186,7 @@ class WebServer( // SLF4J placeholders produce wrong formatting here. --DS, 23-Feb-2026 if (debugEnabled) log.debug("Caught java.net.SocketException '$e'.") - if (e.message?.contains("Closed", ignoreCase = true) == true) { + if (isSocketClosed(e)) { if (debugEnabled) log.debug("WebServer socket closed, shutting down.") break } @@ -420,25 +199,18 @@ class WebServer( // SLF4J placeholders produce wrong formatting here. --DS, 23-Feb-2026 if (debugEnabled) log.debug("Caught exception '$e'.") - if (e is java.net.SocketException && e.message?.contains("Closed", ignoreCase = true) == true) { + if (e is java.net.SocketException && isSocketClosed(e)) { if (debugEnabled) log.debug("Client disconnected: {}", e.message) } else { log.error("Error handling client: {}", e.message) - clientSocket?.let { socket -> - try { - val output = socket.outputStream - - sendError(PrintWriter(output, true), output, httpInternalServerError, "Internal Server Error 1") - } catch (e2: Exception) { - log.error("Error sending error response: {}", e2.message) - } - } + clientSocket?.let { sendInternalServerError(it) } } } } finally { clientSocket?.close() - // CodeRabbit objects to the following line because clientSocket may print out as "null." This is intentional. --DS + // CodeRabbit objects to the following line because clientSocket may print out + // as "null." This is intentional. --DS if (debugEnabled) log.debug("clientSocket was {}.", clientSocket) } } @@ -448,21 +220,30 @@ class WebServer( if (::serverSocket.isInitialized) { serverSocket.close() } - // database is opened before the stopRequested check that can abort start() - // early (and before the accept loop on every other exit path), so it must be - // closed here too, not just serverSocket -- isInitialized guards the case - // where opening it above failed and this finally still runs. - if (::database.isInitialized) { - try { - database.close() - } catch (e: Exception) { - log.error("Cannot close database: {}", e.message) - } - } + + // The database is opened before the stopRequested check that can abort start() early + // (and before the accept loop on every other exit path), so it has to be closed here + // too, not just serverSocket. Closing an unopened source is a no-op, and the source + // closes under its own write lock: awaitTermination above can time out, and a worker + // that outlived it finishes its read before the handle goes. + contentSource.close() TrafficStats.clearThreadStatsTag() } } + private fun sendInternalServerError(clientSocket: Socket) { + try { + val output = clientSocket.outputStream + + sendError(PrintWriter(output, true), output, httpInternalServerError, "Internal Server Error 1") + } catch (e: Exception) { + log.error("Error sending error response: {}", e.message) + } + } + + /** A closed socket reports itself only in the exception's message, hence the string test. */ + private fun isSocketClosed(e: java.net.SocketException): Boolean = e.message?.contains("Closed", ignoreCase = true) == true + /** * Reads a single line from the stream (bytes until newline). Same stream is used for headers * and body so POST body bytes are not lost to a separate buffered reader. HTTP header lines are ASCII. @@ -534,22 +315,34 @@ class WebServer( return sendError(writer, output, 501, "Not Implemented") } - // check to see if there is a newer version of the documentation.db database on the sdcard - // if there is use that for our responses - val debugDatabaseTimestamp = getDatabaseTimestamp(config.debugDatabasePath, true) - if (debugDatabaseTimestamp > databaseTimestamp && debugDatabaseTimestamp != failedDebugSwapTimestamp) { - try { - switchToDatabase(config.debugDatabasePath, debugDatabaseTimestamp) - failedDebugSwapTimestamp = -1 - } catch (e: Exception) { - failedDebugSwapTimestamp = debugDatabaseTimestamp - log.error( - "Cannot swap to debug database '{}'; ignoring it until it changes: {}", - config.debugDatabasePath, - e.message, - ) - } + // Use a newer documentation.db from the sdcard if one has appeared. Outside the read lock + // below, because swapping takes the write lock and this lock does not upgrade. + serveRequest(writer, output, path) + } + + /** + * Drops what this server cached from a database the source has since swapped away -- just the + * bookshelf template id, now that the compiled templates live in the source with the swap. + */ + private fun discardCachesIfDatabaseChanged() { + if (contentSource.generation == cachedDatabaseGeneration) return + + synchronized(cacheLock) { + val generation = contentSource.generation + if (generation == cachedDatabaseGeneration) return + + bookshelfTemplateId = -1 + cachedDatabaseGeneration = generation } + } + + /** Answers one parsed request. */ + private fun serveRequest( + writer: PrintWriter, + output: java.io.OutputStream, + path: String, + ) { + discardCachesIfDatabaseChanged() // Handle the special "pr" endpoint with highest priority if (path.startsWith("pr/", false)) { @@ -564,206 +357,54 @@ class WebServer( } } - // Lazily (re)loaded here -- the one place the dictionary is actually consumed (see - // decompressBrotli) -- rather than eagerly at database-open/swap time, but only once per - // database change: a swap (just above) marks compressionDictionaryStale rather than - // reloading immediately, so this only hits the database again when that flag is set. - // Only clears the flag on a clean load (definitive dictionary or definitive absence) -- - // an unexpected exception leaves it set so the next request retries, rather than caching - // a transient failure as "no dictionary" for the rest of this database's lifetime. - if (compressionDictionaryStale) { - try { - compressionDictionary = loadCompressionDictionary(database) - compressionDictionaryStale = false - } catch (e: Exception) { - log.error("Could not load compression dictionary; will retry on the next request: {}", e.message) + when (val lookup = contentSource.lookup(path)) { + is DocumentationLookup.Found -> { + sendContent(writer, output, lookup.content) } - } - // Database fetch - val query = """ - SELECT C.content, CT.value, CT.compression, C.templateId - FROM Content C, ContentTypes CT - WHERE C.contentTypeID = CT.id - AND C.path = ? - """ - val cursor = database.rawQuery(query, arrayOf(path)) - - // Process database fetch - try { - if (cursor.count != 1) { - return if (cursor.count == 0) { - sendError(writer, output, httpNotFound, "Not Found") - } else { - sendError( - writer, - output, - httpInternalServerError, - "Corrupt database - multiple records found when unique record expected, Path requested: '$path'.", - ) - } + is DocumentationLookup.NotFound -> { + sendError(writer, output, httpNotFound, "Not Found") } - cursor.moveToFirst() - val firstChunk = cursor.getBlob(0) - val dbMimeType = cursor.getString(1) - var compression = cursor.getString(2) - val templateId = cursor.getInt(3) - - // Fragment handling for large content (> 1MB). The chunks stay a list rather than - // being eagerly concatenated: the old accumulate-into-a-ByteArrayOutputStream-then-copy - // held both the doubling buffer and its toByteArray() copy of the *compressed* chunks - // live at once, on top of the decompressed output that follows -- for the largest - // bundled PDF (8.8 MB over 9 chunks) that's a real, if partial, reduction: the - // decompressed output still goes through a comparable accumulate-then-copy in - // decompressBrotli's own readBytes() call, so the compressed-side saving here doesn't - // eliminate that separate transient. - val chunks = mutableListOf(firstChunk) - if (firstChunk.size == contentChunkSize) { - val query2 = "SELECT content FROM Content WHERE path = ? AND languageId = 1" - var fragmentNumber = 1 - var nextChunk = firstChunk - while (nextChunk.size == contentChunkSize) { - val path2 = "$path-$fragmentNumber" - val cursor2 = database.rawQuery(query2, arrayOf(path2)) - try { - if (cursor2.moveToFirst()) { - nextChunk = cursor2.getBlob(0) - chunks.add(nextChunk) - fragmentNumber++ - } else { - break - } - } finally { - cursor2.close() - } - } + is DocumentationLookup.Ambiguous -> { + sendError( + writer, + output, + httpInternalServerError, + "Corrupt database - ${lookup.rowCount} records found when unique record expected, Path requested: '$path'.", + ) } - // Content is compressed at rest with brotli -- most rows against the shared dictionary - // loaded into compressionDictionary (see ADFA-5153), but plugin-contributed Tier 3 docs - // (PluginDocumentationManager/BrotliCompressor) are plain brotli with no dictionary. - // This server always decompresses before responding, so it never needs to negotiate - // Content-Encoding with the client. - var dbContent = - if (compression == "brotli") { - compression = "none" - decompressBrotli(chunks) - } else { - joinChunks(chunks) - } - - // If the file is associated with a template, instantiate that template and send the result to the client - if (templateId > 0) { - dbContent = instantiatePebbleTemplate(templateId, dbContent, path, dbMimeType, compression) + is DocumentationLookup.Failed -> { + sendError(writer, output, httpInternalServerError, "Internal Server Error", lookup.cause.message ?: "") } + } + } + + /** + * Writes [content] to the client. The source hands back rows already decompressed and rendered, + * so this transport neither negotiates `Content-Encoding` nor knows about templates. + */ + private fun sendContent( + writer: PrintWriter, + output: java.io.OutputStream, + content: DocumentationContent, + ) { + try { + val bytes = content.bytes writer.println("HTTP/1.1 200 OK") - writer.println("Content-Type: ${ContentTypeHeaders.headerValue(dbMimeType)}") - writer.println("Content-Length: ${dbContent.size}") + writer.println("Content-Type: ${ContentTypeHeaders.headerValue(content.mimeType)}") + writer.println("Content-Length: ${bytes.size}") writer.println("Connection: close") writer.println() writer.flush() - output.write(dbContent) + output.write(bytes) output.flush() } catch (e: Exception) { log.error("Error processing request: {}", e.message) sendError(writer, output, httpInternalServerError, "Internal Server Error", e.message ?: "") - } finally { - cursor.close() - } - } - - /** - * Renders a Pebble template identified by `templateId` using the provided JSON data and returns the rendered output as bytes. - * - * @param templateId The database ID of the Pebble template to load and compile. - * @param dbContent JSON bytes that will be parsed and supplied as the template context. - * @param path The request/content path associated with this template (used for diagnostic/logging purposes). - * @param dbMimeType The MIME type of the stored content (used for diagnostic/logging purposes). - * @param compression The compression label of the stored content (always "none" by this point, since decompression already happened) (used for diagnostic/logging purposes). - * @return The rendered template encoded as UTF-8 bytes. - * @throws Exception If the template ID is not found, is duplicated in the database, or if template lookup/instantiation fails. - */ - private fun instantiatePebbleTemplate( - templateId: Int, - dbContent: ByteArray, - path: String, - dbMimeType: String, - compression: String, - ): ByteArray { - if (debugEnabled) log.debug("Processing template for templateId={}", templateId) - - // 1. Get or Compile Template from Cache - val compiledTemplate = - templateCache.getOrPut(templateId) { - if (debugEnabled) { - log.debug( - "Template cache miss for ID {}, path {}, MIME type {}, compression {}}", - templateId, - path, - dbMimeType, - compression, - ) - } - - val tQuery = "SELECT content FROM Templates WHERE id = ?" - val tCursor = database.rawQuery(tQuery, arrayOf(templateId.toString())) - tCursor.use { cursor -> - when { - cursor.count == 0 -> { - log.debug( - "Template not found, for ID {}, path {}, MIME type {}, compression {}", - templateId, - path, - dbMimeType, - compression, - ) - throw Exception("Template ID $templateId not found in the database") - } - - cursor.count > 1 -> { - log.debug( - "More than one template found, for ID {}, path {}, MIME type {}, compression {}", - templateId, - path, - dbMimeType, - compression, - ) - throw Exception("Template ID $templateId is shared by more than one template") - } - - !cursor.moveToFirst() -> { - log.debug( - "Template not found, for ID {}, path {}, MIME type {}, compression {}", - templateId, - path, - dbMimeType, - compression, - ) - throw Exception("Template ID $templateId not found in database.") - } - - else -> { - val templateBlob = cursor.getBlob(0) - if (debugEnabled) log.debug("templateBlob = '${String(templateBlob)}'") - pebbleEngine.getTemplate(templateBlob.toString(Charsets.UTF_8)) - } - } - } - } - - // Load JSON data into a template context Map<> for instantiation - val dbContentStr = dbContent.toString(Charsets.UTF_8) - if (dbContentStr.isBlank() || dbContentStr.trim() == "null") { - throw Exception("Template ID $templateId has empty or null JSON context") } - val context: Map = gson.fromJson(dbContentStr, dbContextType) - - // Evaluate template with loaded data and return the output - val sw = StringWriter() - compiledTemplate.evaluate(sw, context) - return sw.toString().toByteArray() } /** @@ -783,6 +424,36 @@ class WebServer( var html: String try { + html = contentSource.withDatabase { database -> lastChangeTableHtml(database) } + + if (debugEnabled) log.debug("html is '{}'.", html) + } catch (e: Exception) { + log.error("Error creating output for /pr/db endpoint: {}", e.message) + sendError( + writer, + output, + httpInternalServerError, + "Internal Server Error 4.1", + "Error creating output.", + ) + return + } + + try { + writeNormalToClient(writer, output, html) + + if (debugEnabled) log.debug("Leaving handleDbEndpoint().") + } catch (e: Exception) { + log.error("Error handling /pr/db endpoint: {}", e.message) + sendError(writer, output, httpInternalServerError, "Internal Server Error 4", "Error generating database table.", true) + } + } + + /** The `LastChange` table, 20 most recent rows, as an HTML table. */ + private fun lastChangeTableHtml(database: SQLiteDatabase): String { + var html: String + + run { // First, get the schema of the LastChange table to determine column count val schemaQuery = "PRAGMA table_info(LastChange)" val schemaCursor = database.rawQuery(schemaQuery, arrayOf()) @@ -845,28 +516,9 @@ class WebServer( } finally { dataCursor.close() } - - if (debugEnabled) log.debug("html is '{}'.", html) - } catch (e: Exception) { - log.error("Error creating output for /pr/db endpoint: {}", e.message) - sendError( - writer, - output, - httpInternalServerError, - "Internal Server Error 4.1", - "Error creating output.", - ) - return } - try { - writeNormalToClient(writer, output, html) - - if (debugEnabled) log.debug("Leaving handleDbEndpoint().") - } catch (e: Exception) { - log.error("Error handling /pr/db endpoint: {}", e.message) - sendError(writer, output, httpInternalServerError, "Internal Server Error 4", "Error generating database table.", true) - } + return html } /** @@ -883,7 +535,7 @@ class WebServer( output: java.io.OutputStream, ) { if (debugEnabled) log.debug("Entering handleBsEndpoint().") - if (clearCacheEnabled) templateCache.clear() + if (clearCacheEnabled) contentSource.clearTemplateCache() var outputStarted = false @@ -991,44 +643,48 @@ ORDER BY BC.category, ); """.trimIndent() - var cursor = database.rawQuery(sqlQuery, arrayOf()) - lateinit var jsonText: ByteArray + // Null means an error response has already been sent, so there is nothing left to write. + val jsonText = + contentSource.withDatabase { database -> + var cursor = database.rawQuery(sqlQuery, arrayOf()) - // Process database fetch - try { - if (!isCursorOneRow(cursor, writer, output)) { - return false - } + try { + if (!isCursorOneRow(cursor, writer, output)) { + return@withDatabase null + } - // get the JSON from the bookshelf table - cursor.moveToFirst() - jsonText = cursor.getBlob(0) - if (debugEnabled) log.debug("json content = '${String(jsonText)}'.") - if (debugEnabled) log.debug("before fetch bookshelf template ID = '$bookshelfTemplateId'") + // get the JSON from the bookshelf table + cursor.moveToFirst() + val json = cursor.getBlob(0) + if (debugEnabled) log.debug("json content = '${String(json)}'.") + if (debugEnabled) log.debug("before fetch bookshelf template ID = '$bookshelfTemplateId'") - // Have we already fetched the template - if (bookshelfTemplateId == -1) { - // safety first, close the cursor - cursor.close() - cursor = database.rawQuery("SELECT id FROM Templates WHERE name = 'bookshelf'", arrayOf()) + // Have we already fetched the template + if (bookshelfTemplateId == -1) { + // safety first, close the cursor + cursor.close() + cursor = database.rawQuery("SELECT id FROM Templates WHERE name = 'bookshelf'", arrayOf()) - if (!isCursorOneRow(cursor, writer, output)) { - return false - } + if (!isCursorOneRow(cursor, writer, output)) { + return@withDatabase null + } - cursor.moveToFirst() - bookshelfTemplateId = cursor.getInt(0) - if (debugEnabled) log.debug("after the fetch bookshelf template ID = '$bookshelfTemplateId'") - } - } catch (e: Exception) { - log.error("Error processing request: {}", e.message) - sendError(writer, output, httpInternalServerError, "Internal Server Error", e.message ?: "") - return false - } finally { - cursor.close() - } + cursor.moveToFirst() + bookshelfTemplateId = cursor.getInt(0) + if (debugEnabled) log.debug("after the fetch bookshelf template ID = '$bookshelfTemplateId'") + } + + json + } catch (e: Exception) { + log.error("Error processing request: {}", e.message) + sendError(writer, output, httpInternalServerError, "Internal Server Error", e.message ?: "") + null + } finally { + cursor.close() + } + } ?: return false - val result = instantiatePebbleTemplate(bookshelfTemplateId, jsonText, "/bookshelf", "application/json", "none") + val result = contentSource.renderTemplate(bookshelfTemplateId, jsonText, "/bookshelf") if (debugEnabled) log.debug("Bookshelf result is '{}'.", String(result)) diff --git a/app/src/test/java/com/itsaky/androidide/localWebServer/BrotliDictionaryDecodeTest.kt b/app/src/test/java/com/itsaky/androidide/localWebServer/BrotliDictionaryDecodeTest.kt index 80a1ac152c..3f7ebc27ba 100644 --- a/app/src/test/java/com/itsaky/androidide/localWebServer/BrotliDictionaryDecodeTest.kt +++ b/app/src/test/java/com/itsaky/androidide/localWebServer/BrotliDictionaryDecodeTest.kt @@ -1,9 +1,14 @@ package com.itsaky.androidide.localWebServer +// The decode helpers moved to common with the shared content source (ADFA-5176); these tests stay +// here, where the brotli4j host-native test wiring lives. import com.aayushatharva.brotli4j.Brotli4jLoader import com.aayushatharva.brotli4j.decoder.BrotliInputStream import com.aayushatharva.brotli4j.encoder.BrotliOutputStream import com.aayushatharva.brotli4j.encoder.Encoder +import com.itsaky.androidide.documentation.chunksAsStream +import com.itsaky.androidide.documentation.joinChunks +import com.itsaky.androidide.documentation.toDirectByteBuffer import org.junit.Assert.assertArrayEquals import org.junit.Assert.assertEquals import org.junit.Assert.assertSame diff --git a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt index 4f66e603e5..ed2b1d6091 100644 --- a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt +++ b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt @@ -59,27 +59,25 @@ class WebServerTest { projectDatabasePath = "/nonexistent/recent-projects.db", ) - // ADFA-5153/ADFA-5220: the dictionary is gated on the MAJOR version the database declares, so - // every test that expects the dictionary to load has to declare one. A relaxed mock answers the - // existence probe with moveToFirst() = false, i.e. "no version table", which would silently turn - // the dictionary tests below into no-ops rather than failing them. + // ADFA-5153/ADFA-5220: DocumentationContentSource gates the dictionary on the MAJOR version the + // database declares, so a test expecting the dictionary to load has to declare one. A relaxed + // mock answers the existence probe with moveToFirst() = false -- "no version table" -- which + // would quietly turn these tests into no-ops instead of failing them. private fun stubDeclaredMajorVersion( db: SQLiteDatabase, - major: Int?, + major: Int = DatabaseVersionResolver.MAJOR_VERSION_WITH_COMPRESSION_DICTIONARY, ) { every { db.rawQuery(match { it.contains("FROM sqlite_master") && it.contains("DocumentationDatabaseVersion") }, any()) - } returns mockk(relaxed = true) { every { moveToFirst() } returns (major != null) } - if (major != null) { - every { - db.rawQuery(match { it.contains("FROM DocumentationDatabaseVersion") }, any()) - } returns - mockk(relaxed = true) { - every { moveToFirst() } returns true - every { isNull(0) } returns false - every { getInt(0) } returns major - } - } + } returns mockk(relaxed = true) { every { moveToFirst() } returns true } + every { + db.rawQuery(match { it.contains("FROM DocumentationDatabaseVersion") }, any()) + } returns + mockk(relaxed = true) { + every { moveToFirst() } returns true + every { isNull(0) } returns false + every { getInt(0) } returns major + } } private fun freePort(): Int = ServerSocket(0).use { it.localPort } @@ -158,7 +156,7 @@ class WebServerTest { val db = mockk(relaxed = true) every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns db - stubDeclaredMajorVersion(db, DatabaseVersionResolver.MAJOR_VERSION_WITH_COMPRESSION_DICTIONARY) + stubDeclaredMajorVersion(db) every { db.rawQuery(match { it.contains("FROM sqlite_master") && it.contains("CompressionDictionary") }, null) } returns dictionaryExistsCursor @@ -227,7 +225,7 @@ class WebServerTest { db: SQLiteDatabase, dictionaryBytes: String, ) { - stubDeclaredMajorVersion(db, DatabaseVersionResolver.MAJOR_VERSION_WITH_COMPRESSION_DICTIONARY) + stubDeclaredMajorVersion(db) every { db.rawQuery(match { it.contains("FROM sqlite_master") && it.contains("CompressionDictionary") }, null) } returns mockk(relaxed = true) { every { moveToFirst() } returns true } @@ -248,7 +246,13 @@ class WebServerTest { stubDatabase(primaryDb, "dict-primary") stubDatabase(debugDb, "dict-debug") - val config = testConfig(port).copy(debugDatabasePath = debugDbFile.absolutePath) + // ADFA-5175 rate-limits the debug-database stat to once a second; this test drops a newer + // file and expects the very next request to see it, so it opts out of the rate limit. + val config = + testConfig(port).copy( + debugDatabasePath = debugDbFile.absolutePath, + debugDatabaseCheckIntervalMs = 0, + ) every { SQLiteDatabase.openDatabase(config.databasePath, isNull(), any()) } returns primaryDb every { SQLiteDatabase.openDatabase(config.debugDatabasePath, isNull(), any()) } returns debugDb @@ -302,85 +306,12 @@ class WebServerTest { } } - // ADFA-5153/ADFA-5220: below MAJOR 2 the dictionary is neither read nor attached, and the - // CompressionDictionary probe does not even run -- table sniffing is precisely what the version - // gate replaces, since a database can carry the table while its content is still plain brotli. - @Test - fun `a database declaring a version below 2 is never asked for a dictionary`() { - assertDictionaryLoads(declaredMajor = 1, expected = 0) - } - - @Test - fun `a database with no version table is never asked for a dictionary`() { - assertDictionaryLoads(declaredMajor = null, expected = 0) - } - - // A later format is still expected to carry the dictionary, so the gate is a floor, not a match. - @Test - fun `a database declaring a version above 2 still loads the dictionary`() { - assertDictionaryLoads(declaredMajor = 3, expected = 1) - } - - // The CompressionDictionary cursors are stubbed as *available* in every case, including the - // ones expecting zero queries: that is what makes this a test of the gate rather than of a - // missing table -- the queries are not skipped for want of an answer. - private fun assertDictionaryLoads( - declaredMajor: Int?, - expected: Int, - ) { - val port = freePort() - val db = mockk(relaxed = true) - every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns db - stubDeclaredMajorVersion(db, declaredMajor) - every { - db.rawQuery(match { it.contains("FROM sqlite_master") && it.contains("CompressionDictionary") }, null) - } returns mockk(relaxed = true) { every { moveToFirst() } returns true } - every { - db.rawQuery(match { it.contains("SELECT data FROM CompressionDictionary") }, null) - } returns - mockk(relaxed = true) { - every { moveToFirst() } returns true - every { getBlob(0) } returns "test-dictionary-bytes".toByteArray() - } - every { - db.rawQuery(match { it.contains("FROM Content") }, any()) - } returns - mockk(relaxed = true) { - every { count } returns 1 - every { moveToFirst() } returns true - every { getBlob(0) } returns "hello".toByteArray() - every { getString(1) } returns "text/plain" - every { getString(2) } returns "none" - every { getInt(3) } returns 0 - } - - val server = WebServer(testConfig(port)) - val serverThread = Thread { server.start() }.apply { isDaemon = true } - serverThread.start() - try { - awaitPortBound(port) - sendRawGetRequestAndAwaitClose(port, "/some/path") - - verify(exactly = expected) { - db.rawQuery(match { it.contains("FROM sqlite_master") && it.contains("CompressionDictionary") }, null) - } - verify(exactly = expected) { - db.rawQuery(match { it.contains("SELECT data FROM CompressionDictionary") }, null) - } - // The version itself is read once per database either way -- the gate is consulted, and - // its answer cached, exactly like the dictionary it guards. - verify(exactly = 1) { - db.rawQuery(match { it.contains("FROM sqlite_master") && it.contains("DocumentationDatabaseVersion") }, any()) - } - } finally { - server.stop() - serverThread.join(2_000) - } - } - - // ADFA-5241: the helper decides the charset, but only a real response proves the header that - // reaches a client. Two thirds of the database's text rows are non-ASCII with no BOM, so an - // undeclared encoding renders them as mojibake in any client that does not assume UTF-8. + // Blocks until the server closes the connection (every response sends "Connection: close"), + // so by the time this returns the server has fully finished processing this one request -- + // making repeated calls a reliable way to serialize several full request/response cycles. + // ADFA-5241: the two transports have to answer the same way about what a response says, and + // only a real response proves what this one sends. The decision itself lives in + // ContentTypeHeaders, shared with DocumentationRequestInterceptor. @Test fun `a text response declares utf-8 and a binary one does not`() { assertContentTypeHeader(storedMimeType = "text/html", expected = "text/html; charset=utf-8") @@ -394,7 +325,7 @@ class WebServerTest { val port = freePort() val db = mockk(relaxed = true) every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns db - stubDeclaredMajorVersion(db, DatabaseVersionResolver.MAJOR_VERSION_WITH_COMPRESSION_DICTIONARY) + stubDeclaredMajorVersion(db) every { db.rawQuery(match { it.contains("FROM Content") }, any()) } returns @@ -436,9 +367,6 @@ class WebServerTest { socket.getInputStream().readBytes().toString(Charsets.ISO_8859_1) } - // Blocks until the server closes the connection (every response sends "Connection: close"), - // so by the time this returns the server has fully finished processing this one request -- - // making repeated calls a reliable way to serialize several full request/response cycles. private fun sendRawGetRequestAndAwaitClose( port: Int, path: String, diff --git a/common/build.gradle.kts b/common/build.gradle.kts index dc01fac8e9..7dbd920da8 100755 --- a/common/build.gradle.kts +++ b/common/build.gradle.kts @@ -59,4 +59,10 @@ dependencies { // brotli4j implementation(libs.brotli4j) + + // Documentation content: Brotli-decoded rows above, Pebble-rendered pages, gson for their + // template context. Moved down from `app` with the shared content source (ADFA-5176), so both + // the web server and the in-process WebView path render the same way. + implementation(libs.pebble) + implementation(libs.google.gson) } diff --git a/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt b/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt index e2bb9c7cc9..d69bcf8354 100644 --- a/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt +++ b/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt @@ -28,179 +28,246 @@ import android.webkit.WebViewClient import androidx.activity.OnBackPressedCallback import androidx.core.net.toUri import androidx.core.view.WindowCompat -import org.adfa.constants.CONTENT_KEY -import com.itsaky.androidide.resources.R import com.itsaky.androidide.app.BaseIDEActivity -import com.itsaky.androidide.common.R as CommonR import com.itsaky.androidide.common.databinding.ActivityHelpBinding +import com.itsaky.androidide.documentation.DocumentationRequestInterceptor +import com.itsaky.androidide.resources.R import com.itsaky.androidide.utils.DeviceFormFactorUtils -import com.itsaky.androidide.utils.isSystemInDarkMode import com.itsaky.androidide.utils.UrlManager import com.itsaky.androidide.utils.applyMultiWindowFlags +import com.itsaky.androidide.utils.isSystemInDarkMode +import org.adfa.constants.CONTENT_KEY import org.adfa.constants.CONTENT_TITLE_KEY +import org.slf4j.LoggerFactory +import com.itsaky.androidide.common.R as CommonR class HelpActivity : BaseIDEActivity() { + companion object { + private val EXTERNAL_SCHEMES = listOf("mailto:", "tel:", "sms:") + private const val MULTI_WINDOW_URI = "cogo-help://tooltip/active-window" + + fun launch( + context: Context, + url: String, + title: String, + ) { + val intent = + Intent(context, HelpActivity::class.java) + .apply { + putExtra(CONTENT_KEY, url) + putExtra(CONTENT_TITLE_KEY, title) + + if (DeviceFormFactorUtils.getCurrent(context).isLargeScreenLike) { + data = MULTI_WINDOW_URI.toUri() + } + }.applyMultiWindowFlags(context) + context.startActivity(intent) + } + } + + private val log = LoggerFactory.getLogger(HelpActivity::class.java) + + // ADFA-5176: answers documentation requests from the database in-process, so loading a page + // no longer opens a TCP connection per asset to the local web server. + private val documentation = DocumentationRequestInterceptor.shared + + // Wall-clock start of the page currently loading, for the ADFA-5176 measurement. + private var pageLoadStartMillis = 0L + + @Suppress("ktlint:standard:backing-property-naming") + private var _binding: ActivityHelpBinding? = null + private val binding: ActivityHelpBinding + get() = + checkNotNull(_binding) { + "HelpActivity has been destroyed" + } + + override fun bindLayout(): View { + _binding = ActivityHelpBinding.inflate(layoutInflater) + return binding.root + } + + override fun onCreate(savedInstanceState: Bundle?) { + super.onCreate(savedInstanceState) + + with(binding) { + setSupportActionBar(toolbar) + supportActionBar!!.setDisplayHomeAsUpEnabled(true) + toolbar.setNavigationOnClickListener { handleBackNavigation() } + + // Set status bar icons to be dark in light mode and light in dark mode + WindowCompat.getInsetsController(this@HelpActivity.window, this@HelpActivity.window.decorView).apply { + isAppearanceLightStatusBars = !isSystemInDarkMode() + isAppearanceLightNavigationBars = !isSystemInDarkMode() + } + + val pageTitle = intent.getStringExtra(CONTENT_TITLE_KEY) + val htmlContent = intent.getStringExtra(CONTENT_KEY) + + supportActionBar?.title = pageTitle ?: getString(R.string.help) + + // Configure WebView settings for localhost access + webView.settings.javaScriptEnabled = true + webView.settings.allowFileAccess = true + webView.settings.allowFileAccessFromFileURLs = true + webView.settings.allowUniversalAccessFromFileURLs = true + webView.settings.domStorageEnabled = true + webView.settings.databaseEnabled = true + webView.settings.mixedContentMode = android.webkit.WebSettings.MIXED_CONTENT_ALWAYS_ALLOW + + // Set WebViewClient to handle page navigation within the WebView + webView.webViewClient = + object : WebViewClient() { + override fun shouldInterceptRequest( + view: android.webkit.WebView, + request: android.webkit.WebResourceRequest, + ): android.webkit.WebResourceResponse? = documentation.intercept(request) ?: super.shouldInterceptRequest(view, request) + + override fun onPageStarted( + view: android.webkit.WebView?, + url: String?, + favicon: android.graphics.Bitmap?, + ) { + super.onPageStarted(view, url, favicon) + pageLoadStartMillis = System.currentTimeMillis() + } + + override fun onPageFinished( + view: android.webkit.WebView?, + url: String?, + ) { + super.onPageFinished(view, url) + invalidateOptionsMenu() + + if (pageLoadStartMillis != 0L) { + log.info( + "Loaded '{}' in {} ms; {}.", + url, + System.currentTimeMillis() - pageLoadStartMillis, + documentation.servedSummary(), + ) + pageLoadStartMillis = 0L + } + } + + override fun doUpdateVisitedHistory( + view: android.webkit.WebView?, + url: String?, + isReload: Boolean, + ) { + super.doUpdateVisitedHistory(view, url, isReload) + invalidateOptionsMenu() + } + + override fun shouldOverrideUrlLoading( + view: android.webkit.WebView?, + url: String?, + ): Boolean = handleUrlLoading(view, url) + + override fun onReceivedError( + view: android.webkit.WebView?, + errorCode: Int, + description: String?, + failingUrl: String?, + ) { + super.onReceivedError(view, errorCode, description, failingUrl) + view?.loadData( + """ + +

Error Loading Content

+

Unable to load: $failingUrl

+

Error: $description

+ + """.trimIndent(), + "text/html", + "UTF-8", + ) + } + } + + // Load the HTML file from the assets folder + htmlContent?.let { url -> + webView.loadUrl(url) + } + } + + // Set up back navigation callback for system back button + onBackPressedDispatcher.addCallback( + this, + object : OnBackPressedCallback(true) { + override fun handleOnBackPressed() { + handleBackNavigation() + } + }, + ) + updateUIFromIntent(intent) + } + + override fun onNewIntent(intent: Intent) { + super.onNewIntent(intent) + setIntent(intent) + updateUIFromIntent(intent) + } + + private fun updateUIFromIntent(currentIntent: Intent) { + val pageTitle = currentIntent.getStringExtra(CONTENT_TITLE_KEY) + supportActionBar?.title = pageTitle ?: getString(R.string.help) + + currentIntent.getStringExtra(CONTENT_KEY)?.let { url -> + binding.webView.loadUrl(url) + } + } + + private fun handleUrlLoading( + view: android.webkit.WebView?, + url: String?, + ): Boolean { + url ?: return false + return when { + EXTERNAL_SCHEMES.any { url.startsWith(it) } -> { + UrlManager.openUrl(url, context = this) + true + } + + url.startsWith("http://localhost:6174/") -> { + view?.loadUrl(url) + true + } + + else -> { + false + } + } + } + + override fun onCreateOptionsMenu(menu: Menu): Boolean { + menuInflater.inflate(CommonR.menu.menu_help, menu) + return true + } + + override fun onPrepareOptionsMenu(menu: Menu): Boolean { + menu.findItem(CommonR.id.action_close_help)?.isVisible = + _binding != null && binding.webView.canGoBack() + return super.onPrepareOptionsMenu(menu) + } + + override fun onOptionsItemSelected(item: MenuItem): Boolean = + when (item.itemId) { + CommonR.id.action_close_help -> { + finish() + true + } - companion object { - private val EXTERNAL_SCHEMES = listOf("mailto:", "tel:", "sms:") - private const val MULTI_WINDOW_URI = "cogo-help://tooltip/active-window" - - fun launch(context: Context, url: String, title: String) { - val intent = Intent(context, HelpActivity::class.java).apply { - putExtra(CONTENT_KEY, url) - putExtra(CONTENT_TITLE_KEY, title) - - if (DeviceFormFactorUtils.getCurrent(context).isLargeScreenLike) { - data = MULTI_WINDOW_URI.toUri() - } - }.applyMultiWindowFlags(context) - context.startActivity(intent) - } - } - - private var _binding: ActivityHelpBinding? = null - private val binding: ActivityHelpBinding - get() = checkNotNull(_binding) { - "HelpActivity has been destroyed" - } - - override fun bindLayout(): View { - _binding = ActivityHelpBinding.inflate(layoutInflater) - return binding.root - } - - override fun onCreate(savedInstanceState: Bundle?) { - super.onCreate(savedInstanceState) - - with(binding) { - setSupportActionBar(toolbar) - supportActionBar!!.setDisplayHomeAsUpEnabled(true) - toolbar.setNavigationOnClickListener { handleBackNavigation() } - - // Set status bar icons to be dark in light mode and light in dark mode - WindowCompat.getInsetsController(this@HelpActivity.window, this@HelpActivity.window.decorView).apply { - isAppearanceLightStatusBars = !isSystemInDarkMode() - isAppearanceLightNavigationBars = !isSystemInDarkMode() - } - - val pageTitle = intent.getStringExtra(CONTENT_TITLE_KEY) - val htmlContent = intent.getStringExtra(CONTENT_KEY) - - supportActionBar?.title = pageTitle ?: getString(R.string.help) - - // Configure WebView settings for localhost access - webView.settings.javaScriptEnabled = true - webView.settings.allowFileAccess = true - webView.settings.allowFileAccessFromFileURLs = true - webView.settings.allowUniversalAccessFromFileURLs = true - webView.settings.domStorageEnabled = true - webView.settings.databaseEnabled = true - webView.settings.mixedContentMode = android.webkit.WebSettings.MIXED_CONTENT_ALWAYS_ALLOW - - // Set WebViewClient to handle page navigation within the WebView - webView.webViewClient = object : WebViewClient() { - override fun onPageStarted(view: android.webkit.WebView?, url: String?, favicon: android.graphics.Bitmap?) { - super.onPageStarted(view, url, favicon) - } - - override fun onPageFinished(view: android.webkit.WebView?, url: String?) { - super.onPageFinished(view, url) - invalidateOptionsMenu() - } - - override fun doUpdateVisitedHistory(view: android.webkit.WebView?, url: String?, isReload: Boolean) { - super.doUpdateVisitedHistory(view, url, isReload) - invalidateOptionsMenu() - } - - override fun shouldOverrideUrlLoading(view: android.webkit.WebView?, url: String?): Boolean { - return handleUrlLoading(view, url) - } - - override fun onReceivedError(view: android.webkit.WebView?, errorCode: Int, description: String?, failingUrl: String?) { - super.onReceivedError(view, errorCode, description, failingUrl) - view?.loadData(""" - -

Error Loading Content

-

Unable to load: $failingUrl

-

Error: $description

- - """.trimIndent(), "text/html", "UTF-8") - } - } - - // Load the HTML file from the assets folder - htmlContent?.let { url -> - webView.loadUrl(url) - } - } - - // Set up back navigation callback for system back button - onBackPressedDispatcher.addCallback(this, object : OnBackPressedCallback(true) { - override fun handleOnBackPressed() { - handleBackNavigation() - } - }) - updateUIFromIntent(intent) - } - - override fun onNewIntent(intent: Intent) { - super.onNewIntent(intent) - setIntent(intent) - updateUIFromIntent(intent) - } - - private fun updateUIFromIntent(currentIntent: Intent) { - val pageTitle = currentIntent.getStringExtra(CONTENT_TITLE_KEY) - supportActionBar?.title = pageTitle ?: getString(R.string.help) - - currentIntent.getStringExtra(CONTENT_KEY)?.let { url -> - binding.webView.loadUrl(url) - } - } - - private fun handleUrlLoading(view: android.webkit.WebView?, url: String?): Boolean { - url ?: return false - return when { - EXTERNAL_SCHEMES.any { url.startsWith(it) } -> { - UrlManager.openUrl(url, context = this) - true - } - url.startsWith("http://localhost:6174/") -> { - view?.loadUrl(url) - true - } - else -> false - } - } - - override fun onCreateOptionsMenu(menu: Menu): Boolean { - menuInflater.inflate(CommonR.menu.menu_help, menu) - return true - } - - override fun onPrepareOptionsMenu(menu: Menu): Boolean { - menu.findItem(CommonR.id.action_close_help)?.isVisible = - _binding != null && binding.webView.canGoBack() - return super.onPrepareOptionsMenu(menu) - } - - override fun onOptionsItemSelected(item: MenuItem): Boolean { - return when (item.itemId) { - CommonR.id.action_close_help -> { - finish() - true - } - else -> super.onOptionsItemSelected(item) - } - } - - private fun handleBackNavigation() { - if (binding.webView.canGoBack()) { - binding.webView.goBack() - } else { - finish() - } - } + else -> { + super.onOptionsItemSelected(item) + } + } + private fun handleBackNavigation() { + if (binding.webView.canGoBack()) { + binding.webView.goBack() + } else { + finish() + } + } } diff --git a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt new file mode 100644 index 0000000000..78c083171a --- /dev/null +++ b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt @@ -0,0 +1,578 @@ +/* + * This file is part of Code on the Go. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.documentation + +import android.database.sqlite.SQLiteDatabase +import com.aayushatharva.brotli4j.decoder.BrotliInputStream +import com.google.gson.Gson +import com.google.gson.GsonBuilder +import com.google.gson.ToNumberPolicy +import com.google.gson.reflect.TypeToken +import com.itsaky.androidide.utils.DatabaseVersionResolver +import io.pebbletemplates.pebble.PebbleEngine +import io.pebbletemplates.pebble.loader.StringLoader +import io.pebbletemplates.pebble.template.PebbleTemplate +import org.slf4j.LoggerFactory +import java.io.ByteArrayInputStream +import java.io.Closeable +import java.io.File +import java.io.IOException +import java.io.InputStream +import java.io.SequenceInputStream +import java.io.StringWriter +import java.nio.ByteBuffer +import java.text.SimpleDateFormat +import java.util.Collections +import java.util.Date +import java.util.Locale +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicLong +import java.util.concurrent.locks.ReentrantReadWriteLock +import kotlin.concurrent.read +import kotlin.concurrent.write + +/** + * Copies [bytes] into a direct [ByteBuffer] -- brotli4j's `attachDictionary` requires a direct + * buffer, a heap-backed one throws `IllegalArgumentException`. + * + * The capacity must be exactly [bytes]`.size`: `attachDictionary` reads the whole capacity and + * ignores position/limit, so trailing slack from an over-allocated buffer is treated as dictionary + * content and every decode then fails with `IOException: corrupted input`. + */ +fun toDirectByteBuffer(bytes: ByteArray): ByteBuffer = + ByteBuffer.allocateDirect(bytes.size).apply { + put(bytes) + flip() + } + +/** + * Reads [chunks] back to back as one stream, without concatenating them into a new array. + * Cheap to build twice, which the no-dictionary retry in [DocumentationContentSource] relies on. + */ +fun chunksAsStream(chunks: List): InputStream = + SequenceInputStream(Collections.enumeration(chunks.map { ByteArrayInputStream(it) })) + +/** + * Joins [chunks] into one exactly-sized array. A ByteArrayOutputStream would repeatedly double its + * buffer and then hand back a second full copy -- avoidable here since the total is known up front. + * Returns the sole element as-is when there is nothing to join. + */ +fun joinChunks(chunks: List): ByteArray { + if (chunks.size == 1) { + return chunks[0] + } + val joined = ByteArray(chunks.sumOf { it.size }) + var offset = 0 + for (chunk in chunks) { + chunk.copyInto(joined, offset) + offset += chunk.size + } + return joined +} + +/** One row of documentation content, decoded and rendered, ready to send. */ +data class DocumentationContent( + val bytes: ByteArray, + val mimeType: String, +) { + // Data class equality over a ByteArray would compare identity, which is never what a caller + // means; content equality on a multi-megabyte blob is not what it wants either. + override fun equals(other: Any?): Boolean = this === other + + override fun hashCode(): Int = System.identityHashCode(this) +} + +/** What a [DocumentationContentSource.lookup] found for a path. */ +sealed interface DocumentationLookup { + data class Found( + val content: DocumentationContent, + ) : DocumentationLookup + + object NotFound : DocumentationLookup + + /** The path matched more than one row, which means the database is corrupt. */ + data class Ambiguous( + val rowCount: Int, + ) : DocumentationLookup + + /** The read itself failed. */ + data class Failed( + val cause: Exception, + ) : DocumentationLookup +} + +/** + * Reads documentation content out of `documentation.db`: the row lookup, reassembly of chunked + * rows, the shared-dictionary Brotli decode (ADFA-5153), and the swap to a newer database dropped + * on the sdcard. + * + * One pipeline with two callers (ADFA-5176): `WebServer`, which wraps it in HTTP, and + * [DocumentationRequestInterceptor], which answers a WebView in-process with no socket at all. A row + * that is a Pebble template context is rendered here too, so both transports serve a finished page + * and neither needs the template engine itself. + * + * Thread-safe: [lookup] and [withDatabase] hold a read lock for the whole read, and the swap takes + * the write lock, since swapping closes the handle a reader could be using. Readers never block + * each other. + */ +class DocumentationContentSource( + private val databaseFile: File, + private val debugDatabaseFile: File, + debugCheckIntervalMs: Long = 1_000, +) : Closeable { + private val log = LoggerFactory.getLogger(DocumentationContentSource::class.java) + + private val databaseLock = ReentrantReadWriteLock() + + // Read without the lock in the open-on-demand check, hence volatile. + @Volatile + private var database: SQLiteDatabase? = null + + @Volatile + private var databaseTimestamp: Long = -1 + + /** + * Bumped on every swap, so a caller can tell that anything it cached from this source -- + * a compiled template, a looked-up template id -- belongs to a database that is gone. + */ + @Volatile + var generation: Long = 0 + private set + + // A debug database whose swap already failed, so a corrupt or unreadable one is not reopened + // on every check. A newer copy has a different timestamp and is retried, which is the case + // that matters: replacing the file is exactly how a developer fixes it. + private var failedDebugSwapTimestamp: Long = -1 + + // The dictionary the Content rows are compressed against. Loaded on the first decode that + // needs it after a swap rather than eagerly, and then cached for that database. Null when the + // active database predates the dictionary migration -- CompressionDictionary won't exist. + private var compressionDictionary: ByteBuffer? = null + private var compressionDictionaryStale = true + + private val pebbleEngine = PebbleEngine.Builder().loader(StringLoader()).build() + + // Compiled templates for the active database, cleared when it is swapped. + private val templateCache = ConcurrentHashMap() + + private val gson: Gson = + GsonBuilder() + .setObjectToNumberStrategy(ToNumberPolicy.LONG_OR_DOUBLE) + .create() + + private val templateContextType = object : TypeToken>() {}.type + + private val debugCheckIntervalNanos = TimeUnit.MILLISECONDS.toNanos(debugCheckIntervalMs) + + // One interval in the past, so the first lookup still checks for a debug database. + private val lastDebugCheckNanos = AtomicLong(System.nanoTime() - debugCheckIntervalNanos) + + /** + * Opens the installed database. Callers that need to fail loudly (the web server, which should + * not bind a port it cannot serve) call this; the rest let [lookup] open it on demand. + */ + fun open() { + databaseLock.write { + if (database != null) return@write + switchToDatabase(databaseFile.absolutePath, timestampOf(databaseFile)) + } + } + + /** The row for [path], decoded but not rendered. */ + fun lookup(path: String): DocumentationLookup { + // Both of these take the write lock when they act, so they run before the read lock below: + // a ReentrantReadWriteLock does not upgrade. + if (!openIfNeeded()) return DocumentationLookup.NotFound + swapDebugDatabaseIfNewer() + + return databaseLock.read { + val database = database ?: return@read DocumentationLookup.NotFound + + try { + readContent(database, path) + } catch (e: Exception) { + log.error("Cannot read '{}': {}", path, e.message) + DocumentationLookup.Failed(e) + } + } + } + + /** + * Runs [block] against the active database with the read lock held, for the queries this class + * does not own -- the bookshelf join, the template lookup, the developer table dumps. + */ + fun withDatabase(block: (SQLiteDatabase) -> T): T { + openIfNeeded() + swapDebugDatabaseIfNewer() + + return databaseLock.read { + val database = checkNotNull(database) { "documentation database '$databaseFile' is not open" } + block(database) + } + } + + /** + * Renders [contextJson] through the template [templateId] -- for a caller that has the context + * and the template id in hand, rather than a path to look up. [path] is for diagnostics only. + */ + fun renderTemplate( + templateId: Int, + contextJson: ByteArray, + path: String, + ): ByteArray = withDatabase { database -> render(database, templateId, contextJson, path) } + + /** Drops the compiled templates, for the developer sentinel that forces a re-render. */ + fun clearTemplateCache() { + templateCache.clear() + } + + /** The last-modified time of [pathname], or -1 when it does not exist. */ + fun timestampOf( + file: File, + silent: Boolean = true, + ): Long { + if (!file.exists()) return -1 + + val timestamp = file.lastModified() + if (!silent) { + val format = SimpleDateFormat("yyyy-MM-dd HH:mm:ss", Locale.getDefault()) + log.debug("{} was last modified at {}.", file, format.format(Date(timestamp))) + } + + return timestamp + } + + override fun close() { + databaseLock.write { + try { + database?.close() + } catch (e: Exception) { + log.error("Cannot close the documentation database: {}", e.message) + } + database = null + } + } + + /** True once the database is open. Failure is logged, not thrown: a caller falls back instead. */ + private fun openIfNeeded(): Boolean { + if (database != null) return true + + return try { + open() + database != null + } catch (e: Exception) { + log.error("Cannot open the documentation database '{}': {}", databaseFile, e.message) + false + } + } + + private fun readContent( + database: SQLiteDatabase, + path: String, + ): DocumentationLookup { + // Primed before the row is read, not inside decompressBrotli, so a database's dictionary is + // loaded on its first content fetch (ADFA-5153's contract) rather than on the first fetch + // that happens to be Brotli-compressed. Still at most once per database. + compressionDictionary(database) + + database.rawQuery(CONTENT_QUERY, arrayOf(path)).use { cursor -> + if (cursor.count == 0) return DocumentationLookup.NotFound + if (cursor.count != 1) return DocumentationLookup.Ambiguous(cursor.count) + + cursor.moveToFirst() + val firstChunk = cursor.getBlob(0) + val mimeType = cursor.getString(1) + val compression = cursor.getString(2) + val templateId = cursor.getInt(3) + + val chunks = readChunks(database, path, firstChunk) + val decoded = if (compression == "brotli") decompressBrotli(database, chunks) else joinChunks(chunks) + val bytes = if (templateId > 0) render(database, templateId, decoded, path) else decoded + + return DocumentationLookup.Found(DocumentationContent(bytes, mimeType)) + } + } + + /** + * Renders one template: [contextJson] is the row's JSON, [templateId] names the template row. + * Compiled templates are cached per database, so a repeat visit re-renders without recompiling. + */ + private fun render( + database: SQLiteDatabase, + templateId: Int, + contextJson: ByteArray, + path: String, + ): ByteArray { + val template = + templateCache.getOrPut(templateId) { + if (log.isDebugEnabled) log.debug("Template cache miss for id {}, path '{}'.", templateId, path) + compileTemplate(database, templateId, path) + } + + val contextString = contextJson.toString(Charsets.UTF_8) + if (contextString.isBlank() || contextString.trim() == "null") { + throw IllegalStateException("Template ID $templateId has empty or null JSON context") + } + val context: Map = gson.fromJson(contextString, templateContextType) + + return StringWriter().also { template.evaluate(it, context) }.toString().toByteArray() + } + + private fun compileTemplate( + database: SQLiteDatabase, + templateId: Int, + path: String, + ): PebbleTemplate = + database.rawQuery("SELECT content FROM Templates WHERE id = ?", arrayOf(templateId.toString())).use { cursor -> + when { + cursor.count > 1 -> { + throw IllegalStateException("Template ID $templateId is shared by more than one template") + } + + !cursor.moveToFirst() -> { + throw IllegalStateException("Template ID $templateId not found in the database, for path '$path'") + } + + else -> { + val body = cursor.getBlob(0) + if (log.isDebugEnabled) log.debug("Compiling template {}, {} bytes.", templateId, body.size) + pebbleEngine.getTemplate(body.toString(Charsets.UTF_8)) + } + } + } + + /** + * Content over [CONTENT_CHUNK_SIZE] is split across rows named `path-1`, `path-2`, ... The + * chunks stay a list rather than being concatenated: accumulating into a + * ByteArrayOutputStream held its doubling buffer *and* the copy from toByteArray() live + * alongside the decompressed output, roughly 35 MB transient for the largest bundled PDF. + */ + private fun readChunks( + database: SQLiteDatabase, + path: String, + firstChunk: ByteArray, + ): List { + val chunks = mutableListOf(firstChunk) + if (firstChunk.size != CONTENT_CHUNK_SIZE) return chunks + + var chunkNumber = 1 + var chunk = firstChunk + while (chunk.size == CONTENT_CHUNK_SIZE) { + database.rawQuery(CHUNK_QUERY, arrayOf("$path-$chunkNumber")).use { cursor -> + if (!cursor.moveToFirst()) return chunks + chunk = cursor.getBlob(0) + chunks.add(chunk) + chunkNumber++ + } + } + + return chunks + } + + /** + * Decompresses one Brotli row. Tries the shared dictionary first, since every migrated row + * requires it, then falls back to a plain decode for rows that were never dictionary + * compressed: plugin-contributed Tier 3 docs (PluginDocumentationManager/BrotliCompressor + * compress with no dictionary) or any row from a pre-migration database. Attaching a dictionary + * to a stream that was not compressed against one reliably fails rather than silently producing + * wrong bytes (verified empirically -- see docs/documentation-database.md), so this ordering + * never lets a dictionary-compressed row fall through to the plain path by accident. + */ + private fun decompressBrotli( + database: SQLiteDatabase, + chunks: List, + ): ByteArray { + val dictionary = compressionDictionary(database) + if (dictionary != null) { + try { + return BrotliInputStream(chunksAsStream(chunks)).use { stream -> + stream.attachDictionary(dictionary) + stream.readBytes() + } + } catch (e: IOException) { + log.debug( + "Dictionary decode failed for a brotli row (likely dictionary-free plugin content); retrying without a dictionary: {}", + e.message, + ) + } + } + + return BrotliInputStream(chunksAsStream(chunks)).use { it.readBytes() } + } + + /** + * The active database's shared dictionary, loaded at most once per database. Synchronized rather + * than volatile-checked: two threads loading a 256 KB direct buffer in parallel is worth + * avoiding, and the read lock a caller already holds does not exclude them. + * + * The stale flag is cleared only after a *clean* load -- a dictionary, or a definitive absence. + * An unexpected failure propagates and leaves the flag set, so the next request retries instead + * of caching a transient error as "this database has no dictionary" for the rest of its life + * (ADFA-5153 review). The caller turns that into one failed request, not a permanent downgrade. + */ + private fun compressionDictionary(database: SQLiteDatabase): ByteBuffer? = + synchronized(this) { + if (compressionDictionaryStale) { + compressionDictionary = dictionaryBytes(database)?.let { toDirectByteBuffer(it) } + compressionDictionaryStale = false + } + compressionDictionary + } + + /** + * The dictionary blob, or null -- logged -- when this database *definitively* has none: it + * declares a documentation version below + * [DatabaseVersionResolver.MAJOR_VERSION_WITH_COMPRESSION_DICTIONARY], or its + * `CompressionDictionary` row is missing, null or empty. Any other failure is left to propagate, + * deliberately (see [compressionDictionary]). + * + * The gate is the declared version rather than the table's presence, matching `WebServer` -- + * table sniffing infers a whole content format from one table existing, and gets it wrong in + * both directions (ADFA-5220). The table checks below still run, for a database that declares a + * new-enough version but has no usable row: without them the data query raises "no such table", + * which the caller treats as transient and would retry on every request. + */ + private fun dictionaryBytes(database: SQLiteDatabase): ByteArray? { + val majorVersion = DatabaseVersionResolver.resolveMajorVersion(database) + if (majorVersion == null || + majorVersion < DatabaseVersionResolver.MAJOR_VERSION_WITH_COMPRESSION_DICTIONARY + ) { + log.warn( + "Database declares documentation version {}, below {}; decoding brotli content without a dictionary.", + majorVersion ?: "none", + DatabaseVersionResolver.MAJOR_VERSION_WITH_COMPRESSION_DICTIONARY, + ) + return null + } + + val tableExists = + database + .rawQuery( + "SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'CompressionDictionary'", + null, + ).use { it.moveToFirst() } + if (!tableExists) { + log.warn("CompressionDictionary table not found; decoding brotli content without a dictionary.") + return null + } + + return database.rawQuery("SELECT data FROM CompressionDictionary WHERE id = 1", null).use { cursor -> + if (!cursor.moveToFirst()) { + log.warn("CompressionDictionary table is empty; decoding brotli content without a dictionary.") + return null + } + + val bytes = cursor.getBlob(0) + when { + bytes == null -> { + log.warn("CompressionDictionary row has a NULL data column; decoding brotli content without a dictionary.") + null + } + + // An empty blob yields a 0-capacity buffer, which attachDictionary rejects -- every + // decode would then fail with nothing above DEBUG to say why. + bytes.isEmpty() -> { + log.warn("CompressionDictionary row has an empty data column; decoding brotli content without a dictionary.") + null + } + + else -> { + bytes + } + } + } + } + + /** + * Swaps to the sdcard debug database when a newer one has appeared. The stat behind this is + * rate-limited: the path is FUSE-backed emulated storage, and it is a developer-only override, + * so it used to cost a stat on every single request (ADFA-5175). Must not be called with the + * read lock held -- it takes the write lock, and this lock does not upgrade. + */ + private fun swapDebugDatabaseIfNewer() { + val debugTimestamp = debugTimestampIfDue() ?: return + if (debugTimestamp <= databaseTimestamp || debugTimestamp == failedDebugSwapTimestamp) return + + databaseLock.write { + // Another thread may have swapped while this one waited for the lock. + if (debugTimestamp <= databaseTimestamp || debugTimestamp == failedDebugSwapTimestamp) return@write + + try { + switchToDatabase(debugDatabaseFile.absolutePath, debugTimestamp) + failedDebugSwapTimestamp = -1 + log.info("Swapped to the debug database '{}'.", debugDatabaseFile) + } catch (e: Exception) { + failedDebugSwapTimestamp = debugTimestamp + log.error( + "Cannot swap to debug database '{}'; ignoring it until it changes: {}", + debugDatabaseFile, + e.message, + ) + } + } + } + + /** The debug database's timestamp, or null when the last check was too recent. */ + private fun debugTimestampIfDue(): Long? { + val now = System.nanoTime() + val last = lastDebugCheckNanos.get() + if (now - last < debugCheckIntervalNanos) return null + if (!lastDebugCheckNanos.compareAndSet(last, now)) return null + + return timestampOf(debugDatabaseFile) + } + + /** + * Opens [path] as the active database and refreshes everything that depends on which file is + * active, as one operation under the write lock. Opens the replacement before closing the old + * handle, so a failed open (this throws) leaves the previous database serving rather than + * leaving a closed handle behind. + */ + private fun switchToDatabase( + path: String, + timestamp: Long, + ) { + val opened = SQLiteDatabase.openDatabase(path, null, SQLiteDatabase.OPEN_READONLY) + val previous = database + + database = opened + databaseTimestamp = timestamp + compressionDictionaryStale = true + generation++ + + try { + previous?.close() + } catch (e: Exception) { + log.error("Cannot close previous database: {}", e.message) + } + } + + companion object { + const val CONTENT_CHUNK_SIZE = 1024 * 1024 + + private const val CONTENT_QUERY = """ + SELECT C.content, CT.value, CT.compression, C.templateId + FROM Content C, ContentTypes CT + WHERE C.contentTypeID = CT.id + AND C.path = ? + """ + + private const val CHUNK_QUERY = "SELECT content FROM Content WHERE path = ? AND languageId = 1" + } +} diff --git a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt new file mode 100644 index 0000000000..981b54a417 --- /dev/null +++ b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt @@ -0,0 +1,147 @@ +/* + * This file is part of Code on the Go. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.documentation + +import android.os.Environment.getExternalStorageDirectory +import android.webkit.WebResourceRequest +import android.webkit.WebResourceResponse +import com.aayushatharva.brotli4j.Brotli4jLoader +import com.itsaky.androidide.utils.ContentTypeHeaders +import com.itsaky.androidide.utils.Environment +import org.slf4j.LoggerFactory +import java.io.ByteArrayInputStream +import java.io.File +import java.util.concurrent.atomic.AtomicLong + +/** + * Answers documentation requests from `documentation.db` in-process, so a WebView never opens a + * socket to `WebServer` for them (ADFA-5176). + * + * Wire it into a WebView through `WebViewClient.shouldInterceptRequest`. The URLs do not change: + * this matches the same `http://localhost:6174/...` space the server listens on, which is what lets + * the strings.xml entries, ToolTipManager's link builder, and the plugin API contract stay as they + * are. Anything this returns null for -- a `/pr/` developer endpoint, an unknown path, a read that + * fails -- falls through to that server unchanged. + * + * Templated pages included: [DocumentationContentSource] renders them, so every documentation path + * a WebView asks for is answered here. + */ +class DocumentationRequestInterceptor( + private val contentSource: DocumentationContentSource, +) { + private val log = LoggerFactory.getLogger(DocumentationRequestInterceptor::class.java) + + // Measurement switch: creating the sentinel puts documentation back on the local web server, so + // one build can compare both transports. Read once, like the server's other file flags. + private val disabled = File(getExternalStorageDirectory(), DISABLE_SENTINEL).exists() + + private val servedRequests = AtomicLong() + private val servedBytes = AtomicLong() + + /** + * The response for [request], or null to let it go to the network. Called on WebView's own + * threads; [DocumentationContentSource] is what makes that safe. + */ + fun intercept(request: WebResourceRequest): WebResourceResponse? = contentFor(request)?.let { response(it) } + + /** + * The content to answer [request] with, or null when it is not this class's to answer. Split out + * from [intercept] so the decision can be tested without a framework WebResourceResponse. + */ + internal fun contentFor(request: WebResourceRequest): DocumentationContent? { + if (disabled) return null + if (!request.method.equals("GET", ignoreCase = true)) return null + + val url = request.url + if (url.host != SERVER_HOST || url.port != SERVER_PORT) return null + + val path = url.path?.removePrefix("/").orEmpty() + if (path.isEmpty() || path.startsWith("pr/")) return null + + val content = + when (val lookup = contentSource.lookup(path)) { + is DocumentationLookup.Found -> lookup.content + else -> return null + } + + servedRequests.incrementAndGet() + servedBytes.addAndGet(content.bytes.size.toLong()) + if (log.isDebugEnabled) log.debug("Served '{}' in-process, {} bytes.", path, content.bytes.size) + + return content + } + + /** What this instance has answered without a socket. */ + fun servedSummary(): String = + if (disabled) { + "in-process serving is off ($DISABLE_SENTINEL exists)" + } else { + "${servedRequests.get()} requests, ${servedBytes.get()} bytes served in-process" + } + + private fun response(content: DocumentationContent): WebResourceResponse { + val (type, charset) = mimeAndCharset(content.mimeType) + return WebResourceResponse(type, charset, ByteArrayInputStream(content.bytes)) + } + + companion object { + /** + * Splits a stored MIME type into what WebResourceResponse wants: the bare type, and the + * charset as its own value. The source hands back decompressed bytes -- a WebView does not + * decode an intercepted response -- so there is no Content-Encoding to declare either. + * + * Which types get a charset is [ContentTypeHeaders]' decision, not this transport's. The two + * transports answering differently about what a response *says* would be worse than either + * answer, and this one used to say `text/` only -- so an SVG served in-process declared no + * encoding while the same row over the socket did (ADFA-5241). + */ + internal fun mimeAndCharset(mimeType: String): Pair { + val type = mimeType.substringBefore(';').trim() + val declared = + mimeType + .substringAfter("charset=", "") + .substringBefore(';') + .trim() + .ifEmpty { null } + + return type to (declared ?: ContentTypeHeaders.charsetFor(mimeType)) + } + + private const val DISABLE_SENTINEL = "Download/CodeOnTheGo.nointercept" + private const val SERVER_HOST = "localhost" + private const val SERVER_PORT = 6174 + + /** + * The interceptor every WebView in the process shares, and with it one database handle and + * one copy of the compression dictionary. Deliberately separate from the source `WebServer` + * builds from its own config: the server's comes and goes with the activity that starts it, + * a WebView can outlive that, and neither should be able to close the other's handle. The + * cost of the second handle is SQLite's page cache plus the dictionary, a few MB. + */ + val shared: DocumentationRequestInterceptor by lazy { + Brotli4jLoader.ensureAvailability() + + DocumentationRequestInterceptor( + DocumentationContentSource( + Environment.DOC_DB, + File(getExternalStorageDirectory(), "Download/documentation.db"), + ), + ) + } + } +} diff --git a/common/src/test/java/com/itsaky/androidide/documentation/DocumentationContentSourceTest.kt b/common/src/test/java/com/itsaky/androidide/documentation/DocumentationContentSourceTest.kt new file mode 100644 index 0000000000..3a2de66629 --- /dev/null +++ b/common/src/test/java/com/itsaky/androidide/documentation/DocumentationContentSourceTest.kt @@ -0,0 +1,287 @@ +package com.itsaky.androidide.documentation + +import android.database.Cursor +import android.database.sqlite.SQLiteDatabase +import com.google.common.truth.Truth.assertThat +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import io.mockk.verify +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.File + +/** + * Covers the pipeline both documentation transports read through (ADFA-5176): what a lookup reports, + * how chunked rows are reassembled, and what a debug-database swap does to the handle and to the + * generation counter callers use to drop their per-database caches. + * + * The database itself is a mock. These tests are about this class's decisions, and a real + * SQLiteDatabase needs a device; the on-device behavior is covered by WebServerTest and by the + * brotli decode tests in the app module. + */ +class DocumentationContentSourceTest { + @get:Rule + val folder = TemporaryFolder() + + private lateinit var installedFile: File + private lateinit var debugFile: File + + @Before + fun setUp() { + installedFile = folder.newFile("documentation.db") + // Not created: a debug database that does not exist is the normal case. + debugFile = File(folder.root, "debug-documentation.db") + + mockkStatic(SQLiteDatabase::class) + } + + @After + fun tearDown() { + unmockkAll() + } + + private fun source(debugCheckIntervalMs: Long = 1_000) = DocumentationContentSource(installedFile, debugFile, debugCheckIntervalMs) + + /** A Content row as the source's query sees it. */ + private fun contentCursor( + bytes: ByteArray = "hello".toByteArray(), + mimeType: String = "text/plain", + compression: String = "none", + templateId: Int = 0, + rowCount: Int = 1, + ) = mockk(relaxed = true) { + every { count } returns rowCount + every { moveToFirst() } returns (rowCount > 0) + every { getBlob(0) } returns bytes + every { getString(1) } returns mimeType + every { getString(2) } returns compression + every { getInt(3) } returns templateId + } + + private fun database(contentCursor: Cursor): SQLiteDatabase = + mockk(relaxed = true) { + every { rawQuery(match { it.contains("FROM Content") }, any()) } returns contentCursor + } + + @Test + fun `lookup returns the row for a path`() { + val database = database(contentCursor(bytes = "page".toByteArray(), mimeType = "text/html")) + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns database + + val lookup = source().lookup("i/index.html") + + assertThat(lookup).isInstanceOf(DocumentationLookup.Found::class.java) + val content = (lookup as DocumentationLookup.Found).content + assertThat(content.bytes.toString(Charsets.UTF_8)).isEqualTo("page") + assertThat(content.mimeType).isEqualTo("text/html") + } + + @Test + fun `lookup reports an unknown path as not found`() { + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns database(contentCursor(rowCount = 0)) + + assertThat(source().lookup("nope")).isEqualTo(DocumentationLookup.NotFound) + } + + @Test + fun `lookup reports duplicate rows as ambiguous, since the path is meant to be unique`() { + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns database(contentCursor(rowCount = 2)) + + val lookup = source().lookup("i/index.html") + + assertThat(lookup).isEqualTo(DocumentationLookup.Ambiguous(2)) + } + + @Test + fun `lookup reports a read that throws rather than propagating it`() { + val database = + mockk(relaxed = true) { + every { rawQuery(match { it.contains("FROM Content") }, any()) } throws IllegalStateException("boom") + } + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns database + + val lookup = source().lookup("i/index.html") + + assertThat(lookup).isInstanceOf(DocumentationLookup.Failed::class.java) + assertThat((lookup as DocumentationLookup.Failed).cause).hasMessageThat().isEqualTo("boom") + } + + @Test + fun `lookup says not found when the database cannot be opened at all`() { + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } throws IllegalStateException("cannot open") + + assertThat(source().lookup("i/index.html")).isEqualTo(DocumentationLookup.NotFound) + } + + @Test + fun `content split across rows is reassembled in order`() { + val first = ByteArray(DocumentationContentSource.CONTENT_CHUNK_SIZE) { 'a'.code.toByte() } + val second = ByteArray(DocumentationContentSource.CONTENT_CHUNK_SIZE) { 'b'.code.toByte() } + val third = "tail".toByteArray() + + val chunkCursors = + listOf(second, third).map { chunk -> + mockk(relaxed = true) { + every { moveToFirst() } returns true + every { getBlob(0) } returns chunk + } + } + val database = + mockk(relaxed = true) { + every { rawQuery(match { it.contains("FROM Content") }, any()) } returns contentCursor(bytes = first) + every { rawQuery(match { it.startsWith("SELECT content FROM Content") }, arrayOf("big-1")) } returns chunkCursors[0] + every { rawQuery(match { it.startsWith("SELECT content FROM Content") }, arrayOf("big-2")) } returns chunkCursors[1] + } + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns database + + val lookup = source().lookup("big") + + val bytes = (lookup as DocumentationLookup.Found).content.bytes + assertThat(bytes.size).isEqualTo(first.size + second.size + third.size) + assertThat(bytes.copyOfRange(bytes.size - third.size, bytes.size).toString(Charsets.UTF_8)).isEqualTo("tail") + } + + @Test + fun `a templated row comes back rendered, so no caller needs the template engine`() { + val database = + mockk(relaxed = true) { + every { rawQuery(match { it.contains("FROM Content") }, any()) } returns + contentCursor(bytes = """{"who": "Kotlin"}""".toByteArray(), templateId = 7) + every { rawQuery(match { it.contains("FROM Templates") }, arrayOf("7")) } returns + mockk(relaxed = true) { + every { count } returns 1 + every { moveToFirst() } returns true + every { getBlob(0) } returns "Hello {{ who }}!".toByteArray() + } + } + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns database + + val lookup = source().lookup("k/html/basic-syntax.html") + + assertThat((lookup as DocumentationLookup.Found).content.bytes.toString(Charsets.UTF_8)) + .isEqualTo("Hello Kotlin!") + } + + @Test + fun `a template is compiled once and reused for the next page that needs it`() { + val database = + mockk(relaxed = true) { + every { rawQuery(match { it.contains("FROM Content") }, any()) } returns + contentCursor(bytes = """{"who": "Kotlin"}""".toByteArray(), templateId = 7) + every { rawQuery(match { it.contains("FROM Templates") }, arrayOf("7")) } returns + mockk(relaxed = true) { + every { count } returns 1 + every { moveToFirst() } returns true + every { getBlob(0) } returns "Hello {{ who }}!".toByteArray() + } + } + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns database + + val source = source() + repeat(3) { source.lookup("k/html/basic-syntax.html") } + + verify(exactly = 1) { database.rawQuery(match { it.contains("FROM Templates") }, arrayOf("7")) } + } + + @Test + fun `a missing template row is reported as a failed lookup, not a crash`() { + val database = + mockk(relaxed = true) { + every { rawQuery(match { it.contains("FROM Content") }, any()) } returns + contentCursor(bytes = "{}".toByteArray(), templateId = 7) + every { rawQuery(match { it.contains("FROM Templates") }, arrayOf("7")) } returns + mockk(relaxed = true) { + every { count } returns 0 + every { moveToFirst() } returns false + } + } + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns database + + assertThat(source().lookup("k/html/basic-syntax.html")).isInstanceOf(DocumentationLookup.Failed::class.java) + } + + @Test + fun `a newer debug database is swapped in, and the generation says so`() { + val installed = database(contentCursor(bytes = "installed".toByteArray())) + val debug = database(contentCursor(bytes = "debug".toByteArray())) + every { SQLiteDatabase.openDatabase(installedFile.absolutePath, isNull(), any()) } returns installed + every { SQLiteDatabase.openDatabase(debugFile.absolutePath, isNull(), any()) } returns debug + + // Zero interval: the rate limit on the stat is not what this test is about. + val source = source(debugCheckIntervalMs = 0) + + assertThat((source.lookup("p") as DocumentationLookup.Found).content.bytes.toString(Charsets.UTF_8)) + .isEqualTo("installed") + val generationBefore = source.generation + + debugFile.writeText("newer") + debugFile.setLastModified(installedFile.lastModified() + 60_000) + + assertThat((source.lookup("p") as DocumentationLookup.Found).content.bytes.toString(Charsets.UTF_8)) + .isEqualTo("debug") + assertThat(source.generation).isGreaterThan(generationBefore) + verify { installed.close() } + } + + @Test + fun `a debug database that will not open leaves the installed one serving`() { + val installed = database(contentCursor(bytes = "installed".toByteArray())) + every { SQLiteDatabase.openDatabase(installedFile.absolutePath, isNull(), any()) } returns installed + every { SQLiteDatabase.openDatabase(debugFile.absolutePath, isNull(), any()) } throws IllegalStateException("corrupt") + + val source = source(debugCheckIntervalMs = 0) + source.lookup("p") + val generationBefore = source.generation + + debugFile.writeText("corrupt") + debugFile.setLastModified(installedFile.lastModified() + 60_000) + + assertThat((source.lookup("p") as DocumentationLookup.Found).content.bytes.toString(Charsets.UTF_8)) + .isEqualTo("installed") + assertThat(source.generation).isEqualTo(generationBefore) + verify(exactly = 0) { installed.close() } + } + + @Test + fun `a failed debug swap is not retried until the file changes again`() { + val installed = database(contentCursor()) + every { SQLiteDatabase.openDatabase(installedFile.absolutePath, isNull(), any()) } returns installed + every { SQLiteDatabase.openDatabase(debugFile.absolutePath, isNull(), any()) } throws IllegalStateException("corrupt") + + val source = source(debugCheckIntervalMs = 0) + debugFile.writeText("corrupt") + debugFile.setLastModified(installedFile.lastModified() + 60_000) + + repeat(3) { source.lookup("p") } + + verify(exactly = 1) { SQLiteDatabase.openDatabase(debugFile.absolutePath, isNull(), any()) } + } + + @Test + fun `withDatabase runs against the open database`() { + val database = database(contentCursor()) + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns database + + val seen = source().withDatabase { it } + + assertThat(seen).isSameInstanceAs(database) + } + + @Test + fun `close closes the handle, and closing twice is harmless`() { + val database = database(contentCursor()) + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns database + + val source = source() + source.lookup("p") + source.close() + source.close() + + verify(exactly = 1) { database.close() } + } +} diff --git a/common/src/test/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptorTest.kt b/common/src/test/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptorTest.kt new file mode 100644 index 0000000000..dc3176ca2b --- /dev/null +++ b/common/src/test/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptorTest.kt @@ -0,0 +1,134 @@ +package com.itsaky.androidide.documentation + +import android.net.Uri +import android.webkit.WebResourceRequest +import com.google.common.truth.Truth.assertThat +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.File +import android.os.Environment as AndroidEnvironment + +/** + * Covers which requests the interceptor takes and which it hands to the local web server + * (ADFA-5176). Asserts on `contentFor` rather than `intercept`, because building the + * WebResourceResponse `intercept` returns needs the framework. + */ +class DocumentationRequestInterceptorTest { + @get:Rule + val folder = TemporaryFolder() + + private lateinit var source: DocumentationContentSource + + @Before + fun setUp() { + // The interceptor reads its off switch from external storage when it is constructed. + mockkStatic(AndroidEnvironment::class) + every { AndroidEnvironment.getExternalStorageDirectory() } returns folder.root + + source = mockk(relaxed = true) + every { source.lookup(any()) } returns + DocumentationLookup.Found(DocumentationContent("page".toByteArray(), "text/html")) + } + + @After + fun tearDown() { + unmockkAll() + } + + private fun request( + url: String = "http://localhost:6174/i/index.html", + method: String = "GET", + ): WebResourceRequest { + // Uri is a framework class, so stand in for the three parts the interceptor reads. + val parsed = java.net.URI(url) + val uri = + mockk { + every { host } returns parsed.host + every { port } returns parsed.port + every { path } returns parsed.path + } + + return mockk { + every { this@mockk.method } returns method + every { this@mockk.url } returns uri + } + } + + @Test + fun `serves a documentation path from the content source`() { + val content = DocumentationRequestInterceptor(source).contentFor(request()) + + assertThat(content).isNotNull() + assertThat(content!!.bytes.toString(Charsets.UTF_8)).isEqualTo("page") + } + + @Test + fun `declines anything but GET, since only the server handles a request with a body`() { + assertThat(DocumentationRequestInterceptor(source).contentFor(request(method = "POST"))).isNull() + } + + @Test + fun `declines a host or port that is not the local documentation server`() { + val interceptor = DocumentationRequestInterceptor(source) + + assertThat(interceptor.contentFor(request(url = "http://example.com:6174/i/index.html"))).isNull() + assertThat(interceptor.contentFor(request(url = "http://localhost:8080/i/index.html"))).isNull() + } + + @Test + fun `declines the developer endpoints, which only the server implements`() { + val interceptor = DocumentationRequestInterceptor(source) + + assertThat(interceptor.contentFor(request(url = "http://localhost:6174/pr/bs"))).isNull() + assertThat(interceptor.contentFor(request(url = "http://localhost:6174/pr/db"))).isNull() + } + + @Test + fun `declines a bare origin with no path`() { + assertThat(DocumentationRequestInterceptor(source).contentFor(request(url = "http://localhost:6174/"))).isNull() + } + + @Test + fun `declines what the source cannot find, so the server can answer it`() { + every { source.lookup(any()) } returns DocumentationLookup.NotFound + + assertThat(DocumentationRequestInterceptor(source).contentFor(request())).isNull() + } + + @Test + fun `the sentinel file puts documentation back on the web server`() { + File(folder.root, "Download").mkdirs() + File(folder.root, "Download/CodeOnTheGo.nointercept").createNewFile() + + val interceptor = DocumentationRequestInterceptor(source) + + assertThat(interceptor.contentFor(request())).isNull() + assertThat(interceptor.servedSummary()).contains("off") + } + + @Test + fun `reports what it has served`() { + val interceptor = DocumentationRequestInterceptor(source) + repeat(3) { interceptor.contentFor(request()) } + + assertThat(interceptor.servedSummary()).isEqualTo("3 requests, 12 bytes served in-process") + } + + @Test + fun `splits a stored MIME type into what a WebResourceResponse needs`() { + assertThat(DocumentationRequestInterceptor.mimeAndCharset("text/html; charset=utf-8")) + .isEqualTo("text/html" to "utf-8") + // Text with no stated charset: the doc set is UTF-8 throughout. + assertThat(DocumentationRequestInterceptor.mimeAndCharset("text/css")).isEqualTo("text/css" to "utf-8") + // Binary content must not claim one, or a WebView will try to decode it as text. + assertThat(DocumentationRequestInterceptor.mimeAndCharset("image/png")).isEqualTo("image/png" to null) + assertThat(DocumentationRequestInterceptor.mimeAndCharset("application/pdf")).isEqualTo("application/pdf" to null) + } +} diff --git a/docs/documentation-database.md b/docs/documentation-database.md index 3055c955f0..38d3b24073 100644 --- a/docs/documentation-database.md +++ b/docs/documentation-database.md @@ -73,16 +73,9 @@ CREATE TABLE Tooltips ( All three sites below open the file with `SQLiteDatabase.openDatabase(..., OPEN_READONLY)` — no writes, ever, from this app (see ADR 0001 for why raw SQLite is justified here instead of Room). -- **`app/.../localWebServer/WebServer.kt`** — serves Tier 3. On each `GET`, runs: - - ```sql - SELECT C.content, CT.value, CT.compression, C.templateId - FROM Content C, ContentTypes CT - WHERE C.contentTypeID = CT.id - AND C.path = ? - ``` - - then reassembles chunked blobs, always decompresses Brotli content (attaching `CompressionDictionary`'s bytes first, if loaded — see above) since this server never negotiates `Content-Encoding` with the client, and instantiates the template if `templateId > 0`. Also serves a Dynamic Bookshelf JSON payload (joining `Content`/`Bookshelf`/`BookCategories`, rendered through the `bookshelf` template) and debug-only HTML dumps at `/pr/db` (`LastChange`, last 20 rows) and `/pr/pr` (recent projects, from a *different* database). +- **`common/.../documentation/DocumentationContentSource.kt`** — the one pipeline that reads this database: row lookup, chunked-row reassembly, dictionary-aware Brotli decode gated on the declared documentation version, and the sdcard debug-database swap, under a read/write lock so several threads can read while a swap cannot close the handle under them. It also renders the rows that are Pebble template contexts (`templateId > 0`, the Kotlin doc set's pages), so both transports below serve finished pages and neither needs the template engine itself. All of that logic exists once (ADFA-5176). +- **`common/.../documentation/DocumentationRequestInterceptor.kt`** — serves Tier 3 *in-process* for the app's WebViews (`HelpActivity`, the tooltip fragment, `FAQActivity`), through `WebViewClient.shouldInterceptRequest`, so a page's assets cost a database read instead of a TCP connection each (ADFA-5176). It matches the same `http://localhost:6174/...` URL space, so the strings.xml entries, `ToolTipManager`'s link builder and the `DocumentationExtension` contract need no changes; anything it declines — a `/pr/` endpoint, an unknown path, a failed read — falls through to `WebServer` unchanged. Both transports get their `Content-Type` charset from the same place, `ContentTypeHeaders` (ADFA-5241), so a row does not describe itself differently depending on which one served it. Set `/sdcard/Download/CodeOnTheGo.nointercept` to force documentation back onto the server. +- **`app/.../localWebServer/WebServer.kt`** — serves Tier 3 over HTTP on port 6174, for WebViews that are not wired to the interceptor above and for the `/pr/` developer endpoints. It reads through `DocumentationContentSource`, so it holds no database, template engine or decode logic of its own; what remains here is HTTP: request parsing, the `/pr/` pages, error responses, and the CSS/asset shortcuts. Also serves a Dynamic Bookshelf JSON payload (joining `Content`/`Bookshelf`/`BookCategories`, rendered through the `bookshelf` template) and debug-only HTML dumps at `/pr/db` (`LastChange`, last 20 rows) and `/pr/pr` (recent projects, from a *different* database). - **`idetooltips/.../ToolTipManager.kt`** — serves Tier 1/2. Looks up `Tooltips` joined to `TooltipCategories` by `(category, tag)`, then `TooltipButtons` for the Tier 3 links shown at the bottom. - **`plugin-manager/.../documentation/PluginDocumentationManager.kt`** (with `Tier3AssetWalker.kt`, and the `DocumentationExtension` contract in `plugin-api`) — lets plugins contribute their own help content into the same lookup paths. From 7d47b0986059cf8353c3306cf802408d83a6e2a5 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 21 Aug 2026 21:11:42 -0700 Subject: [PATCH 2/8] ADFA-5176: Address the code review on the in-process transport Ten findings from the review on PR #1726. The three that could break something in production: * switchToDatabase bumped generation and closed the old handle but never cleared templateCache, so Pebble templates compiled from a closed database kept rendering. The old WebServer.switchToDatabase cleared it and the field comment still promised it. An edited template row in a swapped-in database would have rendered the previous database's markup for the rest of the session. * The brotli warm-up did not move with the decode. WebServer.decompressBrotli wrapped Brotli4jLoader.ensureAvailability() so a missing native library cost one failed read instead of the process; when the decode moved into DocumentationContentSource the guard stayed behind, and an UnsatisfiedLinkError is an Error, so it escapes every catch between there and the accept loop. Restored on the decode path, where the decode now is. * The interceptor's `shared` initializer called ensureAvailability() eagerly. That runs during Activity and Fragment construction, so a missing library was a hard crash on opening Help rather than a failed page. Removed: the source warms lazily and converts the Error, which covers both transports. mimeAndCharset now delegates to ContentTypeHeaders.typeAndCharset instead of re-parsing with substringAfter("charset="). The duplicate parse disagreed with the socket transport on quoted parameters and on case -- for `; Charset=UTF-8` it missed the parameter and the response went out with no encoding at all, which is the ADFA-5241 failure this class's KDoc claims to prevent. sendContent also builds its header before the status line, so a throw cannot append a second status line to a response that already claimed 200. Six unused imports left in WebServer.kt -- four of them the ADFA-5175 worker pool's -- would have failed spotlessCheck in CI under the file-level ratchet. HelpActivity loaded every page twice: once in onCreate and again through updateUIFromIntent. This PR's own device log proves it -- "2 requests, 231036 bytes" for one 115,518-byte page -- so every open paid two full reads, and the 142 ms figure quoted against the socket path's 99 ms was timing two loads against one. The instrumentation also used wall-clock time (an NTP correction mid-load reports nonsense) and presented process-cumulative counters as the page's own; it now uses elapsedRealtime and says "totals so far". The three ADFA-5220 version-gate tests deleted when WebServerTest was replaced are restored in DocumentationContentSourceTest, where dictionaryBytes now lives: below 2, no version table, and above 2, with the dictionary cursors stubbed as available in every case so they test the gate rather than a missing table. failedDebugSwapTimestamp is @Volatile: the check reads it outside the write lock, so without it a second thread misses the first's failure marker and re-attempts openDatabase on a broken file while holding the lock -- and a 64-bit read is not atomic on armeabi-v7a. Three WebSettings property reads with no assignment were deleted rather than turned into `= true`. They are no-ops today; assigning them would silently enable three security-relevant settings, universal file access among them, as a side effect of a reindentation. If the fragment's file:///android_asset handling needs file access, that is its own change with its own reasoning. Also: comments describing a worker pool, awaitTermination and a templateCache this class no longer has, and ARCHITECTURE.md's raw-SQLite exception, which still named WebServer as the holder of the database handle. 363 tests across app and common pass; spotlessCheck is clean. Co-Authored-By: Claude Opus 5 (1M context) --- ARCHITECTURE.md | 4 +- .../fragments/IDETooltipWebViewFragment.kt | 3 - .../androidide/localWebServer/WebServer.kt | 24 +++---- .../activities/editor/HelpActivity.kt | 19 +++-- .../DocumentationContentSource.kt | 31 ++++++++ .../DocumentationRequestInterceptor.kt | 24 ++----- .../DocumentationContentSourceTest.kt | 70 +++++++++++++++++++ 7 files changed, 130 insertions(+), 45 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 59df122920..4f70dacbe6 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -12,7 +12,7 @@ There is **no single architectural philosophy** across the whole app. This large Feature code layers as **UI → ViewModel → Repository → data source**, with state flowing up and events/intents flowing down. Koin provides dependencies (`coreModule`, `pluginModule`), constructor-injected into ViewModels. -- **Data sources** — Room (`RecentProjectRoomDatabase` + DAO, `suspend` functions), raw SQLite (`SQLiteOpenHelper`, e.g. `localWebServer/WebServer`), the filesystem/preferences, the embedded `tooling-api` (on-device Gradle), and external clients (Gemini via the Google GenAI SDK, on-device llama.cpp, JGit). Most are exposed through `suspend` functions. +- **Data sources** — Room (`RecentProjectRoomDatabase` + DAO, `suspend` functions), raw SQLite (`SQLiteOpenHelper`, e.g. `common/.../documentation/DocumentationContentSource`), the filesystem/preferences, the embedded `tooling-api` (on-device Gradle), and external clients (Gemini via the Google GenAI SDK, on-device llama.cpp, JGit). Most are exposed through `suspend` functions. - **Repositories** — e.g. `agent/repository/GeminiRepository`, `repositories/PluginRepository`, `repositories/BreakpointRepository`. They wrap data sources and hide threading/IO from the ViewModel. - **ViewModels** — run work in `viewModelScope` on `Dispatchers.IO`, hold a private `MutableStateFlow`/`MutableSharedFlow`, and expose read-only `StateFlow`/`SharedFlow`. One-shot effects (toasts, navigation, dialogs) go through a separate `SharedFlow` of a sealed `*UiEffect` type. - **UI (Fragments / Activities / Views)** — collect state in a lifecycle-aware coroutine and render it; user actions return to the ViewModel as method calls or sealed `*UiEvent` intents. The existing UI is **Android Views + Fragments + RecyclerView adapters**; new UI is Jetpack Compose ([ADR 0009](docs/adr/0009-jetpack-compose-for-new-ui.md)). (`compose-preview` previews the *user's* Compose code, not CoGo's own.) @@ -102,7 +102,7 @@ These structural facts shape every module. Day-to-day build *commands* live in ` > > **Recent Projects** is the reference example of the default: `app/src/main/java/com/itsaky/androidide/roomData/recentproject/` (`RecentProjectRoomDatabase`, `@Database version = 4` with migrations 1→4; `RecentProjectDao`; the `RecentProject` `@Entity` → table `recent_project_table`). It's provided via Koin in `di/AppModule.kt` and consumed by `MainViewModel`, `RecentProjectsViewModel`, `MainActivity`, `ProjectInfoBottomSheet`, and `ProjectCreationManager`. > -> **Raw SQLite is allowed only when** the database is prebuilt and opened read-only, the data is performance/allocation-critical and needs granular schema control, or the schema is shared across a process/component boundary. Current exceptions: symbol indexing (`lsp/indexing/SQLiteIndex.kt`), tooltips (`idetooltips/ToolTipManager.kt`), in-app/plugin help (`plugin-manager/.../documentation/PluginDocumentationManager.kt`), and the local web server (`app/.../localWebServer/WebServer.kt`). The `androidx.room:*` strings in `editor`'s `GroovyAutoComplete` are autocomplete suggestions for the *user's* code, not CoGo persistence. +> **Raw SQLite is allowed only when** the database is prebuilt and opened read-only, the data is performance/allocation-critical and needs granular schema control, or the schema is shared across a process/component boundary. Current exceptions: symbol indexing (`lsp/indexing/SQLiteIndex.kt`), tooltips (`idetooltips/ToolTipManager.kt`), in-app/plugin help (`plugin-manager/.../documentation/PluginDocumentationManager.kt`), and documentation serving (`common/.../documentation/DocumentationContentSource.kt`, the one pipeline behind both the in-process WebView transport and `app/.../localWebServer/WebServer.kt`). The `androidx.room:*` strings in `editor`'s `GroovyAutoComplete` are autocomplete suggestions for the *user's* code, not CoGo persistence. > > The tooltip, in-app/plugin-help, and local-web-server exceptions all read `documentation.db`, the prebuilt Tier 1/2/3 help database — see [docs/documentation-database.md](docs/documentation-database.md) for its schema and how each consumer queries it. diff --git a/app/src/main/java/com/itsaky/androidide/fragments/IDETooltipWebViewFragment.kt b/app/src/main/java/com/itsaky/androidide/fragments/IDETooltipWebViewFragment.kt index f9874372ae..aff663b718 100644 --- a/app/src/main/java/com/itsaky/androidide/fragments/IDETooltipWebViewFragment.kt +++ b/app/src/main/java/com/itsaky/androidide/fragments/IDETooltipWebViewFragment.kt @@ -97,9 +97,6 @@ class IDETooltipWebviewFragment : Fragment() { // Set up WebChromeClient to support JavaScript // webView.webChromeClient = WebChromeClient() - webView.settings.allowFileAccessFromFileURLs - webView.settings.allowFileAccess - webView.settings.allowUniversalAccessFromFileURLs webView.scrollBarStyle = WebView.SCROLLBARS_OUTSIDE_OVERLAY webView.scrollBarDefaultDelayBeforeFade = 1000 diff --git a/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt b/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt index 4d89da2f98..27466a4fc4 100644 --- a/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt +++ b/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt @@ -9,9 +9,7 @@ import com.itsaky.androidide.documentation.DocumentationContentSource import com.itsaky.androidide.documentation.DocumentationLookup import com.itsaky.androidide.utils.ContentTypeHeaders import com.itsaky.androidide.utils.DatabaseVersionResolver -import okio.ByteString.Companion.toByteString import org.slf4j.LoggerFactory -import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream import java.io.File import java.io.InputStream @@ -20,12 +18,7 @@ import java.net.InetSocketAddress import java.net.ServerSocket import java.net.Socket import java.net.URLDecoder -import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.RejectedExecutionException -import java.util.concurrent.SynchronousQueue -import java.util.concurrent.ThreadPoolExecutor import java.util.concurrent.TimeUnit -import java.util.concurrent.atomic.AtomicLong import java.util.concurrent.atomic.AtomicReference data class ServerConfig( @@ -93,14 +86,15 @@ class WebServer( // Frozen at startup; restart the server to pick up a change. private val clearCacheEnabled: Boolean = File(config.clearCacheEnablePath).exists() - // Read and written by any worker; -1 means "not fetched yet". Two workers racing to fetch it - // both write the same id, so a plain volatile is enough. + // -1 means "not fetched yet". Volatile because the WebView transport shares this server's + // process, and the interceptor's reads can run on WebView threads while the accept loop writes. @Volatile private var bookshelfTemplateId: Int = -1 private val cacheLock = Any() - // Which of the source's databases templateCache and bookshelfTemplateId were filled from. + // Which of the source's databases bookshelfTemplateId was filled from. The compiled templates + // themselves live in the source and are dropped by its own swap. @Volatile private var cachedDatabaseGeneration = 0L private val httpInternalServerError = 500 @@ -221,11 +215,11 @@ class WebServer( serverSocket.close() } - // The database is opened before the stopRequested check that can abort start() early - // (and before the accept loop on every other exit path), so it has to be closed here - // too, not just serverSocket. Closing an unopened source is a no-op, and the source - // closes under its own write lock: awaitTermination above can time out, and a worker - // that outlived it finishes its read before the handle goes. + // The source is opened before the stopRequested check that can abort start() early (and + // before the accept loop on every other exit path), so it has to be closed here too, + // not just serverSocket. Closing an unopened source is a no-op, and it closes under its + // own write lock, so a read in flight on another thread -- a WebView's, through the + // interceptor's separate source -- finishes before any handle goes. contentSource.close() TrafficStats.clearThreadStatsTag() } diff --git a/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt b/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt index d69bcf8354..45fd0a5ee2 100644 --- a/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt +++ b/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt @@ -21,6 +21,7 @@ import android.app.Activity import android.content.Context import android.content.Intent import android.os.Bundle +import android.os.SystemClock import android.view.Menu import android.view.MenuItem import android.view.View @@ -72,6 +73,8 @@ class HelpActivity : BaseIDEActivity() { private val documentation = DocumentationRequestInterceptor.shared // Wall-clock start of the page currently loading, for the ADFA-5176 measurement. + // elapsedRealtime, not currentTimeMillis: an NTP correction or a user clock change between + // onPageStarted and onPageFinished would otherwise report a negative or absurd duration. private var pageLoadStartMillis = 0L @Suppress("ktlint:standard:backing-property-naming") @@ -129,7 +132,7 @@ class HelpActivity : BaseIDEActivity() { favicon: android.graphics.Bitmap?, ) { super.onPageStarted(view, url, favicon) - pageLoadStartMillis = System.currentTimeMillis() + pageLoadStartMillis = SystemClock.elapsedRealtime() } override fun onPageFinished( @@ -140,10 +143,13 @@ class HelpActivity : BaseIDEActivity() { invalidateOptionsMenu() if (pageLoadStartMillis != 0L) { + // The summary's counters are process-cumulative, so they are labelled as + // such rather than read as this page's -- the tenth page in a session + // would otherwise report the whole session's bytes as its own. log.info( - "Loaded '{}' in {} ms; {}.", + "Loaded '{}' in {} ms; in-process totals so far: {}.", url, - System.currentTimeMillis() - pageLoadStartMillis, + SystemClock.elapsedRealtime() - pageLoadStartMillis, documentation.servedSummary(), ) pageLoadStartMillis = 0L @@ -185,10 +191,9 @@ class HelpActivity : BaseIDEActivity() { } } - // Load the HTML file from the assets folder - htmlContent?.let { url -> - webView.loadUrl(url) - } + // The page itself is loaded by updateUIFromIntent below -- the one place that does it, + // since onNewIntent needs the same path. Loading it here as well made every open pay + // two full reads: decode, render and serve the same row twice. } // Set up back navigation callback for system back button diff --git a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt index 78c083171a..3b74dc4e58 100644 --- a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt +++ b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt @@ -18,6 +18,7 @@ package com.itsaky.androidide.documentation import android.database.sqlite.SQLiteDatabase +import com.aayushatharva.brotli4j.Brotli4jLoader import com.aayushatharva.brotli4j.decoder.BrotliInputStream import com.google.gson.Gson import com.google.gson.GsonBuilder @@ -158,6 +159,11 @@ class DocumentationContentSource( // A debug database whose swap already failed, so a corrupt or unreadable one is not reopened // on every check. A newer copy has a different timestamp and is retried, which is the case // that matters: replacing the file is exactly how a developer fixes it. + // Volatile because the check that reads it happens outside the write lock: without it a second + // thread never sees the first's failure marker and re-attempts openDatabase on the broken file + // while holding the write lock, serialising every reader behind a failing open -- the exact + // behaviour this field exists to prevent. A 64-bit read is not atomic on armeabi-v7a either. + @Volatile private var failedDebugSwapTimestamp: Long = -1 // The dictionary the Content rows are compressed against. Loaded on the first decode that @@ -385,6 +391,30 @@ class DocumentationContentSource( return chunks } + /** + * Loads brotli4j's native library if nothing else has yet, and turns its absence into a failed + * read rather than a dead app. + * + * Nothing here owns that load: it happens as a side effect of `AssetsInstallationHelper`'s + * install or `ToolsManager`'s tooling-jar update, neither of which runs on an ordinary cold + * start. A process that skips both reaches the first brotli row with the natives unregistered, + * and `DecoderJNI.nativeCreate` raises `UnsatisfiedLinkError` -- an Error, not an Exception, so + * it escapes every `catch (e: Exception)` between here and the accept loop and kills the app + * (observed on-device, 20-Aug). The guard lived in `WebServer.decompressBrotli`; when the decode + * moved here it had to move with it. + * + * Referencing [Brotli4jLoader] triggers the static init that performs the load, so this call is + * the warm-up; afterwards `ensureAvailability` is a single static null-check, cheap enough to + * leave on the per-decode path. + */ + private fun ensureBrotliAvailable() { + try { + Brotli4jLoader.ensureAvailability() + } catch (e: UnsatisfiedLinkError) { + throw IOException("brotli4j's native library is unavailable, so brotli content cannot be decoded", e) + } + } + /** * Decompresses one Brotli row. Tries the shared dictionary first, since every migrated row * requires it, then falls back to a plain decode for rows that were never dictionary @@ -398,6 +428,7 @@ class DocumentationContentSource( database: SQLiteDatabase, chunks: List, ): ByteArray { + ensureBrotliAvailable() val dictionary = compressionDictionary(database) if (dictionary != null) { try { diff --git a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt index 981b54a417..95f1cb8cab 100644 --- a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt +++ b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt @@ -20,7 +20,6 @@ package com.itsaky.androidide.documentation import android.os.Environment.getExternalStorageDirectory import android.webkit.WebResourceRequest import android.webkit.WebResourceResponse -import com.aayushatharva.brotli4j.Brotli4jLoader import com.itsaky.androidide.utils.ContentTypeHeaders import com.itsaky.androidide.utils.Environment import org.slf4j.LoggerFactory @@ -105,22 +104,13 @@ class DocumentationRequestInterceptor( * charset as its own value. The source hands back decompressed bytes -- a WebView does not * decode an intercepted response -- so there is no Content-Encoding to declare either. * - * Which types get a charset is [ContentTypeHeaders]' decision, not this transport's. The two - * transports answering differently about what a response *says* would be worse than either - * answer, and this one used to say `text/` only -- so an SVG served in-process declared no - * encoding while the same row over the socket did (ADFA-5241). + * Both the split and the default come from [ContentTypeHeaders], so the two transports cannot + * disagree about what a response says. This used to parse the charset itself, with the naive + * `substringAfter("charset=")` that ContentTypeHeaders warns against: for + * `text/html; note="charset=iso-8859-1"` it declared iso-8859-1 while the socket declared + * utf-8, and for `; Charset=UTF-8` it missed the parameter entirely (ADFA-5241). */ - internal fun mimeAndCharset(mimeType: String): Pair { - val type = mimeType.substringBefore(';').trim() - val declared = - mimeType - .substringAfter("charset=", "") - .substringBefore(';') - .trim() - .ifEmpty { null } - - return type to (declared ?: ContentTypeHeaders.charsetFor(mimeType)) - } + internal fun mimeAndCharset(mimeType: String): Pair = ContentTypeHeaders.typeAndCharset(mimeType) private const val DISABLE_SENTINEL = "Download/CodeOnTheGo.nointercept" private const val SERVER_HOST = "localhost" @@ -134,8 +124,6 @@ class DocumentationRequestInterceptor( * cost of the second handle is SQLite's page cache plus the dictionary, a few MB. */ val shared: DocumentationRequestInterceptor by lazy { - Brotli4jLoader.ensureAvailability() - DocumentationRequestInterceptor( DocumentationContentSource( Environment.DOC_DB, diff --git a/common/src/test/java/com/itsaky/androidide/documentation/DocumentationContentSourceTest.kt b/common/src/test/java/com/itsaky/androidide/documentation/DocumentationContentSourceTest.kt index 3a2de66629..6ff57fe02b 100644 --- a/common/src/test/java/com/itsaky/androidide/documentation/DocumentationContentSourceTest.kt +++ b/common/src/test/java/com/itsaky/androidide/documentation/DocumentationContentSourceTest.kt @@ -68,6 +68,76 @@ class DocumentationContentSourceTest { every { rawQuery(match { it.contains("FROM Content") }, any()) } returns contentCursor } + /** A database that declares [major] in ADFA-5220's version table, or none when null. */ + private fun database( + contentCursor: Cursor, + declaredMajorVersion: Int?, + dictionary: ByteArray? = "test-dictionary".toByteArray(), + ): SQLiteDatabase = + mockk(relaxed = true) { + every { rawQuery(match { it.contains("FROM Content") }, any()) } returns contentCursor + every { + rawQuery(match { it.contains("FROM sqlite_master") && it.contains("DocumentationDatabaseVersion") }, any()) + } returns mockk(relaxed = true) { every { moveToFirst() } returns (declaredMajorVersion != null) } + if (declaredMajorVersion != null) { + every { rawQuery(match { it.contains("FROM DocumentationDatabaseVersion") }, any()) } returns + mockk(relaxed = true) { + every { moveToFirst() } returns true + every { isNull(0) } returns false + every { getInt(0) } returns declaredMajorVersion + } + } + every { + rawQuery(match { it.contains("FROM sqlite_master") && it.contains("CompressionDictionary") }, null) + } returns mockk(relaxed = true) { every { moveToFirst() } returns (dictionary != null) } + every { rawQuery(match { it.contains("SELECT data FROM CompressionDictionary") }, null) } returns + mockk(relaxed = true) { + every { moveToFirst() } returns (dictionary != null) + every { getBlob(0) } returns dictionary + } + } + + // ADFA-5220: the dictionary is gated on the version the database declares, not on whether a + // CompressionDictionary table happens to exist. These three cover the gate that WebServerTest + // used to own before the decode moved into this class. + @Test + fun `a database declaring a version below 2 is never asked for a dictionary`() { + assertDictionaryRead(declaredMajorVersion = 1, expected = 0) + } + + @Test + fun `a database with no version table is never asked for a dictionary`() { + assertDictionaryRead(declaredMajorVersion = null, expected = 0) + } + + // The gate is a floor, not a match: a later format still carries the dictionary. + @Test + fun `a database declaring a version above 2 still loads the dictionary`() { + assertDictionaryRead(declaredMajorVersion = 3, expected = 1) + } + + /** + * The dictionary cursors are stubbed as *available* in every case, including the ones expecting + * zero reads: that is what makes this a test of the gate rather than of a missing table. + */ + private fun assertDictionaryRead( + declaredMajorVersion: Int?, + expected: Int, + ) { + val database = database(contentCursor(compression = "brotli"), declaredMajorVersion) + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns database + + source().use { it.lookup("a/page.html") } + + verify(exactly = expected) { + database.rawQuery(match { it.contains("SELECT data FROM CompressionDictionary") }, null) + } + // The version itself is always consulted -- that is the gate being reached at all. + verify(atLeast = 1) { + database.rawQuery(match { it.contains("FROM sqlite_master") && it.contains("DocumentationDatabaseVersion") }, any()) + } + } + @Test fun `lookup returns the row for a path`() { val database = database(contentCursor(bytes = "page".toByteArray(), mimeType = "text/html")) From fd4c8fa0c0060c9fde92210a4d65398788341c61 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Fri, 21 Aug 2026 21:21:29 -0700 Subject: [PATCH 3/8] ADFA-5176: Close the last three review findings on the two transports The three that needed a decision rather than a patch. **Cache staleness was judged before the swap it depends on.** serveRequest called discardCachesIfDatabaseChanged() first, but the source performs the debug-database swap inside lookup()/withDatabase(). On the request that swaps, the generation read was the pre-swap one, so bookshelfTemplateId still pointed at the previous database's template row -- rendering the old bookshelf, or 500ing when that id does not exist in the new database. The source now exposes refreshDatabase(), which applies a pending swap without reading, and the discard runs after it. Idempotent and throttled by the debug check interval, so the cost is one extra timestamp comparison. **Priming the dictionary could fail an unrelated lookup.** readContent primes it before reading the row, and dictionaryBytes deliberately lets unexpected failures propagate so a transient error is not cached as "no dictionary". On the lookup path that meant a locked database during one dictionary query failed *every* request, including rows with compression = 'none' that need no dictionary at all. The priming call is now best-effort: it logs, leaves the staleness flag set so the next read retries, and a brotli row that genuinely cannot resolve its dictionary still fails loudly from inside decompressBrotli. **The two transports disagreed about what a path is.** The interceptor matches WebResourceRequest.url.path, which is percent-decoded; WebServer matched the raw target. For any path the WebView encodes -- a space, a literal % -- the interceptor found the row and the server 404ed it, so setting the nointercept sentinel changed *which pages work*, defeating its purpose of comparing the two transports on equal terms. WebServer now decodes, with two details worth keeping: "+" is protected first, because URLDecoder alone turns it into a space and would break a stored path containing a literal plus (c++.html is a real shape here), and a malformed escape logs and falls back to the verbatim path rather than failing the request, so it 404s naturally. Three tests on the decoding -- encoded space, literal plus, malformed escape -- assert the path the server actually queries with, not just that a request succeeds. 366 tests across app and common pass; spotlessCheck is clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../androidide/localWebServer/WebServer.kt | 30 +++++++++++- .../localWebServer/WebServerTest.kt | 47 +++++++++++++++++++ .../DocumentationContentSource.kt | 26 +++++++++- 3 files changed, 101 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt b/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt index 27466a4fc4..2c2125ff3d 100644 --- a/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt +++ b/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt @@ -319,6 +319,12 @@ class WebServer( * bookshelf template id, now that the compiled templates live in the source with the swap. */ private fun discardCachesIfDatabaseChanged() { + // Apply any pending swap first. The source swaps inside lookup()/withDatabase(), so checking + // the generation before those runs reads the generation from before the swap: on the very + // request that swaps, this would leave bookshelfTemplateId pointing at the previous + // database's template row -- rendering the old bookshelf, or 500ing if that id is absent. + contentSource.refreshDatabase() + if (contentSource.generation == cachedDatabaseGeneration) return synchronized(cacheLock) { @@ -330,6 +336,28 @@ class WebServer( } } + /** + * Percent-decodes a request target the way the in-process transport does. + * + * `WebResourceRequest.url.path` is already decoded, so the interceptor looks up `a/my file.html` + * while this server, matching the raw target, looked up `a/my%20file.html` and returned 404 for + * a row the interceptor found. Setting the `nointercept` sentinel then changed *which pages + * work*, defeating its purpose of comparing the two transports on equal terms. + * + * `URLDecoder` is used rather than `Uri.decode` so this stays testable off-device, and `+` is + * protected first because `URLDecoder` -- alone among the two -- turns it into a space, which + * would break any stored path containing a literal plus. + */ + private fun decodeRequestPath(path: String): String = + try { + URLDecoder.decode(path.replace("+", "%2B"), "UTF-8") + } catch (e: IllegalArgumentException) { + // A malformed escape ("%zz") is not a reason to fail the request: look it up verbatim + // and let the row simply not be found. + log.warn("Cannot decode request path '{}', using it as-is: {}", path, e.message) + path + } + /** Answers one parsed request. */ private fun serveRequest( writer: PrintWriter, @@ -351,7 +379,7 @@ class WebServer( } } - when (val lookup = contentSource.lookup(path)) { + when (val lookup = contentSource.lookup(decodeRequestPath(path))) { is DocumentationLookup.Found -> { sendContent(writer, output, lookup.content) } diff --git a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt index c1eb4ab88a..98f0a1ab7d 100644 --- a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt +++ b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt @@ -306,6 +306,53 @@ class WebServerTest { } } + // The in-process transport matches on WebResourceRequest.url.path, which is already decoded, so + // this one has to decode too or the nointercept sentinel changes which pages resolve. + @Test + fun `a percent-encoded request path is looked up decoded, as the other transport does`() { + assertLookedUpPath(requested = "/a/my%20file.html", expected = "a/my file.html") + } + + // URLDecoder turns "+" into a space; a stored path containing a literal plus must survive. + @Test + fun `a plus in a request path stays a plus`() { + assertLookedUpPath(requested = "/a/c++.html", expected = "a/c++.html") + } + + // A malformed escape is not a reason to fail the request: look it up verbatim and 404 naturally. + @Test + fun `a malformed escape is looked up verbatim rather than failing the request`() { + assertLookedUpPath(requested = "/a/%zz.html", expected = "a/%zz.html") + } + + private fun assertLookedUpPath( + requested: String, + expected: String, + ) { + val port = freePort() + val db = mockk(relaxed = true) + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns db + val queried = mutableListOf>() + every { db.rawQuery(match { it.contains("FROM Content") }, any()) } answers + { + @Suppress("UNCHECKED_CAST") + (secondArg?>())?.let { queried += it as Array } + mockk(relaxed = true) { every { count } returns 0 } + } + + val server = WebServer(testConfig(port)) + val serverThread = Thread { server.start() }.apply { isDaemon = true } + serverThread.start() + try { + awaitPortBound(port) + sendRawGetRequestAndAwaitClose(port, requested) + assertEquals(listOf(expected), queried.map { it.first() }) + } finally { + server.stop() + serverThread.join(2_000) + } + } + // Blocks until the server closes the connection (every response sends "Connection: close"), // so by the time this returns the server has fully finished processing this one request -- // making repeated calls a reliable way to serialize several full request/response cycles. diff --git a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt index 3b74dc4e58..d0ef307e89 100644 --- a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt +++ b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt @@ -219,6 +219,20 @@ class DocumentationContentSource( } } + /** + * Applies any pending debug-database swap, so a caller can decide whether its own per-database + * caches are stale *before* it reads. + * + * Without this, a caller that checks [generation] and then calls [lookup] or [withDatabase] + * checks the generation from before the swap those calls perform: on the request that swaps, it + * still believes its caches belong to the new database. Idempotent and throttled by the debug + * check interval, so calling it and then reading costs one extra timestamp comparison. + */ + fun refreshDatabase() { + openIfNeeded() + swapDebugDatabaseIfNewer() + } + /** * Runs [block] against the active database with the read lock held, for the queries this class * does not own -- the bookshelf join, the template lookup, the developer table dumps. @@ -295,7 +309,17 @@ class DocumentationContentSource( // Primed before the row is read, not inside decompressBrotli, so a database's dictionary is // loaded on its first content fetch (ADFA-5153's contract) rather than on the first fetch // that happens to be Brotli-compressed. Still at most once per database. - compressionDictionary(database) + // + // Best-effort, unlike the call inside the decode: this is an optimisation, and letting a + // transient failure here propagate would fail *every* lookup -- including rows with + // compression = 'none', which need no dictionary at all. The staleness flag is left set, so + // the next read retries, and a brotli row that genuinely cannot resolve its dictionary still + // fails loudly from decompressBrotli. + try { + compressionDictionary(database) + } catch (e: Exception) { + log.warn("Could not prime the compression dictionary; will retry on the next read: {}", e.message) + } database.rawQuery(CONTENT_QUERY, arrayOf(path)).use { cursor -> if (cursor.count == 0) return DocumentationLookup.NotFound From 3be53d3f5e84538bfa76f3f3f0b890895aa221a9 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 24 Aug 2026 11:44:25 -0700 Subject: [PATCH 4/8] ADFA-5176: Stop DocumentationContent pretending to have value semantics It was a data class whose equals/hashCode were overridden back to identity, because generated equality over a ByteArray compares identity anyway and comparing multi-megabyte content is not what any caller wants. What that left behind was copy(), which returned an object unequal to its source. Nothing here needs value semantics, so the class no longer offers them. Co-Authored-By: Claude Opus 5 --- .../DocumentationContentSource.kt | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt index d0ef307e89..20515c2278 100644 --- a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt +++ b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt @@ -87,17 +87,19 @@ fun joinChunks(chunks: List): ByteArray { return joined } -/** One row of documentation content, decoded and rendered, ready to send. */ -data class DocumentationContent( +/** + * One row of documentation content, decoded and rendered, ready to send. + * + * Deliberately not a `data class`. Generated equality over a [ByteArray] compares identity, which is + * never what a caller means, and comparing multi-megabyte content is not what it wants either -- so + * this used to be a data class with `equals`/`hashCode` overridden back to identity. That left + * `copy()` behind, returning an object unequal to the one it was copied from. Nothing needs + * value semantics here, so the class simply does not offer them. + */ +class DocumentationContent( val bytes: ByteArray, val mimeType: String, -) { - // Data class equality over a ByteArray would compare identity, which is never what a caller - // means; content equality on a multi-megabyte blob is not what it wants either. - override fun equals(other: Any?): Boolean = this === other - - override fun hashCode(): Int = System.identityHashCode(this) -} +) /** What a [DocumentationContentSource.lookup] found for a path. */ sealed interface DocumentationLookup { From 8da9283eba92055f28f2bea79f384fc74fc89d2c Mon Sep 17 00:00:00 2001 From: David Schachter Date: Tue, 25 Aug 2026 15:31:15 -0700 Subject: [PATCH 5/8] ADFA-5176: Do not build the shared interceptor during Activity construction HelpActivity and IDETooltipWebViewFragment forced DocumentationRequestInterceptor.shared from a property initializer, so the whole lazy ran during construction, before onCreate, on the main thread. Two consequences. Environment.DOC_DB is a plain static File with no initializer, assigned only by Environment.init(). DeviceProtectedApplicationLoader wraps that call in runCatching and the credential-protected loader returns before it when storage is not ready, so null is a state the app can really be in -- and the non-null parameter turned it into an NPE that killed the activity before it existed. shared is nullable now and declines instead, which puts the request back on the local web server: the same thing a null from intercept() already means everywhere else. The lazy also stats external storage for the nointercept sentinel. On the main thread that is a disk read under a StrictMode policy built with detectAll(), and on a contended FUSE mount it stalls the frame that opens the screen. Touched from shouldInterceptRequest instead, on a WebView thread, the way FAQActivity already did it. Not covered here: intercept() still has no throw guard, so an Error (an OOM decoding a large row) escapes onto a Chromium thread rather than falling through to the server the way the class documents. That is a separate finding from the same review. Found in review of PR #1726. --- .../activities/editor/FAQActivity.kt | 2 +- .../fragments/IDETooltipWebViewFragment.kt | 7 +++-- .../activities/editor/HelpActivity.kt | 15 ++++++++-- .../DocumentationRequestInterceptor.kt | 29 ++++++++++++++----- 4 files changed, 40 insertions(+), 13 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/FAQActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/FAQActivity.kt index d1251917f3..7c07e13d16 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/FAQActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/FAQActivity.kt @@ -71,7 +71,7 @@ class FAQActivity : EdgeToEdgeIDEActivity() { view: WebView, request: WebResourceRequest, ): WebResourceResponse? = - DocumentationRequestInterceptor.shared.intercept(request) + DocumentationRequestInterceptor.shared?.intercept(request) ?: super.shouldInterceptRequest(view, request) } diff --git a/app/src/main/java/com/itsaky/androidide/fragments/IDETooltipWebViewFragment.kt b/app/src/main/java/com/itsaky/androidide/fragments/IDETooltipWebViewFragment.kt index aff663b718..58f1439001 100644 --- a/app/src/main/java/com/itsaky/androidide/fragments/IDETooltipWebViewFragment.kt +++ b/app/src/main/java/com/itsaky/androidide/fragments/IDETooltipWebViewFragment.kt @@ -38,7 +38,10 @@ import com.itsaky.androidide.documentation.DocumentationRequestInterceptor class IDETooltipWebviewFragment : Fragment() { private lateinit var webView: WebView private lateinit var website : String - private val documentation = DocumentationRequestInterceptor.shared + // by lazy: an initializer here runs during Fragment construction on the main thread, where the + // interceptor's sentinel check is a disk read StrictMode reports and a missing database is a + // crash before the view exists. See HelpActivity for the same note. + private val documentation by lazy { DocumentationRequestInterceptor.shared } //This warning is unnecessary because we control the content @SuppressLint("SetJavaScriptEnabled") @@ -83,7 +86,7 @@ class IDETooltipWebviewFragment : Fragment() { override fun shouldInterceptRequest( view: WebView, request: WebResourceRequest, - ): WebResourceResponse? = documentation.intercept(request) ?: super.shouldInterceptRequest(view, request) + ): WebResourceResponse? = documentation?.intercept(request) ?: super.shouldInterceptRequest(view, request) override fun shouldOverrideUrlLoading(view: WebView, request: WebResourceRequest): Boolean { // Allow loading of local assets files diff --git a/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt b/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt index 45fd0a5ee2..ecbafda31c 100644 --- a/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt +++ b/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt @@ -70,7 +70,13 @@ class HelpActivity : BaseIDEActivity() { // ADFA-5176: answers documentation requests from the database in-process, so loading a page // no longer opens a TCP connection per asset to the local web server. - private val documentation = DocumentationRequestInterceptor.shared + // + // by lazy, not an initializer: forcing it here runs during Activity construction, before + // onCreate, on the main thread -- which both stats external storage for the sentinel under a + // StrictMode policy that reports it, and dies before this screen exists if the shared + // interceptor cannot be built. Touched from shouldInterceptRequest instead, on a WebView + // thread, the way FAQActivity does it. + private val documentation by lazy { DocumentationRequestInterceptor.shared } // Wall-clock start of the page currently loading, for the ADFA-5176 measurement. // elapsedRealtime, not currentTimeMillis: an NTP correction or a user clock change between @@ -124,7 +130,10 @@ class HelpActivity : BaseIDEActivity() { override fun shouldInterceptRequest( view: android.webkit.WebView, request: android.webkit.WebResourceRequest, - ): android.webkit.WebResourceResponse? = documentation.intercept(request) ?: super.shouldInterceptRequest(view, request) + ): android.webkit.WebResourceResponse? { + val intercepted = documentation?.intercept(request) + return intercepted ?: super.shouldInterceptRequest(view, request) + } override fun onPageStarted( view: android.webkit.WebView?, @@ -150,7 +159,7 @@ class HelpActivity : BaseIDEActivity() { "Loaded '{}' in {} ms; in-process totals so far: {}.", url, SystemClock.elapsedRealtime() - pageLoadStartMillis, - documentation.servedSummary(), + documentation?.servedSummary(), ) pageLoadStartMillis = 0L } diff --git a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt index 95f1cb8cab..cf82d93988 100644 --- a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt +++ b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt @@ -123,13 +123,28 @@ class DocumentationRequestInterceptor( * a WebView can outlive that, and neither should be able to close the other's handle. The * cost of the second handle is SQLite's page cache plus the dictionary, a few MB. */ - val shared: DocumentationRequestInterceptor by lazy { - DocumentationRequestInterceptor( - DocumentationContentSource( - Environment.DOC_DB, - File(getExternalStorageDirectory(), "Download/documentation.db"), - ), - ) + val shared: DocumentationRequestInterceptor? by lazy { + // Null when Environment.init() has not run, which is a real state, not a defensive + // nicety: DeviceProtectedApplicationLoader wraps that call in runCatching, and the + // credential-protected loader returns before it when storage is not ready. DOC_DB is a + // plain static File with no initializer, so it is null in both cases, and the non-null + // parameter below turns that into an NPE at the first touch of this property. Declining + // instead puts the request on the local web server, which is exactly what a null return + // from intercept() already means everywhere else. + val database = Environment.DOC_DB + if (database == null) { + LoggerFactory + .getLogger(DocumentationRequestInterceptor::class.java) + .warn("Environment.DOC_DB is not set; documentation requests stay on the web server.") + null + } else { + DocumentationRequestInterceptor( + DocumentationContentSource( + database, + File(getExternalStorageDirectory(), "Download/documentation.db"), + ), + ) + } } } } From 3336ad6432c88d5573f94b9be330f9e7b98e5c79 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 15:48:51 +0000 Subject: [PATCH 6/8] ADFA-5176: Close the review findings on cache clearing and null handling - switchToDatabase clears templateCache with the swap, restoring what stage's WebServer.switchToDatabase did; the comments saying templates are dropped on swap are true again. - The cs0 clear-cache sentinel also clears the shared interceptor's source, not just the server's own. The interceptor's shared property is backed by an explicit Lazy so an interceptor that was never built is not created just to empty its cache. - realHandleBsEndpoint answers an empty bookshelf join with a 500: group_concat over an empty subquery yields one row whose value is NULL, which isCursorOneRow passes and the null then fell out of withDatabase as a zero-byte closed connection. - HelpActivity.onPageFinished no longer force-initializes the shared interceptor on the main thread: the lazy is explicit, and the load summary reads it only when shouldInterceptRequest already did. Regression tests: a swap re-renders templates from the new database (common), and /pr/bs over an empty join sends HTTP 500 (app). --- .../androidide/localWebServer/WebServer.kt | 16 ++++- .../localWebServer/WebServerTest.kt | 27 ++++++++ .../activities/editor/HelpActivity.kt | 19 ++++-- .../DocumentationContentSource.kt | 1 + .../DocumentationRequestInterceptor.kt | 61 ++++++++++++------- .../DocumentationContentSourceTest.kt | 34 +++++++++++ 6 files changed, 131 insertions(+), 27 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt b/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt index 2c2125ff3d..f8cc942cad 100644 --- a/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt +++ b/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt @@ -7,6 +7,7 @@ import android.os.Environment.getExternalStorageDirectory import com.itsaky.androidide.documentation.DocumentationContent import com.itsaky.androidide.documentation.DocumentationContentSource import com.itsaky.androidide.documentation.DocumentationLookup +import com.itsaky.androidide.documentation.DocumentationRequestInterceptor import com.itsaky.androidide.utils.ContentTypeHeaders import com.itsaky.androidide.utils.DatabaseVersionResolver import org.slf4j.LoggerFactory @@ -563,7 +564,12 @@ class WebServer( output: java.io.OutputStream, ) { if (debugEnabled) log.debug("Entering handleBsEndpoint().") - if (clearCacheEnabled) contentSource.clearTemplateCache() + if (clearCacheEnabled) { + // The in-app WebViews are served by the shared interceptor's own source, not this + // server's, so the developer sentinel must clear both caches. + contentSource.clearTemplateCache() + DocumentationRequestInterceptor.clearSharedTemplateCache() + } var outputStarted = false @@ -684,6 +690,14 @@ ORDER BY BC.category, // get the JSON from the bookshelf table cursor.moveToFirst() val json = cursor.getBlob(0) + if (json == null) { + // group_concat over an empty join still yields one row, whose value is + // NULL -- so isCursorOneRow passes. Answer it here, or the null would + // fall out of withDatabase as a zero-byte closed connection. + log.error("Bookshelf query returned no rows.") + sendError(writer, output, httpInternalServerError, "Internal Server Error", "Bookshelf query returned no rows.") + return@withDatabase null + } if (debugEnabled) log.debug("json content = '${String(json)}'.") if (debugEnabled) log.debug("before fetch bookshelf template ID = '$bookshelfTemplateId'") diff --git a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt index 98f0a1ab7d..240592db8e 100644 --- a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt +++ b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt @@ -353,6 +353,33 @@ class WebServerTest { } } + // A bookshelf join that matches nothing still yields one row from group_concat -- its value is + // just NULL. That must come back as an explicit 500, not as a zero-byte closed connection. + @Test + fun `an empty bookshelf join answers 500 instead of closing with no bytes`() { + val port = freePort() + val db = mockk(relaxed = true) + every { SQLiteDatabase.openDatabase(any(), isNull(), any()) } returns db + every { db.rawQuery(match { it.contains("group_concat") }, any()) } returns + mockk(relaxed = true) { + every { count } returns 1 + every { moveToFirst() } returns true + every { getBlob(0) } returns null + } + + val server = WebServer(testConfig(port)) + val serverThread = Thread { server.start() }.apply { isDaemon = true } + serverThread.start() + try { + awaitPortBound(port) + val response = sendRawGetRequest(port, "/pr/bs") + assertTrue("Expected a 500 status line, got:\n$response", response.startsWith("HTTP/1.1 500")) + } finally { + server.stop() + serverThread.join(2_000) + } + } + // Blocks until the server closes the connection (every response sends "Connection: close"), // so by the time this returns the server has fully finished processing this one request -- // making repeated calls a reliable way to serialize several full request/response cycles. diff --git a/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt b/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt index ecbafda31c..95814765d6 100644 --- a/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt +++ b/common/src/main/java/com/itsaky/androidide/activities/editor/HelpActivity.kt @@ -71,12 +71,14 @@ class HelpActivity : BaseIDEActivity() { // ADFA-5176: answers documentation requests from the database in-process, so loading a page // no longer opens a TCP connection per asset to the local web server. // - // by lazy, not an initializer: forcing it here runs during Activity construction, before + // Lazy, not an initializer: forcing it here runs during Activity construction, before // onCreate, on the main thread -- which both stats external storage for the sentinel under a // StrictMode policy that reports it, and dies before this screen exists if the shared // interceptor cannot be built. Touched from shouldInterceptRequest instead, on a WebView - // thread, the way FAQActivity does it. - private val documentation by lazy { DocumentationRequestInterceptor.shared } + // thread, the way FAQActivity does it. An explicit Lazy, so onPageFinished (main thread) can + // log without being the first touch. + private val documentationLazy = lazy { DocumentationRequestInterceptor.shared } + private val documentation by documentationLazy // Wall-clock start of the page currently loading, for the ADFA-5176 measurement. // elapsedRealtime, not currentTimeMillis: an NTP correction or a user clock change between @@ -155,11 +157,20 @@ class HelpActivity : BaseIDEActivity() { // The summary's counters are process-cumulative, so they are labelled as // such rather than read as this page's -- the tenth page in a session // would otherwise report the whole session's bytes as its own. + // Only read the interceptor if some request already forced it: this + // runs on the main thread, and being the first touch would build the + // interceptor (and stat external storage) here. + val summary = + if (documentationLazy.isInitialized()) { + documentation?.servedSummary() + } else { + "interceptor not yet used" + } log.info( "Loaded '{}' in {} ms; in-process totals so far: {}.", url, SystemClock.elapsedRealtime() - pageLoadStartMillis, - documentation?.servedSummary(), + summary, ) pageLoadStartMillis = 0L } diff --git a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt index 20515c2278..ff7e14360f 100644 --- a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt +++ b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationContentSource.kt @@ -611,6 +611,7 @@ class DocumentationContentSource( database = opened databaseTimestamp = timestamp compressionDictionaryStale = true + templateCache.clear() generation++ try { diff --git a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt index cf82d93988..7420675807 100644 --- a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt +++ b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt @@ -93,6 +93,9 @@ class DocumentationRequestInterceptor( "${servedRequests.get()} requests, ${servedBytes.get()} bytes served in-process" } + /** Drops this interceptor's compiled templates; the source recompiles them on demand. */ + fun clearTemplateCache() = contentSource.clearTemplateCache() + private fun response(content: DocumentationContent): WebResourceResponse { val (type, charset) = mimeAndCharset(content.mimeType) return WebResourceResponse(type, charset, ByteArrayInputStream(content.bytes)) @@ -116,6 +119,33 @@ class DocumentationRequestInterceptor( private const val SERVER_HOST = "localhost" private const val SERVER_PORT = 6174 + // Explicit Lazy rather than `by lazy` alone, so clearSharedTemplateCache can ask whether + // the interceptor exists yet without creating it. + private val sharedLazy = + lazy { + // Null when Environment.init() has not run, which is a real state, not a defensive + // nicety: DeviceProtectedApplicationLoader wraps that call in runCatching, and the + // credential-protected loader returns before it when storage is not ready. DOC_DB is a + // plain static File with no initializer, so it is null in both cases, and the non-null + // parameter below turns that into an NPE at the first touch of this property. Declining + // instead puts the request on the local web server, which is exactly what a null return + // from intercept() already means everywhere else. + val database = Environment.DOC_DB + if (database == null) { + LoggerFactory + .getLogger(DocumentationRequestInterceptor::class.java) + .warn("Environment.DOC_DB is not set; documentation requests stay on the web server.") + null + } else { + DocumentationRequestInterceptor( + DocumentationContentSource( + database, + File(getExternalStorageDirectory(), "Download/documentation.db"), + ), + ) + } + } + /** * The interceptor every WebView in the process shares, and with it one database handle and * one copy of the compression dictionary. Deliberately separate from the source `WebServer` @@ -123,28 +153,15 @@ class DocumentationRequestInterceptor( * a WebView can outlive that, and neither should be able to close the other's handle. The * cost of the second handle is SQLite's page cache plus the dictionary, a few MB. */ - val shared: DocumentationRequestInterceptor? by lazy { - // Null when Environment.init() has not run, which is a real state, not a defensive - // nicety: DeviceProtectedApplicationLoader wraps that call in runCatching, and the - // credential-protected loader returns before it when storage is not ready. DOC_DB is a - // plain static File with no initializer, so it is null in both cases, and the non-null - // parameter below turns that into an NPE at the first touch of this property. Declining - // instead puts the request on the local web server, which is exactly what a null return - // from intercept() already means everywhere else. - val database = Environment.DOC_DB - if (database == null) { - LoggerFactory - .getLogger(DocumentationRequestInterceptor::class.java) - .warn("Environment.DOC_DB is not set; documentation requests stay on the web server.") - null - } else { - DocumentationRequestInterceptor( - DocumentationContentSource( - database, - File(getExternalStorageDirectory(), "Download/documentation.db"), - ), - ) - } + val shared: DocumentationRequestInterceptor? by sharedLazy + + /** + * Clears [shared]'s compiled templates, for the developer clear-cache sentinel. A no-op + * when [shared] has never been touched: it is not created just to empty a cache that does + * not exist yet. + */ + fun clearSharedTemplateCache() { + if (sharedLazy.isInitialized()) sharedLazy.value?.clearTemplateCache() } } } diff --git a/common/src/test/java/com/itsaky/androidide/documentation/DocumentationContentSourceTest.kt b/common/src/test/java/com/itsaky/androidide/documentation/DocumentationContentSourceTest.kt index 6ff57fe02b..eb6a0eda90 100644 --- a/common/src/test/java/com/itsaky/androidide/documentation/DocumentationContentSourceTest.kt +++ b/common/src/test/java/com/itsaky/androidide/documentation/DocumentationContentSourceTest.kt @@ -298,6 +298,40 @@ class DocumentationContentSourceTest { verify { installed.close() } } + @Test + fun `a debug-database swap drops the compiled templates, so pages render from the new database`() { + val installed = templatedDatabase(template = "Hello {{ who }}!") + val debug = templatedDatabase(template = "Goodbye {{ who }}!") + every { SQLiteDatabase.openDatabase(installedFile.absolutePath, isNull(), any()) } returns installed + every { SQLiteDatabase.openDatabase(debugFile.absolutePath, isNull(), any()) } returns debug + + val source = source(debugCheckIntervalMs = 0) + + // Compiles and caches the installed database's template. + assertThat((source.lookup("p") as DocumentationLookup.Found).content.bytes.toString(Charsets.UTF_8)) + .isEqualTo("Hello Kotlin!") + + debugFile.writeText("newer") + debugFile.setLastModified(installedFile.lastModified() + 60_000) + + // Same template id in the new database: only a cleared cache recompiles it from there. + assertThat((source.lookup("p") as DocumentationLookup.Found).content.bytes.toString(Charsets.UTF_8)) + .isEqualTo("Goodbye Kotlin!") + } + + /** A database whose single Content row is templated with id 7, and whose template is [template]. */ + private fun templatedDatabase(template: String): SQLiteDatabase = + mockk(relaxed = true) { + every { rawQuery(match { it.contains("FROM Content") }, any()) } returns + contentCursor(bytes = """{"who": "Kotlin"}""".toByteArray(), templateId = 7) + every { rawQuery(match { it.contains("FROM Templates") }, arrayOf("7")) } returns + mockk(relaxed = true) { + every { count } returns 1 + every { moveToFirst() } returns true + every { getBlob(0) } returns template.toByteArray() + } + } + @Test fun `a debug database that will not open leaves the installed one serving`() { val installed = database(contentCursor(bytes = "installed".toByteArray())) From 28d7608b1f0cda6b75a6e57d456f5dac056a8319 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 15:49:17 +0000 Subject: [PATCH 7/8] ADFA-5176: Fix two comments the last refactors left behind - WebServer.serveRequest's caller talked about a read lock that no longer exists there; the swap now happens inside the content source. - The note explaining sendRawGetRequestAndAwaitClose sat above an unrelated test; move it to the helper it documents. --- .../java/com/itsaky/androidide/localWebServer/WebServer.kt | 3 +-- .../com/itsaky/androidide/localWebServer/WebServerTest.kt | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt b/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt index f8cc942cad..6e7c392dba 100644 --- a/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt +++ b/app/src/main/java/com/itsaky/androidide/localWebServer/WebServer.kt @@ -310,8 +310,7 @@ class WebServer( return sendError(writer, output, 501, "Not Implemented") } - // Use a newer documentation.db from the sdcard if one has appeared. Outside the read lock - // below, because swapping takes the write lock and this lock does not upgrade. + // serveRequest applies any pending sdcard debug-database swap via the content source. serveRequest(writer, output, path) } diff --git a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt index 240592db8e..3417016676 100644 --- a/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt +++ b/app/src/test/java/com/itsaky/androidide/localWebServer/WebServerTest.kt @@ -380,9 +380,6 @@ class WebServerTest { } } - // Blocks until the server closes the connection (every response sends "Connection: close"), - // so by the time this returns the server has fully finished processing this one request -- - // making repeated calls a reliable way to serialize several full request/response cycles. // ADFA-5241: the two transports have to answer the same way about what a response says, and // only a real response proves what this one sends. The decision itself lives in // ContentTypeHeaders, shared with DocumentationRequestInterceptor. @@ -445,6 +442,9 @@ class WebServerTest { socket.getInputStream().readBytes().toString(Charsets.ISO_8859_1) } + // Blocks until the server closes the connection (every response sends "Connection: close"), + // so by the time this returns the server has fully finished processing this one request -- + // making repeated calls a reliable way to serialize several full request/response cycles. private fun sendRawGetRequestAndAwaitClose( port: Int, path: String, From 8c29578ca836c4df2956879458bf2a5129a639f9 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Wed, 26 Aug 2026 15:34:44 -0700 Subject: [PATCH 8/8] ADFA-5176: Retry a missing DOC_DB instead of caching the refusal forever Making `shared` nullable last round fixed the NPE and introduced a quieter bug: `lazy` memoizes whatever the initializer returned, including null. Environment.init() runs inside the loader coroutine -- DeviceProtectedApplicationLoader wraps it in runCatching, and the credential-protected loader returns early when storage is not ready -- so a WebView that asks during direct boot saw DOC_DB unset, and that answer was then cached for the life of the process. In-process documentation stayed off afterwards, and since WebServer is started only by MainActivity and stopped in its onDestroy, opening Help from the editor later had nothing to fall back to either. Only a successful construction is cached now; a null is retried on the next request. clearSharedTemplateCache reads the field directly, so asking whether the interceptor exists still cannot create it. intercept() also catches Throwable. The class documents that anything it returns null for falls through to the web server, and that only held for values: decoding the largest bundled row (8.8 MB over nine chunks) can raise OutOfMemoryError, a pathological template a StackOverflowError, and lookup()'s catch (e: Exception) sees neither. This runs on a Chromium thread, where an escaping Error takes the process down -- the socket transport confined the same failure to one 500. The file's own ensureBrotliAvailable KDoc describes exactly this hazard for the other transport. 102 common tests pass, :app compiles. Found in review of PR #1726. --- .../DocumentationRequestInterceptor.kt | 79 ++++++++++++------- 1 file changed, 50 insertions(+), 29 deletions(-) diff --git a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt index 7420675807..e6b9119881 100644 --- a/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt +++ b/common/src/main/java/com/itsaky/androidide/documentation/DocumentationRequestInterceptor.kt @@ -56,7 +56,19 @@ class DocumentationRequestInterceptor( * The response for [request], or null to let it go to the network. Called on WebView's own * threads; [DocumentationContentSource] is what makes that safe. */ - fun intercept(request: WebResourceRequest): WebResourceResponse? = contentFor(request)?.let { response(it) } + fun intercept(request: WebResourceRequest): WebResourceResponse? = + try { + contentFor(request)?.let { response(it) } + } catch (e: Throwable) { + // Throwable, not Exception. The documented contract is that anything this returns null for + // falls through to the web server, and that only holds for values, not throws: decoding the + // largest bundled row (8.8 MB over nine chunks) can raise OutOfMemoryError and a pathological + // template a StackOverflowError, neither of which lookup()'s catch (e: Exception) sees. This + // runs on a Chromium thread, where an escaping Error takes the process down -- the socket + // transport confined the same failure to one 500 (ADFA-5176 review). + log.error("Serving {} in-process failed; falling back to the web server", request.url, e) + null + } /** * The content to answer [request] with, or null when it is not this class's to answer. Split out @@ -119,32 +131,18 @@ class DocumentationRequestInterceptor( private const val SERVER_HOST = "localhost" private const val SERVER_PORT = 6174 - // Explicit Lazy rather than `by lazy` alone, so clearSharedTemplateCache can ask whether - // the interceptor exists yet without creating it. - private val sharedLazy = - lazy { - // Null when Environment.init() has not run, which is a real state, not a defensive - // nicety: DeviceProtectedApplicationLoader wraps that call in runCatching, and the - // credential-protected loader returns before it when storage is not ready. DOC_DB is a - // plain static File with no initializer, so it is null in both cases, and the non-null - // parameter below turns that into an NPE at the first touch of this property. Declining - // instead puts the request on the local web server, which is exactly what a null return - // from intercept() already means everywhere else. - val database = Environment.DOC_DB - if (database == null) { - LoggerFactory - .getLogger(DocumentationRequestInterceptor::class.java) - .warn("Environment.DOC_DB is not set; documentation requests stay on the web server.") - null - } else { - DocumentationRequestInterceptor( - DocumentationContentSource( - database, - File(getExternalStorageDirectory(), "Download/documentation.db"), - ), - ) - } - } + // Not `lazy`: it memoizes whatever the initializer returned, including null, and null here is + // a transient state rather than a verdict. Environment.init() runs inside the loader coroutine + // -- DeviceProtectedApplicationLoader wraps it in runCatching, and the credential-protected + // loader returns early when storage is not ready -- so a WebView that asks during direct boot + // sees DOC_DB unset. Caching that answer disabled in-process documentation for the whole + // process, and since WebServer is started only by MainActivity and stopped in its onDestroy, + // opening Help from the editor afterwards then had nothing to fall back to (ADFA-5176 review). + // Only a successful construction is cached; a null is retried on the next request. + @Volatile + private var sharedInstance: DocumentationRequestInterceptor? = null + + private val log = LoggerFactory.getLogger(DocumentationRequestInterceptor::class.java) /** * The interceptor every WebView in the process shares, and with it one database handle and @@ -152,8 +150,30 @@ class DocumentationRequestInterceptor( * builds from its own config: the server's comes and goes with the activity that starts it, * a WebView can outlive that, and neither should be able to close the other's handle. The * cost of the second handle is SQLite's page cache plus the dictionary, a few MB. + * + * Null while `Environment.DOC_DB` is still unset, which puts that request on the local web + * server and leaves the next one free to try again. */ - val shared: DocumentationRequestInterceptor? by sharedLazy + val shared: DocumentationRequestInterceptor? + get() { + sharedInstance?.let { return it } + return synchronized(this) { + sharedInstance ?: run { + val database = Environment.DOC_DB + if (database == null) { + log.warn("Environment.DOC_DB is not set yet; this documentation request stays on the web server.") + null + } else { + DocumentationRequestInterceptor( + DocumentationContentSource( + database, + File(getExternalStorageDirectory(), "Download/documentation.db"), + ), + ).also { sharedInstance = it } + } + } + } + } /** * Clears [shared]'s compiled templates, for the developer clear-cache sentinel. A no-op @@ -161,7 +181,8 @@ class DocumentationRequestInterceptor( * not exist yet. */ fun clearSharedTemplateCache() { - if (sharedLazy.isInitialized()) sharedLazy.value?.clearTemplateCache() + // sharedInstance, not shared: asking must not construct the interceptor as a side effect. + sharedInstance?.clearTemplateCache() } } }