Skip to content

Commit

Permalink
fix dynamic config API calls to pass correct input (opensearch-projec…
Browse files Browse the repository at this point in the history
…t#6474)

* update dynamic API calls to pass correct input

Signed-off-by: Tianle Huang <[email protected]>

* add unit tests

Signed-off-by: Tianle Huang <[email protected]>

* add changelog

Signed-off-by: Tianle Huang <[email protected]>

* revert yml

Signed-off-by: Tianle Huang <[email protected]>

---------

Signed-off-by: Tianle Huang <[email protected]>
  • Loading branch information
tianleh authored Apr 16, 2024
1 parent 236ec4e commit 9a97b43
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 26 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
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
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
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

0 comments on commit 9a97b43

Please sign in to comment.