Add Android Preload List

[jkmassel]

What?

Adds an Android preloading system for the Gutenberg editor that prefetches editor dependencies before display, matching the existing iOS implementation.

Why?

The Gutenberg editor requires several API calls and asset downloads during initialization, which can cause slow launch times. This PR introduces a preloading architecture that:

• Pre-fetches API responses (post types, themes, settings) so the editor doesn't need to make network calls during startup
• Downloads and caches JavaScript/CSS asset bundles for faster subsequent launches
• Provides progress feedback to the user during preparation
• Establishes feature parity with the existing iOS preloading system

How?

New Core Components:

• EditorService - Orchestrates loading all editor dependencies with progress tracking
• EditorPreloadList - Pre-fetched API responses injected into the editor JavaScript
• EditorAssetsLibrary (in stores/) - Downloads and caches asset bundles
• EditorHTTPClient - Authenticated HTTP client for WordPress REST API
• RESTAPIRepository - REST API calls with response caching
• EditorURLCache - URL-level caching with configurable policies

New Model Classes:

• EditorAssetBundle - Represents downloaded editor assets
• EditorCachePolicy - Configurable caching strategies (Always, MaxAge, Ignore)
• EditorSettings, EditorDependencies - Editor configuration models
• GBKitGlobal, LocalEditorAssetManifest, RemoteEditorAssetManifest - Asset manifest parsing

Demo App Updates:

• Added SitePreparationActivity and SitePreparationViewModel showing the preloading flow with progress UI

Other Changes:

• Minor iOS test fixes and EditorSettings improvements for consistency
• This PR removes the existing warmup code – a more recent version of androidx.webkit exposes prerenderUrlAsync which we should adopt for this purpose.

Testing Instructions

1. Build and run the Android demo app
2. Select a site configuration
3. Observe the "Site Preparation" screen showing loading progress
4. Tap "Prepare from scratch" to test full loading without cache
5. Tap "Prepare" to test cached loading
6. Verify the editor loads correctly with preloaded data
7. Run make test-android to verify all new unit tests pass

See the tests in #250 for examples of tests sites you can use.

[adalpari]

⛏️Aren't we using injection in the example project? ViewModels can be simply injected.

[jkmassel]

I don't think we are at the moment – I'd be happy to adopt it, but I think it's outside the scope of this PR :)

[adalpari]

Do we want to track the exception here?

[jkmassel]

In this case probably not, because we'll treat a cache failure as a simple cache miss. I've modified this to use runCatching in c64592b, which should hopefully make the intent more clear!

[dcalhoun]

@claude

[claude]

Claude finished @dcalhoun's task in 3m 13s —— View job

---

Comprehensive Code Review Complete ✅

I've completed a thorough review of PR #260: Add Android Preload List. This is an excellent implementation that brings feature parity with iOS. Below is my detailed analysis across all requested areas.

---

Executive Summary

Overall Assessment: ✅ Approved with Minor Recommendations

This is a well-architected, thoroughly tested implementation. The code quality is high, security practices are sound, and the test coverage is excellent. I identified a few minor areas for potential improvement, but none are blocking issues.

Key Strengths:

• Comprehensive test coverage (19 test files with 5,000+ lines of tests)
• Well-documented code with clear KDoc comments
• Strong separation of concerns and modular architecture
• Excellent error handling throughout
• Thoughtful caching strategies with configurable policies

---

1. Code Quality ✅

Strengths

• Clean architecture: Clear separation between services, stores, models, and UI
• Consistent naming: Follows Kotlin conventions and project standards
• Well-documented: Comprehensive KDoc comments on all public APIs
• DRY principles: Good reuse of common functionality (e.g., EditorHTTPClient, EditorURLCache)
• Immutability: Models use data class and immutable properties where appropriate

Observations

