Media: Remove manual cache busting

[swissspidy]

Task Description

In the editor we currently preload media items using the wp.apiFetch preloading middleware.

It is basically works as a cache that can’t be invalidated.

So if /path/to/load/media is preloaded, it's not possible to request /path/to/load/media again to fetch new data as it will always use the cached data.

This causes issues like #4109 and requires solutions like #4310.

One possible alternative solution mentioned by @Jhtin is to make the initial call look different from the subsequent calls. Then you preload the first call that looks different and then every subsequent call isn’t cached. Basically the opposite of what #4310 dos.

Essentially, what we need is a way to pre-populate media state upon page load, ideally one that is not so tightly coupled to WordPress, i.e. avoiding wp.apiFetch preloading middleware

[swissspidy]

Related: Gutenberg uses the concept of initialEdits:

https://github.com/WordPress/gutenberg/blob/1ad7a0ba48358ae2b6ec340f9472defa0d112910/packages/edit-post/src/editor.js#L172-L180

https://github.com/WordPress/gutenberg/blob/4c3300c148a6398f7ab03fb71ad5f635268c8605/packages/editor/src/store/actions.js#L31-L79

[mohdsayed]

Just for the reference of my findings:

1. Gutenberg also preloads multiple paths by using wp.apiFetch.createPreloadingMiddleware

https://github.com/WordPress/WordPress/blob/e175db4ae12056774d691f8b377cb61a4b9655aa/wp-includes/block-editor.php#L453

2. initialEdits is the fifth parameter of wp.editPost.initializeEditor
   https://github.com/WordPress/WordPress/blob/e175db4ae12056774d691f8b377cb61a4b9655aa/wp-admin/edit-form-blocks.php#L270-L281
   Which is null by default and holds an object containing title, content and excerpt when the post status is auto-draft initially.
   https://github.com/WordPress/WordPress/blob/e175db4ae12056774d691f8b377cb61a4b9655aa/wp-admin/edit-form-blocks.php#L80-L90

---

So looks like you want to remove this wp.apiFetch.createPreloadingMiddleware for two reasons.

1. To remove the workaround of passing cacheBust=true to invalidate cache.
2. In a non-WordPress environment when may be apiFetch is not used, we don't expect the integration to use some middleware to preload data like we currently do?

If both the assumptions are correct, I can only think of one possible solution for now.

With the proposed changes for API calls in #8966, all the apiFetch calls have been moved to wp-story-editor package however we are still in control of calling those API callbacks from the core editor.

https://github.com/google/web-stories-wp/blob/8a5f1915e903e93255aa87acca0f0cf165e7e1a2/packages/story-editor/src/app/api/apiProvider.js#L170-L174

What if we add one more parameter of apiPath and pass the preloaded data via window.webStoriesEditorSettings
https://github.com/google/web-stories-wp/blob/9d18d5f52f1b8a88f170df6be099b0a9d69e40c2/includes/templates/admin/edit-story.php#L130-L141

Something like this

window.webStoriesEditorSettings.preloadAPIResponse = {
  '/web-stories/v1/web-story/5/?_embed=..': {},
  '/web-stories/v1/media..': {},
  '/web-stories/v1/users/?per_page=100..': {}
    ...
}

and then we create some helper function as our own little middleware/HOF of these callbacks in story-editor which will return the preloaded response only if it is the initial load unlike wp.apiFetch.createPreloadingMiddleware which always returns the pre cached response for the same API path. Something like

actions.getMedia = useCallback(
  ({ apiPath, mediaType, searchTerm, pagingNum }) =>
    withInitialPreload( apiPath )( () => getMedia({ mediaType, searchTerm, pagingNum }, media) ) ,
  [media, getMedia]
);

withInitialPreload would somehow be aware of the initial load and return the preloaded response with Promise.resolve( preloadData ) or else would call the provided API Callback and return the fresh response.

[swissspidy]

I just had another look at this and it turns out this was fixed in Gutenberg a while ago! 🎉

See:

• [apiFetch] Fix preloading middleware referencing stale data WordPress/gutenberg#25550
• WordPress/gutenberg@57e2151

Preloading now only works a single time, the cache is invalidated after that.

That means we no longer need this cacheBust parameter and don't need to overcomplicate things with initialEdits etc.

[swissspidy]

I suppose we still need to keep this for WP < 5.7, which is where this fix was included (Gutenberg 9.1 maps to WP 5.7)

[spacedmonkey]

I suppose we still need to keep this for WP < 5.7, which is where this fix was included (Gutenberg 9.1 maps to WP 5.7)

As we are not going to support 5.7- soon, do we still need this ticket?

[swissspidy]

Well, we need the ticket to finally remove this code.

We've added this manual cache busting in the past because we had to support older WordPress versions.

