Allow partialFilterExpression in backbeat index add payload - #6274
Allow partialFilterExpression in backbeat index add payload#6274maeldonn wants to merge 1 commit into
Conversation
Since BB-773 (backbeat 9.4.1) the lifecycle conductor sends index specs with a partialFilterExpression, rejected as an unknown field by the Joi schema of POST /_/backbeat/index?operation=add. The v2 lifecycle indexes were never created. Allow the field and add route-level tests. Issue: CLDSRV-991
Hello maeldonn,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
@@ Coverage Diff @@
## development/9.4 #6274 +/- ##
================================================
Coverage 86.50% 86.50%
================================================
Files 212 212
Lines 14579 14579
================================================
Hits 12611 12611
Misses 1968 1968
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
delthas
left a comment
delthas
left a comment
There was a problem hiding this comment.
Oh wow, good catch 😅 Thanks!
| }), | ||
| ) | ||
| .required(), | ||
| partialFilterExpression: joi.object(), |
There was a problem hiding this comment.
I mean declare the schema ? Can be hard as it's mongo but pretty sure we can find some joi lib to validate mongo ?
There was a problem hiding this comment.
it is validated by mongo during the call, so is it worth duplicating the validation?
- we don't actually own the format, we just pass it through: so not sure we add value.
- it's fine not to own it, since this is an internal API only.
(the alternative would be to define our own "index definition format", and map it to mongo format in cloudserver : in that case we should indeed do validation)
| }); | ||
| }); | ||
|
|
||
| describe('routeBackbeat index add payload validation', () => { |
There was a problem hiding this comment.
Missing a test : should reject if the field is not well formatted ?
| }), | ||
| ) | ||
| .required(), | ||
| partialFilterExpression: joi.object(), |
There was a problem hiding this comment.
it is validated by mongo during the call, so is it worth duplicating the validation?
- we don't actually own the format, we just pass it through: so not sure we add value.
- it's fine not to own it, since this is an internal API only.
(the alternative would be to define our own "index definition format", and map it to mongo format in cloudserver : in that case we should indeed do validation)
| assert.strictEqual(err.code, 'BadRequest'); | ||
| assert.strictEqual(metadata.putBucketIndexes.called, false); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
do we have "integration" test, where we actually call mongo?
would be nice to have these, and thus test that we reject the call if either keys or partialFilterExpression is not a valid mongo format and that we pass the fields appropriately
| afterEach(() => { | ||
| sinon.restore(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
missing test where the request fails if backend is not mongo? or is it in some integration tests?
5 participants
Since BB-773 (backbeat >= 9.4.1) the lifecycle conductor sends its v2 index
specs with a
partialFilterExpression, rejected as an unknown field by theJoi schema of
POST /_/backbeat/index?operation=add. The v2 lifecycleindexes were never created and buckets fell back to v1 listings (BB-866).
Declare the field as an optional object (the rest of the chain already
forwarded it to MongoDB) and add route-level tests.
Issue: CLDSRV-991