From 12d81cdc51aceec7811db3c40262dd21ab809c47 Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Thu, 28 Jan 2021 12:42:18 +1100 Subject: [PATCH] Replace batch processing store with a simpler API (#28210) - Replaces the batch-processing data store with a stripped back API that doesn't use any persistence. - Adds __experimentalBatch() to core-data. - Changes edit-widgets to use __experimentalBatch(). - Fixes batch processing race condition by using batch.add( thunk ). --- .../developers/data/data-core.md | 3 + packages/core-data/README.md | 3 + packages/core-data/src/actions.js | 146 ++++++++++-- packages/core-data/src/batch/create-batch.js | 187 +++++++++++++++ .../core-data/src/batch/default-processor.js | 45 ++++ packages/core-data/src/batch/index.js | 2 + .../core-data/src/batch/test/create-batch.js | 131 ++++++++++ .../src/batch/test/default-processor.js | 91 +++++++ packages/core-data/src/controls.js | 14 ++ packages/core-data/src/test/actions.js | 84 +++++++ packages/edit-widgets/src/store/actions.js | 72 +++--- .../src/store/batch-processing/actions.js | 174 -------------- .../src/store/batch-processing/constants.js | 11 - .../src/store/batch-processing/controls.js | 132 ----------- .../src/store/batch-processing/index.js | 31 --- .../src/store/batch-processing/reducer.js | 223 ------------------ .../src/store/batch-processing/selectors.js | 17 -- .../src/store/batch-processing/test/test.js | 83 ------- .../edit-widgets/src/store/batch-support.js | 119 ---------- packages/edit-widgets/src/store/index.js | 1 - 20 files changed, 714 insertions(+), 855 deletions(-) create mode 100644 packages/core-data/src/batch/create-batch.js create mode 100644 packages/core-data/src/batch/default-processor.js create mode 100644 packages/core-data/src/batch/index.js create mode 100644 packages/core-data/src/batch/test/create-batch.js create mode 100644 packages/core-data/src/batch/test/default-processor.js delete mode 100644 packages/edit-widgets/src/store/batch-processing/actions.js delete mode 100644 packages/edit-widgets/src/store/batch-processing/constants.js delete mode 100644 packages/edit-widgets/src/store/batch-processing/controls.js delete mode 100644 packages/edit-widgets/src/store/batch-processing/index.js delete mode 100644 packages/edit-widgets/src/store/batch-processing/reducer.js delete mode 100644 packages/edit-widgets/src/store/batch-processing/selectors.js delete mode 100644 packages/edit-widgets/src/store/batch-processing/test/test.js delete mode 100644 packages/edit-widgets/src/store/batch-support.js diff --git a/docs/designers-developers/developers/data/data-core.md b/docs/designers-developers/developers/data/data-core.md index e079ad3b9ffc0..a901c5080b83f 100644 --- a/docs/designers-developers/developers/data/data-core.md +++ b/docs/designers-developers/developers/data/data-core.md @@ -531,6 +531,8 @@ _Parameters_ - _name_ `string`: Name of the deleted entity. - _recordId_ `string`: Record ID of the deleted entity. - _query_ `?Object`: Special query parameters for the DELETE API call. +- _options_ `[Object]`: Delete options. +- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a control descriptor. # **editEntityRecord** @@ -697,6 +699,7 @@ _Parameters_ - _record_ `Object`: Record to be saved. - _options_ `Object`: Saving options. - _options.isAutosave_ `[boolean]`: Whether this is an autosave. +- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a control descriptor. # **undo** diff --git a/packages/core-data/README.md b/packages/core-data/README.md index 3829c1509b8c4..793be65307457 100644 --- a/packages/core-data/README.md +++ b/packages/core-data/README.md @@ -64,6 +64,8 @@ _Parameters_ - _name_ `string`: Name of the deleted entity. - _recordId_ `string`: Record ID of the deleted entity. - _query_ `?Object`: Special query parameters for the DELETE API call. +- _options_ `[Object]`: Delete options. +- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a control descriptor. # **editEntityRecord** @@ -230,6 +232,7 @@ _Parameters_ - _record_ `Object`: Record to be saved. - _options_ `Object`: Saving options. - _options.isAutosave_ `[boolean]`: Whether this is an autosave. +- _options.\_\_unstableFetch_ `[Function]`: Internal use only. Function to call instead of `apiFetch()`. Must return a control descriptor. # **undo** diff --git a/packages/core-data/src/actions.js b/packages/core-data/src/actions.js index b3d9f4bd23a34..c7454f51e28f7 100644 --- a/packages/core-data/src/actions.js +++ b/packages/core-data/src/actions.js @@ -8,7 +8,7 @@ import { v4 as uuid } from 'uuid'; * WordPress dependencies */ import { controls } from '@wordpress/data'; -import { apiFetch } from '@wordpress/data-controls'; +import { apiFetch, __unstableAwaitPromise } from '@wordpress/data-controls'; import { addQueryArgs } from '@wordpress/url'; /** @@ -20,6 +20,8 @@ import { __unstableAcquireStoreLock, __unstableReleaseStoreLock, } from './locks'; +import { createBatch } from './batch'; +import { getDispatch } from './controls'; /** * Returns an action object used in signalling that authors have been received. @@ -154,12 +156,23 @@ export function receiveEmbedPreview( url, preview ) { /** * Action triggered to delete an entity record. * - * @param {string} kind Kind of the deleted entity. - * @param {string} name Name of the deleted entity. - * @param {string} recordId Record ID of the deleted entity. - * @param {?Object} query Special query parameters for the DELETE API call. + * @param {string} kind Kind of the deleted entity. + * @param {string} name Name of the deleted entity. + * @param {string} recordId Record ID of the deleted entity. + * @param {?Object} query Special query parameters for the + * DELETE API call. + * @param {Object} [options] Delete options. + * @param {Function} [options.__unstableFetch] Internal use only. Function to + * call instead of `apiFetch()`. + * Must return a control descriptor. */ -export function* deleteEntityRecord( kind, name, recordId, query ) { +export function* deleteEntityRecord( + kind, + name, + recordId, + query, + { __unstableFetch = null } = {} +) { const entities = yield getKindEntities( kind ); const entity = find( entities, { kind, name } ); let error; @@ -188,10 +201,17 @@ export function* deleteEntityRecord( kind, name, recordId, query ) { path = addQueryArgs( path, query ); } - deletedRecord = yield apiFetch( { + const options = { path, method: 'DELETE', - } ); + }; + if ( __unstableFetch ) { + deletedRecord = yield __unstableAwaitPromise( + __unstableFetch( options ) + ); + } else { + deletedRecord = yield apiFetch( options ); + } yield removeItems( kind, name, recordId, true ); } catch ( _error ) { @@ -329,17 +349,21 @@ export function __unstableCreateUndoLevel() { /** * Action triggered to save an entity record. * - * @param {string} kind Kind of the received entity. - * @param {string} name Name of the received entity. - * @param {Object} record Record to be saved. - * @param {Object} options Saving options. - * @param {boolean} [options.isAutosave=false] Whether this is an autosave. + * @param {string} kind Kind of the received entity. + * @param {string} name Name of the received entity. + * @param {Object} record Record to be saved. + * @param {Object} options Saving options. + * @param {boolean} [options.isAutosave=false] Whether this is an autosave. + * @param {Function} [options.__unstableFetch] Internal use only. Function to + * call instead of `apiFetch()`. + * Must return a control + * descriptor. */ export function* saveEntityRecord( kind, name, record, - { isAutosave = false } = { isAutosave: false } + { isAutosave = false, __unstableFetch = null } = {} ) { const entities = yield getKindEntities( kind ); const entity = find( entities, { kind, name } ); @@ -441,11 +465,18 @@ export function* saveEntityRecord( : data.status, } ); - updatedRecord = yield apiFetch( { + const options = { path: `${ path }/autosaves`, method: 'POST', data, - } ); + }; + if ( __unstableFetch ) { + updatedRecord = yield __unstableAwaitPromise( + __unstableFetch( options ) + ); + } else { + updatedRecord = yield apiFetch( options ); + } // An autosave may be processed by the server as a regular save // when its update is requested by the author and the post had // draft or auto-draft status. @@ -510,12 +541,18 @@ export function* saveEntityRecord( ), }; } - - updatedRecord = yield apiFetch( { + const options = { path, method: recordId ? 'PUT' : 'POST', data: edits, - } ); + }; + if ( __unstableFetch ) { + updatedRecord = yield __unstableAwaitPromise( + __unstableFetch( options ) + ); + } else { + updatedRecord = yield apiFetch( options ); + } yield receiveEntityRecords( kind, name, @@ -543,6 +580,75 @@ export function* saveEntityRecord( } } +/** + * Runs multiple core-data actions at the same time using one API request. + * + * Example: + * + * ``` + * const [ savedRecord, updatedRecord, deletedRecord ] = + * await dispatch( 'core' ).__experimentalBatch( [ + * ( { saveEntityRecord } ) => saveEntityRecord( 'root', 'widget', widget ), + * ( { saveEditedEntityRecord } ) => saveEntityRecord( 'root', 'widget', 123 ), + * ( { deleteEntityRecord } ) => deleteEntityRecord( 'root', 'widget', 123, null ), + * ] ); + * ``` + * + * @param {Array} requests Array of functions which are invoked simultaneously. + * Each function is passed an object containing + * `saveEntityRecord`, `saveEditedEntityRecord`, and + * `deleteEntityRecord`. + * + * @return {Promise} A promise that resolves to an array containing the return + * values of each function given in `requests`. + */ +export function* __experimentalBatch( requests ) { + const batch = createBatch(); + const dispatch = yield getDispatch(); + const api = { + saveEntityRecord( kind, name, record, options ) { + return batch.add( ( add ) => + dispatch( 'core' ).saveEntityRecord( kind, name, record, { + ...options, + __unstableFetch: add, + } ) + ); + }, + saveEditedEntityRecord( kind, name, recordId, options ) { + return batch.add( ( add ) => + dispatch( 'core' ).saveEditedEntityRecord( + kind, + name, + recordId, + { + ...options, + __unstableFetch: add, + } + ) + ); + }, + deleteEntityRecord( kind, name, recordId, query, options ) { + return batch.add( ( add ) => + dispatch( 'core' ).deleteEntityRecord( + kind, + name, + recordId, + query, + { + ...options, + __unstableFetch: add, + } + ) + ); + }, + }; + const resultPromises = requests.map( ( request ) => request( api ) ); + const [ , ...results ] = yield __unstableAwaitPromise( + Promise.all( [ batch.run(), ...resultPromises ] ) + ); + return results; +} + /** * Action triggered to save an entity record's edits. * @@ -571,7 +677,7 @@ export function* saveEditedEntityRecord( kind, name, recordId, options ) { recordId ); const record = { id: recordId, ...edits }; - yield* saveEntityRecord( kind, name, record, options ); + return yield* saveEntityRecord( kind, name, record, options ); } /** diff --git a/packages/core-data/src/batch/create-batch.js b/packages/core-data/src/batch/create-batch.js new file mode 100644 index 0000000000000..6009aee86f25c --- /dev/null +++ b/packages/core-data/src/batch/create-batch.js @@ -0,0 +1,187 @@ +/** + * External dependencies + */ +import { isFunction, zip } from 'lodash'; + +/** + * Internal dependencies + */ +import defaultProcessor from './default-processor'; + +/** + * Creates a batch, which can be used to combine multiple API requests into one + * API request using the WordPress batch processing API (/v1/batch). + * + * ``` + * const batch = createBatch(); + * const dunePromise = batch.add( { + * path: '/v1/books', + * method: 'POST', + * data: { title: 'Dune' } + * } ); + * const lotrPromise = batch.add( { + * path: '/v1/books', + * method: 'POST', + * data: { title: 'Lord of the Rings' } + * } ); + * const isSuccess = await batch.run(); // Sends one POST to /v1/batch. + * if ( isSuccess ) { + * console.log( + * 'Saved two books:', + * await dunePromise, + * await lotrPromise + * ); + * } + * ``` + * + * @param {Function} [processor] Processor function. Can be used to replace the + * default functionality which is to send an API + * request to /v1/batch. Is given an array of + * inputs and must return a promise that + * resolves to an array of objects containing + * either `output` or `error`. + */ +export default function createBatch( processor = defaultProcessor ) { + let lastId = 0; + let queue = []; + const pending = new ObservableSet(); + + return { + /** + * Adds an input to the batch and returns a promise that is resolved or + * rejected when the input is processed by `batch.run()`. + * + * You may also pass a thunk which allows inputs to be added + * asychronously. + * + * ``` + * // Both are allowed: + * batch.add( { path: '/v1/books', ... } ); + * batch.add( ( add ) => add( { path: '/v1/books', ... } ) ); + * ``` + * + * If a thunk is passed, `batch.run()` will pause until either: + * + * - The thunk calls its `add` argument, or; + * - The thunk returns a promise and that promise resolves, or; + * - The thunk returns a non-promise. + * + * @param {any|Function} inputOrThunk Input to add or thunk to execute. + + * @return {Promise|any} If given an input, returns a promise that + * is resolved or rejected when the batch is + * processed. If given a thunk, returns the return + * value of that thunk. + */ + add( inputOrThunk ) { + const id = ++lastId; + pending.add( id ); + + const add = ( input ) => + new Promise( ( resolve, reject ) => { + queue.push( { + input, + resolve, + reject, + } ); + pending.delete( id ); + } ); + + if ( isFunction( inputOrThunk ) ) { + return Promise.resolve( inputOrThunk( add ) ).finally( () => { + pending.delete( id ); + } ); + } + + return add( inputOrThunk ); + }, + + /** + * Runs the batch. This calls `batchProcessor` and resolves or rejects + * all promises returned by `add()`. + * + * @return {Promise} A promise that resolves to a boolean that is true + * if the processor returned no errors. + */ + async run() { + if ( pending.size ) { + await new Promise( ( resolve ) => { + const unsubscribe = pending.subscribe( () => { + if ( ! pending.size ) { + unsubscribe(); + resolve(); + } + } ); + } ); + } + + let results; + + try { + results = await processor( + queue.map( ( { input } ) => input ) + ); + + if ( results.length !== queue.length ) { + throw new Error( + 'run: Array returned by processor must be same size as input array.' + ); + } + } catch ( error ) { + for ( const { reject } of queue ) { + reject( error ); + } + + throw error; + } + + let isSuccess = true; + + for ( const [ result, { resolve, reject } ] of zip( + results, + queue + ) ) { + if ( result?.error ) { + reject( result.error ); + isSuccess = false; + } else { + resolve( result?.output ?? result ); + } + } + + queue = []; + + return isSuccess; + }, + }; +} + +class ObservableSet { + constructor( ...args ) { + this.set = new Set( ...args ); + this.subscribers = new Set(); + } + + get size() { + return this.set.size; + } + + add( ...args ) { + this.set.add( ...args ); + this.subscribers.forEach( ( subscriber ) => subscriber() ); + return this; + } + + delete( ...args ) { + const isSuccess = this.set.delete( ...args ); + this.subscribers.forEach( ( subscriber ) => subscriber() ); + return isSuccess; + } + + subscribe( subscriber ) { + this.subscribers.add( subscriber ); + return () => { + this.subscribers.delete( subscriber ); + }; + } +} diff --git a/packages/core-data/src/batch/default-processor.js b/packages/core-data/src/batch/default-processor.js new file mode 100644 index 0000000000000..1d98d5af1fb8f --- /dev/null +++ b/packages/core-data/src/batch/default-processor.js @@ -0,0 +1,45 @@ +/** + * WordPress dependencies + */ +import apiFetch from '@wordpress/api-fetch'; + +/** + * Default batch processor. Sends its input requests to /v1/batch. + * + * @param {Array} requests List of API requests to perform at once. + * + * @return {Promise} Promise that resolves to a list of objects containing + * either `output` (if that request was succesful) or `error` + * (if not ). + */ +export default async function defaultProcessor( requests ) { + const batchResponse = await apiFetch( { + path: '/v1/batch', + method: 'POST', + data: { + validation: 'require-all-validate', + requests: requests.map( ( request ) => ( { + path: request.path, + body: request.data, // Rename 'data' to 'body'. + method: request.method, + headers: request.headers, + } ) ), + }, + } ); + + if ( batchResponse.failed ) { + return batchResponse.responses.map( ( response ) => ( { + error: response?.body, + } ) ); + } + + return batchResponse.responses.map( ( response ) => { + const result = {}; + if ( response.status >= 200 && response.status < 300 ) { + result.output = response.body; + } else { + result.error = response.body; + } + return result; + } ); +} diff --git a/packages/core-data/src/batch/index.js b/packages/core-data/src/batch/index.js new file mode 100644 index 0000000000000..88edeff074dc7 --- /dev/null +++ b/packages/core-data/src/batch/index.js @@ -0,0 +1,2 @@ +export { default as createBatch } from './create-batch'; +export { default as defaultProcessor } from './default-processor'; diff --git a/packages/core-data/src/batch/test/create-batch.js b/packages/core-data/src/batch/test/create-batch.js new file mode 100644 index 0000000000000..8cefa9983d55d --- /dev/null +++ b/packages/core-data/src/batch/test/create-batch.js @@ -0,0 +1,131 @@ +/** + * Internal dependencies + */ +import createBatch from '../create-batch'; + +describe( 'createBatch', () => { + test( 'running an empty batch', async () => { + const processor = async ( inputs ) => inputs; + const batch = createBatch( processor ); + expect( await batch.run() ).toBe( true ); + } ); + + test( 'running resolves promises when processor returns output', async () => { + const processor = async ( inputs ) => + inputs.map( ( input ) => ( { + output: input, + } ) ); + const batch = createBatch( processor ); + let promise1Value, promise2Value; + batch.add( 1 ).then( ( value ) => ( promise1Value = value ) ); + batch.add( 2 ).then( ( value ) => ( promise2Value = value ) ); + expect( await batch.run() ).toBe( true ); + expect( promise1Value ).toBe( 1 ); + expect( promise2Value ).toBe( 2 ); + } ); + + test( 'running resolves promises when processor returns non-objects', async () => { + const processor = async ( inputs ) => inputs.map( ( input ) => input ); + const batch = createBatch( processor ); + let promise1Value, promise2Value; + batch.add( 1 ).then( ( value ) => ( promise1Value = value ) ); + batch.add( 2 ).then( ( value ) => ( promise2Value = value ) ); + expect( await batch.run() ).toBe( true ); + expect( promise1Value ).toBe( 1 ); + expect( promise2Value ).toBe( 2 ); + } ); + + test( 'running waits for all thunks to finish', async () => { + const processor = async ( inputs ) => + inputs.map( ( input ) => ( { + output: input, + } ) ); + const batch = createBatch( processor ); + let promise1Value, promise2Value; + batch.add( async ( add ) => { + await Promise.resolve(); // Simulates a delay. + return add( 1 ).then( ( value ) => ( promise1Value = value ) ); + } ); + batch.add( async ( add ) => { + await Promise.resolve(); // Simulates a delay. + return add( 2 ).then( ( value ) => ( promise2Value = value ) ); + } ); + expect( await batch.run() ).toBe( true ); + expect( promise1Value ).toBe( 1 ); + expect( promise2Value ).toBe( 2 ); + } ); + + test( "running doesn't time out when a thunk doesn't call add()", async () => { + const processor = async ( inputs ) => + inputs.map( ( input ) => ( { + output: input, + } ) ); + const batch = createBatch( processor ); + batch.add( async () => { + await Promise.resolve(); // Simulates a delay. + } ); + batch.add( () => { + // No delay. + } ); + expect( await batch.run() ).toBe( true ); + } ); + + test( 'running resets the batch when finished', async () => { + const processor = jest.fn( async ( inputs ) => + inputs.map( ( input ) => ( { + output: input, + } ) ) + ); + const batch = createBatch( processor ); + let promise1Value; + batch.add( 1 ).then( ( value ) => ( promise1Value = value ) ); + expect( await batch.run() ).toBe( true ); + expect( promise1Value ).toBe( 1 ); + expect( processor ).toHaveBeenCalledTimes( 1 ); + expect( processor ).toHaveBeenCalledWith( [ 1 ] ); + let promise2Value; + batch.add( 2 ).then( ( value ) => ( promise2Value = value ) ); + expect( await batch.run() ).toBe( true ); + expect( promise2Value ).toBe( 2 ); + expect( processor ).toHaveBeenCalledTimes( 2 ); + expect( processor ).toHaveBeenCalledWith( [ 2 ] ); + } ); + + test( 'running rejects promises when processor returns errors', async () => { + const processor = async ( inputs ) => + inputs.map( ( input ) => ( { + error: input, + } ) ); + const batch = createBatch( processor ); + let promise1Error, promise2Error; + batch.add( 1 ).catch( ( error ) => ( promise1Error = error ) ); + batch.add( 2 ).catch( ( error ) => ( promise2Error = error ) ); + expect( await batch.run() ).toBe( false ); + expect( promise1Error ).toBe( 1 ); + expect( promise2Error ).toBe( 2 ); + } ); + + test( 'running rejects promises and rethrows when processor throws', async () => { + const processor = async () => { + throw 'Jikes!'; + }; + const batch = createBatch( processor ); + let promise1Error, promise2Error; + batch.add( 1 ).catch( ( error ) => ( promise1Error = error ) ); + batch.add( 2 ).catch( ( error ) => ( promise2Error = error ) ); + await expect( batch.run() ).rejects.toBe( 'Jikes!' ); + expect( promise1Error ).toBe( 'Jikes!' ); + expect( promise2Error ).toBe( 'Jikes!' ); + } ); + + test( 'running rejects promises and throws when processor returns wrong length', async () => { + const processor = async () => [ 1 ]; + const batch = createBatch( processor ); + let promise1Error, promise2Error; + batch.add( 1 ).catch( ( error ) => ( promise1Error = error ) ); + batch.add( 2 ).catch( ( error ) => ( promise2Error = error ) ); + await expect( batch.run() ).rejects.toBeInstanceOf( Error ); + expect( promise1Error ).toBeInstanceOf( Error ); + expect( promise2Error ).toBeInstanceOf( Error ); + } ); +} ); diff --git a/packages/core-data/src/batch/test/default-processor.js b/packages/core-data/src/batch/test/default-processor.js new file mode 100644 index 0000000000000..888ead29e883d --- /dev/null +++ b/packages/core-data/src/batch/test/default-processor.js @@ -0,0 +1,91 @@ +/** + * WordPress dependencies + */ +import apiFetch from '@wordpress/api-fetch'; + +/** + * Internal dependencies + */ +import defaultProcessor from '../default-processor'; + +jest.mock( '@wordpress/api-fetch' ); + +describe( 'defaultProcessor', () => { + const requests = [ + { + path: '/v1/cricketers', + data: 'Lyon', + method: 'POST', + headers: {}, + }, + { + path: '/v1/cricketers/1', + data: 'Starc', + method: 'POST', + headers: {}, + }, + ]; + + const expectedFetchOptions = { + path: '/v1/batch', + method: 'POST', + data: { + validation: 'require-all-validate', + requests: [ + { + path: '/v1/cricketers', + body: 'Lyon', + method: 'POST', + headers: {}, + }, + { + path: '/v1/cricketers/1', + body: 'Starc', + method: 'POST', + headers: {}, + }, + ], + }, + }; + + it( 'handles a successful request', async () => { + apiFetch.mockImplementation( async () => ( { + failed: false, + responses: [ + { + status: 200, + body: 'Lyon', + }, + { + status: 400, + body: 'Error!', + }, + ], + } ) ); + const results = await defaultProcessor( requests ); + expect( apiFetch ).toHaveBeenCalledWith( expectedFetchOptions ); + expect( results ).toEqual( [ + { output: 'Lyon' }, + { error: 'Error!' }, + ] ); + } ); + + it( 'handles a failed request', async () => { + apiFetch.mockImplementation( async () => ( { + failed: true, + responses: [ + null, + { + status: 400, + body: 'Error!', + }, + ], + } ) ); + const results = await defaultProcessor( requests ); + expect( apiFetch ).toHaveBeenCalledWith( expectedFetchOptions ); + expect( results ).toEqual( [ + { error: undefined }, + { error: 'Error!' }, + ] ); + } ); +} ); diff --git a/packages/core-data/src/controls.js b/packages/core-data/src/controls.js index e1e069a8e0306..00f8ca36641c1 100644 --- a/packages/core-data/src/controls.js +++ b/packages/core-data/src/controls.js @@ -1,9 +1,21 @@ +/** + * WordPress dependencies + */ +import { createRegistryControl } from '@wordpress/data'; + export function regularFetch( url ) { return { type: 'REGULAR_FETCH', url, }; } + +export function getDispatch() { + return { + type: 'GET_DISPATCH', + }; +} + const controls = { async REGULAR_FETCH( { url } ) { const { data } = await window @@ -12,6 +24,8 @@ const controls = { return data; }, + + GET_DISPATCH: createRegistryControl( ( { dispatch } ) => () => dispatch ), }; export default controls; diff --git a/packages/core-data/src/test/actions.js b/packages/core-data/src/test/actions.js index cf0bc142348ca..6dfec991e2a03 100644 --- a/packages/core-data/src/test/actions.js +++ b/packages/core-data/src/test/actions.js @@ -14,6 +14,7 @@ import { receiveUserPermission, receiveAutosaves, receiveCurrentUser, + __experimentalBatch, } from '../actions'; jest.mock( '../locks/actions', () => ( { @@ -29,6 +30,15 @@ jest.mock( '../locks/actions', () => ( { ] ), } ) ); +jest.mock( '../batch', () => { + const { createBatch } = jest.requireActual( '../batch' ); + return { + createBatch() { + return createBatch( ( inputs ) => Promise.resolve( inputs ) ); + }, + }; +} ); + describe( 'editEntityRecord', () => { it( 'throws when the edited entity does not have a loaded config.', () => { const entity = { kind: 'someKind', name: 'someName', id: 'someId' }; @@ -292,3 +302,77 @@ describe( 'receiveCurrentUser', () => { } ); } ); } ); + +describe( '__experimentalBatch', () => { + it( 'batches multiple actions together', async () => { + const generator = __experimentalBatch( + [ + ( { saveEntityRecord: _saveEntityRecord } ) => + _saveEntityRecord( 'root', 'widget', {} ), + ( { saveEditedEntityRecord: _saveEditedEntityRecord } ) => + _saveEditedEntityRecord( 'root', 'widget', 123 ), + ( { deleteEntityRecord: _deleteEntityRecord } ) => + _deleteEntityRecord( 'root', 'widget', 123, {} ), + ], + { __unstableProcessor: ( inputs ) => Promise.resolve( inputs ) } + ); + // Run generator up to `yield getDispatch()`. + const { value: getDispatchControl } = generator.next(); + expect( getDispatchControl ).toEqual( { type: 'GET_DISPATCH' } ); + const actions = { + saveEntityRecord: jest.fn( + ( kind, name, record, { __unstableFetch } ) => { + __unstableFetch( {} ); + return { id: 123, created: true }; + } + ), + saveEditedEntityRecord: jest.fn( + ( kind, name, recordId, { __unstableFetch } ) => { + __unstableFetch( {} ); + return { id: 123, updated: true }; + } + ), + deleteEntityRecord: jest.fn( + ( kind, name, recordId, query, { __unstableFetch } ) => { + __unstableFetch( {} ); + return { id: 123, deleted: true }; + } + ), + }; + const dispatch = () => actions; + // Run generator up to `yield __unstableAwaitPromise( ... )`. + const { value: awaitPromiseControl } = generator.next( dispatch ); + expect( actions.saveEntityRecord ).toHaveBeenCalledWith( + 'root', + 'widget', + {}, + { __unstableFetch: expect.any( Function ) } + ); + expect( actions.saveEditedEntityRecord ).toHaveBeenCalledWith( + 'root', + 'widget', + 123, + { __unstableFetch: expect.any( Function ) } + ); + expect( actions.deleteEntityRecord ).toHaveBeenCalledWith( + 'root', + 'widget', + 123, + {}, + { __unstableFetch: expect.any( Function ) } + ); + expect( awaitPromiseControl ).toEqual( { + type: 'AWAIT_PROMISE', + promise: expect.any( Promise ), + } ); + // Run generator to the end. + const { value: results } = generator.next( + await awaitPromiseControl.promise + ); + expect( results ).toEqual( [ + { id: 123, created: true }, + { id: 123, updated: true }, + { id: 123, deleted: true }, + ] ); + } ); +} ); diff --git a/packages/edit-widgets/src/store/actions.js b/packages/edit-widgets/src/store/actions.js index 8c236f0efb201..05df8ac82c471 100644 --- a/packages/edit-widgets/src/store/actions.js +++ b/packages/edit-widgets/src/store/actions.js @@ -2,13 +2,11 @@ * WordPress dependencies */ import { __, sprintf } from '@wordpress/i18n'; -import { dispatch as dataDispatch } from '@wordpress/data'; import { store as noticesStore } from '@wordpress/notices'; import { store as interfaceStore } from '@wordpress/interface'; /** * Internal dependencies */ -import { STATE_SUCCESS } from './batch-processing/constants'; import { dispatch, select } from './controls'; import { transformBlockToWidget } from './transformers'; import { @@ -118,6 +116,7 @@ export function* saveWidgetArea( widgetAreaId ) { ); const batchMeta = []; + const batchTasks = []; const sidebarWidgetsIds = []; for ( let i = 0; i < widgetsBlocks.length; i++ ) { const block = widgetsBlocks[ i ]; @@ -153,18 +152,16 @@ export function* saveWidgetArea( widgetAreaId ) { continue; } - dataDispatch( 'core' ).saveEditedEntityRecord( - 'root', - 'widget', - widgetId, - widget + batchTasks.push( ( { saveEditedEntityRecord } ) => + saveEditedEntityRecord( 'root', 'widget', widgetId ) ); } else { - // This is a new widget instance. - dataDispatch( 'core' ).saveEntityRecord( 'root', 'widget', { - ...widget, - sidebar: widgetAreaId, - } ); + batchTasks.push( ( { saveEntityRecord } ) => + saveEntityRecord( 'root', 'widget', { + ...widget, + sidebar: widgetAreaId, + } ) + ); } batchMeta.push( { @@ -174,35 +171,31 @@ export function* saveWidgetArea( widgetAreaId ) { } ); } - // HACK: Await any promise here so that rungen passes execution back to - // `saveEntityRecord` above. This prevents `processBatch` from being called - // here before `addToBatch` is called by `saveEntityRecord`. - // See https://github.com/WordPress/gutenberg/issues/27173. - yield { - type: 'AWAIT_PROMISE', - promise: Promise.resolve(), - }; - - const batch = yield dispatch( - 'core/__experimental-batch-processing', - 'processBatch', - 'WIDGETS_API_FETCH', - 'default' - ); + const records = yield dispatch( 'core', '__experimentalBatch', batchTasks ); - if ( batch.state !== STATE_SUCCESS ) { - const errors = batch.sortedItemIds.map( ( id ) => batch.errors[ id ] ); - const failedWidgetNames = []; + const failedWidgetNames = []; - for ( let i = 0; i < errors.length; i++ ) { - if ( ! errors[ i ] ) { - continue; - } + for ( let i = 0; i < records.length; i++ ) { + const widget = records[ i ]; + const { block, position } = batchMeta[ i ]; - const { block } = batchMeta[ i ]; + const error = yield select( + 'core', + 'getLastEntitySaveError', + 'root', + 'widget', + widget.id + ); + if ( error ) { failedWidgetNames.push( block.attributes?.name || block?.name ); } + if ( ! sidebarWidgetsIds[ position ] ) { + sidebarWidgetsIds[ position ] = widget.id; + } + } + + if ( failedWidgetNames.length ) { throw new Error( sprintf( /* translators: %s: List of widget names */ @@ -212,15 +205,6 @@ export function* saveWidgetArea( widgetAreaId ) { ); } - for ( let i = 0; i < batch.sortedItemIds.length; i++ ) { - const itemId = batch.sortedItemIds[ i ]; - const widget = batch.results[ itemId ]; - const { position } = batchMeta[ i ]; - if ( ! sidebarWidgetsIds[ position ] ) { - sidebarWidgetsIds[ position ] = widget.id; - } - } - yield dispatch( 'core', 'editEntityRecord', diff --git a/packages/edit-widgets/src/store/batch-processing/actions.js b/packages/edit-widgets/src/store/batch-processing/actions.js deleted file mode 100644 index a21a7433e44f0..0000000000000 --- a/packages/edit-widgets/src/store/batch-processing/actions.js +++ /dev/null @@ -1,174 +0,0 @@ -/** - * External dependencies - */ -import { v4 as uuid } from 'uuid'; - -/** - * Internal dependencies - */ -import { - select, - dispatch, - enqueueItemAndAutocommit as enqueueAutocommitControl, - processTransaction, -} from './controls'; -import { STATE_ERROR, STATE_SUCCESS } from './constants'; - -export const enqueueItemAndAutocommit = function* ( queue, context, item ) { - return yield enqueueAutocommitControl( queue, context, item ); -}; - -export const enqueueItemAndWaitForResults = function* ( queue, context, item ) { - const { itemId } = yield dispatch( 'enqueueItem', queue, context, item ); - const { promise } = yield* getOrSetupPromise( queue, context ); - - return { - wait: promise.then( ( batch ) => { - if ( batch.state === STATE_ERROR ) { - throw batch.errors[ itemId ]; - } - - return batch.results[ itemId ]; - } ), - }; -}; - -export const enqueueItem = function ( queue, context, item ) { - const itemId = uuid(); - return { - type: 'ENQUEUE_ITEM', - queue, - context, - item, - itemId, - }; -}; - -const setupPromise = function ( queue, context ) { - const action = { - type: 'SETUP_PROMISE', - queue, - context, - }; - - action.promise = new Promise( ( resolve, reject ) => { - action.resolve = resolve; - action.reject = reject; - } ); - - return action; -}; - -const getOrSetupPromise = function* ( queue, context ) { - const promise = yield select( 'getPromise', queue, context ); - - if ( promise ) { - return promise; - } - - yield setupPromise( queue, context ); - - return yield select( 'getPromise', queue, context ); -}; - -export const processBatch = function* ( queue, context, meta = {} ) { - const batchId = uuid(); - yield prepareBatchForProcessing( queue, context, batchId, meta ); - const { transactions } = yield select( 'getBatch', batchId ); - - yield { - queue, - context, - batchId, - type: 'BATCH_START', - }; - - let failed = false; - for ( const transactionId of Object.keys( transactions ) ) { - const result = yield* commitTransaction( batchId, transactionId ); - if ( result.state === STATE_ERROR ) { - failed = true; - // Don't break the loop as we still need results for any remaining transactions. - // Queue processor receives the batch object and may choose whether to - // process other transactions or short-circuit with an error. - } - } - - const promise = yield select( 'getPromise', queue, context ); - yield { - queue, - context, - batchId, - type: 'BATCH_FINISH', - state: failed ? STATE_ERROR : STATE_SUCCESS, - }; - const batch = yield select( 'getBatch', batchId ); - - if ( promise ) { - promise.resolve( batch ); - } - - return batch; -}; - -export function* commitTransaction( batchId, transactionId ) { - yield { - batchId, - transactionId, - type: 'COMMIT_TRANSACTION_START', - }; - const batch = yield select( 'getBatch', batchId ); - - let failed = false, - errors, - exception, - results; - try { - results = yield processTransaction( batch, transactionId ); - } catch ( _exception ) { - failed = true; - - // If the error isn't in the expected format, something is wrong - let's rethrow - if ( ! _exception.isTransactionError ) { - throw _exception; - } - exception = _exception; - errors = exception.errorsById; - } - - const finishedAction = { - batchId, - transactionId, - results, - errors, - exception, - type: 'COMMIT_TRANSACTION_FINISH', - state: failed ? STATE_ERROR : STATE_SUCCESS, - }; - - yield finishedAction; - return finishedAction; -} - -export function prepareBatchForProcessing( - queue, - context, - batchId, - meta = {} -) { - return { - type: 'PREPARE_BATCH_FOR_PROCESSING', - queue, - context, - batchId, - meta, - }; -} - -export const registerProcessor = function ( queue, callback ) { - return { - type: 'REGISTER_PROCESSOR', - queue, - callback, - }; -}; diff --git a/packages/edit-widgets/src/store/batch-processing/constants.js b/packages/edit-widgets/src/store/batch-processing/constants.js deleted file mode 100644 index 56c0bbe4fb1e8..0000000000000 --- a/packages/edit-widgets/src/store/batch-processing/constants.js +++ /dev/null @@ -1,11 +0,0 @@ -/** - * Module Constants - */ -export const STORE_NAME = 'core/__experimental-batch-processing'; - -export const BATCH_MAX_SIZE = 20; - -export const STATE_NEW = 'NEW'; -export const STATE_IN_PROGRESS = 'IN_PROGRESS'; -export const STATE_SUCCESS = 'SUCCESS'; -export const STATE_ERROR = 'ERROR'; diff --git a/packages/edit-widgets/src/store/batch-processing/controls.js b/packages/edit-widgets/src/store/batch-processing/controls.js deleted file mode 100644 index a9314a5ecaf70..0000000000000 --- a/packages/edit-widgets/src/store/batch-processing/controls.js +++ /dev/null @@ -1,132 +0,0 @@ -/** - * WordPress dependencies - */ -import { createRegistryControl } from '@wordpress/data'; - -/** - * Internal dependencies - */ -import { STORE_NAME, STATE_ERROR } from './constants'; - -/** - * Calls a selector using chosen registry. - * - * @param {string} selectorName Selector name. - * @param {Array} args Selector arguments. - * @return {Object} control descriptor. - */ -export function select( selectorName, ...args ) { - return { - type: 'SELECT', - selectorName, - args, - }; -} - -/** - * Dispatches an action using chosen registry. - * - * @param {string} actionName Action name. - * @param {Array} args Selector arguments. - * @return {Object} control descriptor. - */ -export function dispatch( actionName, ...args ) { - return { - type: 'DISPATCH', - actionName, - args, - }; -} - -export function processTransaction( batch, transactionId ) { - return { - type: 'PROCESS_TRANSACTION', - batch, - transactionId, - }; -} - -export function enqueueItemAndAutocommit( queue, context, item ) { - return { - type: 'ENQUEUE_ITEM_AND_AUTOCOMMIT', - queue, - context, - item, - }; -} - -const controls = { - SELECT: createRegistryControl( - ( registry ) => ( { selectorName, args } ) => { - return registry.select( STORE_NAME )[ selectorName ]( ...args ); - } - ), - - DISPATCH: createRegistryControl( - ( registry ) => ( { actionName, args } ) => { - return registry.dispatch( STORE_NAME )[ actionName ]( ...args ); - } - ), - - ENQUEUE_ITEM_AND_AUTOCOMMIT: createRegistryControl( - ( registry ) => async ( { queue, context, item } ) => { - const { itemId } = await registry - .dispatch( STORE_NAME ) - .enqueueItem( queue, context, item ); - - // @TODO autocommit when batch size exceeds the maximum or n milliseconds passes - const batch = await registry - .dispatch( STORE_NAME ) - .processBatch( queue, context ); - - if ( batch.state === STATE_ERROR ) { - throw batch.errors[ itemId ]; - } - - return batch.results[ itemId ]; - } - ), - - PROCESS_TRANSACTION: createRegistryControl( - ( registry ) => async ( { batch, transactionId } ) => { - const { transactions, queue } = batch; - const transaction = transactions[ transactionId ]; - const processor = registry - .select( STORE_NAME ) - .getProcessor( queue ); - if ( ! processor ) { - throw new Error( - `There is no batch processor registered for "${ queue }" queue. ` + - `Register one by dispatching registerProcessor() action on ${ STORE_NAME } store.` - ); - } - const itemIds = transaction.items.map( ( { id } ) => id ); - const items = transaction.items.map( ( { item } ) => item ); - let results; - try { - results = await processor( items, batch ); - } catch ( exception ) { - const errorsById = {}; - for ( let i = 0, max = itemIds.length; i < max; i++ ) { - errorsById[ itemIds[ i ] ] = Array.isArray( exception ) - ? exception[ i ] - : exception; - } - throw { - isTransactionError: true, - exception, - errorsById, - }; - } - - // @TODO Assert results.length == items.length - const resultsById = {}; - for ( let i = 0, max = itemIds.length; i < max; i++ ) { - resultsById[ itemIds[ i ] ] = results[ i ]; - } - return resultsById; - } - ), -}; - -export default controls; diff --git a/packages/edit-widgets/src/store/batch-processing/index.js b/packages/edit-widgets/src/store/batch-processing/index.js deleted file mode 100644 index 2e94c4d0364f2..0000000000000 --- a/packages/edit-widgets/src/store/batch-processing/index.js +++ /dev/null @@ -1,31 +0,0 @@ -/** - * WordPress dependencies - */ -import { registerStore } from '@wordpress/data'; - -/** - * Internal dependencies - */ -import * as actions from './actions'; -import reducer from './reducer'; -import controls from './controls'; -import * as selectors from './selectors'; -import { STORE_NAME } from './constants'; - -/** - * Block editor data store configuration. - * - * @see https://github.com/WordPress/gutenberg/blob/HEAD/packages/data/README.md#registerStore - * - * @type {Object} - */ -const storeConfig = { - actions, - reducer, - controls, - selectors, -}; - -const store = registerStore( STORE_NAME, storeConfig ); - -export default store; diff --git a/packages/edit-widgets/src/store/batch-processing/reducer.js b/packages/edit-widgets/src/store/batch-processing/reducer.js deleted file mode 100644 index aa692b9d80d61..0000000000000 --- a/packages/edit-widgets/src/store/batch-processing/reducer.js +++ /dev/null @@ -1,223 +0,0 @@ -/** - * External dependencies - */ -import { omit } from 'lodash'; - -/** - * Internal dependencies - */ -import { - BATCH_MAX_SIZE, - STATE_NEW, - STATE_IN_PROGRESS, - STATE_SUCCESS, - STATE_ERROR, -} from './constants'; - -const defaultState = { - lastBatchId: 0, - enqueuedItems: {}, - batches: {}, - processors: {}, - promises: {}, -}; - -export default function reducer( state = defaultState, action ) { - switch ( action.type ) { - case 'ENQUEUE_ITEM': { - const { queue, context, item, itemId } = action; - - const stateQueue = state.enqueuedItems[ queue ] || {}; - const stateItems = stateQueue[ context ] || []; - - return { - ...state, - enqueuedItems: { - ...state.enqueuedItems, - [ queue ]: { - ...stateQueue, - [ context ]: [ ...stateItems, { id: itemId, item } ], - }, - }, - }; - } - - case 'PREPARE_BATCH_FOR_PROCESSING': { - const { queue, context, batchId, meta } = action; - - if ( batchId in state.batches ) { - throw new Error( `Batch ${ batchId } already exists` ); - } - - const stateQueue = state.enqueuedItems[ queue ] || {}; - const enqueuedItems = [ ...( stateQueue[ context ] || [] ) ]; - const sortedItemIds = enqueuedItems.map( ( { id } ) => id ); - const transactions = {}; - let transactionNb = 0; - while ( enqueuedItems.length ) { - const transactionId = `${ batchId }-${ transactionNb }`; - transactions[ transactionId ] = { - number: transactionNb, - id: transactionId, - items: enqueuedItems.splice( 0, BATCH_MAX_SIZE ), - }; - ++transactionNb; - } - - const batch = { - id: batchId, - state: STATE_NEW, - queue, - context, - sortedItemIds, - transactions, - results: {}, - meta, - }; - - return { - ...state, - enqueuedItems: { - ...state.enqueuedItems, - [ queue ]: { - ...stateQueue, - [ context ]: [], - }, - }, - batches: { - ...state.batches, - [ batchId ]: batch, - }, - }; - } - - case 'SETUP_PROMISE': { - return { - ...state, - promises: { - ...state.promises, - [ action.queue ]: { - ...( state.promises[ action.queue ] || {} ), - [ action.context ]: { - promise: action.promise, - resolve: action.resolve, - reject: action.reject, - }, - }, - }, - }; - } - - case 'BATCH_START': { - const { batchId } = action; - return { - ...state, - batches: { - ...state.batches, - [ batchId ]: { - ...state.batches[ batchId ], - state: STATE_IN_PROGRESS, - }, - }, - }; - } - - case 'BATCH_FINISH': { - const { batchId, state: commitState } = action; - return { - ...state, - batches: { - ...state.batches, - [ batchId ]: { - ...state.batches[ batchId ], - state: - commitState === STATE_SUCCESS - ? STATE_SUCCESS - : STATE_ERROR, - }, - }, - promises: { - ...state.promises, - [ action.queue ]: omit( - state.promises[ action.queue ] || {}, - [ action.context ] - ), - }, - }; - } - - case 'COMMIT_TRANSACTION_START': { - const { batchId, transactionId } = action; - return { - ...state, - batches: { - ...state.batches, - [ batchId ]: { - ...state.batches[ batchId ], - transactions: { - ...state.batches[ batchId ].transactions, - [ transactionId ]: { - ...state.batches[ batchId ].transactions[ - transactionId - ], - state: STATE_IN_PROGRESS, - }, - }, - }, - }, - }; - } - - case 'COMMIT_TRANSACTION_FINISH': { - const { - batchId, - state: transactionState, - transactionId, - results = {}, - errors = {}, - exception, - } = action; - - const stateBatch = state.batches[ batchId ] || {}; - return { - ...state, - batches: { - ...state.batches, - [ batchId ]: { - ...stateBatch, - transactions: { - ...stateBatch.transactions, - [ transactionId ]: { - ...stateBatch.transactions[ transactionId ], - state: STATE_SUCCESS, - }, - }, - results: { - ...stateBatch.results, - ...results, - }, - state: - transactionState === STATE_SUCCESS - ? STATE_SUCCESS - : STATE_ERROR, - errors, - exception, - }, - }, - }; - } - - case 'REGISTER_PROCESSOR': - const { queue, callback } = action; - - return { - ...state, - processors: { - ...state.processors, - [ queue ]: callback, - }, - }; - } - - return state; -} diff --git a/packages/edit-widgets/src/store/batch-processing/selectors.js b/packages/edit-widgets/src/store/batch-processing/selectors.js deleted file mode 100644 index c19274e4b82e8..0000000000000 --- a/packages/edit-widgets/src/store/batch-processing/selectors.js +++ /dev/null @@ -1,17 +0,0 @@ -export const getBatch = ( state, batchId ) => { - return state.batches[ batchId ]; -}; - -export const getProcessor = ( state, queue ) => { - return state.processors[ queue ]; -}; - -export const getPromise = ( state, queue, context ) => { - return state.promises[ queue ]?.[ context ]; -}; - -export const getPromises = ( state, queue ) => { - return Object.values( state.promises[ queue ] || {} ).map( - ( { promise } ) => promise - ); -}; diff --git a/packages/edit-widgets/src/store/batch-processing/test/test.js b/packages/edit-widgets/src/store/batch-processing/test/test.js deleted file mode 100644 index f48fcfa7befa5..0000000000000 --- a/packages/edit-widgets/src/store/batch-processing/test/test.js +++ /dev/null @@ -1,83 +0,0 @@ -/** - * Internal dependencies - */ -import store from '../index'; -import { - registerProcessor, - enqueueItemAndWaitForResults, - processBatch, -} from '../actions'; -import { STATE_ERROR, STATE_SUCCESS } from '../constants'; - -const TEST_QUEUE = 'TEST_QUEUE'; -const TEST_CONTEXT = 'default'; - -async function processor( requests, transaction ) { - if ( transaction.state === STATE_ERROR ) { - throw { - code: 'transaction_failed', - data: { status: 500 }, - message: 'Transaction failed.', - }; - } - - return await Promise.resolve( - requests.map( ( request ) => ( { - done: true, - name: request.name, - } ) ) - ); -} - -async function testItem( name ) { - const item = { name }; - const { wait } = await store.dispatch( - enqueueItemAndWaitForResults( TEST_QUEUE, TEST_CONTEXT, item ) - ); - - const expected = { done: true, name }; - - // We can't await this until the batch is processed. - // eslint-disable-next-line jest/valid-expect - const promise = expect( wait ).resolves.toEqual( expected ); - - return { expected, promise }; -} - -describe( 'waitForResults', function () { - store.dispatch( registerProcessor( TEST_QUEUE, processor ) ); - - it( 'works', async () => { - expect.assertions( 4 ); - - const { expected: i1, promise: p1 } = await testItem( 'i1' ); - const { expected: i2, promise: p2 } = await testItem( 'i2' ); - - const resolves = [ p1, p2 ]; - const batch = await store.dispatch( - processBatch( TEST_QUEUE, TEST_CONTEXT ) - ); - - expect( batch.state ).toEqual( STATE_SUCCESS ); - expect( Object.values( batch.results ) ).toEqual( [ i1, i2 ] ); - - await Promise.all( resolves ); - } ); - - it( 'can use the same context more than once', async () => { - expect.assertions( 4 ); - - const { promise: p1 } = await testItem( 'i1' ); - await store.dispatch( processBatch( TEST_QUEUE, TEST_CONTEXT ) ); - await p1; - - const { expected: i2, promise: p2 } = await testItem( 'i2' ); - const batch = await store.dispatch( - processBatch( TEST_QUEUE, TEST_CONTEXT ) - ); - - expect( batch.state ).toEqual( STATE_SUCCESS ); - expect( Object.values( batch.results ) ).toEqual( [ i2 ] ); - await p2; - } ); -} ); diff --git a/packages/edit-widgets/src/store/batch-support.js b/packages/edit-widgets/src/store/batch-support.js deleted file mode 100644 index ad6afe9cb5e05..0000000000000 --- a/packages/edit-widgets/src/store/batch-support.js +++ /dev/null @@ -1,119 +0,0 @@ -/** - * WordPress dependencies - */ -import apiFetch from '@wordpress/api-fetch'; -import { select, dispatch } from '@wordpress/data'; -import { __ } from '@wordpress/i18n'; - -/** - * Internal dependencies - */ -import './batch-processing'; -import { STATE_ERROR } from './batch-processing/constants'; - -function shoehornBatchSupport() { - apiFetch.use( async ( options, next ) => { - if ( - ! [ 'POST', 'PUT', 'PATCH', 'DELETE' ].includes( options.method ) || - ! isWidgetsEndpoint( options.path ) - ) { - return next( options ); - } - - const { wait } = await addToBatch( options ); - - return wait.catch( ( error ) => { - // If this item didn't encounter an error specifically, the REST API - // will return `null`. We need to provide an error object of some kind - // to the apiFetch caller as they expect either a valid response, or - // an error. Null wouldn't be acceptable. - if ( error === null ) { - error = { - code: 'transaction_failed', - data: { status: 400 }, - message: __( - 'This item could not be saved because another item encountered an error when trying to save.' - ), - }; - } - - throw error; - } ); - } ); - - // This is a hack to prevent the following timing problem: - // * batch request starts, cache is invalidated -> - // * resolvers sends GET request to /wp/v2/widgets?per_page=-1 before the batch is finished -> - // * batch request is processed and new widgets are saved -> - // * core/data stores the new version of the data -> - // * GET request is processed and returns the old widgets -> - // * core/data overwrites good data with stale data - // - // The ultimate solution is to fix the problem in core/data but this has to do for now - apiFetch.use( async ( options, next ) => { - if ( - [ 'GET', undefined ].includes( options.method ) && - isWidgetsEndpoint( options.path ) - ) { - // Wait for any batch requests already in progress - await Promise.all( - select( 'core/__experimental-batch-processing' ).getPromises( - 'WIDGETS_API_FETCH' - ) - ); - } - return next( options ); - } ); - - dispatch( 'core/__experimental-batch-processing' ).registerProcessor( - 'WIDGETS_API_FETCH', - batchProcessor - ); -} - -function isWidgetsEndpoint( path ) { - // This should be more sophisticated in reality: - return path.startsWith( '/wp/v2/widgets' ); -} - -function addToBatch( request ) { - return dispatch( - 'core/__experimental-batch-processing' - ).enqueueItemAndWaitForResults( 'WIDGETS_API_FETCH', 'default', request ); -} - -async function batchProcessor( requests, transaction ) { - if ( transaction.state === STATE_ERROR ) { - throw { - code: 'transaction_failed', - data: { status: 500 }, - message: 'Transaction failed.', - }; - } - - const response = await apiFetch( { - path: '/v1/batch', - method: 'POST', - data: { - validation: 'require-all-validate', - requests: requests.map( ( options ) => ( { - path: options.path, - body: options.data, - method: options.method, - headers: options.headers, - } ) ), - }, - } ); - - if ( response.failed ) { - throw response.responses.map( ( itemResponse ) => { - // The REST API returns null if the request did not have an error. - return itemResponse === null ? null : itemResponse.body; - } ); - } - - return response.responses.map( ( { body } ) => body ); -} - -// setTimeout is a hack to ensure batch-processing store is available for dispatching -setTimeout( shoehornBatchSupport ); diff --git a/packages/edit-widgets/src/store/index.js b/packages/edit-widgets/src/store/index.js index cedbcf8f1c786..cf294eb493f1c 100644 --- a/packages/edit-widgets/src/store/index.js +++ b/packages/edit-widgets/src/store/index.js @@ -12,7 +12,6 @@ import * as resolvers from './resolvers'; import * as selectors from './selectors'; import * as actions from './actions'; import controls from './controls'; -import './batch-support'; import { STORE_NAME } from './constants'; /**