Skip to content

Media: cache-aware create_media → cache → collection-update path is untested (port the post-side test) #1405

Description

@jkmassel

Summary

The cache-aware media-create path — MediaService::create_media → cache upsert → UpdateHook → live collection membership — has no test, even though a live consumer (the WordPress iOS Media Library grid) silently depends on it to refresh after an upload. The post side already has the exact test this needs; it was never ported to media.

This is a coverage / regression-risk gap, not a known break — the path works as of 0.4.0 (2bd38f23).

The path (verified at 0.4.0)

MediaService::create_media (wp_mobile/src/service/media.rs, ~L725–749):

  1. POSTs the new media, then self.cache.execute(|conn| MediaRepository::<EditContext>::new().upsert(conn, &self.db_site, &media)) — writes the row into media_edit_context.
  2. That INSERT fires UpdateHook { action: Insert, table: MediaEditContext, .. } (the update-hook callback in wp_mobile_cache/src/lib.rs).
  3. self.notify_collections(media.id.0) fans out to live collections (update_media_membership).
  4. A MediaMetadataCollection built via create_media_metadata_collection_with_edit_context lists MediaEditContext in relevant_data_tables, so is_relevant_update returns true and observers reload — no explicit "created" channel needed.

The gap

  • The cache-aware create_media is never invoked by any test — repo-wide, create_media( resolves only to its own definition.
  • The update-hook machinery (start_listening_for_updates / did_update) is untested for any table.
  • Closest-but-insufficient: wp_mobile_integration_tests/tests/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.

Why this is an omission, not a decision

The post side already has this testwp_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 test

Port the post-side test to the media path (adds a direct cache-row assertion so a dropped upsert can't pass):

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, assertion (a) fails immediately; if notify_collections were dropped, assertion (b) fails while (a) passes — pinpointing which half broke.

Caveats: true integration test (live wp-env backend + credentials, like its siblings); must be #[serial] (mutates server state, ends with RestoreServer::db()); 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. Line numbers above are from 0.4.0 / 2bd38f23 (the WordPress iOS pin) — adjust to current trunk.

Context

Surfaced while reviewing the WordPress iOS Media V2 upload stack (wordpress-mobile/WordPress-iOS#25621, #25622). The iOS Media Library grid has no explicit "upload succeeded" channel — it refreshes purely via this cache hook — so the behavior tested here is exactly what keeps the grid current after an upload. The iOS-side tracking issue wordpress-mobile/WordPress-iOS#25667 is being closed in favor of this one, since the test belongs here rather than in the app.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions