-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[index patterns] Add pattern validation method to index patterns fetcher #90170
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
01b9979
add pattern validation method to index patterns fetcher
stephmilovic 3d99bde
if no pattern valid, throw error
stephmilovic 8788337
Merge branch 'master' into validate_patterns
stephmilovic bc4c790
may it please CI
stephmilovic 8ed8cf7
change to count method
stephmilovic f744acb
pr change
stephmilovic 9b717c9
get rid of some imports
stephmilovic df0f7c9
Add catch for error thrown on non-wildcard non-existing patterns
stephmilovic File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
24 changes: 24 additions & 0 deletions
24
...na-plugin-plugins-data-server.indexpatternsfetcher.validatepatternlistactive.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<!-- Do not edit this file. It is automatically generated by API Documenter. --> | ||
|
||
[Home](./index.md) > [kibana-plugin-plugins-data-server](./kibana-plugin-plugins-data-server.md) > [IndexPatternsFetcher](./kibana-plugin-plugins-data-server.indexpatternsfetcher.md) > [validatePatternListActive](./kibana-plugin-plugins-data-server.indexpatternsfetcher.validatepatternlistactive.md) | ||
|
||
## IndexPatternsFetcher.validatePatternListActive() method | ||
|
||
Returns an index pattern list of only those index pattern strings in the given list that return indices | ||
|
||
<b>Signature:</b> | ||
|
||
```typescript | ||
validatePatternListActive(patternList: string[]): Promise<string[]>; | ||
``` | ||
|
||
## Parameters | ||
|
||
| Parameter | Type | Description | | ||
| --- | --- | --- | | ||
| patternList | <code>string[]</code> | | | ||
|
||
<b>Returns:</b> | ||
|
||
`Promise<string[]>` | ||
|
72 changes: 72 additions & 0 deletions
72
src/plugins/data/server/index_patterns/fetcher/index_patterns_fetcher.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { IndexPatternsFetcher } from '.'; | ||
import { ElasticsearchClient } from 'kibana/server'; | ||
import * as indexNotFoundException from '../../../common/search/test_data/index_not_found_exception.json'; | ||
|
||
describe('Index Pattern Fetcher - server', () => { | ||
let indexPatterns: IndexPatternsFetcher; | ||
let esClient: ElasticsearchClient; | ||
const emptyResponse = { | ||
body: { | ||
count: 0, | ||
}, | ||
}; | ||
const response = { | ||
body: { | ||
count: 1115, | ||
}, | ||
}; | ||
const patternList = ['a', 'b', 'c']; | ||
beforeEach(() => { | ||
esClient = ({ | ||
count: jest.fn().mockResolvedValueOnce(emptyResponse).mockResolvedValue(response), | ||
} as unknown) as ElasticsearchClient; | ||
indexPatterns = new IndexPatternsFetcher(esClient); | ||
}); | ||
|
||
it('Removes pattern without matching indices', async () => { | ||
const result = await indexPatterns.validatePatternListActive(patternList); | ||
expect(result).toEqual(['b', 'c']); | ||
}); | ||
|
||
it('Returns all patterns when all match indices', async () => { | ||
esClient = ({ | ||
count: jest.fn().mockResolvedValue(response), | ||
} as unknown) as ElasticsearchClient; | ||
indexPatterns = new IndexPatternsFetcher(esClient); | ||
const result = await indexPatterns.validatePatternListActive(patternList); | ||
expect(result).toEqual(patternList); | ||
}); | ||
it('Removes pattern when "index_not_found_exception" error is thrown', async () => { | ||
class ServerError extends Error { | ||
public body?: Record<string, any>; | ||
constructor( | ||
message: string, | ||
public readonly statusCode: number, | ||
errBody?: Record<string, any> | ||
) { | ||
super(message); | ||
this.body = errBody; | ||
} | ||
} | ||
|
||
esClient = ({ | ||
count: jest | ||
.fn() | ||
.mockResolvedValueOnce(response) | ||
.mockRejectedValue( | ||
new ServerError('index_not_found_exception', 404, indexNotFoundException) | ||
), | ||
} as unknown) as ElasticsearchClient; | ||
indexPatterns = new IndexPatternsFetcher(esClient); | ||
const result = await indexPatterns.validatePatternListActive(patternList); | ||
expect(result).toEqual([patternList[0]]); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,16 +58,24 @@ export class IndexPatternsFetcher { | |
rollupIndex?: string; | ||
}): Promise<FieldDescriptor[]> { | ||
const { pattern, metaFields, fieldCapsOptions, type, rollupIndex } = options; | ||
const patternList = Array.isArray(pattern) ? pattern : pattern.split(','); | ||
let patternListActive: string[] = patternList; | ||
// if only one pattern, don't bother with validation. We let getFieldCapabilities fail if the single pattern is bad regardless | ||
if (patternList.length > 1) { | ||
patternListActive = await this.validatePatternListActive(patternList); | ||
} | ||
const fieldCapsResponse = await getFieldCapabilities( | ||
this.elasticsearchClient, | ||
pattern, | ||
// if none of the patterns are active, pass the original list to get an error | ||
patternListActive.length > 0 ? patternListActive : patternList, | ||
metaFields, | ||
{ | ||
allow_no_indices: fieldCapsOptions | ||
? fieldCapsOptions.allow_no_indices | ||
: this.allowNoIndices, | ||
} | ||
); | ||
|
||
if (type === 'rollup' && rollupIndex) { | ||
const rollupFields: FieldDescriptor[] = []; | ||
const rollupIndexCapabilities = getCapabilitiesForRollupIndices( | ||
|
@@ -118,4 +126,34 @@ export class IndexPatternsFetcher { | |
} | ||
return await getFieldCapabilities(this.elasticsearchClient, indices, metaFields); | ||
} | ||
|
||
/** | ||
* Returns an index pattern list of only those index pattern strings in the given list that return indices | ||
* | ||
* @param patternList string[] | ||
* @return {Promise<string[]>} | ||
*/ | ||
async validatePatternListActive(patternList: string[]) { | ||
const result = await Promise.all( | ||
patternList | ||
.map((pattern) => | ||
this.elasticsearchClient.count({ | ||
index: pattern, | ||
}) | ||
) | ||
.map((p) => | ||
p.catch((e) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the |
||
if (e.body.error.type === 'index_not_found_exception') { | ||
return { body: { count: 0 } }; | ||
} | ||
throw e; | ||
}) | ||
) | ||
); | ||
return result.reduce( | ||
(acc: string[], { body: { count } }, patternListIndex) => | ||
count > 0 ? [...acc, patternList[patternListIndex]] : acc, | ||
[] | ||
); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if none of the patterns are active, pass the original list to get an error:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to add this as a comment to the code. It definitely makes sense but its not obvious why this is being done.