Function ordering follows project conventions (android/Gutenberg/src/main/java/org/wordpress/gutenberg/services/EditorService.kt:260-374):
The code correctly orders functions by usage/call order rather than alphabetically, as specified in CLAUDE.md. Main methods like prepare() come first, followed by helper methods in the order they're called.

Excellent use of companion objects for factory methods (android/Gutenberg/src/main/java/org/wordpress/gutenberg/services/EditorService.kt:71-169):
The create() and createForTesting() factory methods provide a clean API and separate production from test concerns.

Minor: Consider extracting magic numbers (android/Gutenberg/src/main/java/org/wordpress/gutenberg/services/EditorService.kt:73):

private const val CLEANUP_INTERVAL_SECONDS = 86_400L // 1 day

This is good, but there are a few other magic numbers that could benefit from named constants:

• requestTimeoutSeconds: Long = 60 in EditorHTTPClient.kt:127
• Progress weight values in DependencyWeights enum are well-named ✅

---

2. Security 🔒

Strengths

• Path traversal protection (android/Gutenberg/src/main/java/org/wordpress/gutenberg/model/EditorAssetBundle.kt:136-151):

  val resolvedFile = File(bundleRoot, urlPath).canonicalFile
  val bundleRootCanonical = bundleRoot.canonicalFile
  
  require(
      resolvedFile.path.startsWith(bundleRootCanonical.path + File.separator) ||
          resolvedFile.path == bundleRootCanonical.path
  ) {
      "Asset path escapes bundle root: $urlPath"
  }

  Excellent prevention of path traversal attacks when storing downloaded assets ✅

• URL validation for assets (android/Gutenberg/src/main/java/org/wordpress/gutenberg/stores/EditorAssetsLibrary.kt:240-261):
  Only HTTP/HTTPS URLs with specific extensions (.js, .css, .js.map) are allowed. This prevents arbitrary file downloads.

• Authorization header handling: The authHeader is properly passed through all HTTP requests and not logged

• Cache key hashing (android/Gutenberg/src/main/java/org/wordpress/gutenberg/stores/EditorURLCache.kt:214-219):
  Uses SHA-256 to hash cache keys, preventing cache key collisions and path traversal via cache keys.

Recommendations

Consider sanitizing logging output (android/Gutenberg/src/main/java/org/wordpress/gutenberg/EditorHTTPClient.kt:154, 201):

Log.e(TAG, "HTTP error downloading $url: $statusCode")
Log.e(TAG, "HTTP error fetching $url: $statusCode")

URLs might contain sensitive query parameters (tokens, etc.). Consider redacting query params in logs:

Log.e(TAG, "HTTP error downloading ${url.substringBefore('?')}: $statusCode")

WordPress error parsing is defensive (android/Gutenberg/src/main/java/org/wordpress/gutenberg/EditorHTTPClient.kt:227-243) ✅:
The tryParseWPError() method safely handles malformed JSON without exposing error details to potential attackers.

---

3. Performance ⚡

Strengths

Concurrent downloads (android/Gutenberg/src/main/java/org/wordpress/gutenberg/stores/EditorAssetsLibrary.kt:194-211):

coroutineScope {
    val downloads = assets.map { assetUrl ->
        async {
            fetchAsset(assetUrl, bundle)
            mutex.withLock {
                completed++
                progress?.invoke(EditorProgress(completed = completed, total = assets.size))
            }
        }
    }
    downloads.awaitAll()
}

Excellent parallel downloading of assets with proper progress tracking ✅

Smart caching strategy:

• Three-tier caching: EditorURLCache for API responses, EditorAssetsLibrary for assets, in-memory for preload list
• Configurable cache policies (Always, MaxAge, Ignore) allow flexibility
• Automatic cleanup of old assets (android/Gutenberg/src/main/java/org/wordpress/gutenberg/stores/EditorAssetsLibrary.kt:276-286)

Memory-conscious design (android/Gutenberg/src/main/java/org/wordpress/gutenberg/EditorHTTPClient.kt:190-194):

