Skip to content

Commit

Permalink
Addressed comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@JiaweiWu
JiaweiWu committed Apr 13, 2023
1 parent 6381964 commit d65b6a9
Show file tree
Hide file tree
Showing 22 changed files with 162 additions and 171 deletions.
2 changes: 1 addition & 1 deletion ...e/server/integration_tests/saved_objects/migrations/group2/check_registered_types.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ describe('checking migration metadata changes on all registered SO types', () =>
"legacy-url-alias": "9b8cca3fbb2da46fd12823d3cd38fdf1c9f24bc8",
"lens": "2f6a8231591e3d62a83506b19e165774d74588ea",
"lens-ui-telemetry": "d6c4e330d170eefc6214dbf77a53de913fa3eebc",
"maintenance-window": "2cb13a3c4b7a58e9557f962286b9afd9156bf0f8",
"maintenance-window": "a9777f4e71381c56b4422bf8d30f626bde301c79",
"map": "7902b2e2a550e0b73fd5aa6c4e2ba3a4e6558877",
"metrics-explorer-view": "713dbf1ab5e067791d19170f715eb82cf07ebbcc",
"ml-job": "12e21f1b1adfcc1052dc0b10c7459de875653b94",
Expand Down
23 changes: 22 additions & 1 deletion ...k/plugins/alerting/server/maintenance_window_client/generate_maintenance_window_events.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,30 @@ export const mergeEvents = ({
}) => {
// If new events have more entries (expiration date got pushed), we merge the old into the new
if (newEvents.length > oldEvents.length) {
return [...oldEvents, ...newEvents.slice(-(newEvents.length - oldEvents.length))];
const modifiedEvents: Array<{ event: DateRange; index: number }> = [];

// Find all modified events in the old events array
oldEvents.forEach((oldEvent, index) => {
const newEvent = newEvents[index];
if (oldEvent.lte !== newEvent.lte || oldEvent.gte !== newEvent.gte) {
modifiedEvents.push({
event: oldEvent,
index,
});
}
});

const newEventsCopy = [...newEvents];

// Update the new event array with old modified events
modifiedEvents.forEach(({ event, index }) => {
newEventsCopy[index] = event;
});

return newEventsCopy;
}
// If new events have less entries (maintenance window got archived), we trim the old events
// to match the same length as the new events
if (oldEvents.length > newEvents.length) {
return oldEvents.slice(0, newEvents.length);
}
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/alerting/server/maintenance_window_client/maintenance_window_client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { Logger, SavedObjectsClientContract } from '@kbn/core/server';
import { create, CreateParams } from './methods/create';
import { get, GetParams } from './methods/get';
import { update, UpdateParams } from './methods/update';
import { find, FindParams, FindResult } from './methods/find';
import { find, FindResult } from './methods/find';
import { deleteMaintenanceWindow, DeleteParams } from './methods/delete';
import { archive, ArchiveParams } from './methods/archive';
import {
Expand Down Expand Up @@ -64,7 +64,7 @@ export class MaintenanceWindowClient {
public get = (params: GetParams): Promise<MaintenanceWindow> => get(this.context, params);
public update = (params: UpdateParams): Promise<MaintenanceWindow> =>
update(this.context, params);
public find = (params: FindParams): Promise<FindResult> => find(this.context, params);
public find = (): Promise<FindResult> => find(this.context);
public delete = (params: DeleteParams): Promise<{}> =>
deleteMaintenanceWindow(this.context, params);
public archive = (params: ArchiveParams): Promise<MaintenanceWindow> =>
Expand Down
4 changes: 1 addition & 3 deletions x-pack/plugins/alerting/server/maintenance_window_client/methods/find.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import { find } from './find';
import { fromKueryExpression } from '@kbn/es-query';
import { savedObjectsClientMock, loggingSystemMock } from '@kbn/core/server/mocks';
import { SavedObjectsFindResponse } from '@kbn/core/server';
import {
Expand Down Expand Up @@ -48,11 +47,10 @@ describe('MaintenanceWindowClient - find', () => {
],
} as unknown as SavedObjectsFindResponse);

const result = await find(mockContext, { filter: 'title: test' });
const result = await find(mockContext);

expect(savedObjectsClient.find).toHaveBeenLastCalledWith({
type: MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,
filter: fromKueryExpression('title: test'),
});

expect(result.data.length).toEqual(2);
Expand Down
15 changes: 2 additions & 13 deletions x-pack/plugins/alerting/server/maintenance_window_client/methods/find.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import Boom from '@hapi/boom';
import { buildKueryNodeFilter } from '../../rules_client/common';
import { getMaintenanceWindowFromRaw } from '../get_maintenance_window_from_raw';
import {
MaintenanceWindowSOAttributes,
Expand All @@ -15,26 +14,16 @@ import {
MaintenanceWindowClientContext,
} from '../../../common';

export interface FindParams {
filter?: string;
}

export interface FindResult {
data: MaintenanceWindow[];
}

export async function find(
context: MaintenanceWindowClientContext,
params: FindParams
): Promise<FindResult> {
export async function find(context: MaintenanceWindowClientContext): Promise<FindResult> {
const { savedObjectsClient, logger } = context;
const { filter } = params;
const filterKueryNode = buildKueryNodeFilter(filter);

try {
const result = await savedObjectsClient.find<MaintenanceWindowSOAttributes>({
type: MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,
filter: filterKueryNode,
});

return {
Expand All @@ -46,7 +35,7 @@ export async function find(
),
};
} catch (e) {
const errorMessage = `Failed to find maintenance window: Filter: ${filter}, Error: ${e}`;
const errorMessage = `Failed to find maintenance window, Error: ${e}`;
logger.error(errorMessage);
throw Boom.boomify(e, { message: errorMessage });
}
Expand Down
14 changes: 7 additions & 7 deletions x-pack/plugins/alerting/server/routes/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ import { getFlappingSettingsRoute } from './get_flapping_settings';
import { updateFlappingSettingsRoute } from './update_flapping_settings';
import { getRuleTagsRoute } from './get_rule_tags';

import { createMaintenanceWindowRoute } from './create_maintenance_window';
import { getMaintenanceWindowRoute } from './get_maintenance_window';
import { updateMaintenanceWindowRoute } from './update_maintenance_window';
import { deleteMaintenanceWindowRoute } from './delete_maintenance_window';
import { findMaintenanceWindowsRoute } from './find_maintenance_windows';
import { archiveMaintenanceWindowRoute } from './archive_maintenance_window';
import { finishMaintenanceWindowRoute } from './finish_maintenance_window';
import { createMaintenanceWindowRoute } from './maintenance_window/create_maintenance_window';
import { getMaintenanceWindowRoute } from './maintenance_window/get_maintenance_window';
import { updateMaintenanceWindowRoute } from './maintenance_window/update_maintenance_window';
import { deleteMaintenanceWindowRoute } from './maintenance_window/delete_maintenance_window';
import { findMaintenanceWindowsRoute } from './maintenance_window/find_maintenance_windows';
import { archiveMaintenanceWindowRoute } from './maintenance_window/archive_maintenance_window';
import { finishMaintenanceWindowRoute } from './maintenance_window/finish_maintenance_window';

export interface RouteOptions {
router: IRouter<AlertingRequestHandlerContext>;
Expand Down
16 changes: 8 additions & 8 deletions ...routes/archive_maintenance_window.test.ts → ...window/archive_maintenance_window.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@
*/

import { httpServiceMock } from '@kbn/core/server/mocks';
import { licenseStateMock } from '../lib/license_state.mock';
import { verifyApiAccess } from '../lib/license_api_access';
import { mockHandlerArguments } from './_mock_handler_arguments';
import { maintenanceWindowClientMock } from '../maintenance_window_client.mock';
import { licenseStateMock } from '../../lib/license_state.mock';
import { verifyApiAccess } from '../../lib/license_api_access';
import { mockHandlerArguments } from '../_mock_handler_arguments';
import { maintenanceWindowClientMock } from '../../maintenance_window_client.mock';
import { archiveMaintenanceWindowRoute } from './archive_maintenance_window';
import { getMockMaintenanceWindow } from '../maintenance_window_client/methods/test_helpers';
import { MaintenanceWindowStatus } from '../../common';
import { rewritePartialMaintenanceBodyRes } from './lib';
import { getMockMaintenanceWindow } from '../../maintenance_window_client/methods/test_helpers';
import { MaintenanceWindowStatus } from '../../../common';
import { rewritePartialMaintenanceBodyRes } from '../lib';

const maintenanceWindowClient = maintenanceWindowClientMock.create();

jest.mock('../lib/license_api_access', () => ({
jest.mock('../../lib/license_api_access', () => ({
verifyApiAccess: jest.fn(),
}));

Expand Down
8 changes: 4 additions & 4 deletions ...rver/routes/archive_maintenance_window.ts → ...ance_window/archive_maintenance_window.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

import { IRouter } from '@kbn/core/server';
import { schema } from '@kbn/config-schema';
import { ILicenseState } from '../lib';
import { verifyAccessAndContext, rewritePartialMaintenanceBodyRes } from './lib';
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../types';
import { MAINTENANCE_WINDOW_API_PRIVILEGES } from '../../common';
import { ILicenseState } from '../../lib';
import { verifyAccessAndContext, rewritePartialMaintenanceBodyRes } from '../lib';
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../../types';
import { MAINTENANCE_WINDOW_API_PRIVILEGES } from '../../../common';

const paramSchema = schema.object({
id: schema.string(),
Expand Down
16 changes: 8 additions & 8 deletions .../routes/create_maintenance_window.test.ts → ..._window/create_maintenance_window.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@
*/

import { httpServiceMock } from '@kbn/core/server/mocks';
import { licenseStateMock } from '../lib/license_state.mock';
import { verifyApiAccess } from '../lib/license_api_access';
import { mockHandlerArguments } from './_mock_handler_arguments';
import { maintenanceWindowClientMock } from '../maintenance_window_client.mock';
import { licenseStateMock } from '../../lib/license_state.mock';
import { verifyApiAccess } from '../../lib/license_api_access';
import { mockHandlerArguments } from '../_mock_handler_arguments';
import { maintenanceWindowClientMock } from '../../maintenance_window_client.mock';
import { createMaintenanceWindowRoute, rewriteQueryReq } from './create_maintenance_window';
import { getMockMaintenanceWindow } from '../maintenance_window_client/methods/test_helpers';
import { MaintenanceWindowStatus } from '../../common';
import { rewritePartialMaintenanceBodyRes } from './lib';
import { getMockMaintenanceWindow } from '../../maintenance_window_client/methods/test_helpers';
import { MaintenanceWindowStatus } from '../../../common';
import { rewritePartialMaintenanceBodyRes } from '../lib';

const maintenanceWindowClient = maintenanceWindowClientMock.create();

jest.mock('../lib/license_api_access', () => ({
jest.mock('../../lib/license_api_access', () => ({
verifyApiAccess: jest.fn(),
}));

Expand Down
8 changes: 4 additions & 4 deletions ...erver/routes/create_maintenance_window.ts → ...nance_window/create_maintenance_window.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@

import { IRouter } from '@kbn/core/server';
import { schema } from '@kbn/config-schema';
import { ILicenseState } from '../lib';
import { ILicenseState } from '../../lib';
import {
verifyAccessAndContext,
rRuleSchema,
RewriteRequestCase,
rewriteMaintenanceWindowRes,
} from './lib';
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../types';
import { MaintenanceWindowSOProperties, MAINTENANCE_WINDOW_API_PRIVILEGES } from '../../common';
} from '../lib';
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../../types';
import { MaintenanceWindowSOProperties, MAINTENANCE_WINDOW_API_PRIVILEGES } from '../../../common';

const bodySchema = schema.object({
title: schema.string(),
Expand Down
14 changes: 7 additions & 7 deletions .../routes/delete_maintenance_window.test.ts → ..._window/delete_maintenance_window.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@
*/

import { httpServiceMock } from '@kbn/core/server/mocks';
import { licenseStateMock } from '../lib/license_state.mock';
import { verifyApiAccess } from '../lib/license_api_access';
import { mockHandlerArguments } from './_mock_handler_arguments';
import { maintenanceWindowClientMock } from '../maintenance_window_client.mock';
import { licenseStateMock } from '../../lib/license_state.mock';
import { verifyApiAccess } from '../../lib/license_api_access';
import { mockHandlerArguments } from '../_mock_handler_arguments';
import { maintenanceWindowClientMock } from '../../maintenance_window_client.mock';
import { deleteMaintenanceWindowRoute } from './delete_maintenance_window';
import { getMockMaintenanceWindow } from '../maintenance_window_client/methods/test_helpers';
import { MaintenanceWindowStatus } from '../../common';
import { getMockMaintenanceWindow } from '../../maintenance_window_client/methods/test_helpers';
import { MaintenanceWindowStatus } from '../../../common';

const maintenanceWindowClient = maintenanceWindowClientMock.create();

jest.mock('../lib/license_api_access', () => ({
jest.mock('../../lib/license_api_access', () => ({
verifyApiAccess: jest.fn(),
}));

Expand Down
8 changes: 4 additions & 4 deletions ...erver/routes/delete_maintenance_window.ts → ...nance_window/delete_maintenance_window.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

import { IRouter } from '@kbn/core/server';
import { schema } from '@kbn/config-schema';
import { ILicenseState } from '../lib';
import { verifyAccessAndContext } from './lib';
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../types';
import { MAINTENANCE_WINDOW_API_PRIVILEGES } from '../../common';
import { ILicenseState } from '../../lib';
import { verifyAccessAndContext } from '../lib';
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../../types';
import { MAINTENANCE_WINDOW_API_PRIVILEGES } from '../../../common';

const paramSchema = schema.object({
id: schema.string(),
Expand Down
41 changes: 15 additions & 26 deletions ...r/routes/find_maintenance_windows.test.ts → ...e_window/find_maintenance_windows.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@
*/

import { httpServiceMock } from '@kbn/core/server/mocks';
import { licenseStateMock } from '../lib/license_state.mock';
import { verifyApiAccess } from '../lib/license_api_access';
import { mockHandlerArguments } from './_mock_handler_arguments';
import { maintenanceWindowClientMock } from '../maintenance_window_client.mock';
import { licenseStateMock } from '../../lib/license_state.mock';
import { verifyApiAccess } from '../../lib/license_api_access';
import { mockHandlerArguments } from '../_mock_handler_arguments';
import { maintenanceWindowClientMock } from '../../maintenance_window_client.mock';
import { findMaintenanceWindowsRoute } from './find_maintenance_windows';
import { getMockMaintenanceWindow } from '../maintenance_window_client/methods/test_helpers';
import { MaintenanceWindowStatus } from '../../common';
import { rewriteMaintenanceWindowRes } from './lib';
import { getMockMaintenanceWindow } from '../../maintenance_window_client/methods/test_helpers';
import { MaintenanceWindowStatus } from '../../../common';
import { rewriteMaintenanceWindowRes } from '../lib';

const maintenanceWindowClient = maintenanceWindowClientMock.create();

jest.mock('../lib/license_api_access', () => ({
jest.mock('../../lib/license_api_access', () => ({
verifyApiAccess: jest.fn(),
}));

Expand Down Expand Up @@ -52,20 +52,15 @@ describe('findMaintenanceWindowsRoute', () => {
findMaintenanceWindowsRoute(router, licenseState);

maintenanceWindowClient.find.mockResolvedValueOnce(mockMaintenanceWindows);
const [config, handler] = router.post.mock.calls[0];
const [context, req, res] = mockHandlerArguments(
{ maintenanceWindowClient },
{ body: { filter: 'maintenance-window.attributes.title: test-title' } }
);
const [config, handler] = router.get.mock.calls[0];
const [context, req, res] = mockHandlerArguments({ maintenanceWindowClient }, { body: {} });

expect(config.path).toEqual('/internal/alerting/rules/maintenance_window/_find');
expect(config.options?.tags?.[0]).toEqual('access:read-maintenance-window');

await handler(context, req, res);

expect(maintenanceWindowClient.find).toHaveBeenLastCalledWith({
filter: 'maintenance-window.attributes.title: test-title',
});
expect(maintenanceWindowClient.find).toHaveBeenCalled();
expect(res.ok).toHaveBeenLastCalledWith({
body: {
data: mockMaintenanceWindows.data.map((data) => rewriteMaintenanceWindowRes(data)),
Expand All @@ -81,11 +76,8 @@ describe('findMaintenanceWindowsRoute', () => {
findMaintenanceWindowsRoute(router, licenseState);

maintenanceWindowClient.find.mockResolvedValueOnce(mockMaintenanceWindows);
const [, handler] = router.post.mock.calls[0];
const [context, req, res] = mockHandlerArguments(
{ maintenanceWindowClient },
{ body: { filter: 'maintenance-window.attributes.title: test-title' } }
);
const [, handler] = router.get.mock.calls[0];
const [context, req, res] = mockHandlerArguments({ maintenanceWindowClient }, { body: {} });
await handler(context, req, res);
expect(verifyApiAccess).toHaveBeenCalledWith(licenseState);
});
Expand All @@ -99,11 +91,8 @@ describe('findMaintenanceWindowsRoute', () => {
(verifyApiAccess as jest.Mock).mockImplementation(() => {
throw new Error('Failure');
});
const [, handler] = router.post.mock.calls[0];
const [context, req, res] = mockHandlerArguments(
{ maintenanceWindowClient },
{ body: { filter: 'maintenance-window.attributes.title: test-title' } }
);
const [, handler] = router.get.mock.calls[0];
const [context, req, res] = mockHandlerArguments({ maintenanceWindowClient }, { body: {} });
expect(handler(context, req, res)).rejects.toMatchInlineSnapshot(`[Error: Failure]`);
});
});
21 changes: 7 additions & 14 deletions ...server/routes/find_maintenance_windows.ts → ...enance_window/find_maintenance_windows.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,27 @@
*/

import { IRouter } from '@kbn/core/server';
import { schema } from '@kbn/config-schema';
import { ILicenseState } from '../lib';
import { verifyAccessAndContext, rewriteMaintenanceWindowRes } from './lib';
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../types';
import { MAINTENANCE_WINDOW_API_PRIVILEGES } from '../../common';

const bodySchema = schema.object({
filter: schema.maybe(schema.string()),
});
import { ILicenseState } from '../../lib';
import { verifyAccessAndContext, rewriteMaintenanceWindowRes } from '../lib';
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../../types';
import { MAINTENANCE_WINDOW_API_PRIVILEGES } from '../../../common';

export const findMaintenanceWindowsRoute = (
router: IRouter<AlertingRequestHandlerContext>,
licenseState: ILicenseState
) => {
router.post(
router.get(
{
path: `${INTERNAL_BASE_ALERTING_API_PATH}/rules/maintenance_window/_find`,
validate: {
body: bodySchema,
},
validate: {},
options: {
tags: [`access:${MAINTENANCE_WINDOW_API_PRIVILEGES.READ_MAINTENANCE_WINDOW}`],
},
},
router.handleLegacyErrors(
verifyAccessAndContext(licenseState, async function (context, req, res) {
const maintenanceWindowClient = (await context.alerting).getMaintenanceWindowClient();
const result = await maintenanceWindowClient.find(req.body);
const result = await maintenanceWindowClient.find();

return res.ok({
body: {
Expand Down
Loading

0 comments on commit d65b6a9

Please sign in to comment.