Now that we are bumping the WP version requirement to WP 5.7+ we should be able to finally remove this manual cache busting, which is why I moved this to Ready for Sprint.

Relevant code sections:

web-stories-wp/packages/story-editor/src/app/media/local/useContextValueProvider.js

Line 190 in 8acb78a

{ mediaType: currentMediaType, cacheBust: true },

web-stories-wp/packages/story-editor/src/app/media/local/useContextValueProvider.js

Line 95 in 8acb78a

cacheBust: cacheBust,

web-stories-wp/packages/story-editor/src/app/media/local/useContextValueProvider.js

Line 110 in 8acb78a

cacheBust: cacheBust,

web-stories-wp/packages/story-editor/src/app/media/local/useContextValueProvider.js

Line 190 in 8acb78a

{ mediaType: currentMediaType, cacheBust: true },

web-stories-wp/packages/story-editor/src/app/media/local/test/useContextValueProvider.js

Lines 201 to 256 in 8acb78a

	it('resetWithFetch calls getMedia with cacheBust:true and then fetchMediaSuccess', async () => {
	// Set up and make initial call to useContextValueProvider (which calls
	// getMedia and fetchMediaSuccess once). Sets an empty media state.
	const getMedia = jest.fn(() =>
	Promise.resolve({
	data: [],
	headers: GET_MEDIA_RESPONSE_HEADER,
	})
	);
	const apiState = {
	actions: {
	getMedia,
	},
	};
	const configState = {
	api: {},
	allowedMimeTypes: {
	image: [],
	vector: [],
	video: [],
	caption: [],
	audio: [],
	},
	capabilities: { hasUploadMediaAction: true },
	};
	const { result } = renderAllProviders({
	reducerState,
	reducerActions,
	configState,
	apiState,
	});
	// This promise will only complete when the "done()" callback is called
	// (see reducerActions.fetchMediaSuccess mock implementation in Promise).
	await new Promise((done) => {
	getMedia.mockImplementation(() => {
	return Promise.resolve({
	data: GET_MEDIA_RESPONSE_BODY,
	headers: GET_MEDIA_RESPONSE_HEADER,
	});
	});
	reducerActions.fetchMediaSuccess.mockImplementation(() => {
	done();
	});
	// Act:
	result.current.actions.resetWithFetch();
	});
	// Assert after fetchMediaSuccess callback is called:
	expect(getMedia).toHaveBeenNthCalledWith(2, {
	mediaType: '',
	searchTerm: '',
	pagingNum: 1,
	cacheBust: true,
	});

web-stories-wp/packages/wp-story-editor/src/api/media.js

Lines 32 to 60 in 8acb78a

	// Important: Keep in sync with REST API preloading definition.
	export function getMedia(
	config,
	{ mediaType, searchTerm, pagingNum, cacheBust }
	) {
	let path = addQueryArgs(config.api.media, {
	context: 'view',
	per_page: 50,
	page: pagingNum,
	_web_stories_envelope: true,
	_fields: MEDIA_FIELDS,
	});
	if (mediaType) {
	path = addQueryArgs(path, { media_type: mediaType });
	}
	if (searchTerm) {
	path = addQueryArgs(path, { search: searchTerm });
	}
	// cacheBusting is due to the preloading logic preloading and caching
	// some requests. (see preload_paths in Dashboard.php)
	// Adding cache_bust forces the path to look different from the preloaded
	// paths and hence skipping the cache. (cache_bust itself doesn't do
	// anything)
	if (cacheBust) {
	path = addQueryArgs(path, { cache_bust: true });
	}

web-stories-wp/packages/wp-story-editor/src/api/test/media.js

Lines 42 to 64 in 8acb78a

	it('getMedia with cacheBust:true should call api with &cache_bust=true', () => {
	apiFetch.mockReturnValue(
	Promise.resolve({
	body: GET_MEDIA_RESPONSE_BODY,
	headers: GET_MEDIA_RESPONSE_HEADER,
	})
	);
	const { getMedia } = bindToCallbacks(apiCallbacks, {
	api: { media: MEDIA_PATH },
	});
	getMedia({
	mediaType: '',
	searchTerm: '',
	pagingNum: 1,
	cacheBust: true,
	});
	expect(apiFetch).toHaveBeenCalledWith(
	expect.objectContaining({
	path: expect.stringMatching('&cache_bust=true'),
	})
	);
	});

[felipebochehin87]

Tested using PR #11759. Dragged and dropped one and multiple files, uploaded one and multiple files. No refresh needed to have files displayed in the media gallery.

https://images.zenhubusercontent.com/235435637/1ed58d3a-8a34-4268-8248-8736b6d8310d/2022_06_26_18_55_37.mp4