Skip to content

Commit

Permalink
[Security Solution][Exceptions] - Update exceptions modal to use exis…
Browse files Browse the repository at this point in the history 
…ting lists plugin useApi hook (#92348)

Doing a quick refactor to help with #90634 . While working on #90634 found that I would introduce a circular dependency if I didn't refactor the hook used by the exception builder component to make use of the existing useApi hook in the lists plugin.

#90634 adds temporary ids to item entries to mitigate some React key requirements and the logic to remove/add these id's isn't at the boundary but in the hooks (so as not to pollute the data for everyone wanting to use the api.

An upside is that it removed some of the looping and seemed to speed things up a bit. I briefly considered adding the bulk SO endpoints for exception items, but they are still experimental.
  • Loading branch information
@yctercero
yctercero committed Feb 23, 2021
1 parent b1eac3c commit 226ed17
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 47 deletions.
63 changes: 62 additions & 1 deletion x-pack/plugins/lists/public/exceptions/hooks/use_api.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,20 @@

import { act, renderHook } from '@testing-library/react-hooks';

import { getUpdateExceptionListItemSchemaMock } from '../../../common/schemas/request/update_exception_list_item_schema.mock';
import { coreMock } from '../../../../../../src/core/public/mocks';
import * as api from '../api';
import { getExceptionListSchemaMock } from '../../../common/schemas/response/exception_list_schema.mock';
import { getFoundExceptionListItemSchemaMock } from '../../../common/schemas/response/found_exception_list_item_schema.mock';
import { getExceptionListItemSchemaMock } from '../../../common/schemas/response/exception_list_item_schema.mock';
import { getCreateExceptionListItemSchemaMock } from '../../../common/schemas/request/create_exception_list_item_schema.mock';
import { HttpStart } from '../../../../../../src/core/public';
import { ApiCallByIdProps, ApiCallByListIdProps } from '../types';
import {
AddExceptionListItemProps,
ApiCallByIdProps,
ApiCallByListIdProps,
UpdateExceptionListItemProps,
} from '../types';

import { ExceptionsApi, useApi } from './use_api';

Expand Down Expand Up @@ -366,4 +373,58 @@ describe('useApi', () => {
expect(onErrorMock).toHaveBeenCalledWith(mockError);
});
});

test('it invokes "addExceptionListItem" when "addExceptionListItem" used', async () => {
const payload = getExceptionListItemSchemaMock();
const itemToCreate = getCreateExceptionListItemSchemaMock();
const spyOnFetchExceptionListItemById = jest
.spyOn(api, 'addExceptionListItem')
.mockResolvedValue(payload);

await act(async () => {
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
useApi(mockKibanaHttpService)
);
await waitForNextUpdate();

await result.current.addExceptionListItem({
listItem: itemToCreate,
});

const expected: AddExceptionListItemProps = {
http: mockKibanaHttpService,
listItem: itemToCreate,
signal: new AbortController().signal,
};

expect(spyOnFetchExceptionListItemById).toHaveBeenCalledWith(expected);
});
});

test('it invokes "updateExceptionListItem" when "getExceptionItem" used', async () => {
const payload = getExceptionListItemSchemaMock();
const itemToUpdate = getUpdateExceptionListItemSchemaMock();
const spyOnUpdateExceptionListItem = jest
.spyOn(api, 'updateExceptionListItem')
.mockResolvedValue(payload);

await act(async () => {
const { result, waitForNextUpdate } = renderHook<HttpStart, ExceptionsApi>(() =>
useApi(mockKibanaHttpService)
);
await waitForNextUpdate();

await result.current.updateExceptionListItem({
listItem: itemToUpdate,
});

const expected: UpdateExceptionListItemProps = {
http: mockKibanaHttpService,
listItem: itemToUpdate,
signal: new AbortController().signal,
};

expect(spyOnUpdateExceptionListItem).toHaveBeenCalledWith(expected);
});
});
});
39 changes: 38 additions & 1 deletion x-pack/plugins/lists/public/exceptions/hooks/use_api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,22 @@ import { useMemo } from 'react';

import * as Api from '../api';
import { HttpStart } from '../../../../../../src/core/public';
import { ExceptionListItemSchema, ExceptionListSchema } from '../../../common/schemas';
import {
CreateExceptionListItemSchema,
ExceptionListItemSchema,
ExceptionListSchema,
UpdateExceptionListItemSchema,
} from '../../../common/schemas';
import { ApiCallFindListsItemsMemoProps, ApiCallMemoProps, ApiListExportProps } from '../types';
import { getIdsAndNamespaces } from '../utils';

