From 06714a88e1fb533c3e47f6e5468eb88604517f34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Tue, 24 Nov 2020 15:52:49 +0100 Subject: [PATCH 1/3] Propose __experimentalBatchSaveEntityRecords core action --- .../developers/data/data-core.md | 1 + packages/core-data/README.md | 1 + packages/core-data/src/actions.js | 87 ++++++++++--- .../src}/batch-processing/actions.js | 26 ++-- .../src}/batch-processing/constants.js | 0 .../src}/batch-processing/controls.js | 4 +- .../src}/batch-processing/index.js | 0 .../src}/batch-processing/reducer.js | 0 .../src}/batch-processing/selectors.js | 2 +- .../src}/batch-processing/test/test.js | 0 packages/core-data/src/batch-support.js | 66 ++++++++++ packages/core-data/src/index.js | 1 + packages/edit-widgets/src/store/actions.js | 35 +++--- .../edit-widgets/src/store/batch-support.js | 119 ------------------ packages/edit-widgets/src/store/index.js | 1 - 15 files changed, 176 insertions(+), 167 deletions(-) rename packages/{edit-widgets/src/store => core-data/src}/batch-processing/actions.js (86%) rename packages/{edit-widgets/src/store => core-data/src}/batch-processing/constants.js (100%) rename packages/{edit-widgets/src/store => core-data/src}/batch-processing/controls.js (95%) rename packages/{edit-widgets/src/store => core-data/src}/batch-processing/index.js (100%) rename packages/{edit-widgets/src/store => core-data/src}/batch-processing/reducer.js (100%) rename packages/{edit-widgets/src/store => core-data/src}/batch-processing/selectors.js (84%) rename packages/{edit-widgets/src/store => core-data/src}/batch-processing/test/test.js (100%) create mode 100644 packages/core-data/src/batch-support.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 37c4ad7d5f858..eb025fc90b61c 100644 --- a/docs/designers-developers/developers/data/data-core.md +++ b/docs/designers-developers/developers/data/data-core.md @@ -696,6 +696,7 @@ _Parameters_ - _record_ `Object`: Record to be saved. - _options_ `Object`: Saving options. - _options.isAutosave_ `[boolean]`: Whether this is an autosave. +- _options.isBatch_ `[boolean]`: Whether this should use a batch request. # **undo** diff --git a/packages/core-data/README.md b/packages/core-data/README.md index bc887ad571c7c..cbb8e5a06731c 100644 --- a/packages/core-data/README.md +++ b/packages/core-data/README.md @@ -229,6 +229,7 @@ _Parameters_ - _record_ `Object`: Record to be saved. - _options_ `Object`: Saving options. - _options.isAutosave_ `[boolean]`: Whether this is an autosave. +- _options.isBatch_ `[boolean]`: Whether this should use a batch request. # **undo** diff --git a/packages/core-data/src/actions.js b/packages/core-data/src/actions.js index 31163cae4dcbf..b29b98a2e5189 100644 --- a/packages/core-data/src/actions.js +++ b/packages/core-data/src/actions.js @@ -7,8 +7,8 @@ import { v4 as uuid } from 'uuid'; /** * WordPress dependencies */ -import { controls } from '@wordpress/data'; -import { apiFetch } from '@wordpress/data-controls'; +import { controls, dispatch } from '@wordpress/data'; +import { __unstableAwaitPromise, apiFetch } from '@wordpress/data-controls'; import { addQueryArgs } from '@wordpress/url'; /** @@ -328,17 +328,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 {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 {boolean} [options.isBatch=false] Whether this should use a batch request. */ export function* saveEntityRecord( kind, name, record, - { isAutosave = false } = { isAutosave: false } + { isAutosave = false, isBatch = false } = { + isAutosave: false, + isBatch: false, + } ) { const entities = yield getKindEntities( kind ); const entity = find( entities, { kind, name } ); @@ -442,11 +446,14 @@ export function* saveEntityRecord( : data.status, } ); - updatedRecord = yield apiFetch( { - path: `${ path }/autosaves`, - method: 'POST', - data, - } ); + updatedRecord = yield* performRequest( + { + path: `${ path }/autosaves`, + method: 'POST', + data, + }, + { isBatch } + ); // 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. @@ -542,11 +549,14 @@ export function* saveEntityRecord( false ); - updatedRecord = yield apiFetch( { - path, - method: recordId ? 'PUT' : 'POST', - data, - } ); + updatedRecord = yield* performRequest( + { + path, + method: recordId ? 'PUT' : 'POST', + data, + }, + { isBatch } + ); yield receiveEntityRecords( kind, name, @@ -601,6 +611,21 @@ export function* saveEntityRecord( } } +function* performRequest( request, options ) { + if ( ! options.isBatch ) { + return yield apiFetch( request ); + } + + const { wait } = yield controls.dispatch( + 'core/__experimental-batch-processing', + 'enqueueItemAndWaitForResults', + 'API_FETCH', + 'default', + request + ); + return yield __unstableAwaitPromise( wait ); +} + /** * Action triggered to save an entity record's edits. * @@ -632,6 +657,34 @@ export function* saveEditedEntityRecord( kind, name, recordId, options ) { yield* saveEntityRecord( kind, name, record, options ); } +/** + * Adds all specified entity records to a batch, and then saves them using one or more batch requests. + * + * @param {Object[]} spec List of { kind, name, key, record } of records to save. + * @return {Object} Finalized batch object. + */ +export function* __experimentalBatchSaveEntityRecords( spec ) { + for ( const { kind, type, key, record } of spec ) { + if ( key ) { + dispatch( 'core' ).saveEditedEntityRecord( kind, type, key, { + isBatch: true, + } ); + } else { + dispatch( 'core' ).saveEntityRecord( kind, type, record, { + isBatch: true, + } ); + } + } + yield __unstableAwaitPromise( + new Promise( ( resolve ) => setTimeout( resolve, 10 ) ) + ); + return yield controls.dispatch( + 'core/__experimental-batch-processing', + 'processBatch', + 'API_FETCH' + ); +} + /** * Returns an action object used in signalling that Upload permissions have been received. * diff --git a/packages/edit-widgets/src/store/batch-processing/actions.js b/packages/core-data/src/batch-processing/actions.js similarity index 86% rename from packages/edit-widgets/src/store/batch-processing/actions.js rename to packages/core-data/src/batch-processing/actions.js index a21a7433e44f0..be62c6d1283aa 100644 --- a/packages/edit-widgets/src/store/batch-processing/actions.js +++ b/packages/core-data/src/batch-processing/actions.js @@ -14,11 +14,19 @@ import { } from './controls'; import { STATE_ERROR, STATE_SUCCESS } from './constants'; -export const enqueueItemAndAutocommit = function* ( queue, context, item ) { +export const enqueueItemAndAutocommit = function* ( + queue, + context = 'default', + item +) { return yield enqueueAutocommitControl( queue, context, item ); }; -export const enqueueItemAndWaitForResults = function* ( queue, context, item ) { +export const enqueueItemAndWaitForResults = function* ( + queue, + context = 'default', + item +) { const { itemId } = yield dispatch( 'enqueueItem', queue, context, item ); const { promise } = yield* getOrSetupPromise( queue, context ); @@ -33,7 +41,7 @@ export const enqueueItemAndWaitForResults = function* ( queue, context, item ) { }; }; -export const enqueueItem = function ( queue, context, item ) { +export const enqueueItem = function ( queue, context = 'default', item ) { const itemId = uuid(); return { type: 'ENQUEUE_ITEM', @@ -44,7 +52,7 @@ export const enqueueItem = function ( queue, context, item ) { }; }; -const setupPromise = function ( queue, context ) { +const setupPromise = function ( queue, context = 'default' ) { const action = { type: 'SETUP_PROMISE', queue, @@ -59,7 +67,7 @@ const setupPromise = function ( queue, context ) { return action; }; -const getOrSetupPromise = function* ( queue, context ) { +const getOrSetupPromise = function* ( queue, context = 'default' ) { const promise = yield select( 'getPromise', queue, context ); if ( promise ) { @@ -71,7 +79,11 @@ const getOrSetupPromise = function* ( queue, context ) { return yield select( 'getPromise', queue, context ); }; -export const processBatch = function* ( queue, context, meta = {} ) { +export const processBatch = function* ( + queue, + context = 'default', + meta = {} +) { const batchId = uuid(); yield prepareBatchForProcessing( queue, context, batchId, meta ); const { transactions } = yield select( 'getBatch', batchId ); @@ -152,7 +164,7 @@ export function* commitTransaction( batchId, transactionId ) { export function prepareBatchForProcessing( queue, - context, + context = 'default', batchId, meta = {} ) { diff --git a/packages/edit-widgets/src/store/batch-processing/constants.js b/packages/core-data/src/batch-processing/constants.js similarity index 100% rename from packages/edit-widgets/src/store/batch-processing/constants.js rename to packages/core-data/src/batch-processing/constants.js diff --git a/packages/edit-widgets/src/store/batch-processing/controls.js b/packages/core-data/src/batch-processing/controls.js similarity index 95% rename from packages/edit-widgets/src/store/batch-processing/controls.js rename to packages/core-data/src/batch-processing/controls.js index a9314a5ecaf70..f907ac3f6a167 100644 --- a/packages/edit-widgets/src/store/batch-processing/controls.js +++ b/packages/core-data/src/batch-processing/controls.js @@ -46,7 +46,7 @@ export function processTransaction( batch, transactionId ) { }; } -export function enqueueItemAndAutocommit( queue, context, item ) { +export function enqueueItemAndAutocommit( queue, context = 'default', item ) { return { type: 'ENQUEUE_ITEM_AND_AUTOCOMMIT', queue, @@ -69,7 +69,7 @@ const controls = { ), ENQUEUE_ITEM_AND_AUTOCOMMIT: createRegistryControl( - ( registry ) => async ( { queue, context, item } ) => { + ( registry ) => async ( { queue, context = 'default', item } ) => { const { itemId } = await registry .dispatch( STORE_NAME ) .enqueueItem( queue, context, item ); diff --git a/packages/edit-widgets/src/store/batch-processing/index.js b/packages/core-data/src/batch-processing/index.js similarity index 100% rename from packages/edit-widgets/src/store/batch-processing/index.js rename to packages/core-data/src/batch-processing/index.js diff --git a/packages/edit-widgets/src/store/batch-processing/reducer.js b/packages/core-data/src/batch-processing/reducer.js similarity index 100% rename from packages/edit-widgets/src/store/batch-processing/reducer.js rename to packages/core-data/src/batch-processing/reducer.js diff --git a/packages/edit-widgets/src/store/batch-processing/selectors.js b/packages/core-data/src/batch-processing/selectors.js similarity index 84% rename from packages/edit-widgets/src/store/batch-processing/selectors.js rename to packages/core-data/src/batch-processing/selectors.js index c19274e4b82e8..4e2bcedc76cc1 100644 --- a/packages/edit-widgets/src/store/batch-processing/selectors.js +++ b/packages/core-data/src/batch-processing/selectors.js @@ -6,7 +6,7 @@ export const getProcessor = ( state, queue ) => { return state.processors[ queue ]; }; -export const getPromise = ( state, queue, context ) => { +export const getPromise = ( state, queue, context = 'default' ) => { return state.promises[ queue ]?.[ context ]; }; diff --git a/packages/edit-widgets/src/store/batch-processing/test/test.js b/packages/core-data/src/batch-processing/test/test.js similarity index 100% rename from packages/edit-widgets/src/store/batch-processing/test/test.js rename to packages/core-data/src/batch-processing/test/test.js diff --git a/packages/core-data/src/batch-support.js b/packages/core-data/src/batch-support.js new file mode 100644 index 0000000000000..4ae4a45d2b2fb --- /dev/null +++ b/packages/core-data/src/batch-support.js @@ -0,0 +1,66 @@ +/** + * WordPress dependencies + */ +import apiFetch from '@wordpress/api-fetch'; +import { __ } from '@wordpress/i18n'; +import { dispatch } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import './batch-processing'; +import { STATE_ERROR } from './batch-processing/constants'; + +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, + } ) ), + }, + } ); + + const failed = + response.failed || + response.responses.filter( + ( { status } ) => status < 200 || status >= 300 + ).length; + if ( failed ) { + throw response.responses.map( ( itemResponse ) => { + // The REST API returns null if the request did not have an error. + return itemResponse === null + ? { + code: 'transaction_failed', + data: { status: 400 }, + message: __( + 'This item could not be saved because another item encountered an error when trying to save.' + ), + } + : itemResponse.body; + } ); + } + + return response.responses.map( ( { body } ) => body ); +} + +// Ensure batch-processing store is available for dispatching +setTimeout( () => { + dispatch( 'core/__experimental-batch-processing' ).registerProcessor( + 'API_FETCH', + batchProcessor + ); +} ); diff --git a/packages/core-data/src/index.js b/packages/core-data/src/index.js index fc2c051a52165..14e90b8d6cf5a 100644 --- a/packages/core-data/src/index.js +++ b/packages/core-data/src/index.js @@ -15,6 +15,7 @@ import * as locksSelectors from './locks/selectors'; import * as locksActions from './locks/actions'; import { defaultEntities, getMethodName } from './entities'; import { STORE_NAME } from './name'; +import './batch-support'; // The entity selectors/resolvers and actions are shortcuts to their generic equivalents // (getEntityRecord, getEntityRecords, updateEntityRecord, updateEntityRecordss) diff --git a/packages/edit-widgets/src/store/actions.js b/packages/edit-widgets/src/store/actions.js index 3b0614dce94df..06b4f913b66f5 100644 --- a/packages/edit-widgets/src/store/actions.js +++ b/packages/edit-widgets/src/store/actions.js @@ -7,12 +7,10 @@ import { invert } from 'lodash'; * WordPress dependencies */ import { __, sprintf } from '@wordpress/i18n'; -import { dispatch as dataDispatch } from '@wordpress/data'; /** * Internal dependencies */ -import { STATE_SUCCESS } from './batch-processing/constants'; import { dispatch, select, getWidgetToClientIdMapping } from './controls'; import { transformBlockToWidget } from './transformers'; import { @@ -133,6 +131,16 @@ export function* saveWidgetArea( widgetAreaId ) { // since order is important. sidebarWidgetsIds.push( widgetId ); + const recordSpec = { + kind: 'root', + type: 'widget', + key: widgetId, + record: { + ...widget, + sidebar: widgetAreaId, + }, + }; + if ( widgetId ) { yield dispatch( 'core', @@ -157,36 +165,23 @@ export function* saveWidgetArea( widgetAreaId ) { if ( ! hasEdits ) { continue; } - - dataDispatch( 'core' ).saveEditedEntityRecord( - 'root', - 'widget', - widgetId, - widget - ); - } else { - // This is a new widget instance. - dataDispatch( 'core' ).saveEntityRecord( 'root', 'widget', { - ...widget, - sidebar: widgetAreaId, - } ); } batchMeta.push( { block, + recordSpec, position: i, clientId: block.clientId, } ); } const batch = yield dispatch( - 'core/__experimental-batch-processing', - 'processBatch', - 'WIDGETS_API_FETCH', - 'default' + 'core', + '__experimentalBatchSaveEntityRecords', + batchMeta.map( ( { recordSpec } ) => recordSpec ) ); - if ( batch.state !== STATE_SUCCESS ) { + if ( batch.state !== 'SUCCESS' ) { const errors = batch.sortedItemIds.map( ( id ) => batch.errors[ id ] ); const failedWidgetNames = []; 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 bdbf487125e33..dc07e3f084df9 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'; /** * Module Constants From 774c3df0dae9780e74899a874d8b35e78f5fdfba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Fri, 27 Nov 2020 13:18:35 +0100 Subject: [PATCH 2/3] Issue a preflight request to get batch size --- .../core-data/src/batch-processing/actions.js | 13 ++++++++-- .../src/batch-processing/constants.js | 2 +- .../src/batch-processing/controls.js | 24 ++++++++++++++++++- .../core-data/src/batch-processing/reducer.js | 9 ++++--- .../src/batch-processing/selectors.js | 6 ++++- packages/core-data/src/batch-support.js | 16 ++++++++++++- 6 files changed, 59 insertions(+), 11 deletions(-) diff --git a/packages/core-data/src/batch-processing/actions.js b/packages/core-data/src/batch-processing/actions.js index be62c6d1283aa..0d7270e4374e8 100644 --- a/packages/core-data/src/batch-processing/actions.js +++ b/packages/core-data/src/batch-processing/actions.js @@ -9,6 +9,7 @@ import { v4 as uuid } from 'uuid'; import { select, dispatch, + getBatchSize, enqueueItemAndAutocommit as enqueueAutocommitControl, processTransaction, } from './controls'; @@ -85,7 +86,8 @@ export const processBatch = function* ( meta = {} ) { const batchId = uuid(); - yield prepareBatchForProcessing( queue, context, batchId, meta ); + const batchSize = yield getBatchSize(queue); + yield prepareBatchForProcessing( queue, context, batchId, batchSize, meta ); const { transactions } = yield select( 'getBatch', batchId ); yield { @@ -166,6 +168,7 @@ export function prepareBatchForProcessing( queue, context = 'default', batchId, + batchSize, meta = {} ) { return { @@ -173,14 +176,20 @@ export function prepareBatchForProcessing( queue, context, batchId, + batchSize, meta, }; } -export const registerProcessor = function ( queue, callback ) { +export const registerProcessor = function ( + queue, + callback, + batchSizeCallback +) { return { type: 'REGISTER_PROCESSOR', queue, callback, + batchSizeCallback, }; }; diff --git a/packages/core-data/src/batch-processing/constants.js b/packages/core-data/src/batch-processing/constants.js index 56c0bbe4fb1e8..1390d58ad0d6e 100644 --- a/packages/core-data/src/batch-processing/constants.js +++ b/packages/core-data/src/batch-processing/constants.js @@ -3,7 +3,7 @@ */ export const STORE_NAME = 'core/__experimental-batch-processing'; -export const BATCH_MAX_SIZE = 20; +export const DEFAULT_BATCH_SIZE = 20; export const STATE_NEW = 'NEW'; export const STATE_IN_PROGRESS = 'IN_PROGRESS'; diff --git a/packages/core-data/src/batch-processing/controls.js b/packages/core-data/src/batch-processing/controls.js index f907ac3f6a167..2cb4883b00325 100644 --- a/packages/core-data/src/batch-processing/controls.js +++ b/packages/core-data/src/batch-processing/controls.js @@ -6,7 +6,20 @@ import { createRegistryControl } from '@wordpress/data'; /** * Internal dependencies */ -import { STORE_NAME, STATE_ERROR } from './constants'; +import { STORE_NAME, STATE_ERROR, DEFAULT_BATCH_SIZE } from './constants'; + +/** + * Finds a batch size for a given queue. + * + * @param {string} queue Queue name. + * @return {Object} control descriptor. + */ +export function getBatchSize( queue ) { + return { + type: 'GET_BATCH_SIZE', + queue, + }; +} /** * Calls a selector using chosen registry. @@ -68,6 +81,15 @@ const controls = { } ), + GET_BATCH_SIZE: createRegistryControl( + ( registry ) => async ( { queue } ) => { + const callback = registry + .select( STORE_NAME ) + .getBatchSizeCallback( queue ); + return callback ? await callback() : DEFAULT_BATCH_SIZE; + } + ), + ENQUEUE_ITEM_AND_AUTOCOMMIT: createRegistryControl( ( registry ) => async ( { queue, context = 'default', item } ) => { const { itemId } = await registry diff --git a/packages/core-data/src/batch-processing/reducer.js b/packages/core-data/src/batch-processing/reducer.js index aa692b9d80d61..358b15e3139ab 100644 --- a/packages/core-data/src/batch-processing/reducer.js +++ b/packages/core-data/src/batch-processing/reducer.js @@ -7,7 +7,6 @@ import { omit } from 'lodash'; * Internal dependencies */ import { - BATCH_MAX_SIZE, STATE_NEW, STATE_IN_PROGRESS, STATE_SUCCESS, @@ -43,7 +42,7 @@ export default function reducer( state = defaultState, action ) { } case 'PREPARE_BATCH_FOR_PROCESSING': { - const { queue, context, batchId, meta } = action; + const { queue, context, batchId, batchSize, meta } = action; if ( batchId in state.batches ) { throw new Error( `Batch ${ batchId } already exists` ); @@ -59,7 +58,7 @@ export default function reducer( state = defaultState, action ) { transactions[ transactionId ] = { number: transactionNb, id: transactionId, - items: enqueuedItems.splice( 0, BATCH_MAX_SIZE ), + items: enqueuedItems.splice( 0, batchSize ), }; ++transactionNb; } @@ -208,13 +207,13 @@ export default function reducer( state = defaultState, action ) { } case 'REGISTER_PROCESSOR': - const { queue, callback } = action; + const { queue, callback, batchSizeCallback } = action; return { ...state, processors: { ...state.processors, - [ queue ]: callback, + [ queue ]: { callback, batchSizeCallback }, }, }; } diff --git a/packages/core-data/src/batch-processing/selectors.js b/packages/core-data/src/batch-processing/selectors.js index 4e2bcedc76cc1..6376e22c536b1 100644 --- a/packages/core-data/src/batch-processing/selectors.js +++ b/packages/core-data/src/batch-processing/selectors.js @@ -3,7 +3,11 @@ export const getBatch = ( state, batchId ) => { }; export const getProcessor = ( state, queue ) => { - return state.processors[ queue ]; + return state.processors[ queue ].callback; +}; + +export const getBatchSizeCallback = ( state, queue ) => { + return state.processors[ queue ].batchSizeCallback; }; export const getPromise = ( state, queue, context = 'default' ) => { diff --git a/packages/core-data/src/batch-support.js b/packages/core-data/src/batch-support.js index 4ae4a45d2b2fb..2aa17e6452157 100644 --- a/packages/core-data/src/batch-support.js +++ b/packages/core-data/src/batch-support.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import { memoize } from 'lodash/function'; + /** * WordPress dependencies */ @@ -57,10 +62,19 @@ async function batchProcessor( requests, transaction ) { return response.responses.map( ( { body } ) => body ); } +const determineBatchSize = memoize( async () => { + const response = await apiFetch( { + path: '/v1/batch', + method: 'OPTIONS', + } ); + return response.endpoints[ 0 ].args.requests.maxItems; +} ); + // Ensure batch-processing store is available for dispatching setTimeout( () => { dispatch( 'core/__experimental-batch-processing' ).registerProcessor( 'API_FETCH', - batchProcessor + batchProcessor, + determineBatchSize ); } ); From 93638fe3fd4feefb780d56aa4f4b982cdf4bf3c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Tue, 1 Dec 2020 16:28:14 +0100 Subject: [PATCH 3/3] Lint and README --- .../core-data/src/batch-processing/README.md | 185 ++++++++++++++++++ .../core-data/src/batch-processing/actions.js | 2 +- packages/core-data/src/batch-support.js | 4 +- 3 files changed, 188 insertions(+), 3 deletions(-) create mode 100644 packages/core-data/src/batch-processing/README.md diff --git a/packages/core-data/src/batch-processing/README.md b/packages/core-data/src/batch-processing/README.md new file mode 100644 index 0000000000000..78b0967b80887 --- /dev/null +++ b/packages/core-data/src/batch-processing/README.md @@ -0,0 +1,185 @@ +## Batch processing + +This package provides generic tooling for processing batches of data. + +The basic usage is as follows: + +```js +const { registerProcessor, enqueueItem, processBatch } = useDispatch( + 'core/__experimental-batch-processing' +); + +useEffect( () => { + async function run() { + // Setup a simple batch processor for "log" queue + registerProcessor( + 'log', + // Log strings + ( inputs ) => { + console.log( inputs ); + // A processor must return a list of `inputs.length` results – one result per input. + return inputs.map( s => 'logged-' + s ); + }, + // Limit transaction size to 2 + () => 2 + ); + + // Enqueue "item1" to "item3" in "log" queue and "default" context: + enqueueItem( 'log', 'default', 'item 1' ); + enqueueItem( 'log', 'default', 'item 2' ); + enqueueItem( 'log', 'default', 'item 3' ); + + // Process log/default queue + const batch = await processBatch( 'log', 'default' ); + // The following output was logged in the console: + // ["item 1", "item 2"] + // ["item 3"] + // This is because processBatch executed two transactions (max size was set to 2), each executing "log" queue procesor: + console.log( batch.transactions.length ); + // 2 + + // Each item was assigned a unique id: + console.log( batch.sortedItemIds ); + // [ "d6cdb799-909c-440a-bd57-9fbe7406aa0f", "e046923c-e340-4c30-89d7-6b6794bff66c", "71acd1d8-ed91-42df-ad1c-c6c1e1117227" ] + + // Results are keyed by these unique ids: + console.log( batch.results ); + // { 71acd1d8-ed91-42df-ad1c-c6c1e1117227: "logged-item 3", d6cdb799-909c-440a-bd57-9fbe7406aa0f: "logged-item 1", e046923c-e340-4c30-89d7-6b6794bff66c: "logged-item 2"} } + + // Get list of sorted results: + console.log( batch.sortedItemIds.map( id => batch.results[id] ) ) + // [ "logged-item 1", "logged-item 2", "logged-item 3" ] + } + run(); +}, [] ); +``` + +Each queue may have multiple "sub-queues" called "contexts": + +```js +const { registerProcessor, enqueueItem, processBatch } = useDispatch( + 'core/__experimental-batch-processing' +); + +useEffect( () => { + async function run() { + // Setup a simple batch processor for "log" queue + registerProcessor( + 'log', + // Log strings + ( inputs ) => { + console.log( inputs ); + // A processor must return a list of `inputs.length` results – one result per input. + return inputs.map( s => 'logged-' + s ); + }, + // Limit transaction size to 2 + () => 2 + ); + + // Enqueue "item1" to "item3" in "log" queue and "default" context: + enqueueItem( 'log', 'default', 'item 1' ); + enqueueItem( 'log', 'default', 'item 2' ); + enqueueItem( 'log', 'default', 'item 3' ); + + // Enqueue "item1" to "item3" in "log" queue and "another" context: + enqueueItem( 'log', 'another', 'item 4' ) + enqueueItem( 'log', 'another', 'item 5' ) + enqueueItem( 'log', 'another', 'item 6' ) + + // Process log/default queue + const batch = await processBatch( 'log', 'default' ); + // ["item 1", "item 2", "item 3"] + + // Process log/default queue + processBatch( 'log', 'another' ); + // ["item 4", "item 5", "item 6"] + } + run(); +}, [] ); +``` + +Processors may also return promises which is useful for http requests: + +```js +registerProcessor( + 'api_fetch', + ( requests ) => { + return apiFetch( { + path: '/v1/batch', + method: 'POST', + data: { + validation: 'require-all-validate', + requests + } + } ).responses.map( ( { body } ) => body ); + }, + () => 10 +); +``` + +If one of the transactions fail, the subsequent ones may choose to short-circuit: + +```js +registerProcessor( + 'api_fetch', + ( requests, batch ) => { + if ( batch.state === STATE_ERROR ) { + throw { + code: 'transaction_failed', + data: { status: 500 }, + message: 'Transaction failed.', + }; + } + // return apiFetch(...) + }, + () => 10 +); + +// Later in the code: +const batch = await processBatch( 'api_fetch', 'default' ); +console.log( batch.state ); +// ERROR + +console.log( batch.exception ); +// { code: "http_404" } +``` + +If the transaction fails, the processor may also throw a list of per-input errors: + +```js +registerProcessor( + 'api_fetch', + async ( requests, batch ) => { + const response = await apiFetch( /* ... */ ); + + if ( response.failed ) { + // One or more sub-requests failed, let's gather error codes and messages + const errors = response.responses.map( ( itemResponse ) => { + // The REST API returns null if the request did not have an error. + return itemResponse === null + ? { + code: 'transaction_failed', + data: { status: 400 }, + message: __( + 'This item could not be saved because another item encountered an error when trying to save.' + ), + } + : itemResponse.body; + } ); + // Throw a list of errors + throw errors; + } + + return response.responses.map( ( { body } ) => body ); + }, + () => 10 +); + +// Later in the code: +const batch = await processBatch( 'api_fetch', 'default' ); +console.log( batch.state ); +// ERROR + +console.log( batch.sortedItemIds.map( id => batch.errors[id] ) ) +// [ { code: 'transaction_failed', /* ... */ }, { code: 'mysql_down', /* ... */ }, ] +``` diff --git a/packages/core-data/src/batch-processing/actions.js b/packages/core-data/src/batch-processing/actions.js index 0d7270e4374e8..8a428affa2210 100644 --- a/packages/core-data/src/batch-processing/actions.js +++ b/packages/core-data/src/batch-processing/actions.js @@ -86,7 +86,7 @@ export const processBatch = function* ( meta = {} ) { const batchId = uuid(); - const batchSize = yield getBatchSize(queue); + const batchSize = yield getBatchSize( queue ); yield prepareBatchForProcessing( queue, context, batchId, batchSize, meta ); const { transactions } = yield select( 'getBatch', batchId ); diff --git a/packages/core-data/src/batch-support.js b/packages/core-data/src/batch-support.js index 2aa17e6452157..532e839a1f3f7 100644 --- a/packages/core-data/src/batch-support.js +++ b/packages/core-data/src/batch-support.js @@ -16,8 +16,8 @@ import { dispatch } from '@wordpress/data'; import './batch-processing'; import { STATE_ERROR } from './batch-processing/constants'; -async function batchProcessor( requests, transaction ) { - if ( transaction.state === STATE_ERROR ) { +async function batchProcessor( requests, batch ) { + if ( batch.state === STATE_ERROR ) { throw { code: 'transaction_failed', data: { status: 500 },