Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ResponseOps][MaintenanceWindow] Introduce pagination for MW find API #197172

Merged
merged 22 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
be73cbb
introduce pagination for MW find API
guskovaue Oct 21, 2024
8a84cc2
add transmorm function
guskovaue Oct 22, 2024
815bfe4
Merge branch 'main' into MX-193076-MX-pagination-backend
guskovaue Oct 22, 2024
3e40644
add integration test for pagination
guskovaue Oct 22, 2024
1995414
fix types and unit tests after changes
guskovaue Oct 22, 2024
6009cd0
nit
guskovaue Oct 24, 2024
32f87a6
fix eslint
guskovaue Oct 24, 2024
a6d02f0
Merge branch 'main' into MX-193076-MX-pagination-backend
guskovaue Oct 24, 2024
cbb3185
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Oct 24, 2024
f1e2e49
fixes after code review
guskovaue Oct 25, 2024
9d4850b
Merge branch 'MX-193076-MX-pagination-backend' of github.com:guskovau…
guskovaue Oct 25, 2024
1d20bee
more fixes after code review
guskovaue Oct 25, 2024
2698ca5
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Oct 25, 2024
7427123
fix integration test
guskovaue Oct 25, 2024
8b83618
Merge branch 'MX-193076-MX-pagination-backend' of github.com:guskovau…
guskovaue Oct 25, 2024
d53050b
new integration test for validation max docs number
guskovaue Oct 25, 2024
f06b777
fix linter
guskovaue Oct 28, 2024
4efdc44
Merge branch 'main' into MX-193076-MX-pagination-backend
guskovaue Oct 28, 2024
f522466
remove fake timer
guskovaue Oct 29, 2024
d18c5c6
Merge branch 'main' into MX-193076-MX-pagination-backend
guskovaue Oct 29, 2024
84bddd0
Merge branch 'main' into MX-193076-MX-pagination-backend
guskovaue Oct 30, 2024
c2cd520
remove ts expect error block
guskovaue Oct 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@
* 2.0.
*/

export type { FindMaintenanceWindowsResponse } from './types/latest';
export {
findMaintenanceWindowsRequestQuerySchema,
findMaintenanceWindowsResponseBodySchema,
} from './schemas/latest';
export type {
FindMaintenanceWindowsRequestQuery,
FindMaintenanceWindowsResponse,
} from './types/latest';

export type { FindMaintenanceWindowsResponse as FindMaintenanceWindowsResponseV1 } from './types/v1';
export {
findMaintenanceWindowsRequestQuerySchema as findMaintenanceWindowsRequestQuerySchemaV1,
findMaintenanceWindowsResponseBodySchema as findMaintenanceWindowsResponseBodySchemaV1,
} from './schemas/v1';
export type {
FindMaintenanceWindowsRequestQuery as FindMaintenanceWindowsRequestQueryV1,
FindMaintenanceWindowsResponse as FindMaintenanceWindowsResponseV1,
} from './types/v1';
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export * from './v1';
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { schema } from '@kbn/config-schema';
import { maintenanceWindowResponseSchemaV1 } from '../../../response';

const MAX_DOCS = 10000;

export const findMaintenanceWindowsRequestQuerySchema = schema.object(
{
page: schema.maybe(
schema.number({
defaultValue: 1,
min: 1,
max: MAX_DOCS,
meta: {
description: 'The page number to return.',
},
})
),
per_page: schema.maybe(
schema.number({
defaultValue: 20,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: Could you please move all the numbers of min, max and defaultValue to const same as MAX_DOCS_PER_PAGE for consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing. I renamed MAX_DOCS_PER_PAGE to MAX_DOCS, because it make more sense.
I thought about your suggestion and decided to keep as it is. Because for this one constant it make sense to be assign to variable, because we reuse it. But others are not reusable and it's a lot of them.

min: 0,
max: 100,
meta: {
description: 'The number of maintenance windows to return per page.',
},
})
),
},
{
validate: (params) => {
const pageAsNumber = params.page ?? 0;
const perPageAsNumber = params.per_page ?? 0;

if (Math.max(pageAsNumber, pageAsNumber * perPageAsNumber) > MAX_DOCS) {
return `The number of documents is too high. Paginating through more than ${MAX_DOCS} documents is not possible.`;
}
},
}
);