export interface ExceptionsApi {
addExceptionListItem: (arg: {
listItem: CreateExceptionListItemSchema;
}) => Promise<ExceptionListItemSchema>;
updateExceptionListItem: (arg: {
listItem: UpdateExceptionListItemSchema;
}) => Promise<ExceptionListItemSchema>;
deleteExceptionItem: (arg: ApiCallMemoProps) => Promise<void>;
deleteExceptionList: (arg: ApiCallMemoProps) => Promise<void>;
getExceptionItem: (
Expand All @@ -29,6 +40,19 @@ export interface ExceptionsApi {
export const useApi = (http: HttpStart): ExceptionsApi => {
return useMemo(
(): ExceptionsApi => ({
async addExceptionListItem({
listItem,
}: {
listItem: CreateExceptionListItemSchema;
}): Promise<ExceptionListItemSchema> {
const abortCtrl = new AbortController();

return Api.addExceptionListItem({
http,
listItem,
signal: abortCtrl.signal,
});
},
async deleteExceptionItem({
id,
namespaceType,
Expand Down Expand Up @@ -184,6 +208,19 @@ export const useApi = (http: HttpStart): ExceptionsApi => {
onError(error);
}
},
async updateExceptionListItem({
listItem,
}: {
listItem: UpdateExceptionListItemSchema;
}): Promise<ExceptionListItemSchema> {
const abortCtrl = new AbortController();

return Api.updateExceptionListItem({
http,
listItem,
signal: abortCtrl.signal,
});
},
}),
[http]
);
Expand Down
74 changes: 29 additions & 45 deletions x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,9 @@ import { UpdateDocumentByQueryResponse } from 'elasticsearch';
import { HttpStart } from '../../../../../../../src/core/public';

import {
addExceptionListItem,
updateExceptionListItem,
ExceptionListItemSchema,
CreateExceptionListItemSchema,
UpdateExceptionListItemSchema,
useApi,
} from '../../../lists_plugin_deps';
import { updateAlertStatus } from '../../../detections/containers/detection_engine/alerts/api';
import { getUpdateAlertsQuery } from '../../../detections/components/alerts_table/actions';
Expand All @@ -25,6 +23,7 @@ import {
import { getQueryFilter } from '../../../../common/detection_engine/get_query_filter';
import { Index } from '../../../../common/detection_engine/schemas/common/schemas';
import { formatExceptionItemForUpdate, prepareExceptionItemsForBulkClose } from './helpers';
import { useKibana } from '../../lib/kibana';

/**
* Adds exception items to the list. Also optionally closes alerts.
Expand Down Expand Up @@ -66,11 +65,13 @@ export const useAddOrUpdateException = ({
onError,
onSuccess,
}: UseAddOrUpdateExceptionProps): ReturnUseAddOrUpdateException => {
const { services } = useKibana();
const [isLoading, setIsLoading] = useState(false);
const addOrUpdateExceptionRef = useRef<AddOrUpdateExceptionItemsFunc | null>(null);
const { addExceptionListItem, updateExceptionListItem } = useApi(services.http);
const addOrUpdateException = useCallback<AddOrUpdateExceptionItemsFunc>(
async (ruleId, exceptionItemsToAddOrUpdate, alertIdToClose, bulkCloseIndex) => {
if (addOrUpdateExceptionRef.current !== null) {
if (addOrUpdateExceptionRef.current != null) {
addOrUpdateExceptionRef.current(
ruleId,
exceptionItemsToAddOrUpdate,
Expand All @@ -86,49 +87,33 @@ export const useAddOrUpdateException = ({
let isSubscribed = true;
const abortCtrl = new AbortController();

const addOrUpdateItems = async (
exceptionItemsToAddOrUpdate: Array<ExceptionListItemSchema | CreateExceptionListItemSchema>
): Promise<void> => {
const toAdd: CreateExceptionListItemSchema[] = [];
const toUpdate: UpdateExceptionListItemSchema[] = [];
exceptionItemsToAddOrUpdate.forEach(
(item: ExceptionListItemSchema | CreateExceptionListItemSchema) => {
if ('id' in item && item.id !== undefined) {
toUpdate.push(formatExceptionItemForUpdate(item));
} else {
toAdd.push(item);
}
}
);

const promises: Array<Promise<ExceptionListItemSchema>> = [];
toAdd.forEach((item: CreateExceptionListItemSchema) => {
promises.push(
addExceptionListItem({
http,
listItem: item,
signal: abortCtrl.signal,
})
);
});
toUpdate.forEach((item: UpdateExceptionListItemSchema) => {
promises.push(
updateExceptionListItem({
http,
listItem: item,
signal: abortCtrl.signal,
})
);
});
await Promise.all(promises);
};

const addOrUpdateExceptionItems: AddOrUpdateExceptionItemsFunc = async (
const onUpdateExceptionItemsAndAlertStatus: AddOrUpdateExceptionItemsFunc = async (
ruleId,
exceptionItemsToAddOrUpdate,
alertIdToClose,
bulkCloseIndex
) => {
const addOrUpdateItems = async (
exceptionListItems: Array<ExceptionListItemSchema | CreateExceptionListItemSchema>
): Promise<void> => {
await Promise.all(
exceptionListItems.map(
(item: ExceptionListItemSchema | CreateExceptionListItemSchema) => {
if ('id' in item && item.id != null) {
const formattedExceptionItem = formatExceptionItemForUpdate(item);
return updateExceptionListItem({
listItem: formattedExceptionItem,
});
} else {
return addExceptionListItem({
listItem: item,
});
}
}
)
);
};

try {
setIsLoading(true);
let alertIdResponse: UpdateDocumentByQueryResponse | undefined;
Expand Down Expand Up @@ -170,7 +155,6 @@ export const useAddOrUpdateException = ({
const updated = (alertIdResponse?.updated ?? 0) + (bulkResponse?.updated ?? 0);
const conflicts =
alertIdResponse?.version_conflicts ?? 0 + (bulkResponse?.version_conflicts ?? 0);

if (isSubscribed) {
setIsLoading(false);
onSuccess(updated, conflicts);
Expand All @@ -187,12 +171,12 @@ export const useAddOrUpdateException = ({
}
};

addOrUpdateExceptionRef.current = addOrUpdateExceptionItems;
addOrUpdateExceptionRef.current = onUpdateExceptionItemsAndAlertStatus;
return (): void => {
isSubscribed = false;
abortCtrl.abort();
};
}, [http, onSuccess, onError]);
}, [http, onSuccess, onError, updateExceptionListItem, addExceptionListItem]);

return [{ isLoading }, addOrUpdateException];
};

0 comments on commit 226ed17

Please sign in to comment.