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.

Sign up for GitHub

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

Jump to bottom

fix dynamic config API calls to pass correct input #6474

Merged
merged 4 commits into from
Apr 16, 2024
Merged

fix dynamic config API calls to pass correct input #6474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e808850
update dynamic API calls to pass correct input
tianleh Apr 16, 2024
2e05e25
add unit tests
tianleh Apr 16, 2024
4eee1c6
add changelog
tianleh Apr 16, 2024
ddbcc36
revert yml
tianleh Apr 16, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [BUG][Multiple Datasource]Read hideLocalCluster setting from yml and set in data source selector and data source menu ([#6361](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6361))
- [BUG][Multiple Datasource] Refactor read-only component to cover more edge cases ([#6416](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6416))
- [BUG] Fix for checkForFunctionProperty so that order does not matter ([#6248](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6248))
- [Dynamic Configurations] Fix dynamic config API calls to pass correct input ([#6474](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6474))
- [BUG][Multiple Datasource] Validation succeed as long as status code in response is 200 ([#6399](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6399))
- [BUG][Multiple Datasource] Add validation for title length to be no longer than 32 characters [#6452](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6452))

Expand Down
86 changes: 78 additions & 8 deletions src/plugins/application_config/server/routes/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ describe('application config routes', () => {
getConfig: jest.fn().mockReturnValue(configurations),
};

const getConfigurationClient = jest.fn().mockReturnValue(client);

const request = {};

const okResponse = {
Expand All @@ -91,7 +93,12 @@ describe('application config routes', () => {

const logger = loggerMock.create();

const returnedResponse = await handleGetConfig(client, request, response, logger);
const returnedResponse = await handleGetConfig(
getConfigurationClient,
request,
response,
logger
);

expect(returnedResponse).toBe(okResponse);

Expand All @@ -100,6 +107,8 @@ describe('application config routes', () => {
value: configurations,
},
});

expect(getConfigurationClient).toBeCalledWith(request);
});

it('return error response when client throws error', async () => {
Expand All @@ -111,6 +120,8 @@ describe('application config routes', () => {
}),
};

const getConfigurationClient = jest.fn().mockReturnValue(client);

const request = {};

const response = {
Expand All @@ -119,7 +130,12 @@ describe('application config routes', () => {

const logger = loggerMock.create();

const returnedResponse = await handleGetConfig(client, request, response, logger);
const returnedResponse = await handleGetConfig(
getConfigurationClient,
request,
response,
logger
);

expect(returnedResponse).toBe(ERROR_RESPONSE);

Expand All @@ -131,6 +147,7 @@ describe('application config routes', () => {
});

expect(logger.error).toBeCalledWith(error);
expect(getConfigurationClient).toBeCalledWith(request);
});
});

Expand All @@ -140,6 +157,8 @@ describe('application config routes', () => {
getEntityConfig: jest.fn().mockReturnValue(ENTITY_VALUE),
};

const getConfigurationClient = jest.fn().mockReturnValue(client);

const okResponse = {
statusCode: 200,
};
Expand All @@ -156,7 +175,12 @@ describe('application config routes', () => {

const logger = loggerMock.create();

const returnedResponse = await handleGetEntityConfig(client, request, response, logger);
const returnedResponse = await handleGetEntityConfig(
getConfigurationClient,
request,
response,
logger
);

expect(returnedResponse).toBe(okResponse);

Expand All @@ -165,6 +189,8 @@ describe('application config routes', () => {
value: ENTITY_VALUE,
},
});

expect(getConfigurationClient).toBeCalledWith(request);
});

it('return error response when client throws error', async () => {
Expand All @@ -176,6 +202,8 @@ describe('application config routes', () => {
}),
};

const getConfigurationClient = jest.fn().mockReturnValue(client);

const request = {
params: {
entity: ENTITY_NAME,
Expand All @@ -188,7 +216,12 @@ describe('application config routes', () => {

const logger = loggerMock.create();

const returnedResponse = await handleGetEntityConfig(client, request, response, logger);
const returnedResponse = await handleGetEntityConfig(
getConfigurationClient,
request,
response,
logger
);

expect(returnedResponse).toBe(ERROR_RESPONSE);

Expand All @@ -200,6 +233,8 @@ describe('application config routes', () => {
});

expect(logger.error).toBeCalledWith(error);

expect(getConfigurationClient).toBeCalledWith(request);
});
});

Expand All @@ -209,6 +244,8 @@ describe('application config routes', () => {
updateEntityConfig: jest.fn().mockReturnValue(ENTITY_NEW_VALUE),
};

const getConfigurationClient = jest.fn().mockReturnValue(client);

const okResponse = {
statusCode: 200,
};
Expand All @@ -228,7 +265,12 @@ describe('application config routes', () => {

const logger = loggerMock.create();

const returnedResponse = await handleUpdateEntityConfig(client, request, response, logger);
const returnedResponse = await handleUpdateEntityConfig(
getConfigurationClient,
request,
response,
logger
);

expect(returnedResponse).toBe(okResponse);

Expand All @@ -241,6 +283,8 @@ describe('application config routes', () => {
});

expect(logger.error).not.toBeCalled();

expect(getConfigurationClient).toBeCalledWith(request);
});

it('return error response when client fails', async () => {
Expand All @@ -252,6 +296,8 @@ describe('application config routes', () => {
}),
};

const getConfigurationClient = jest.fn().mockReturnValue(client);

const request = {
params: {
entity: ENTITY_NAME,
Expand All @@ -267,7 +313,12 @@ describe('application config routes', () => {

const logger = loggerMock.create();

const returnedResponse = await handleUpdateEntityConfig(client, request, response, logger);
const returnedResponse = await handleUpdateEntityConfig(
getConfigurationClient,
request,
response,
logger
);

expect(returnedResponse).toBe(ERROR_RESPONSE);

Expand All @@ -279,6 +330,8 @@ describe('application config routes', () => {
});

expect(logger.error).toBeCalledWith(error);

expect(getConfigurationClient).toBeCalledWith(request);
});
});

Expand All @@ -288,6 +341,8 @@ describe('application config routes', () => {
deleteEntityConfig: jest.fn().mockReturnValue(ENTITY_NAME),
};

const getConfigurationClient = jest.fn().mockReturnValue(client);

const okResponse = {
statusCode: 200,
};
Expand All @@ -304,7 +359,12 @@ describe('application config routes', () => {

const logger = loggerMock.create();

const returnedResponse = await handleDeleteEntityConfig(client, request, response, logger);
const returnedResponse = await handleDeleteEntityConfig(
getConfigurationClient,
request,
response,
logger
);

expect(returnedResponse).toBe(okResponse);

Expand All @@ -317,6 +377,7 @@ describe('application config routes', () => {
});

expect(logger.error).not.toBeCalled();
expect(getConfigurationClient).toBeCalledWith(request);
});

it('return error response when client fails', async () => {
Expand All @@ -328,6 +389,8 @@ describe('application config routes', () => {
}),
};

const getConfigurationClient = jest.fn().mockReturnValue(client);

const request = {
params: {
entity: ENTITY_NAME,
Expand All @@ -340,7 +403,12 @@ describe('application config routes', () => {

const logger = loggerMock.create();

const returnedResponse = await handleDeleteEntityConfig(client, request, response, logger);
const returnedResponse = await handleDeleteEntityConfig(
getConfigurationClient,
request,
response,
logger
);

expect(returnedResponse).toBe(ERROR_RESPONSE);

Expand All @@ -352,6 +420,8 @@ describe('application config routes', () => {
});

expect(logger.error).toBeCalledWith(error);

expect(getConfigurationClient).toBeCalledWith(request);
});
});
});
35 changes: 17 additions & 18 deletions src/plugins/application_config/server/routes/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import { schema } from '@osd/config-schema';
import {
IRouter,
IScopedClusterClient,
Logger,
OpenSearchDashboardsRequest,
OpenSearchDashboardsResponseFactory,
Expand All @@ -15,7 +14,7 @@ import { ConfigurationClient } from '../types';

export function defineRoutes(
router: IRouter,
getConfigurationClient: (configurationClient: IScopedClusterClient) => ConfigurationClient,
getConfigurationClient: (request?: OpenSearchDashboardsRequest) => ConfigurationClient,
logger: Logger
) {
router.get(
Expand All @@ -24,9 +23,7 @@ export function defineRoutes(
validate: false,
},
async (context, request, response) => {
const client = getConfigurationClient(context.core.opensearch.client);

return await handleGetConfig(client, request, response, logger);
return await handleGetConfig(getConfigurationClient, request, response, logger);
}
);
router.get(
Expand All @@ -39,9 +36,7 @@ export function defineRoutes(
},
},
async (context, request, response) => {
const client = getConfigurationClient(context.core.opensearch.client);

return await handleGetEntityConfig(client, request, response, logger);
return await handleGetEntityConfig(getConfigurationClient, request, response, logger);
}
);
router.post(
Expand All @@ -57,9 +52,7 @@ export function defineRoutes(
},
},
async (context, request, response) => {
const client = getConfigurationClient(context.core.opensearch.client);

return await handleUpdateEntityConfig(client, request, response, logger);
return await handleUpdateEntityConfig(getConfigurationClient, request, response, logger);
}
);
router.delete(
Expand All @@ -72,21 +65,21 @@ export function defineRoutes(
},
},
async (context, request, response) => {
const client = getConfigurationClient(context.core.opensearch.client);

return await handleDeleteEntityConfig(client, request, response, logger);
return await handleDeleteEntityConfig(getConfigurationClient, request, response, logger);
}
);
}

export async function handleGetEntityConfig(
client: ConfigurationClient,
getConfigurationClient: (request?: OpenSearchDashboardsRequest) => ConfigurationClient,
request: OpenSearchDashboardsRequest,
response: OpenSearchDashboardsResponseFactory,
logger: Logger
) {
logger.info(`Received a request to get entity config for ${request.params.entity}.`);

const client = getConfigurationClient(request);

try {
const result = await client.getEntityConfig(request.params.entity, {
headers: request.headers,
Expand All @@ -103,7 +96,7 @@ export async function handleGetEntityConfig(
}

export async function handleUpdateEntityConfig(
client: ConfigurationClient,
getConfigurationClient: (request?: OpenSearchDashboardsRequest) => ConfigurationClient,
request: OpenSearchDashboardsRequest,
response: OpenSearchDashboardsResponseFactory,
logger: Logger
Expand All @@ -112,6 +105,8 @@ export async function handleUpdateEntityConfig(
`Received a request to update entity ${request.params.entity} with new value ${request.body.newValue}.`
);

const client = getConfigurationClient(request);

try {
const result = await client.updateEntityConfig(request.params.entity, request.body.newValue, {
headers: request.headers,
Expand All @@ -128,13 +123,15 @@ export async function handleUpdateEntityConfig(
}

export async function handleDeleteEntityConfig(
client: ConfigurationClient,
getConfigurationClient: (request?: OpenSearchDashboardsRequest) => ConfigurationClient,
request: OpenSearchDashboardsRequest,
response: OpenSearchDashboardsResponseFactory,
logger: Logger
) {
logger.info(`Received a request to delete entity ${request.params.entity}.`);

const client = getConfigurationClient(request);

try {
const result = await client.deleteEntityConfig(request.params.entity, {
headers: request.headers,
Expand All @@ -151,13 +148,15 @@ export async function handleDeleteEntityConfig(
}

export async function handleGetConfig(
client: ConfigurationClient,
getConfigurationClient: (request?: OpenSearchDashboardsRequest) => ConfigurationClient,
request: OpenSearchDashboardsRequest,
response: OpenSearchDashboardsResponseFactory,
logger: Logger
) {
logger.info('Received a request to get all configurations.');

const client = getConfigurationClient(request);

try {
const result = await client.getConfig({ headers: request.headers });
return response.ok({
Expand Down
Loading