From fd418fc41a93c4a4f8948edce7e488c8c508750f Mon Sep 17 00:00:00 2001 From: Kawika Avilla Date: Thu, 7 Dec 2023 01:11:40 +0000 Subject: [PATCH] [Cleanup] utilize the same helper function Originally when implementing the fix the historical comment caused concern about potential breaking changes. But after discussion, we decided it is more clear to consolidate the helper functions. Signed-off-by: Kawika Avilla --- .../custom_filter_matches_index.test.ts | 51 ------------------- .../custom_filter_matches_index.ts | 17 ------- .../filter_matches_index.test.ts | 14 +++++ .../opensearch_query/filter_matches_index.ts | 10 ++-- .../opensearch_query/from_filters.ts | 6 +-- 5 files changed, 20 insertions(+), 78 deletions(-) delete mode 100644 src/plugins/data/common/opensearch_query/opensearch_query/custom_filter_matches_index.test.ts delete mode 100644 src/plugins/data/common/opensearch_query/opensearch_query/custom_filter_matches_index.ts diff --git a/src/plugins/data/common/opensearch_query/opensearch_query/custom_filter_matches_index.test.ts b/src/plugins/data/common/opensearch_query/opensearch_query/custom_filter_matches_index.test.ts deleted file mode 100644 index 50a6084dafa6..000000000000 --- a/src/plugins/data/common/opensearch_query/opensearch_query/custom_filter_matches_index.test.ts +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import { Filter } from '../filters'; -import { customFilterMatchesIndex } from './custom_filter_matches_index'; -import { IIndexPattern } from '../../index_patterns'; - -describe('customFilterMatchesIndex', () => { - it('should return true if the custom filter has no meta', () => { - const filter = {} as Filter; - const indexPattern = { id: 'foo', fields: [{ name: 'bar' }] } as IIndexPattern; - - expect(customFilterMatchesIndex(filter, indexPattern)).toBe(true); - }); - - it('should return true if no index pattern is passed', () => { - const filter = { meta: { index: 'foo', key: 'bar', type: 'custom' } } as Filter; - - expect(customFilterMatchesIndex(filter, undefined)).toBe(true); - }); - - it('should return true if the custom filter has meta without a key', () => { - const filter = { meta: { index: 'foo', type: 'custom' } } as Filter; - const indexPattern = { id: 'foo', fields: [{ name: 'bar' }] } as IIndexPattern; - - expect(customFilterMatchesIndex(filter, indexPattern)).toBe(true); - }); - - it('should return false if the filter is not custom', () => { - const filter = { meta: { index: 'foo', key: 'bar', type: 'match_all' } } as Filter; - const indexPattern = { id: 'foo', fields: [{ name: 'bar' }] } as IIndexPattern; - - expect(customFilterMatchesIndex(filter, indexPattern)).toBe(false); - }); - - it('should return false if the custom filter is a different index id', () => { - const filter = { meta: { index: 'foo', key: 'bar', type: 'custom' } } as Filter; - const indexPattern = { id: 'bar', fields: [{ name: 'foo' }] } as IIndexPattern; - - expect(customFilterMatchesIndex(filter, indexPattern)).toBe(false); - }); - - it('should return true if the custom filter is the same index id', () => { - const filter = { meta: { index: 'foo', key: 'bar', type: 'custom' } } as Filter; - const indexPattern = { id: 'foo', fields: [{ name: 'barf' }] } as IIndexPattern; - - expect(customFilterMatchesIndex(filter, indexPattern)).toBe(true); - }); -}); diff --git a/src/plugins/data/common/opensearch_query/opensearch_query/custom_filter_matches_index.ts b/src/plugins/data/common/opensearch_query/opensearch_query/custom_filter_matches_index.ts deleted file mode 100644 index c98f6d1575a1..000000000000 --- a/src/plugins/data/common/opensearch_query/opensearch_query/custom_filter_matches_index.ts +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -import { IIndexPattern } from '../../index_patterns'; -import { Filter } from '../filters'; - -export function customFilterMatchesIndex(filter: Filter, indexPattern?: IIndexPattern | null) { - if (!filter.meta?.key || !indexPattern) { - return true; - } - if (filter.meta?.type !== 'custom') { - return false; - } - return filter.meta.index === indexPattern.id; -} diff --git a/src/plugins/data/common/opensearch_query/opensearch_query/filter_matches_index.test.ts b/src/plugins/data/common/opensearch_query/opensearch_query/filter_matches_index.test.ts index f610b1e7179f..ad68e14b2c54 100644 --- a/src/plugins/data/common/opensearch_query/opensearch_query/filter_matches_index.test.ts +++ b/src/plugins/data/common/opensearch_query/opensearch_query/filter_matches_index.test.ts @@ -66,4 +66,18 @@ describe('filterMatchesIndex', () => { expect(filterMatchesIndex(filter, indexPattern)).toBe(true); }); + + it('should return false if the custom filter is a different index id', () => { + const filter = { meta: { index: 'foo', key: 'bar', type: 'custom' } } as Filter; + const indexPattern = { id: 'bar', fields: [{ name: 'foo' }] } as IIndexPattern; + + expect(filterMatchesIndex(filter, indexPattern)).toBe(false); + }); + + it('should return true if the custom filter is the same index id', () => { + const filter = { meta: { index: 'foo', key: 'bar', type: 'custom' } } as Filter; + const indexPattern = { id: 'foo', fields: [{ name: 'barf' }] } as IIndexPattern; + + expect(filterMatchesIndex(filter, indexPattern)).toBe(true); + }); }); diff --git a/src/plugins/data/common/opensearch_query/opensearch_query/filter_matches_index.ts b/src/plugins/data/common/opensearch_query/opensearch_query/filter_matches_index.ts index f8c2ab67ee95..529e68609aeb 100644 --- a/src/plugins/data/common/opensearch_query/opensearch_query/filter_matches_index.ts +++ b/src/plugins/data/common/opensearch_query/opensearch_query/filter_matches_index.ts @@ -31,14 +31,14 @@ import { IIndexPattern, IFieldType } from '../../index_patterns'; import { Filter } from '../filters'; -/* - * TODO: We should base this on something better than `filter.meta.key`. We should probably modify - * this to check if `filter.meta.index` matches `indexPattern.id` instead, but that's a breaking - * change. - */ export function filterMatchesIndex(filter: Filter, indexPattern?: IIndexPattern | null) { if (!filter.meta?.key || !indexPattern) { return true; } + + if (filter.meta?.type === 'custom') { + return filter.meta.index === indexPattern.id; + } + return indexPattern.fields.some((field: IFieldType) => field.name === filter.meta.key); } diff --git a/src/plugins/data/common/opensearch_query/opensearch_query/from_filters.ts b/src/plugins/data/common/opensearch_query/opensearch_query/from_filters.ts index e7b13f5a10d6..990dca144abf 100644 --- a/src/plugins/data/common/opensearch_query/opensearch_query/from_filters.ts +++ b/src/plugins/data/common/opensearch_query/opensearch_query/from_filters.ts @@ -34,7 +34,6 @@ import { filterMatchesIndex } from './filter_matches_index'; import { Filter, cleanFilter, isFilterDisabled } from '../filters'; import { IIndexPattern } from '../../index_patterns'; import { handleNestedFilter } from './handle_nested_filter'; -import { customFilterMatchesIndex } from './custom_filter_matches_index'; /** * Create a filter that can be reversed for filters with negate set @@ -77,10 +76,7 @@ export const buildQueryFromFilters = ( return filters .filter(filterNegate(negate)) .filter( - (filter) => - !ignoreFilterIfFieldNotInIndex || - filterMatchesIndex(filter, indexPattern) || - customFilterMatchesIndex(filter, indexPattern) + (filter) => !ignoreFilterIfFieldNotInIndex || filterMatchesIndex(filter, indexPattern) ) .map((filter) => { return migrateFilter(filter, indexPattern);