export const findMaintenanceWindowsResponseBodySchema = schema.object({
page: schema.number(),
per_page: schema.number(),
total: schema.number(),
data: schema.arrayOf(maintenanceWindowResponseSchemaV1),
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
* 2.0.
*/

export type { FindMaintenanceWindowsResponse } from './v1';
export * from './v1';
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@
* 2.0.
*/

import { MaintenanceWindowResponseV1 } from '../../../response';
import { TypeOf } from '@kbn/config-schema';
import {
findMaintenanceWindowsResponseBodySchema,
findMaintenanceWindowsRequestQuerySchema,
} from '..';

export interface FindMaintenanceWindowsResponse {
body: {
data: MaintenanceWindowResponseV1[];
total: number;
};
}
export type FindMaintenanceWindowsResponse = TypeOf<
typeof findMaintenanceWindowsResponseBodySchema
>;
export type FindMaintenanceWindowsRequestQuery = TypeOf<
typeof findMaintenanceWindowsRequestQuerySchema
>;
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { schema } from '@kbn/config-schema';
import { maintenanceWindowStatusV1 } from '..';
import { maintenanceWindowStatus as maintenanceWindowStatusV1 } from '../constants/v1';
import { maintenanceWindowCategoryIdsSchemaV1 } from '../../shared';
import { rRuleResponseSchemaV1 } from '../../../r_rule';
import { alertsFilterQuerySchemaV1 } from '../../../alerts_filter_query';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export async function findMaintenanceWindows({
}: {
http: HttpSetup;
}): Promise<MaintenanceWindow[]> {
const res = await http.get<FindMaintenanceWindowsResponse['body']>(
const res = await http.get<FindMaintenanceWindowsResponse>(
`${INTERNAL_BASE_ALERTING_API_PATH}/rules/maintenance_window/_find`
);
return res.data.map((mw) => transformMaintenanceWindowResponse(mw));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,
} from '../../../../../common';
import { getMockMaintenanceWindow } from '../../../../data/maintenance_window/test_helpers';
import { findMaintenanceWindowsParamsSchema } from './schemas';

const savedObjectsClient = savedObjectsClientMock.create();
const uiSettings = uiSettingsServiceMock.createClient();
Expand All @@ -37,8 +38,47 @@ describe('MaintenanceWindowClient - find', () => {
jest.useRealTimers();
});

it('throws an error if page is string', async () => {
savedObjectsClient.find.mockResolvedValueOnce({
saved_objects: [
{
attributes: getMockMaintenanceWindow({ expirationDate: new Date().toISOString() }),
id: 'test-1',
},
{
attributes: getMockMaintenanceWindow({ expirationDate: new Date().toISOString() }),
id: 'test-2',
},
],
page: 1,
per_page: 5,
} as unknown as SavedObjectsFindResponse);

await expect(
// @ts-expect-error: testing validation of strings
findMaintenanceWindows(mockContext, { page: 'dfsd', perPage: 10 })
).rejects.toThrowErrorMatchingInlineSnapshot(
'"Error validating find maintenance windows data - [page]: expected value of type [number] but got [string]"'
);
});

it('throws an error if savedObjectsClient.find will throw an error', async () => {
jest.useFakeTimers().setSystemTime(new Date('2023-02-26T00:00:00.000Z'));

savedObjectsClient.find.mockImplementation(() => {
throw new Error('something went wrong!');
});

await expect(
findMaintenanceWindows(mockContext, { page: 1, perPage: 10 })
).rejects.toThrowErrorMatchingInlineSnapshot(
'"Failed to find maintenance window, Error: Error: something went wrong!: something went wrong!"'
);
});

it('should find maintenance windows', async () => {
jest.useFakeTimers().setSystemTime(new Date('2023-02-26T00:00:00.000Z'));
const spy = jest.spyOn(findMaintenanceWindowsParamsSchema, 'validate');

savedObjectsClient.find.mockResolvedValueOnce({
saved_objects: [
Expand All @@ -51,16 +91,21 @@ describe('MaintenanceWindowClient - find', () => {
id: 'test-2',
},
],
page: 1,
per_page: 5,
} as unknown as SavedObjectsFindResponse);

const result = await findMaintenanceWindows(mockContext);
const result = await findMaintenanceWindows(mockContext, {});

expect(spy).toHaveBeenCalledWith({});
expect(savedObjectsClient.find).toHaveBeenLastCalledWith({
type: MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,
});

expect(result.data.length).toEqual(2);
expect(result.data[0].id).toEqual('test-1');
expect(result.data[1].id).toEqual('test-2');
expect(result.page).toEqual(1);
expect(result.perPage).toEqual(5);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,35 @@ import Boom from '@hapi/boom';
import { MaintenanceWindowClientContext } from '../../../../../common';
import { transformMaintenanceWindowAttributesToMaintenanceWindow } from '../../transforms';
import { findMaintenanceWindowSo } from '../../../../data/maintenance_window';
import type { FindMaintenanceWindowsResult } from './types';
import type { FindMaintenanceWindowsResult, FindMaintenanceWindowsParams } from './types';
import { findMaintenanceWindowsParamsSchema } from './schemas';

export async function findMaintenanceWindows(
context: MaintenanceWindowClientContext
context: MaintenanceWindowClientContext,
params?: FindMaintenanceWindowsParams
): Promise<FindMaintenanceWindowsResult> {
const { savedObjectsClient, logger } = context;

try {
const result = await findMaintenanceWindowSo({ savedObjectsClient });
if (params) {
findMaintenanceWindowsParamsSchema.validate(params);
}
} catch (error) {
throw Boom.badRequest(`Error validating find maintenance windows data - ${error.message}`);
}

try {
const result = await findMaintenanceWindowSo({
savedObjectsClient,
...(params
? { savedObjectsFindOptions: { page: params.page, perPage: params.perPage } }
: {}),
});

return {
page: result.page,
perPage: result.per_page,
total: result.total,
data: result.saved_objects.map((so) =>
transformMaintenanceWindowAttributesToMaintenanceWindow({
attributes: so.attributes,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { schema } from '@kbn/config-schema';

export const findMaintenanceWindowsParamsSchema = schema.object({
perPage: schema.maybe(schema.number()),
page: schema.maybe(schema.number()),
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,8 @@ import { schema } from '@kbn/config-schema';
import { maintenanceWindowSchema } from '../../../schemas';

export const findMaintenanceWindowsResultSchema = schema.object({
page: schema.number(),
perPage: schema.number(),
data: schema.arrayOf(maintenanceWindowSchema),
total: schema.number(),
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
*/

export { findMaintenanceWindowsResultSchema } from './find_maintenance_windows_result_schema';
export { findMaintenanceWindowsParamsSchema } from './find_maintenance_window_params_schema';
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { TypeOf } from '@kbn/config-schema';
import { findMaintenanceWindowsParamsSchema } from '../schemas';

export type FindMaintenanceWindowsParams = TypeOf<typeof findMaintenanceWindowsParamsSchema>;
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
*/

export type { FindMaintenanceWindowsResult } from './find_maintenance_window_result';
export type { FindMaintenanceWindowsParams } from './find_maintenance_window_params';
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const findMaintenanceWindowSo = <MaintenanceWindowAggregation = Record<st
const { savedObjectsClient, savedObjectsFindOptions } = params;

return savedObjectsClient.find<MaintenanceWindowAttributes, MaintenanceWindowAggregation>({
...savedObjectsFindOptions,
...(savedObjectsFindOptions ? savedObjectsFindOptions : {}),
type: MAINTENANCE_WINDOW_SAVED_OBJECT_TYPE,
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ import type { GetMaintenanceWindowParams } from '../application/maintenance_wind
import { updateMaintenanceWindow } from '../application/maintenance_window/methods/update/update_maintenance_window';
import type { UpdateMaintenanceWindowParams } from '../application/maintenance_window/methods/update/types';
import { findMaintenanceWindows } from '../application/maintenance_window/methods/find/find_maintenance_windows';
import type { FindMaintenanceWindowsResult } from '../application/maintenance_window/methods/find/types';
import type {
FindMaintenanceWindowsResult,
FindMaintenanceWindowsParams,
} from '../application/maintenance_window/methods/find/types';
import { deleteMaintenanceWindow } from '../application/maintenance_window/methods/delete/delete_maintenance_window';
import type { DeleteMaintenanceWindowParams } from '../application/maintenance_window/methods/delete/types';
import { archiveMaintenanceWindow } from '../application/maintenance_window/methods/archive/archive_maintenance_window';
Expand Down Expand Up @@ -75,7 +78,8 @@ export class MaintenanceWindowClient {
getMaintenanceWindow(this.context, params);
public update = (params: UpdateMaintenanceWindowParams): Promise<MaintenanceWindow> =>
updateMaintenanceWindow(this.context, params);
public find = (): Promise<FindMaintenanceWindowsResult> => findMaintenanceWindows(this.context);
public find = (params?: FindMaintenanceWindowsParams): Promise<FindMaintenanceWindowsResult> =>
findMaintenanceWindows(this.context, params);
public delete = (params: DeleteMaintenanceWindowParams): Promise<{}> =>
deleteMaintenanceWindow(this.context, params);
public archive = (params: ArchiveMaintenanceWindowParams): Promise<MaintenanceWindow> =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ jest.mock('../../../../lib/license_api_access', () => ({
}));

const mockMaintenanceWindows = {
page: 1,
perPage: 3,
total: 2,
data: [
{
...getMockMaintenanceWindow(),
Expand Down Expand Up @@ -67,11 +70,54 @@ describe('findMaintenanceWindowsRoute', () => {

await handler(context, req, res);

expect(maintenanceWindowClient.find).toHaveBeenCalled();
expect(maintenanceWindowClient.find).toHaveBeenCalledWith({});
expect(res.ok).toHaveBeenLastCalledWith({
body: {
data: mockMaintenanceWindows.data.map((data) => rewriteMaintenanceWindowRes(data)),
total: 2,
page: 1,
per_page: 3,
},
});
});

test('should find the maintenance windows with query', async () => {
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();

findMaintenanceWindowsRoute(router, licenseState);

maintenanceWindowClient.find.mockResolvedValueOnce(mockMaintenanceWindows);
const [config, handler] = router.get.mock.calls[0];
const [context, req, res] = mockHandlerArguments(
{ maintenanceWindowClient },
{
query: {
page: 1,
per_page: 3,
},
}
);

expect(config.path).toEqual('/internal/alerting/rules/maintenance_window/_find');
expect(config.options).toMatchInlineSnapshot(`
Object {
"access": "internal",
"tags": Array [
"access:read-maintenance-window",
],
}
`);

await handler(context, req, res);

expect(maintenanceWindowClient.find).toHaveBeenCalledWith({ page: 1, perPage: 3 });
expect(res.ok).toHaveBeenLastCalledWith({
body: {
data: mockMaintenanceWindows.data.map((data) => rewriteMaintenanceWindowRes(data)),
total: 2,
page: 1,
per_page: 3,
},
});
});
Expand Down
Loading