Skip to content

Commit

Permalink
quick exceptions refactor removing some of the circular dependiness
Browse files Browse the repository at this point in the history
  • Loading branch information
@yctercero
yctercero committed Feb 23, 2021
1 parent b1d76ce commit 11e3396
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 11e3396

Please sign in to comment.