Media V2: upload → grid auto-refresh is untested (create→cache→collection-update path)

[jkmassel]

Summary

The Media V2 upload flow relies on a wordpress-rs cache side effect to refresh the Media Library grid after an upload, and that path has no test on either side of the FFI boundary. The mechanism works today (verified against pinned wordpress-rs 0.4.0 / 2bd38f23), so this is a regression-risk / coverage gap, not a current break.

Surfaced while reviewing the V2 upload stack (#25621 MediaUploader, #25622 grid wiring).

How the grid refresh actually works (verified)

MediaUploader discards the created media on success (MediaUploader.swift:261 — _ = try await transport.upload(...)) and just removes the queue row. The uploaded item still appears in the grid because of a cache side effect, not an explicit "succeeded" channel:

1. DefaultMediaUploadTransport.upload → WpService.uploadMedia → cache-aware media().createMedia(...) (WpService+Extensions.swift:14, enabled on iOS via the PROGRESS_REPORTING_ENABLED define in wordpress-rs Package.swift:48).
2. MediaService::create_media upserts the new attachment into the cache media_edit_context table (wp_mobile/src/service/media.rs:738-744), firing a SQLite UpdateHook { action: Insert, table: MediaEditContext }.
3. MediaLibraryViewModel.observe() filters client.cache.databaseUpdatesPublisher() by collection.isRelevantUpdate(hook:) (which treats MediaEditContext as relevant) and calls reload() (MediaLibraryViewModel.swift:125-136).

The chain is correct — but every hop in it is unverified.

The gap (two layers)

wordpress-rs (root cause). The cache-aware create_media is never invoked by any test at the pinned revision — a repo-wide search for create_media( finds only its own definition. The update-hook machinery (start_listening_for_updates / did_update) is untested for any table. Closest-but-insufficient: test_media_collection.rs covers only the list/refresh() read-through path; wp_api_integration_tests/.../test_media_mut.rs::upload_media hits the raw, non-cache API and asserts only the title; the media.rs unit tests cover upsert round-trips, not the create_media composition.

iOS. No test locks observe() → reload() on a relevant cache hook. It is structurally unreachable from a unit test today — MediaLibraryViewModel builds its client / collection directly from WpService with no injection seam, and MediaLibraryViewModelFilterTests.swift:7 documents exactly this: the "load, observe, ... paths run through the app, not unit tests."

Why this is an omission, not a decision

The post side already has this exact test: wp_mobile_integration_tests/tests/test_posts_mut.rs::test_create_draft_inserts_into_draft_collection creates a draft via PostService::create_post and asserts the live collection picks it up with no refresh. The media service is a deliberate mirror (source comments: "Mirrors PostService::create_post"), but the create → cache → observe test was never ported.

Proposed fix

Port the post-side test to the media path. Draft below (mirrors the template, and adds a direct cache-row assertion so a dropped upsert can't sneak through):

use wp_api::media::MediaCreateParams;
use wp_mobile::collection::MediaItemState;
use wp_mobile::filters::MediaListFilter;
use wp_mobile_integration_tests::*;
// If the harness glob does not re-export the fixture path, add:
// use wp_api_integration_tests::MEDIA_TEST_FILE_PATH;

#[tokio::test]
#[serial]
async fn test_create_media_inserts_into_media_collection() {
    // Mirror of `test_posts_mut.rs::test_create_draft_inserts_into_draft_collection`,
    // ported to the media path.
    //
    // 1. Create a default-filter media collection and `refresh()` to load page 1.
    // 2. Record the current item count.
    // 3. Upload a new media item via the cache-aware `WpService.media().create_media`
    //    (the `MediaService::create_media` path), which upserts the new media into
    //    the cache and fans out to live collections via `notify_collections` →
    //    `update_media_membership`.
    //
    // Expected result: the new media appears in `load_items()` WITHOUT a second
    // `refresh()`, the cache holds the row, and the live collection observed it.

    let ctx = create_test_context();

    let collection = ctx
        .service
        .media()
        .create_media_metadata_collection_with_edit_context(MediaListFilter::default(), 10);

    collection.refresh().await.expect("refresh should succeed");

    let items_before = collection
        .load_items()
        .await
        .expect("load_items should succeed");

    // Create a new media item through the cache-aware service path. `file_path`
    // is required (not Option) on `MediaCreateParams`; reuse the same fixture the
    // wp_api integration tests upload.
    let created = ctx
        .service
        .media()
        .create_media(
            &MediaCreateParams {
                title: Some("Integration Test Media".to_string()),
                file_path: MEDIA_TEST_FILE_PATH.to_string(),
                ..Default::default()
            },
            None, // no RequestContext: we're not exercising upload progress here
        )
        .await
        .expect("create_media should succeed");

    // (a) The cache must contain the freshly-created row. This is the assertion
    // that directly catches a regression where `create_media` stops upserting.
    let cached = ctx
        .service
        .media()
        .read_media_by_ids_from_db(&[created.id.0])
        .expect("read_media_by_ids_from_db should succeed");
    assert_eq!(cached.len(), 1, "created media should be written to the cache");
    assert_eq!(cached[0].data.id, created.id);

    // (b) The live collection must observe the new item WITHOUT a manual refresh,
    // proving create -> cache -> update-hook/notify -> collection-membership works.
    let items_after = collection
        .load_items()
        .await
        .expect("load_items should succeed");
    assert_eq!(
        items_after.len(),
        items_before.len() + 1,
        "creating media should add one item to the collection without a refresh"
    );

    let inserted = items_after
        .iter()
        .find(|item| item.id == created.id.0)
        .expect("new media should appear in load_items without a refresh");
    assert!(
        matches!(inserted.state, MediaItemState::Fresh { .. }),
        "newly created media should be Fresh (data present in cache), got {:?}",
        inserted.state
    );

    RestoreServer::db().await;
}

What it locks: the full create → cache → update-hook → collection-membership path for media. If create_media stopped upserting (media.rs:738-744), assertion (a) fails immediately; if notify_collections (media.rs:746) were dropped, assertion (b) fails while (a) passes — pinpointing which half broke.

Caveats: it is a true integration test (live wp-env backend + credentials, like its siblings); it must be #[serial] (it mutates server state and ends with RestoreServer::db()); and it needs use wp_api_integration_tests::MEDIA_TEST_FILE_PATH; for the fixture if the harness glob doesn't re-export it. A no-network unit variant is possible (mock the create response, assert read_media_by_ids_from_db + collection membership) but requires hand-rolling a valid MediaWithEditContext JSON body, so the integration port is the lower-maintenance choice.

Optionally, add an injection seam to MediaLibraryViewModel so the iOS observe() → reload() half can be unit-tested by publishing a synthetic relevant UpdateHook and asserting a reload.

References

• iOS: Add MediaUploader actor with broadcast queue lifecycle #25621, Add upload UI and wire the uploader into the grid #25622 · MediaUploader.swift:261, MediaLibraryViewModel.swift:125-136
• wordpress-rs 0.4.0 (2bd38f23): create path wp_mobile/src/service/media.rs:725-749; template wp_mobile_integration_tests/tests/test_posts_mut.rs::test_create_draft_inserts_into_draft_collection
• The primary fix (the test above) lands in Automattic/wordpress-rs; filing here as the follow-up to the V2 upload stack. Happy to cross-file or move it to wordpress-rs.

[dangermattic]

Thanks for reporting! 👍

[jkmassel]

The substance here is a wordpress-rs test: the create → cache → UpdateHook → collection-membership path is implemented in wp_mobile/wp_mobile_cache and is best tested at the source — for all consumers, with the post-side template (test_posts_mut.rs::test_create_draft_inserts_into_draft_collection) and the wp-env harness already in place.

Refiled with the drafted test as Automattic/wordpress-rs#1405. Closing in favor of that.

The only iOS-side residue — MediaLibraryViewModel.observe() → reload() on a relevant cache hook — is thin, stable glue that isn't unit-testable without an injection seam, and the real regression risk is the wordpress-rs composition. Not worth tracking on its own; reopen if we add that seam for other reasons.