Skip to content

Commit

Permalink
[Security Solution][Exceptions] Adds autocomplete workaround for .tex…
Browse files Browse the repository at this point in the history
…t fields (#73761) (#73888)

## Summary

This PR provides a workaround for the autocomplete service not providing suggestions when the selected field is `someField.text`. As endpoint exceptions will be largely using `.text` for now, wanted to still provide the autocomplete service.

Updates to the autocomplete components were done after seeing some React errors that were popping up related to memory leaks. This is due to the use of `debounce` I believe. The calls were still executed even after the builder component was unmounted. This resulted in the subsequent calls from the autocomplete service not always going through (sometimes being canceled) when reopening the builder.

Moved the filtering of endpoint fields to occur in the existing helper function so that we would still have access to the corresponding `keyword` field of `text` fields when formatting the entries for the builder.
  • Loading branch information
yctercero authored Jul 30, 2020
1 parent b710f75 commit a82959b
Show file tree
Hide file tree
Showing 13 changed files with 540 additions and 142 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ describe('AutocompleteFieldMatchComponent', () => {
fields,
},
value: 'value 1',
signal: new AbortController().signal,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,13 @@ export const AutocompleteFieldMatchComponent: React.FC<AutocompleteFieldMatchPro
};

const onSearchChange = (searchVal: string): void => {
const signal = new AbortController().signal;

updateSuggestions({
fieldSelected: selectedField,
value: `${searchVal}`,
patterns: indexPattern,
signal,
});
if (updateSuggestions != null) {
updateSuggestions({
fieldSelected: selectedField,
value: searchVal,
patterns: indexPattern,
});
}
};

const isValid = useMemo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ describe('AutocompleteFieldMatchAnyComponent', () => {
fields,
},
value: 'value 1',
signal: new AbortController().signal,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,13 @@ export const AutocompleteFieldMatchAnyComponent: React.FC<AutocompleteFieldMatch
};

const onSearchChange = (searchVal: string) => {
const signal = new AbortController().signal;

updateSuggestions({
fieldSelected: selectedField,
value: `${searchVal}`,
patterns: indexPattern,
signal,
});
if (updateSuggestions != null) {
updateSuggestions({
fieldSelected: selectedField,
value: searchVal,
patterns: indexPattern,
});
}
};

const onCreateOption = (option: string) => onChange([...(selectedValue || []), option]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,17 @@ describe('useFieldValueAutocomplete', () => {
await waitForNextUpdate();
await waitForNextUpdate();

result.current[2]({
fieldSelected: getField('@tags'),
value: 'hello',
patterns: stubIndexPatternWithFields,
signal: new AbortController().signal,
});
expect(result.current[2]).not.toBeNull();

// Added check for typescripts sake, if null,
// would not reach below logic as test would stop above
if (result.current[2] != null) {
result.current[2]({
fieldSelected: getField('@tags'),
value: 'hello',
patterns: stubIndexPatternWithFields,
});
}

await waitForNextUpdate();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,13 @@ import { IFieldType, IIndexPattern } from '../../../../../../../../src/plugins/d
import { useKibana } from '../../../../common/lib/kibana';
import { OperatorTypeEnum } from '../../../../lists_plugin_deps';

export type UseFieldValueAutocompleteReturn = [
boolean,
string[],
(args: {
fieldSelected: IFieldType | undefined;
value: string | string[] | undefined;
patterns: IIndexPattern | undefined;
signal: AbortSignal;
}) => void
];
type Func = (args: {
fieldSelected: IFieldType | undefined;
value: string | string[] | undefined;
patterns: IIndexPattern | undefined;
}) => void;

export type UseFieldValueAutocompleteReturn = [boolean, string[], Func | null];

export interface UseFieldValueAutocompleteProps {
selectedField: IFieldType | undefined;
Expand All @@ -41,62 +38,77 @@ export const useFieldValueAutocomplete = ({
const { services } = useKibana();
const [isLoading, setIsLoading] = useState(false);
const [suggestions, setSuggestions] = useState<string[]>([]);
const updateSuggestions = useRef(
debounce(
const updateSuggestions = useRef<Func | null>(null);

useEffect(() => {
let isSubscribed = true;
const abortCtrl = new AbortController();

const fetchSuggestions = debounce(
async ({
fieldSelected,
value,
patterns,
signal,
}: {
fieldSelected: IFieldType | undefined;
value: string | string[] | undefined;
patterns: IIndexPattern | undefined;
signal: AbortSignal;
}) => {
if (fieldSelected == null || patterns == null) {
return;
}
const inputValue: string | string[] = value ?? '';
const userSuggestion: string = Array.isArray(inputValue)
? inputValue[inputValue.length - 1] ?? ''
: inputValue;

setIsLoading(true);
try {
if (isSubscribed) {
if (fieldSelected == null || patterns == null) {
return;
}

// Fields of type boolean should only display two options
if (fieldSelected.type === 'boolean') {
setIsLoading(false);
setSuggestions(['true', 'false']);
return;
}
setIsLoading(true);

const newSuggestions = await services.data.autocomplete.getValueSuggestions({
indexPattern: patterns,
field: fieldSelected,
query: '',
signal,
});
// Fields of type boolean should only display two options
if (fieldSelected.type === 'boolean') {
setIsLoading(false);
setSuggestions(['true', 'false']);
return;
}

setIsLoading(false);
setSuggestions(newSuggestions);
const newSuggestions = await services.data.autocomplete.getValueSuggestions({
indexPattern: patterns,
field: fieldSelected,
query: userSuggestion.trim(),
signal: abortCtrl.signal,
});

setIsLoading(false);
setSuggestions([...newSuggestions]);
}
} catch (error) {
if (isSubscribed) {
setSuggestions([]);
setIsLoading(false);
}
}
},
500
)
);

useEffect(() => {
const abortCtrl = new AbortController();
);

if (operatorType !== OperatorTypeEnum.EXISTS) {
updateSuggestions.current({
fetchSuggestions({
fieldSelected: selectedField,
value: fieldValue,
patterns: indexPattern,
signal: abortCtrl.signal,
});
}

updateSuggestions.current = fetchSuggestions;

return (): void => {
isSubscribed = false;
abortCtrl.abort();
};
}, [updateSuggestions, selectedField, operatorType, fieldValue, indexPattern]);
}, [services.data.autocomplete, selectedField, operatorType, fieldValue, indexPattern]);

return [isLoading, suggestions, updateSuggestions.current];
};
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand All @@ -81,6 +82,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand Down Expand Up @@ -111,6 +113,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand Down Expand Up @@ -143,6 +146,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand Down Expand Up @@ -175,6 +179,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand Down Expand Up @@ -207,6 +212,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand Down Expand Up @@ -239,6 +245,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand Down Expand Up @@ -271,6 +278,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand Down Expand Up @@ -306,6 +314,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand All @@ -331,6 +340,62 @@ describe('BuilderEntryItem', () => {
).toBeTruthy();
});

test('it uses "correspondingKeywordField" if it exists', () => {
const wrapper = mount(
<BuilderEntryItem
entry={{
field: {
name: 'extension.text',
type: 'string',
esTypes: ['text'],
count: 0,
scripted: false,
searchable: false,
aggregatable: false,
readFromDocValues: true,
},
operator: isOneOfOperator,
value: ['1234'],
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: {
name: 'extension',
type: 'string',
esTypes: ['keyword'],
count: 0,
scripted: false,
searchable: true,
aggregatable: true,
readFromDocValues: true,
},
}}
indexPattern={{
id: '1234',
title: 'logstash-*',
fields,
}}
showLabel={false}
listType="detection"
addNested={false}
onChange={jest.fn()}
/>
);

expect(
wrapper.find('[data-test-subj="exceptionBuilderEntryFieldMatchAny"]').prop('selectedField')
).toEqual({
name: 'extension',
type: 'string',
esTypes: ['keyword'],
count: 0,
scripted: false,
searchable: true,
aggregatable: true,
readFromDocValues: true,
});
});

test('it invokes "onChange" when new field is selected and resets operator and value fields', () => {
const mockOnChange = jest.fn();
const wrapper = mount(
Expand All @@ -342,6 +407,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand Down Expand Up @@ -376,6 +442,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand Down Expand Up @@ -410,6 +477,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand Down Expand Up @@ -444,6 +512,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand Down Expand Up @@ -478,6 +547,7 @@ describe('BuilderEntryItem', () => {
nested: undefined,
parent: undefined,
entryIndex: 0,
correspondingKeywordField: undefined,
}}
indexPattern={{
id: '1234',
Expand Down
Loading

0 comments on commit a82959b

Please sign in to comment.