// Note: This loads the entire response into memory. This is acceptable because
// this method is only used for WordPress REST API responses (editor settings, post
// data, themes, etc.) which are expected to be small (KB range). Large assets like
// JS/CSS files use the download() method which streams directly to disk.

Great comment explaining the design decision. Large files are streamed to disk, small API responses are loaded into memory.

Progress tracking is granular (android/Gutenberg/src/main/java/org/wordpress/gutenberg/services/EditorService.kt:57-68):
Weighted progress reporting gives users accurate feedback on long-running operations.

Observations

Thread-safe caching (android/Gutenberg/src/main/java/org/wordpress/gutenberg/stores/EditorURLCache.kt:30):
Uses ReentrantReadWriteLock for thread-safe cache access ✅

Potential optimization: Consider adding a memory cache layer for frequently accessed items in EditorURLCache. Currently, every cache read parses JSON from disk. For hot paths (like editor settings), an in-memory LRU cache could reduce I/O.

---

4. Testing 🧪

Strengths

Comprehensive coverage: 19 test files covering all major components:

• Core services: EditorServiceTest, EditorHTTPClientTest, RESTAPIRepositoryTest
• Stores: EditorAssetsLibraryTest (764 lines!), EditorURLCacheTest (595 lines!)
• Models: 11 model test files covering all data classes
• Integration: Tests use MockWebServer for realistic HTTP testing

Excellent test quality (android/Gutenberg/src/test/java/org/wordpress/gutenberg/EditorHTTPClientTest.kt):

• Clear test names following Given-When-Then pattern
• Good edge case coverage (unicode, error cases, empty data)
• Proper use of test fixtures and helpers
• Mock implementations for dependencies

Test utilities (android/Gutenberg/src/test/java/org/wordpress/gutenberg/services/EditorServiceTest.kt:133-221):
The EditorServiceMockHTTPClient provides a reusable mock implementation that's easy to understand and maintain.

Observations

Test coverage appears excellent: Every new class has a corresponding test file. The tests cover:

• ✅ Happy paths
• ✅ Error cases
• ✅ Edge cases (empty data, null handling)
• ✅ Concurrency scenarios
• ✅ Serialization/deserialization

Missing: Integration tests for the full prepare → load flow. While unit tests are comprehensive, an end-to-end test that exercises the full flow from EditorService.prepare() through GutenbergView loading would provide additional confidence. Consider adding this in a follow-up.

---

5. Documentation 📚

Strengths

Excellent KDoc coverage: All public APIs have clear documentation with:

• Purpose and behavior descriptions
• Parameter documentation
• Return value descriptions
• Usage examples (e.g., android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt:46-84)

PR description is exemplary: The PR body clearly explains:

• What was added
• Why it was needed
• How it works
• Testing instructions

Code comments explain "why" not "what":

// Note: This loads the entire response into memory. This is acceptable because...

Comments focus on design decisions and rationale, not just describing code.

Recommendations

README update needed: The main README doesn't appear to have been updated to document the new preloading system. Consider adding:

• A section on EditorService and how to use it
• Examples of preloading dependencies before creating a GutenbergView
• Performance benefits of preloading

Demo app is well-implemented: The SitePreparationActivity serves as excellent documentation-by-example for how to use the preloading system.

---

Specific File Reviews

EditorService.kt ⭐ Excellent

• Clean separation of concerns
• Good progress tracking implementation
• Automatic cleanup scheduling is clever (onceEvery helper)
• Minor: The onceEvery method uses SharedPreferences, which could fail silently if null. Consider logging when SharedPreferences is unavailable (line 384)

EditorHTTPClient.kt ✅ Good

• Solid error handling with typed errors
• Timeout configuration is appropriate
• WP error parsing is defensive
• Minor logging concern mentioned in Security section

EditorAssetsLibrary.kt ⭐ Excellent

• Concurrent downloads with proper error handling (fail-fast is correct)
• Atomic bundle moves prevent partial bundles
• Cleanup logic is safe and tested
• Bundle validation is thorough

