Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propose batch saveEntityRecords action #27241

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/designers-developers/developers/data/data-core.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

<a name="undo" href="#undo">#</a> **undo**

Expand Down
1 change: 1 addition & 0 deletions packages/core-data/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

<a name="undo" href="#undo">#</a> **undo**

Expand Down
87 changes: 70 additions & 17 deletions packages/core-data/src/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -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 } );
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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 ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check seems weird, why sometimes we have a key and other times a record

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're editing an existing record then there is a key. If we're creating a new record then there is no 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 ) )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need a random timeout here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is that it addresses an issue similar to what I outlined in #27173 (comment). Rungen hands execution back to __experimentalBatchSaveEntityRecords which calls processBatch before addToBatch has been called.

Probably we should replace this setTimeout with something akin to await waitUntilNItemsAreEnqueued( spec.length ).

);
return yield controls.dispatch(
'core/__experimental-batch-processing',
'processBatch',
'API_FETCH'
);
}

/**
* Returns an action object used in signalling that Upload permissions have been received.
*
Expand Down
185 changes: 185 additions & 0 deletions packages/core-data/src/batch-processing/README.md
Original file line number Diff line number Diff line change
@@ -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', /* ... */ }, ]
```
Loading