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

Fix generic events on search queries #1698

Merged
merged 3 commits into from
Jul 2, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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 doc/2/api/essentials/error-codes/core/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ description: error codes definitions
| core.fatal.service_unavailable<br/><pre>0x00000002</pre> | [ExternalServiceError](/core/2/api/essentials/error-handling#externalserviceerror) <pre>(500)</pre> | An external service is unavailable |
| core.fatal.service_timeout<br/><pre>0x00000003</pre> | [InternalError](/core/2/api/essentials/error-handling#internalerror) <pre>(500)</pre> | Service initialization timeout |
| core.fatal.unreadable_log_dir<br/><pre>0x00000004</pre> | [InternalError](/core/2/api/essentials/error-handling#internalerror) <pre>(500)</pre> | Cannot read the content of the log directory |
| core.fatal.assertion_failed<br/><pre>0x00000005</pre> | [InternalError](/core/2/api/essentials/error-handling#internalerror) <pre>(500)</pre> | A runtime assertion has failed. Please contact support. |

---

Expand Down
7 changes: 7 additions & 0 deletions doc/2/plugins/guides/events/generic-document-events/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ As with other API events, generic ones are triggered before and after documents

All generic events share the same payload signature, and pipes plugged to them must resolve to the updated (or not) array of documents received in their parameters.

::: info

"before" actions are only triggered on queries asking for specific documents. API routes such as `search`, `deleteByQuery` or `updateByQuery` cannot trigger "before" events, they only trigger "after" ones.

:::

---

## generic:document:afterDelete
Expand Down Expand Up @@ -153,6 +159,7 @@ class PipePlugin {

- [document:update](/core/2/api/controllers/document/update)
- [document:mUpdate](/core/2/api/controllers/document/m-update)
- [document:updateByQuery](/core/2/api/controllers/document/update-by-query)

---

Expand Down
46 changes: 40 additions & 6 deletions lib/api/documentExtractor.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@

'use strict';

const assertionError = require('../kerror').wrap('api', 'assert');
const kerror = require('../kerror');

const assertionError = kerror.wrap('api', 'assert');

const extractors = ([
{
methods: {
Expand Down Expand Up @@ -154,7 +157,27 @@ const extractors = ([
return request;
}
},
targets: ['mCreate', 'mCreateOrReplace', 'mReplace', 'mUpdate']
targets: ['mCreate', 'mCreateOrReplace', 'mReplace', 'mUpdate', 'updateByQuery']
},
{
methods: {
extractFromRequest: null, // not eligible for search queries
extractFromResult: request => {
return request.result.hits || request.result.documents;
},
insertInRequest: null, // not eligible for search queries
insertInResult: (documents, request) => {
if (request.input.action === 'search') {
request.result.hits = documents;
}
else if (request.input.action === 'deleteByQuery') {
request.result.documents = documents;
}

return request;
},
},
targets: ['search', 'deleteByQuery'],
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought updateByQuery was not eligible either?

Copy link
Contributor Author

@scottinet scottinet Jun 30, 2020

Choose a reason for hiding this comment

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

updateByQuery is handled by the same extractor than mCreate, mReplace, etc. because they share the same format.

The blacklist is done in the documentEventAliases.js file: https://github.com/kuzzleio/kuzzle/pull/1698/files#diff-cda9b5cb3d677fb4f72bd365d3318910

So the extractFromRequest and insertInRequest extractor methods cannot be invoked for updateByQuery.

},
{
methods: {
Expand All @@ -170,7 +193,7 @@ const extractors = ([
return request;
}
},
targets: ['create', 'createOrReplace', 'replace', 'update', '_default']
targets: ['create', 'createOrReplace', 'replace', 'update']
}
]).reduce((acc, extractor) => {
extractor.targets.forEach(target => {
Expand All @@ -183,9 +206,20 @@ class DocumentExtractor {
constructor(request) {
this.request = request;

const extractor = extractors[request.input.action] || extractors._default;
this.extractMethod = extractor[request.result ? 'extractFromResult' : 'extractFromRequest'];
this.insertMethod = extractor[request.result ? 'insertInResult' : 'insertInRequest'];
const extractor = extractors[request.input.action];

if (extractor === undefined) {
throw kerror.get('core', 'fatal', 'assertion_failed', `no generic documents extractor for ${request.input.action}`);
}

if (request.result) {
this.extractMethod = extractor.extractFromResult;
this.insertMethod = extractor.insertInResult;
}
else {
this.extractMethod = extractor.extractFromRequest;
this.insertMethod = extractor.insertInRequest;
}
}

extract() {
Expand Down
7 changes: 3 additions & 4 deletions lib/api/funnel.js
Original file line number Diff line number Diff line change
Expand Up @@ -486,10 +486,9 @@ class Funnel {
return request;
}

const
alias = this.documentEventAliases.mirrorList[action],
event = `${this.documentEventAliases.namespace}:${prefix}${capitalize(alias)}`,
extractor = new DocumentExtractor(request);
const alias = this.documentEventAliases.mirrorList[action];
const event = `${this.documentEventAliases.namespace}:${prefix}${capitalize(alias)}`;
const extractor = new DocumentExtractor(request);

const documents = await this.kuzzle.pipe(event, extractor.extract(), request);

Expand Down
2 changes: 1 addition & 1 deletion lib/config/documentEventAliases.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,5 @@ module.exports = {
'write': ['create', 'createOrReplace', 'mCreate', 'mCreateOrReplace', 'mReplace', 'replace']
},
namespace: 'generic:document',
notBefore: ['search', 'deleteByQuery']
notBefore: ['search', 'deleteByQuery', 'updateByQuery'],
};
7 changes: 6 additions & 1 deletion lib/kerror/codes/core.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,13 @@
"code": 4,
"message": "Cannot read log directory '%s' : %s.",
"class": "InternalError"
},
"assertion_failed": {
"description": "A runtime assertion has failed. Please contact support.",
"code": 5,
"message": "Runtime assertion failed: %s",
"class": "InternalError"
}

}
},
"realtime": {
Expand Down
Loading