EditorURLCache.kt ✅ Good

• Thread-safe implementation with ReentrantReadWriteLock
• SHA-256 cache keys prevent collisions
• Clean() method for expired entries is well-designed
• Consider: In-memory cache layer for hot paths (mentioned in Performance)

GutenbergView.kt ✅ Good

• Well-documented constructor with coroutine scope ownership clearly explained
• Fast path for pre-loaded dependencies is smart
• Lifecycle management looks correct
• Excellent improvement: The previous review comments about coroutine scope injection were addressed (lines 84, 118)

Model Classes ✅ Good

• All models are well-structured data classes
• Proper use of kotlinx.serialization
• Good companion object factories
• Path traversal protection in EditorAssetBundle is excellent

---

Addressing Previous Review Comments

All previous review comments from @adalpari have been properly addressed:

• ✅ Nullability issues fixed (EditorDependenciesSerializer)
• ✅ Kotlin idioms applied (using orEmpty())
• ✅ Coroutine scope injection implemented
• ✅ Error handling clarified with runCatching

---

Final Recommendations

Must Address

None - no blocking issues found.

Should Consider

1. Add integration test: End-to-end test for prepare → load flow
2. Update README: Document the new preloading system
3. Sanitize URLs in logs: Redact query parameters in error logs to prevent credential leaks

Nice to Have

4. In-memory cache layer: Add LRU cache for hot paths in EditorURLCache
5. Metrics: Consider adding telemetry for cache hit rates, download times
6. Named constants: Extract remaining magic numbers (timeout values)

---

Conclusion

This is an excellent implementation that I'm happy to approve. The code quality is high, the architecture is sound, testing is comprehensive, and the security practices are solid. The recommendations above are minor improvements that don't block merging.

The PR successfully achieves its goals:

• ✅ Feature parity with iOS preloading system
• ✅ Improved editor startup performance
• ✅ Clean, maintainable architecture
• ✅ Comprehensive test coverage
• ✅ Good documentation

Great work! 🎉

---

[claude]

Security consideration: URLs might contain sensitive query parameters (auth tokens, etc.). Consider sanitizing URLs in logs to prevent credential leakage:

val sanitizedUrl = url.substringBefore('?')
Log.e(TAG, "HTTP error downloading $sanitizedUrl: $statusCode")

This applies to lines 154, 201, and any other places where URLs are logged.

[claude]

Security: Excellent path traversal protection ✅

This code properly prevents directory traversal attacks by:

1. Canonicalizing both the resolved file and bundle root
2. Checking that the resolved path is within the bundle root
3. Throwing an exception if the path escapes the bundle

This is a critical security control for handling untrusted asset URLs.

[claude]

Performance: Excellent concurrent downloads ⚡

This implementation properly:

• Downloads assets in parallel using async
• Protects shared state (progress) with a mutex
• Uses awaitAll() to fail-fast if any download fails
• Reports granular progress to the user

The fail-fast approach (line 196 comment) is the right choice - a partial bundle would result in a broken editor.

[claude]

Security: Good cache key design ✅

Using SHA-256 to hash method:url prevents:

• Cache key collisions
• Path traversal via cache keys (URLs could contain ../)
• Information leakage through predictable cache file names

Consider: For additional security, you could include the site ID in the hash to ensure complete isolation between different sites.

[jkmassel]

@nbradbury's issue is fixed for iOS in #288.

This PR is already pretty huge, so I'll open a follow-up to address it there.

[dcalhoun]

@nbradbury's issue is fixed for iOS in #288.

This PR is already pretty huge, so I'll open a follow-up to address it there.

To clarify, I believe the Demo app does not support OAuth tokens at all, so WPCOM Simple sites will not work there. They must be tested in the WordPress/Jetpack app.

For the Gutenberg rest_no_route issue, this seems like CMM-1163-linear-issue, which is a regression we need to address for both iOS and Android.